iModel integration by jjspace · Pull Request #12289 · CesiumGS/cesium

jjspace · 2024-11-05T16:12:54Z

Description

Beginning steps for the iTwin/iModel integration utilizing the mesh-export API

This PR went through a few different iterations but I think we've landed on what we want for the initial implementation.
The functions added are very "bare bones" and targeted at loading iModel exports from iModel ids.
The auth with the iTwin platform should happen on the "app side" and used to set the ITwinPlatform.defaultAccessToken.
Exports for iModels should be auto-generated by the iTwin platform after being requested for an iModel the first time.

Assorted tasklist

Verify how auth should work.
- Does one top level defaultAccessToken even make sense if different routes need different scopes?
- is there a way to get a long term token without a separate login?
Do we even want to keep the checking for an existing Cesium export? or is that on the APP to implement?
How do the functions I've written handle changesets? When it there's a new one? When one's specified specifically? etc.
- We've opted to skip including the ability to specify a specific changeset at this time. This integration is focused on viewing models and a majority of the time that will mean the latest version of said models. Can be revisited in the future if there's demand for this functionality
Confirm the sample iTwins we want to use and update the sandcastle to use the correct ids
Update the sandcastle to point to the ion token generation route for authentication with our read only service app

Issue number and link

no issue number

Testing plan

Running the test sandcastle
- Load the new sandcastle and make sure it loads. note this may take up to a min of 20ish seconds, I usually see around 7 sec. check the console for actual errors

Author checklist

I have submitted a Contributor License Agreement
I have added my name to CONTRIBUTORS.md
I have updated CHANGES.md with a short summary of my change
I have added or updated unit tests to ensure consistent code coverage
I have updated the inline documentation, and included code examples where relevant
I have performed a self-review of my code

github-actions · 2024-11-05T16:13:17Z

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

jjspace · 2024-11-05T21:42:13Z

@ggetz just pushed an update to restructure the api a bit. I moved the itwin API functions under the ITwin namespace. I also split the tileset functions into 2 functions. One loads a tileset from only the export id and the other takes a model id + changeset combo and tries to find and load a CESIUM export if possible.

Looking at the API more it seems exports can be created for the same "model id + changeset" with different export parameters which could mean multiple CESIUM type exports for the same combo. I think this is probably less likely and when it's needed then users can use the load function with the export id directly and manage their own handling of the list.

pjcozzi · 2024-11-05T21:54:03Z

@aruniverse would you or another iTwin colleague be able to take a look at this?

aruniverse · 2024-11-05T22:01:06Z

@aruniverse would you or another iTwin colleague be able to take a look at this?

Thanks for sharing, this is exciting to see! I'll take a pass over it.

fyi @danieliborra

aruniverse

Did a quick first pass, Josh this looks pretty good so far. I will look at trying to run this locally tomorrow.

Added some minor nit comments and notes. I've tagged some of the other leads from iTwin Platform working on the services you are using here for visibility.

Apps/Sandcastle/gallery/iTwin Demo.html

packages/engine/Source/Core/ITwin.js

packages/engine/Source/Scene/createIModel3DTileset.js

Apps/Sandcastle/gallery/iTwin Demo.html

pjcozzi · 2024-11-06T03:13:49Z

@aruniverse would you or another iTwin colleague be able to take a look at this?

Thanks for sharing, this is exciting to see! I'll take a pass over it.

Appreciate the fast turn, thanks @aruniverse.

jjspace · 2024-11-06T18:51:36Z

Added some minor nit comments and notes.

@aruniverse thank you for the thorough comments so quick! This is still a Draft PR and very much a work in progress and nothing is final. I agree with most of your comments but it was not ready for such scrutiny 😅 I opened the PR to work through the general process and API structure that we'd need. A lot of the code is still very much thrown together to prove out the concept

danieliborra

It looks promising.
We're now modifying the API, so we will keep you updated of any change in the API.

packages/engine/Source/Core/ITwin.js

