Skip to content

Commit

Permalink
Merge pull request #6955 from TerriaJS/fix-wms-nested-ids
Browse files Browse the repository at this point in the history
Fix WMS group nested ids
  • Loading branch information
nf-s authored Nov 3, 2023
2 parents 2567156 + a4fb9dd commit 044831c
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 27 additions & 14 deletions lib/Models/Catalog/Ows/WebMapServiceCatalogGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}

Expand Down
61 changes: 45 additions & 16 deletions test/Models/Catalog/Ows/WebMapServiceCatalogGroupSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});

Expand All @@ -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");
Expand Down
47 changes: 47 additions & 0 deletions wwwroot/test/WMS/wms_nested_groups.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

0 comments on commit 044831c

Please sign in to comment.