GH-5221 fix NotifyingSail bug (#5224)

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,

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);
 				}
 			}

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;

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);
 			}

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);
+		}
 
 	}
 

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();
 	}
 
 }

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,116 @@ 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();
+
+		}
+
+		try (SailRepositoryConnection connection = repo.getConnection()) {
+			Set<Statement> added = new HashSet<>();
+			Set<Statement> removed = new HashSet<>();
+
+			List<Statement> addedRaw = new ArrayList<>();
+			List<Statement> removedRaw = new ArrayList<>();
+
+			registerConnectionListener(connection, added, removed, addedRaw, removedRaw);
+
+			connection.prepareUpdate("" +
+					"DELETE {?a ?b ?c}" +
+					"INSERT {?a ?b ?c}" +
+					"WHERE {?a ?b ?c}").execute();
+
+			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.add(painting, RDF.TYPE, RDFS.CLASS);
+			connection.commit();
+
+		}
+
+		try (SailRepositoryConnection connection = repo.getConnection()) {
+			Set<Statement> added = new HashSet<>();
+			Set<Statement> removed = new HashSet<>();
+
+			List<Statement> addedRaw = new ArrayList<>();
+			List<Statement> removedRaw = new ArrayList<>();
+
+			registerConnectionListener(connection, added, removed, addedRaw, removedRaw);
+
+			String statement = "<" + painter + "> <" + RDF.TYPE + "> <" + RDFS.CLASS + "> .";
+
+			connection.prepareUpdate("" +
+					"DELETE {" + statement + "}" +
+					"INSERT {" + statement + "}" +
+					"WHERE {?a ?b ?c}").execute();
+
+			assertEquals(added, removed, "Added (expected) is not the same as removed (actual)");
+
+			assertEquals(2, addedRaw.size());
+			assertEquals(2, removedRaw.size());
+
+			assertEquals(1, added.size());
+			assertEquals(1, removed.size());
+
+		}
+
+		assertEquals(2, con.size());
+
+	}
+
+	private static void registerConnectionListener(SailRepositoryConnection connection, Set<Statement> added,
+			Set<Statement> removed, List<Statement> addedRaw, List<Statement> 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()) {