Fix PATCH not working in Transactions

Resolves both: https://www.pivotaltracker.com/story/show/66262856 , and https://www.pivotaltracker.com/story/show/64058980
fcrepo · Mar 18, 2014 · cd363f9 · cd363f9
1 parent 1288353
commit cd363f9
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 5 deletions.
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraTransactionsIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraTransactionsIT.java
@@ -23,10 +23,13 @@
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPatch;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.BasicHttpEntity;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
 import static com.hp.hpl.jena.graph.Node.ANY;
@@ -37,6 +40,8 @@
 import static java.lang.Thread.sleep;
 import static java.util.UUID.randomUUID;
 import static java.util.regex.Pattern.compile;
+import static javax.ws.rs.core.Response.Status.NO_CONTENT;
+import static org.fcrepo.kernel.RdfLexicon.DC_TITLE;
 import static org.jgroups.util.Util.assertFalse;
 import static org.fcrepo.kernel.TransactionImpl.DEFAULT_TIMEOUT;
 import static org.fcrepo.kernel.TransactionImpl.TIMEOUT_SYSTEM_PROPERTY;
@@ -265,6 +270,75 @@ public void testCreateDoStuffAndCommitTransactionSeparateConnections() throws Ex
 
     }
 
+    /**
+     * Tests whether a Sparql update is visible within a transaction
+     * and if the update is made persistent along with the commit.
+     * @throws Exception
+     */
+    @Test
+    public void testIngestNewWithSparqlPatchWithinTransaction() throws Exception {
+        final String objectInTxCommit = randomUUID().toString();
+
+        /* create new tx */
+        final HttpPost createTx = new HttpPost(serverAddress + "fcr:tx");
+        final HttpResponse response = execute(createTx);
+        assertEquals(201, response.getStatusLine().getStatusCode());
+
+        /* create a new object inside the tx */
+        final String txLocation =
+                response.getFirstHeader("Location").getValue();
+
+        client = createClient();
+        final String newObjectLocation = txLocation + "/" + objectInTxCommit;
+        final HttpPost postNew =
+                new HttpPost(newObjectLocation);
+        HttpResponse resp = execute(postNew);
+        assertEquals(201, resp.getStatusLine().getStatusCode());
+
+        /* update sparql */
+        final HttpPatch method = new HttpPatch(newObjectLocation);
+        method.addHeader("Content-Type", "application/sparql-update");
+        final BasicHttpEntity entity = new BasicHttpEntity();
+        final String title = "this is a new title";
+        entity.setContent(new ByteArrayInputStream(
+                ("INSERT { <> <http://purl.org/dc/elements/1.1/title> \"" + title + "\" } WHERE {}")
+                        .getBytes()));
+        method.setEntity(entity);
+        final HttpResponse responseFromPatch = client.execute(method);
+        final int status = responseFromPatch.getStatusLine().getStatusCode();
+        assertEquals("Didn't get a 204 status! Got status:\n" + status,
+                NO_CONTENT.getStatusCode(), status);
+
+        /* make sure the change was made within the tx */
+        final HttpGet httpGet = new HttpGet(newObjectLocation);
+        final GraphStore graphStore = getGraphStore(httpGet);
+        assertTrue("The sparql update did not succeed within a transaction",
+                graphStore.contains(ANY, createResource(newObjectLocation).asNode(),
+                DC_TITLE.asNode(), createPlainLiteral(title)
+                .asNode()));
+
+        /* commit */
+        client = createClient();
+        final HttpPost commitTx =
+                new HttpPost(txLocation + "/fcr:tx/fcr:commit");
+        resp = execute(commitTx);
+
+        assertEquals(204, resp.getStatusLine().getStatusCode());
+
+        /* it must exist after commit */
+        client = createClient();
+        final HttpGet getObjCommitted =
+                new HttpGet(serverAddress + objectInTxCommit);
+        final GraphStore graphStoreAfterCommit = getGraphStore(getObjCommitted);
+        assertTrue("The inserted triple does not exist after the transaction has committed",
+                graphStoreAfterCommit.contains(ANY, ANY,
+                DC_TITLE.asNode(), createPlainLiteral(title)
+                .asNode()));
+
+
+    }
+
+
     /**
      * Tests that transactions are treated as atomic with regards to nodes.
      *

diff --git a/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpGraphSubjects.java b/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpGraphSubjects.java
@@ -18,6 +18,7 @@
 
 import com.google.common.base.Function;
 import com.hp.hpl.jena.rdf.model.Resource;
+import org.apache.commons.lang.StringUtils;
 import org.fcrepo.kernel.exception.RepositoryRuntimeException;
 import org.fcrepo.kernel.rdf.GraphSubjects;
 import org.fcrepo.kernel.services.functions.GetDefaultWorkspace;
@@ -223,7 +224,16 @@ protected String getPathFromGraphSubject(@NotNull final String subjectUri) throw
 
     private boolean isValidJcrPath(final String absPath) {
         try {
-            session.getValueFactory().createValue(absPath, PATH);
+            String pathToValidate = absPath;
+            String txId = getCurrentTransactionId(session);
+            if (txId != null) {
+                String txIdWithSlash = "/" + TX_PREFIX + txId;
+                /* replace the first occurrence of tx within the path */
+                pathToValidate = StringUtils.replaceOnce(absPath, txIdWithSlash, StringUtils.EMPTY);
+                LOGGER.debug("removed {} from URI {}. Path for JCR validation is now: {}",
+                        txIdWithSlash, absPath, pathToValidate);
+            }
+            session.getValueFactory().createValue(pathToValidate, PATH);
             return true;
         } catch (final ValueFormatException e) {
             LOGGER.trace("Unable to validate JCR path", e);

diff --git a/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/api/rdf/GraphSubjectsTest.java b/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/api/rdf/GraphSubjectsTest.java
@@ -18,6 +18,7 @@
 
 import com.hp.hpl.jena.rdf.model.Resource;
 import com.sun.jersey.api.uri.UriBuilderImpl;
+import org.fcrepo.kernel.TxSession;
 import org.fcrepo.kernel.rdf.GraphSubjects;
 import org.junit.Before;
 import org.mockito.Mock;
@@ -46,6 +47,9 @@ public abstract class GraphSubjectsTest {
     @Mock
     protected Session mockSession;
 
+    @Mock
+    protected TxSession mockSessionTx;
+
     @Mock
     protected Repository mockRepository;
 
@@ -72,6 +76,7 @@ public void setUp() throws RepositoryException {
         when(mockSession.getValueFactory()).thenReturn(mockValueFactory);
         when(mockSession.getRepository()).thenReturn(mockRepository);
         testObj = getTestObj();
+
     }
 
     /**

diff --git a/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/api/rdf/HttpGraphSubjectsTest.java b/fcrepo-http-commons/src/test/java/org/fcrepo/http/commons/api/rdf/HttpGraphSubjectsTest.java
@@ -16,21 +16,22 @@
 
 package org.fcrepo.http.commons.api.rdf;
 
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import java.net.URI;
-import javax.jcr.Node;
-import javax.jcr.PathNotFoundException;
-import javax.jcr.RepositoryException;
+import java.util.UUID;
+import javax.jcr.*;
 import javax.ws.rs.Path;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 
 import com.hp.hpl.jena.rdf.model.ResourceFactory;
-import org.fcrepo.http.commons.api.rdf.HttpGraphSubjects;
+import org.fcrepo.kernel.rdf.GraphSubjects;
 import org.junit.Test;
 import org.mockito.Mockito;
 
@@ -39,11 +40,18 @@
 
 public class HttpGraphSubjectsTest extends GraphSubjectsTest {
 
+    @Override
     protected HttpGraphSubjects getTestObj() {
         return new HttpGraphSubjects(mockSession, MockNodeController.class,
                 uriInfo);
     }
 
+    protected HttpGraphSubjects getTestObjTx(final String path) {
+        return new HttpGraphSubjects(mockSessionTx, MockNodeController.class,
+                getUriInfoImpl(path));
+    }
+
+
     @Test
     public void testGetGraphSubject() throws RepositoryException {
         final String expected = "http://localhost:8080/fcrepo/rest" + testPath;
@@ -114,6 +122,22 @@ public void testIsFedoraGraphSubject() {
         assertEquals(false, actual);
     }
 
+    @Test
+    public void testIsFedoraGraphSubjectWithTx() throws RepositoryException {
+        final String txId = UUID.randomUUID().toString();
+        final String testPathTx = "/" + txId + "/hello";
+
+        when(mockSessionTx.getValueFactory()).thenReturn(mockValueFactory);
+        GraphSubjects testObjTx = getTestObjTx(testPathTx);
+        when(mockSessionTx.getTxId()).thenReturn(txId);
+        when(mockSubject.getURI()).thenReturn(
+                "http://localhost:8080/fcrepo/rest/tx:" + txId + "/hello");
+        when(mockSubject.isURIResource()).thenReturn(true);
+        boolean actual = testObjTx.isFedoraGraphSubject(mockSubject);
+        verify(mockValueFactory).createValue("/hello", PropertyType.PATH);
+        assertTrue("Must be valid GraphSubject", actual);
+    }
+
     @Test
     public void testGetContext() {
         assertEquals(uriInfo.getRequestUri().toString(), testObj.getContext().getURI());