[GEOS-8187] Legend Preview not correctly interpreting SvgParameter

geoserver · Oct 28, 2017 · fa3bf81 · fa3bf81
1 parent cbd03ac
commit fa3bf81
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 84 deletions.
diff --git a/...schema/app-schema-test/src/test/java/org/geoserver/test/AbstractAppSchemaTestSupport.java b/...schema/app-schema-test/src/test/java/org/geoserver/test/AbstractAppSchemaTestSupport.java
@@ -587,42 +587,6 @@ protected int countNonBlankPixels(String testName, BufferedImage image, Color bg
         return pixelsDiffer;
     }
 
-    /**
-     * Checks the pixel i/j has the specified color
-     * @param image
-     * @param i
-     * @param j
-     * @param color
-     */
-    protected void assertPixel(BufferedImage image, int i, int j, Color color) {
-        Color actual = getPixelColor(image, i, j);
-
-
-        assertEquals(color, actual);
-    }
-
-    /**
-     * Gets a specific pixel color from the specified buffered image
-     * @param image
-     * @param i
-     * @param j
-     * @param color
-     *
-     */
-    protected Color getPixelColor(BufferedImage image, int i, int j) {
-        ColorModel cm = image.getColorModel();
-        Raster raster = image.getRaster();
-        Object pixel = raster.getDataElements(i, j, null);
-
-        Color actual;
-        if(cm.hasAlpha()) {
-            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), cm.getAlpha(pixel));
-        } else {
-            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), 255);
-        }
-        return actual;
-    }
-
     /**
      * Checks that the identifiers of the features in the provided collection match the specified ids.
      * 

diff --git a/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java b/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java
@@ -12,7 +12,10 @@
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
+import java.awt.*;
 import java.awt.image.BufferedImage;
+import java.awt.image.ColorModel;
+import java.awt.image.Raster;
 import java.awt.image.RenderedImage;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
@@ -1671,6 +1674,53 @@ protected void assertStatusCodeForRequest(int code, String method, String path,
         assertEquals(code, response.getErrorCode());
     }
 
+    /**
+     * Gets a specific pixel color from the specified buffered image
+     * @param image
+     * @param i
+     * @param j
+     * @param color
+     *
+     */
+    protected Color getPixelColor(BufferedImage image, int i, int j) {
+        ColorModel cm = image.getColorModel();
+        Raster raster = image.getRaster();
+        Object pixel = raster.getDataElements(i, j, null);
+
+        Color actual;
+        if(cm.hasAlpha()) {
+            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), cm.getAlpha(pixel));
+        } else {
+            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), 255);
+        }
+        return actual;
+    }
+
+    /**
+     * Checks the pixel i/j has the specified color
+     * @param image
+     * @param i
+     * @param j
+     * @param color
+     */
+    protected void assertPixel(BufferedImage image, int i, int j, Color color) {
+        Color actual = getPixelColor(image, i, j);
+
+
+        assertEquals(color, actual);
+    }
+
+    /**
+     * Checks the pixel i/j is fully transparent
+     * @param image
+     * @param i
+     * @param j
+     */
+    protected void assertPixelIsTransparent(BufferedImage image, int i, int j) {
+  	    int pixel = image.getRGB(i,j);
+        assertEquals(true, (pixel>>24) == 0x00);
+    }
+
     //
     // xml validation helpers
     //

diff --git a/src/web/wms/src/main/java/org/geoserver/wms/web/data/StyleAdminPanel.java b/src/web/wms/src/main/java/org/geoserver/wms/web/data/StyleAdminPanel.java
@@ -67,6 +67,7 @@
 import org.geoserver.wms.web.publish.StyleTypeModel;
 import org.geoserver.wms.web.publish.StylesModel;
 import org.geotools.styling.Style;
