Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GraphicRepresentation types according to latest Mesh Export API docs #7570

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
17 changes: 4 additions & 13 deletions common/api/frontend-tiles.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,27 +80,18 @@ export type GraphicRepresentation = {
format: GraphicRepresentationFormat;
dataSource: DataSource;
} & ({
status: Omit<GraphicRepresentationStatus, GraphicRepresentationStatus.Complete>;
status: Exclude<GraphicRepresentationStatus, "Complete">;
url?: string;
} | {
status: GraphicRepresentationStatus.Complete;
status: "Complete";
url: string;
});

// @beta
export type GraphicRepresentationFormat = "IMDL" | "3DTILES" | string;
export type GraphicRepresentationFormat = "3DFT" | "3DTiles" | "CESIUM" | "IMODEL" | string;

// @beta
export enum GraphicRepresentationStatus {
// (undocumented)
Complete = "Complete",
// (undocumented)
Failed = "Failed",
// (undocumented)
InProgress = "In progress",
// (undocumented)
NotStarted = "Not started"
}
export type GraphicRepresentationStatus = "Complete" | "InProgress" | "Invalid" | "NotStarted";

// @beta
export function initializeFrontendTiles(options: FrontendTilesOptions): void;
Expand Down
2 changes: 1 addition & 1 deletion common/api/summary/frontend-tiles.exports.csv
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ alpha;interface;GetGeoscienceTilesetArgs
alpha;function;getGeoscienceTilesetUrl
beta;type;GraphicRepresentation
beta;type;GraphicRepresentationFormat
beta;enum;GraphicRepresentationStatus
beta;type;GraphicRepresentationStatus
beta;function;initializeFrontendTiles
beta;interface;MeshExport
internal;interface;MeshExports
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/frontend-tiles",
"comment": "Update GraphicRepresentationFormat type and GraphicRepresentationStatus enum to reflect the latest Mesh Export Service docs.",
"type": "none"
}
],
"packageName": "@itwin/frontend-tiles"
}
6 changes: 3 additions & 3 deletions extensions/frontend-tiles/src/FrontendTiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { IModelApp, IModelConnection, SpatialTileTreeReferences, SpatialViewState } from "@itwin/core-frontend";
import { createBatchedSpatialTileTreeReferences } from "./BatchedSpatialTileTreeRefs";
import { queryGraphicRepresentations } from "./GraphicsProvider/GraphicRepresentationProvider";
import { queryGraphicRepresentations, QueryGraphicRepresentationsArgs } from "./GraphicsProvider/GraphicRepresentationProvider";
import { AccessToken } from "@itwin/core-bentley";
import { obtainIModelTilesetUrl, ObtainIModelTilesetUrlArgs } from "./GraphicsProvider/GraphicsProvider";

