From 7a1a8f5880e18dc11de35b1177b7a4586e625606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 28 Dec 2024 08:35:47 +0100 Subject: [PATCH 1/5] GH-5221 add test --- .../testsuite/sail/RDFNotifyingStoreTest.java | 163 +++++++++++++++++- 1 file changed, 162 insertions(+), 1 deletion(-) diff --git a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java index 9f15fd3d9d0..f6fa783670e 100644 --- a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java +++ b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java @@ -13,11 +13,21 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.rdf4j.model.Statement; import org.eclipse.rdf4j.model.vocabulary.RDF; import org.eclipse.rdf4j.model.vocabulary.RDFS; +import org.eclipse.rdf4j.repository.sail.SailRepository; +import org.eclipse.rdf4j.repository.sail.SailRepositoryConnection; import org.eclipse.rdf4j.sail.NotifyingSail; +import org.eclipse.rdf4j.sail.NotifyingSailConnection; import org.eclipse.rdf4j.sail.SailChangedEvent; import org.eclipse.rdf4j.sail.SailChangedListener; +import org.eclipse.rdf4j.sail.SailConnectionListener; import org.eclipse.rdf4j.sail.SailException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -36,6 +46,7 @@ public abstract class RDFNotifyingStoreTest extends RDFStoreTest implements Sail private int removeEventCount; private int addEventCount; + private SailRepository repo; /*---------* * Methods * @@ -54,7 +65,9 @@ public abstract class RDFNotifyingStoreTest extends RDFStoreTest implements Sail public void addSailChangedListener() { // set self as listener ((NotifyingSail) sail).addSailChangedListener(this); - + removeEventCount = 0; + addEventCount = 0; + this.repo = new SailRepository(sail); } @Test @@ -99,6 +112,154 @@ public void testNotifyingRemoveAndClear() { assertEquals(3, removeEventCount, "There should have been 3 events in which statements were removed"); } + @Test + public void testUpdateQuery() { + + try (SailRepositoryConnection connection = repo.getConnection()) { + connection.begin(); + connection.add(painter, RDF.TYPE, RDFS.CLASS); + connection.add(painting, RDF.TYPE, RDFS.CLASS); + connection.add(picasso, RDF.TYPE, painter); + connection.add(guernica, RDF.TYPE, painting); + connection.add(picasso, paints, guernica); + connection.commit(); + + } + + addEventCount = 0; + removeEventCount = 0; + + try (SailRepositoryConnection connection = repo.getConnection()) { + Set added = new HashSet<>(); + Set removed = new HashSet<>(); + + List addedRaw = new ArrayList<>(); + List removedRaw = new ArrayList<>(); + + ((NotifyingSailConnection) connection.getSailConnection()) + .addConnectionListener(new SailConnectionListener() { + @Override + public void statementAdded(Statement st) { + boolean add = added.add(st); + if (!add) { + removed.remove(st); + } + + addedRaw.add(st); + } + + @Override + public void statementRemoved(Statement st) { + boolean add = removed.add(st); + if (!add) { + added.remove(st); + } + + removedRaw.add(st); + } + } + ); + + connection.prepareUpdate("" + + "DELETE {?a ?b ?c}" + + "INSERT {?a ?b ?c}" + + "WHERE {?a ?b ?c}").execute(); + + System.out.println("Added Raw Size: " + addedRaw.size()); + System.out.println("Removed Raw Size: " + removedRaw.size()); + System.out.println("Added Raw: " + addedRaw); + System.out.println("Removed Raw: " + removedRaw); + System.out.println("Added Size: " + added.size()); + System.out.println("Removed Size: " + removed.size()); + System.out.println("Added: " + added); + System.out.println("Removed: " + removed); + + assertEquals(5, added.size()); + assertEquals(5, removed.size()); + assertEquals(5, addedRaw.size()); + assertEquals(5, removedRaw.size()); + + assertEquals(added, removed); + + } + + assertEquals(5, con.size()); + + } + + @Test + public void testUpdateQuery2() { + + try (SailRepositoryConnection connection = repo.getConnection()) { + connection.begin(); + connection.add(painter, RDF.TYPE, RDFS.CLASS); + connection.commit(); + + } + + String statement = "<" + painter + "> <" + RDF.TYPE + "> <" + RDFS.CLASS + "> ."; + + addEventCount = 0; + removeEventCount = 0; + + try (SailRepositoryConnection connection = repo.getConnection()) { + Set added = new HashSet<>(); + Set removed = new HashSet<>(); + + List addedRaw = new ArrayList<>(); + List removedRaw = new ArrayList<>(); + + ((NotifyingSailConnection) connection.getSailConnection()) + .addConnectionListener(new SailConnectionListener() { + @Override + public void statementAdded(Statement st) { + boolean add = added.add(st); + if (!add) { + removed.remove(st); + } + + addedRaw.add(st); + } + + @Override + public void statementRemoved(Statement st) { + boolean add = removed.add(st); + if (!add) { + added.remove(st); + } + + removedRaw.add(st); + } + } + ); + + connection.prepareUpdate("" + + "DELETE {" + statement + "}" + + "INSERT {" + statement + "}" + + "WHERE {?a ?b ?c}").execute(); + + System.out.println("Added Raw Size: " + addedRaw.size()); + System.out.println("Removed Raw Size: " + removedRaw.size()); + System.out.println("Added Raw: " + addedRaw); + System.out.println("Removed Raw: " + removedRaw); + System.out.println("Added Size: " + added.size()); + System.out.println("Removed Size: " + removed.size()); + System.out.println("Added: " + added); + System.out.println("Removed: " + removed); + + assertEquals(1, added.size()); + assertEquals(1, removed.size()); + assertEquals(1, addedRaw.size()); + assertEquals(1, removedRaw.size()); + + assertEquals(added, removed); + + } + + assertEquals(1, con.size()); + + } + @Override public void sailChanged(SailChangedEvent event) { if (event.statementsAdded()) { From 28edab6167eb781b9b88267d9be32db4b9f31a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 28 Dec 2024 08:42:59 +0100 Subject: [PATCH 2/5] GH-5221 fix bug where added statements from a SPARQL update were not notified and were also not added. This only happens when the exact statement was removed by the DELETE clause. --- .../sail/helpers/SailUpdateExecutor.java | 2 +- .../org/eclipse/rdf4j/sail/base/Changeset.java | 4 +++- .../rdf4j/sail/base/SailSourceConnection.java | 13 ++++++++++--- .../valuefactory/ExtensibleStatementImpl.java | 15 +++++++-------- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/repository/sail/src/main/java/org/eclipse/rdf4j/repository/sail/helpers/SailUpdateExecutor.java b/core/repository/sail/src/main/java/org/eclipse/rdf4j/repository/sail/helpers/SailUpdateExecutor.java index 06fcf840f25..edd0f0a812c 100644 --- a/core/repository/sail/src/main/java/org/eclipse/rdf4j/repository/sail/helpers/SailUpdateExecutor.java +++ b/core/repository/sail/src/main/java/org/eclipse/rdf4j/repository/sail/helpers/SailUpdateExecutor.java @@ -447,8 +447,8 @@ protected void executeModify(Modify modify, UpdateContext uc, int maxExecutionTi whereClause, uc, maxExecutionTime)) { while (sourceBindings.hasNext()) { BindingSet sourceBinding = sourceBindings.next(); - deleteBoundTriples(sourceBinding, modify.getDeleteExpr(), uc); + deleteBoundTriples(sourceBinding, modify.getDeleteExpr(), uc); insertBoundTriples(sourceBinding, modify.getInsertExpr(), uc); } } diff --git a/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/Changeset.java b/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/Changeset.java index 9dbeeeaf5ca..2283a3e1c96 100644 --- a/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/Changeset.java +++ b/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/Changeset.java @@ -29,6 +29,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.eclipse.rdf4j.common.annotation.Experimental; import org.eclipse.rdf4j.common.annotation.InternalUseOnly; import org.eclipse.rdf4j.common.transaction.IsolationLevels; import org.eclipse.rdf4j.model.IRI; @@ -175,7 +176,8 @@ boolean hasApproved(Resource subj, IRI pred, Value obj, Resource[] contexts) { } } - boolean hasDeprecated(Resource subj, IRI pred, Value obj, Resource[] contexts) { + @Experimental + public boolean hasDeprecated(Resource subj, IRI pred, Value obj, Resource[] contexts) { assert !closed; if ((deprecated == null || deprecatedEmpty) && deprecatedContexts == null) { return false; diff --git a/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceConnection.java b/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceConnection.java index 7020c983396..7942984593a 100644 --- a/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceConnection.java +++ b/core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceConnection.java @@ -763,8 +763,13 @@ private void add(Resource subj, IRI pred, Value obj, SailDataset dataset, SailSi if (hasConnectionListeners()) { if (!hasStatement(dataset, subj, pred, obj, NULL_CTX)) { notifyStatementAdded(vf.createStatement(subj, pred, obj)); - sink.approve(subj, pred, obj, null); + } else if (sink instanceof Changeset && ((Changeset) sink).hasDeprecated(subj, pred, obj, NULL_CTX)) { + notifyStatementAdded(vf.createStatement(subj, pred, obj)); } + + // always approve the statement, even if it already exists + sink.approve(subj, pred, obj, null); + } else { sink.approve(subj, pred, obj, null); } @@ -784,8 +789,11 @@ private void add(Resource subj, IRI pred, Value obj, SailDataset dataset, SailSi if (hasConnectionListeners()) { if (!hasStatement(dataset, subj, pred, obj, contextsToCheck)) { notifyStatementAdded(vf.createStatement(subj, pred, obj, ctx)); - sink.approve(subj, pred, obj, ctx); + } else if (sink instanceof Changeset + && ((Changeset) sink).hasDeprecated(subj, pred, obj, contextsToCheck)) { + notifyStatementAdded(vf.createStatement(subj, pred, obj)); } + sink.approve(subj, pred, obj, ctx); } else { sink.approve(subj, pred, obj, ctx); } @@ -830,7 +838,6 @@ private boolean remove(Resource subj, IRI pred, Value obj, SailDataset dataset, while (iter.hasNext()) { Statement st = iter.next(); sink.deprecate(st); - statementsRemoved = true; notifyStatementRemoved(st); } diff --git a/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/valuefactory/ExtensibleStatementImpl.java b/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/valuefactory/ExtensibleStatementImpl.java index b3f8ac04eb7..1e564ac837f 100644 --- a/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/valuefactory/ExtensibleStatementImpl.java +++ b/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/valuefactory/ExtensibleStatementImpl.java @@ -14,6 +14,7 @@ import org.eclipse.rdf4j.model.IRI; import org.eclipse.rdf4j.model.Resource; +import org.eclipse.rdf4j.model.Statement; import org.eclipse.rdf4j.model.Value; import org.eclipse.rdf4j.model.impl.GenericStatement; @@ -45,19 +46,17 @@ public boolean equals(Object o) { if (this == o) { return true; } - if (!(o instanceof ExtensibleStatementImpl)) { + if (!(o instanceof Statement)) { return false; } + if (!(o instanceof ExtensibleStatement)) { + return super.equals(o); + } if (!super.equals(o)) { return false; } - ExtensibleStatementImpl that = (ExtensibleStatementImpl) o; - return inferred == that.inferred; - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), inferred); + ExtensibleStatement that = (ExtensibleStatement) o; + return inferred == that.isInferred(); } } From f5aeee30865099b662139d53257e6ea0b1f3444a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 28 Dec 2024 08:43:26 +0100 Subject: [PATCH 3/5] add a small cache to the regex implementation --- .../RegexValueEvaluationStepSupplier.java | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/RegexValueEvaluationStepSupplier.java b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/RegexValueEvaluationStepSupplier.java index 5fead2eab96..0d2fa5c7b52 100644 --- a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/RegexValueEvaluationStepSupplier.java +++ b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/RegexValueEvaluationStepSupplier.java @@ -38,6 +38,9 @@ public class RegexValueEvaluationStepSupplier { private static final class ChangingRegexQueryValueEvaluationStep implements QueryValueEvaluationStep { private final Regex node; private final EvaluationStrategy strategy; + private Value parg; + private Value farg; + private Pattern pattern; private ChangingRegexQueryValueEvaluationStep(Regex node, EvaluationStrategy strategy) { this.node = node; @@ -56,16 +59,33 @@ public Value evaluate(BindingSet bindings) throws QueryEvaluationException { if (QueryEvaluationUtility.isStringLiteral(arg) && QueryEvaluationUtility.isSimpleLiteral(parg) && (farg == null || QueryEvaluationUtility.isSimpleLiteral(farg))) { + + Pattern pattern = getPattern((Literal) parg, farg); + String text = ((Literal) arg).getLabel(); - String ptn = ((Literal) parg).getLabel(); - // TODO should this Pattern be cached? - int f = extractRegexFlags(farg); - Pattern pattern = Pattern.compile(ptn, f); boolean result = pattern.matcher(text).find(); return BooleanLiteral.valueOf(result); } throw new ValueExprEvaluationException(); } + + private Pattern getPattern(Literal parg, Value farg) { + if (this.parg == parg && this.farg == farg) { + return pattern; + } + + String ptn = parg.getLabel(); + int f = extractRegexFlags(farg); + Pattern pattern = Pattern.compile(ptn, f); + + // cache the pattern object and the current parg and farg so that we can reuse it if the parg and farg are + // reused or somehow constant + this.parg = parg; + this.farg = farg; + this.pattern = pattern; + + return pattern; + } } public static QueryValueEvaluationStep make(EvaluationStrategy strategy, Regex node, From b66f5e9c6d3491326892622572a54ae6afd744ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 28 Dec 2024 11:11:17 +0100 Subject: [PATCH 4/5] code cleanup --- .../testsuite/sail/RDFNotifyingStoreTest.java | 109 ++++++------------ 1 file changed, 35 insertions(+), 74 deletions(-) diff --git a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java index f6fa783670e..85cafbe4c4b 100644 --- a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java +++ b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java @@ -126,9 +126,6 @@ public void testUpdateQuery() { } - addEventCount = 0; - removeEventCount = 0; - try (SailRepositoryConnection connection = repo.getConnection()) { Set added = new HashSet<>(); Set removed = new HashSet<>(); @@ -136,44 +133,13 @@ public void testUpdateQuery() { List addedRaw = new ArrayList<>(); List removedRaw = new ArrayList<>(); - ((NotifyingSailConnection) connection.getSailConnection()) - .addConnectionListener(new SailConnectionListener() { - @Override - public void statementAdded(Statement st) { - boolean add = added.add(st); - if (!add) { - removed.remove(st); - } - - addedRaw.add(st); - } - - @Override - public void statementRemoved(Statement st) { - boolean add = removed.add(st); - if (!add) { - added.remove(st); - } - - removedRaw.add(st); - } - } - ); + registerConnectionListener(connection, added, removed, addedRaw, removedRaw); connection.prepareUpdate("" + "DELETE {?a ?b ?c}" + "INSERT {?a ?b ?c}" + "WHERE {?a ?b ?c}").execute(); - System.out.println("Added Raw Size: " + addedRaw.size()); - System.out.println("Removed Raw Size: " + removedRaw.size()); - System.out.println("Added Raw: " + addedRaw); - System.out.println("Removed Raw: " + removedRaw); - System.out.println("Added Size: " + added.size()); - System.out.println("Removed Size: " + removed.size()); - System.out.println("Added: " + added); - System.out.println("Removed: " + removed); - assertEquals(5, added.size()); assertEquals(5, removed.size()); assertEquals(5, addedRaw.size()); @@ -193,15 +159,11 @@ public void testUpdateQuery2() { try (SailRepositoryConnection connection = repo.getConnection()) { connection.begin(); connection.add(painter, RDF.TYPE, RDFS.CLASS); + connection.add(painting, RDF.TYPE, RDFS.CLASS); connection.commit(); } - String statement = "<" + painter + "> <" + RDF.TYPE + "> <" + RDFS.CLASS + "> ."; - - addEventCount = 0; - removeEventCount = 0; - try (SailRepositoryConnection connection = repo.getConnection()) { Set added = new HashSet<>(); Set removed = new HashSet<>(); @@ -209,57 +171,56 @@ public void testUpdateQuery2() { List addedRaw = new ArrayList<>(); List removedRaw = new ArrayList<>(); - ((NotifyingSailConnection) connection.getSailConnection()) - .addConnectionListener(new SailConnectionListener() { - @Override - public void statementAdded(Statement st) { - boolean add = added.add(st); - if (!add) { - removed.remove(st); - } - - addedRaw.add(st); - } - - @Override - public void statementRemoved(Statement st) { - boolean add = removed.add(st); - if (!add) { - added.remove(st); - } + registerConnectionListener(connection, added, removed, addedRaw, removedRaw); - removedRaw.add(st); - } - } - ); + String statement = "<" + painter + "> <" + RDF.TYPE + "> <" + RDFS.CLASS + "> ."; connection.prepareUpdate("" + "DELETE {" + statement + "}" + "INSERT {" + statement + "}" + "WHERE {?a ?b ?c}").execute(); - System.out.println("Added Raw Size: " + addedRaw.size()); - System.out.println("Removed Raw Size: " + removedRaw.size()); - System.out.println("Added Raw: " + addedRaw); - System.out.println("Removed Raw: " + removedRaw); - System.out.println("Added Size: " + added.size()); - System.out.println("Removed Size: " + removed.size()); - System.out.println("Added: " + added); - System.out.println("Removed: " + removed); - assertEquals(1, added.size()); assertEquals(1, removed.size()); - assertEquals(1, addedRaw.size()); - assertEquals(1, removedRaw.size()); + assertEquals(2, addedRaw.size()); + assertEquals(2, removedRaw.size()); assertEquals(added, removed); } - assertEquals(1, con.size()); + assertEquals(2, con.size()); } + private static void registerConnectionListener(SailRepositoryConnection connection, Set added, + Set removed, List addedRaw, List removedRaw) { + ((NotifyingSailConnection) connection.getSailConnection()) + .addConnectionListener( + new SailConnectionListener() { + @Override + public void statementAdded(Statement st) { + boolean add = added.add(st); + if (!add) { + removed.remove(st); + } + + addedRaw.add(st); + } + + @Override + public void statementRemoved(Statement st) { + boolean add = removed.add(st); + if (!add) { + added.remove(st); + } + + removedRaw.add(st); + } + } + ); + } + @Override public void sailChanged(SailChangedEvent event) { if (event.statementsAdded()) { From 557a5f632626d71b97efe40e53a1573b3df0f537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Sat, 28 Dec 2024 12:48:30 +0100 Subject: [PATCH 5/5] fixed bug in Extensible Store --- .../sail/extensiblestore/ReadCommittedWrapper.java | 12 ++++++++---- .../rdf4j/testsuite/sail/RDFNotifyingStoreTest.java | 7 ++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/ReadCommittedWrapper.java b/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/ReadCommittedWrapper.java index 20ddfd0dc6b..4e211848902 100644 --- a/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/ReadCommittedWrapper.java +++ b/core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/ReadCommittedWrapper.java @@ -50,14 +50,18 @@ class ReadCommittedWrapper implements DataStructureInterface { @Override public void addStatement(ExtensibleStatement statement) { - internalAdded.put(statement, statement); - internalRemoved.remove(statement); - + ExtensibleStatement put = internalAdded.put(statement, statement); + if (put == null) { + internalRemoved.remove(statement); + } } @Override public void removeStatement(ExtensibleStatement statement) { - internalRemoved.put(statement, statement); + ExtensibleStatement put = internalRemoved.put(statement, statement); + if (put == null) { + internalAdded.remove(statement); + } } diff --git a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java index 85cafbe4c4b..53c5265a097 100644 --- a/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java +++ b/testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java @@ -180,12 +180,13 @@ public void testUpdateQuery2() { "INSERT {" + statement + "}" + "WHERE {?a ?b ?c}").execute(); - assertEquals(1, added.size()); - assertEquals(1, removed.size()); + assertEquals(added, removed, "Added (expected) is not the same as removed (actual)"); + assertEquals(2, addedRaw.size()); assertEquals(2, removedRaw.size()); - assertEquals(added, removed); + assertEquals(1, added.size()); + assertEquals(1, removed.size()); }