Skip to content

Commit

Permalink
Addresses invisible resources during transactions
Browse files Browse the repository at this point in the history 
Disables Last-Modified and Etag headers during transactions
Fixes fcrepo-67754438 and fcrepo-67755044
  • Loading branch information
@kaimst
kaimst authored and Andrew Woods committed Aug 7, 2014
1 parent 3845a80 commit 3be3016
Show file tree
Hide file tree
Showing 16 changed files with 246 additions and 76 deletions.
28 changes: 21 additions & 7 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/ContentExposingResource.java
View file
Expand Up @@ -16,14 +16,17 @@
package org.fcrepo.http.api;

import com.sun.jersey.core.header.ContentDisposition;

import org.fcrepo.http.commons.AbstractResource;
import org.fcrepo.http.commons.api.rdf.HttpIdentifierTranslator;
import org.fcrepo.http.commons.domain.Range;
import org.fcrepo.http.commons.responses.RangeRequestInputStream;
import org.fcrepo.kernel.Datastream;
import org.fcrepo.kernel.impl.services.TransactionServiceImpl;

import javax.jcr.Binary;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.CacheControl;
Expand Down Expand Up @@ -59,13 +62,16 @@ public abstract class ContentExposingResource extends AbstractResource {
*/
protected Response getDatastreamContentResponse(final Datastream ds, final String rangeValue, final Request request,
final HttpServletResponse servletResponse,
final HttpIdentifierTranslator subjects) throws
final HttpIdentifierTranslator subjects,
final Session session) throws
RepositoryException, IOException {

// we include an explicit etag, because the default behavior is to use the JCR node's etag, not
// the jcr:content node digest.
checkCacheControlHeaders(request, servletResponse, ds);

// the jcr:content node digest. The etag is only included if we are not within a transaction.
final String txId = TransactionServiceImpl.getCurrentTransactionId(session);
if (txId == null) {
checkCacheControlHeaders(request, servletResponse, ds, session);
}
final CacheControl cc = new CacheControl();
cc.setMaxAge(0);
cc.setMustRevalidate(true);
Expand Down Expand Up @@ -150,11 +156,13 @@ protected Response getDatastreamContentResponse(final Datastream ds, final Strin
* @param request
* @param servletResponse
* @param resource
* @param session
* @throws javax.jcr.RepositoryException
*/
protected static void checkCacheControlHeaders(final Request request,
final HttpServletResponse servletResponse,
final Datastream resource) throws RepositoryException {
final Datastream resource,
final Session session) throws RepositoryException {

final EntityTag etag = new EntityTag(resource.getContentDigest().toString());
final Date date = resource.getLastModifiedDate();
Expand All @@ -177,7 +185,7 @@ protected static void checkCacheControlHeaders(final Request request,
.lastModified(date).tag(etag).build());
}

addCacheControlHeaders(servletResponse, resource);
addCacheControlHeaders(servletResponse, resource, session);
}

/**
Expand All @@ -187,7 +195,13 @@ protected static void checkCacheControlHeaders(final Request request,
* @throws RepositoryException
*/
protected static void addCacheControlHeaders(final HttpServletResponse servletResponse,
final Datastream resource) throws RepositoryException {
final Datastream resource,
final Session session) throws RepositoryException {

final String txId = TransactionServiceImpl.getCurrentTransactionId(session);
if (txId != null) {
return;
}

final EntityTag etag = new EntityTag(resource.getContentDigest().toString());
final Date date = resource.getLastModifiedDate();
Expand Down
9 changes: 4 additions & 5 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraContent.java
View file
Expand Up @@ -142,7 +142,7 @@ public Response create(@PathParam("path")
final ResponseBuilder builder = created(new URI(subjects.getSubject(
datastream.getContentNode().getPath()).getURI()));

addCacheControlHeaders(servletResponse, datastream);
addCacheControlHeaders(servletResponse, datastream, session);

return builder.build();

Expand Down Expand Up @@ -180,8 +180,7 @@ public Response modifyContent(@PathParam("path") final List<PathSegment> pathLis

final Datastream ds =
datastreamService.getDatastream(session, path);

evaluateRequestPreconditions(request, ds);
evaluateRequestPreconditions(request, servletResponse, ds, session);
}

LOGGER.debug("create Datastream {}", path);
Expand Down Expand Up @@ -209,7 +208,7 @@ public Response modifyContent(@PathParam("path") final List<PathSegment> pathLis
builder = noContent();
}

addCacheControlHeaders(servletResponse, datastream);
addCacheControlHeaders(servletResponse, datastream, session);

return builder.build();
} finally {
Expand Down Expand Up @@ -241,7 +240,7 @@ public Response getContent(@PathParam("path") final List<PathSegment> pathList,
new HttpIdentifierTranslator(session, FedoraNodes.class,
uriInfo);
return getDatastreamContentResponse(ds, rangeValue, request, servletResponse,
subjects);
subjects, session);

} finally {
session.logout();
Expand Down
24 changes: 13 additions & 11 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraNodes.java
View file
Expand Up @@ -190,7 +190,7 @@ public Response head(@PathParam("path") final List<PathSegment> pathList,
final HttpIdentifierTranslator subjects =
new HttpIdentifierTranslator(session, this.getClass(), uriInfo);

checkCacheControlHeaders(request, servletResponse, resource);
checkCacheControlHeaders(request, servletResponse, resource, session);

addResourceHttpHeaders(servletResponse, resource, subjects);

Expand Down Expand Up @@ -227,7 +227,7 @@ public RdfStream describe(@PathParam("path") final List<PathSegment> pathList,

final FedoraResource resource = nodeService.getObject(session, path);

checkCacheControlHeaders(request, servletResponse, resource);
checkCacheControlHeaders(request, servletResponse, resource, session);

final HttpIdentifierTranslator subjects =
new HttpIdentifierTranslator(session, this.getClass(), uriInfo);
Expand Down Expand Up @@ -398,7 +398,7 @@ public Response updateSparql(@PathParam("path")
final FedoraResource resource =
nodeService.getObject(session, path);

evaluateRequestPreconditions(request, resource);
evaluateRequestPreconditions(request, servletResponse, resource, session);

final Dataset properties = resource.updatePropertiesDataset(new HttpIdentifierTranslator(
session, FedoraNodes.class, uriInfo), requestBody);
Expand All @@ -423,7 +423,7 @@ public Response updateSparql(@PathParam("path")
session.save();
versionService.nodeUpdated(resource.getNode());

addCacheControlHeaders(servletResponse, resource);
addCacheControlHeaders(servletResponse, resource, session);

return noContent().build();

Expand Down Expand Up @@ -491,7 +491,7 @@ public Response createOrReplaceObjectRdf(
preexisting = false;
}

evaluateRequestPreconditions(request, resource);
evaluateRequestPreconditions(request, servletResponse, resource, session);

final HttpIdentifierTranslator graphSubjects =
new HttpIdentifierTranslator(session, FedoraNodes.class, uriInfo);
Expand All @@ -511,7 +511,7 @@ public Response createOrReplaceObjectRdf(
}

session.save();
addCacheControlHeaders(servletResponse, resource);
addCacheControlHeaders(servletResponse, resource, session);
versionService.nodeUpdated(resource.getNode());

return response.build();
Expand Down Expand Up @@ -647,7 +647,7 @@ public Response createObject(@PathParam("path")

LOGGER.debug("Finished creating {} with path: {}", mixin, newObjectPath);

addCacheControlHeaders(servletResponse, result);
addCacheControlHeaders(servletResponse, result, session);

return response.build();

Expand Down Expand Up @@ -745,7 +745,8 @@ public Response createObjectFromFormPost(
@Timed
public Response deleteObject(@PathParam("path")
final List<PathSegment> pathList,
@Context final Request request) throws RepositoryException {
@Context final Request request,
@Context final HttpServletResponse servletResponse) throws RepositoryException {
throwIfPathIncludesJcr(pathList, "DELETE");
init(uriInfo);

Expand All @@ -755,7 +756,7 @@ public Response deleteObject(@PathParam("path")

final FedoraResource resource =
nodeService.getObject(session, path);
evaluateRequestPreconditions(request, resource);
evaluateRequestPreconditions(request, servletResponse, resource, session);

nodeService.deleteObject(session, path);
session.save();
Expand Down Expand Up @@ -821,7 +822,8 @@ public Response copyObject(@PathParam("path") final List<PathSegment> path,
@Timed
public Response moveObject(@PathParam("path") final List<PathSegment> pathList,
@HeaderParam("Destination") final String destinationUri,
@Context final Request request)
@Context final Request request,
@Context final HttpServletResponse servletResponse)
throws RepositoryException, URISyntaxException {
throwIfPathIncludesJcr(pathList, "MOVE");
init(uriInfo);
Expand All @@ -839,7 +841,7 @@ public Response moveObject(@PathParam("path") final List<PathSegment> pathList,
nodeService.getObject(session, path);


evaluateRequestPreconditions(request, resource);
evaluateRequestPreconditions(request, servletResponse, resource, session);

final IdentifierTranslator subjects =
new HttpIdentifierTranslator(session, FedoraNodes.class, uriInfo);
Expand Down
4 changes: 2 additions & 2 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraVersions.java
View file
Expand Up @@ -244,7 +244,7 @@ public RdfStream getVersion(@PathParam("path")
throw new WebApplicationException(status(NOT_FOUND).build());
}
final FedoraResource resource = new FedoraResourceImpl(node);
checkCacheControlHeaders(request, servletResponse, resource);
checkCacheControlHeaders(request, servletResponse, resource, session);
return resource.getTriples(nodeTranslator()).session(session).topic(
nodeTranslator().getSubject(resource.getNode().getPath()).asNode());
}
Expand Down Expand Up @@ -274,7 +274,7 @@ public Response getHistoricContent(@PathParam("path")
final HttpIdentifierTranslator subjects =
new HttpIdentifierTranslator(session, FedoraNodes.class,
uriInfo);
return getDatastreamContentResponse(ds, rangeValue, request, servletResponse, subjects);
return getDatastreamContentResponse(ds, rangeValue, request, servletResponse, subjects, session);

} finally {
session.logout();
Expand Down
2 changes: 1 addition & 1 deletion fcrepo-http-api/src/main/java/org/fcrepo/http/api/responses/BaseHtmlProvider.java
View file
Expand Up @@ -190,7 +190,7 @@ public void writeTo(final Dataset rdf, final Class<?> type,

// add standard headers
httpHeaders.put("Content-type", of((Object) TEXT_HTML));
setCachingHeaders(httpHeaders, rdf);
setCachingHeaders(httpHeaders, rdf, uriInfo);

final Template nodeTypeTemplate =
getTemplate(rdf, subject, annotations);
Expand Down
18 changes: 13 additions & 5 deletions fcrepo-http-api/src/test/java/org/fcrepo/http/api/FedoraNodesTest.java
View file
Expand Up @@ -338,7 +338,7 @@ public void testDeleteObject() throws RepositoryException {
.thenReturn(mockObject);
when(mockObject.getEtagValue()).thenReturn("");

final Response actual = testObj.deleteObject(createPathList(pid), mockRequest);
final Response actual = testObj.deleteObject(createPathList(pid), mockRequest, mockResponse);

assertNotNull(actual);
assertEquals(NO_CONTENT.getStatusCode(), actual.getStatus());
Expand Down Expand Up @@ -559,7 +559,7 @@ public void testMoveObject() throws RepositoryException, URISyntaxException {

final String pid = "foo";

testObj.moveObject(createPathList(pid), "http://localhost/fcrepo/bar", mockRequest);
testObj.moveObject(createPathList(pid), "http://localhost/fcrepo/bar", mockRequest, mockResponse);
verify(mockNodes).moveObject(mockSession, "/foo", "/bar");
}

Expand All @@ -574,7 +574,10 @@ public void testMoveMissingObject() throws RepositoryException, URISyntaxExcepti

final String pid = "foo";

final Response response = testObj.moveObject(createPathList(pid), "http://localhost/fcrepo/bar", mockRequest);
final Response response =
testObj.moveObject(createPathList(pid),
"http://localhost/fcrepo/bar", mockRequest,
mockResponse);
assertEquals(CONFLICT.getStatusCode(), response.getStatus());
}

Expand All @@ -590,7 +593,10 @@ public void testMoveObjectToExistingDestination() throws RepositoryException, UR

final String pid = "foo";

final Response response = testObj.moveObject(createPathList(pid), "http://localhost/fcrepo/baz", mockRequest);
final Response response =
testObj.moveObject(createPathList(pid),
"http://localhost/fcrepo/baz", mockRequest,
mockResponse);

assertEquals(PRECONDITION_FAILED.getStatusCode(), response.getStatus());
}
Expand All @@ -606,7 +612,9 @@ public void testMoveObjectWithBadDestination() throws RepositoryException, URISy

final String pid = "foo";

final Response response = testObj.moveObject(createPathList(pid), "http://somewhere/else/baz", mockRequest);
final Response response =
testObj.moveObject(createPathList(pid),
"http://somewhere/else/baz", mockRequest, mockResponse);

// BAD GATEWAY
assertEquals(SC_BAD_GATEWAY, response.getStatus());
Expand Down
13 changes: 10 additions & 3 deletions fcrepo-http-api/src/test/java/org/fcrepo/http/api/responses/BaseHtmlProviderTest.java
View file
Expand Up @@ -37,14 +37,18 @@
import java.io.Writer;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;

import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.UriInfo;

import org.apache.velocity.Template;
import org.apache.velocity.context.Context;
import org.fcrepo.http.commons.responses.HtmlTemplate;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;

Expand All @@ -60,11 +64,12 @@
*/
public class BaseHtmlProviderTest {

final BaseHtmlProvider baseHtmlProvider = new BaseHtmlProvider();
private final BaseHtmlProvider baseHtmlProvider = new BaseHtmlProvider();

Dataset testData = new DatasetImpl(createDefaultModel());
private final Dataset testData = new DatasetImpl(createDefaultModel());

{
@Before
public void setup() {
testData.asDatasetGraph().getDefaultGraph().add(
new Triple(createURI("test:subject"),
createURI("test:predicate"),
Expand All @@ -73,6 +78,8 @@ public class BaseHtmlProviderTest {
new Triple(createURI("test:subject"), primaryTypePredicate,
createLiteral("nt:file")));

final UriInfo info = Mockito.mock(UriInfo.class);
setField(baseHtmlProvider, "uriInfo", info);
}

@Test
Expand Down
13 changes: 13 additions & 0 deletions fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/AbstractResourceIT.java
View file
Expand Up @@ -272,6 +272,19 @@ protected HttpResponse setProperty(final String pid, final String txId,
return dcResp;
}

/**
* Creates a transaction, asserts that it's successful and
* returns the transaction location.
* @return
* @throws IOException
*/
protected String createTransaction() throws IOException {
final HttpPost createTx = new HttpPost(serverAddress + "fcr:tx");
final HttpResponse response = execute(createTx);
assertEquals(201, response.getStatusLine().getStatusCode());
return response.getFirstHeader("Location").getValue();
}

protected static void addMixin(final String pid, final String mixinUrl) throws IOException {
final HttpPatch updateObjectGraphMethod =
new HttpPatch(serverAddress + pid);
Expand Down

0 comments on commit 3be3016

Please sign in to comment.