Addressing test and session comments

fcrepo · Jul 17, 2015 · b0c99a7 · b0c99a7
1 parent ed08d37
commit b0c99a7
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 53 deletions.
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
@@ -1065,8 +1065,8 @@ public void testGetObjectReferencesIndirect() throws Exception {
         final String uuid = getRandomUniqueId();
         final String pid1 = uuid + "/parent";
         final String pid2 = uuid + "/child";
-        createObject(pid1);
-        createObject(pid2);
+        createObjectAndClose(pid1);
+        createObjectAndClose(pid2);
 
         final String memberRelation = "http://pcdm.org/models#hasMember";
 
@@ -1091,50 +1091,50 @@ public void testGetObjectReferencesIndirect() throws Exception {
         // retrieve the parent and verify the outbound triples exist
         final HttpGet getParent =  new HttpGet(serverAddress + pid1);
         getParent.addHeader("Accept", "application/n-triples");
-        final GraphStore parentGraph = getGraphStore(getParent);
-
-        assertTrue(parentGraph.contains(Node.ANY,
-                createURI(serverAddress + pid1),
-                createURI(memberRelation),
-                createURI(serverAddress + pid2)));
+        try (final CloseableGraphStore parentGraph = getGraphStore(getParent)) {
+            assertTrue(parentGraph.contains(Node.ANY,
+                    createURI(serverAddress + pid1),
+                    createURI(memberRelation),
+                    createURI(serverAddress + pid2)));
+        }
 
         // retrieve the members container and verify the LDP triples exist
         final HttpGet getContainer =  new HttpGet(serverAddress + pid1 + "/members");
         getContainer.addHeader("Prefer", "return=representation;include=\"http://www.w3.org/ns/ldp#PreferMembership\"");
         getContainer.addHeader("Accept", "application/n-triples");
-        final GraphStore containerGraph = getGraphStore(getContainer);
-
-        assertTrue(containerGraph.contains(Node.ANY,
-                createURI(serverAddress + pid1 + "/members"),
-                createURI("http://www.w3.org/ns/ldp#hasMemberRelation"),
-                createURI(memberRelation)));
+        try (final CloseableGraphStore containerGraph = getGraphStore(getContainer)) {
+            assertTrue(containerGraph.contains(Node.ANY,
+                    createURI(serverAddress + pid1 + "/members"),
+                    createURI("http://www.w3.org/ns/ldp#hasMemberRelation"),
+                    createURI(memberRelation)));
 
-        assertTrue(containerGraph.contains(Node.ANY,
-                createURI(serverAddress + pid1 + "/members"),
-                createURI("http://www.w3.org/ns/ldp#insertedContentRelation"),
-                createURI("http://www.openarchives.org/ore/terms/proxyFor")));
+            assertTrue(containerGraph.contains(Node.ANY,
+                    createURI(serverAddress + pid1 + "/members"),
+                    createURI("http://www.w3.org/ns/ldp#insertedContentRelation"),
+                    createURI("http://www.openarchives.org/ore/terms/proxyFor")));
 
-        assertTrue(containerGraph.contains(Node.ANY,
-                createURI(serverAddress + pid1 + "/members"),
-                createURI("http://www.w3.org/ns/ldp#membershipResource"),
-                createURI(serverAddress + pid1)));
+            assertTrue(containerGraph.contains(Node.ANY,
+                    createURI(serverAddress + pid1 + "/members"),
+                    createURI("http://www.w3.org/ns/ldp#membershipResource"),
+                    createURI(serverAddress + pid1)));
+        }
 
 
         // retrieve the member and verify inbound triples exist
         final HttpGet getMember =  new HttpGet(serverAddress + pid2);
         getMember.addHeader("Prefer", "return=representation; include=\"" + INBOUND_REFERENCES.toString() + "\"");
         getMember.addHeader("Accept", "application/n-triples");
-        final GraphStore memberGraph = getGraphStore(getMember);
-
-        assertTrue(memberGraph.contains(Node.ANY,
-                Node.ANY,
-                createURI("http://www.openarchives.org/ore/terms/proxyFor"),
-                createURI(serverAddress + pid2)));
+        try (final CloseableGraphStore memberGraph = getGraphStore(getMember)) {
+            assertTrue(memberGraph.contains(Node.ANY,
+                    Node.ANY,
+                    createURI("http://www.openarchives.org/ore/terms/proxyFor"),
+                    createURI(serverAddress + pid2)));
 
-        assertTrue(memberGraph.contains(Node.ANY,
-                createURI(serverAddress + pid1),
-                createURI(memberRelation),
-                createURI(serverAddress + pid2)));
+            assertTrue(memberGraph.contains(Node.ANY,
+                    createURI(serverAddress + pid1),
+                    createURI(memberRelation),
+                    createURI(serverAddress + pid2)));
+        }
     }
 
     @Test

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/converters/ValueConverter.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/converters/ValueConverter.java
@@ -27,6 +27,7 @@
 import org.slf4j.Logger;
 
 import javax.jcr.Node;
+import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.Value;
@@ -153,15 +154,26 @@ private static RDFNode stringliteral2node(final String literal) {
         }
     }
 
