[GEOS-12149] GeoServer WMTS capabilities put query parameters in the wrong place in generated URLs

[fernandor777]

https://osgeo-org.atlassian.net/browse/GEOS-12149

This PR fixes WMTS capabilities URL generation so propagated query parameters, such as projecttoken, are appended after the existing WMTS query string instead of being injected into the middle of the URL path.

What changed:

• Refactored WMTS capabilities URL handling to keep base URLs and propagated query parameters separate until final serialization.
• Updated WMTS URL construction so resource and metadata backlinks preserve path placeholders and append propagated query parameters last.
• Added/updated WMTS tests to cover:
  • existing query parameters
  • no existing query parameters
  • correct placement of propagated parameters
  • regression coverage for capabilities and TileJSON outputs

Behavior after the change, generated URLs now follow the expected structure:

• .../rest/BasicPolygons/{style}/{TileMatrixSet}/{TileMatrix}/{TileRow}/{TileCol}?format=image/png&projecttoken=abc123
• .../rest/WMTSCapabilities.xml?projecttoken=abc123

Instead of malformed forms where query parameters appears before unresolved path segments.

[aaime]

I've tried to review the PR but I'm having trouble getting the review to an end. There are just too many little changes, they do not need to be this verbose.

I've started with two suggestions:

• Do not reinvent the wheel, use the URL mangling mechanism already well established in geoserver (that one manages query parameters too)
• Please centralize as much URL handling code as possible in one helper. For reference, in GeoServer when buliding a URL with full mangling one just needs to do the following:

ResponseUtils.buildURL(baseURL, path, kvp, urlType)

I've tried to suggest something below, not sure if useful or not, but for sure we need better centralization, less repetition, and simpler code in the various URL build sites.

[aaime]

Please keep on familiar grounds and expose a method just like the GeoServer one instead:

    /**
     * Callback that can change the contents of the baseURL, the path or the KVP map
     *
     * @param baseURL the base URL, containing host, port and application
     * @param path after the application name
     * @param kvp the GET request parameters
     * @param type URL type (External, resource or service) for consideration during mangling
     */
    public void mangleURL(StringBuilder baseURL, StringBuilder path, Map<String, String> kvp, URLType type);

The kvp is meant to be modified by the callers.

[aaime]

Eugh, this is somewhat painful to read... I think it would read bettter if you had it in WMTSUrls instead, something like:

      public String serviceMetadataUrl() {
          return WMTSUtils.getKvpServiceMetadataURL(serviceBaseUrl, serviceQueryParameters);
      }

      public String withServiceQuery(String url) {
          return WMTSUtils.appendQueryParameters(url, serviceQueryParameters);
      }

      public String withRestQuery(String url) {
          return WMTSUtils.appendQueryParameters(url, restQueryParameters);

[fernandor777]

Thanks @aaime for your review and directions. I performed several refactors in orderr to move forward to the desired reestructuring for this fix, mainly the alignment with GeoServer url mangler and code cleaning. Please let me know if this is the right direction and what would be still missing to achieve the PR approval.

[aaime]

The changes here look good, but when I build here, and then try to build the downstream PR, I get a compile error:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.1:compile (default-compile) on project gs-gwc: Compilation failure
[ERROR] /home/aaime/devel/git-gs/src/gwc/src/main/java/org/geoserver/gwc/ResponseUtilsURLMangler.java:[17,105] cannot find symbol
[ERROR]   symbol:   class URLType
[ERROR]   location: interface org.geowebcache.util.URLMangler
[ERROR] 

I have not merged because of it, can you double check?

[fernandor777]

I rebased and pushed the updates to the GeoServer PR branch:
geoserver/geoserver#9697

I testesd the build locally and tests passed. @aaime please would you like to give it another check?

[aaime]

Looks good, I tested builds both here and in the follow-up GeoServer message. Merging.