Merge pull request #13 from eilmiv/leaky_conjunctive_graphs

Fix conjunctive graphs leaking between stores

rdflib_django/store.py

@@ -94,6 +94,33 @@ def _get_named_graph(self, context):
             identifier=context.identifier, store=self.store
         )[0]
 
+    def _get_query_sets_for_triple(self, triple, context):
+        """
+        Determine correct query sets based on triple and context.
+
+        Respects None as wildcard.
+        If the object in triple is None the resulting list has two query sets,
+        one for Literal results and one for URIRef results.
+
+        This method always returns a list of size at least one.
+        """
+        s, p, o = triple
+        named_graph = self._get_named_graph(context)
+        query_sets = _get_query_sets_for_object(o)
+        filter_parameters = dict()
+        if named_graph is not None:
+            filter_parameters["context_id"] = named_graph.id
+        else:
+            filter_parameters["context__store"] = self.store
+        if s is not None:
+            filter_parameters["subject"] = s
+        if p is not None:
+            filter_parameters["predicate"] = p
+        if o is not None:
+            filter_parameters["object"] = o
+        query_sets = [qs.filter(**filter_parameters) for qs in query_sets]
+        return query_sets
+
     def open(self, configuration: str, create: bool = False) -> Optional[int]:
         """
         Opens the underlying store. This is only necessary when opening
@@ -167,45 +194,15 @@ def remove(self, triple, context=None):
         """
         Removes a triple from the store.
         """
-        s, p, o = triple
-        named_graph = self._get_named_graph(context)
-        query_sets = _get_query_sets_for_object(o)
-
-        filter_parameters = dict()
-        if named_graph is not None:
-            filter_parameters["context_id"] = named_graph.id
-        if s is not None:
-            filter_parameters["subject"] = s
-        if p is not None:
-            filter_parameters["predicate"] = p
-        if o is not None:
-            filter_parameters["object"] = o
-
-        query_sets = [qs.filter(**filter_parameters) for qs in query_sets]
-
+        query_sets = self._get_query_sets_for_triple(triple, context)
         for qs in query_sets:
             qs.delete()
 
     def triples(self, triple, context=None):
         """
         Returns all triples in the current store.
         """
-        s, p, o = triple
-        named_graph = self._get_named_graph(context)
-        query_sets = _get_query_sets_for_object(o)
-
-        filter_parameters = dict()
-        if named_graph is not None:
-            filter_parameters["context_id"] = named_graph.id
-        if s is not None:
-            filter_parameters["subject"] = s
-        if p is not None:
-            filter_parameters["predicate"] = p
-        if o is not None:
-            filter_parameters["object"] = o
-
-        query_sets = [qs.filter(**filter_parameters) for qs in query_sets]
-
+        query_sets = self._get_query_sets_for_triple(triple, context)
         for qs in query_sets:
             for statement in qs:
                 triple = statement.as_triple()
@@ -217,29 +214,11 @@ def __len__(self, context=None):
         """
         Returns the number of statements in this Graph.
         """
-        named_graph = self._get_named_graph(context)
-        if named_graph is not None:
-            return (
-                models.LiteralStatement.objects.filter(
-                    context_id=named_graph.id
-                ).count()
-                + models.URIStatement.objects.filter(
-                    context_id=named_graph.id
-                ).count()
-            )
-        else:
-            return (
-                models.URIStatement.objects.values(
-                    "subject", "predicate", "object"
-                )
-                .distinct()
-                .count()
-                + models.LiteralStatement.objects.values(
-                    "subject", "predicate", "object"
-                )
-                .distinct()
-                .count()
-            )
+        queries = self._get_query_sets_for_triple((None, None, None), context)
+        return sum(
+            qs.values("subject", "predicate", "object").distinct().count()
+            for qs in queries
+        )
 
     ####################
     # CONTEXT MANAGEMENT

rdflib_django/utils.py

@@ -8,15 +8,16 @@
 from .store import DEFAULT_STORE, DjangoStore
 
 
-def get_conjunctive_graph(store_id=None):
+def get_conjunctive_graph(store_id=DEFAULT_STORE, identifier=None):
     """
     Returns an open conjunctive graph.
-    """
-    if not store_id:
-        store_id = DEFAULT_STORE
 
-    store = DjangoStore(DEFAULT_STORE)
-    graph = ConjunctiveGraph(store=store, identifier=store_id)
+    The returned graph reads triples from all graphs in the store.
+    Write operations happen on the graph specified by the identifier parameter
+    or a graph identified by a blank node if the identifier is not provided.
+    """
+    store = DjangoStore(identifier=store_id)
+    graph = ConjunctiveGraph(store=store, identifier=identifier)
     if graph.open(None) != VALID_STORE:
         raise ValueError(
             "The store identified by {} is not a valid store".format(store_id)
@@ -31,7 +32,7 @@ def get_named_graph(identifier, store_id=DEFAULT_STORE, create=True):
     if not isinstance(identifier, URIRef):
         identifier = URIRef(identifier)
 
-    store = DjangoStore(store_id)
+    store = DjangoStore(identifier=store_id)
     graph = Graph(store, identifier=identifier)
     if graph.open(None, create=create) != VALID_STORE:
         raise ValueError(

test/test_utils.py

@@ -0,0 +1,48 @@
+"""
+Unit tests for the utils module.
+"""
+from django import test
+from rdflib import Namespace
+
+from rdflib_django.utils import get_named_graph, get_conjunctive_graph
+
+EX = Namespace("https://www.example.com/")
+
+
+class GraphTest(test.TestCase):
+    """
+    Checks on the utils module.
+    """
+
+    def test_conjunctive_and_named_graphs(self):
+        g1 = get_named_graph(EX.g1)
+        g2 = get_conjunctive_graph()
+        g1_s2 = get_named_graph(EX.g1, "s2")
+        g3 = get_conjunctive_graph("s2")
+        g4 = get_named_graph(EX.g4, store_id="s3")
+        g5 = get_conjunctive_graph("s3", identifier=EX.g4)
+
+        g1.add((EX.a, EX.a, EX.a))
+        g2.add((EX.b, EX.b, EX.b))
+        g1_s2.add((EX.c, EX.c, EX.c))
+        g3.add((EX.d, EX.d, EX.d))
+        g4.add((EX.x, EX.x, EX.x))
+        g5.add((EX.y, EX.y, EX.y))
+
+        self.assertEqual(len(g1), 1)
+        self.assertEqual(len(g1_s2), 1)
+        self.assertEqual(len(g2), 2)
+        self.assertEqual(len(g3), 2)
+        self.assertEqual(len(g4), 2)
+
+        self.assertEqual(set(g1), {(EX.a, EX.a, EX.a)})
+        self.assertEqual(set(g2), {(EX.a, EX.a, EX.a), (EX.b, EX.b, EX.b)})
+        self.assertEqual(set(g1_s2), {(EX.c, EX.c, EX.c)})
+        self.assertEqual(set(g3), {(EX.c, EX.c, EX.c), (EX.d, EX.d, EX.d)})
+        self.assertEqual(set(g4), set(g5))
+
+        g2.remove((None, None, None))
+
+        self.assertEqual(list(g1), [])
+        self.assertEqual(list(g2), [])
+        self.assertEqual(list(g1_s2), [(EX.c, EX.c, EX.c)])