Allow ExternalGraphic image to be used for GetCapabilities and GetLegendGraphics

[jodygarnett]

This is a replacement for #400 adding LegendInfo to StyleInfo for use in WMS GetCapabilities generation. The data structure (if available) is also used in GetLegendGraphics.

• http://jira.codehaus.org/browse/GEOS-6425
• http://geoserver.org/display/GEOS/GSIP+111+GetLegendGraphicsRequest+Cleanup

The scope of this feature has been refined to restrict these external graphic references to images that are being served by GeoServer (for example from the styles directory). This allows GeoServer to still be responsible for all the information it is publishing, and ensure that the images are available for the GetLegendGraphics operation.

Update: Due to feedback from @aaime the above restriction is lifted, a warning will be provided in the user interface if you configure GeoServer with an absolute URL pointing to an external server. An absolute URL that starts with the base url is fine. During GetLegendGraphics any online resource that cannot be resolved will result in a warning in the logs, and the code will proceed with dynamic generation.

To do:

• Update ExternalGraphicPanel to restrict graphics to images in the styles directory
• Submit proposal to cleanup GetLegendGraphicsRequest (GSIP 111)
• Recording images in GetLegendGraphicRequest using resource name, change to use of style name to allow for the same data with two different styles
• Resolve images in GetLegendGraphicRequest as local file references (KVP parser has access to data directory) to allow local test cases to pass
• Test GetCapabilities
• Test GetLegendGraphic with LAYER and STYLE
• Test GetLegendGraphic with LAYER and STYLE (rescale)
• Test GetLegendGraphic with with no style (i.e. ensure default style picked up)
• Test GetLegendGraphic with with no layer (i.e. used as a style preview)
• Test GetLegendGraphic with group layer
• Update user docs
• Issue: Prepending of styles directory (see http://localhost:8080/geoserver/styles/http://visual.merriam-webster.com/images/earth/meteorology/station-model.jpg)
• Issue: Warnings not cleared in wicket form
• Issue: Validation not run on form submit (only when "update" link is clicked).
• Issue: Confirm assignment agreement for Updated patch to allow an ExternalGraphic to be used instead of the GetLegendGraphic service in the WMS GetCapabilities LegendURL #400

[jodygarnett]

Discussion with Andrea resulted in the request to relax the restriction on locating external graphic in the styles directory.

Compromise shown below checks the external graphic and offers a warning if it is located outside of GeoServer.

[jodygarnett]

Style preview (ie GetLegendGraphic request with no layer) working:

[jodygarnett]

To test the above branch:

1. From the styles page edit the "grass" style to have grass_fill.png
2. Click Auto-detect image size and type
3. Test WMS GetCapabilities links
4. Use Demo Request page to test use (no rescale)

http://localhost:8080/geoserver/wms?REQUEST=GetLegendGraphic&VERSION=1.0.0&FORMAT=image/png&WIDTH=32&HEIGHT=32&LAYER=tiger:poly_landmarks&STYLE=grass

1. Use Demo Request page to test with rescale

http://localhost:8080/geoserver/wms?REQUEST=GetLegendGraphic&VERSION=1.0.0&FORMAT=image/png&WIDTH=22&HEIGHT=22&LAYER=tiger:poly_landmarks&STYLE=grass

[jodygarnett]

Test with group layer.

[mike-es]

Jody, thanks for working on this. I'm sorry for not getting back to you sooner. I'll checkout your branch and do some testing.

Michael Romero

[jodygarnett]

Thanks @mike-es - um I actually have a question you may be able to help me with. I am happy with this pull request, but need to confirm that the original #400 is covered by a contribution agreement :)

The document is here: http://docs.geoserver.org/stable/en/developer/policies/committing.html

The PDF needs to be printed, signed and emailed. Perhaps you are in position to check if this has been done already?

[mike-es]

Mike Benowitz, the author of #400, went through the process of getting a contribution agreement signed for a different pull request (geotools/geotools#286). I believe that should cover him for #400 as well.

[jodygarnett]

Excellent! It will indeed. Okay so we have GSIP 111 to wait for and we should be good to go.

[mike-es]

Jody, I did some testing and relative paths for the image works well. I tried various things:

• Different image sizes: 32x32, 20x20, and 40x40
• Subdirectory in styles dir
• WMS Capabilities document, 1.3.0 and 1.1.1
• Layer group with default style as shown in image above and setting grass with grass_fill.png as the default.

I notice a couple issues with using a full URL:

• The proxy URL (or localhost:8080) is appended to the URL for WMS 1.3.0 and 1.0.0 capabilities document:
  http://localhost:8080/geoserver/styles/http://visual.merriam-webster.com/images/earth/meteorology/station-model.jpg
• There are 2 "Recommend use of styles..." warnings instead of just 1.
• Clicking "Discard legend" leaves a warning. Next click Submit, then go back to the style and the image is still there (this is the same for relative and full URL).

[aaime]

Jody and Mike, a contributor assignement signed for GeoTools does not cover GeoServer, the copyright assignment does not go to the same organisation (OsGeo vs OpenPlans).

[jodygarnett]

Added the issues to the todo list. It was my intention to provide "Recommended use of styles" warnings. The use of URLs are not served by GeoServer is a danger (as breaks web clients).

I do not understand your issue with generation of WMS Capabilities document creating a file. Edit your comment to provide more detail.

I may need some advice on handling of wicket warnings (I also note that validation occurs when clicking on the link, but not when hitting "Submit").

[mike-es]

I realized I should have tested WMS 1.1.1, not 1.0.0. You can disregard the comment about creating the file.

[aaime]

Any progress? How are we doing contribution agreement wise?

[jodygarnett]

I am still waiting.

On Saturday, April 19, 2014, Andrea Aime notifications@github.com wrote:

Any progress? How are we doing contribution agreement wise?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/551#issuecomment-40864738
.

Jody Garnett

[aaime]

Hi, it's been another month, are the copyright assignments moving on?

[jodygarnett]

@mike-es can you check on the paperwork please?

[mike-es]

I just sent an email to the person responsible for getting the signed agreement and CC'd both of you.

[aaime]

Are we still waiting on the agreements?

[jodygarnett]

I told management on my end that they were too slow...

Even with this agreement in-hand today there would be QA needed based on the issues @mike-es listed above. I guess I could just turn off wicket validation (rather than figure out how to cascade clear the warnings from the subform).

[aaime]

Hem... any progress with the CA?

[jodygarnett]

Um - yes there is progress. We changed out organisation - and this crew had already signed for GeoTools ...

So we should be good now?

[aaime]

License wise it would seem that way. The code base has shifted enough during the last six months that the pull request cannot be merged anymore though.

[aaime]

So... I guess this one will be merged for 2.8.x?

[jodygarnett]

Unless I get to it tomorrow :) Need to rebase and see if it is any good...