packages/engine/Source/Scene/createIModel3DTileset.js

jjspace · 2024-11-11T21:52:25Z

Given that the iTwin platform currently has no way to create long running API keys I spent some time looking into the oauth workflows. The main goal right now is to be able to create one or more sandcastles that can demonstrate using iModels in CesiumJS without requiring a login from the users. @ggetz had the idea to use the ion servers for oauth and return a token that can be used in Sandcastle.

My latest commit includes a demo server in itwin-oauth-demo to act in place of ion to demonstrate using the iTwin Service App and iTwin Web App auth processes. Run with cd itwin-oauth-demo && node server.js. Ask me for the config if you wanna use my app keys.
The Web App process requires the user to log in but then gives access to all of that user's iTwins/iModels. The Service App process does not require a login but the "service app user" needs to be added to a specific iTwin and given permissions to do things with that iTwin/iModels. The Projects page can be used to view iTwin projects and manage users/roles.

The only way I found to add the app user was through the Access Control API which needs you to create a role first for the role id. I found the sandcastle was able to load data using a service app token with these permissions on it's role:

The roles don't perfectly match with the previous API scopes and there's none that describe the model exports so I'm not 100% sure which role permissions we'll need but this was just to prove out the concept and it is working.

The demo sandcastle has been updated to use the new local auth server. If you want to try the Web App flow that requires a login just uncomment/comment the code near the top and reload the sandcastle then log in. You will need access to the iModel you're trying to load.

ggetz

Thanks @jjspace!

I left a few comments for the things that jumped out to me.

However, I understand that some of this is in flux. I believe at this point we have enough info to settle on the public API. Please do a pass on that and the docs, then ping me when ready for another a review pass.

ggetz · 2024-11-14T18:44:26Z

Apps/Sandcastle/gallery/iTwin Demo.html

+        const serviceResponse = await fetch("http://localhost:3000/service");
+        const { token } = await serviceResponse.json();


Thinking about user workflow, should this be encapsulated in ITwin.requestAccessToken or a similar function?

That way, these first few code block is simplified to:

Cesium.ITwin.defaultAccessToken = await Cesium.ITwin.requestAccessToken();

and we could make small changes if needed without needing user code to change.

I could add a function like that but I don't think users should ever be running this themselves. They should do their own oauth and just set the token. This request is just for the sandcastle examples and will only provide access to the sample itwins that we add to the service account.

I ask as it seams a bit strange to me that we would include a direct call to fetch in a Sandcastle example.

It doesn't seem too foreign to me that we would have a function that abstracts away the relevant Cesium ion URL to get the access token under the hood that we plan on using for authentication. For example, this is similar to what IonResource.fromAssetId is doing. In the future, we could potentially allow this function to reach out to an iTwin endpoint if available, or a user-provided endpoint without having to change the flow.

But let me know what you think.

@jjspace Thoughts?

Apps/Sandcastle/gallery/iTwin Demo.html

packages/engine/Source/Core/ITwin.js

packages/engine/Source/Scene/createIModel3DTileset.js

jjspace · 2024-11-15T22:03:57Z

@ggetz thanks for the comments, this should be updated in response to all of them. I'm still looking into the reality data functionality and hope to have some functions for that added early next week.

ggetz · 2024-11-18T16:59:33Z

I'm still looking into the reality data functionality and hope to have some functions for that added early next week.

Sounds good! Please include those in a separate PR if this is not merged first.

danieliborra

My apologies for not keeping the documentation up-to-date, which made you develop all the endpoints, when a few would have been enough.
On the other side, I think it has also helped us to understand how to improve the documentation.

packages/engine/Source/Scene/ITwinData.js

Apps/Sandcastle/gallery/iTwin Demo.html

packages/engine/Source/Core/ITwinPlatform.js

packages/engine/Source/Scene/ITwinData.js

packages/engine/Source/Core/ITwinPlatform.js

ggetz