Expand Down Expand Up @@ -83,7 +83,7 @@ export interface QueryMeshExportsArgs {
* @beta
*/
export async function* queryMeshExports(args: QueryMeshExportsArgs): AsyncIterableIterator<MeshExport> {
const graphicsArgs = {
const graphicsArgs: QueryGraphicRepresentationsArgs = {
accessToken: args.accessToken,
sessionId: IModelApp.sessionId,
dataSource: {
Expand All @@ -92,7 +92,7 @@ export async function* queryMeshExports(args: QueryMeshExportsArgs): AsyncIterab
changeId: args.changesetId,
type: "IMODEL",
},
format: "IMDL",
format: "IMODEL",
urlPrefix: args.urlPrefix,
includeIncomplete: args.includeIncomplete,
enableCDN: args.enableCDN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,14 @@ import { IModelApp, ITWINJS_CORE_VERSION } from "@itwin/core-frontend";
/** The expected format of the Graphic Representation
* @beta
*/
/* eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents */
export type GraphicRepresentationFormat = "IMDL" | "3DTILES" | string;
// eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
export type GraphicRepresentationFormat = "3DFT" | "3DTiles" | "CESIUM" | "IMODEL" | string;

/** Graphic representations are generated from Data Sources.
* The status of a Graphic Representation indicates the progress of that generation process.
* @beta
*/
// ###TODO this needs to be expanded to include more statuses, and/or "failed" needs to be replaced with "invalid".
export enum GraphicRepresentationStatus {
InProgress = "In progress",
Complete = "Complete",
NotStarted = "Not started",
Failed = "Failed",
}
export type GraphicRepresentationStatus = "Complete" | "InProgress" | "Invalid" | "NotStarted";

/**
* Represents a data source for a graphic representation.
Expand Down Expand Up @@ -73,10 +67,10 @@ export type GraphicRepresentation = {
* Therefore, the url is optional if the status is not complete, and required if the status is complete.
*/
} & ({
status: Omit<GraphicRepresentationStatus, GraphicRepresentationStatus.Complete>;
status: Exclude<GraphicRepresentationStatus, "Complete">;
url?: string;
} | {
status: GraphicRepresentationStatus.Complete;
status: "Complete";
url: string;
});

Expand Down Expand Up @@ -138,8 +132,6 @@ export async function* queryGraphicRepresentations(args: QueryGraphicRepresentat
iModelId: string;
changesetId: string;
exportType: string;
geometryOptions: any;
viewDefinitionFilter: any;
};

/* eslint-disable-next-line @typescript-eslint/naming-convention */
Expand Down Expand Up @@ -184,21 +176,40 @@ export async function* queryGraphicRepresentations(args: QueryGraphicRepresentat
break;
}

const foundSources = result.exports.filter((x) => x.request.exportType === args.dataSource.type && (args.includeIncomplete || x.status === GraphicRepresentationStatus.Complete));
const foundSources = result.exports.filter((x) => x.request.exportType === args.dataSource.type && (args.includeIncomplete || x.status === "Complete"));
for (const foundSource of foundSources) {
const graphicRepresentation: GraphicRepresentation = {
displayName: foundSource.displayName,
representationId: foundSource.id,
status: foundSource.status,
format: args.format,
url: foundSource._links?.mesh.href,
dataSource: {
iTwinId: args.dataSource.iTwinId,
id: foundSource.request.iModelId,
changeId: foundSource.request.changesetId,
type: foundSource.request.exportType,
},
};
let graphicRepresentation: GraphicRepresentation;
if (foundSource.status === "Complete") {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the GraphicRepresentationFormat type to be a union of strings, which unfortunately requires this long if statement to enforce that url has type string only if status === "Complete", and type string | undefined otherwise. I tried to do this with a ternary operator but that doesn't work so I think this is the best solution to avoid more type assertions.

// If the service response's status is complete, it's guaranteed to have a url defined in links.mesh.
graphicRepresentation = {
displayName: foundSource.displayName,
representationId: foundSource.id,
status: foundSource.status,
format: args.format,
url: foundSource._links!.mesh.href,
dataSource: {
iTwinId: args.dataSource.iTwinId,
id: foundSource.request.iModelId,
changeId: foundSource.request.changesetId,
type: foundSource.request.exportType,
},
};
} else {
// Otherwise the url may be undefined because the representation is not finished being generated
graphicRepresentation = {
displayName: foundSource.displayName,
representationId: foundSource.id,
status: foundSource.status,
format: args.format,
url: foundSource._links?.mesh.href,
dataSource: {
iTwinId: args.dataSource.iTwinId,
id: foundSource.request.iModelId,
changeId: foundSource.request.changesetId,
type: foundSource.request.exportType,
},
};
}

yield graphicRepresentation;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { AccessToken, Logger } from "@itwin/core-bentley";
import { IModelApp } from "@itwin/core-frontend";
import { obtainGraphicRepresentationUrl } from "./GraphicRepresentationProvider";
import { obtainGraphicRepresentationUrl, ObtainGraphicRepresentationUrlArgs } from "./GraphicRepresentationProvider";
import { loggerCategory } from "../LoggerCategory";

/** Arguments supplied to [[obtainIModelTilesetUrl]].
Expand Down Expand Up @@ -48,7 +48,7 @@ export async function obtainIModelTilesetUrl(args: ObtainIModelTilesetUrlArgs):
return undefined;
}

const graphicsArgs = {
const graphicsArgs: ObtainGraphicRepresentationUrlArgs = {
accessToken: args.accessToken,
sessionId: IModelApp.sessionId,
dataSource: {
Expand All @@ -57,7 +57,7 @@ export async function obtainIModelTilesetUrl(args: ObtainIModelTilesetUrlArgs):
changeId: args.changesetId,
type: "IMODEL",
},
format: "IMDL",
format: "IMODEL",
urlPrefix: args.urlPrefix,
requireExactVersion: args.requireExactChangeset,
enableCDN: args.enableCDN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { expect, use } from "chai";
import * as chaiAsPromised from "chai-as-promised";
import * as sinon from "sinon";
import { IModelApp } from "@itwin/core-frontend";
import { createGraphicRepresentationsQueryUrl, obtainGraphicRepresentationUrl, queryGraphicRepresentations, QueryGraphicRepresentationsArgs } from "../../GraphicsProvider/GraphicRepresentationProvider";
import { createGraphicRepresentationsQueryUrl, GraphicRepresentationFormat, obtainGraphicRepresentationUrl, queryGraphicRepresentations, QueryGraphicRepresentationsArgs } from "../../GraphicsProvider/GraphicRepresentationProvider";

use(chaiAsPromised);

Expand Down Expand Up @@ -113,7 +113,17 @@ async function makeSourcesResponse(props: SourcesProps): Promise<Response> {
return makeResponse(async () => Promise.resolve(makeSources(props)));
}

const testArgs = {
const testArgs: {
accessToken: string;
sessionId: string;
dataSource: {
iTwinId: string;
id: string;
changeId?: string;
type: string;
};
format: GraphicRepresentationFormat;
} = {
accessToken: "this-is-a-fake-access-token",
sessionId: "testSession",
dataSource: {
Expand All @@ -122,7 +132,7 @@ const testArgs = {
changeId: undefined,
type: "srcType",
},
format: "IMDL",
format: "IMODEL",
};

describe("queryGraphicRepresentations", () => {
Expand All @@ -144,7 +154,11 @@ describe("queryGraphicRepresentations", () => {

it("produces one set of results", async () => {
await mockFetch(
async () => makeSourcesResponse({ exports: [{ id: "a" }, { id: "b" }, { id: "c" }] }),
async () => makeSourcesResponse({ exports: [
{ id: "a", href: "http://tiles.com/a" },
{ id: "b", href: "http://tiles.com/b" },
{ id: "c", href: "http://tiles.com/c" }
] }),
async () => expectSources(["a", "b", "c"], testArgs),
);
});
Expand All @@ -155,20 +169,32 @@ describe("queryGraphicRepresentations", () => {
async () => {
if (!fetchedFirst) {
fetchedFirst = true;
return makeSourcesResponse({ exports: [{ id: "a" }, { id: "b" }], next: "next.org" });
return makeSourcesResponse({ exports: [
{ id: "a", href: "http://tiles.com/a" },
{ id: "b", href: "http://tiles.com/b" },
],
next: "next.org"
});
} else {
return makeSourcesResponse({ exports: [{ id: "c" }, { id: "d" }] });
return makeSourcesResponse({ exports: [
{ id: "c", href: "http://tiles.com/c" },
{ id: "d", href: "http://tiles.com/d" }
] });
}
},
async () => expectSources(["a", "b", "c", "d"], testArgs));
});

it("includes only completed Data Sources unless otherwise specified", async () => {
await mockFetch(
async () => makeSourcesResponse({ exports: [{ id: "a", status: "Complete" }, { id: "b", status: "Feeling Blessed" }] }),
async () => makeSourcesResponse({ exports: [
{ id: "a", status: "Complete", href: "http://tiles.com/a" },
{ id: "b", status: "InProgress" },
{ id: "c", status: "Invalid" }
] }),
async () => {
await expectSources(["a"], testArgs);
await expectSources(["a", "b"], { ...testArgs, includeIncomplete: true }),
await expectSources(["a", "b", "c"], { ...testArgs, includeIncomplete: true }),
await expectSources(["a"], { ...testArgs, includeIncomplete: false });
},
);
Expand Down Expand Up @@ -216,7 +242,7 @@ describe("obtainGraphicRepresentationUrl", () => {
changeId: args.versionId,
type: "srcType",
},
format: "IMDL",
format: "IMODEL",
requireExactVersion: args.exact,
});

Expand Down
Loading