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.

Sign up for GitHub

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(i3s): fix for /statistics/summary path #3154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −0
Open

fix(i3s): fix for /statistics/summary path #3154

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/i3s/src/lib/parsers/parse-slpk/slpk-archieve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const PATH_DESCRIPTIONS: {test: RegExp; extensions: string[]}[] = [
test: /statistics\/f_\d+\/\d+$/,
extensions: ['.json.gz']
},
{
test: /statistics\/summary$/,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This path is not a part of the spec https://docs.ogc.org/cs/17-014r9/17-014r9.html#_example_1_2_slpk_structure_summary_for_3d_objects

Copy link
Author

@bortomar-intellmaps bortomar-intellmaps Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I didn't know that but I see it now. I create this pull request based on fix from @dariaterekhova-actionengine. I described the issue in #3148 (reply in thread).

Copy link
Author

@bortomar-intellmaps bortomar-intellmaps Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't understand that slpk archive from Esri/i3s-spec is not compatible with the standard.
Am I missing something?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is simply a clean extension of the standard (i.e. an additional optional path that provides additional, non-critical information), then there is no compatibility issues.

It would however be nice to separate these things in the code (using some lightweight mechanism separate files, functions, tables etc) and call it out in comments.

It would also be nice to cover it in the documentation as well:

  • Add a section that lists Esri additions to the standard
  • Mention which ones are supported, and how (if this path is present, how do we use it and what extra capabilities are offered to the user as a result).

extensions: ['.json.gz']
},
{
test: /nodes\/\d+\/shared$/,
extensions: ['/sharedResource.json.gz']
Expand Down