Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use getRawStatusCode to avoid getting an exception on fake status code 999, convert it to 406 Not Acceptable #3497

Merged
merged 2 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
public class ErrorResponseClientHttpResponse extends AbstractClientHttpResponse {
private final Exception exception;

/** HTTP code use in response for non HTTP errors. */
private static final int FAKE_HTTP_ERROR_CODE = 999;
/** HTTP code use in response for non HTTP errors, (Not Acceptable). */
private static final int FAKE_HTTP_ERROR_CODE = 406;

public ErrorResponseClientHttpResponse(final Exception e) {
assert e != null;
Expand Down Expand Up @@ -38,7 +38,8 @@ public int getRawStatusCode() {
@Nonnull
public String getStatusText() {
return String.format(
"%s: %s", this.exception.getClass().getSimpleName(), this.exception.getMessage());
"Not true HTTP code, %s: %s, see above error",
this.exception.getClass().getSimpleName(), this.exception.getMessage());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public FeaturesParser(
}

@VisibleForTesting
static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
static CoordinateReferenceSystem parseCoordinateReferenceSystem(
final MfClientHttpRequestFactory requestFactory,
final JSONObject geojson,
final boolean forceLongitudeFirst) {
Expand Down Expand Up @@ -93,7 +93,7 @@ static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
requestFactory.createRequest(new URI(uri), HttpMethod.GET);
try (ClientHttpResponse response = request.execute()) {

if (response.getStatusCode() == HttpStatus.OK) {
if (response.getRawStatusCode() == HttpStatus.OK.value()) {
final String wkt =
IOUtils.toString(response.getBody(), Constants.DEFAULT_ENCODING);
try {
Expand All @@ -108,7 +108,8 @@ static final CoordinateReferenceSystem parseCoordinateReferenceSystem(
}
}
} else {
LOGGER.warn("Unable to load linked CRS from geojson: \n{}", crsJson);
LOGGER.warn(
"Unsupported link type {} in linked CRS from geojson: \n{}", linkType, crsJson);
}
} else {
code.append(getProperty(crsJson, "code"));
Expand Down Expand Up @@ -147,7 +148,6 @@ private static String getProperty(final JSONObject crsJson, final String nameCod
* @param template the template
* @param features what to parse
* @return the feature collection
* @throws IOException
*/
public final SimpleFeatureCollection autoTreat(final Template template, final String features)
throws IOException {
Expand Down Expand Up @@ -189,7 +189,6 @@ public final SimpleFeatureCollection treatStringAsURL(
*
* @param geoJsonString what to parse
* @return the feature collection
* @throws IOException
*/
public final SimpleFeatureCollection treatStringAsGeoJson(final String geoJsonString)
throws IOException {
Expand Down Expand Up @@ -231,7 +230,7 @@ private SimpleFeatureType createFeatureType(@Nonnull final String geojsonData) {
builder.setName("GeosjonFeatureType");
final JSONArray features = geojson.getJSONArray("features");

if (features.length() == 0) {
if (features.isEmpty()) {
// do not try to build the feature type if there are no features
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,15 @@ private boolean isResponseStatusCodeValid(
final String stringBody,
final String baseMetricName)
throws IOException {
if (httpResponse.getStatusCode() != HttpStatus.OK) {
if (httpResponse.getRawStatusCode() != HttpStatus.OK.value()) {
String message =
String.format(
"Invalid status code for %s (%d!=%d).With request headers:\n%s\n"
"Invalid status code for %s (%d!=%d), status: %s. With request headers:\n%s\n"
sebr72 marked this conversation as resolved.
Show resolved Hide resolved
+ "The response was: '%s'\nWith response headers:\n%s",
request.getURI(),
httpResponse.getStatusCode().value(),
httpResponse.getRawStatusCode(),
HttpStatus.OK.value(),
httpResponse.getStatusText(),
String.join("\n", Utils.getPrintableHeadersList(request.getHeaders())),
httpResponse.getStatusText(),
String.join("\n", Utils.getPrintableHeadersList(httpResponse.getHeaders())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static Optional<Style> loadStyleAsURI(
final ClientHttpRequestFactory clientHttpRequestFactory,
final String styleRef,
final Function<byte[], @Nullable Optional<Style>> loadFunction) {
HttpStatus statusCode;
int httpStatusCode;
final byte[] input;

URI uri;
Expand All @@ -47,13 +47,13 @@ public static Optional<Style> loadStyleAsURI(
try {
final ClientHttpRequest request = clientHttpRequestFactory.createRequest(uri, HttpMethod.GET);
try (ClientHttpResponse response = request.execute()) {
statusCode = response.getStatusCode();
httpStatusCode = response.getRawStatusCode();
input = IOUtils.toByteArray(response.getBody());
}
} catch (Exception e) {
return Optional.empty();
}
if (statusCode == HttpStatus.OK) {
if (httpStatusCode == HttpStatus.OK.value()) {
return loadFunction.apply(input);
} else {
return Optional.empty();
Expand Down
23 changes: 12 additions & 11 deletions core/src/main/java/org/mapfish/print/map/tiled/CoverageTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,17 +254,18 @@ protected Tile compute() {

private Tile handleSpecialStatuses(
final ClientHttpResponse response, final String baseMetricName) throws IOException {
final HttpStatus statusCode = response.getStatusCode();
if (statusCode == HttpStatus.NO_CONTENT || statusCode == HttpStatus.NOT_FOUND) {
if (statusCode == HttpStatus.NOT_FOUND) {
final int httpStatusCode = response.getRawStatusCode();
if (httpStatusCode == HttpStatus.NO_CONTENT.value()
|| httpStatusCode == HttpStatus.NOT_FOUND.value()) {
if (httpStatusCode == HttpStatus.NOT_FOUND.value()) {
LOGGER.info(
"The request {} returns a not found status code, we consider it as an empty tile.",
this.tileRequest.getURI());
}
// Empty response, nothing special to do
return new Tile(null, getTileIndexX(), getTileIndexY());
} else if (statusCode != HttpStatus.OK) {
return handleNonOkStatus(statusCode, response, baseMetricName);
} else if (httpStatusCode != HttpStatus.OK.value()) {
return handleNonOkStatus(response, baseMetricName);
}
return null;
}
Expand All @@ -281,19 +282,19 @@ private BufferedImage getImageFromResponse(
return image;
}

private Tile handleNonOkStatus(
final HttpStatus statusCode, final ClientHttpResponse response, final String baseMetricName)
private Tile handleNonOkStatus(final ClientHttpResponse response, final String baseMetricName)
throws IOException {
final int httpStatusCode = response.getRawStatusCode();
String errorMessage =
String.format(
"Error making tile request: %s\n\tStatus: %s\n\tStatus message: %s",
this.tileRequest.getURI(), statusCode, response.getStatusText());
"Error making tile request: %s\n\tStatus: %d\n\tStatus message: %s",
this.tileRequest.getURI(), httpStatusCode, response.getStatusText());
LOGGER.debug(
String.format(
"Error making tile request: %s\nStatus: %s\n"
"Error making tile request: %s\nStatus: %d\n"
+ "Status message: %s\nServer:%s\nBody:\n%s",
this.tileRequest.getURI(),
statusCode,
httpStatusCode,
response.getStatusText(),
response.getHeaders().getFirst(HttpHeaders.SERVER),
IOUtils.toString(response.getBody(), StandardCharsets.UTF_8)));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.mapfish.print.processor.jasper;

import com.google.common.annotations.VisibleForTesting;
import java.awt.image.BufferedImage;
import java.io.IOException;
import java.net.URI;
Expand Down Expand Up @@ -29,7 +30,7 @@ public final class HttpImageResolver implements TableColumnConverter<BufferedIma
private static final int IMAGE_SIZE = 48;
private Pattern urlExtractor = Pattern.compile("(.*)");
private int urlGroup = 1;
private final BufferedImage defaultImage;
@VisibleForTesting final BufferedImage defaultImage;

/** Constructor. */
public HttpImageResolver() {
Expand Down Expand Up @@ -82,7 +83,7 @@ public BufferedImage resolve(final MfClientHttpRequestFactory requestFactory, fi

private BufferedImage getImageFromResponse(final ClientHttpResponse response, final URI url)
throws IOException {
if (response.getStatusCode() == HttpStatus.OK) {
if (response.getRawStatusCode() == HttpStatus.OK.value()) {
try {
final BufferedImage image = ImageIO.read(response.getBody());
if (image == null) {
Expand All @@ -97,7 +98,7 @@ private BufferedImage getImageFromResponse(final ClientHttpResponse response, fi
LOGGER.warn(
"Error loading the table row image: {}.\nStatus Code: {}\nStatus Text: {}",
url,
response.getStatusCode(),
response.getRawStatusCode(),
response.getStatusText());
}
return this.defaultImage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.google.common.annotations.VisibleForTesting;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics2D;
Expand Down Expand Up @@ -161,7 +162,14 @@ private void createTasks(
if (icons != null && icons.length > 0) {
for (URL icon : icons) {
tasks.add(
new IconTask(icon, dpi, context, level, tempTaskDirectory, clientHttpRequestFactory));
new IconTask(
icon,
dpi,
context,
level,
tempTaskDirectory,
clientHttpRequestFactory,
metricRegistry));
}
}
if (legendAttributes.classes != null) {
Expand Down Expand Up @@ -269,7 +277,8 @@ protected void extraValidation(
// no checks needed
}

private synchronized BufferedImage getMissingImage() {
@VisibleForTesting
synchronized BufferedImage getMissingImage() {
if (this.missingImage == null) {
this.missingImage =
new BufferedImage(
Expand Down Expand Up @@ -311,7 +320,7 @@ public static final class Output {
/** The datasource for the legend object in the report. */
public final JRTableModelDataSource legendDataSource;

/** The path to the compiled subreport. */
/** The path to the compiled sub-report. */
public final String legendSubReport;

/** The number of rows in the legend. */
Expand Down Expand Up @@ -343,28 +352,33 @@ public Object[] call() {
}
}

private final class IconTask implements Callable<Object[]> {
@VisibleForTesting
final class IconTask implements Callable<Object[]> {

private final URL icon;
private final double iconDPI;
private final ExecutionContext context;
private final MfClientHttpRequestFactory clientHttpRequestFactory;
private final int level;
private final File tempTaskDirectory;
private final MetricRegistry metricRegistry;

private IconTask(
@VisibleForTesting
IconTask(
final URL icon,
final double iconDPI,
final ExecutionContext context,
final int level,
final File tempTaskDirectory,
final MfClientHttpRequestFactory clientHttpRequestFactory) {
final MfClientHttpRequestFactory clientHttpRequestFactory,
final MetricRegistry metricRegistry) {
this.icon = icon;
this.iconDPI = iconDPI;
this.context = context;
this.level = level;
this.clientHttpRequestFactory = clientHttpRequestFactory;
this.tempTaskDirectory = tempTaskDirectory;
this.metricRegistry = metricRegistry;
}

@Override
Expand All @@ -389,10 +403,9 @@ private BufferedImage loadImage(final URI uri) {
this.context.stopIfCanceled();
final ClientHttpRequest request =
this.clientHttpRequestFactory.createRequest(uri, HttpMethod.GET);
try (Timer.Context ignored =
LegendProcessor.this.metricRegistry.timer(metricName).time()) {
try (Timer.Context ignored = metricRegistry.timer(metricName).time()) {
try (ClientHttpResponse httpResponse = request.execute()) {
if (httpResponse.getStatusCode() == HttpStatus.OK) {
if (httpResponse.getRawStatusCode() == HttpStatus.OK.value()) {
image = ImageIO.read(httpResponse.getBody());
if (image == null) {
LOGGER.warn("There is no image in this response body {}", httpResponse.getBody());
Expand All @@ -409,7 +422,7 @@ private BufferedImage loadImage(final URI uri) {

if (image == null) {
image = getMissingImage();
LegendProcessor.this.metricRegistry.counter(metricName + ".error").inc();
metricRegistry.counter(metricName + ".error").inc();
}

return image;
Expand All @@ -422,7 +435,7 @@ private void logNotOkResponseStatus(final ClientHttpResponse httpResponse) throw
+ "\tResponse Text: {}\n"
+ "\tWith Headers:\n\t{}",
this.icon,
httpResponse.getStatusCode(),
httpResponse.getRawStatusCode(),
httpResponse.getStatusText(),
String.join("\n\t", Utils.getPrintableHeadersList(httpResponse.getHeaders())));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.io.File;
Expand Down Expand Up @@ -36,6 +37,10 @@ public class FeaturesParserTest extends AbstractMapfishSpringTest {

private static final String EXAMPLE_GEOJSONFILE =
"geojson/geojson-inconsistent-attributes-2.json";
private static final String WKT =
"GEOGCS[\"GCS_WGS_1984\",DATUM[\"D_WGS_1984\""
+ ",SPHEROID[\"WGS_1984\",6378137,298.257223563]]"
+ ",PRIMEM[\"Greenwich\",0],UNIT[\"Degree\",0.017453292519943295]]";
@Autowired private TestHttpClientFactory requestFactory;
@Autowired private ConfigurationFactory configurationFactory;

Expand Down Expand Up @@ -131,19 +136,32 @@ public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
@Test
@DirtiesContext
public void testParseCRSLinkEsriWkt() {
final MockClientHttpResponse clientHttpResponse =
new MockClientHttpResponse(WKT.getBytes(Constants.DEFAULT_CHARSET), HttpStatus.OK);

CoordinateReferenceSystem crs = parseCoordinateReferenceSystemFromResponse(clientHttpResponse);
assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
}

@Test
@DirtiesContext
public void testParseCRSLinkEsriWktWithUnsupportedErrorCode() {
final MockClientHttpResponse clientHttpResponse =
new MockClientHttpResponse(WKT.getBytes(Constants.DEFAULT_CHARSET), 999);

CoordinateReferenceSystem crs = parseCoordinateReferenceSystemFromResponse(clientHttpResponse);
assertSame(DefaultEngineeringCRS.GENERIC_2D, crs);
}

private CoordinateReferenceSystem parseCoordinateReferenceSystemFromResponse(
final MockClientHttpResponse clientHttpResponse) {
requestFactory.registerHandler(
input -> input != null && input.getHost().equals("spatialreference.org"),
new TestHttpClientFactory.Handler() {
@Override
public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
String wkt =
"GEOGCS[\"GCS_WGS_1984\",DATUM[\"D_WGS_1984\",SPHEROID[\"WGS_1984\",6378137,298"
+ ".257223563]],"
+ "PRIMEM[\"Greenwich\","
+ "0],UNIT[\"Degree\",0.017453292519943295]]";
MockClientHttpRequest mockClientHttpRequest = new MockClientHttpRequest();
mockClientHttpRequest.setResponse(
new MockClientHttpResponse(wkt.getBytes(Constants.DEFAULT_CHARSET), HttpStatus.OK));
mockClientHttpRequest.setResponse(clientHttpResponse);
return mockClientHttpRequest;
}
});
Expand All @@ -161,10 +179,11 @@ public MockClientHttpRequest handleRequest(URI uri, HttpMethod httpMethod) {
CoordinateReferenceSystem crs =
FeaturesParser.parseCoordinateReferenceSystem(this.requestFactory, crsJSON, false);
assertNotNull(crs);
assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
return crs;
}

public void testParseCRSLinkProj4() {
@Test
public void testUnsupportedLinkTypeProj4() {
JSONObject crsJSON =
new JSONObject(
"{\"crs\":{\n"
Expand All @@ -179,7 +198,7 @@ public void testParseCRSLinkProj4() {
CoordinateReferenceSystem crs =
FeaturesParser.parseCoordinateReferenceSystem(this.requestFactory, crsJSON, false);
assertNotNull(crs);
assertNotSame(DefaultEngineeringCRS.GENERIC_2D, crs);
assertSame(DefaultEngineeringCRS.GENERIC_2D, crs);
}

@Test
Expand Down
Loading
Loading