Skip to content

Conversation

@ggetz
Copy link
Contributor

@ggetz ggetz commented Oct 28, 2025

Description

This PR tidies a bit of handling for imagery assets proxied via a Cesium ion asset, including Google 2D Imagery and shortly, Azure Imagery. It ensures we are including the X-Cesium-Client-* headers that are typically sent to ion URLs.

  • Previously, we treated proxied assets as "external". This meant we were omitting headers and requesting images in a slightly different different way. Now, the endpoint options are modified such that assets that are proxied are treated like typical ion imagery.
  • Tidies up special handling for Google2DImageryProvider and Azure2DImageryProvider so that their logic is now isolated to one file: IonImageryProviderFactory.
  • I pulled out logic from IonImageryProvider into IonImageryProviderFactory because 1) The former file was getting big and unwieldy, but now 2) we can call the logic from other places without creating a circular dependency on IonImageryProvider, allowing us to simplify Google2DImageryProvider.fromIonAssetId a bit.
  • Various testing cleanup, namely per jasmine's sharing behaviors recommendations, and minor source code changes to facilitate.

Issue number and link

Fixes #13000

Testing plan

  1. Confirm various imagery types—including Bing, Google 2D, ArcGIS, etc—still work as expected
    • With the debugger in the Network tab, confirm the imagery requests to assets.ion.com include the X-Cesium-Client and X-Cesium-Client-Version headers.
    • Confirm imagery requests to external (non-cesium.com urls) do not include these headers.
image
  1. Confirm existing Google 2D Imagery API via IonImageryProvider.fromAssetId properly refreshes sessions.
    • Open local sandcastle or deployed equivalent
    • Wait ~3 seconds to invalidate the original session
    • Navigate around to load new tiles. It's ok to see some error messages in the console while the session token is being refreshed, but they should quickly stop.
    • Confirm the refreshed imagery requests to assets.ion.com include the X-Cesium-Client and X-Cesium-Client-Version headers
  2. Confirm existing Google 2D Imagery API via Google2DImageryProvider.fromIonAssetId properly refreshes sessions.
    • Open local sandcastle or deployed equivalent
    • Wait ~3 seconds to invalidate the original session
    • Navigate around to load new tiles. It's ok to see some error messages in the console while the session token is being refreshed, but they should quickly stop.
    • Confirm the refreshed imagery requests to assets.ion.com include the X-Cesium-Client and X-Cesium-Client-Version headers

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 This should not affect users, so I did not update CHANGES.md
  • 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
Copy link

Thank you for the pull request, @ggetz!

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

@ggetz ggetz requested a review from lukemckinstry October 28, 2025 18:27
@ggetz
Copy link
Contributor Author

ggetz commented Oct 28, 2025

@lukemckinstry Could you please review? This may make sense to merge before #13001.

@lukemckinstry
Copy link
Contributor

The Google Contour asset (id: 3830186) throws this error in http://localhost:8080/Apps/Sandcastle2/index.html?id=imagery-assets-available-from-ion

RuntimeError: Ion.createResource does not support external imagery assets; use IonImageryProvider instead.
Error

The other assets look ok

@ggetz
Copy link
Contributor Author

ggetz commented Oct 29, 2025

The Google Contour asset (id: 3830186) throws this error in http://localhost:8080/Apps/Sandcastle2/index.html?id=imagery-assets-available-from-ion

@lukemckinstry Thanks for catching that! This sandcastle example exposes an issue with how the cached endpoint responses were working. I have a fix and will also add unit tests.

The caching issue came up because addToolbarMenu is invoking the first function in the options list. I don't think this is expected as that is the behavior specified for addDefaultToolbarMenu, and the example accidentally creates the provider twice. Since that's out of scope of this PR, I will open a separate issue.

@lukemckinstry

This comment was marked as resolved.

Copy link
Contributor

@lukemckinstry lukemckinstry left a comment

Choose a reason for hiding this comment

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

The IonResource additional Cesium client request headers and the retryCallback refactor look good 👍

@ggetz
Copy link
Contributor Author

ggetz commented Oct 30, 2025

Thanks for the review @lukemckinstry! I just pushed up the caching fix and unit test updates.

Assuming the updates look good, this should be good to merge.

@ggetz ggetz mentioned this pull request Oct 30, 2025
7 tasks
@ggetz ggetz self-assigned this Oct 30, 2025
@lukemckinstry lukemckinstry added this pull request to the merge queue Oct 30, 2025
Merged via the queue into main with commit 453b40d Oct 30, 2025
9 checks passed
@lukemckinstry lukemckinstry deleted the ion-headers branch October 30, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Headers not set on requests to ion

3 participants