Skip to content

Commit

Permalink
Merge pull request #3292 from mapfish/CloseResources
Browse files Browse the repository at this point in the history
Close resources. Stop timers. Use docker compose. Stop charset encoding conversions.
  • Loading branch information
sebr72 authored Jun 11, 2024
2 parents cd27be5 + ed172bf commit 8ed7bf5
Show file tree
Hide file tree
Showing 18 changed files with 668 additions and 429 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String> 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<String> 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<String> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -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<String> 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<String> 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<String> compatibleMimeType : COMPATIBLE_MIMETYPES) {
if (compatibleMimeType.contains(mimeType.toLowerCase())) {
Expand Down
Loading

0 comments on commit 8ed7bf5

Please sign in to comment.