Skip to content
Permalink
Browse files

Update error conditions when content-type (and/or an empty body) are …

…provided
  • Loading branch information...
cbeer committed Oct 9, 2014
1 parent 2e0e3cd commit 633f72ce48b70dde76b10ca305aedfd730f8e57e
@@ -52,7 +52,6 @@
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

@@ -72,7 +71,6 @@
import static javax.ws.rs.core.Response.Status.UNSUPPORTED_MEDIA_TYPE;
import static javax.ws.rs.core.Response.created;
import static javax.ws.rs.core.Response.noContent;
import static javax.ws.rs.core.Response.notAcceptable;
import static javax.ws.rs.core.Response.ok;
import static org.apache.commons.lang.StringUtils.isBlank;
import static org.apache.jena.riot.WebContent.contentTypeSPARQLUpdate;
@@ -219,7 +217,7 @@ public Response createOrReplaceObjectRdf(
@ContentLocation final InputStream requestBodyStream,
@QueryParam("checksum") final String checksum,
@HeaderParam("Content-Disposition") final ContentDisposition contentDisposition)
throws InvalidChecksumException {
throws InvalidChecksumException, IOException {

final FedoraResource resource;
final Response.ResponseBuilder response;
@@ -243,7 +241,9 @@ public Response createOrReplaceObjectRdf(

evaluateRequestPreconditions(request, servletResponse, resource, session);

if (requestContentType != null && requestBodyStream != null) {
if (requestBodyStream == null && !resource.isNew()) {

This comment has been minimized.

Copy link
@awoods

awoods Oct 12, 2014

Member

Given the idempotent nature of PUT, this behavior seems odd.

This comment has been minimized.

Copy link
@cbeer

cbeer Oct 13, 2014

Author Member

This is no different than the old behavior.

This comment has been minimized.

Copy link
@awoods

awoods Oct 13, 2014

Member

OK, we can leave it. However, do you agree with this behavior with PUT?

This comment has been minimized.

Copy link
@cbeer

cbeer Oct 13, 2014

Author Member

I think it is rightly a conflict. You PUT nothing, we added our fcrepo4 triples, if you put again, you're asking to remove our fcrepo4 triples (which you can't do), so you get a conflict. I don't think we necessarily need special exception handling for it, but the provided message might be nice for clients.

This comment has been minimized.

Copy link
@awoods

awoods Oct 13, 2014

Member

fair enough

throw new ClientErrorException("No RDF provided and the resource already exists!", CONFLICT);
} else if (requestBodyStream != null) {
if ((resource instanceof FedoraObject || resource instanceof Datastream)
&& isRdfContentType(contentType.toString())) {
try {
@@ -253,13 +253,10 @@ public Response createOrReplaceObjectRdf(
}
} else if (resource instanceof FedoraBinary) {
replaceResourceBinaryWithStream((FedoraBinary) resource,
requestBodyStream, contentDisposition, requestContentType.toString(), checksum);
} else {
throw new ClientErrorException(UNSUPPORTED_MEDIA_TYPE);
requestBodyStream, contentDisposition, contentType.toString(), checksum);
} else if (!resource.isNew()) {
throw new ClientErrorException("Invalid Content Type " + requestContentType, UNSUPPORTED_MEDIA_TYPE);
}

} else if (!resource.isNew()) {
throw new ClientErrorException("No RDF provided and the resource already exists!", CONFLICT);
}

try {
@@ -367,7 +364,7 @@ public Response createObject(@QueryParam("mixin") final String mixin,
effectiveContentType,
newObjectPath, contentDisposition);

if (requestBodyStream == null || requestContentType == null) {
if (requestBodyStream == null) {
LOGGER.trace("No request body detected");
} else {
LOGGER.trace("Received createObject with a request body and content type \"{}\"", contentTypeString);
@@ -385,8 +382,9 @@ public Response createObject(@QueryParam("mixin") final String mixin,
LOGGER.trace("Found SPARQL-Update content, applying..");
patchResourcewithSparql(result, IOUtils.toString(requestBodyStream));
} else {
throw new WebApplicationException(notAcceptable(null)
.entity("Invalid Content Type " + contentTypeString).build());
if (requestBodyStream.read() != -1) {
throw new ClientErrorException("Invalid Content Type " + contentTypeString, UNSUPPORTED_MEDIA_TYPE);
}
}
}

@@ -1298,9 +1298,36 @@ public void testRepeatedPut() throws Exception {
assertEquals(201, getStatus(firstPut));

final HttpPut secondPut = new HttpPut(serverAddress + pid);
secondPut.setHeader("Content-Type", "text/turtle");
assertEquals(409, getStatus(secondPut));
}

@Test
public void testCreateResourceWithoutContentType() throws Exception {
final String pid = getRandomUniquePid();

final HttpPut httpPut = new HttpPut(serverAddress + pid);
assertEquals(201, getStatus(httpPut));
}

@Test
public void testUpdateObjectWithoutContentType() throws Exception {
final String pid = getRandomUniquePid();
createObject(pid);

final HttpPut httpPut = new HttpPut(serverAddress + pid);
assertEquals(415, getStatus(httpPut));
}

@Test
public void testUpdateBinaryWithoutContentType() throws Exception {
final String pid = getRandomUniquePid();
createDatastream(pid, "x", "xyz");

final HttpPut httpPut = new HttpPut(serverAddress + pid + "/x");
assertEquals(204, getStatus(httpPut));
}

@Test
public void testRoundTripReplaceGraphForDatastream() throws Exception {

0 comments on commit 633f72c

Please sign in to comment.
You can’t perform that action at this time.