+import org.geotools.util.Version;
 import org.geotools.util.logging.Logging;
 
 /**
@@ -328,7 +329,8 @@ protected void onClick(AjaxRequestTarget target, Form<?> form) {
                     //No external legend, use generated legend
                     GeoServerDataDirectory dd = GeoServerExtensions.bean(GeoServerDataDirectory.class, stylePage.getGeoServerApplication().getApplicationContext());
                     StyleInfo si = new StyleInfoImpl(stylePage.getCatalog());
-                    si.setFormat(stylePage.getStyleInfo().getFormat());
+                    String format = stylePage.getStyleInfo().getFormat();
+                    si.setFormat(format);
                     String styleName = "tmp" + UUID.randomUUID().toString();
                     String styleFileName = styleName + '.' + Styles.handler(si.getFormat()).getFileExtension();
                     si.setFilename(styleFileName);
@@ -340,13 +342,17 @@ protected void onClick(AjaxRequestTarget target, Form<?> form) {
                         try(OutputStream os = styleResource.out()) {
                             IOUtils.write(stylePage.editor.getInput(), os);
                         }
+                        // guess the version, the style in the editor might be using one that's different from the
+                        // the one in the
+                        Version version = Styles.handler(format).version(stylePage.editor.getInput());
+                        si.setFormatVersion(version);
                         Style style = dd.parsedStyle(si);
                         if (style != null) {
                             GetLegendGraphicRequest request = new GetLegendGraphicRequest();
                             request.setLayer(null);
                             request.setStyle(style);
                             request.setStrict(false);
-                            Map<String, String> legendOptions = new HashMap<String, String>();
+                            Map<String, String> legendOptions = new HashMap<>();
                             legendOptions.put("forceLabels", "on");
                             legendOptions.put("fontAntiAliasing", "true");
                             request.setLegendOptions(legendOptions);

diff --git a/src/web/wms/src/test/java/org/geoserver/wms/web/data/StyleEditPageTest.java b/src/web/wms/src/test/java/org/geoserver/wms/web/data/StyleEditPageTest.java
@@ -11,6 +11,7 @@
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.junit.Assert.*;
 
+import java.awt.*;
 import java.io.ByteArrayInputStream;
 
 import java.io.FileReader;
@@ -26,6 +27,7 @@
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 
+import it.geosolutions.rendered.viewer.RenderedImageBrowser;
 import org.apache.commons.io.IOUtils;
 import org.apache.wicket.Component;
 import org.apache.wicket.Session;
@@ -586,4 +588,36 @@ public void testChangeWsDefaultStyle() {
         styleTest.startPage(page);
         styleTest.assertDisabled("styleForm:context:panel:workspace");
     }
+
+    @Test
+    public void testPreviewSLD11Legend() throws Exception {
+        String xml =
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                        "<StyledLayerDescriptor xmlns=\"http://www.opengis.net/sld\" version=\"1.1.0\" " +
+                        "xmlns:se=\"http://www.opengis.net/se\">\n" +
+                        "  <NamedLayer>\n" +
+                        "    <se:Name>ne_110m_admin_0_countries</se:Name>\n" +
+                        "    <UserStyle>\n" +
+                        "      <se:Name>ne_110m_admin_0_countries</se:Name>\n" +
+                        "      <se:FeatureTypeStyle>\n" +
+                        "        <se:Rule>\n" +
+                        "          <se:Name>Single symbol</se:Name>\n" +
+                        "          <se:PolygonSymbolizer>\n" +
+                        "            <se:Fill>\n" +
+                        "              <se:SvgParameter name=\"fill\">#ff0000</se:SvgParameter>\n" +
+                        "            </se:Fill>\n" +
+                        "          </se:PolygonSymbolizer>\n" +
+                        "        </se:Rule>\n" +
+                        "      </se:FeatureTypeStyle>\n" +
+                        "    </UserStyle>\n" +
+                        "  </NamedLayer>\n" +
+                        "</StyledLayerDescriptor>";
+
+        // tester.debugComponentTrees();
+        tester.newFormTester("styleForm").setValue("styleEditor:editorContainer:editorParent:editor", xml);
+        tester.clickLink("styleForm:context:panel:preview", true);
+        StyleAdminPanel panel = (StyleAdminPanel) tester.getComponentFromLastRenderedPage("styleForm:context:panel");
+        // check the SvgParameter has been interpreted and we get a red fill, not a gray one
+        assertPixel(panel.legendImage, 10, 10, Color.RED);
+    }
 }
diff --git a/src/wms/src/test/java/org/geoserver/wms/WMSTestSupport.java b/src/wms/src/test/java/org/geoserver/wms/WMSTestSupport.java
@@ -530,53 +530,7 @@ protected void checkImage(MockHttpServletResponse response, String mimeType, int
             fail("Could not read image returned from GetMap:" + t.getLocalizedMessage());
         }
     }
-
-    /**
-     * Checks the pixel i/j has the specified color
-     * @param image
-     * @param i
-     * @param j
-     * @param color
-     */
-    protected void assertPixel(BufferedImage image, int i, int j, Color color) {
-        Color actual = getPixelColor(image, i, j);
-
 
-        assertEquals(color, actual);
-    }
-
-    /**
-     * Checks the pixel i/j is fully transparent
-     * @param image
-     * @param i
-     * @param j
-     */
-    protected void assertPixelIsTransparent(BufferedImage image, int i, int j) {
-  	    int pixel = image.getRGB(i,j);
-        assertEquals(true, (pixel>>24) == 0x00);
-    }
-
-    /**
-     * Gets a specific pixel color from the specified buffered image
-     * @param image
-     * @param i
-     * @param j
-     * @param color
-     *
-     */
-    protected Color getPixelColor(BufferedImage image, int i, int j) {
-        ColorModel cm = image.getColorModel();
-        Raster raster = image.getRaster();
-        Object pixel = raster.getDataElements(i, j, null);
-
-        Color actual;
-        if(cm.hasAlpha()) {
-            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), cm.getAlpha(pixel));
-        } else {
-            actual = new Color(cm.getRed(pixel), cm.getGreen(pixel), cm.getBlue(pixel), 255);
-        }
-        return actual;
-    }
     /**
      * Sets up a template in a feature type directory.
      *