[GEOS-11353] MapML Make default image format for raster requests configurable for layers and layer groups #7612

turingtestfail · 2024-05-08T15:19:04Z

-Added tests for dropdown values and tile cache version

getting from enabled tile cache

added ajax

added to layergroup panel

Added document builder read and write of default mime

layer group and unit test

cleanup

removed default call

made sure default representation goes to html

Checklist

I have read the contribution guidelines.
I have sent a Contribution Licence Agreement (not required for small changes, e.g., fixing typos in documentation).
First PR targets the main branch (backports managed later; ignore for branch specific issues).
All the build checks are green (see automated QA checks).

For core and extension modules:

New unit tests have been added covering the changes.
Documentation has been updated (if change is visible to end users).
The REST API docs have been updated (when changing configuration objects or the REST controllers).
There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
Bug fixes and small new features are presented as a single commit.
Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

aaime

I've tested the layer case, the UI does not seem to be responsive:

I uncheck "use features", the mime type drop-down is still disabled
I go to the tile caching tab, add a format, it does not show in the drop-down either

In both cases, even pressing "apply" does not help, one has to exit the page and come back into it.

The changes seem to introduce a new format option to control the format: as I correct? Is it documented anywhere?

aaime · 2024-05-13T15:28:32Z

src/extension/mapml/src/main/java/org/geoserver/mapml/MapMLDocumentBuilder.java

+ defaultMimeType =
+ layerMeta.get(MapMLConstants.MAPML_MIME) != null
+ ? layerMeta.get(MapMLConstants.MAPML_MIME).toString()
+ : defaultMimeType;


