Merge pull request #5055 from TerriaJS/v7-catalog-support-clean

Finalise shareKeys

CHANGES.md

@@ -4,10 +4,20 @@ Change Log
 ### MobX Development
 
 #### next release (8.0.0-alpha.63)
+* Add `v7initializationUrls` to terria config. It will convert catalogs to v8 and print warning messages to console.
+* Add `shareKeys` support for Madga map-config maps (through `terria` aspect)
+* Revert WMS-group item ID generation to match v7
+* Add `addShareKeysToMembers` to `GroupMixin` to generate `shareKeys` for dynamic groups (eg `wms-group)
+* Added `InitDataPromise` to `InitSources`
+* Add reverse `modelIdShareKeysMap` map - `model.id` -> `shareKeys`
+* Upgraded `catalog-converter` to 0.0.2-alpha.4
+* Reverted Legend use of `object` instead of `img` - sometimes it was showing html error responses
+* Legend will now hide if an error is thrown
 * Update youtube urls to nocookie version
+* Share link conversion (through `catalog-converter`) is now done client-side
+* Fix Geoserver legend font colour bug
 * [The next improvement]
 
-
 #### 8.0.0-alpha.62
 * Fixed an issue with not loading the base map from init file and an issue with viewerMode from init files overriding the persisted viewerMode
 * Fixed issues surrounding tabbed catalog mode

architecture/0005-root-group-ids-and-shareKeys.md

@@ -5,7 +5,7 @@ Version: 3
 
 ## Status
 
-In discussion
+Accepted
 
 ## Context
 
@@ -21,10 +21,9 @@ In discussion
 
 Most maps (v7 and v8) use autogenerated `id` for catalog members - these are used if catalog members don't have an `id` explicitly defined.
 
+The root group `id` is very important for autoIDs. By default v8 uses `"/"` and v7 uses `"Root Group"`
 
-The root group `id` is very important for autoIDs. By default v8 uses `"/"` and v7 uses `"/Root Group"`
-
-- v7 ID has format `/Root Group/$someContainerId/$someLowerContainerId/$catalogName`
+- v7 ID has format `Root Group/$someContainerId/$someLowerContainerId/$catalogName`
 - v8 ID has format `//$someContainerId/$someLowerContainerId/$catalogName`
 
 Using autoIDs solidifies catalog - you can't move items around without changing IDs, which will break share links.
@@ -197,7 +196,7 @@ This also means that if we chain transformations - for example:
 
 We will have 3 `shareKeys` to maintain:
 
-- v7 autoIDs: `/Root Group/$someContainerId/$someLowerContainerId/$catalogName`
+- v7 autoIDs: `Root Group/$someContainerId/$someLowerContainerId/$catalogName`
 - Old magda JSON based autoIDs: `$magda-config-record-id/$someContainerId/$someLowerContainerId/$catalogName`
 
 Also, if we ever change the `$magda-config-record-id` - this will result in a whole new set of IDs that need to be added to `shareKeys` (for Magda JSON-based maps).

architecture/0006-dynamic-groups-and-shareKeys.md

@@ -0,0 +1,123 @@
+# 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.
+
+Changing `wms-group` IDs will break sharekeys for existing v8 maps.
+
+### 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`

doc/customizing/client-side-config.md

@@ -23,6 +23,10 @@ If a string ends with `.json`, it is assumed to be a complete relative or absolu
 
 If the string does not end with `.json`, such as `"foo"`, it refers to an init file on the same web server at `init/foo.json`.  In a TerriaMap directory on your computer, it can be found at `wwwroot/init/foo.json`.
 
+### v7intializationUrls
+
+It is also possible to add version 7 init files — these will be converted on-the-fly in `terriajs` when a map is loaded. See [`catalog-converter`](https://github.com/TerriaJS/catalog-converter) repo for more information.
+
 ## parameters
 
 Specifies various options for configuring TerriaJS:

lib/Core/addedByUser.ts

@@ -1,15 +1,17 @@
 import { BaseModel } from "../Models/Model";
 import i18next from "i18next";
+import isDefined from "./isDefined";
 export const USER_ADDED_CATEGORY_ID = "__User-Added_Data__";
 
 export default function addedByUser(
-  catalogMember: BaseModel,
+  catalogMember: BaseModel | undefined,
   options: {
     depth: number;
   } = {
     depth: 0
   }
 ): boolean {
+  if (!isDefined(catalogMember)) return false;
   const depth = options.depth;
   if (depth > 100) {
     console.error(
@@ -26,10 +28,9 @@ export default function addedByUser(
   return sourceReference.knownContainerUniqueIds.some(containerId => {
     return (
       containerId === USER_ADDED_CATEGORY_ID ||
-      addedByUser(
-        <BaseModel>catalogMember.terria.getModelById(BaseModel, containerId),
-        { depth: depth + 1 }
-      )
+      addedByUser(catalogMember.terria.getModelById(BaseModel, containerId), {
+        depth: depth + 1
+      })
     );
   });
 }

lib/ModelMixins/GroupMixin.ts

@@ -1,14 +1,15 @@
-import { action, computed, observable, runInAction } from "mobx";
-import DeveloperError from "terriajs-cesium/Source/Core/DeveloperError";
+import { action, computed } from "mobx";
 import clone from "terriajs-cesium/Source/Core/clone";
+import DeveloperError from "terriajs-cesium/Source/Core/DeveloperError";
+import AsyncLoader from "../Core/AsyncLoader";
 import Constructor from "../Core/Constructor";
 import filterOutUndefined from "../Core/filterOutUndefined";
 import isDefined from "../Core/isDefined";
+import Group from "../Models/Group";
 import Model, { BaseModel } from "../Models/Model";
 import GroupTraits from "../Traits/GroupTraits";
 import ModelReference from "../Traits/ModelReference";
-import AsyncLoader from "../Core/AsyncLoader";
-import Group from "../Models/Group";
+import CatalogMemberMixin from "./CatalogMemberMixin";
 
 function GroupMixin<T extends Constructor<Model<GroupTraits>>>(Base: T) {
   abstract class GroupMixin extends Base implements Group {
@@ -65,9 +66,8 @@ function GroupMixin<T extends Constructor<Model<GroupTraits>>>(Base: T) {
      */
     loadMembers(): Promise<void> {
       return this._memberLoader.load().finally(() => {
-        if (this.uniqueId) {
-          this.refreshKnownContainerUniqueIds(this.uniqueId);
-        }
+        this.refreshKnownContainerUniqueIds(this.uniqueId);
+        this.addShareKeysToMembers();
       });
     }
 
@@ -81,6 +81,39 @@ function GroupMixin<T extends Constructor<Model<GroupTraits>>>(Base: T) {
       });
     }
 
+    @action
+    addShareKeysToMembers(): void {
+      const groupId = this.uniqueId;
+      if (!groupId) return;
+
+      // Get shareKeys for this Group
+      const shareKeys = this.terria.modelIdShareKeysMap.get(groupId);
+      if (!shareKeys || shareKeys.length === 0) return;
+
+      /**
+       * Go through each shareKey and create new shareKeys for members
+       * - 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"]
+       */
+
+      this.memberModels.forEach((model: BaseModel) => {
+        // Only add shareKey if model.uniqueId is an autoID (i.e. contains groupId)
+        if (isDefined(model.uniqueId) && model.uniqueId.includes(groupId)) {
+          shareKeys.forEach(groupShareKey =>
+            this.terria.addShareKey(
+              model.uniqueId!,
+              model.uniqueId!.replace(groupId, groupShareKey)
+            )
+          );
+        }
+      });
+    }
+
     @action
     add(stratumId: string, member: BaseModel) {
       if (member.uniqueId === undefined) {

lib/Models/InitSource.ts

@@ -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]"
+  );
+}
+
 export function isInitOptions(
   initSource: InitSource
 ): initSource is InitOptions {