Fix the SPARQL node type deletion bug to return a clear error message…

… from the backend. Resolves: https://www.pivotaltracker.com/story/show/72805066
fcrepo · Jul 25, 2014 · 386906e · 386906e
1 parent 32110c8
commit 386906e
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 5 deletions.
diff --git a/fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraNodes.java b/fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraNodes.java
@@ -92,10 +92,14 @@
 import javax.ws.rs.core.UriInfo;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.hp.hpl.jena.rdf.model.Literal;
 import com.hp.hpl.jena.rdf.model.ResourceFactory;
+import com.hp.hpl.jena.rdf.model.Statement;
+import com.hp.hpl.jena.rdf.model.StmtIterator;
 import com.sun.jersey.multipart.FormDataParam;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.jena.riot.Lang;
 import org.fcrepo.http.commons.AbstractResource;
 import org.fcrepo.http.commons.api.rdf.HttpIdentifierTranslator;
@@ -379,7 +383,15 @@ public Response updateSparql(@PathParam("path")
                 LOGGER.info(
                         "Found these problems updating the properties for {}: {}",
                         path, problems);
-                return status(FORBIDDEN).entity(problems.toString())
+                final StringBuilder error = new StringBuilder();
+                final StmtIterator sit = problems.listStatements();
+                while (sit.hasNext()) {
+                    final String message = getMessage(sit.next());
+                    if (StringUtils.isNotEmpty(message) && error.indexOf(message) < 0) {
+                        error.append(message + " \n");
+                    }
+                }
+                return status(FORBIDDEN).entity(error.length() > 0 ? error.toString() : problems.toString())
                         .build();
             }
 
@@ -390,7 +402,7 @@ public Response updateSparql(@PathParam("path")
 
             return noContent().build();
 
-        } catch ( RuntimeException ex ) {
+        } catch ( final RuntimeException ex ) {
             final Throwable cause = ex.getCause();
             if ( cause != null && cause instanceof PathNotFoundException ) {
                 // the sparql update referred to a repository resource that doesn't exist
@@ -719,7 +731,7 @@ public Response deleteObject(@PathParam("path")
             nodeService.deleteObject(session, path);
             session.save();
             return noContent().build();
-        } catch (WebApplicationException ex) {
+        } catch (final WebApplicationException ex) {
             return (Response)ex.getResponse();
         } finally {
             session.logout();
@@ -858,4 +870,16 @@ protected void throwIfPathIncludesJcr(final List<PathSegment> pathList, final St
         }
     }
 
+    /*
+     * Return the statement's predicate and it's literal value if there's any
+     * @param stmt
+     * @return
+     */
+    private String getMessage(final Statement stmt) {
+        final Literal literal = stmt.getLiteral();
+        if (literal != null) {
+            return stmt.getPredicate().getURI() + ": " + literal.getString();
+        }
+        return null;
+    }
 }
diff --git a/...-kernel-impl/src/main/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListener.java b/...-kernel-impl/src/main/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListener.java
@@ -28,6 +28,7 @@
 import javax.jcr.Value;
 
 import com.hp.hpl.jena.rdf.model.AnonId;
+import org.apache.commons.lang.StringUtils;
 import org.fcrepo.kernel.RdfLexicon;
 import org.fcrepo.kernel.rdf.IdentifierTranslator;
 import org.fcrepo.kernel.impl.rdf.JcrRdfTools;
@@ -203,11 +204,11 @@ public void removedStatement(final Statement s) {
             if (s.getPredicate().equals(RDF.type)
                     && s.getObject().isResource()) {
                 final Resource mixinResource = s.getObject().asResource();
+                final String errorPrefix = "Error removing node type";
                 try {
                     final String namespacePrefix = session.getNamespacePrefix(mixinResource.getNameSpace());
                     final String mixinName = namespacePrefix + ":" + mixinResource.getLocalName();
 
-
                     if (FedoraTypesUtils.getNodeTypeManager(subjectNode).hasNodeType(mixinName)) {
                         try {
                             subjectNode.removeMixin(mixinName);
@@ -218,13 +219,30 @@ public void removedStatement(final Statement s) {
                                     RdfLexicon.COULD_NOT_STORE_PROPERTY,
                                     s.getPredicate().getURI(),
                                     e);
-                            problems.add(subject, RdfLexicon.COULD_NOT_STORE_PROPERTY, s.getPredicate().getURI());
+                            String errorMessage = e.getMessage();
+                            if (StringUtils.isNotBlank(errorMessage)) {
+                                errorMessage = errorPrefix + " '" + mixinName +
+                                        "': \n" + e.getClass().getName() + ": " + errorMessage;
+                            } else {
+                                errorMessage = errorPrefix + " '" + mixinName +
+                                        "': \n" + e.getClass().getName();
+                            }
+                            problems.add(subject, RdfLexicon.COULD_NOT_STORE_PROPERTY, errorMessage);
                         }
                         return;
                     }
 
                 } catch (final NamespaceException e) {
                     LOGGER.trace("Unable to resolve registered namespace for resource {}: {}", mixinResource, e);
+
+                    String errorMessage = e.getMessage();
+                    if (StringUtils.isNotBlank(errorMessage)) {
+                        errorMessage = errorPrefix + " " +
+                                e.getClass().getName() + ": "  + errorMessage;
+                    } else {
+                        errorMessage = errorPrefix + ": " + e.getClass().getName();
+                    }
+                    problems.add(subject, RdfLexicon.COULD_NOT_STORE_PROPERTY, errorMessage);
                 }
 
             }