Merge pull request #3284 from mapfish/gsmfp-22

Improve class management and bug fixes

core/.dockerignore

@@ -1,3 +1,3 @@
 *
 !docker/
-!jettyRunExtraFiles/
+!extraConfigFor/

core/Dockerfile

@@ -50,8 +50,7 @@ CMD ["/usr/local/tomcat/bin/docker-start-print"]
 
 FROM runner AS tester
 
-COPY jettyRunExtraFiles/mapfish-spring-application-context-override-acceptencetests.xml \
-  /usr/local/tomcat/webapps/ROOT/WEB-INF/classes/mapfish-spring-application-context-override.xml
+COPY extraConfigFor/acceptanceTests/mapfish-spring-application-context-override.xml /usr/local/tomcat/webapps/ROOT/WEB-INF/classes/
 
 FROM runner AS watcher
 

core/build.gradle

@@ -45,7 +45,7 @@ distributions {
 }
 
 
-def appDir = new File(project.buildDir, 'install')
+def appDir = new File(getLayout().getBuildDirectory().getAsFile().get(), 'install')
 installDist.doFirst {
     appDir.deleteDir()
 }

core/jettyRunExtraFiles/mapfish-spring-application-context-override-acceptencetests.xml → core/extraConfigFor/acceptanceTests/mapfish-spring-application-context-override.xml

@@ -1,12 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <beans xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-       xmlns:context="http://www.springframework.org/schema/context"
        xmlns:security="http://www.springframework.org/schema/security"
        xmlns="http://www.springframework.org/schema/beans"
        xsi:schemaLocation="
         http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
-        http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd
         http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security.xsd">
 
     <bean name="bcryptEncoder"

core/jettyRunExtraFiles/mapfish-spring-application-context-override.xml → core/extraConfigFor/docs/mapfish-spring-application-context-override.xml

@@ -12,9 +12,7 @@
     <context:property-placeholder system-properties-mode="FALLBACK" file-encoding="UTF-8"
                                   location="classpath:mapfish-spring.properties"/>
 
-    <bean id="mapPrinterFactory" class="org.mapfish.print.servlet.ServletMapPrinterFactory">
-        <property name="appsRootDirectory" value="${path_to_examples}"/>
-    </bean>
+    <bean id="mapPrinterFactory" class="org.mapfish.print.servlet.ServletMapPrinterFactory"/>
 
     <bean name="bcryptEncoder"
           class="org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder"/>

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