This block is duplicated here and below. It can be shared outside of the if, because layerMeta itself can be obtained calling PublishedInfo.getMetadata (that is, no need to know if it's a layer group or not).

Also no need to call onto that metadata entry twice, e.g.:

Suggested change

defaultMimeType =

layerMeta.get(MapMLConstants.MAPML_MIME) != null

? layerMeta.get(MapMLConstants.MAPML_MIME).toString()

: defaultMimeType;

defaultMimeType = Optional.ofNullable(layerMeta.get(MapMLConstants.MAPML_MIME)).orElse(defaultMimeType);

Changed to Optional.ofNullable format but still needs to appear in both parts of if because metadata is stored in resourceInfo in the case of LayerInfo and in the main metadata in the case of LayerGroupInfo.

aaime · 2024-05-13T15:32:32Z

src/extension/mapml/src/main/java/org/geoserver/mapml/MapMLLayerGroupConfigurationPanel.html

@@ -1,65 +1,98 @@
 <html xmlns:wicket="http://wicket.apache.org/">
 <head></head>
 <body>
- <form name="theForm">


Mass reformat here?

aaime · 2024-05-13T15:33:57Z

src/extension/mapml/src/main/java/org/geoserver/mapml/MapMLLayerGroupConfigurationPanel.java

+ boolean useTilesFromModel =
+ Boolean.TRUE.equals(
+ model.getObject().getMetadata().get(MAPML_USE_TILES, Boolean.class));
+ mime =


What happes if use tiles gets flipped? This "boolean" here does not look like it would update.

Ajax update added.

aaime · 2024-05-13T15:34:39Z

src/extension/mapml/src/test/java/org/geoserver/mapml/MapMLLayerConfigurationPanelTest.java

+ // GridSetBroker gridSetBroker =
+ // new GridSetBroker(Collections.singletonList(new DefaultGridsets(true, true)));
+ // tileLayer = new
+ // GeoServerTileLayer(getCatalog().getLayerByName(MockData.PONDS.getLocalPart()),
+ // gridSetBroker, tileLayerInfo);
+ // GridSet testGridSet = namedGridsetCopy("TEST",
+ // gridSetBroker.getDefaults().worldEpsg4326());


Avoid commented out code.

aaime · 2024-05-13T15:36:39Z

src/extension/mapml/src/test/java/org/geoserver/mapml/MapMLLayerConfigurationPanelTest.java

+ assertThat(
+ dropDownChoiceTile.getChoices(),
+ Matchers.containsInAnyOrder("image/jpeg", "image/png"));


While I don't expect the mapml client to handle tiff related format, it's browser based, should be able to handle all the png/jpeg variations, including the "jpeg or png" one. Give it a try?

Tested and confirmed that tiff is not supported. Removed from regex.

aaime · 2024-05-14T12:50:31Z

src/extension/mapml/src/test/java/org/geoserver/mapml/MapMLLayerConfigurationPanelTest.java

+ "image/vnd.jpeg-png",
+ "image/jpeg",
+ "image/vnd.jpeg-png8",
+ "image/tiff8",


tiff is normally not handled properly by clients... what about the MapML one, can it?

See above, not handled. Removed

…igurable for layers and layer groups -Added tests for dropdown values and tile cache version getting from enabled tile cache added ajax added to layergroup panel Added document builder read and write of default mime layer group and unit test cleanup removed default call made sure default representation goes to html pr changes formatting

turingtestfail · 2024-05-21T01:18:50Z

UI is fixed - use features now uses Ajax for disabling dropdown. Tile caching format, once apply is clicked is added to list. The dropdown is documented in index.rst.

I've tested the layer case, the UI does not seem to be responsive:

I uncheck "use features", the mime type drop-down is still disabled

I go to the tile caching tab, add a format, it does not show in the drop-down either

In both cases, even pressing "apply" does not help, one has to exit the page and come back into it.

The changes seem to introduce a new format option to control the format: as I correct? Is it documented anywher

switched to text actually xpath is better community compile issue

…igurable for layers and layer groups -Added tests for dropdown values and tile cache version getting from enabled tile cache added ajax added to layergroup panel Added document builder read and write of default mime layer group and unit test cleanup removed default call made sure default representation goes to html pr changes formatting

turingtestfail requested review from aaime and prushforth May 8, 2024 15:19

aaime requested changes May 13, 2024

View reviewed changes

aaime reviewed May 14, 2024

View reviewed changes

turingtestfail force-pushed the GEOS-11353-MapML-default-image-format branch from 5649c70 to d32890f Compare May 20, 2024 22:52

turingtestfail added 2 commits May 20, 2024 19:09

more pr

ef4e44b

turingtestfail force-pushed the GEOS-11353-MapML-default-image-format branch from d32890f to ef4e44b Compare May 21, 2024 01:10

turingtestfail and others added 4 commits May 21, 2024 10:03

[GEOS-11397] App-Schema Includes fix Integration Tests (geoserver#7633)

4eae466

switched to text actually xpath is better community compile issue

more pr

d94926d

Rebase GEOS-11353-MapML-default-image-format

b064804

aaime approved these changes May 31, 2024

View reviewed changes

aaime merged commit 36aadf1 into geoserver:main May 31, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GEOS-11353] MapML Make default image format for raster requests configurable for layers and layer groups #7612

[GEOS-11353] MapML Make default image format for raster requests configurable for layers and layer groups #7612

turingtestfail commented May 8, 2024 •

edited

aaime left a comment

aaime May 13, 2024

aaime May 13, 2024

turingtestfail May 21, 2024

aaime May 13, 2024

turingtestfail May 21, 2024

aaime May 13, 2024

turingtestfail May 21, 2024

aaime May 13, 2024

turingtestfail May 21, 2024

aaime May 13, 2024

turingtestfail May 21, 2024

aaime May 14, 2024

turingtestfail May 21, 2024

turingtestfail commented May 21, 2024

[GEOS-11353] MapML Make default image format for raster requests configurable for layers and layer groups #7612

[GEOS-11353] MapML Make default image format for raster requests configurable for layers and layer groups #7612

Conversation

turingtestfail commented May 8, 2024 • edited

Checklist

aaime left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turingtestfail commented May 21, 2024

turingtestfail commented May 8, 2024 •

edited