Skip to content

Commit fbf6bea

Browse files
hmottestadhmottestad
authored
GH-5221 fix NotifyingSail bug (#5224)
2 parents 0f42c6c + 557a5f6 commit fbf6bea

7 files changed

Lines changed: 177 additions & 22 deletions

File tree

‎core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/evaluationsteps/RegexValueEvaluationStepSupplier.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ public class RegexValueEvaluationStepSupplier {
3838
private static final class ChangingRegexQueryValueEvaluationStep implements QueryValueEvaluationStep {
3939
private final Regex node;
4040
private final EvaluationStrategy strategy;
41+
private Value parg;
42+
private Value farg;
43+
private Pattern pattern;
4144

4245
private ChangingRegexQueryValueEvaluationStep(Regex node, EvaluationStrategy strategy) {
4346
this.node = node;
@@ -56,16 +59,33 @@ public Value evaluate(BindingSet bindings) throws QueryEvaluationException {
5659

5760
if (QueryEvaluationUtility.isStringLiteral(arg) && QueryEvaluationUtility.isSimpleLiteral(parg)
5861
&& (farg == null || QueryEvaluationUtility.isSimpleLiteral(farg))) {
62+
63+
Pattern pattern = getPattern((Literal) parg, farg);
64+
5965
String text = ((Literal) arg).getLabel();
60-
String ptn = ((Literal) parg).getLabel();
61-
// TODO should this Pattern be cached?
62-
int f = extractRegexFlags(farg);
63-
Pattern pattern = Pattern.compile(ptn, f);
6466
boolean result = pattern.matcher(text).find();
6567
return BooleanLiteral.valueOf(result);
6668
}
6769
throw new ValueExprEvaluationException();
6870
}
71+
72+
private Pattern getPattern(Literal parg, Value farg) {
73+
if (this.parg == parg && this.farg == farg) {
74+
return pattern;
75+
}
76+
77+
String ptn = parg.getLabel();
78+
int f = extractRegexFlags(farg);
79+
Pattern pattern = Pattern.compile(ptn, f);
80+
81+
// cache the pattern object and the current parg and farg so that we can reuse it if the parg and farg are
82+
// reused or somehow constant
83+
this.parg = parg;
84+
this.farg = farg;
85+
this.pattern = pattern;
86+
87+
return pattern;
88+
}
6989
}
7090

7191
public static QueryValueEvaluationStep make(EvaluationStrategy strategy, Regex node,

‎core/repository/sail/src/main/java/org/eclipse/rdf4j/repository/sail/helpers/SailUpdateExecutor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,8 @@ protected void executeModify(Modify modify, UpdateContext uc, int maxExecutionTi
447447
whereClause, uc, maxExecutionTime)) {
448448
while (sourceBindings.hasNext()) {
449449
BindingSet sourceBinding = sourceBindings.next();
450-
deleteBoundTriples(sourceBinding, modify.getDeleteExpr(), uc);
451450

451+
deleteBoundTriples(sourceBinding, modify.getDeleteExpr(), uc);
452452
insertBoundTriples(sourceBinding, modify.getInsertExpr(), uc);
453453
}
454454
}

‎core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/Changeset.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.stream.Collectors;
3030
import java.util.stream.Stream;
3131

32+
import org.eclipse.rdf4j.common.annotation.Experimental;
3233
import org.eclipse.rdf4j.common.annotation.InternalUseOnly;
3334
import org.eclipse.rdf4j.common.transaction.IsolationLevels;
3435
import org.eclipse.rdf4j.model.IRI;
@@ -175,7 +176,8 @@ boolean hasApproved(Resource subj, IRI pred, Value obj, Resource[] contexts) {
175176
}
176177
}
177178

178-
boolean hasDeprecated(Resource subj, IRI pred, Value obj, Resource[] contexts) {
179+
@Experimental
180+
public boolean hasDeprecated(Resource subj, IRI pred, Value obj, Resource[] contexts) {
179181
assert !closed;
180182
if ((deprecated == null || deprecatedEmpty) && deprecatedContexts == null) {
181183
return false;

‎core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceConnection.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -763,8 +763,13 @@ private void add(Resource subj, IRI pred, Value obj, SailDataset dataset, SailSi
763763
if (hasConnectionListeners()) {
764764
if (!hasStatement(dataset, subj, pred, obj, NULL_CTX)) {
765765
notifyStatementAdded(vf.createStatement(subj, pred, obj));
766-
sink.approve(subj, pred, obj, null);
766+
} else if (sink instanceof Changeset && ((Changeset) sink).hasDeprecated(subj, pred, obj, NULL_CTX)) {
767+
notifyStatementAdded(vf.createStatement(subj, pred, obj));
767768
}
769+
770+
// always approve the statement, even if it already exists
771+
sink.approve(subj, pred, obj, null);
772+
768773
} else {
769774
sink.approve(subj, pred, obj, null);
770775
}
@@ -784,8 +789,11 @@ private void add(Resource subj, IRI pred, Value obj, SailDataset dataset, SailSi
784789
if (hasConnectionListeners()) {
785790
if (!hasStatement(dataset, subj, pred, obj, contextsToCheck)) {
786791
notifyStatementAdded(vf.createStatement(subj, pred, obj, ctx));
787-
sink.approve(subj, pred, obj, ctx);
792+
} else if (sink instanceof Changeset
793+
&& ((Changeset) sink).hasDeprecated(subj, pred, obj, contextsToCheck)) {
794+
notifyStatementAdded(vf.createStatement(subj, pred, obj));
788795
}
796+
sink.approve(subj, pred, obj, ctx);
789797
} else {
790798
sink.approve(subj, pred, obj, ctx);
791799
}
@@ -830,7 +838,6 @@ private boolean remove(Resource subj, IRI pred, Value obj, SailDataset dataset,
830838
while (iter.hasNext()) {
831839
Statement st = iter.next();
832840
sink.deprecate(st);
833-
834841
statementsRemoved = true;
835842
notifyStatementRemoved(st);
836843
}

‎core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/ReadCommittedWrapper.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,18 @@ class ReadCommittedWrapper implements DataStructureInterface {
5050

5151
@Override
5252
public void addStatement(ExtensibleStatement statement) {
53-
internalAdded.put(statement, statement);
54-
internalRemoved.remove(statement);
55-
53+
ExtensibleStatement put = internalAdded.put(statement, statement);
54+
if (put == null) {
55+
internalRemoved.remove(statement);
56+
}
5657
}
5758

5859
@Override
5960
public void removeStatement(ExtensibleStatement statement) {
60-
internalRemoved.put(statement, statement);
61+
ExtensibleStatement put = internalRemoved.put(statement, statement);
62+
if (put == null) {
63+
internalAdded.remove(statement);
64+
}
6165

6266
}
6367

‎core/sail/extensible-store/src/main/java/org/eclipse/rdf4j/sail/extensiblestore/valuefactory/ExtensibleStatementImpl.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import org.eclipse.rdf4j.model.IRI;
1616
import org.eclipse.rdf4j.model.Resource;
17+
import org.eclipse.rdf4j.model.Statement;
1718
import org.eclipse.rdf4j.model.Value;
1819
import org.eclipse.rdf4j.model.impl.GenericStatement;
1920

@@ -45,19 +46,17 @@ public boolean equals(Object o) {
4546
if (this == o) {
4647
return true;
4748
}
48-
if (!(o instanceof ExtensibleStatementImpl)) {
49+
if (!(o instanceof Statement)) {
4950
return false;
5051
}
52+
if (!(o instanceof ExtensibleStatement)) {
53+
return super.equals(o);
54+
}
5155
if (!super.equals(o)) {
5256
return false;
5357
}
54-
ExtensibleStatementImpl that = (ExtensibleStatementImpl) o;
55-
return inferred == that.inferred;
56-
}
57-
58-
@Override
59-
public int hashCode() {
60-
return Objects.hash(super.hashCode(), inferred);
58+
ExtensibleStatement that = (ExtensibleStatement) o;
59+
return inferred == that.isInferred();
6160
}
6261

6362
}

‎testsuites/sail/src/main/java/org/eclipse/rdf4j/testsuite/sail/RDFNotifyingStoreTest.java‎

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,21 @@
1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.junit.jupiter.api.Assertions.assertEquals;
1515

16+
import java.util.ArrayList;
17+
import java.util.HashSet;
18+
import java.util.List;
19+
import java.util.Set;
20+
21+
import org.eclipse.rdf4j.model.Statement;
1622
import org.eclipse.rdf4j.model.vocabulary.RDF;
1723
import org.eclipse.rdf4j.model.vocabulary.RDFS;
24+
import org.eclipse.rdf4j.repository.sail.SailRepository;
25+
import org.eclipse.rdf4j.repository.sail.SailRepositoryConnection;
1826
import org.eclipse.rdf4j.sail.NotifyingSail;
27+
import org.eclipse.rdf4j.sail.NotifyingSailConnection;
1928
import org.eclipse.rdf4j.sail.SailChangedEvent;
2029
import org.eclipse.rdf4j.sail.SailChangedListener;
30+
import org.eclipse.rdf4j.sail.SailConnectionListener;
2131
import org.eclipse.rdf4j.sail.SailException;
2232
import org.junit.jupiter.api.BeforeEach;
2333
import org.junit.jupiter.api.Test;
@@ -36,6 +46,7 @@ public abstract class RDFNotifyingStoreTest extends RDFStoreTest implements Sail
3646
private int removeEventCount;
3747

3848
private int addEventCount;
49+
private SailRepository repo;
3950

4051
/*---------*
4152
* Methods *
@@ -54,7 +65,9 @@ public abstract class RDFNotifyingStoreTest extends RDFStoreTest implements Sail
5465
public void addSailChangedListener() {
5566
// set self as listener
5667
((NotifyingSail) sail).addSailChangedListener(this);
57-
68+
removeEventCount = 0;
69+
addEventCount = 0;
70+
this.repo = new SailRepository(sail);
5871
}
5972

6073
@Test
@@ -99,6 +112,116 @@ public void testNotifyingRemoveAndClear() {
99112
assertEquals(3, removeEventCount, "There should have been 3 events in which statements were removed");
100113
}
101114

115+
@Test
116+
public void testUpdateQuery() {
117+
118+
try (SailRepositoryConnection connection = repo.getConnection()) {
119+
connection.begin();
120+
connection.add(painter, RDF.TYPE, RDFS.CLASS);
121+
connection.add(painting, RDF.TYPE, RDFS.CLASS);
122+
connection.add(picasso, RDF.TYPE, painter);
123+
connection.add(guernica, RDF.TYPE, painting);
124+
connection.add(picasso, paints, guernica);
125+
connection.commit();
126+
127+
}
128+
129+
try (SailRepositoryConnection connection = repo.getConnection()) {
130+
Set<Statement> added = new HashSet<>();
131+
Set<Statement> removed = new HashSet<>();
132+
133+
List<Statement> addedRaw = new ArrayList<>();
134+
List<Statement> removedRaw = new ArrayList<>();
135+
136+
registerConnectionListener(connection, added, removed, addedRaw, removedRaw);
137+
138+
connection.prepareUpdate("" +
139+
"DELETE {?a ?b ?c}" +
140+
"INSERT {?a ?b ?c}" +
141+
"WHERE {?a ?b ?c}").execute();
142+
143+
assertEquals(5, added.size());
144+
assertEquals(5, removed.size());
145+
assertEquals(5, addedRaw.size());
146+
assertEquals(5, removedRaw.size());
147+
148+
assertEquals(added, removed);
149+
150+
}
151+
152+
assertEquals(5, con.size());
153+
154+
}
155+
156+
@Test
157+
public void testUpdateQuery2() {
158+
159+
try (SailRepositoryConnection connection = repo.getConnection()) {
160+
connection.begin();
161+
connection.add(painter, RDF.TYPE, RDFS.CLASS);
162+
connection.add(painting, RDF.TYPE, RDFS.CLASS);
163+
connection.commit();
164+
165+
}
166+
167+
try (SailRepositoryConnection connection = repo.getConnection()) {
168+
Set<Statement> added = new HashSet<>();
169+
Set<Statement> removed = new HashSet<>();
170+
171+
List<Statement> addedRaw = new ArrayList<>();
172+
List<Statement> removedRaw = new ArrayList<>();
173+
174+
registerConnectionListener(connection, added, removed, addedRaw, removedRaw);
175+
176+
String statement = "<" + painter + "> <" + RDF.TYPE + "> <" + RDFS.CLASS + "> .";
177+
178+
connection.prepareUpdate("" +
179+
"DELETE {" + statement + "}" +
180+
"INSERT {" + statement + "}" +
181+
"WHERE {?a ?b ?c}").execute();
182+
183+
assertEquals(added, removed, "Added (expected) is not the same as removed (actual)");
184+
185+
assertEquals(2, addedRaw.size());
186+
assertEquals(2, removedRaw.size());
187+
188+
assertEquals(1, added.size());
189+
assertEquals(1, removed.size());
190+
191+
}
192+
193+
assertEquals(2, con.size());
194+
195+
}
196+
197+
private static void registerConnectionListener(SailRepositoryConnection connection, Set<Statement> added,
198+
Set<Statement> removed, List<Statement> addedRaw, List<Statement> removedRaw) {
199+
((NotifyingSailConnection) connection.getSailConnection())
200+
.addConnectionListener(
201+
new SailConnectionListener() {
202+
@Override
203+
public void statementAdded(Statement st) {
204+
boolean add = added.add(st);
205+
if (!add) {
206+
removed.remove(st);
207+
}
208+
209+
addedRaw.add(st);
210+
}
211+
212+
@Override
213+
public void statementRemoved(Statement st) {
214+
boolean add = removed.add(st);
215+
if (!add) {
216+
added.remove(st);
217+
}
218+
219+
removedRaw.add(st);
220+
}
221+
}
222+
);
223+
}
224+
102225
@Override
103226
public void sailChanged(SailChangedEvent event) {
104227
if (event.statementsAdded()) {

0 commit comments

Comments
 (0)