Skip to content

Commit

Permalink
662 Strange behaviour of CUDS remove (DataspaceSession) (#668)
Browse files Browse the repository at this point in the history
Allow the CUDS objects to be initialized with extra triples, instead of having to add them after already having created the object.

This solves #668 because previously, the class of the `Wrapper` CUDS objects was not defined on the server until after having spawned the CUDS object, because it is created empty and then filled with triples. This caused it to go to the added buffer, ignoring the changes introduced in #638, which solved the issue for local sessions, for which the CUDS objects are not created first empty and then filled with triples.
  • Loading branch information
kysrpex authored Jul 15, 2021
1 parent ea9ce17 commit 6eb231e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 10 deletions.
24 changes: 18 additions & 6 deletions osp/core/cuds.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

import logging
from uuid import uuid4, UUID
from typing import Union, List, Iterator, Dict, Any, Optional, Tuple
from rdflib import URIRef, RDF, Graph, Literal
from typing import Union, List, Iterator, Dict, Any, Optional, Tuple, Iterable
from rdflib import URIRef, BNode, RDF, Graph, Literal
from osp.core.namespaces import cuba, from_iri
from osp.core.ontology.relationship import OntologyRelationship
from osp.core.ontology.attribute import OntologyAttribute
Expand Down Expand Up @@ -41,9 +41,11 @@ def __init__(self,
session: Session = None,
iri: URIRef = None,
uid: Union[UUID, URIRef] = None,
# Create an empty CUDS and add the triples externally
# afterwards.
_from_triples: bool = False):
# Specify extra triples for the CUDS object.
extra_triples: Iterable[
Tuple[Union[URIRef, BNode],
Union[URIRef, BNode],
Union[URIRef, BNode]]] = tuple()):
"""Initialize a CUDS object."""
# Set uid. This is a "user-facing" method, so strict types
# checks are performed.
Expand Down Expand Up @@ -71,7 +73,17 @@ def __init__(self,
self._graph.add((
self.iri, RDF.type, oclass.iri
))
elif not _from_triples:
extra_oclass = False
for s, p, o in extra_triples:
if s != self.iri:
raise ValueError("Trying to add extra triples to a CUDS "
"object with a subject that does not match "
"the CUDS object's IRI.")
elif p == RDF.type:
extra_oclass = True
self._graph.add((s, p, o))
oclass_assigned = bool(oclass) or extra_oclass
if not oclass_assigned:
raise TypeError(f"No oclass associated with {self}! "
f"Did you install the required ontology?")

Expand Down
6 changes: 4 additions & 2 deletions osp/core/utils/wrapper_development.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,17 @@ def create_from_triples(triples, neighbor_triples, session,
for rel in rels:
cuds_object.remove(rel=rel)
session.graph.remove((cuds_object.iri, None, None))
for triple in set(triples):
session.graph.add(triple)
else: # create new
cuds_object = Cuds(attributes={},
oclass=None,
session=session,
uid=uid,
_from_triples=True)
extra_triples=set(triples))

# add the triples
for triple in set(triples) | set(neighbor_triples):
for triple in set(neighbor_triples):
session.graph.add(triple)
if isinstance(session, WrapperSession):
session._store_checks(cuds_object)
Expand Down
3 changes: 1 addition & 2 deletions tests/test_api_city.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,10 @@ def test_add_multi_session(self):
def test_cuds_without_oclass(self):
"""Tries to create a cuds without oclass.
Should fail except if the argument _from_triples is set to True.
Should always fail.
"""
self.assertRaises(TypeError,
Cuds, oclass=None, attributes={})
self.assertTrue(Cuds(oclass=None, attributes={}, _from_triples=True))


if __name__ == '__main__':
Expand Down

2 comments on commit 6eb231e

@kysrpex
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 6eb231e Previous: ea9ce17 Ratio
benchmark_cuds_api.py::benchmark_cuds_iter_byuiduuid 305.637595851815 iter/sec (stddev: 0.07126502080951806) 479.3349031116214 iter/sec (stddev: 0.045131895237755945) 1.57

This comment was automatically generated by workflow using github-action-benchmark.

CC: @yoavnash @pablo-de-andres @kysrpex

@kysrpex
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning Performance Alert warning

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
Benchmark suite Current: 6eb231e Previous: ea9ce17 Ratio
benchmark_cuds_api.py::benchmark_cuds_iter_byuiduuid 305.637595851815 iter/sec (stddev: 0.07126502080951806) 479.3349031116214 iter/sec (stddev: 0.045131895237755945) 1.57

This comment was automatically generated by workflow using github-action-benchmark.

CC: @yoavnash @pablo-de-andres @kysrpex

No part of the program related to the benchmark changed, so this might be a false positive (a unit test was also running, could have overlapped). We should however watch the next merge to dev.

Please sign in to comment.