Skip to content

Commit f5e5909

Browse files
committed
fixes based on review
1 parent 1686245 commit f5e5909

4 files changed

Lines changed: 113 additions & 22 deletions

File tree

core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShaclSailConnection.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,28 +1601,30 @@ private Boolean invokeBooleanStatementMethod(Statement statement, String methodN
16011601
}
16021602

16031603
/**
1604-
* Reject legacy no-flag callbacks only when a shape explicitly disables inferred statements using
1605-
* rsx:includeInferredStatements=false. In that case, inferred-vs-explicit classification is required for correct
1604+
* Reject legacy no-flag callbacks whenever effective includeInferredStatements=false is active for at least one
1605+
* validated shape (globally through ShaclSail#setIncludeInferredStatements(false) and/or via
1606+
* rsx:includeInferredStatements=false). In that case, inferred-vs-explicit classification is required for correct
16061607
* validation and stores must use statementAdded/Removed callbacks with the inferred boolean argument.
16071608
*/
16081609
private void validateLegacyCallbackInferredSupport(List<ContextWithShape> shapesToValidate) {
16091610
if (!legacyStatementAddedWithoutInferredFlagObserved && !legacyStatementRemovedWithoutInferredFlagObserved) {
16101611
return;
16111612
}
16121613

1613-
boolean hasExplicitIncludeInferredDisabled = shapesToValidate.stream()
1614+
boolean includeInferredStatementsEnabledByDefault = sail.isIncludeInferredStatements();
1615+
boolean requiresInferredClassification = shapesToValidate.stream()
16141616
.filter(ContextWithShape::hasShape)
16151617
.map(ContextWithShape::getShape)
1616-
.map(Shape::getIncludeInferredStatementsOverride)
1617-
.anyMatch(Boolean.FALSE::equals);
1618+
.anyMatch(shape -> !shape.usesIncludeInferredStatements(includeInferredStatementsEnabledByDefault));
16181619

1619-
if (!hasExplicitIncludeInferredDisabled) {
1620+
if (!requiresInferredClassification) {
16201621
return;
16211622
}
16221623

16231624
String callbackDetails = getObservedLegacyCallbacksWithoutInferredFlag();
16241625
String message = "Underlying Sail does not support shapes that explicitly set "
1625-
+ "rsx:includeInferredStatements=false because it emits deprecated "
1626+
+ "rsx:includeInferredStatements=false or globally configure "
1627+
+ "ShaclSail#setIncludeInferredStatements(false), because it emits deprecated "
16261628
+ "SailConnectionListener callbacks without inferred flags (" + callbackDetails + "). "
16271629
+ "Implement statementAdded(Statement, boolean inferred) and "
16281630
+ "statementRemoved(Statement, boolean inferred).";

core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ShapeValidationContainer.java

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class ShapeValidationContainer {
3939
private final ConnectionsGroup connectionsGroup;
4040
private final boolean closeConnectionsGroup;
4141
private volatile CloseableIteration<? extends ValidationTuple> iterator;
42+
private volatile boolean connectionsGroupClosed;
4243

4344
public ShapeValidationContainer(Shape shape, Supplier<PlanNode> planNodeSupplier, boolean logValidationExecution,
4445
boolean logValidationViolations, long effectiveValidationResultsLimitPerConstraint,
@@ -113,13 +114,7 @@ public ShapeValidationContainer(Shape shape, Supplier<PlanNode> planNodeSupplier
113114
this.planNode = planNode;
114115
}
115116
} catch (Throwable e) {
116-
if (closeConnectionsGroup) {
117-
try {
118-
connectionsGroup.close();
119-
} catch (Exception closeException) {
120-
logger.debug("Error closing connections group after plan construction failure", closeException);
121-
}
122-
}
117+
closeConnectionsGroup("after plan construction failure");
123118
logger.warn("Error processing SHACL Shape {}", shape.getId(), e);
124119
logger.warn("Error processing SHACL Shape\n{}", shape, e);
125120
if (e instanceof Error) {
@@ -135,7 +130,11 @@ public Shape getShape() {
135130
}
136131

137132
public boolean hasPlanNode() {
138-
return !(planNode.isGuaranteedEmpty());
133+
boolean hasPlanNode = !(planNode.isGuaranteedEmpty());
134+
if (!hasPlanNode) {
135+
closeConnectionsGroup("for guaranteed-empty plan");
136+
}
137+
return hasPlanNode;
139138
}
140139

141140
public ValidationResultIterator performValidation() throws SailException {
@@ -166,17 +165,28 @@ public ValidationResultIterator performValidation() throws SailException {
166165
}
167166
} finally {
168167
this.iterator = null;
169-
if (closeConnectionsGroup) {
170-
try {
171-
connectionsGroup.close();
172-
} catch (Exception closeException) {
173-
logger.debug("Error closing connections group after validation", closeException);
174-
}
175-
}
168+
closeConnectionsGroup("after validation");
176169
}
177170

178171
}
179172

173+
private void closeConnectionsGroup(String context) {
174+
if (!closeConnectionsGroup || connectionsGroupClosed) {
175+
return;
176+
}
177+
synchronized (this) {
178+
if (connectionsGroupClosed) {
179+
return;
180+
}
181+
connectionsGroupClosed = true;
182+
}
183+
try {
184+
connectionsGroup.close();
185+
} catch (Exception closeException) {
186+
logger.debug("Error closing connections group {}", context, closeException);
187+
}
188+
}
189+
180190
private long getTimeStamp() {
181191
if (performanceLogging) {
182192
return System.currentTimeMillis();

core/sail/shacl/src/test/java/org/eclipse/rdf4j/sail/shacl/RdfsReasoningShaclSailTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,34 @@ public void legacyCallbacksWithoutInferenceMetadataFailWhenAShapeDisablesInferre
209209
}
210210
}
211211

212+
@Test
213+
public void legacyCallbacksWithoutInferenceMetadataFailWhenGlobalIncludeInferredStatementsIsDisabled()
214+
throws Exception {
215+
SailRepository repo = createRepositoryWithLegacyCallbackForwarding(false);
216+
217+
IRI ontologyGraph = repo.getValueFactory().createIRI("urn:ontology");
218+
IRI dataGraph = repo.getValueFactory().createIRI("urn:data");
219+
220+
loadTurtle(repo, ontologyTurtle(), ontologyGraph);
221+
loadTurtle(repo, railcarBrakeShapeTurtle(null, null), RDF4J.SHACL_SHAPE_GRAPH);
222+
223+
IRI wagon1 = repo.getValueFactory().createIRI("urn:wagon1");
224+
IRI freightWagon = repo.getValueFactory().createIRI("https://example.com/trains/FreightWagon");
225+
226+
try (RepositoryConnection conn = repo.getConnection()) {
227+
conn.begin();
228+
conn.add(wagon1, RDF.TYPE, freightWagon, dataGraph);
229+
ShaclSailValidationException exception = assertThrows(ShaclSailValidationException.class,
230+
() -> commitAndRethrow(conn));
231+
assertTrue(exception.getMessage()
232+
.contains("deprecated SailConnectionListener callbacks without inferred flags"));
233+
assertTrue(exception.getMessage().contains("statementAdded(Statement, boolean inferred)"));
234+
assertTrue(exception.getMessage().contains("statementRemoved(Statement, boolean inferred)"));
235+
} finally {
236+
repo.shutDown();
237+
}
238+
}
239+
212240
@Test
213241
public void legacyCallbacksWithoutInferenceMetadataAreAcceptedWhenAllShapesIncludeInferredStatements()
214242
throws Exception {
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Eclipse RDF4J contributors.
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Distribution License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/org/documents/edl-v10.php.
8+
*
9+
* SPDX-License-Identifier: BSD-3-Clause
10+
*******************************************************************************/
11+
// Some portions generated by Codex
12+
13+
package org.eclipse.rdf4j.sail.shacl;
14+
15+
import static org.junit.jupiter.api.Assertions.assertFalse;
16+
import static org.mockito.Mockito.mock;
17+
import static org.mockito.Mockito.verify;
18+
import static org.mockito.Mockito.when;
19+
20+
import org.eclipse.rdf4j.sail.shacl.ast.Shape;
21+
import org.eclipse.rdf4j.sail.shacl.ast.planNodes.PlanNode;
22+
import org.eclipse.rdf4j.sail.shacl.wrapper.data.ConnectionsGroup;
23+
import org.junit.jupiter.api.Test;
24+
import org.slf4j.LoggerFactory;
25+
26+
class ShapeValidationContainerLifecycleTest {
27+
28+
@Test
29+
void emptyPlanShouldCloseOwnedConnectionsGroup() {
30+
Shape shape = mock(Shape.class);
31+
PlanNode planNode = mock(PlanNode.class);
32+
ConnectionsGroup connectionsGroup = mock(ConnectionsGroup.class);
33+
34+
when(planNode.isGuaranteedEmpty()).thenReturn(true);
35+
36+
ShapeValidationContainer container = new ShapeValidationContainer(
37+
shape,
38+
() -> planNode,
39+
false,
40+
false,
41+
1,
42+
false,
43+
false,
44+
LoggerFactory.getLogger(ShapeValidationContainerLifecycleTest.class),
45+
connectionsGroup,
46+
true);
47+
48+
assertFalse(container.hasPlanNode());
49+
verify(connectionsGroup).close();
50+
}
51+
}

0 commit comments

Comments
 (0)