Merge pull request #6955 from TerriaJS/fix-wms-nested-ids

Fix WMS group nested ids
TerriaJS · Nov 3, 2023 · 044831c · 044831c
2 parents 2567156 + a4fb9dd
commit 044831c
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 30 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -3,6 +3,7 @@
 #### next release (8.3.8)
 
 - Remove `jsx-control-statements` dependency
+- Fix WMS nested group IDs - nested groups with the same name were not being created
 - WMS `isEsri` default value will now check for case-insensitive `mapserver/wmsserver` (instead of `MapServer/WMSServer`)
 - Tweak ArcGis MapServer WMS `GetFeatureInfo` default behaviour
   - Add `application/geo+json` and `application/vnd.geo+json` default `GetFeatureInfo` (after `application/json` in priority list)

diff --git a/lib/Models/Catalog/Ows/WebMapServiceCatalogGroup.ts b/lib/Models/Catalog/Ows/WebMapServiceCatalogGroup.ts
@@ -168,8 +168,8 @@ class GetCapabilitiesStratum extends LoadableStratum(
   }
 
   @action
-  createMemberFromLayer(layer: CapabilitiesLayer) {
-    const layerId = this.getLayerId(layer);
+  createMemberFromLayer(layer: CapabilitiesLayer, parentLayerId?: string) {
+    const layerId = this.getLayerId(layer, parentLayerId);
 
     if (!layerId) {
       return;
@@ -185,7 +185,7 @@ class GetCapabilitiesStratum extends LoadableStratum(
         members = [layer.Layer as CapabilitiesLayer];
       }
 
-      members.forEach((member) => this.createMemberFromLayer(member));
+      members.forEach((member) => this.createMemberFromLayer(member, layerId));
 
       // Create group
       const existingModel = this.catalogGroup.terria.getModelById(
@@ -201,7 +201,7 @@ class GetCapabilitiesStratum extends LoadableStratum(
           // At the moment we ignore duplicate layers
           this.catalogGroup.terria.addModel(
             model,
-            this.getLayerShareKeys(layer)
+            this.getLayerShareKeys(layer, layerId)
           );
         } catch (e) {
           TerriaError.from(e, "Failed to add CatalogGroup").log();
@@ -215,7 +215,9 @@ class GetCapabilitiesStratum extends LoadableStratum(
       model.setTrait(
         CommonStrata.definition,
         "members",
-        filterOutUndefined(members.map((member) => this.getLayerId(member)))
+        filterOutUndefined(
+          members.map((member) => this.getLayerId(member, layerId))
+        )
       );
 
       // Set group `info` trait if applicable
@@ -251,7 +253,10 @@ class GetCapabilitiesStratum extends LoadableStratum(
       try {
         // Sometimes WMS Layers have duplicate names
         // At the moment we ignore duplicate layers
-        this.catalogGroup.terria.addModel(model, this.getLayerShareKeys(layer));
+        this.catalogGroup.terria.addModel(
+          model,
+          this.getLayerShareKeys(layer, layerId)
+        );
       } catch (e) {
         TerriaError.from(e, "Failed to add WebMapServiceCatalogItem").log();
         return;
@@ -318,22 +323,30 @@ class GetCapabilitiesStratum extends LoadableStratum(
     model.createGetCapabilitiesStratumFromParent(this.capabilities);
   }
 
-  getLayerId(layer: CapabilitiesLayer) {
-    if (!isDefined(this.catalogGroup.uniqueId)) {
+  getLayerId(layer: CapabilitiesLayer, parentLayerId?: string) {
+    if (!isDefined(this.catalogGroup.uniqueId) && !isDefined(parentLayerId)) {
       return;
     }
-    return `${this.catalogGroup.uniqueId}/${layer.Name ?? layer.Title}`;
+    return `${parentLayerId ?? this.catalogGroup.uniqueId}/${
+      layer.Name ?? layer.Title
+    }`;
   }
 
   /** For backward-compatibility.
-   * If layer.Name is defined, we will use it to create layer autoID (see `this.getLayerId`).
-   * Previously we used layer.Title, so we now add it as a shareKey
+   * Previously we have used the following IDs
+   * - `WMS Group Catalog ID/WMS Layer Name` - regardless of nesting
+   * - `WMS Group Catalog ID/WMS Layer Title`
    */
-  getLayerShareKeys(layer: CapabilitiesLayer) {
+  getLayerShareKeys(layer: CapabilitiesLayer, layerId: string) {
+    const shareKeys: string[] = [];
+
+    if (layerId !== `${this.catalogGroup.uniqueId}/${layer.Name}`)
+      shareKeys.push(`${this.catalogGroup.uniqueId}/${layer.Name}`);
+
     if (isDefined(layer.Name) && layer.Title !== layer.Name)
-      return [`${this.catalogGroup.uniqueId}/${layer.Title}`];
+      shareKeys.push(`${this.catalogGroup.uniqueId}/${layer.Title}`);
 
-    return [];
+    return shareKeys;
   }
 }
 

diff --git a/test/Models/Catalog/Ows/WebMapServiceCatalogGroupSpec.ts b/test/Models/Catalog/Ows/WebMapServiceCatalogGroupSpec.ts
@@ -133,26 +133,53 @@ describe("WebMapServiceCatalogGroup", function () {
     });
 
     it("loads", async function () {
-      expect(wms.members.length).toEqual(3);
-      expect(wms.memberModels.length).toEqual(3);
+      expect(wms.members.length).toEqual(2);
+      expect(wms.memberModels.length).toEqual(2);
 
-      const firstGroup = wms.memberModels[0];
+      const firstGroup = wms.memberModels[0] as WebMapServiceCatalogGroup;
+      expect(firstGroup.uniqueId).toEqual(
+        "test/Digital Earth Australia - OGC Web Services"
+      );
       expect(
         GroupMixin.isMixedInto(firstGroup) && firstGroup.members.length
       ).toEqual(3);
 
-      const firstGroupFirstModel =
-        GroupMixin.isMixedInto(firstGroup) && firstGroup.memberModels[0];
-      expect(
-        firstGroupFirstModel &&
-          CatalogMemberMixin.isMixedInto(firstGroupFirstModel) &&
-          firstGroupFirstModel.name
-      ).toEqual("Surface Reflectance 25m Annual Geomedian (Landsat 8)");
+      const firstSubGroup = firstGroup
+        .memberModels[0] as WebMapServiceCatalogGroup;
+      expect(firstSubGroup.uniqueId).toEqual(
+        "test/Digital Earth Australia - OGC Web Services/Surface Reflectance"
+      );
+      expect(firstSubGroup.name).toEqual("Surface Reflectance");
+      expect(firstSubGroup.members.length).toEqual(3);
 
-      const thirdGroup = wms.memberModels[2];
-      expect(
-        GroupMixin.isMixedInto(thirdGroup) && thirdGroup.members.length
-      ).toEqual(1);
+      const firstSubGroupModel = firstSubGroup
+        .memberModels[0] as WebMapServiceCatalogItem;
+      expect(firstSubGroupModel.uniqueId).toEqual(
+        "test/Digital Earth Australia - OGC Web Services/Surface Reflectance/ls8_nbart_geomedian_annual"
+      );
+      expect(firstSubGroupModel.name).toEqual(
+        "Surface Reflectance 25m Annual Geomedian (Landsat 8)"
+      );
+
+      const secondGroup = wms.memberModels[1] as WebMapServiceCatalogGroup;
+      expect(secondGroup.uniqueId).toEqual("test/Some other catalog");
+      expect(secondGroup.name).toEqual("Some other catalog");
+      expect(secondGroup.memberModels.length).toEqual(1);
+
+      const secondSubGroup = secondGroup
+        .memberModels[0] as WebMapServiceCatalogGroup;
+      expect(secondSubGroup.uniqueId).toEqual(
+        "test/Some other catalog/Surface Reflectance"
+      );
+      expect(secondSubGroup.name).toEqual("Surface Reflectance");
+      expect(secondSubGroup.members.length).toEqual(1);
+
+      const secondSubGroupModel = secondSubGroup
+        .memberModels[0] as WebMapServiceCatalogItem;
+      expect(secondSubGroupModel.uniqueId).toEqual(
+        "test/Some other catalog/Surface Reflectance/some_layer"
+      );
+      expect(secondSubGroupModel.name).toEqual("Some layer");
     });
   });
 
@@ -176,8 +203,10 @@ describe("WebMapServiceCatalogGroup", function () {
     });
 
     it("sets traits correctly", async function () {
-      const wmsItem = (wms.memberModels[0] as WebMapServiceCatalogGroup)
-        .memberModels[0] as WebMapServiceCatalogItem;
+      const wmsItem = (
+        (wms.memberModels[0] as WebMapServiceCatalogGroup)
+          .memberModels[0] as WebMapServiceCatalogGroup
+      ).memberModels[0] as WebMapServiceCatalogItem;
 
       expect(wmsItem.linkedWcsUrl).toEqual("some-url");
       expect(wmsItem.linkedWcsCoverage).toEqual("ls8_nbart_geomedian_annual");

diff --git a/wwwroot/test/WMS/wms_nested_groups.xml b/wwwroot/test/WMS/wms_nested_groups.xml
@@ -601,5 +601,52 @@ NOTE this layer has no EX_GeographicBoundingBox
         </Layer>
       </Layer>
     </Layer>
+    <Layer>
+      <Title>Some other catalog</Title>
+      <Abstract>
+            Some other catalog
+      </Abstract>
+      <CRS>EPSG:3857</CRS>
+      <CRS>EPSG:4326</CRS>
+      <CRS>EPSG:3577</CRS>
+      <CRS>EPSG:3111</CRS>
+      <EX_GeographicBoundingBox>
+        <westBoundLongitude>100</westBoundLongitude>
+        <eastBoundLongitude>160</eastBoundLongitude>
+        <southBoundLatitude>-50</southBoundLatitude>
+        <northBoundLatitude>-10</northBoundLatitude>
+      </EX_GeographicBoundingBox>
+      <Layer>
+        <Title>Surface Reflectance</Title>
+        <Abstract>This is another layer called Surface Reflectance</Abstract>
+        <Layer queryable="1">
+          <Name>some_layer</Name>
+          <Title>Some layer</Title>
+          <Abstract>Some layer
+          </Abstract>
+          <KeywordList>
+            <Keyword>WOfS</Keyword>
+          </KeywordList>
+          <EX_GeographicBoundingBox>
+            <westBoundLongitude>109.989859933428</westBoundLongitude>
+            <eastBoundLongitude>156.101505058599</eastBoundLongitude>
+            <southBoundLatitude>-45.2413329418709</southBoundLatitude>
+            <northBoundLatitude>-9.02727104242042</northBoundLatitude>
+          </EX_GeographicBoundingBox>
+          <BoundingBox CRS="EPSG:3111" minx="-1623290.9363679164" maxx="3983581.4498637905" miny="1042109.9920098714" maxy="5725855.407866031" />
+          <BoundingBox CRS="EPSG:3577" minx="-2407984.8524649167" maxx="2834259.110253389" miny="-5195512.771063175" maxy="-936228.5063655999" />
+          <BoundingBox CRS="EPSG:3857" minx="12324052.57369671" maxx="17488921.644948885" miny="-5742240.963567747" maxy="-1001009.9542990683" />
+          <BoundingBox CRS="EPSG:4326" minx="-45.76168492731699" maxx="-8.955535935066568" miny="110.70884789244278" maxy="157.10565616426302" />
+          <Dimension name="time" units="ISO8601">
+            2013-01-01,2014-01-01,2015-01-01,2016-01-01,2017-01-01,2018-01-01
+          </Dimension>
+          <Style>
+            <Name>simple_rgb</Name>
+            <Title>Simple RGB</Title>
+            <Abstract>Simple true-colour image, using the red, green and blue bands</Abstract>
+          </Style>
+        </Layer>
+      </Layer>
+    </Layer>
   </Capability>
 </WMS_Capabilities>