Repository: apache/jena (java)

Issue:
DocumentGraphRepository: prevent double loading of the same graph
### Version

5.1.0

### Feature

Currently, it is possible to load the same document source multiple times, which will result in several different graphs with the same data.
I think it would be more convenient and useful to change `DocumentRepository#addMapping` so that it doesn't create a new source every time.

Related discussion: https://lists.apache.org/thread/tb58m9c84rvov2zgotdo43r1g7xc61r1

### Are you interested in contributing a solution yourself?

Yes

Pull request description:
GH-2628: DocumentGraphRepository -- allow multiple mappings, + minor changes

GitHub issue resolved #2628

Pull request Description:

Refactoring of `DocumentGraphRepository`; allow multiple mappings

----

 - [x] Tests are included.
 - [ ] Documentation change and updates are provided for the [Apache Jena website](https://github.com/apache/jena-site/)
 - [x] Commits have been squashed to remove intermediate development commit messages.
 - [x] Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).

----

See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).


Patch:
diff --git a/jena-ontapi/src/main/java/org/apache/jena/ontapi/GraphRepository.java b/jena-ontapi/src/main/java/org/apache/jena/ontapi/GraphRepository.java
index 56085f4944..aefc3a9805 100644
--- a/jena-ontapi/src/main/java/org/apache/jena/ontapi/GraphRepository.java
+++ b/jena-ontapi/src/main/java/org/apache/jena/ontapi/GraphRepository.java
@@ -38,11 +38,11 @@ import java.util.stream.Stream;
 public interface GraphRepository {
 
     /**
-     * A factory method to creates {@link GraphRepository} instance
-     * that loads graphs on demand from the location to memory.
+     * A factory method to create {@link GraphRepository} instance
+     * that loads graphs on demand from the given location.
      * The location is specified by the method {@link DocumentGraphRepository#addMapping(String, String)}.
      * If there is no mapping specified,
-     * graph id (see {@link GraphRepository#get(String)}) will be used as a source URL or file path.
+     * graph id will be used as a source.
      *
      * @return {@link DocumentGraphRepository}
      */
@@ -52,12 +52,12 @@ public interface GraphRepository {
 
     /**
      * A factory method to creates {@link GraphRepository} instance
-     * that loads graphs on demand from the location.
+     * that loads graphs on demand from the given location.
      * The location is specified by the method {@link DocumentGraphRepository#addMapping(String, String)}.
      * If there is no mapping specified,
-     * graph id (see {@link GraphRepository#get(String)}) will be used as a source URL or file path.
+     * graph id will be used as a source URL or file path.
      *
-     * @param factory {@link Supplier} to produce new {@link Graph}, {@code null} for default
+     * @param factory {@link Supplier} to produce new {@link Graph}, not {@code null}
      * @return {@link DocumentGraphRepository}
      */
     static DocumentGraphRepository createGraphDocumentRepository(Supplier<Graph> factory) {
@@ -100,7 +100,7 @@ public interface GraphRepository {
      * Removes graph.
      *
      * @param id {@code String} Graph's identifier
-     * @return {@link Graph}
+     * @return {@link Graph} associated with the id, or null if there was no graph for the given id
      */
     Graph remove(String id);
 
diff --git a/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/DocumentGraphRepository.java b/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/DocumentGraphRepository.java
index 6f88719300..0c307e4efc 100644
--- a/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/DocumentGraphRepository.java
+++ b/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/DocumentGraphRepository.java
@@ -18,9 +18,9 @@
 
 package org.apache.jena.ontapi.impl.repositories;
 
-import org.apache.jena.ontapi.GraphRepository;
 import org.apache.jena.graph.Graph;
 import org.apache.jena.graph.GraphMemFactory;
+import org.apache.jena.ontapi.GraphRepository;
 import org.apache.jena.riot.Lang;
 import org.apache.jena.riot.RDFLanguages;
 import org.apache.jena.riot.RDFParser;
@@ -28,13 +28,11 @@ import org.apache.jena.shared.JenaException;
 
 import java.net.URI;
 import java.nio.file.Paths;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
@@ -43,8 +41,9 @@ import java.util.stream.Stream;
 public class DocumentGraphRepository implements GraphRepository {
 
     private final Supplier<Graph> factory;
-    private final Map<String, Source> mappings = new HashMap<>();
-    private final Map<String, Graph> graphs = new HashMap<>();
+
+    private final Map<String, Source> idToSource = new HashMap<>();
+    private final Map<Source, Graph> sourceToGraph = new HashMap<>();
 
     public DocumentGraphRepository() {
         this(GraphMemFactory::createDefaultGraph);
@@ -55,39 +54,39 @@ public class DocumentGraphRepository implements GraphRepository {
     }
 
     /**
-     * Validates the specified String is a valid URI or file path.
+     * Checks whether the specified string is a valid URI or a file path.
      *
-     * @param fileNameOrURI to validate
+     * @param uriOrFile to validate
      * @return the same string
      */
-    public static String check(String fileNameOrURI) {
-        Objects.requireNonNull(fileNameOrURI, "Null fileNameOrURI");
-        RuntimeException ex = new RuntimeException("Wrong URI: <" + fileNameOrURI + ">");
+    public static String checkLocation(String uriOrFile) {
+        Objects.requireNonNull(uriOrFile, "Null fileNameOrURI");
+        RuntimeException ex = new RuntimeException("Wrong URI: <" + uriOrFile + ">");
         String file = null;
-        if (fileNameOrURI.startsWith("file:")) {
-            file = fileNameOrURI.replace("file:", "/");
-        } else if (fileNameOrURI.startsWith("/")) {
-            file = fileNameOrURI;
+        if (uriOrFile.startsWith("file:")) {
+            file = uriOrFile.replace("file:", "/");
+        } else if (uriOrFile.startsWith("/")) {
+            file = uriOrFile;
         }
         try {
             if (file != null) {
                 Paths.get(file);
             }
-            return fileNameOrURI;
+            return uriOrFile;
         } catch (Exception e) {
             ex.addSuppressed(e);
         }
         try {
-            new URI(fileNameOrURI);
-            return fileNameOrURI;
+            new URI(uriOrFile);
+            return uriOrFile;
         } catch (Exception e) {
             ex.addSuppressed(e);
         }
         throw ex;
     }
 
-    private static Source parseLocation(String fileNameOrUri) {
-        return new Source(check(fileNameOrUri), RDFLanguages.resourceNameToLang(fileNameOrUri, Lang.RDFXML));
+    private static Source parseLocation(String uriOrFile) {
+        return new Source(checkLocation(uriOrFile), RDFLanguages.resourceNameToLang(uriOrFile, Lang.RDFXML));
     }
 
     private static Graph read(Source source, Graph target) {
@@ -97,68 +96,138 @@ public class DocumentGraphRepository implements GraphRepository {
 
     /**
      * Adds mapping Graph's ID &lt;-&gt; source document location,
-     * which can be a file path, class-resource path, or URI (ftp or http).
-     * Note that class-resource path string should be without leading "/" symbol.
+     * which can be an OS file path, class-resource path, or URI (ftp or http).
+     * File URL should be of the form {@code file:///...}.
+     * Class-resource path string should be without leading "/" symbol.
+     * After successful load,
+     * the graph will be available both by {@code id} and
+     * by {@code fileNameOrUri} (via {@link #get(String)} method).
+     * A graph can be associated with different identifiers but only with one source.
      *
-     * @param id            Graph's id
-     * @param fileNameOrUri location of the Graph document
+     * @param id        Graph's id, arbitrary string
+     * @param uriOrFile location of the Graph document (e.g. "file://ontology.ttl")
      * @return this instance
+     * @see DocumentGraphRepository#get(String)
      */
-    public DocumentGraphRepository addMapping(String id, String fileNameOrUri) {
-        mappings.put(Objects.requireNonNull(id, "Null Graph Id"), parseLocation(fileNameOrUri));
+    public DocumentGraphRepository addMapping(String id, String uriOrFile) {
+        Objects.requireNonNull(id, "Null Graph Id");
+        var source = parseLocation(Objects.requireNonNull(uriOrFile, "location (file or uri) is required"));
+        idToSource.put(id, source);
+        idToSource.put(uriOrFile, source);
         return this;
     }
 
-    private Source getMapping(String id) {
-        return mappings.computeIfAbsent(id, DocumentGraphRepository::parseLocation);
-    }
-
     /**
-     * Gets Graph by ID.
+     * Gets the graph by its ID, which can be ontology id, location (file or uri) or arbitrary identifier,
+     * if there is a mapping for it.
+     * The method attempts to load the graph if it is not yet in the repository.
      *
      * @param id {@code String} Graph's identifier
      * @return {@link Graph}
-     * @throws JenaException if graph can't be found
+     * @see DocumentGraphRepository#addMapping(String, String)
+     * @throws org.apache.jena.shared.JenaException if graph cannot be loaded
      */
     @Override
     public Graph get(String id) {
-        return graphs.computeIfAbsent(Objects.requireNonNull(id, "Null Graph Id"), s -> read(getMapping(s), factory.get()));
+        try {
+            var source = idToSource.computeIfAbsent(Objects.requireNonNull(id, "Null Graph Id"),
+                    DocumentGraphRepository::parseLocation);
+            return sourceToGraph.computeIfAbsent(source, it -> read(it, factory.get()));
+        } catch (JenaException e) {
+            idToSource.remove(id);
+            throw e;
+        }
     }
 
+    /**
+     * Lists all graph's identifiers.
+     * Note that the number of identifiers may exceed the number of graphs if there are multiple id-source mappings.
+     *
+     * @return {@code Stream} of ids
+     */
     @Override
     public Stream<String> ids() {
         return getIds().stream();
     }
 
+    /**
+     * Associates the graph with the specified id.
+     * If there is no id-source mapping yet, the given id will be used as a document source.
+     *
+     * @param id    {@code String} Graph's identifier, which can be arbitrary string
+     * @param graph {@link Graph}
+     * @return {@link Graph} the previously associated with id or {@code null}
+     */
     @Override
     public Graph put(String id, Graph graph) {
-        return graphs.put(Objects.requireNonNull(id, "Null Graph Id"), Objects.requireNonNull(graph, "Null Graph"));
+        Objects.requireNonNull(id, "Null Graph Id");
+        Objects.requireNonNull(graph, "Null Graph");
+        var source = idToSource.computeIfAbsent(id, it -> new Source(it, RDFLanguages.resourceNameToLang(id, Lang.RDFXML)));
+        return sourceToGraph.put(source, graph);
     }
 
+    /**
+     * Removes and returns the graph identified by the specified {@code id}, along with all its associations.
+     *
+     * @param id {@code String} Graph's identifier
+     * @return {@link Graph} or {@code null} if the graph is not found
+     */
     @Override
     public Graph remove(String id) {
-        mappings.remove(id);
-        return graphs.remove(Objects.requireNonNull(id, "Null Graph Id"));
+        var source = idToSource.remove(Objects.requireNonNull(id, "Null Graph Id"));
+        if (source == null) {
+            return null;
+        }
+        idToSource.entrySet().stream().toList().stream()
+                .filter(it -> source.equals(it.getValue()))
+                .forEach(it -> idToSource.remove(it.getKey()));
+        return sourceToGraph.remove(source);
     }
 
+    /**
+     * Removes all graphs.
+     */
     @Override
     public void clear() {
-        mappings.clear();
-        graphs.clear();
+        sourceToGraph.clear();
+        idToSource.clear();
     }
 
+    /**
+     * Returns the number of identifiers.
+     * Note that it may exceed the number of graphs if there are multiple associations.
+     *
+     * @return {@code long}
+     */
     @Override
     public long count() {
         return getIds().size();
     }
 
+    /**
+     * Returns all already loaded graphs.
+     * Note that the number of returned graphs may not be equal to the number of mappings ({@link #count()}).
+     *
+     * @return distinct {@code Stream} of {@link Graph}s
+     */
+    @Override
+    public Stream<Graph> graphs() {
+        return sourceToGraph.values().stream();
+    }
+
+    /**
+     * Returns {@code true} if a mapping for the specified identifier exists in the repository.
+     *
+     * @param id {@code String} Graph's identifier
+     * @return boolean
+     */
     @Override
     public boolean contains(String id) {
-        return graphs.containsKey(id) || mappings.containsKey(id);
+        return idToSource.containsKey(Objects.requireNonNull(id, "Null Graph Id"));
     }
 
-    public Set<String> getIds() {
-        return Stream.of(graphs.keySet(), mappings.keySet()).flatMap(Collection::stream).collect(Collectors.toUnmodifiableSet());
+    protected Set<String> getIds() {
+        return idToSource.keySet();
     }
 
     private record Source(String location, Lang lang) {
diff --git a/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/PersistentGraphRepository.java b/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/PersistentGraphRepository.java
index d6a797f8db..01e69d038a 100644
--- a/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/PersistentGraphRepository.java
+++ b/jena-ontapi/src/main/java/org/apache/jena/ontapi/impl/repositories/PersistentGraphRepository.java
@@ -37,7 +37,6 @@ import java.util.stream.Stream;
  * Note that this repository does not permit modification of underlying storage ({@code source}).
  * Removing graph from the {@code source} is not reflected by this repository,
  * so it will still contain reference to the graph, even the graph is no longer available via {@link GraphMaker}.
- * This repository does not permit modification of underlying storage ({@code source}).
  */
 public class PersistentGraphRepository implements GraphRepository {
     protected final GraphMaker source;
@@ -78,12 +77,12 @@ public class PersistentGraphRepository implements GraphRepository {
             if (Graphs.dataGraphs(graph).anyMatch(it -> !graphs.contains(it))) {
                 throw new IllegalArgumentException(
                         "Operation 'put' is not supported for the given UnionGraph (id = " + id + "):" +
-                                "it contains subgraphs that do not managed by the underlying storage");
+                                "it contains subgraphs that are not managed by the underlying storage");
             }
         } else if (!graphs.contains(graph)) {
             throw new IllegalArgumentException(
                     "Operation 'put' is not supported for the given Graph (id = " + id + "):" +
-                            "it does not managed by the underlying storage");
+                            "it is not managed by the underlying storage");
         }
         var prev = this.graphs.entrySet().stream()
                 .filter(it -> graph.equals(it.getValue()))


Test patch:
diff --git a/jena-ontapi/src/test/java/org/apache/jena/ontapi/DocumentGraphRepositoryTest.java b/jena-ontapi/src/test/java/org/apache/jena/ontapi/DocumentGraphRepositoryTest.java
index 74de302c8a..5f8f1ceada 100644
--- a/jena-ontapi/src/test/java/org/apache/jena/ontapi/DocumentGraphRepositoryTest.java
+++ b/jena-ontapi/src/test/java/org/apache/jena/ontapi/DocumentGraphRepositoryTest.java
@@ -18,17 +18,20 @@
 
 package org.apache.jena.ontapi;
 
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.GraphMemFactory;
 import org.apache.jena.ontapi.impl.UnionGraphImpl;
 import org.apache.jena.ontapi.impl.repositories.DocumentGraphRepository;
 import org.apache.jena.ontapi.testutils.MiscUtils;
 import org.apache.jena.ontapi.utils.Graphs;
-import org.apache.jena.graph.Graph;
-import org.apache.jena.graph.GraphMemFactory;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
 import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 public class DocumentGraphRepositoryTest {
 
@@ -39,23 +42,35 @@ public class DocumentGraphRepositoryTest {
                                 () -> new UnionGraphImpl(GraphMemFactory.createDefaultGraph())
                         )
                         .addMapping("http://www.w3.org/2002/07/owl#", "builtins-owl.rdf")
-                        .addMapping("X", "builtins-rdfs.rdf");
+                        .addMapping("http://www.w3.org/2002/07/owl#", "builtins-owl.rdf")
+                        .addMapping("X", "builtins-rdfs.rdf")
+                        .addMapping("Y", "builtins-rdfs.rdf");
 
         Graph g1 = repository.get("http://www.w3.org/2002/07/owl#");
         Assertions.assertEquals(159, g1.size());
         Assertions.assertInstanceOf(UnionGraph.class, g1);
         Assertions.assertFalse(((UnionGraph) g1).hasSubGraph());
+        Graph g2 = repository.get("builtins-owl.rdf");
+        Assertions.assertSame(g1, g2);
+        Assertions.assertEquals(List.of(g1), repository.graphs().toList());
 
-        Graph g2 = repository.get("X");
-        Assertions.assertEquals(163, g2.size());
-        Assertions.assertInstanceOf(UnionGraph.class, g2);
-        Assertions.assertFalse(((UnionGraph) g2).hasSubGraph());
+        Graph g3 = repository.get("X");
+        Assertions.assertEquals(163, g3.size());
+        Assertions.assertInstanceOf(UnionGraph.class, g3);
+        Assertions.assertFalse(((UnionGraph) g3).hasSubGraph());
+        Graph g4 = repository.get("builtins-rdfs.rdf");
+        Assertions.assertSame(g3, g4);
+        Assertions.assertEquals(Set.of(g1, g3), repository.graphs().collect(Collectors.toSet()));
 
-        Assertions.assertEquals(2, repository.ids().count());
+        Assertions.assertEquals(
+                List.of("X", "Y", "builtins-owl.rdf", "builtins-rdfs.rdf", "http://www.w3.org/2002/07/owl#"),
+                repository.ids().sorted().toList());
+        Assertions.assertEquals(5, repository.count());
 
-        Assertions.assertSame(g2, repository.remove("X"));
+        Assertions.assertSame(g3, repository.remove("X"));
 
-        Assertions.assertEquals(1, repository.ids().count());
+        Assertions.assertEquals(List.of("builtins-owl.rdf", "http://www.w3.org/2002/07/owl#"), repository.ids().sorted().toList());
+        Assertions.assertEquals(2, repository.count());
 
         repository.clear();
 
diff --git a/jena-ontapi/src/test/java/org/apache/jena/ontapi/OntUnionGraphRepositoryTest.java b/jena-ontapi/src/test/java/org/apache/jena/ontapi/OntUnionGraphRepositoryTest.java
index 2e95799810..8df9aa5bdd 100644
--- a/jena-ontapi/src/test/java/org/apache/jena/ontapi/OntUnionGraphRepositoryTest.java
+++ b/jena-ontapi/src/test/java/org/apache/jena/ontapi/OntUnionGraphRepositoryTest.java
@@ -66,7 +66,7 @@ public class OntUnionGraphRepositoryTest {
         Assertions.assertEquals(List.of(uFood), uWine.subGraphs().collect(Collectors.toList()));
         Assertions.assertEquals(List.of(uWine), uFood.subGraphs().collect(Collectors.toList()));
 
-        Assertions.assertEquals(2, repository.count());
+        Assertions.assertEquals(4, repository.count());
         Assertions.assertEquals(Set.of(uWine, uFood), repository.graphs().collect(Collectors.toSet()));
     }