Skip to content

Commit

Permalink
[GEOS-9207] REST exception handler and controllers do not always set …
Browse files Browse the repository at this point in the history
…the response content type
  • Loading branch information
aaime committed May 15, 2019
1 parent 5e4489f commit 45b6d5e
Show file tree
Hide file tree
Showing 45 changed files with 200 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ public Object backupDelete(
MediaType.APPLICATION_XML_VALUE,
MediaType.APPLICATION_JSON_VALUE,
MediaTypeExtensions.TEXT_JSON_VALUE
}
},
produces = MediaType.TEXT_PLAIN_VALUE
)
@ResponseStatus(HttpStatus.CREATED)
public Object backupPost(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ public Object restoreDelete(
MediaType.APPLICATION_XML_VALUE,
MediaType.APPLICATION_JSON_VALUE,
MediaTypeExtensions.TEXT_JSON_VALUE
}
},
produces = MediaType.TEXT_PLAIN_VALUE
)
@ResponseStatus(HttpStatus.CREATED)
public Object restorePost(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ private ResponseEntity<String> returnCreatedCollectionReference(
baseURL, "/rest/oseo/collections/" + eoId, null, URLType.RESOURCE);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(new URI(newCollectionLocation));
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(eoId, headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ private ResponseEntity<String> returnCreatedProductReference(
URLType.RESOURCE);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(new URI(newCollectionLocation));
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(productId, headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public ResponseEntity<String> postEchoParameter(@RequestBody EchoParameter newVa
// return the location of the created echo parameter
HttpHeaders headers = new HttpHeaders();
headers.setLocation(new URI(RequestInfo.get().pageURI(newValue.getId())));
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(newValue.getId(), headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public ResponseEntity<String> postRule(@RequestBody Rule newValue) throws URISyn
// return the location of the created echo parameter
HttpHeaders headers = new HttpHeaders();
headers.setLocation(new URI(RequestInfo.get().pageURI(newValue.getId())));
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(newValue.getId(), headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.geoserver.rest.RestBaseController;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
Expand Down Expand Up @@ -122,7 +123,11 @@ public ResponseEntity<Long> insert(@RequestBody JaxbAdminRule rule) {
return new ResponseEntity<>(adminService.insert(rule.toRule()), HttpStatus.CREATED);
}

@RequestMapping(value = "/adminrules/id/{id}", method = RequestMethod.POST)
@RequestMapping(
value = "/adminrules/id/{id}",
method = RequestMethod.POST,
produces = MediaType.TEXT_PLAIN_VALUE
)
public @ResponseStatus(HttpStatus.OK) void update(
@PathVariable("id") Long id, @RequestBody JaxbAdminRule rule) {
if (rule.getPriority() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,11 @@ public JaxbRuleList count(
MediaType.APPLICATION_XML_VALUE,
MediaType.APPLICATION_JSON_VALUE,
MediaTypeExtensions.TEXT_JSON_VALUE
}
},
produces = MediaType.TEXT_PLAIN_VALUE
)
@ResponseStatus(HttpStatus.CREATED)
public ResponseEntity<Long> insert(@RequestBody(required = true) JaxbRule rule) {
public String insert(@RequestBody(required = true) JaxbRule rule) {
long priority = rule.getPriority() == null ? 0 : rule.getPriority().longValue();
if (adminService.getRuleByPriority(priority) != null) {
adminService.shift(priority, 1);
Expand All @@ -206,7 +207,7 @@ public ResponseEntity<Long> insert(@RequestBody(required = true) JaxbRule rule)
adminService.setDetails(id, rule.getLayerDetails().toLayerDetails(null));
}

return new ResponseEntity<Long>(id, HttpStatus.CREATED);
return String.valueOf(id);
}

@RequestMapping(value = "/rules/id/{id}", method = RequestMethod.POST)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void testInsertUpdateDelete() {
JaxbRule rule2 = new JaxbRule();
rule2.setPriority(5L);
rule2.setAccess("DENY");
long id2 = controller.insert(rule2).getBody();
long id2 = Long.parseLong(controller.insert(rule2));

realRule = adminService.get(id);
assertEquals(6L, realRule.getPriority());
Expand Down Expand Up @@ -622,7 +622,7 @@ protected long prepareGeoFenceTestRules(JaxbRule rule) {
}
}

long id = controller.insert(rule).getBody();
long id = Long.parseLong(controller.insert(rule));
return id;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public ResponseEntity<Object> postImports(
}
UriComponents uriComponents = getUriComponents(context.getId().toString(), builder);
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.setLocation(uriComponents.toUri());
return new ResponseEntity<>(context, headers, HttpStatus.CREATED);
}
Expand Down Expand Up @@ -118,6 +119,7 @@ public ResponseEntity<Object> putImport(
UriComponents uriComponents = getUriComponents(context.getId().toString(), builder);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.APPLICATION_JSON);
return new ResponseEntity<>(context, headers, HttpStatus.CREATED);
} else {
throw new RestException("ID must be provided for PUT", HttpStatus.BAD_REQUEST);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public ResponseEntity postTransform(
taskId.toString(),
task.getTransform().getTransforms().size() - 1)
.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<String>("", headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.springframework.http.MediaType;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;

Expand Down Expand Up @@ -223,7 +224,7 @@ public void testPutWithId() throws Exception {
putAsServletResponse(
RestBaseController.ROOT_PATH + "/imports/8675309", "", "application/json");
assertEquals(201, resp.getStatus());

assertEquals(MediaType.APPLICATION_JSON_VALUE, resp.getContentType());
JSONObject json = (JSONObject) json(resp);
JSONObject imprt = json.getJSONObject("import");

Expand All @@ -240,6 +241,7 @@ public void testPutWithId() throws Exception {
resp =
putAsServletResponse(
RestBaseController.ROOT_PATH + "/imports/8675000", "", "application/json");
assertEquals(MediaType.APPLICATION_JSON_VALUE, resp.getContentType());
assertEquals(201, resp.getStatus());
// it should be one more than the latest

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,9 @@ void testDirectExecutePhasedInternal(boolean async) throws Exception {
}
}
} else {
json = (JSONObject) getAsJSON("/rest/imports/" + importId);
MockHttpServletResponse response = getAsServletResponse("/rest/imports/" + importId);
assertEquals("application/json", response.getContentType());
json = (JSONObject) json(response);
state = json.getJSONObject("import").getString("state");
}
Thread.sleep(500);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ private ResponseEntity<String> buildResponse(
.buildAndExpand(transformInfoName);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(transformInfoName, headers, status);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.geoserver.wfs.xslt.config.TransformRepository;
import org.junit.Before;
import org.junit.Test;
import org.springframework.http.MediaType;
import org.springframework.mock.web.MockHttpServletResponse;
import org.w3c.dom.Document;

Expand Down Expand Up @@ -134,6 +135,7 @@ public void testPostXML() throws Exception {
postAsServletResponse(
RestBaseController.ROOT_PATH + "/services/wfs/transforms", xml);
assertEquals(201, response.getStatus());
assertEquals(MediaType.TEXT_PLAIN_VALUE, response.getContentType());
assertNotNull(response.getHeader("Location"));
assertTrue(
response.getHeader("Location")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.geoserver.platform.GeoServerExtensions;
import org.geotools.util.logging.Logging;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.util.StreamUtils;
Expand Down Expand Up @@ -84,6 +85,7 @@ public void handleResourceNotFound(
LOGGER.log(Level.SEVERE, e.getMessage(), e);
}
response.setStatus(404);
response.setContentType(MediaType.TEXT_PLAIN_VALUE);
StreamUtils.copy(message, Charset.forName("UTF-8"), os);
}

Expand All @@ -99,6 +101,7 @@ public void handleRestException(
} else {
response.setStatus(e.getStatus().value());
}
response.setContentType(MediaType.TEXT_PLAIN_VALUE);
StreamUtils.copy(e.getMessage(), Charset.forName("UTF-8"), os);
}

Expand All @@ -119,6 +122,7 @@ public void handleGeneralException(
notifyExceptionToCallbacks(request, response, e);

response.setStatus(500);
response.setContentType(MediaType.TEXT_PLAIN_VALUE);
StreamUtils.copy(e.getMessage(), Charset.forName("UTF-8"), os);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* (c) 2019 Open Source Geospatial Foundation - all rights reserved
* This code is licensed under the GPL 2.0 license, available at the root
* application directory.
*/
package org.geoserver.rest;

import static org.junit.Assert.assertEquals;

import org.geoserver.data.test.SystemTestData;
import org.geoserver.test.GeoServerSystemTestSupport;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletResponse;

public class ExceptionHandlingTest extends GeoServerSystemTestSupport {

@Override
protected void setUpTestData(SystemTestData testData) throws Exception {
// no data needed for this test
}

@Test
public void testRestException() throws Exception {
MockHttpServletResponse response =
getAsServletResponse(
RestBaseController.ROOT_PATH + "/exception?code=400&message=error");
assertEquals(400, response.getStatus());
assertEquals("text/plain", response.getContentType());
}

@Test
public void testInternalError() throws Exception {
MockHttpServletResponse response =
getAsServletResponse(RestBaseController.ROOT_PATH + "/error");
assertEquals(500, response.getStatus());
assertEquals("text/plain", response.getContentType());
}

@Test
public void testNotFound() throws Exception {
MockHttpServletResponse response =
getAsServletResponse(RestBaseController.ROOT_PATH + "/notfound");
assertEquals(404, response.getStatus());
assertEquals("text/plain", response.getContentType());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,28 @@
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping(path = RestBaseController.ROOT_PATH + "/exception")
public class ExceptionThrowingController {

@GetMapping
public void handleGet(
@RequestMapping(path = RestBaseController.ROOT_PATH + "/exception")
public void handleException(
@RequestParam(name = "message", required = false) String message,
@RequestParam(name = "code", required = false) Integer code) {

throw new RestException(
message != null ? message : "Unknown error",
code != null ? HttpStatus.valueOf(code) : HttpStatus.INTERNAL_SERVER_ERROR);
}

@GetMapping
@RequestMapping(path = RestBaseController.ROOT_PATH + "/error")
public void handleError() {
throw new RuntimeException("An internal error occurred");
}

@GetMapping
@RequestMapping(path = RestBaseController.ROOT_PATH + "/notfound")
public void handleNotFound() {
throw new ResourceNotFoundException("I'm not there");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import static org.junit.Assert.*;

import org.easymock.EasyMock;
import org.geoserver.data.test.SystemTestData;
import org.geoserver.platform.GeoServerExtensionsHelper;
import org.geoserver.test.GeoServerSystemTestSupport;
import org.junit.Before;
Expand All @@ -22,6 +23,11 @@ public class RESTDispatcherCallbackTest extends GeoServerSystemTestSupport {

DispatcherCallback callback;

@Override
protected void setUpTestData(SystemTestData testData) throws Exception {
// no data needed for this test
}

@Before
public void prepareCallback() throws Exception {
callback = EasyMock.createMock(DispatcherCallback.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public RestWrapper<SettingsInfo> localSettingsGet(@PathVariable String workspace
MediaTypeExtensions.TEXT_JSON_VALUE,
MediaType.APPLICATION_XML_VALUE,
MediaType.TEXT_XML_VALUE
}
},
produces = MediaType.TEXT_PLAIN_VALUE
)
@ResponseStatus(HttpStatus.CREATED)
public String localSettingsCreate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ public ResponseEntity<String> coveragePost(
}
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(coverageName, headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public ResponseEntity<String> coverageStorePost(
.buildAndExpand(workspaceName, storeName);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(storeName, headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public ResponseEntity<String> dataStorePost(
.buildAndExpand(workspaceName, storeName);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(storeName, headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ public ResponseEntity featureTypePost(

HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>("", headers, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public ResponseEntity<String> layerGroupPost(
builder.path("/layergroups/{layerGroupName}").buildAndExpand(layerGroupName);
HttpHeaders httpHeaders = new HttpHeaders();
httpHeaders.setLocation(uriComponents.toUri());

httpHeaders.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(layerGroupName, httpHeaders, HttpStatus.CREATED);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public ResponseEntity<String> namespacePost(
UriComponents uriComponents = getUriComponents(name, builder);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(uriComponents.toUri());
headers.setContentType(MediaType.TEXT_PLAIN);
return new ResponseEntity<>(name, headers, HttpStatus.CREATED);
}

Expand Down

0 comments on commit 45b6d5e

Please sign in to comment.