Code review

Author: acoburn

solid/src/main/java/com/inrupt/client/solid/SolidContainer.java

@@ -83,9 +83,10 @@ public SolidContainer(final URI identifier, final Dataset dataset, final Metadat
      */
     public Set<SolidResource> getResources() {
         // As defined by the Solid Protocol, containers always end with a slash.
-        if (getIdentifier().getPath().endsWith("/")) {
-            final String container = normalize(getIdentifier());
-            final Node node = new Node(rdf.createIRI(getIdentifier().toString()), getGraph());
+        final URI base = getIdentifier().normalize();
+        if (isContainer(base)) {
+            final String container = normalize(base);
+            final Node node = new Node(rdf.createIRI(base.toString()), getGraph());
             try (final Stream<Node.TypedNode> stream = node.getResources()) {
                 return stream.filter(child -> verifyContainmentIri(container, child)).map(child -> {
                     final Metadata.Builder builder = Metadata.newBuilder();
@@ -102,12 +103,13 @@ public Set<SolidResource> getResources() {
     public ValidationResult validate() {
         final List<String> messages = new ArrayList<>();
         // Verify that the container URI path ends with a slash
-        if (!getIdentifier().getPath().endsWith("/")) {
+        final URI base = getIdentifier().normalize();
+        if (!isContainer(base)) {
             messages.add("Container URI does not end in a slash");
         }
 
         // Get the normalized container URI
-        final String container = normalize(getIdentifier());
+        final String container = normalize(base);
         // Verify that all ldp:contains triples align with Solid expectations
         getGraph().stream(null, rdf.createIRI(LDP.contains.toString()), null)
             .collect(Collectors.partitioningBy(verifyContainmentTriple(container)))
@@ -121,6 +123,10 @@ public ValidationResult validate() {
         return new ValidationResult(false, messages);
     }
 
+    static boolean isContainer(final URI uri) {
+        return uri.normalize().getPath().endsWith("/");
+    }
+
     static String normalize(final URI uri) {
         return uri.normalize().toString().split("#")[0].split("\\?")[0];
     }
@@ -142,29 +148,30 @@ static Predicate<Triple> verifyContainmentTriple(final String container) {
 
     static boolean verifyContainmentIri(final String container, final IRI object) {
 
-        // 1 URI Structure Tests
+        // URI Structure Tests
+        final URI base = URI.create(container).normalize();
         final URI normalized = URI.create(object.getIRIString()).normalize();
 
-        // 1.A Query strings are not allowed in object URI
-        if (normalized.getQuery() != null) {
+        // Query strings are not allowed in subject or object URI
+        if (base.getQuery() != null || normalized.getQuery() != null) {
             return false;
         }
 
-        // 1.B URI fragments are not allowed in object URI
-        if (normalized.getFragment() != null) {
+        // URI fragments are not allowed in subject or object URI
+        if (base.getFragment() != null || normalized.getFragment() != null) {
             return false;
         }
 
-        // 2 Relative path tests
-        final URI base = URI.create(container).normalize();
-        final URI relative = base.relativize(normalized);
-
-        // 2.A Base URI cannot equal the object URI
-        if (base.equals(normalized)) {
+        // Base URI cannot equal the object URI
+        if (base.getScheme().equals(normalized.getScheme()) &&
+                base.getSchemeSpecificPart().equals(normalized.getSchemeSpecificPart())) {
             return false;
         }
 
-        // 2.B Object URI must be relative to (contained in) the base URI
+        // Relative path tests
+        final URI relative = base.relativize(normalized);
+
+        // Object URI must be relative to (contained in) the base URI
         if (relative.isAbsolute()) {
             return false;
         }
@@ -173,7 +180,7 @@ static boolean verifyContainmentIri(final String container, final IRI object) {
         final String normalizedPath = relativePath.endsWith("/") ?
             relativePath.substring(0, relativePath.length() - 1) : relativePath;
 
-        // 2.C Containment cannot skip intermediate nodes
+        // Containment cannot skip intermediate nodes
         if (normalizedPath.contains("/")) {
             return false;
         }

solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java

@@ -231,6 +231,8 @@ void testLowLevelSolidContainer() {
         expected.add(URIBuilder.newBuilder(uri).path("newContainer/").build());
         expected.add(URIBuilder.newBuilder(uri).path("test.txt").build());
         expected.add(URIBuilder.newBuilder(uri).path("test2.txt").build());
+        expected.add(URIBuilder.newBuilder(uri).path("test3").build());
+        expected.add(URIBuilder.newBuilder(uri).path("test4").build());
 
         client.send(Request.newBuilder(uri).build(), SolidResourceHandlers.ofSolidContainer())
             .thenAccept(response -> {

solid/src/test/resources/__files/container.ttl

@@ -7,6 +7,14 @@
     a ldp:BasicContainer ;
     dct:modified "2022-11-25T10:36:36Z"^^xsd:dateTime;
     ldp:contains <newContainer/>, <test.txt>, <test2.txt> .
+<intermediate/..>
+    a ldp:BasicContainer ;
+    dct:modified "2022-11-25T10:38:12Z"^^xsd:dateTime ;
+    ldp:contains <test3> .
+<intermediate/../>
+    a ldp:BasicContainer ;
+    dct:modified "2022-11-25T10:38:47Z"^^xsd:dateTime ;
+    ldp:contains <test4> .
 <newContainer/>
     a ldp:BasicContainer ;
     dct:modified "2022-11-25T10:36:36Z"^^xsd:dateTime .
@@ -16,10 +24,16 @@
 <test2.txt>
     a pl:Resource, ldp:NonRDFSource;
     dct:modified "2022-11-25T10:37:06Z"^^xsd:dateTime .
+<test3>
+    a ldp:RDFSource ;
+    dct:modified "2022-11-25T10:37:31Z"^^xsd:dateTime .
+<test4>
+    a ldp:RDFSource ;
+    dct:modified "2022-11-25T10:39:22Z"^^xsd:dateTime .
 
 # These containment triples should not be included in a getResources response
 <>
-    ldp:contains <https://example.com/other> , <newContainer/child> , <> , <./> ,
+    ldp:contains <https://example.com/other> , <newContainer/child> , <newContainer%2Fchild2> ,  <> , <./> ,
     <?foo> , <#bar> , <?foo#bar> , <./?foo> , <./#bar> , <./?foo#bar> ,
     <../?foo> , <../#bar> , <../?foo#bar> , <child?foo> , <child#bar> , <child?foo#bar> .
 <https://example.test/container/>