-    private RDFNode traverseLink(final Value v)
-            throws RepositoryException {
-        final javax.jcr.Node refNode;
+    private RDFNode traverseLink(final Value v) throws RepositoryException {
+        return getGraphSubject(nodeForValue(session,v));
+    }
+
+    /**
+     * Get the node that a property value refers to.
+     * @param session Session to use to load the node.
+     * @param v Value that refers to a node.
+     * @throws RepositoryException When there is an error accessing the node.
+     * @throws RepositoryRuntimeException When the value type is not PATH, REFERENCE or WEAKREFERENCE.
+    **/
+    public static javax.jcr.Node nodeForValue(final Session session, final Value v) throws RepositoryException {
         if (v.getType() == PATH) {
-            refNode = session.getNode(v.getString());
+            return session.getNode(v.getString());
+        } else if (v.getType() == REFERENCE || v.getType() == WEAKREFERENCE) {
+            return session.getNodeByIdentifier(v.getString());
         } else {
-            refNode = session.getNodeByIdentifier(v.getString());
+            throw new RepositoryRuntimeException("Cannot convert value of type "
+                    + PropertyType.nameFromValue(v.getType()) + " to a node reference");
         }
-        return getGraphSubject(refNode);
     }
 
     private RDFNode getGraphSubject(final javax.jcr.Node n) {

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/LdpContainerRdfContext.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/LdpContainerRdfContext.java
@@ -203,13 +203,9 @@ public Iterator<Triple> apply(final FedoraResource child) {
                 return Iterators.transform(values, new Function<Value, Triple>() {
                     @Override
                     public Triple apply(final Value input) {
-                        try {
-                            final RDFNode membershipResource = new ValueConverter(resource().getNode().getSession(),
-                                    translator()).convert(input);
-                            return create(subject(), memberRelation, membershipResource.asNode());
-                        } catch (final RepositoryException e) {
-                            throw new RepositoryRuntimeException(e);
-                        }
+                        final RDFNode membershipResource = new ValueConverter(session(),
+                                translator()).convert(input);
+                        return create(subject(), memberRelation, membershipResource.asNode());
                     }
                 });
             } catch (final RepositoryException e) {

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/NodeRdfContext.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/NodeRdfContext.java
@@ -18,7 +18,9 @@
 import static org.fcrepo.kernel.impl.identifiers.NodeResourceConverter.nodeToResource;
 
 import javax.jcr.Node;
+import javax.jcr.RepositoryException;
 
+import org.fcrepo.kernel.exception.RepositoryRuntimeException;
 import org.fcrepo.kernel.identifiers.IdentifierConverter;
 import org.fcrepo.kernel.models.FedoraResource;
 import org.fcrepo.kernel.utils.iterators.RdfStream;
@@ -53,6 +55,11 @@ public NodeRdfContext(final FedoraResource resource,
         this.resource = resource;
         this.idTranslator = idTranslator;
         this.subject = idTranslator.reverse().convert(resource).asNode();
+        try {
+            session(resource.getNode().getSession());
+        } catch (RepositoryException ex) {
+            throw new RepositoryRuntimeException(ex);
+        }
     }
 
     /**

diff --git a/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/ReferencesRdfContext.java b/fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/rdf/impl/ReferencesRdfContext.java
@@ -16,6 +16,7 @@
 package org.fcrepo.kernel.impl.rdf.impl;
 
 import static org.fcrepo.kernel.impl.identifiers.NodeResourceConverter.nodeConverter;
+import static org.fcrepo.kernel.impl.rdf.converters.ValueConverter.nodeForValue;
 import static javax.jcr.PropertyType.PATH;
 import static javax.jcr.PropertyType.REFERENCE;
 import static javax.jcr.PropertyType.WEAKREFERENCE;
@@ -28,7 +29,6 @@
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
-import javax.jcr.Session;
 import javax.jcr.Value;
 
 import java.util.Iterator;
@@ -42,7 +42,6 @@
 public class ReferencesRdfContext extends NodeRdfContext {
 
     private final PropertyToTriple property2triple;
-    private final Session session;
 
     /**
      * Add the inbound references from other nodes to this resource to the stream
@@ -57,7 +56,6 @@ public ReferencesRdfContext(final FedoraResource resource,
         throws RepositoryException {
         super(resource, idTranslator);
         property2triple = new PropertyToTriple(resource.getNode().getSession(), idTranslator);
-        session = resource.getNode().getSession();
         putReferencesIntoContext(resource.getNode().getWeakReferences());
         putReferencesIntoContext(resource.getNode().getReferences());
     }
@@ -80,10 +78,8 @@ private void putReferencesIntoContext(final Iterator<Property> properties) throw
         }
     }
     private void putProxyReferencesIntoContext(final Value v) throws RepositoryException {
-        if (v.getType() == PATH) {
-            putProxyReferencesIntoContext(session.getNode(v.getString()));
-        } else if (v.getType() == REFERENCE || v.getType() == WEAKREFERENCE) {
-            putProxyReferencesIntoContext(session.getNodeByIdentifier(v.getString()));
+        if (v.getType() == PATH || v.getType() == REFERENCE || v.getType() == WEAKREFERENCE) {
+            putProxyReferencesIntoContext(nodeForValue(session(), v));
         }
     }
     private void putProxyReferencesIntoContext(final Node n) throws RepositoryException {

diff --git a/fcrepo-kernel-impl/src/test/java/org/fcrepo/kernel/impl/rdf/impl/LdpRdfContextTest.java b/fcrepo-kernel-impl/src/test/java/org/fcrepo/kernel/impl/rdf/impl/LdpRdfContextTest.java
@@ -25,6 +25,8 @@
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 
 import org.fcrepo.kernel.models.Container;
@@ -51,6 +53,10 @@ public class LdpRdfContextTest {
 
     @Mock
     private Container mockContainer;
+
+    @Mock
+    private Node mockNode;
+
     @Mock
     private Session mockSession;
 
@@ -60,11 +66,15 @@ public class LdpRdfContextTest {
 
 
     @Before
-    public void setUp() {
+    public void setUp() throws RepositoryException {
         initMocks(this);
         when(mockResource.getPath()).thenReturn("/a");
         when(mockBinary.getPath()).thenReturn("/a");
         when(mockContainer.getPath()).thenReturn("/a");
+        when(mockResource.getNode()).thenReturn(mockNode);
+        when(mockBinary.getNode()).thenReturn(mockNode);
+        when(mockContainer.getNode()).thenReturn(mockNode);
+        when(mockNode.getSession()).thenReturn(mockSession);
 
         subjects = new DefaultIdentifierTranslator(mockSession);
     }