Check for parent tombstones on 404 errors

Resolves: https://www.pivotaltracker.com/story/show/81855584
fcrepo · Oct 31, 2014 · a5d74fc · a5d74fc
1 parent 1e1282b
commit a5d74fc
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 7 deletions.
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/AbstractResourceIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/AbstractResourceIT.java
@@ -22,7 +22,6 @@
 import static javax.ws.rs.core.Response.Status.NO_CONTENT;
 import static javax.ws.rs.core.Response.Status.OK;
 import static org.fcrepo.http.commons.test.util.TestHelpers.parseTriples;
-import static org.hamcrest.CoreMatchers.anyOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
@@ -44,6 +43,7 @@
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
 import org.apache.http.client.methods.HttpPatch;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
@@ -179,7 +179,9 @@ protected static int getStatus(final HttpUriRequest method)
         final int result = response.getStatusLine().getStatusCode();
         if (!(result > 199) || !(result < 400)) {
             logger.warn("Got status {}", result);
-            logger.warn(EntityUtils.toString(response.getEntity()));
+            if (response.getEntity() != null) {
+                logger.warn(EntityUtils.toString(response.getEntity()));
+            }
         }
         return result;
     }
@@ -317,6 +319,7 @@ protected static String getRandomPropertyValue() {
     }
 
     protected static void assertDeleted(final String location) throws IOException {
-        assertThat("Expected object to be deleted", getStatus(new HttpGet(location)), anyOf(is(404), is(410)));
+        assertThat("Expected object to be deleted", getStatus(new HttpHead(location)), is(410));
+        assertThat("Expected object to be deleted", getStatus(new HttpGet(location)), is(410));
     }
 }
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraLdpIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraLdpIT.java
@@ -479,6 +479,18 @@ public void testDeleteObject() throws Exception {
         assertDeleted(location);
     }
 
+    @Test
+    public void testDeleteHierarchy() throws Exception {
+        final String pid = getRandomUniquePid();
+
+        createObject(pid + "/foo");
+
+        final String location = serverAddress + pid;
+        assertEquals(204, getStatus(new HttpDelete(location)));
+        assertDeleted(location);
+        assertDeleted(location + "/foo");
+    }
+
     @Test
     public void testDeleteBinary() throws Exception {
         final String pid = getRandomUniquePid();

diff --git a/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpResourceConverter.java b/fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpResourceConverter.java
@@ -25,6 +25,7 @@
 import static org.fcrepo.jcr.FedoraJcrTypes.FCR_VERSIONS;
 import static org.fcrepo.kernel.impl.identifiers.NodeResourceConverter.nodeConverter;
 import static org.fcrepo.kernel.impl.services.TransactionServiceImpl.getCurrentTransactionId;
+import static org.fcrepo.kernel.impl.utils.FedoraTypesUtils.getClosestExistingAncestor;
 import static org.fcrepo.kernel.impl.utils.FedoraTypesUtils.isFrozenNode;
 import static org.modeshape.jcr.api.JcrConstants.JCR_CONTENT;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -49,7 +50,9 @@
 import org.fcrepo.kernel.models.FedoraResource;
 import org.fcrepo.kernel.exception.IdentifierConversionException;
 import org.fcrepo.kernel.exception.RepositoryRuntimeException;
+import org.fcrepo.kernel.exception.TombstoneException;
 import org.fcrepo.kernel.identifiers.IdentifierConverter;
+import org.fcrepo.kernel.impl.TombstoneImpl;
 import org.fcrepo.kernel.impl.identifiers.HashConverter;
 import org.fcrepo.kernel.impl.identifiers.NamespaceConverter;
 import org.glassfish.jersey.uri.UriTemplate;
@@ -103,11 +106,9 @@ private UriBuilder uriBuilder() {
 
     @Override
     protected FedoraResource doForward(final Resource resource) {
+        final HashMap<String, String> values = new HashMap<>();
+        final String path = asString(resource, values);
         try {
-            final HashMap<String, String> values = new HashMap<>();
-
-            final String path = asString(resource, values);
-
             if (path != null) {
                 final Node node = getNode(path);
 
@@ -124,6 +125,17 @@ protected FedoraResource doForward(final Resource resource) {
             throw new IdentifierConversionException("Asked to translate a resource " + resource
                     + " that doesn't match the URI template");
         } catch (final RepositoryException e) {
+            try {
+                if ( e instanceof PathNotFoundException ) {
+                    final Node preexistingNode = getClosestExistingAncestor(session, path);
+                    if (TombstoneImpl.hasMixin(preexistingNode)) {
+                        throw new TombstoneException(new TombstoneImpl(preexistingNode));
+                    }
+                }
+            } catch (RepositoryException inner) {
+                LOGGER.debug("Error checking for parent tombstones", inner);
+            }
+
             throw new RepositoryRuntimeException(e);
         }
     }