-
Notifications
You must be signed in to change notification settings - Fork 378
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
Finalise shareKeys #5055
Merged
Merged
Finalise shareKeys #5055
Changes from 14 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
1b09433
add v7 catalog support through v7initializationUrls
nf-s cd6cd4c
add test
nf-s 1b2f479
Merge remote-tracking branch 'origin/next' into v7-catalog-support-clean
nf-s aac476d
add v7initUrls docs + fix wrong v7 autoID
nf-s 9bbcc1c
add magda record terria aspect sharekeys to root group
nf-s 7c1a9f9
fix bad typing
nf-s fc7b9c1
add dodgy GroupMixin shareKeys
nf-s 4b8c333
add dynamic group sharekeys ADR
nf-s 4c611f8
add 0006 adr decision
nf-s 9b115c2
adr type
nf-s 87c23f1
change WMS-group member ids to match v7
nf-s bdd332a
add group shareKeys propagation
nf-s 77a25b6
fix tests
nf-s 1be250c
fix test
nf-s 5d13c2b
add WMS group `skareKeys` test
nf-s 7839d85
bump catalog converter versoin
nf-s cd079bb
fix legend image scaling
nf-s 717152a
fix test
nf-s 66905da
Merge remote-tracking branch 'origin/next' into v7-catalog-support-clean
nf-s 5d072b1
udpate changes
nf-s 05369a9
comment
nf-s f4804ef
changes
nf-s a3eb9a5
move catalog-share conversion from server to terriajs
nf-s 9a5501b
remove makeModelsMagdaCompatible
nf-s 9059ccd
Update lib/ModelMixins/GroupMixin.ts
nf-s e2d552a
Update lib/Models/Terria.ts
nf-s ca117c8
Update lib/ReactViews/Notification/shareConvertNotification.tsx
nf-s 2b61cf8
fix geoserver legend font colour
nf-s f4ebdcd
refactor initSource.optoins
nf-s 6f129b8
fix shareConvertNotification
nf-s 81fb978
changes
nf-s c5e9fc3
remove unused vars
nf-s File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
# 6. Dynamic groups and `shareKeys` / share link compatibility | ||
|
||
Date: 2020-12-10 | ||
Version: 1 | ||
|
||
## Status | ||
|
||
Accepted | ||
|
||
## Context | ||
|
||
See [architecture\0005-root-group-ids-and-shareKeys.md](architecture\0005-root-group-ids-and-shareKeys.md) for background. | ||
|
||
### Glossary | ||
|
||
- `wms-group` is `WebMapServiceCatalogGroup` - it loads `GetCapabilities` request to create `WebMapServiceCatalogItem`... | ||
- `wms` is `WebMapServiceCatalogItem` | ||
|
||
### How to handle loadable (dynamic) groups with ids - for example `wms-group` | ||
|
||
#### Using autoIDs | ||
|
||
autoIDs are generated hierarchically - using catalog member `name`. | ||
|
||
- v7 autoID has format `Root Group/$someContainerId/$someLowerContainerId/$catalogName` | ||
- v8 autoID has format `//$someContainerId/$someLowerContainerId/$catalogName` | ||
|
||
For example - a `wms-group` will have: | ||
|
||
- v7 autoID: `Root Group/$wmsGroup.name` | ||
- v8 autoID: `//$wmsGroup.name` | ||
|
||
and when it is loaded - `wms` items will have: | ||
|
||
- v7 autoID: `Root Group/$wmsGroup.name/$wmsItem.name` | ||
- v8 autoID: `//$wmsGroup.name/$wmsItem.name` | ||
|
||
#### Missing `shareKeys` | ||
|
||
Even if the `wms-group` has `shareKeys`, the `wms` item ID will be incorrect for a v7 share link | ||
|
||
- wms-group v7 autoID will be `Root Group/$wmsGroup.name` | ||
- which will be in `shareKeys` (this is added by `catalog-converter`) - so ID will be resolved correctly | ||
- A wms-item v7 autoID will be `Root Group/$wmsGroup.name/$wmsItem.name` | ||
- the item won't have any `shareKeys`, therefore the ID will **not** be resolved | ||
|
||
#### Using manual `id` for `wms-group` | ||
|
||
What happens when we set an explicit `id` for a `wms-group`? | ||
|
||
- A `wms-group` used to have no `id`, therefore it had a v8 autoID `//$wmsGroup.name` | ||
- Now the `wms-group` has `id`:`wms-group-random-id` (and `shareKeys=["//$wmsGroup.name"]`) | ||
- All share links which refer to `//$wmsGroup.name` will automatically resolve to `wms-group-random-id` | ||
- The `wms-group` is loaded and creates a bunch of `wms` items: | ||
- Before `wms-group` `id` change, items had v8 autoID of form `//$wmsGroup.name/$wmsItem.name` | ||
- After `id` change - `wms-group-random-id/$wmsItem.name` | ||
- Items have **no `shareKeys`** | ||
- Therefore, `wms` items in old share links **will not work!** | ||
|
||
This is also relevant if moving from v8 JSON to v8 Magda catalog - or any other transformation which sets explicit `id`s for catalog members. | ||
|
||
### Auto IDs for `CatalogGroups` | ||
|
||
Different dynamic `CatalogGroups` may generate member autoIDs differently. | ||
|
||
#### WMS-Group | ||
|
||
For example - `wms-group` autoIDs for `wms` items are generated from the item `name`: | ||
|
||
- in v8: `${layer.Name || layer.Title}` | ||
- in v7: | ||
|
||
```js | ||
if (wmsGroup.titleField === "name") { | ||
return layer.Name; | ||
} else if (wmsGroup.titleField === "abstract") { | ||
return layer.Abstract; | ||
} else { | ||
return layer.Title; | ||
} | ||
``` | ||
|
||
#### CKAN Resource | ||
|
||
- in v8: `parentId + "/" + ckanDataset.id + "/" + ckanResource.id` | ||
|
||
### Summary | ||
|
||
How do we add `shareKeys` to items created by dynamic groups? | ||
|
||
## Decisions | ||
|
||
### Add `shareKeys` to group items on load | ||
|
||
- Go through each share key in `group.shareKeys` and create new `member.shareKeys`: | ||
- Look at current `member.uniqueId` | ||
- Replace instances of `group.uniqueID` in `member.uniqueId` with `shareKey` | ||
- For example: | ||
- `group.uniqueId = "some-group-id"` | ||
- `member.uniqueId = "some-group-id/some-member-id"` | ||
- `group.shareKeys = ["old-group-id"]` | ||
- So we want to create `member.shareKeys = ["old-group-id/some-member-id"]` | ||
|
||
To do this, we need a reverse map of `id` -> `shareKeys`. | ||
|
||
### Revert v8 WMS-group member ids to match v7 | ||
|
||
This means we don't need to add manual `shareKeys` to WMS-groups. | ||
|
||
## Consequences | ||
|
||
This does not solve for the different methods of generating autoIDs across v7 and v8. | ||
|
||
### Known incompatible groups | ||
|
||
- CKAN Group | ||
|
||
If we have compatibility issues with certain `CatalogGroup`s, we can either: | ||
|
||
- Change id generation in v8 to match v7 | ||
- Add manual `shareKey` generation to each `CatalogGroup` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,11 +11,13 @@ interface InitData { | |
data: JsonObject; | ||
} | ||
|
||
type InitDataPromise = Promise<InitData>; | ||
|
||
interface InitOptions { | ||
options: InitSource[]; | ||
} | ||
|
||
type InitSource = InitUrl | InitData | InitOptions; | ||
type InitSource = InitUrl | InitData | InitOptions | InitDataPromise; | ||
|
||
export function isInitUrl(initSource: InitSource): initSource is InitUrl { | ||
return "initUrl" in initSource; | ||
|
@@ -25,6 +27,15 @@ export function isInitData(initSource: InitSource): initSource is InitData { | |
return "data" in initSource; | ||
} | ||
|
||
export function isInitDataPromise( | ||
initSource: InitSource | ||
): initSource is InitDataPromise { | ||
return ( | ||
initSource && | ||
Object.prototype.toString.call(initSource) === "[object Promise]" | ||
); | ||
} | ||
|
||
Comment on lines
+30
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Used for v7-v8 catalog converting |
||
export function isInitOptions( | ||
initSource: InitSource | ||
): initSource is InitOptions { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed bad
<BaseModel>
typing here