[aaime]

2.8.x ready to receive this (just sayin... :-p )

[aaime]

Another couple of months passed since last comment. Still in the pipeline for being worked on?

[jodygarnett]

Yeah I asked Travis to take it to a branch so we could both look at it:
https://github.com/boundlessgeo/geoserver/tree/legend-capabilities

Near as I can tell he got stuck in wicket code.

Jody Garnett

On 18 April 2015 at 10:21, Andrea Aime notifications@github.com wrote:

Another couple of months passed since last comment. Still in the pipeline
for being worked on?

—
Reply to this email directly or view it on GitHub
#551 (comment).

[aaime]

Any relationship between this pull request and this ticket?
https://osgeo-org.atlassian.net/browse/GEOS-7160

Maybe they are similar but not same, cannot decide :)

[jodygarnett]

I think this feature (ability to parse out a LegendGraphic from SLD) may in
fact be covered.

Jody Garnett

On 18 August 2015 at 04:57, Andrea Aime notifications@github.com wrote:

Any relationship between this pull request and this ticket?
https://osgeo-org.atlassian.net/browse/GEOS-7160

Maybe they are similar but not same, cannot decide :)

—
Reply to this email directly or view it on GitHub
#551 (comment).

[jodygarnett]

@tbarsballe thanks for the help on this one, can you review the checklist at the top of this pull request and cross off them items that were fixed?

I think we may be done?!

[tbarsballe]

Of the remaining unchecked issues, I did not explicitly fix (or test) any of them, although from general testing I think both "prepending of styles directory" and "validation on submit" are fixed now.

Other fixes:

• Wicket tests for Style Legend Graphic fixed - All the unit tests pass now
• Discard Legend button now works as expected
• Use buttons instead of links to show/hide the legend entry (for better consistency with the rest of the GeoServer UI.

[tbarsballe]

Looks like @jodygarnett added test cases for the remaining issues last week. Now that all the test cases pass, we can consider these issues fixed.

[tbarsballe]

#1237 broke the mergeability of this PR.

[tbarsballe]

Manualy squashed and merged to master: 83bb4d6

[bencaradocdavies]

@jodygarnett @tbarsballe please do not write test images to user home directory. Fixed in 4380977 on master. These images were not used in the test and I assume that they were for test development and inadvertently included in this PR. At the least, ensure that test images are confined to module target so they can be cleaned. See gt-render for examples of how to write test fixtures that enable interactive development of tests and by default write no files.

[jodygarnett]

Thanks Ben, sorry we missed that in such a long lived patch.
On Wed, Nov 4, 2015 at 4:16 AM Ben Caradoc-Davies notifications@github.com
wrote:

In
src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/GetLegendGraphicTest.java
#551 (comment):

•    BufferedImage image = getAsImage(base, "image/png");
  

•    Resource resource = getResourceLoader().get("styles/legend.png");
  

•    BufferedImage expected = ImageIO.read( resource.file() );
  

•    assertEquals( getPixelColor(expected,10,2).getRGB(), getPixelColor(image,10,2).getRGB() );
  

•    // test rescale
  

•    base = "wms?service=WMS&version=1.1.1&request=GetLegendGraphic" +
  

•            "&layer=sf:states&style=custom" +
  

•            "&format=image/png&width=16&height=16";
  

•    image = getAsImage(base, "image/png");
  

•    String user_home = System.getProperty("user.home");
  

•    File home = new File(user_home);
  

•    ImageIO.write(expected, "png",  new File(home,"expected.png"));
  

@jodygarnett https://github.com/jodygarnett @tbarsballe
https://github.com/tbarsballe please do not write test images to user
home directory. Fixed in 4380977
4380977
on master. These images were not used in the test and I assume that they
were for test development and inadvertently included in this PR. At the
least, ensure that test images are confined to module target so they can be
cleaned. See gt-render for examples of how to write test fixtures that
enable interactive development of tests and by default write no files.

—
Reply to this email directly or view it on GitHub
https://github.com/geoserver/geoserver/pull/551/files#r43839044.

Jody Garnett

[bencaradocdavies]

No worries, Jody.

On 04/11/15 17:11, Jody Garnett wrote:

Thanks Ben, sorry we missed that in such a long lived patch.

Ben Caradoc-Davies ben@transient.nz
Director
Transient Software Limited http://transient.nz/
New Zealand