Merge pull request #3508 from mapfish/backport/3497-to-master

[Backport master] Use getRawStatusCode to avoid getting an exception on fake status code 999, convert it to 406 Not Acceptable

core/src/main/java/org/mapfish/print/http/ErrorResponseClientHttpResponse.java

@@ -9,8 +9,8 @@
 public class ErrorResponseClientHttpResponse extends AbstractClientHttpResponse {
   private final Exception exception;
 
-  /** HTTP code use in response for non HTTP errors. */
-  private static final int FAKE_HTTP_ERROR_CODE = 999;
+  /** HTTP code use in response for non HTTP errors, (Not Acceptable). */
+  private static final int FAKE_HTTP_ERROR_CODE = 406;
 
   public ErrorResponseClientHttpResponse(final Exception e) {
     assert e != null;
@@ -38,7 +38,8 @@ public int getRawStatusCode() {
   @Nonnull
   public String getStatusText() {
     return String.format(
-        "%s: %s", this.exception.getClass().getSimpleName(), this.exception.getMessage());
+        "Not true HTTP code, %s: %s, see above error",
+        this.exception.getClass().getSimpleName(), this.exception.getMessage());
   }
 
   @Override

core/src/main/java/org/mapfish/print/map/geotools/FeaturesParser.java

@@ -61,7 +61,7 @@ public FeaturesParser(
   }
 
   @VisibleForTesting
-  static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
+  static CoordinateReferenceSystem parseCoordinateReferenceSystem(
       final MfClientHttpRequestFactory requestFactory,
       final JSONObject geojson,
       final boolean forceLongitudeFirst) {
@@ -93,7 +93,7 @@ static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
                   requestFactory.createRequest(new URI(uri), HttpMethod.GET);
               try (ClientHttpResponse response = request.execute()) {
 
-                if (response.getStatusCode() == HttpStatus.OK) {
+                if (response.getRawStatusCode() == HttpStatus.OK.value()) {
                   final String wkt =
                       IOUtils.toString(response.getBody(), Constants.DEFAULT_ENCODING);
                   try {
@@ -108,7 +108,8 @@ static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
               }
             }
           } else {
-            LOGGER.warn("Unable to load linked CRS from geojson: \n{}", crsJson);
+            LOGGER.warn(
+                "Unsupported link type {} in linked CRS from geojson: \n{}", linkType, crsJson);
           }
         } else {
           code.append(getProperty(crsJson, "code"));
@@ -147,7 +148,6 @@ private static String getProperty(final JSONObject crsJson, final String nameCod
    * @param template the template
    * @param features what to parse
    * @return the feature collection
-   * @throws IOException
    */
   public final SimpleFeatureCollection autoTreat(final Template template, final String features)
       throws IOException {
@@ -189,7 +189,6 @@ public final SimpleFeatureCollection treatStringAsURL(
    *
    * @param geoJsonString what to parse
    * @return the feature collection
-   * @throws IOException
    */
   public final SimpleFeatureCollection treatStringAsGeoJson(final String geoJsonString)
       throws IOException {
@@ -231,7 +230,7 @@ private SimpleFeatureType createFeatureType(@Nonnull final String geojsonData) {
         builder.setName("GeosjonFeatureType");
         final JSONArray features = geojson.getJSONArray("features");
 
-        if (features.length() == 0) {
+        if (features.isEmpty()) {
           // do not try to build the feature type if there are no features
           return null;
         }

core/src/main/java/org/mapfish/print/map/image/AbstractSingleImageLayer.java

@@ -202,14 +202,15 @@ private boolean isResponseStatusCodeValid(
       final String stringBody,
       final String baseMetricName)
       throws IOException {
-    if (httpResponse.getStatusCode() != HttpStatus.OK) {
+    if (httpResponse.getRawStatusCode() != HttpStatus.OK.value()) {
       String message =
           String.format(
-              "Invalid status code for %s (%d!=%d).With request headers:\n%s\n"
+              "Invalid status code for %s (%d!=%d), status: %s. With request headers:\n%s\n"
                   + "The response was: '%s'\nWith response headers:\n%s",
               request.getURI(),
-              httpResponse.getStatusCode().value(),
+              httpResponse.getRawStatusCode(),
               HttpStatus.OK.value(),
+              httpResponse.getStatusText(),
               String.join("\n", Utils.getPrintableHeadersList(request.getHeaders())),
               httpResponse.getStatusText(),
               String.join("\n", Utils.getPrintableHeadersList(httpResponse.getHeaders())));

core/src/main/java/org/mapfish/print/map/style/ParserPluginUtils.java

@@ -34,7 +34,7 @@ public static Optional<Style> loadStyleAsURI(
       final ClientHttpRequestFactory clientHttpRequestFactory,
       final String styleRef,
       final Function<byte[], @Nullable Optional<Style>> loadFunction) {
-    HttpStatus statusCode;
+    int httpStatusCode;
     final byte[] input;
 
     URI uri;
@@ -47,13 +47,13 @@ public static Optional<Style> loadStyleAsURI(
     try {
       final ClientHttpRequest request = clientHttpRequestFactory.createRequest(uri, HttpMethod.GET);
       try (ClientHttpResponse response = request.execute()) {
-        statusCode = response.getStatusCode();
+        httpStatusCode = response.getRawStatusCode();
         input = IOUtils.toByteArray(response.getBody());
       }
     } catch (Exception e) {
       return Optional.empty();
     }
-    if (statusCode == HttpStatus.OK) {
+    if (httpStatusCode == HttpStatus.OK.value()) {
       return loadFunction.apply(input);
     } else {
       return Optional.empty();

core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java

@@ -254,17 +254,18 @@ protected Tile compute() {
 
     private Tile handleSpecialStatuses(
         final ClientHttpResponse response, final String baseMetricName) throws IOException {
-      final HttpStatus statusCode = response.getStatusCode();
-      if (statusCode == HttpStatus.NO_CONTENT || statusCode == HttpStatus.NOT_FOUND) {
-        if (statusCode == HttpStatus.NOT_FOUND) {
+      final int httpStatusCode = response.getRawStatusCode();
+      if (httpStatusCode == HttpStatus.NO_CONTENT.value()
+          || httpStatusCode == HttpStatus.NOT_FOUND.value()) {
+        if (httpStatusCode == HttpStatus.NOT_FOUND.value()) {
           LOGGER.info(
               "The request {} returns a not found status code, we consider it as an empty tile.",
               this.tileRequest.getURI());
         }
         // Empty response, nothing special to do
         return new Tile(null, getTileIndexX(), getTileIndexY());
-      } else if (statusCode != HttpStatus.OK) {
-        return handleNonOkStatus(statusCode, response, baseMetricName);
+      } else if (httpStatusCode != HttpStatus.OK.value()) {
+        return handleNonOkStatus(response, baseMetricName);
       }
       return null;
     }
@@ -281,19 +282,19 @@ private BufferedImage getImageFromResponse(
       return image;
     }
 
-    private Tile handleNonOkStatus(
-        final HttpStatus statusCode, final ClientHttpResponse response, final String baseMetricName)
+    private Tile handleNonOkStatus(final ClientHttpResponse response, final String baseMetricName)
         throws IOException {
+      final int httpStatusCode = response.getRawStatusCode();
       String errorMessage =
           String.format(
-              "Error making tile request: %s\n\tStatus: %s\n\tStatus message: %s",
-              this.tileRequest.getURI(), statusCode, response.getStatusText());
+              "Error making tile request: %s\n\tStatus: %d\n\tStatus message: %s",
+              this.tileRequest.getURI(), httpStatusCode, response.getStatusText());
       LOGGER.debug(
           String.format(
-              "Error making tile request: %s\nStatus: %s\n"
+              "Error making tile request: %s\nStatus: %d\n"
                   + "Status message: %s\nServer:%s\nBody:\n%s",
               this.tileRequest.getURI(),
-              statusCode,
+              httpStatusCode,
               response.getStatusText(),
               response.getHeaders().getFirst(HttpHeaders.SERVER),
               IOUtils.toString(response.getBody(), StandardCharsets.UTF_8)));

core/src/main/java/org/mapfish/print/processor/jasper/HttpImageResolver.java

@@ -1,5 +1,6 @@
 package org.mapfish.print.processor.jasper;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.awt.image.BufferedImage;
 import java.io.IOException;
 import java.net.URI;
@@ -29,7 +30,7 @@ public final class HttpImageResolver implements TableColumnConverter<BufferedIma
   private static final int IMAGE_SIZE = 48;
   private Pattern urlExtractor = Pattern.compile("(.*)");
   private int urlGroup = 1;
-  private final BufferedImage defaultImage;
+  @VisibleForTesting final BufferedImage defaultImage;
 
   /** Constructor. */
   public HttpImageResolver() {
@@ -82,7 +83,7 @@ public BufferedImage resolve(final MfClientHttpRequestFactory requestFactory, fi
 
   private BufferedImage getImageFromResponse(final ClientHttpResponse response, final URI url)
       throws IOException {
-    if (response.getStatusCode() == HttpStatus.OK) {
+    if (response.getRawStatusCode() == HttpStatus.OK.value()) {
       try {
         final BufferedImage image = ImageIO.read(response.getBody());
         if (image == null) {
@@ -97,7 +98,7 @@ private BufferedImage getImageFromResponse(final ClientHttpResponse response, fi
       LOGGER.warn(
           "Error loading the table row image: {}.\nStatus Code: {}\nStatus Text: {}",
           url,
-          response.getStatusCode(),
+          response.getRawStatusCode(),
           response.getStatusText());
     }
     return this.defaultImage;

core/src/main/java/org/mapfish/print/processor/jasper/LegendProcessor.java

@@ -2,6 +2,7 @@
 
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Timer;
+import com.google.common.annotations.VisibleForTesting;
 import java.awt.Color;
 import java.awt.Dimension;
 import java.awt.Graphics2D;
@@ -161,7 +162,14 @@ private void createTasks(
     if (icons != null && icons.length > 0) {
       for (URL icon : icons) {
         tasks.add(
-            new IconTask(icon, dpi, context, level, tempTaskDirectory, clientHttpRequestFactory));
+            new IconTask(
+                icon,
+                dpi,
+                context,
+                level,
+                tempTaskDirectory,
+                clientHttpRequestFactory,
+                metricRegistry));
       }
     }
     if (legendAttributes.classes != null) {
@@ -269,7 +277,8 @@ protected void extraValidation(
     // no checks needed
   }
 
-  private synchronized BufferedImage getMissingImage() {
+  @VisibleForTesting
+  synchronized BufferedImage getMissingImage() {
     if (this.missingImage == null) {
       this.missingImage =
           new BufferedImage(
@@ -311,7 +320,7 @@ public static final class Output {
     /** The datasource for the legend object in the report. */
     public final JRTableModelDataSource legendDataSource;
 
-    /** The path to the compiled subreport. */
+    /** The path to the compiled sub-report. */
     public final String legendSubReport;
 
     /** The number of rows in the legend. */
@@ -343,28 +352,33 @@ public Object[] call() {
     }
   }
 
-  private final class IconTask implements Callable<Object[]> {
+  @VisibleForTesting
+  final class IconTask implements Callable<Object[]> {
 
     private final URL icon;
     private final double iconDPI;
     private final ExecutionContext context;
     private final MfClientHttpRequestFactory clientHttpRequestFactory;
     private final int level;
     private final File tempTaskDirectory;
+    private final MetricRegistry metricRegistry;
 
-    private IconTask(
+    @VisibleForTesting
+    IconTask(
         final URL icon,
         final double iconDPI,
         final ExecutionContext context,
         final int level,
         final File tempTaskDirectory,
-        final MfClientHttpRequestFactory clientHttpRequestFactory) {
+        final MfClientHttpRequestFactory clientHttpRequestFactory,
+        final MetricRegistry metricRegistry) {
       this.icon = icon;
       this.iconDPI = iconDPI;
       this.context = context;
       this.level = level;
       this.clientHttpRequestFactory = clientHttpRequestFactory;
       this.tempTaskDirectory = tempTaskDirectory;
+      this.metricRegistry = metricRegistry;
     }
 
     @Override
@@ -389,10 +403,9 @@ private BufferedImage loadImage(final URI uri) {
           this.context.stopIfCanceled();
           final ClientHttpRequest request =
               this.clientHttpRequestFactory.createRequest(uri, HttpMethod.GET);
-          try (Timer.Context ignored =
-              LegendProcessor.this.metricRegistry.timer(metricName).time()) {
+          try (Timer.Context ignored = metricRegistry.timer(metricName).time()) {
             try (ClientHttpResponse httpResponse = request.execute()) {
-              if (httpResponse.getStatusCode() == HttpStatus.OK) {
+              if (httpResponse.getRawStatusCode() == HttpStatus.OK.value()) {
                 image = ImageIO.read(httpResponse.getBody());
                 if (image == null) {
                   LOGGER.warn("There is no image in this response body {}", httpResponse.getBody());
@@ -409,7 +422,7 @@ private BufferedImage loadImage(final URI uri) {
 
       if (image == null) {
         image = getMissingImage();
-        LegendProcessor.this.metricRegistry.counter(metricName + ".error").inc();
+        metricRegistry.counter(metricName + ".error").inc();
       }
 
       return image;
@@ -422,7 +435,7 @@ private void logNotOkResponseStatus(final ClientHttpResponse httpResponse) throw
               + "\tResponse Text: {}\n"
               + "\tWith Headers:\n\t{}",
           this.icon,
-          httpResponse.getStatusCode(),
+          httpResponse.getRawStatusCode(),
           httpResponse.getStatusText(),
           String.join("\n\t", Utils.getPrintableHeadersList(httpResponse.getHeaders())));
     }

core/src/test/java/org/mapfish/print/map/geotools/FeaturesParserTest.java

@@ -3,6 +3,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -36,6 +37,10 @@ public class FeaturesParserTest extends AbstractMapfishSpringTest {
 
   private static final String EXAMPLE_GEOJSONFILE =
       "geojson/geojson-inconsistent-attributes-2.json";
+  private static final String WKT =
+      "GEOGCS[\"GCS_WGS_1984\",DATUM[\"D_WGS_1984\""
+          + ",SPHEROID[\"WGS_1984\",6378137,298.257223563]]"
+          + ",PRIMEM[\"Greenwich\",0],UNIT[\"Degree\",0.017453292519943295]]";
   @Autowired private TestHttpClientFactory requestFactory;
   @Autowired private ConfigurationFactory configurationFactory;
 
@@ -131,19 +136,32 @@ public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
   @Test
   @DirtiesContext
   public void testParseCRSLinkEsriWkt() {
+    final MockClientHttpResponse clientHttpResponse =
+        new MockClientHttpResponse(WKT.getBytes(Constants.DEFAULT_CHARSET), HttpStatus.OK);
+
+    CoordinateReferenceSystem crs = parseCoordinateReferenceSystemFromResponse(clientHttpResponse);
+    assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
+  }
+
+  @Test
+  @DirtiesContext
+  public void testParseCRSLinkEsriWktWithUnsupportedErrorCode() {
+    final MockClientHttpResponse clientHttpResponse =
+        new MockClientHttpResponse(WKT.getBytes(Constants.DEFAULT_CHARSET), 999);
+
+    CoordinateReferenceSystem crs = parseCoordinateReferenceSystemFromResponse(clientHttpResponse);
+    assertSame(DefaultEngineeringCRS.GENERIC_2D, crs);
+  }
+
+  private CoordinateReferenceSystem parseCoordinateReferenceSystemFromResponse(
+      final MockClientHttpResponse clientHttpResponse) {
     requestFactory.registerHandler(
         input -> input != null && input.getHost().equals("spatialreference.org"),
         new TestHttpClientFactory.Handler() {
           @Override
           public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
-            String wkt =
-                "GEOGCS[\"GCS_WGS_1984\",DATUM[\"D_WGS_1984\",SPHEROID[\"WGS_1984\",6378137,298"
-                    + ".257223563]],"
-                    + "PRIMEM[\"Greenwich\","
-                    + "0],UNIT[\"Degree\",0.017453292519943295]]";
             MockClientHttpRequest mockClientHttpRequest = new MockClientHttpRequest();
-            mockClientHttpRequest.setResponse(
-                new MockClientHttpResponse(wkt.getBytes(Constants.DEFAULT_CHARSET), HttpStatus.OK));
+            mockClientHttpRequest.setResponse(clientHttpResponse);
             return mockClientHttpRequest;
           }
         });
@@ -161,10 +179,11 @@ public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
     CoordinateReferenceSystem crs =
         FeaturesParser.parseCoordinateReferenceSystem(this.requestFactory, crsJSON, false);
     assertNotNull(crs);
-    assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
+    return crs;
   }
 
-  public void testParseCRSLinkProj4() {
+  @Test
+  public void testUnsupportedLinkTypeProj4() {
     JSONObject crsJSON =
         new JSONObject(
             "{\"crs\":{\n"
@@ -179,7 +198,7 @@ public void testParseCRSLinkProj4() {
     CoordinateReferenceSystem crs =
         FeaturesParser.parseCoordinateReferenceSystem(this.requestFactory, crsJSON, false);
     assertNotNull(crs);
-    assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
+    assertSame(DefaultEngineeringCRS.GENERIC_2D, crs);
   }
 
   @Test