Thanks @jjspace! Once you take a pass on these comments, could you please update the PR description with the current status? I'm a bit lost on what you're considering ready vs what is temporary.

packages/engine/Source/Core/ITwinPlatform.js

Apps/Sandcastle/gallery/iTwin Demo.html

itwin-oauth-demo/index.html

packages/engine/Source/Core/ITwinPlatform.js

.prettierignore

danieliborra

It looks good to me!
(I just added a few nitpicking comments)

Apps/Sandcastle/gallery/iTwin Demo.html

packages/engine/Source/Core/ITwinPlatform.js

jjspace · 2024-11-20T18:35:29Z

@ggetz I think this is getting close and ready for another look. I also updated the main PR description to indicate a summary of our decisions and the remaining tasks

aruniverse

This looks great, thanks @jjspace, can't wait to get our hands on it!

CHANGES.md

jjspace · 2024-11-22T22:19:52Z

This is very nearly there now. We're just waiting on the ion route to be updated so auth can work for everyone but the sandcastle has been updated with the new sample data.

Remaining:

Add tests for the new API functions (first on my list on Monday)
Update the Sandcastle auth route
Potentially (only if it's really quick to do so) implement some camera locations inside the station since I realized it's fully modeled and would be a good AEC showcase

The sandcastle should also have reality mesh data in it but that will be implemented through a follow up PR on Monday.

danieliborra

Looks good to me!

ggetz

Thanks for the update here @jjspace! The tests look great.

I did a final code pass, mainly on documentation and wording.

eslint.config.js

CHANGES.md

Apps/Sandcastle/gallery/iTwin Demo.html

packages/engine/Source/Core/ITwinPlatform.js

packages/engine/Source/Scene/ITwinData.js

ggetz · 2024-11-25T15:07:56Z

packages/engine/Source/Core/ITwinPlatform.js

+ * @enum {string}
+ */
+ITwinPlatform.ExportStatus = Object.freeze({
+  NotStarted: "NotStarted",


For these to show up on the documentation page, you'll need to add a jsDoc comment for each property, i.e.

Suggested change

NotStarted: "NotStarted",

/**

* A status indicating the export has not yet started.

*

* @type {string}

* @constant

*/

NotStarted: "NotStarted",

I had intended these to be private but then the types generation will complain about the places they're used. One of the many issues with how private is being used to mean both actually private and just not public outside internal code. I'll add descriptions but no users should really ever have to use these

I tried adding these comments but it caused issues with the typescript build. I tried a couple different jsdoc arrangements and none seemed to both 1) make our typescript generation happy and accurate and 2) generate the docs in the correct place (under the ITwinPlatform). I don't think JSDoc or tsd-jsdoc can handle the "nested" nature of these enums correctly...
I've removed for now but can try to look into it more if you think it's worth for this release

It's not ideal but I've added the values in the enum descriptions directly. I think that's an ok compromise for now.

jjspace · 2024-11-25T17:35:43Z

@ggetz this has been updated with the ion oauth route. I believe this PR should be good to go now. Reality data PR open very soon

ggetz · 2024-11-25T17:45:42Z

🎉 This is looking great @jjspace! Two comments on the Sandcastle:

I think we're missing a thumbnail image:

Can we start with the "Focus Station" view? I think it's more visually interesting than the bird's eye view.

jjspace · 2024-11-25T18:12:21Z

@ggetz Good to go I think

ggetz · 2024-11-25T18:31:12Z

Awesome! Thanks @jjspace! And a big thanks to @danieliborra and @aruniverse for your help!

initial rough api

0f6db77

jjspace requested a review from ggetz November 5, 2024 16:12

restructure api, create tileset from model id

421e5dd

aruniverse reviewed Nov 5, 2024

View reviewed changes

small adjustments

4ea3908

danieliborra reviewed Nov 6, 2024

View reviewed changes

oauth testing web app and service app

d779bc1

small cleanup and removing code we won't use

50dfe46

ggetz requested changes Nov 14, 2024

View reviewed changes

jjspace added 3 commits November 15, 2024 15:58

cleanup and adjustments from pr comments

c953b6f

Merge remote-tracking branch 'origin/main' into itwin-integration

dc79961

rename namespaces

46bd2f0

danieliborra suggested changes Nov 18, 2024

View reviewed changes

ggetz reviewed Nov 18, 2024

View reviewed changes

jjspace added 3 commits November 19, 2024 16:32

partial cleanup, remove changeset ids

1757f77

remove get export and creation routes

b8c1ac5

update sandcastle for exports still processing

958756e

danieliborra reviewed Nov 20, 2024

View reviewed changes

re-organize, resource instead of fetch, clean up types

7049dbe

jjspace marked this pull request as ready for review November 20, 2024 18:29

minor renaming and descriptions

3726464

ggetz added the priority - next release label Nov 21, 2024

jjspace added 2 commits November 22, 2024 13:53

pull out dev auth server

d2055b2

condense code, update docs

47e1642

aruniverse reviewed Nov 22, 2024

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

jjspace added 2 commits November 22, 2024 16:52

streamline sandcastle with new imodels

990004b

Merge remote-tracking branch 'origin/main' into itwin-integration

c435e2b

jjspace added 2 commits November 23, 2024 16:24

add tests

fa1ef66

sandcastle photospheres

d478736

danieliborra approved these changes Nov 25, 2024

View reviewed changes

ggetz requested changes Nov 25, 2024

View reviewed changes

jjspace added 2 commits November 25, 2024 11:02

doc updates

153020b

switch to ion auth, remove enum comments

d066248

add enum values in comments

77a45d8

sandcastle adjustments

99eb928

jjspace mentioned this pull request Nov 25, 2024

iTwin Reality Data integration #12334

Merged

6 tasks

Merge branch 'main' into itwin-integration

0cc0603

ggetz approved these changes Nov 25, 2024

View reviewed changes

ggetz added this pull request to the merge queue Nov 25, 2024

Merged via the queue into main with commit 19486f2 Nov 25, 2024

ggetz deleted the itwin-integration branch November 25, 2024 18:31

jjspace mentioned this pull request Dec 2, 2024

Rename iModel sandcastle #12351

Merged

jason-crow mentioned this pull request Dec 20, 2024

Cesium.ITwinPlatform.defaultAccessToken should be a function instead of a string #12386

Open

jjspace mentioned this pull request Jul 30, 2025

Add support for optional changesets when loading iModels #12778

Merged

6 tasks

		const serviceResponse = await fetch("http://localhost:3000/service");
		const { token } = await serviceResponse.json();

-  NotStarted: "NotStarted",
+  /**
+   * A status indicating the export has not yet started.
+   *
+   * @type {string}
+   * @constant
+   */
+  NotStarted: "NotStarted",

Conversation

jjspace commented Nov 5, 2024 • edited by ggetz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Assorted tasklist

Issue number and link

Testing plan

Author checklist

Uh oh!

github-actions bot commented Nov 5, 2024

Uh oh!

jjspace commented Nov 5, 2024

Uh oh!

pjcozzi commented Nov 5, 2024

Uh oh!

aruniverse commented Nov 5, 2024

Uh oh!

aruniverse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pjcozzi commented Nov 6, 2024

Uh oh!

jjspace commented Nov 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danieliborra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jjspace commented Nov 11, 2024

Uh oh!

ggetz left a comment

Choose a reason for hiding this comment

Uh oh!

ggetz Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

jjspace Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggetz Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

ggetz Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jjspace commented Nov 15, 2024

Uh oh!

ggetz commented Nov 18, 2024

Uh oh!

danieliborra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jjspace commented Nov 5, 2024 •

edited by ggetz

Loading

jjspace commented Nov 6, 2024 •

edited

Loading

jjspace Nov 15, 2024 •

edited

Loading

jjspace commented Nov 22, 2024 •

edited

Loading

jjspace Nov 25, 2024 •

edited

Loading