From a66c40a70c043caf8700824be5ade4425d055b21 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 22 May 2024 09:45:59 +0200 Subject: [PATCH 1/9] Close resources and refactoring required to do it. Minor fixes. --- .../map/image/AbstractSingleImageLayer.java | 156 +++++++++++------ .../mapfish/print/map/tiled/CoverageTask.java | 162 ++++++++++-------- 2 files changed, 188 insertions(+), 130 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/map/image/AbstractSingleImageLayer.java b/core/src/main/java/org/mapfish/print/map/image/AbstractSingleImageLayer.java index 3e170d067..419916e4d 100644 --- a/core/src/main/java/org/mapfish/print/map/image/AbstractSingleImageLayer.java +++ b/core/src/main/java/org/mapfish/print/map/image/AbstractSingleImageLayer.java @@ -152,74 +152,116 @@ protected BufferedImage createErrorImage(final Rectangle area) { protected BufferedImage fetchImage( @Nonnull final ClientHttpRequest request, @Nonnull final MapfishMapContext transformer) throws IOException { - final String baseMetricName = - getClass().getName() + ".read." + StatsUtils.quotePart(request.getURI().getHost()); - final Timer.Context timerDownload = this.registry.timer(baseMetricName).time(); - try (ClientHttpResponse httpResponse = request.execute()) { - final List contentType = httpResponse.getHeaders().get("Content-Type"); - String stringBody = null; - if (contentType == null || contentType.size() != 1) { - LOGGER.debug("The image {} didn't return a valid content type header.", request.getURI()); - } else if (!contentType.get(0).startsWith("image/")) { - final byte[] data; - try (InputStream body = httpResponse.getBody()) { - data = IOUtils.toByteArray(body); - } - stringBody = new String(data, StandardCharsets.UTF_8); - } + final String baseMetricName = getBaseMetricName(request); + try (Timer.Context ignored = this.registry.timer(baseMetricName).time()) { + try (ClientHttpResponse httpResponse = request.execute()) { + final List contentType = httpResponse.getHeaders().get("Content-Type"); + final String invalidRespBody = getInvalidResponseBody(request, contentType, httpResponse); - if (httpResponse.getStatusCode() != HttpStatus.OK) { - String message = - String.format( - "Invalid status code for %s (%d!=%d).With request headers:\n%s\n" - + "The response was: '%s'\nWith response headers:\n%s", - request.getURI(), - httpResponse.getStatusCode().value(), - HttpStatus.OK.value(), - String.join("\n", Utils.getPrintableHeadersList(request.getHeaders())), - httpResponse.getStatusText(), - String.join("\n", Utils.getPrintableHeadersList(httpResponse.getHeaders()))); - if (stringBody != null) { - message += "\nContent:\n" + stringBody; + if (!isResponseStatusCodeValid(request, httpResponse, invalidRespBody, baseMetricName)) { + return createErrorImage(transformer.getPaintArea()); } - this.registry.counter(baseMetricName + ".error").inc(); - if (getFailOnError()) { - throw new RuntimeException(message); - } else { - LOGGER.warn(message); + + if (!isResponseBodyValid(invalidRespBody, request, contentType, baseMetricName)) { return createErrorImage(transformer.getPaintArea()); } + + return fetchImageFromHttpResponse(request, httpResponse, transformer, baseMetricName); + } catch (RuntimeException e) { + this.registry.counter(baseMetricName + ".error").inc(); + throw e; } + } + } + private String getBaseMetricName(@Nonnull final ClientHttpRequest request) { + return getClass().getName() + ".read." + StatsUtils.quotePart(request.getURI().getHost()); + } + + private static String getInvalidResponseBody( + final ClientHttpRequest request, + final List contentType, + final ClientHttpResponse httpResponse) + throws IOException { + if (contentType == null || contentType.size() != 1) { + LOGGER.debug("The image {} didn't return a valid content type header.", request.getURI()); + } else if (!contentType.get(0).startsWith("image/")) { + final byte[] data; + try (InputStream body = httpResponse.getBody()) { + data = IOUtils.toByteArray(body); + } + return new String(data, StandardCharsets.UTF_8); + } + return null; + } + + private boolean isResponseStatusCodeValid( + final ClientHttpRequest request, + final ClientHttpResponse httpResponse, + final String stringBody, + final String baseMetricName) + throws IOException { + if (httpResponse.getStatusCode() != HttpStatus.OK) { + String message = + String.format( + "Invalid status code for %s (%d!=%d).With request headers:\n%s\n" + + "The response was: '%s'\nWith response headers:\n%s", + request.getURI(), + httpResponse.getStatusCode().value(), + HttpStatus.OK.value(), + String.join("\n", Utils.getPrintableHeadersList(request.getHeaders())), + httpResponse.getStatusText(), + String.join("\n", Utils.getPrintableHeadersList(httpResponse.getHeaders()))); if (stringBody != null) { - LOGGER.debug( - "We get a wrong image for {}, content type: {}\nresult:\n{}", - request.getURI(), - contentType.get(0), - stringBody); - this.registry.counter(baseMetricName + ".error").inc(); - if (getFailOnError()) { - throw new RuntimeException("Wrong content-type : " + contentType.get(0)); - } else { - return createErrorImage(transformer.getPaintArea()); - } + message += "\nContent:\n" + stringBody; + } + this.registry.counter(baseMetricName + ".error").inc(); + if (getFailOnError()) { + throw new RuntimeException(message); + } else { + LOGGER.warn(message); + return false; } + } + return true; + } - final BufferedImage image = ImageIO.read(httpResponse.getBody()); - if (image == null) { - LOGGER.warn("Cannot read image from {}", request.getURI()); - this.registry.counter(baseMetricName + ".error").inc(); - if (getFailOnError()) { - throw new RuntimeException("Cannot read image from " + request.getURI()); - } else { - return createErrorImage(transformer.getPaintArea()); - } + private boolean isResponseBodyValid( + final String responseBody, + final ClientHttpRequest request, + final List contentType, + final String baseMetricName) { + if (responseBody != null) { + LOGGER.debug( + "We get a wrong image for {}, content type: {}\nresult:\n{}", + request.getURI(), + contentType.get(0), + responseBody); + this.registry.counter(baseMetricName + ".error").inc(); + if (getFailOnError()) { + throw new RuntimeException("Wrong content-type : " + contentType.get(0)); + } else { + return false; } - timerDownload.stop(); - return image; - } catch (RuntimeException e) { + } + return true; + } + + private BufferedImage fetchImageFromHttpResponse( + final ClientHttpRequest request, + final ClientHttpResponse httpResponse, + final MapfishMapContext transformer, + final String baseMetricName) + throws IOException { + final BufferedImage image = ImageIO.read(httpResponse.getBody()); + if (image == null) { + LOGGER.warn("Cannot read image from {}", request.getURI()); this.registry.counter(baseMetricName + ".error").inc(); - throw e; + if (getFailOnError()) { + throw new RuntimeException("Cannot read image from " + request.getURI()); + } + return createErrorImage(transformer.getPaintArea()); } + return image; } } diff --git a/core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java b/core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java index 6e8667ca2..381191dcd 100644 --- a/core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java +++ b/core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java @@ -53,7 +53,7 @@ public final class CoverageTask implements Callable { * @param tileCacheInfo the object used to create the tile requests. * @param configuration the configuration. */ - protected CoverageTask( + CoverageTask( @Nonnull final TilePreparationInfo tilePreparationInfo, final boolean failOnError, @Nonnull final MetricRegistry registry, @@ -87,25 +87,7 @@ public GridCoverage2D call() { Graphics2D graphics = coverageImage.createGraphics(); try { for (SingleTilePreparationInfo tileInfo : this.tilePreparationInfo.getSingleTiles()) { - final TileTask task; - if (tileInfo.getTileRequest() != null) { - task = - new SingleTileLoaderTask( - tileInfo.getTileRequest(), - this.errorImage, - tileInfo.getTileIndexX(), - tileInfo.getTileIndexY(), - this.failOnError, - this.registry, - this.context); - } else { - task = - new PlaceHolderImageTask( - this.tiledLayer.getMissingTileImage(), - tileInfo.getTileIndexX(), - tileInfo.getTileIndexY()); - } - Tile tile = task.call(); + final Tile tile = getTile(tileInfo); if (tile.getImage() != null) { // crop the image here BufferedImage noBufferTileImage; @@ -154,6 +136,28 @@ public GridCoverage2D call() { } } + private Tile getTile(final SingleTilePreparationInfo tileInfo) { + final TileTask task; + if (tileInfo.getTileRequest() != null) { + task = + new SingleTileLoaderTask( + tileInfo.getTileRequest(), + this.errorImage, + tileInfo.getTileIndexX(), + tileInfo.getTileIndexY(), + this.failOnError, + this.registry, + this.context); + } else { + task = + new PlaceHolderImageTask( + this.tiledLayer.getMissingTileImage(), + tileInfo.getTileIndexX(), + tileInfo.getTileIndexY()); + } + return task.call(); + } + /** Tile Task. */ public abstract static class TileTask extends RecursiveTask implements Callable { private final int tileIndexX; @@ -229,59 +233,78 @@ protected Tile compute() { + ".read." + StatsUtils.quotePart(this.tileRequest.getURI().getHost()); LOGGER.debug("{} -- {}", this.tileRequest.getMethod(), this.tileRequest.getURI()); - final Timer.Context timerDownload = this.registry.timer(baseMetricName).time(); - try (ClientHttpResponse response = this.tileRequest.execute()) { - final HttpStatus statusCode = response.getStatusCode(); - if (statusCode == HttpStatus.NO_CONTENT || statusCode == HttpStatus.NOT_FOUND) { - if (statusCode == HttpStatus.NOT_FOUND) { - LOGGER.info( - "The request {} returns a not found status code, we consider it as an " - + "empty tile.", - this.tileRequest.getURI()); + try (Timer.Context timerDownload = this.registry.timer(baseMetricName).time()) { + try (ClientHttpResponse response = this.tileRequest.execute()) { + final Tile x = handleSpecialStatuses(response, baseMetricName); + if (x != null) { + return x; } - // Empty response, nothing special to do - return new Tile(null, getTileIndexX(), getTileIndexY()); - } else if (statusCode != HttpStatus.OK) { - String errorMessage = - String.format( - "Error making tile request: %s\n\tStatus: %s\n\tStatus message: %s", - this.tileRequest.getURI(), statusCode, response.getStatusText()); - LOGGER.debug( - String.format( - "Error making tile request: %s\nStatus: %s\n" - + "Status message: %s\nServer:%s\nBody:\n%s", - this.tileRequest.getURI(), - statusCode, - response.getStatusText(), - response.getHeaders().getFirst(HttpHeaders.SERVER), - IOUtils.toString(response.getBody(), StandardCharsets.UTF_8))); - this.registry.counter(baseMetricName + ".error").inc(); - if (this.failOnError) { - throw new RuntimeException(errorMessage); - } else { - LOGGER.info(errorMessage); - return new Tile(this.errorImage, getTileIndexX(), getTileIndexY()); - } - } - BufferedImage image = ImageIO.read(response.getBody()); - if (image == null) { - LOGGER.warn( - "The URL: {} is an image format that cannot be decoded", - this.tileRequest.getURI()); - image = this.errorImage; - this.registry.counter(baseMetricName + ".error").inc(); - } else { + BufferedImage image = getImageFromResponse(response, baseMetricName); timerDownload.stop(); - } - return new Tile(image, getTileIndexX(), getTileIndexY()); - } catch (IOException e) { - this.registry.counter(baseMetricName + ".error").inc(); - throw new PrintException("Failed to compute Coverage Task", e); + return new Tile(image, getTileIndexX(), getTileIndexY()); + } catch (IOException | RuntimeException e) { + this.registry.counter(baseMetricName + ".error").inc(); + throw new PrintException("Failed to compute Coverage Task", e); + } } }); } + + 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) { + 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); + } + return null; + } + + private BufferedImage getImageFromResponse( + final ClientHttpResponse response, final String baseMetricName) throws IOException { + BufferedImage image = ImageIO.read(response.getBody()); + if (image == null) { + LOGGER.warn( + "The URL: {} is an image format that cannot be decoded", this.tileRequest.getURI()); + image = this.errorImage; + this.registry.counter(baseMetricName + ".error").inc(); + } + return image; + } + + private Tile handleNonOkStatus( + final HttpStatus statusCode, final ClientHttpResponse response, final String baseMetricName) + throws IOException { + String errorMessage = + String.format( + "Error making tile request: %s\n\tStatus: %s\n\tStatus message: %s", + this.tileRequest.getURI(), statusCode, response.getStatusText()); + LOGGER.debug( + String.format( + "Error making tile request: %s\nStatus: %s\n" + + "Status message: %s\nServer:%s\nBody:\n%s", + this.tileRequest.getURI(), + statusCode, + response.getStatusText(), + response.getHeaders().getFirst(HttpHeaders.SERVER), + IOUtils.toString(response.getBody(), StandardCharsets.UTF_8))); + this.registry.counter(baseMetricName + ".error").inc(); + if (this.failOnError) { + throw new RuntimeException(errorMessage); + } else { + LOGGER.info(errorMessage); + return new Tile(this.errorImage, getTileIndexX(), getTileIndexY()); + } + } } /** PlaceHolder Tile Loader Task. */ @@ -319,13 +342,6 @@ public static final class Tile { /** The y index of the image. the y coordinate to draw this tile is yIndex * tileSizeY */ private final int yIndex; - /** - * Constructor. - * - * @param image - * @param xIndex - * @param yIndex - */ private Tile(final BufferedImage image, final int xIndex, final int yIndex) { this.image = image; this.xIndex = xIndex; From b3ee2424037d2052a80e78eb1a0469d035d0aa69 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 22 May 2024 19:04:05 +0200 Subject: [PATCH 2/9] Use docker compose instead of docker-compose --- Makefile | 6 +++--- README.md | 2 +- docker-compose.override.sample.yaml | 3 --- docker-compose.yaml | 3 --- examples/README.md | 6 +++--- .../src/test/java/org/mapfish/print/MetricsApiTest.java | 4 ++-- 6 files changed, 9 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 0ecdc1e90..3f78b8dca 100644 --- a/Makefile +++ b/Makefile @@ -50,11 +50,11 @@ tests: build-builder acceptance-tests-up: build .env # Required to avoid root ownership of reports folder mkdir -p examples/build/reports/ || true - docker-compose up --detach + docker compose up --detach .PHONY: acceptance-tests-run acceptance-tests-run: .env - docker-compose exec -T tests gradle \ + docker compose exec -T tests gradle \ --exclude-task=:core:spotbugsMain --exclude-task=:core:checkstyleMain \ --exclude-task=:core:spotbugsTest --exclude-task=:core:checkstyleTest --exclude-task=:core:testCLI \ :examples:integrationTest @@ -63,7 +63,7 @@ acceptance-tests-run: .env .PHONY: acceptance-tests-down acceptance-tests-down: .env - docker-compose down || true + docker compose down || true docker run --rm --volume=/tmp/geoserver-data:/mnt/geoserver_datadir camptocamp/geoserver \ bash -c 'rm -rf /mnt/geoserver_datadir/*' rmdir /tmp/geoserver-data diff --git a/README.md b/README.md index 36d926e0b..f7ac49e4b 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ make acceptance-tests-up Run the example: ```bash -docker-compose exec builder gradle print -PprintArgs="-config /src/examples/src/test/resources/examples/simple/config.yaml -spec /src/examples/src/test/resources/examples/simple/requestData.json -output /src/examples/output.pdf" +docker compose exec builder gradle print -PprintArgs="-config /src/examples/src/test/resources/examples/simple/config.yaml -spec /src/examples/src/test/resources/examples/simple/requestData.json -output /src/examples/output.pdf" ``` # To use in Eclipse diff --git a/docker-compose.override.sample.yaml b/docker-compose.override.sample.yaml index 1cb55d0f3..c0e4f26b4 100644 --- a/docker-compose.override.sample.yaml +++ b/docker-compose.override.sample.yaml @@ -1,6 +1,3 @@ ---- -version: '2.1' - services: builder: image: mapfish_print_builder diff --git a/docker-compose.yaml b/docker-compose.yaml index 67fb8e001..64a114db6 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,6 +1,3 @@ ---- -version: '2.1' - services: geoserver: image: camptocamp/geoserver:2.24.1 diff --git a/examples/README.md b/examples/README.md index 0b6019d8b..0eb0bba77 100644 --- a/examples/README.md +++ b/examples/README.md @@ -6,7 +6,7 @@ many of the mapfish-print options as possible. To run the integration tests: - docker-compose up -d + docker compose up -d The task is a gradle test task and more details on how to run single tests or subgroups of tests can be understood by referring to: @@ -17,7 +17,7 @@ understood by referring to: The test server includes a client which can be used for testing. To start the server, run: - docker-compose up -d + docker compose up -d In the docker-comose context GeoServer can be accessed at and MapFish Print can be accessed at @@ -27,7 +27,7 @@ MapFish Print can be accessed at By default the test server is in daemon mode, which mean that the servers will be run in a background thread and be shutdown when the build completes. In order to be able to run the tests in a IDE one can run: - docker-compose up -d + docker compose up -d This will start the test servers in non-daemon mode allowing one to start the server and then run tests in your IDE against that server for development. diff --git a/examples/src/test/java/org/mapfish/print/MetricsApiTest.java b/examples/src/test/java/org/mapfish/print/MetricsApiTest.java index 8ce4423f9..2cbc6f2e1 100644 --- a/examples/src/test/java/org/mapfish/print/MetricsApiTest.java +++ b/examples/src/test/java/org/mapfish/print/MetricsApiTest.java @@ -19,8 +19,8 @@ * * Should be run inside docker composition: * - * docker-compose up -d - * docker-compose exec tests gradle :examples:test + * docker compose up -d + * docker compose exec tests gradle :examples:test */ public class MetricsApiTest extends AbstractApiTest { From c97818fe72ffe71005747fa6985dee7448bc92c1 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 22 May 2024 19:25:31 +0200 Subject: [PATCH 3/9] Close Resource properly. Read it even when bytes are not all available at once. --- .../output/ResourceBundleClassLoader.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java b/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java index 3ff14a6e4..be22b4661 100644 --- a/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java +++ b/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java @@ -1,13 +1,15 @@ package org.mapfish.print.output; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.nio.charset.StandardCharsets; -/** This is use to load the utf-8 ResourceBundle files. */ +/** This is used to load the utf-8 ResourceBundle files. */ public class ResourceBundleClassLoader extends ClassLoader { private final File configDir; @@ -31,13 +33,24 @@ protected URL findResource(final String resource) { @Override public InputStream getResourceAsStream(final String resource) { - try { - final InputStream is = super.getResourceAsStream(resource); - byte[] bytes = new byte[is.available()]; - is.read(bytes); - return new ByteArrayInputStream(new String(bytes, "utf-8").getBytes("iso-8859-1")); + ByteArrayOutputStream buffer; + try (InputStream is = super.getResourceAsStream(resource)) { + if (is == null) { + throw new IllegalArgumentException("Resource not found: " + resource); + } + buffer = new ByteArrayOutputStream(); + int nRead; + byte[] data = new byte[1024]; + + while ((nRead = is.read(data, 0, data.length)) != -1) { + buffer.write(data, 0, nRead); + } + + buffer.flush(); } catch (IOException e) { throw new RuntimeException(e); } + return new ByteArrayInputStream( + buffer.toString(StandardCharsets.UTF_8).getBytes(StandardCharsets.ISO_8859_1)); } } From 9f3ad72f919d8fd91a88734d6803bbc60ef5bd87 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 22 May 2024 19:58:45 +0200 Subject: [PATCH 4/9] Refactoring to close Resource. Fix bug to match type in width expression --- .../map/style/json/JsonStyleParserHelper.java | 97 ++++++++++++------- 1 file changed, 61 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/map/style/json/JsonStyleParserHelper.java b/core/src/main/java/org/mapfish/print/map/style/json/JsonStyleParserHelper.java index 98b46cbaf..3c761b73c 100644 --- a/core/src/main/java/org/mapfish/print/map/style/json/JsonStyleParserHelper.java +++ b/core/src/main/java/org/mapfish/print/map/style/json/JsonStyleParserHelper.java @@ -697,7 +697,7 @@ Stroke createStroke(final PJsonObject styleJson, final boolean allowNull) { (final String input) -> Double.parseDouble(styleJson.getString(JSON_STROKE_OPACITY))); Expression widthExpression = parseExpression( - 1, + 1.0, styleJson, JSON_STROKE_WIDTH, (final String input) -> Double.parseDouble(styleJson.getString(JSON_STROKE_WIDTH))); @@ -770,49 +770,74 @@ Stroke createStroke(final PJsonObject styleJson, final boolean allowNull) { @VisibleForTesting String getGraphicFormat(final String externalGraphicFile, final PJsonObject styleJson) { - String mimeType = null; - if (!StringUtils.isEmpty(styleJson.optString(JSON_GRAPHIC_FORMAT))) { - mimeType = styleJson.getString(JSON_GRAPHIC_FORMAT); - } else { - Matcher matcher = DATA_FORMAT_PATTERN.matcher(externalGraphicFile); - if (matcher.find()) { - mimeType = matcher.group(1); - } + String mimeType = getMimeTypeFromStyleJson(styleJson); - if (mimeType == null) { - int separatorPos = externalGraphicFile.lastIndexOf("."); - if (separatorPos >= 0) { - mimeType = "image/" + externalGraphicFile.substring(separatorPos + 1).toLowerCase(); - } - } + if (mimeType == null) { + mimeType = getMimeTypeFromExternalFile(externalGraphicFile); + } - if (mimeType == null) { - try { - URI uri; - try { - uri = new URI(externalGraphicFile); - } catch (URISyntaxException e) { - uri = new File(externalGraphicFile).toURI(); - } + if (mimeType == null) { + mimeType = getMimeTypeFromFileExtension(externalGraphicFile); + } - ClientHttpResponse httpResponse = this.requestFactory.createRequest(uri, HEAD).execute(); - List contentTypes = httpResponse.getHeaders().get("Content-Type"); - if (contentTypes != null && contentTypes.size() == 1) { - String contentType = contentTypes.get(0); - int index = contentType.lastIndexOf(";"); - mimeType = index >= 0 ? contentType.substring(0, index) : contentType; - } else { - LOGGER.info("No content type found for: {}", externalGraphicFile); - } - } catch (IOException e) { - throw new RuntimeException("Unable to get a mime type for the external graphic", e); - } - } + if (mimeType == null) { + mimeType = getMimeTypeFromHttpResponse(externalGraphicFile); } + mimeType = toSupportedMimeType(mimeType); return mimeType; } + private String getMimeTypeFromStyleJson(final PJsonObject styleJson) { + return !StringUtils.isEmpty(styleJson.optString(JSON_GRAPHIC_FORMAT)) + ? styleJson.getString(JSON_GRAPHIC_FORMAT) + : null; + } + + private String getMimeTypeFromExternalFile(final String externalGraphicFile) { + Matcher matcher = DATA_FORMAT_PATTERN.matcher(externalGraphicFile); + return matcher.find() ? matcher.group(1) : null; + } + + private String getMimeTypeFromFileExtension(final String externalGraphicFile) { + int separatorPos = externalGraphicFile.lastIndexOf("."); + return separatorPos >= 0 + ? "image/" + externalGraphicFile.substring(separatorPos + 1).toLowerCase() + : null; + } + + private String getMimeTypeFromHttpResponse(final String externalGraphicFile) { + try { + URI uri = getUri(externalGraphicFile); + final ClientHttpRequest request = this.requestFactory.createRequest(uri, HEAD); + List contentTypes; + try (ClientHttpResponse httpResponse = request.execute()) { + contentTypes = httpResponse.getHeaders().get("Content-Type"); + } + + if (contentTypes != null && contentTypes.size() == 1) { + String contentType = contentTypes.get(0); + int index = contentType.lastIndexOf(";"); + return index >= 0 ? contentType.substring(0, index) : contentType; + } + + LOGGER.info("No content type found for: {}", externalGraphicFile); + } catch (IOException e) { + throw new RuntimeException( + "Unable to get a mime type for the external graphic " + externalGraphicFile, e); + } + + return null; + } + + private URI getUri(final String externalGraphicFile) { + try { + return new URI(externalGraphicFile); + } catch (URISyntaxException e) { + return new File(externalGraphicFile).toURI(); + } + } + private String toSupportedMimeType(final String mimeType) { for (Set compatibleMimeType : COMPATIBLE_MIMETYPES) { if (compatibleMimeType.contains(mimeType.toLowerCase())) { From 2a02c872fef93c4f18569fd4b67fc6cce34aa945 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 24 May 2024 16:06:41 +0200 Subject: [PATCH 5/9] Remove cross charsets resource conversions --- .../org/mapfish/print/output/ResourceBundleClassLoader.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java b/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java index be22b4661..441258201 100644 --- a/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java +++ b/core/src/main/java/org/mapfish/print/output/ResourceBundleClassLoader.java @@ -7,7 +7,6 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; -import java.nio.charset.StandardCharsets; /** This is used to load the utf-8 ResourceBundle files. */ public class ResourceBundleClassLoader extends ClassLoader { @@ -50,7 +49,6 @@ public InputStream getResourceAsStream(final String resource) { } catch (IOException e) { throw new RuntimeException(e); } - return new ByteArrayInputStream( - buffer.toString(StandardCharsets.UTF_8).getBytes(StandardCharsets.ISO_8859_1)); + return new ByteArrayInputStream(buffer.toByteArray()); } } From c6b2c492ca1e8404eb99d34ef60e134666a38e9a Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 7 Jun 2024 13:51:14 +0200 Subject: [PATCH 6/9] Match description to the tests's purpose. --- examples/src/test/resources/examples/resource_bundle/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/src/test/resources/examples/resource_bundle/README.md b/examples/src/test/resources/examples/resource_bundle/README.md index 9bbe2527d..70b258600 100644 --- a/examples/src/test/resources/examples/resource_bundle/README.md +++ b/examples/src/test/resources/examples/resource_bundle/README.md @@ -1 +1 @@ -A simple configuration showing a GeoJSON layer in a map. +Demonstrates Mapfish print supports multilingual projects: Properties files are opened using UTF-8 encoding. From 6af09375c51a99bec381e1fb850f607f1f828992 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 7 Jun 2024 14:23:25 +0200 Subject: [PATCH 7/9] Close resources when possible. And minor refactorings. --- .../print/processor/ProcessorGraphNode.java | 97 ++++----- .../processor/jasper/HttpImageResolver.java | 56 +++-- .../processor/jasper/JasperReportBuilder.java | 71 +++--- .../processor/jasper/LegendProcessor.java | 117 +++++----- .../processor/map/CreateMapProcessor.java | 202 +++++++++++------- 5 files changed, 312 insertions(+), 231 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java b/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java index 2600d7769..6d4c76c4f 100644 --- a/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java +++ b/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java @@ -6,6 +6,7 @@ import com.google.common.collect.HashBiMap; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.RecursiveTask; @@ -67,22 +68,14 @@ private void addRequirement(final ProcessorGraphNode node) { this.requirements.add(node); } - protected Set getRequirements() { + Set getRequirements() { return this.requirements; } - protected Set> getDependencies() { + Set> getDependencies() { return this.dependencies; } - /** - * Returns true if the node has requirements, that is there are other nodes that should be run - * first. - */ - public boolean hasRequirements() { - return !this.requirements.isEmpty(); - } - /** * Create a ForkJoinTask for running in a fork join pool. * @@ -180,49 +173,11 @@ protected Values compute() { final Values values = this.execContext.getValues(); final Processor process = this.node.processor; - final MetricRegistry registry = this.node.metricRegistry; - final String name = + final String timerName = String.format( "%s.compute.%s", ProcessorGraphNode.class.getName(), process.getClass().getName()); - Timer.Context timerContext = registry.timer(name).time(); - try { - 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 { - 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) { - ProcessorUtils.writeProcessorOutputToValues(output, process, values); - } - } finally { - this.execContext.finished(this.node); - final long processorTime = - TimeUnit.MILLISECONDS.convert(timerContext.stop(), TimeUnit.NANOSECONDS); - LOGGER.info( - "Time taken to run processor: '{}' was {} ms", - process.getClass(), - processorTime); - } + executeProcess(process, values, timerName); this.execContext.getContext().stopIfCanceled(); ProcessorDependencyGraph.tryExecuteNodes( @@ -231,5 +186,47 @@ protected Values compute() { return values; }); } + + private void executeProcess( + final Processor process, final Values values, final String timerName) { + final Timer.Context timerContext = this.node.metricRegistry.timer(timerName).time(); + try { + final In inputParameter = ProcessorUtils.populateInputParameter(process, values); + + Out output; + try { + LOGGER.debug("Executing process: {}", process); + output = process.execute(inputParameter, this.execContext.getContext()); + LOGGER.debug("Succeeded in executing process: {}", process); + } catch (RuntimeException e) { + throw handleException(e, e, process, timerName); + } catch (Exception e) { + throw handleException(e, null, process, timerName); + } + + if (output != null) { + ProcessorUtils.writeProcessorOutputToValues(output, process, values); + } + } finally { + this.execContext.finished(this.node); + final long processorTime = + TimeUnit.MILLISECONDS.convert(timerContext.stop(), TimeUnit.NANOSECONDS); + LOGGER.info( + "Time taken to run processor: '{}' was {} ms", process.getClass(), processorTime); + } + } + + private RuntimeException handleException( + final Exception cause, + final RuntimeException runtimeCause, + final Processor process, + final String timerName) { + LOGGER.info("Error while executing process: {}", process, cause); + // the processor is already canceled, so we don't care if something fails + this.execContext.getContext().stopIfCanceled(); + this.node.metricRegistry.counter(timerName + ".error").inc(); + return Objects.requireNonNullElseGet( + runtimeCause, () -> new PrintException("Failed to execute process:" + process, cause)); + } } } diff --git a/core/src/main/java/org/mapfish/print/processor/jasper/HttpImageResolver.java b/core/src/main/java/org/mapfish/print/processor/jasper/HttpImageResolver.java index 05256a501..3cbab5135 100644 --- a/core/src/main/java/org/mapfish/print/processor/jasper/HttpImageResolver.java +++ b/core/src/main/java/org/mapfish/print/processor/jasper/HttpImageResolver.java @@ -29,7 +29,7 @@ public final class HttpImageResolver implements TableColumnConverterFor example: .*&img src="([^"]+)".* @@ -62,34 +62,44 @@ public BufferedImage resolve(final MfClientHttpRequestFactory requestFactory, fi Matcher urlMatcher = this.urlExtractor.matcher(text); if (urlMatcher.matches() && urlMatcher.group(this.urlGroup) != null) { - final String uriString = urlMatcher.group(this.urlGroup); + final String uriText = urlMatcher.group(this.urlGroup); try { - URI url = new URI(uriString); + URI url = new URI(uriText); final ClientHttpRequest request = requestFactory.createRequest(url, HttpMethod.GET); - final ClientHttpResponse response = request.execute(); - if (response.getStatusCode() == HttpStatus.OK) { - try { - final BufferedImage image = ImageIO.read(response.getBody()); - if (image == null) { - LOGGER.warn("The URL: {} is NOT an image format that can be decoded", url); - return this.defaultImage; - } - return image; - } catch (IOException e) { - LOGGER.warn("Image loaded from '{}'is not valid", url, e); - } - } else { - LOGGER.warn( - "Error loading the table row image: {}.\nStatus Code: {}\nStatus Text: {}", - url, - response.getStatusCode(), - response.getStatusText()); + try (ClientHttpResponse response = request.execute()) { + return getImageFromResponse(response, url); } } catch (RuntimeException | URISyntaxException | IOException e) { - LOGGER.warn("Error loading table row image: {}", uriString, e); + LOGGER.warn( + "Ignored following exception while loading table row image: {} and fallback to default" + + " image", + uriText, + e); } } + return this.defaultImage; + } + private BufferedImage getImageFromResponse(final ClientHttpResponse response, final URI url) + throws IOException { + if (response.getStatusCode() == HttpStatus.OK) { + try { + final BufferedImage image = ImageIO.read(response.getBody()); + if (image == null) { + LOGGER.warn("The URL: {} is NOT an image format that can be decoded", url); + return this.defaultImage; + } + return image; + } catch (IOException e) { + LOGGER.warn("Image loaded from '{}'is not valid", url, e); + } + } else { + LOGGER.warn( + "Error loading the table row image: {}.\nStatus Code: {}\nStatus Text: {}", + url, + response.getStatusCode(), + response.getStatusText()); + } return this.defaultImage; } diff --git a/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java b/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java index 3ff38f1d3..f5cd9b1af 100644 --- a/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java +++ b/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java @@ -1,5 +1,7 @@ package org.mapfish.print.processor.jasper; +import static java.nio.file.Files.move; + import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import java.io.File; @@ -77,28 +79,12 @@ File compileJasperReport(final File buildFile, final File jasperFile) throws JRE // another thread is reading it, use a temporary file as a target instead and // move it (atomic operation) when done. Worst case: we compile a file twice instead // of once. - File tmpBuildFile = - File.createTempFile( - "temp_", JASPER_REPORT_COMPILED_FILE_EXT, buildFile.getParentFile()); - LOGGER.info("Building Jasper report: {}", jasperFile.getAbsolutePath()); LOGGER.debug("To: {}", buildFile.getAbsolutePath()); - final Timer.Context compileJasperReport = - this.metricRegistry.timer(getClass().getName() + ".compile." + jasperFile).time(); - try { - JasperCompileManager.compileReportToFile( - jasperFile.getAbsolutePath(), tmpBuildFile.getAbsolutePath()); - } catch (JRValidationException e) { - LOGGER.error("The report '{}' isn't valid.", jasperFile.getAbsolutePath()); - throw e; - } finally { - final long compileTime = - TimeUnit.MILLISECONDS.convert(compileJasperReport.stop(), TimeUnit.NANOSECONDS); - LOGGER.info("Report '{}' built in {}ms.", jasperFile.getAbsolutePath(), compileTime); + final String timerName = getClass().getName() + ".compile." + jasperFile; + try (Timer.Context compileJasperReport = this.metricRegistry.timer(timerName).time()) { + doCompileAndMoveReport(buildFile, jasperFile, compileJasperReport); } - - java.nio.file.Files.move( - tmpBuildFile.toPath(), buildFile.toPath(), StandardCopyOption.ATOMIC_MOVE); } catch (IOException e) { throw new JRException(e); } @@ -108,11 +94,45 @@ File compileJasperReport(final File buildFile, final File jasperFile) throws JRE return buildFile; } + private static void doCompileAndMoveReport( + final File buildFile, final File jasperFile, final Timer.Context compileTimerContext) + throws JRException, IOException { + final File tmpBuildFile = + File.createTempFile("temp_", JASPER_REPORT_COMPILED_FILE_EXT, buildFile.getParentFile()); + try { + JasperCompileManager.compileReportToFile( + jasperFile.getAbsolutePath(), tmpBuildFile.getAbsolutePath()); + } catch (JRValidationException e) { + LOGGER.error("The report '{}' isn't valid.", jasperFile.getAbsolutePath()); + throw e; + } finally { + final long compileTime = + TimeUnit.MILLISECONDS.convert(compileTimerContext.stop(), TimeUnit.NANOSECONDS); + LOGGER.info("Report '{}' built in {}ms.", jasperFile.getAbsolutePath(), compileTime); + } + move(tmpBuildFile.toPath(), buildFile.toPath(), StandardCopyOption.ATOMIC_MOVE); + } + private Iterable jasperXmlFiles() { - File directoryToSearch = this.directory; - if (directoryToSearch == null) { + File directoryToSearch = getDirectoryToSearch(); + final File[] children = directoryToSearch.listFiles(); + if (children != null) { + return StreamSupport.stream(Arrays.spliterator(children), false) + .filter(input -> input != null && input.getName().endsWith(JASPER_REPORT_XML_FILE_EXT)) + .collect(Collectors.toList()); + } else { + throw new IllegalArgumentException(String.format("%s is not a directory", directoryToSearch)); + } + } + + private File getDirectoryToSearch() { + File directoryToSearch; + if (this.directory == null) { directoryToSearch = this.configuration.getDirectory(); + } else { + directoryToSearch = this.directory; } + final String configurationAbsolutePath = this.configuration.getDirectory().getAbsolutePath(); if (!directoryToSearch.getAbsolutePath().startsWith(configurationAbsolutePath)) { throw new IllegalArgumentException( @@ -121,14 +141,7 @@ private Iterable jasperXmlFiles() { + "configuration directory: %s is not in %s.", directoryToSearch, this.configuration.getDirectory())); } - final File[] children = directoryToSearch.listFiles(); - if (children != null) { - return StreamSupport.stream(Arrays.spliterator(children), false) - .filter(input -> input != null && input.getName().endsWith(JASPER_REPORT_XML_FILE_EXT)) - .collect(Collectors.toList()); - } else { - throw new IllegalArgumentException(String.format("%s is not a directory", directoryToSearch)); - } + return directoryToSearch; } @Override diff --git a/core/src/main/java/org/mapfish/print/processor/jasper/LegendProcessor.java b/core/src/main/java/org/mapfish/print/processor/jasper/LegendProcessor.java index a3ef8bb96..9ae04d99e 100644 --- a/core/src/main/java/org/mapfish/print/processor/jasper/LegendProcessor.java +++ b/core/src/main/java/org/mapfish/print/processor/jasper/LegendProcessor.java @@ -61,16 +61,16 @@ public final class LegendProcessor @Resource(name = "requestForkJoinPool") private ForkJoinPool requestForkJoinPool; - private Dimension missingImageSize = new Dimension(24, 24); + private final Dimension missingImageSize = new Dimension(24, 24); private BufferedImage missingImage; - private Color missingImageColor = Color.PINK; + private final Color missingImageColor = Color.PINK; private String template; private Integer maxWidth = null; private Double dpi = Constants.PDF_DPI; private boolean scaled = false; /** Constructor. */ - protected LegendProcessor() { + LegendProcessor() { super(Output.class); } @@ -238,10 +238,7 @@ private BufferedImage scaleToMaxWidth(final BufferedImage image, final double sc final BufferedImage inter = (image.getType() == BufferedImage.TYPE_BYTE_INDEXED || image.getType() == BufferedImage.TYPE_BYTE_BINARY) - ? new BufferedImage( - (int) Math.round(image.getWidth()), - (int) Math.round(image.getHeight()), - BufferedImage.TYPE_4BYTE_ABGR) + ? new BufferedImage(image.getWidth(), image.getHeight(), BufferedImage.TYPE_4BYTE_ABGR) : image; if (image.getType() == BufferedImage.TYPE_BYTE_INDEXED || image.getType() == BufferedImage.TYPE_BYTE_BINARY) { @@ -335,8 +332,8 @@ public static final class Output { private static final class NameTask implements Callable { - private String name; - private int level; + private final String name; + private final int level; private NameTask(final String name, final int level) { this.name = name; @@ -351,12 +348,12 @@ public Object[] call() { private final class IconTask implements Callable { - private URL icon; - private double iconDPI; - private ExecutionContext context; - private MfClientHttpRequestFactory clientHttpRequestFactory; - private int level; - private File tempTaskDirectory; + 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 IconTask( final URL icon, @@ -377,54 +374,60 @@ private IconTask( public Object[] call() throws Exception { return context.mdcContextEx( () -> { - BufferedImage image = null; final URI uri = this.icon.toURI(); - final String metricName = - LegendProcessor.class.getName() + ".read." + StatsUtils.quotePart(uri.getHost()); - try { - if (this.icon.getProtocol().equals("data")) { - image = ImageIO.read(this.icon); - } else { - this.context.stopIfCanceled(); - final ClientHttpRequest request = - this.clientHttpRequestFactory.createRequest(uri, HttpMethod.GET); - final Timer.Context timer = - LegendProcessor.this.metricRegistry.timer(metricName).time(); - try (ClientHttpResponse httpResponse = request.execute()) { - if (httpResponse.getStatusCode() == HttpStatus.OK) { - image = ImageIO.read(httpResponse.getBody()); - if (image == null) { - LOGGER.warn( - "The URL: {} is NOT an image format that can be decoded", this.icon); - } else { - timer.stop(); - } - } else { - LOGGER.warn( - "Failed to load image from: {} due to server side error.\n" - + "\tResponse Code: {}\n" - + "\tResponse Text: {}\n" - + "\tWith Headers:\n\t{}", - this.icon, - httpResponse.getStatusCode(), - httpResponse.getStatusText(), - String.join( - "\n\t", Utils.getPrintableHeadersList(httpResponse.getHeaders()))); - } + final BufferedImage image = loadImage(uri); + final URI report = createSubReport(image, this.iconDPI, this.tempTaskDirectory); + return new Object[] {null, image, report.toString(), this.level}; + }); + } + + private BufferedImage loadImage(final URI uri) { + final String metricName = + LegendProcessor.class.getName() + ".read." + StatsUtils.quotePart(uri.getHost()); + BufferedImage image = null; + try { + if (this.icon.getProtocol().equals("data")) { + image = ImageIO.read(this.icon); + } else { + this.context.stopIfCanceled(); + final ClientHttpRequest request = + this.clientHttpRequestFactory.createRequest(uri, HttpMethod.GET); + try (Timer.Context ignored = + LegendProcessor.this.metricRegistry.timer(metricName).time()) { + try (ClientHttpResponse httpResponse = request.execute()) { + if (httpResponse.getStatusCode() == HttpStatus.OK) { + image = ImageIO.read(httpResponse.getBody()); + if (image == null) { + LOGGER.warn("There is no image in this response body {}", httpResponse.getBody()); } + } else { + logNotOkResponseStatus(httpResponse); } - } catch (Exception e) { - LOGGER.warn("Failed to load image from: {}", this.icon, e); } + } + } + } catch (Exception e) { + LOGGER.warn("Failed to load image from: {}", this.icon, e); + } - if (image == null) { - image = getMissingImage(); - LegendProcessor.this.metricRegistry.counter(metricName + ".error").inc(); - } + if (image == null) { + image = getMissingImage(); + LegendProcessor.this.metricRegistry.counter(metricName + ".error").inc(); + } - String report = createSubReport(image, this.iconDPI, this.tempTaskDirectory).toString(); - return new Object[] {null, image, report, this.level}; - }); + return image; + } + + private void logNotOkResponseStatus(final ClientHttpResponse httpResponse) throws IOException { + LOGGER.warn( + "Failed to load image from: {} due to server side error.\n" + + "\tResponse Code: {}\n" + + "\tResponse Text: {}\n" + + "\tWith Headers:\n\t{}", + this.icon, + httpResponse.getStatusCode(), + httpResponse.getStatusText(), + String.join("\n\t", Utils.getPrintableHeadersList(httpResponse.getHeaders()))); } } } diff --git a/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java b/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java index 3bb4c8d97..8d9663fc3 100644 --- a/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java +++ b/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java @@ -281,9 +281,8 @@ private URI createMergedGraphic( int height = mapContext.getMapSize().height; if ("pdf".equalsIgnoreCase(outputFormat)) { - final com.lowagie.text.Document document = - new com.lowagie.text.Document(new com.lowagie.text.Rectangle(width, height)); - try { + try (com.lowagie.text.Document document = + new com.lowagie.text.Document(new com.lowagie.text.Rectangle(width, height))) { PdfWriter writer = PdfWriter.getInstance(document, new FileOutputStream(mergedGraphic)); document.open(); PdfContentByte pdfCB = writer.getDirectContent(); @@ -295,8 +294,6 @@ private URI createMergedGraphic( } } catch (DocumentException e) { throw new IOException(e); - } finally { - document.close(); } } else { @@ -386,14 +383,57 @@ private List createLayerGraphics( final ExecutionContext context, final MapfishMapContext mapContext) throws IOException, ParserConfigurationException { - // reverse layer list to draw from bottom to top. normally position 0 is top-most layer. - final List layers = new ArrayList<>(mapValues.getLayers()); - Collections.reverse(layers); + final List layers = + prepareLayers(printDirectory, clientHttpRequestFactory, mapValues, context, mapContext); final AreaOfInterest areaOfInterest = addAreaOfInterestLayer(mapValues, layers); - final String mapKey = UUID.randomUUID().toString(); final List graphics = new ArrayList<>(layers.size()); + String timerName = getClass().getName() + ".buildLayers"; + try (Timer.Context ignored = this.metricRegistry.timer(timerName).time()) { + int fileNumber = 0; + for (LayerGroup layerGroup : LayerGroup.buildGroups(layers, pdfA)) { + if (layerGroup.renderType == RenderType.SVG) { + fileNumber = + renderLayersAsSvg( + printDirectory, + clientHttpRequestFactory, + context, + mapContext, + layerGroup, + areaOfInterest, + mapKey, + fileNumber, + graphics); + } else { + fileNumber = + renderLayerAsRasterGraphic( + printDirectory, + clientHttpRequestFactory, + pdfA, + context, + mapContext, + layerGroup, + areaOfInterest, + mapKey, + fileNumber, + graphics); + } + } + } + + return graphics; + } + + private List prepareLayers( + final File printDirectory, + final MfClientHttpRequestFactory clientHttpRequestFactory, + final MapAttributeValues mapValues, + final ExecutionContext context, + final MapfishMapContext mapContext) { + // reverse layer list to draw from bottom to top. normally position 0 is top-most layer. + final List layers = new ArrayList<>(mapValues.getLayers()); + Collections.reverse(layers); HttpRequestFetcher cache = new HttpRequestFetcher( @@ -406,74 +446,92 @@ private List createLayerGraphics( getTransformer(mapContext, layer.getImageBufferScaling()); layer.prefetchResources(cache, clientHttpRequestFactory, transformer, context); } + return layers; + } - final Timer.Context timer = - this.metricRegistry.timer(getClass().getName() + ".buildLayers").time(); - int fileNumber = 0; - for (LayerGroup layerGroup : LayerGroup.buildGroups(layers, pdfA)) { - if (layerGroup.renderType == RenderType.SVG) { - // render layers as SVG - for (MapLayer layer : layerGroup.layers) { - context.stopIfCanceled(); - final SVGGraphics2D graphics2D = createSvgGraphics(mapContext.getMapSize()); - - try { - final Graphics2D clippedGraphics2D = - createClippedGraphics(mapContext, areaOfInterest, graphics2D); - layer.render(clippedGraphics2D, clientHttpRequestFactory, mapContext, context); - - final File path = new File(printDirectory, mapKey + "_layer_" + fileNumber++ + ".svg"); - saveSvgFile(graphics2D, path); - graphics.add(path.toURI()); - } finally { - graphics2D.dispose(); - } - } - } else { - // render layers as raster graphic - final boolean needTransparency = !layerGroup.opaque && !pdfA; - final BufferedImage bufferedImage = - new BufferedImage( - (int) Math.round(mapContext.getMapSize().width * layerGroup.imageBufferScaling), - (int) Math.round(mapContext.getMapSize().height * layerGroup.imageBufferScaling), - needTransparency ? BufferedImage.TYPE_4BYTE_ABGR : BufferedImage.TYPE_3BYTE_BGR); - final Graphics2D graphics2D = - createClippedGraphics(mapContext, areaOfInterest, bufferedImage.createGraphics()); - try { - if (layerGroup.opaque || pdfA) { - // the image is opaque and therefore needs a white background - final Color prevColor = graphics2D.getColor(); - graphics2D.setColor(Color.WHITE); - graphics2D.fillRect(0, 0, bufferedImage.getWidth(), bufferedImage.getHeight()); - graphics2D.setColor(prevColor); - } + private int renderLayersAsSvg( + final File printDirectory, + final MfClientHttpRequestFactory clientHttpRequestFactory, + final ExecutionContext context, + final MapfishMapContext mapContext, + final LayerGroup layerGroup, + final AreaOfInterest areaOfInterest, + final String mapKey, + final int fileNumber, + final List graphics) + throws ParserConfigurationException, IOException { + int fileCount = fileNumber; + for (MapLayer layer : layerGroup.layers) { + context.stopIfCanceled(); + final SVGGraphics2D graphics2D = createSvgGraphics(mapContext.getMapSize()); - final MapfishMapContext transformer = - getTransformer(mapContext, layerGroup.imageBufferScaling); - for (MapLayer cur : layerGroup.layers) { - context.stopIfCanceled(); - warnIfDifferentRenderType(layerGroup.renderType, cur, !pdfA); - cur.render(graphics2D, clientHttpRequestFactory, transformer, context); - } + try { + final Graphics2D clippedGraphics2D = + createClippedGraphics(mapContext, areaOfInterest, graphics2D); + layer.render(clippedGraphics2D, clientHttpRequestFactory, mapContext, context); - // Try to respect the original format of the layer. But if it needs to be transparent, - // no choice, we need PNG. - final String formatName = - layerGroup.opaque && layerGroup.renderType == RenderType.JPEG ? "JPEG" : "PNG"; - final File path = - new File( - printDirectory, - String.format("%s_layer_%d.%s", mapKey, fileNumber++, formatName.toLowerCase())); - ImageUtils.writeImage(bufferedImage, formatName, path); - graphics.add(path.toURI()); - } finally { - graphics2D.dispose(); - } + final File path = new File(printDirectory, mapKey + "_layer_" + fileCount++ + ".svg"); + saveSvgFile(graphics2D, path); + graphics.add(path.toURI()); + } finally { + graphics2D.dispose(); } } - timer.stop(); + return fileCount; + } - return graphics; + private int renderLayerAsRasterGraphic( + final File printDirectory, + final MfClientHttpRequestFactory clientHttpRequestFactory, + final boolean pdfA, + final ExecutionContext context, + final MapfishMapContext mapContext, + final LayerGroup layerGroup, + final AreaOfInterest areaOfInterest, + final String mapKey, + final int fileNumber, + final List graphics) + throws IOException { + int fileCount = fileNumber; + final boolean needTransparency = !layerGroup.opaque && !pdfA; + final BufferedImage bufferedImage = + new BufferedImage( + (int) Math.round(mapContext.getMapSize().width * layerGroup.imageBufferScaling), + (int) Math.round(mapContext.getMapSize().height * layerGroup.imageBufferScaling), + needTransparency ? BufferedImage.TYPE_4BYTE_ABGR : BufferedImage.TYPE_3BYTE_BGR); + final Graphics2D graphics2D = + createClippedGraphics(mapContext, areaOfInterest, bufferedImage.createGraphics()); + try { + if (layerGroup.opaque || pdfA) { + // the image is opaque and therefore needs a white background + final Color prevColor = graphics2D.getColor(); + graphics2D.setColor(Color.WHITE); + graphics2D.fillRect(0, 0, bufferedImage.getWidth(), bufferedImage.getHeight()); + graphics2D.setColor(prevColor); + } + + final MapfishMapContext transformer = + getTransformer(mapContext, layerGroup.imageBufferScaling); + for (MapLayer cur : layerGroup.layers) { + context.stopIfCanceled(); + warnIfDifferentRenderType(layerGroup.renderType, cur, !pdfA); + cur.render(graphics2D, clientHttpRequestFactory, transformer, context); + } + + // Try to respect the original format of the layer. But if it needs to be transparent, + // no choice, we need PNG. + final String formatName = + layerGroup.opaque && layerGroup.renderType == RenderType.JPEG ? "JPEG" : "PNG"; + final File path = + new File( + printDirectory, + String.format("%s_layer_%d.%s", mapKey, fileCount++, formatName.toLowerCase())); + ImageUtils.writeImage(bufferedImage, formatName, path); + graphics.add(path.toURI()); + } finally { + graphics2D.dispose(); + } + return fileCount; } private AreaOfInterest addAreaOfInterestLayer( @@ -693,7 +751,7 @@ public void setStyle( String tagName = element.getTagName(); for (final Object o : styleMap.keySet()) { String styleName = (String) o; - if (element.getAttributeNS(null, styleName).length() == 0) { + if (element.getAttributeNS(null, styleName).isEmpty()) { if (styleName.equals("opacity")) { if (appliesTo(styleName, tagName)) { element.setAttributeNS(null, "fill-opacity", (String) styleMap.get(styleName)); From 5569636ad54abe710427ff17b50e54475809149b Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 7 Jun 2024 18:03:31 +0200 Subject: [PATCH 8/9] Resolve checkstyle errors --- .../processor/map/CreateMapProcessor.java | 153 +++++++++--------- .../print/processor/map/ImageWriter.java | 49 ++++++ .../print/processor/map/SvgFileGenerator.java | 23 +++ 3 files changed, 151 insertions(+), 74 deletions(-) create mode 100644 core/src/main/java/org/mapfish/print/processor/map/ImageWriter.java create mode 100644 core/src/main/java/org/mapfish/print/processor/map/SvgFileGenerator.java diff --git a/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java b/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java index 8d9663fc3..2bd0a8e87 100644 --- a/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java +++ b/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java @@ -187,7 +187,7 @@ public static MapBounds adjustBoundsToScaleAndMapSize( } /** - * Create a SVG graphic with the give dimensions. + * Create an SVG graphic with the give dimensions. * * @param size The size of the SVG graphic. */ @@ -208,7 +208,7 @@ public static SVGGraphics2D createSvgGraphics(final Dimension size) } /** - * Save a SVG graphic to the given path. + * Save an SVG graphic to the given path. * * @param graphics2d The SVG graphic to save. * @param path The file. @@ -394,30 +394,43 @@ private List createLayerGraphics( int fileNumber = 0; for (LayerGroup layerGroup : LayerGroup.buildGroups(layers, pdfA)) { if (layerGroup.renderType == RenderType.SVG) { - fileNumber = - renderLayersAsSvg( - printDirectory, - clientHttpRequestFactory, - context, - mapContext, - layerGroup, - areaOfInterest, - mapKey, - fileNumber, - graphics); + SvgFileGenerator svgFileGenerator = + new SvgFileGenerator(printDirectory, mapKey, fileNumber); + renderLayersAsSvg( + svgFileGenerator, + clientHttpRequestFactory, + context, + mapContext, + layerGroup, + areaOfInterest, + graphics); + fileNumber = svgFileGenerator.getCurrentFileNumber(); } else { - fileNumber = - renderLayerAsRasterGraphic( + final BufferedImage bufferedImage = createBufferedImage(layerGroup, pdfA, mapContext); + ImageWriter imageWriter = + new ImageWriter( printDirectory, - clientHttpRequestFactory, - pdfA, - context, - mapContext, - layerGroup, - areaOfInterest, mapKey, + layerGroup.opaque, + layerGroup.renderType, fileNumber, + bufferedImage, graphics); + Graphics2D graphics2D = + createGraphics2D(layerGroup, pdfA, mapContext, areaOfInterest, bufferedImage); + try { + renderLayerAsRasterGraphic( + imageWriter, + clientHttpRequestFactory, + pdfA, + context, + mapContext, + layerGroup, + graphics2D); + } finally { + graphics2D.dispose(); + } + fileNumber = imageWriter.getCurrentFileNumber(); } } } @@ -425,6 +438,38 @@ private List createLayerGraphics( return graphics; } + private Graphics2D createGraphics2D( + final LayerGroup layerGroup, + final boolean pdfA, + final MapfishMapContext mapContext, + final AreaOfInterest areaOfInterest, + final BufferedImage bufferedImage) { + final Graphics2D graphics2D = + createClippedGraphics(mapContext, areaOfInterest, bufferedImage.createGraphics()); + try { + if (layerGroup.opaque || pdfA) { + // the image is opaque and therefore needs a white background + final Color prevColor = graphics2D.getColor(); + graphics2D.setColor(Color.WHITE); + graphics2D.fillRect(0, 0, bufferedImage.getWidth(), bufferedImage.getHeight()); + graphics2D.setColor(prevColor); + } + } catch (RuntimeException e) { + graphics2D.dispose(); + throw e; + } + return graphics2D; + } + + private static BufferedImage createBufferedImage( + final LayerGroup layerGroup, final boolean pdfA, final MapfishMapContext mapContext) { + final boolean needTransparency = !layerGroup.opaque && !pdfA; + return new BufferedImage( + (int) Math.round(mapContext.getMapSize().width * layerGroup.imageBufferScaling), + (int) Math.round(mapContext.getMapSize().height * layerGroup.imageBufferScaling), + needTransparency ? BufferedImage.TYPE_4BYTE_ABGR : BufferedImage.TYPE_3BYTE_BGR); + } + private List prepareLayers( final File printDirectory, final MfClientHttpRequestFactory clientHttpRequestFactory, @@ -449,18 +494,15 @@ private List prepareLayers( return layers; } - private int renderLayersAsSvg( - final File printDirectory, + private void renderLayersAsSvg( + final SvgFileGenerator svgFileGenerator, final MfClientHttpRequestFactory clientHttpRequestFactory, final ExecutionContext context, final MapfishMapContext mapContext, final LayerGroup layerGroup, final AreaOfInterest areaOfInterest, - final String mapKey, - final int fileNumber, final List graphics) throws ParserConfigurationException, IOException { - int fileCount = fileNumber; for (MapLayer layer : layerGroup.layers) { context.stopIfCanceled(); final SVGGraphics2D graphics2D = createSvgGraphics(mapContext.getMapSize()); @@ -470,68 +512,31 @@ private int renderLayersAsSvg( createClippedGraphics(mapContext, areaOfInterest, graphics2D); layer.render(clippedGraphics2D, clientHttpRequestFactory, mapContext, context); - final File path = new File(printDirectory, mapKey + "_layer_" + fileCount++ + ".svg"); + final File path = svgFileGenerator.generate(); saveSvgFile(graphics2D, path); graphics.add(path.toURI()); } finally { graphics2D.dispose(); } } - return fileCount; } - private int renderLayerAsRasterGraphic( - final File printDirectory, + private void renderLayerAsRasterGraphic( + final ImageWriter imageWriter, final MfClientHttpRequestFactory clientHttpRequestFactory, final boolean pdfA, final ExecutionContext context, final MapfishMapContext mapContext, final LayerGroup layerGroup, - final AreaOfInterest areaOfInterest, - final String mapKey, - final int fileNumber, - final List graphics) + final Graphics2D graphics2D) throws IOException { - int fileCount = fileNumber; - final boolean needTransparency = !layerGroup.opaque && !pdfA; - final BufferedImage bufferedImage = - new BufferedImage( - (int) Math.round(mapContext.getMapSize().width * layerGroup.imageBufferScaling), - (int) Math.round(mapContext.getMapSize().height * layerGroup.imageBufferScaling), - needTransparency ? BufferedImage.TYPE_4BYTE_ABGR : BufferedImage.TYPE_3BYTE_BGR); - final Graphics2D graphics2D = - createClippedGraphics(mapContext, areaOfInterest, bufferedImage.createGraphics()); - try { - if (layerGroup.opaque || pdfA) { - // the image is opaque and therefore needs a white background - final Color prevColor = graphics2D.getColor(); - graphics2D.setColor(Color.WHITE); - graphics2D.fillRect(0, 0, bufferedImage.getWidth(), bufferedImage.getHeight()); - graphics2D.setColor(prevColor); - } - - final MapfishMapContext transformer = - getTransformer(mapContext, layerGroup.imageBufferScaling); - for (MapLayer cur : layerGroup.layers) { - context.stopIfCanceled(); - warnIfDifferentRenderType(layerGroup.renderType, cur, !pdfA); - cur.render(graphics2D, clientHttpRequestFactory, transformer, context); - } - - // Try to respect the original format of the layer. But if it needs to be transparent, - // no choice, we need PNG. - final String formatName = - layerGroup.opaque && layerGroup.renderType == RenderType.JPEG ? "JPEG" : "PNG"; - final File path = - new File( - printDirectory, - String.format("%s_layer_%d.%s", mapKey, fileCount++, formatName.toLowerCase())); - ImageUtils.writeImage(bufferedImage, formatName, path); - graphics.add(path.toURI()); - } finally { - graphics2D.dispose(); + final MapfishMapContext transformer = getTransformer(mapContext, layerGroup.imageBufferScaling); + for (MapLayer cur : layerGroup.layers) { + context.stopIfCanceled(); + warnIfDifferentRenderType(layerGroup.renderType, cur, !pdfA); + cur.render(graphics2D, clientHttpRequestFactory, transformer, context); } - return fileCount; + imageWriter.writeImage(); } private AreaOfInterest addAreaOfInterestLayer( @@ -596,7 +601,7 @@ private void zoomToFeatures( if (!bounds.isNull()) { if (mapValues.zoomToFeatures.zoomType == ZoomType.CENTER) { - // center the map on the center of the feature bounds + // center the map in the center of the feature bounds Coordinate center = bounds.centre(); mapValues.center = new double[] {center.x, center.y}; if (mapValues.zoomToFeatures.minScale != null) { diff --git a/core/src/main/java/org/mapfish/print/processor/map/ImageWriter.java b/core/src/main/java/org/mapfish/print/processor/map/ImageWriter.java new file mode 100644 index 000000000..11b562856 --- /dev/null +++ b/core/src/main/java/org/mapfish/print/processor/map/ImageWriter.java @@ -0,0 +1,49 @@ +package org.mapfish.print.processor.map; + +import java.awt.image.BufferedImage; +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.util.List; +import org.mapfish.print.ImageUtils; +import org.mapfish.print.attribute.map.MapLayer; + +class ImageWriter { + private final File printDirectory; + private final String mapKey; + private final String formatName; + private final BufferedImage bufferedImage; + private final List graphics; + private int currentFileNumber; + + ImageWriter( + final File printDirectory, + final String mapKey, + final boolean isLayerGroupOpaque, + final MapLayer.RenderType renderType, + final int fileNumber, + final BufferedImage bufferedImage, + final List graphics) { + this.printDirectory = printDirectory; + this.mapKey = mapKey; + this.formatName = isLayerGroupOpaque && renderType == MapLayer.RenderType.JPEG ? "JPEG" : "PNG"; + this.currentFileNumber = fileNumber; + this.bufferedImage = bufferedImage; + this.graphics = graphics; + } + + public void writeImage() throws IOException { + // Try to respect the original format of the layer. But if it needs to be transparent, + // no choice, we need PNG. + final File path = + new File( + printDirectory, + String.format("%s_layer_%d.%s", mapKey, currentFileNumber++, formatName.toLowerCase())); + ImageUtils.writeImage(bufferedImage, formatName, path); + graphics.add(path.toURI()); + } + + public int getCurrentFileNumber() { + return currentFileNumber; + } +} diff --git a/core/src/main/java/org/mapfish/print/processor/map/SvgFileGenerator.java b/core/src/main/java/org/mapfish/print/processor/map/SvgFileGenerator.java new file mode 100644 index 000000000..6053b52fd --- /dev/null +++ b/core/src/main/java/org/mapfish/print/processor/map/SvgFileGenerator.java @@ -0,0 +1,23 @@ +package org.mapfish.print.processor.map; + +import java.io.File; + +class SvgFileGenerator { + private final File printDirectory; + private final String mapKey; + private int currentFileNumber; + + SvgFileGenerator(final File printDirectory, final String mapKey, final int fileNumber) { + this.printDirectory = printDirectory; + this.mapKey = mapKey; + this.currentFileNumber = fileNumber; + } + + public File generate() { + return new File(printDirectory, mapKey + "_layer_" + currentFileNumber++ + ".svg"); + } + + public int getCurrentFileNumber() { + return currentFileNumber; + } +} From ed172bf321ddd0f5d9927d47286bdb5b0dbd164a Mon Sep 17 00:00:00 2001 From: sebr72 Date: Tue, 11 Jun 2024 17:38:46 +0200 Subject: [PATCH 9/9] Stop timers appropriately --- .../print/processor/ProcessorGraphNode.java | 15 +++++---------- .../processor/jasper/JasperReportBuilder.java | 10 ++++------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java b/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java index 6d4c76c4f..17101752f 100644 --- a/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java +++ b/core/src/main/java/org/mapfish/print/processor/ProcessorGraphNode.java @@ -171,14 +171,7 @@ protected Values compute() { .mdcContext( () -> { final Values values = this.execContext.getValues(); - - final Processor process = this.node.processor; - final String timerName = - String.format( - "%s.compute.%s", - ProcessorGraphNode.class.getName(), process.getClass().getName()); - executeProcess(process, values, timerName); - + executeProcess(this.node.processor, values); this.execContext.getContext().stopIfCanceled(); ProcessorDependencyGraph.tryExecuteNodes( this.node.dependencies, this.execContext, true); @@ -187,8 +180,10 @@ protected Values compute() { }); } - private void executeProcess( - final Processor process, final Values values, final String timerName) { + private void executeProcess(final Processor process, final Values values) { + final String timerName = + String.format( + "%s.compute.%s", ProcessorGraphNode.class.getName(), process.getClass().getName()); final Timer.Context timerContext = this.node.metricRegistry.timer(timerName).time(); try { final In inputParameter = ProcessorUtils.populateInputParameter(process, values); diff --git a/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java b/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java index f5cd9b1af..bafdda38b 100644 --- a/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java +++ b/core/src/main/java/org/mapfish/print/processor/jasper/JasperReportBuilder.java @@ -81,10 +81,7 @@ File compileJasperReport(final File buildFile, final File jasperFile) throws JRE // of once. LOGGER.info("Building Jasper report: {}", jasperFile.getAbsolutePath()); LOGGER.debug("To: {}", buildFile.getAbsolutePath()); - final String timerName = getClass().getName() + ".compile." + jasperFile; - try (Timer.Context compileJasperReport = this.metricRegistry.timer(timerName).time()) { - doCompileAndMoveReport(buildFile, jasperFile, compileJasperReport); - } + doCompileAndMoveReport(buildFile, jasperFile); } catch (IOException e) { throw new JRException(e); } @@ -94,11 +91,12 @@ File compileJasperReport(final File buildFile, final File jasperFile) throws JRE return buildFile; } - private static void doCompileAndMoveReport( - final File buildFile, final File jasperFile, final Timer.Context compileTimerContext) + private void doCompileAndMoveReport(final File buildFile, final File jasperFile) throws JRException, IOException { final File tmpBuildFile = File.createTempFile("temp_", JASPER_REPORT_COMPILED_FILE_EXT, buildFile.getParentFile()); + final String timerName = getClass().getName() + ".compile." + jasperFile; + Timer.Context compileTimerContext = this.metricRegistry.timer(timerName).time(); try { JasperCompileManager.compileReportToFile( jasperFile.getAbsolutePath(), tmpBuildFile.getAbsolutePath());