@@ -173,7 +173,9 @@ private ClientHttpResponse attemptToFetchResponse(
     ClientHttpResponse response = executeCallbacksAndRequest(requestUsed);
     if (response.getRawStatusCode() < 500) {
       LOGGER.debug(
-          "Fetching success URI resource {}, error code {}", getURI(), response.getRawStatusCode());
+          "Fetching success URI resource {}, status code {}",
+          getURI(),
+          response.getRawStatusCode());
       return response;
     }
     LOGGER.debug(

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

@@ -0,0 +1,42 @@
+package org.mapfish.print.http;
+
+import java.io.InputStream;
+import javax.annotation.Nonnull;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.client.AbstractClientHttpResponse;
+import org.springframework.util.StreamUtils;
+
+public class ErrorResponseClientHttpResponse extends AbstractClientHttpResponse {
+  private final Exception exception;
+
+  public ErrorResponseClientHttpResponse(final Exception e) {
+    assert e != null;
+    this.exception = e;
+  }
+
+  @Override
+  @Nonnull
+  public HttpHeaders getHeaders() {
+    return new HttpHeaders();
+  }
+
+  @Override
+  @Nonnull
+  public InputStream getBody() {
+    return StreamUtils.emptyInput();
+  }
+
+  @Override
+  public int getRawStatusCode() {
+    return 500;
+  }
+
+  @Override
+  @Nonnull
+  public String getStatusText() {
+    return exception.getMessage();
+  }
+
+  @Override
+  public void close() {}
+}

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

@@ -24,7 +24,6 @@
 import org.springframework.http.client.AbstractClientHttpResponse;
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.http.client.ClientHttpResponse;
-import org.springframework.util.StreamUtils;
 
 /**
  * Schedule tasks for caching Http Requests that can be run simultaneously.
@@ -139,7 +138,7 @@ private final class CachedClientHttpRequest implements ClientHttpRequest, Callab
     private final ClientHttpRequest originalRequest;
     private final Processor.ExecutionContext context;
     @Nullable private ClientHttpResponse response;
-    @Nullable private ForkJoinTask<Void> future;
+    private ForkJoinTask<Void> future;
 
     private CachedClientHttpRequest(
         final ClientHttpRequest request, final Processor.ExecutionContext context) {
@@ -155,7 +154,7 @@ public HttpMethod getMethod() {
     @Override
     @Nonnull
     public String getMethodValue() {
-      final HttpMethod method = this.originalRequest.getMethod();
+      final HttpMethod method = getMethod();
       return method != null ? method.name() : "";
     }
 
@@ -182,13 +181,10 @@ public OutputStream getBody() {
     @Nonnull
     public ClientHttpResponse execute() {
       assert this.future != null;
-      final Timer.Context timerWait =
-          HttpRequestFetcher.this
-              .registry
-              .timer(HttpRequestFetcher.class.getName() + ".waitDownloader")
-              .time();
-      this.future.join();
-      timerWait.stop();
+      Timer timerWait = HttpRequestFetcher.this.registry.timer(buildMetricName(".waitDownloader"));
+      try (Timer.Context ignored = timerWait.time()) {
+        this.future.join();
+      }
       assert this.response != null;
       LOGGER.debug("Loading cached URI resource {}", this.originalRequest.getURI());
 
@@ -200,58 +196,32 @@ public ClientHttpResponse execute() {
       return result;
     }
 
+    private String buildMetricName(final String suffix) {
+      return HttpRequestFetcher.class.getName() + suffix;
+    }
+
     @Override
     public Void call() throws Exception {
       return context.mdcContextEx(
           () -> {
             final String baseMetricName =
-                HttpRequestFetcher.class.getName()
-                    + ".read."
-                    + StatsUtils.quotePart(getURI().getHost());
-            final Timer.Context timerDownload =
-                HttpRequestFetcher.this.registry.timer(baseMetricName).time();
-            try {
-              context.stopIfCanceled();
-              this.response = new CachedClientHttpResponse(this.originalRequest.execute());
-            } catch (IOException e) {
-              LOGGER.error("Request failed {}", this.originalRequest.getURI(), e);
-              this.response =
-                  new AbstractClientHttpResponse() {
-                    @Override
-                    @Nonnull
-                    public HttpHeaders getHeaders() {
-                      return new HttpHeaders();
-                    }
-
-                    @Override
-                    @Nonnull
-                    public InputStream getBody() {
-                      return StreamUtils.emptyInput();
-                    }
-
-                    @Override
-                    public int getRawStatusCode() {
-                      return 500;
-                    }
-
-                    @Override
-                    @Nonnull
-                    public String getStatusText() {
-                      return e.getMessage();
-                    }
-
-                    @Override
-                    public void close() {}
-                  };
-              HttpRequestFetcher.this.registry.counter(baseMetricName + ".error").inc();
-            } finally {
-              timerDownload.stop();
+                buildMetricName(".read." + StatsUtils.quotePart(getURI().getHost()));
+            Timer timerDownload = HttpRequestFetcher.this.registry.timer(baseMetricName);
+            try (Timer.Context ignored = timerDownload.time()) {
+              try {
+                context.stopIfCanceled();
+                this.response = new CachedClientHttpResponse(this.originalRequest.execute());
+              } catch (IOException | RuntimeException e) {
+                LOGGER.error("Request failed {}", this.originalRequest.getURI(), e);
+                this.response = new ErrorResponseClientHttpResponse(e);
+                HttpRequestFetcher.this.registry.counter(baseMetricName + ".error").inc();
+              }
             }
             return null;
           });
     }
 
-    public void setFuture(final ForkJoinTask<Void> future) {
+    public void setFuture(final @Nonnull ForkJoinTask<Void> future) {
       this.future = future;
     }
   }

core/src/main/java/org/mapfish/print/processor/AbstractProcessor.java

@@ -206,7 +206,7 @@ public String toString() {
   public static final class Context implements ExecutionContext {
     @Nonnull private final Map<String, String> mdcContext;
     private volatile boolean canceled = false;
-    private ExecutionStats stats = new ExecutionStats();
+    private final ExecutionStats stats = new ExecutionStats();
 
     /**
      * @param mdcContext The MDC context.

core/src/main/java/org/mapfish/print/processor/ExecutionStats.java

@@ -13,9 +13,9 @@
 
 /** Statisctics about the execution of a print job. */
 public class ExecutionStats {
-  private List<MapStats> mapStats = new ArrayList<>();
-  private List<PageStats> pageStats = new ArrayList<>();
-  private List<String> emailDests = new ArrayList<>();
+  private final List<MapStats> mapStats = new ArrayList<>();
+  private final List<PageStats> pageStats = new ArrayList<>();
+  private final List<String> emailDests = new ArrayList<>();
   private boolean storageUsed = false;
 
   /**

core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java

@@ -190,19 +190,25 @@ protected Values compute() {
                   final In inputParameter = ProcessorUtils.populateInputParameter(process, values);
 
                   Out output;
+                  boolean isThrowingException = false;
                   try {
                     LOGGER.debug("Executing process: {}", process);
                     output = process.execute(inputParameter, this.execContext.getContext());
                     LOGGER.debug("Succeeded in executing process: {}", process);
                   } catch (RuntimeException e) {
+                    isThrowingException = true;
+                    LOGGER.info("Error while executing process: {}", process, e);
                     throw e;
                   } catch (Exception e) {
+                    isThrowingException = true;
+                    LOGGER.info("Error while executing process: {}", process, e);
                     throw new PrintException("Failed to execute process:" + process, e);
                   } finally {
-                    // the processor is already canceled, so we don't care if something fails
-                    this.execContext.getContext().stopIfCanceled();
-                    LOGGER.info("Error while executing process: {}", process);
-                    registry.counter(name + ".error").inc();
+                    if (isThrowingException) {
+                      // the processor is already canceled, so we don't care if something fails
+                      this.execContext.getContext().stopIfCanceled();
+                      registry.counter(name + ".error").inc();
+                    }
                   }
 
                   if (output != null) {

core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java

@@ -741,7 +741,7 @@ && getSupportedRenderType(layers.get(i).getRenderType()) == renderType
               && imageBufferScaling == layers.get(i).getImageBufferScaling()) {
             // will always go there the first time
             l = layers.get(i);
-            LOGGER.debug("Adding layer {} to the group", l.getName());
+            LOGGER.debug("Adding layer: {} named: {} to the group", i, l.getName());
             group.layers.add(l);
             group.opaque = group.opaque || (renderType == RenderType.JPEG && l.getOpacity() == 1.0);
             ++i;

core/src/main/java/org/mapfish/print/processor/map/NorthArrowGraphic.java

@@ -8,21 +8,22 @@
 import java.awt.geom.AffineTransform;
 import java.awt.image.BufferedImage;
 import java.io.BufferedInputStream;
+import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import javax.imageio.ImageIO;
 import org.apache.batik.anim.dom.SAXSVGDocumentFactory;
 import org.apache.batik.anim.dom.SVGDOMImplementation;
 import org.apache.batik.dom.util.DOMUtilities;
 import org.apache.batik.util.SVGConstants;
 import org.apache.batik.util.XMLResourceDescriptor;
-import org.apache.commons.io.output.FileWriterWithEncoding;
 import org.apache.commons.lang3.StringUtils;
 import org.mapfish.print.FloatingPointUtil;
 import org.mapfish.print.ImageUtils;
@@ -341,10 +342,8 @@ private static SVGElement parseSvg(final InputStream inputStream) throws IOExcep
   private static File writeSvgToFile(final Document document, final File workingDir)
       throws IOException {
     final File path = File.createTempFile("north-arrow-", ".svg", workingDir);
-    try (FileWriterWithEncoding fw =
-        new FileWriterWithEncoding(path, Charset.forName("UTF-8").newEncoder())) {
+    try (BufferedWriter fw = new BufferedWriter(new FileWriter(path, StandardCharsets.UTF_8))) {
       DOMUtilities.writeDocument(document, fw);
-      fw.flush();
     }
     return path;
   }

core/src/main/java/org/mapfish/print/servlet/BaseMapServlet.java

@@ -2,9 +2,6 @@
 
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
-import java.util.Date;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.mapfish.print.PrintException;
@@ -17,38 +14,6 @@ public abstract class BaseMapServlet {
   private static final Logger LOGGER = LoggerFactory.getLogger(BaseMapServlet.class);
   private int cacheDurationInSeconds = 3600;
 
-  /**
-   * Remove commas and whitespace from a string.
-   *
-   * @param original the starting string.
-   */
-  protected static String cleanUpName(final String original) {
-    return original.replace(",", "").replaceAll("\\s+", "_");
-  }
-
-  /**
-   * Update a variable name with a date if the variable is detected as being a date.
-   *
-   * @param variableName the variable name.
-   * @param date the date to replace the value with if the variable is a date variable.
-   */
-  public static String findReplacement(final String variableName, final Date date) {
-    if (variableName.equalsIgnoreCase("date")) {
-      return cleanUpName(DateFormat.getDateInstance().format(date));
-    } else if (variableName.equalsIgnoreCase("datetime")) {
-      return cleanUpName(DateFormat.getDateTimeInstance().format(date));
-    } else if (variableName.equalsIgnoreCase("time")) {
-      return cleanUpName(DateFormat.getTimeInstance().format(date));
-    } else {
-      try {
-        return new SimpleDateFormat(variableName).format(date);
-      } catch (Exception e) {
-        LOGGER.error("Unable to format timestamp according to pattern: {}", variableName, e);
-        return "${" + variableName + "}";
-      }
-    }
-  }
-
   /**
    * Send an error to the client with a message.
    *
@@ -82,23 +47,6 @@ protected static void setNoCache(final HttpServletResponse response) {
     response.setHeader("Cache-Control", "max-age=0, must-revalidate, no-cache, no-store");
   }
 
-  /**
-   * Send an error to the client with an exception.
-   *
-   * @param httpServletResponse the http response to send the error to
-   * @param e the error that occurred
-   */
-  protected final void error(final HttpServletResponse httpServletResponse, final Throwable e) {
-    httpServletResponse.setContentType("text/plain");
-    httpServletResponse.setStatus(HttpStatus.INTERNAL_SERVER_ERROR.value());
-    try (PrintWriter out = httpServletResponse.getWriter()) {
-      out.println("Error while processing request:");
-      LOGGER.warn("Error while processing request", e);
-    } catch (IOException ex) {
-      throw new PrintException("", e);
-    }
-  }
-
   /**
    * Returns the base URL of the print servlet.
    *