Skip to content

Commit c31e80f

Browse files
hmottestadhmottestad
authored
GH-5575 SHACL recursion detection and failure handling (#5577)
1 parent 7d884b7 commit c31e80f

File tree

53 files changed

+1000
-138
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+1000
-138
lines changed

‎PLANS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,4 @@ In crates/foo/planner.rs, define:
150150
If you follow the guidance above, a single, stateless agent -- or a human novice -- can read your ExecPlan from top to bottom and produce a working, observable result. That is the bar: SELF-CONTAINED, SELF-SUFFICIENT, NOVICE-GUIDING, OUTCOME-FOCUSED.
151151

152152
When you revise a plan, you must ensure your changes are comprehensively reflected across all sections, including the living document sections, and you must write a note at the bottom of the plan describing the change and the reason why. ExecPlans must describe not just the what but the why for almost everything.
153+

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/PropertyShape.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*******************************************************************************/
1111
package org.eclipse.rdf4j.sail.shacl.ast;
1212

13+
import java.util.IdentityHashMap;
1314
import java.util.List;
1415
import java.util.Objects;
1516
import java.util.Set;
@@ -344,14 +345,8 @@ public SourceConstraintComponent getConstraintComponent() {
344345
}
345346

346347
@Override
347-
public boolean equals(Object o) {
348-
if (this == o) {
349-
return true;
350-
}
351-
if (o == null || getClass() != o.getClass()) {
352-
return false;
353-
}
354-
if (!super.equals(o)) {
348+
public boolean equals(ConstraintComponent o, IdentityHashMap<Shape, Shape> kvIdentityHashMap) {
349+
if (!(o instanceof PropertyShape)) {
355350
return false;
356351
}
357352

@@ -369,12 +364,18 @@ public boolean equals(Object o) {
369364
if (!Objects.equals(group, that.group)) {
370365
return false;
371366
}
372-
return Objects.equals(path, that.path);
367+
368+
if (!Objects.equals(path, that.path)) {
369+
return false;
370+
}
371+
372+
return super.equals(o, kvIdentityHashMap);
373+
373374
}
374375

375376
@Override
376-
public int hashCode() {
377-
int result = super.hashCode();
377+
public int hashCode(IdentityHashMap<Shape, Boolean> cycleDetection) {
378+
int result = super.hashCode(cycleDetection);
378379
result = 31 * result + (name != null ? name.hashCode() : 0);
379380
result = 31 * result + (description != null ? description.hashCode() : 0);
380381
result = 31 * result + (defaultValue != null ? defaultValue.hashCode() : 0);

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/ShaclShapeParsingException.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ public ShaclShapeParsingException(String msg, Throwable t, Resource id) {
3232
this.id = id;
3333
}
3434

35+
public ShaclShapeParsingException(String msg, Throwable t) {
36+
super(msg, t);
37+
}
38+
3539
public ShaclShapeParsingException(Throwable t, Resource id) {
3640
super(t.getMessage() + " - Caused by shape with id: " + resourceToString(id), t);
3741
this.id = id;

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/Shape.java‎

Lines changed: 128 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@
1414
import java.util.ArrayList;
1515
import java.util.Collection;
1616
import java.util.HashSet;
17+
import java.util.IdentityHashMap;
1718
import java.util.List;
1819
import java.util.Objects;
1920
import java.util.Set;
2021
import java.util.stream.Collectors;
2122
import java.util.stream.Stream;
2223

2324
import org.eclipse.rdf4j.common.annotation.InternalUseOnly;
25+
import org.eclipse.rdf4j.common.exception.RDF4JException;
2426
import org.eclipse.rdf4j.model.IRI;
2527
import org.eclipse.rdf4j.model.Literal;
2628
import org.eclipse.rdf4j.model.Model;
@@ -43,6 +45,7 @@
4345
import org.eclipse.rdf4j.sail.shacl.SourceConstraintComponent;
4446
import org.eclipse.rdf4j.sail.shacl.ValidationSettings;
4547
import org.eclipse.rdf4j.sail.shacl.ast.StatementMatcher.Variable;
48+
import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.AbstractConstraintComponent;
4649
import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.AndConstraintComponent;
4750
import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.ClassConstraintComponent;
4851
import org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents.ClosedConstraintComponent;
@@ -201,7 +204,7 @@ public void toModel(Resource subject, IRI predicate, Model model, Set<Resource>
201204
modelBuilder.subject(getId());
202205

203206
if (deactivated) {
204-
modelBuilder.add(SHACL.DEACTIVATED, deactivated);
207+
modelBuilder.add(SHACL.DEACTIVATED, true);
205208
}
206209

207210
for (Literal literal : message) {
@@ -530,16 +533,32 @@ public List<Literal> getDefaultMessage() {
530533
}
531534

532535
@Override
533-
public boolean equals(Object o) {
536+
public final boolean equals(Object o) {
534537
if (this == o) {
535538
return true;
536539
}
537540
if (!(o instanceof Shape)) {
538541
return false;
539542
}
540543

544+
return equals((Shape) o, new IdentityHashMap<>());
545+
}
546+
547+
@Override
548+
public boolean equals(ConstraintComponent o, IdentityHashMap<Shape, Shape> comparisonGuard) {
549+
Object alreadyComparing = comparisonGuard.get(this);
550+
if (alreadyComparing == o) {
551+
return true;
552+
}
553+
554+
if (!(o instanceof Shape)) {
555+
return false;
556+
}
557+
541558
Shape shape = (Shape) o;
542559

560+
comparisonGuard.put(this, shape);
561+
543562
if (produceValidationReports != shape.produceValidationReports) {
544563
return false;
545564
}
@@ -555,47 +574,130 @@ public boolean equals(Object o) {
555574
if (severity != shape.severity) {
556575
return false;
557576
}
558-
if (!Objects.equals(constraintComponents, shape.constraintComponents)) {
577+
578+
if (constraintComponents.size() != shape.constraintComponents.size()) {
559579
return false;
560580
}
581+
582+
boolean[] matchedRightSide = new boolean[shape.constraintComponents.size()];
583+
584+
for (int i = 0; i < constraintComponents.size(); i++) {
585+
ConstraintComponent left = constraintComponents.get(i);
586+
587+
if (left == null) {
588+
continue;
589+
}
590+
591+
int matchIndex = -1;
592+
593+
if (!matchedRightSide[i]) {
594+
ConstraintComponent right = shape.constraintComponents.get(i);
595+
if (left.equals(right, comparisonGuard)) {
596+
matchIndex = i;
597+
}
598+
}
599+
600+
if (matchIndex == -1) {
601+
for (int j = 0; j < shape.constraintComponents.size(); j++) {
602+
if (matchedRightSide[j]) {
603+
continue;
604+
}
605+
ConstraintComponent candidate = shape.constraintComponents.get(j);
606+
if (left.equals(candidate, comparisonGuard)) {
607+
matchIndex = j;
608+
break;
609+
}
610+
}
611+
}
612+
613+
if (matchIndex == -1) {
614+
return false;
615+
}
616+
617+
matchedRightSide[matchIndex] = true;
618+
}
619+
561620
return true;
562621
}
563622

564623
@Override
565-
public int hashCode() {
624+
public final int hashCode() {
625+
return hashCode(new IdentityHashMap<>());
626+
}
627+
628+
@Override
629+
public int hashCode(IdentityHashMap<Shape, Boolean> cycleDetection) {
630+
if (cycleDetection.put(this, Boolean.TRUE) != null) {
631+
throw new ShaclShapeParsingException("Recursive shape definition detected while computing hashCode",
632+
getId());
633+
}
634+
566635
int result = (produceValidationReports ? 1 : 0);
567636
result = 31 * result + (target != null ? target.hashCode() : 0);
568637
result = 31 * result + (deactivated ? 1 : 0);
569638
result = 31 * result + (message != null ? message.hashCode() : 0);
570639
result = 31 * result + (severity != null ? severity.hashCode() : 0);
571-
result = 31 * result + (constraintComponents != null ? constraintComponents.hashCode() : 0);
640+
641+
long temp = 0;
642+
643+
for (ConstraintComponent constraintComponent : constraintComponents) {
644+
int componentHash;
645+
if (constraintComponent instanceof Shape) {
646+
componentHash = constraintComponent.hashCode(cycleDetection);
647+
} else if (constraintComponent instanceof AbstractConstraintComponent) {
648+
componentHash = constraintComponent.hashCode(cycleDetection);
649+
} else {
650+
componentHash = constraintComponent != null ? constraintComponent.hashCode() : 0;
651+
}
652+
temp += componentHash;
653+
}
654+
655+
result = 31 * result + Long.hashCode(temp);
656+
657+
cycleDetection.remove(this);
572658
return result;
573659
}
574660

575661
public static class Factory {
576662

577663
public static List<ContextWithShape> getShapes(ShapeSource shapeSource, ParseSettings parseSettings) {
578664

579-
List<ContextWithShape> parsed = parse(shapeSource, parseSettings);
580-
581-
return getShapes(parsed);
665+
try {
666+
List<ContextWithShape> parsed = parse(shapeSource, parseSettings);
667+
return getShapes(parsed);
668+
} catch (RDF4JException e) {
669+
logger.error(e.getMessage(), e);
670+
throw e;
671+
} catch (Throwable e) {
672+
logger.error("Unexpected error while parsing shapes", e);
673+
throw new ShaclShapeParsingException("Unexpected error while parsing shapes", e);
674+
}
582675

583676
}
584677

585678
public static List<ContextWithShape> getShapes(List<ContextWithShape> parsed) {
586-
return parsed.stream()
587-
.flatMap(contextWithShapes -> {
588-
List<Shape> split = split(contextWithShapes.getShape());
589-
calculateTargetChain(split);
590-
calculateIfProducesValidationResult(split);
591-
return split.stream().map(s -> {
592-
return new ContextWithShape(contextWithShapes.getDataGraph(),
593-
contextWithShapes.getShapeGraph(), s);
594-
});
595-
})
596-
.filter(ContextWithShape::hasShape)
597-
.distinct()
598-
.collect(Collectors.toList());
679+
try {
680+
return parsed.stream()
681+
.flatMap(contextWithShapes -> {
682+
List<Shape> split = split(contextWithShapes.getShape());
683+
calculateTargetChain(split);
684+
calculateIfProducesValidationResult(split);
685+
return split.stream().map(s -> {
686+
return new ContextWithShape(contextWithShapes.getDataGraph(),
687+
contextWithShapes.getShapeGraph(), s);
688+
});
689+
})
690+
.filter(ContextWithShape::hasShape)
691+
.distinct()
692+
.peek(a -> a.getShape().verifyNotRecursive())
693+
.collect(Collectors.toList());
694+
} catch (RDF4JException e) {
695+
logger.error(e.getMessage(), e);
696+
throw e;
697+
} catch (Throwable e) {
698+
logger.error("Unexpected error while parsing shapes", e);
699+
throw new ShaclShapeParsingException("Unexpected error while parsing shapes", e);
700+
}
599701
}
600702

601703
private static void calculateIfProducesValidationResult(List<Shape> split) {
@@ -757,6 +859,11 @@ public static List<ContextWithShape> getShapesInContext(ShapeSource shapeSource,
757859

758860
}
759861

862+
private void verifyNotRecursive() {
863+
// calling hashCode will throw an exception if recursion is detected
864+
int i = hashCode(new IdentityHashMap<>());
865+
}
866+
760867
@Override
761868
public String toString() {
762869
Model statements = toModel(new DynamicModel(new LinkedHashModelFactory()));

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractConstraintComponent.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
package org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents;
1313

14+
import java.util.IdentityHashMap;
15+
1416
import org.eclipse.rdf4j.model.IRI;
1517
import org.eclipse.rdf4j.model.Literal;
1618
import org.eclipse.rdf4j.model.Resource;
@@ -178,4 +180,21 @@ static PlanNode getAllTargetsIncludingThoseAddedByPath(ConnectionsGroup connecti
178180
return allTargets;
179181
}
180182

183+
@Override
184+
public final int hashCode() {
185+
return hashCode(new IdentityHashMap<>());
186+
}
187+
188+
@Override
189+
public final boolean equals(Object o) {
190+
if (this == o) {
191+
return true;
192+
}
193+
if (o == null || getClass() != o.getClass()) {
194+
return false;
195+
}
196+
197+
return equals(((AbstractConstraintComponent) o), new IdentityHashMap<>());
198+
}
199+
181200
}

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AbstractPairwiseConstraintComponent.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents;
1313

14+
import java.util.IdentityHashMap;
1415
import java.util.Optional;
1516
import java.util.Set;
1617

@@ -268,7 +269,7 @@ public boolean requiresEvaluation(ConnectionsGroup connectionsGroup, Scope scope
268269
}
269270

270271
@Override
271-
public boolean equals(Object o) {
272+
public boolean equals(ConstraintComponent o, IdentityHashMap<Shape, Shape> kvIdentityHashMap) {
272273
if (this == o) {
273274
return true;
274275
}
@@ -282,7 +283,7 @@ public boolean equals(Object o) {
282283
}
283284

284285
@Override
285-
public int hashCode() {
286+
public int hashCode(IdentityHashMap<Shape, Boolean> identityHashMap) {
286287
return predicate.hashCode() + "AbstractPairwiseConstraintComponent".hashCode();
287288
}
288289

‎core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/AndConstraintComponent.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package org.eclipse.rdf4j.sail.shacl.ast.constraintcomponents;
1313

14+
import java.util.IdentityHashMap;
1415
import java.util.List;
1516
import java.util.Set;
1617
import java.util.stream.Collectors;
@@ -176,21 +177,31 @@ public List<Literal> getDefaultMessage() {
176177
}
177178

178179
@Override
179-
public boolean equals(Object o) {
180-
if (this == o) {
181-
return true;
182-
}
183-
if (o == null || getClass() != o.getClass()) {
180+
public boolean equals(ConstraintComponent o, IdentityHashMap<Shape, Shape> guard) {
181+
if (!(o instanceof AndConstraintComponent)) {
184182
return false;
185183
}
186184

187185
AndConstraintComponent that = (AndConstraintComponent) o;
188186

189-
return and.equals(that.and);
187+
if (and.size() != that.and.size()) {
188+
return false;
189+
}
190+
191+
for (int i = 0; i < and.size(); i++) {
192+
if (!and.get(i).equals(that.and.get(i), guard)) {
193+
return false;
194+
}
195+
}
196+
return true;
190197
}
191198

192199
@Override
193-
public int hashCode() {
194-
return and.hashCode() + "AndConstraintComponent".hashCode();
200+
public int hashCode(IdentityHashMap<Shape, Boolean> guard) {
201+
int result = "AndConstraintComponent".hashCode();
202+
for (Shape shape : and) {
203+
result = 31 * result + shape.hashCode(guard);
204+
}
205+
return result;
195206
}
196207
}

0 commit comments

Comments
 (0)