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:
https://www.pivotaltracker.com/story/show/67755044 and
https://www.pivotaltracker.com/story/show/67754438
  • Loading branch information
@kaimst
kaimst authored and Andrew Woods committed Aug 7, 2014
1 parent 3c12712 commit 9b0ecac
Show file tree
Hide file tree
Showing 16 changed files with 256 additions and 80 deletions.
33 changes: 25 additions & 8 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 @@ -57,15 +60,20 @@ public abstract class ContentExposingResource extends AbstractResource {
* A helper method that does most of the work associated with processing a request
* for content (or a range of the content) into a Response
*/
protected Response getDatastreamContentResponse(final Datastream ds, final String rangeValue, final Request request,
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 +158,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 +187,7 @@ protected static void checkCacheControlHeaders(final Request request,
.lastModified(date).tag(etag).build());
}

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

/**
Expand All @@ -187,7 +197,14 @@ 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 {

// Do not add caching headers if in a transaction
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
27 changes: 14 additions & 13 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 @@ -743,9 +743,9 @@ public Response createObjectFromFormPost(
*/
@DELETE
@Timed
public Response deleteObject(@PathParam("path")
final List<PathSegment> pathList,
@Context final Request request) throws RepositoryException {
public Response deleteObject(@PathParam("path") final List<PathSegment> pathList,
@Context final Request request,
@Context final HttpServletResponse servletResponse) throws RepositoryException {
throwIfPathIncludesJcr(pathList, "DELETE");
init(uriInfo);

Expand All @@ -755,7 +755,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 +821,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 +840,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
19 changes: 14 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,10 @@ 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 9b0ecac

Please sign in to comment.