Skip to content

Commit cd83a48

Browse files
committed
Add validation of cyclic dependency for reference elements
1 parent 3548d4c commit cd83a48

7 files changed

Lines changed: 193 additions & 21 deletions

File tree

core/src/main/java/com/github/flexca/enot/core/parser/EnotParser.java

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.github.flexca.enot.core.EnotContext;
44
import com.github.flexca.enot.core.exception.EnotParsingException;
5+
import com.github.flexca.enot.core.parser.context.ParsingContext;
56
import com.github.flexca.enot.core.registry.EnotElementBodyResolver;
67
import com.github.flexca.enot.core.registry.EnotElementSpecification;
78
import com.github.flexca.enot.core.registry.EnotTypeSpecification;
@@ -16,6 +17,24 @@
1617

1718
import java.util.*;
1819

20+
/**
21+
* Parses eNot template JSON into a list of {@link EnotElement} instances.
22+
*
23+
* <p>The parser accepts a JSON object (single root element) or a JSON array
24+
* (multiple root elements) and walks the tree recursively, resolving element
25+
* types via the {@link com.github.flexca.enot.core.registry.EnotRegistry},
26+
* validating attributes and body structure, and delegating dynamic body
27+
* resolution to any registered {@link EnotElementBodyResolver}.</p>
28+
*
29+
* <p>Cyclic-dependency detection is handled transparently: a fresh
30+
* {@link ParsingContext} is created for each top-level {@link #parse(String, EnotContext)}
31+
* call, and a snapshot copy is passed to each body resolver so that sibling
32+
* branches do not interfere with each other's tracking sets.</p>
33+
*
34+
* <p>All parse errors are collected into {@link EnotJsonError} entries and
35+
* reported together via a single {@link EnotParsingException}, making it easy
36+
* to surface multiple problems in one pass.</p>
37+
*/
1938
public class EnotParser {
2039

2140
public static final String ENOT_ELEMENT_TYPE_NAME = "type";
@@ -33,7 +52,45 @@ public EnotParser(ObjectMapper objectMapper) {
3352
this.objectMapper = objectMapper;
3453
}
3554

55+
/**
56+
* Parses {@code json} into a list of {@link EnotElement} instances using a
57+
* fresh {@link ParsingContext}.
58+
*
59+
* <p>This is the standard entry point for top-level parsing. A new
60+
* {@link ParsingContext} is created automatically so that cyclic-dependency
61+
* detection starts from an empty state.</p>
62+
*
63+
* @param json the eNot template as a JSON string; must not be blank
64+
* @param enotContext the registry and shared services for this parse run
65+
* @return a non-empty list of parsed root elements
66+
* @throws EnotParsingException if the input is blank, not valid JSON, or
67+
* contains structural or type errors
68+
*/
3669
public List<EnotElement> parse(String json, EnotContext enotContext) throws EnotParsingException {
70+
ParsingContext parsingContext = new ParsingContext();
71+
return parse(json, enotContext, parsingContext);
72+
}
73+
74+
/**
75+
* Parses {@code json} into a list of {@link EnotElement} instances using the
76+
* supplied {@link ParsingContext}.
77+
*
78+
* <p>Use this overload when parsing is initiated from within an
79+
* {@link EnotElementBodyResolver} (e.g. {@code system/reference} resolution),
80+
* so that the caller's already-populated context — carrying the set of
81+
* composite identifiers currently being resolved — is passed through.
82+
* This allows cycle detection to span across template boundaries.</p>
83+
*
84+
* @param json the eNot template as a JSON string; must not be blank
85+
* @param enotContext the registry and shared services for this parse run
86+
* @param parsingContext the active parsing context propagated from the caller;
87+
* must be a {@link ParsingContext#copy() copy} so that
88+
* sibling branches remain independent
89+
* @return a non-empty list of parsed root elements
90+
* @throws EnotParsingException if the input is blank, not valid JSON, or
91+
* contains structural or type errors
92+
*/
93+
public List<EnotElement> parse(String json, EnotContext enotContext, ParsingContext parsingContext) throws EnotParsingException {
3794

3895
String currentPath = "";
3996
if (StringUtils.isBlank(json)) {
@@ -55,7 +112,7 @@ public List<EnotElement> parse(String json, EnotContext enotContext) throws Enot
55112

56113
if (rootNode.isArray()) {
57114
try {
58-
elements.addAll(parseElements(rootNode.asArray(), currentPath, jsonErrors, enotContext));
115+
elements.addAll(parseElements(rootNode.asArray(), currentPath, jsonErrors, enotContext, parsingContext));
59116
} catch (Exception e) {
60117
cause = e;
61118
jsonErrors.add(EnotJsonError.of(currentPath, e.getMessage()));
@@ -65,7 +122,8 @@ public List<EnotElement> parse(String json, EnotContext enotContext) throws Enot
65122
}
66123
} else if (rootNode.isObject()) {
67124
try {
68-
Optional<EnotElement> element = parseElement(rootNode.asObject(), currentPath, jsonErrors, enotContext);
125+
Optional<EnotElement> element = parseElement(rootNode.asObject(), currentPath, jsonErrors, enotContext,
126+
parsingContext);
69127
element.ifPresent(elements::add);
70128
} catch (Exception e) {
71129
cause = e;
@@ -87,14 +145,15 @@ public List<EnotElement> parse(String json, EnotContext enotContext) throws Enot
87145
}
88146

89147
private List<EnotElement> parseElements(ArrayNode elementsArray, String parentPath, List<EnotJsonError> jsonErrors,
90-
EnotContext enotContext) {
148+
EnotContext enotContext, ParsingContext parsingContext) {
91149

92150
List<EnotElement> elements = new ArrayList<>(elementsArray.size());
93151
for (int i = 0; i < elementsArray.size(); i++) {
94152
JsonNode itemNode = elementsArray.get(i);
95153
String currentPath = parentPath + "/" + i;
96154
if (itemNode.isObject()) {
97-
Optional<EnotElement> element = parseElement(itemNode.asObject(), currentPath, jsonErrors, enotContext);
155+
Optional<EnotElement> element = parseElement(itemNode.asObject(), currentPath, jsonErrors, enotContext,
156+
parsingContext);
98157
element.ifPresent(elements::add);
99158
} else {
100159
jsonErrors.add(EnotJsonError.of(currentPath, "eNot expecting object, but get " + itemNode.getNodeType().name()));
@@ -104,7 +163,7 @@ private List<EnotElement> parseElements(ArrayNode elementsArray, String parentPa
104163
}
105164

106165
private Optional<EnotElement> parseElement(ObjectNode jsonElement, String parentPath, List<EnotJsonError> jsonErrors,
107-
EnotContext enotContext) {
166+
EnotContext enotContext, ParsingContext parsingContext) {
108167

109168
JsonNode typeNode = jsonElement.get(ENOT_ELEMENT_TYPE_NAME);
110169
if (typeNode == null) {
@@ -150,16 +209,11 @@ private Optional<EnotElement> parseElement(ObjectNode jsonElement, String parent
150209
}
151210
EnotElementBodyResolver bodyResolver = elementSpecification.getBodyResolver();
152211
if (bodyResolver == null) {
153-
Optional<Object> elementBody = extractElementBody(jsonElement, parentPath, jsonErrors, enotContext);
212+
Optional<Object> elementBody = extractElementBody(jsonElement, parentPath, jsonErrors, enotContext, parsingContext);
154213
elementBody.ifPresent(element::setBody);
155214
} else {
156215
try {
157-
158-
String compositeIdentifier = bodyResolver.getUniqueCompositeIdentifier(element);
159-
if (StringUtils.isNotBlank(compositeIdentifier)) {
160-
// TODO: detect cyclic dependency
161-
}
162-
element.setBody(bodyResolver.resolveBody(element, enotContext));
216+
element.setBody(bodyResolver.resolveBody(element, enotContext, parsingContext.copy()));
163217
} catch(Exception e) {
164218
jsonErrors.add(EnotJsonError.of(parentPath + "/" + ENOT_ELEMENT_BODY_NAME,
165219
"failure during resolving element body, reason: " + e.getMessage()));
@@ -224,7 +278,7 @@ private Map<EnotAttribute, Object> extractElementAttributes(ObjectNode jsonEleme
224278
}
225279

226280
private Optional<Object> extractElementBody(ObjectNode jsonElement, String parentPath, List<EnotJsonError> jsonErrors,
227-
EnotContext enotContext) {
281+
EnotContext enotContext, ParsingContext parsingContext) {
228282

229283
String currentPath = parentPath + "/" + ENOT_ELEMENT_BODY_NAME;
230284

@@ -237,7 +291,7 @@ private Optional<Object> extractElementBody(ObjectNode jsonElement, String paren
237291
return Optional.empty();
238292
}
239293
if (bodyArrayNode.get(0).isObject()) {
240-
List<EnotElement> elements = parseElements(bodyArrayNode, currentPath, jsonErrors, enotContext);
294+
List<EnotElement> elements = parseElements(bodyArrayNode, currentPath, jsonErrors, enotContext, parsingContext);
241295
return CollectionUtils.isEmpty(elements) ? Optional.empty() : Optional.of(elements);
242296
} else {
243297
List<Object> primitiveValues = new ArrayList<>();
@@ -255,7 +309,7 @@ private Optional<Object> extractElementBody(ObjectNode jsonElement, String paren
255309
return CollectionUtils.isEmpty(primitiveValues) ? Optional.empty() : Optional.of(primitiveValues);
256310
}
257311
} else if (bodyNode.isObject()) {
258-
Optional<EnotElement> element = parseElement(bodyNode.asObject(), currentPath, jsonErrors, enotContext);
312+
Optional<EnotElement> element = parseElement(bodyNode.asObject(), currentPath, jsonErrors, enotContext, parsingContext);
259313
return element.isEmpty() ? Optional.empty() : Optional.of(element.get());
260314
} else {
261315
Optional<Object> objectBody = extractPrimitiveValue(bodyNode);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package com.github.flexca.enot.core.parser.context;
2+
3+
import java.util.HashSet;
4+
import java.util.Set;
5+
6+
public class ParsingContext {
7+
8+
private final Set<String> compositeIdentifiers;
9+
10+
public ParsingContext() {
11+
compositeIdentifiers = new HashSet<>();
12+
}
13+
14+
private ParsingContext(Set<String> compositeIdentifiers) {
15+
this.compositeIdentifiers = new HashSet<>(compositeIdentifiers);
16+
}
17+
18+
public boolean addCompositeIdentifier(String identifier) {
19+
return compositeIdentifiers.add(identifier);
20+
}
21+
22+
public ParsingContext copy() {
23+
ParsingContext parsingContext = new ParsingContext(compositeIdentifiers);
24+
return parsingContext;
25+
}
26+
}

core/src/main/java/com/github/flexca/enot/core/registry/EnotElementBodyResolver.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.github.flexca.enot.core.EnotContext;
44
import com.github.flexca.enot.core.element.EnotElement;
5+
import com.github.flexca.enot.core.parser.context.ParsingContext;
56

67
/**
78
* Strategy interface for resolving an element's body dynamically at parse time.
@@ -59,5 +60,5 @@ public interface EnotElementBodyResolver {
5960
* @return the resolved body; the concrete type depends on the element kind
6061
* (e.g. {@code List<EnotElement>} for {@code system/reference})
6162
*/
62-
Object resolveBody(EnotElement element, EnotContext enotContext);
63+
Object resolveBody(EnotElement element, EnotContext enotContext, ParsingContext parsingContext);
6364
}

core/src/main/java/com/github/flexca/enot/core/registry/EnotElementReferenceResolver.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
import com.github.flexca.enot.core.EnotContext;
44
import com.github.flexca.enot.core.element.EnotElement;
55
import com.github.flexca.enot.core.parser.EnotParser;
6+
import com.github.flexca.enot.core.parser.context.ParsingContext;
67

78
import java.util.List;
89

910
public interface EnotElementReferenceResolver {
1011

1112
String getReferenceType();
1213

13-
List<EnotElement> resolve(String referenceIdentifier, EnotContext enotContext);
14+
List<EnotElement> resolve(String referenceIdentifier, EnotContext enotContext, ParsingContext parsingContext);
1415
}

core/src/main/java/com/github/flexca/enot/core/types/system/SystemReferenceBodyResolver.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.github.flexca.enot.core.EnotContext;
44
import com.github.flexca.enot.core.element.EnotElement;
55
import com.github.flexca.enot.core.exception.EnotInvalidArgumentException;
6+
import com.github.flexca.enot.core.parser.context.ParsingContext;
67
import com.github.flexca.enot.core.registry.EnotElementBodyResolver;
78
import com.github.flexca.enot.core.registry.EnotElementReferenceResolver;
89
import com.github.flexca.enot.core.types.system.attribute.SystemAttribute;
@@ -30,7 +31,13 @@ public String getUniqueCompositeIdentifier(EnotElement element) {
3031
}
3132

3233
@Override
33-
public Object resolveBody(EnotElement element, EnotContext enotContext) {
34+
public Object resolveBody(EnotElement element, EnotContext enotContext, ParsingContext parsingContext) {
35+
36+
String compositeIdentifier = getUniqueCompositeIdentifier(element);
37+
if (!parsingContext.addCompositeIdentifier(compositeIdentifier)) {
38+
throw new EnotInvalidArgumentException("cyclic dependency detected for element with composite identifier: "
39+
+ compositeIdentifier);
40+
}
3441

3542
String referenceType = getReferenceType(element);
3643
EnotElementReferenceResolver referenceResolver = enotContext.getEnotRegistry().getElementReferenceResolver(referenceType);
@@ -39,7 +46,7 @@ public Object resolveBody(EnotElement element, EnotContext enotContext) {
3946
+ referenceType);
4047
}
4148
String referenceIdentifier = getReferenceIdentifier(element);
42-
return referenceResolver.resolve(referenceIdentifier, enotContext);
49+
return referenceResolver.resolve(referenceIdentifier, enotContext, parsingContext);
4350
}
4451

4552
private String getReferenceType(EnotElement element) {

core/src/test/java/com/github/flexca/enot/core/parser/EnotParserReferenceResolverTest.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import java.util.Map;
2121

2222
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
24+
25+
import com.github.flexca.enot.core.exception.EnotParsingException;
2326

2427
public class EnotParserReferenceResolverTest {
2528

@@ -122,4 +125,83 @@ void testReferenceResolvedAtParseTime() throws Exception {
122125
assertThat(taggedObject.getType()).isEqualTo(Asn1TypeSpecification.TYPE_NAME);
123126
assertThat(taggedObject.getAttribute(Asn1Attribute.TAG)).isEqualTo("tagged_object");
124127
}
128+
129+
@Test
130+
void testCyclicDependencyAtoB() throws Exception {
131+
// A → B → A: the reference element inside cyclic-a.json has identifier
132+
// "test_resources:json/cyclic/cyclic-b.json". When cyclic-b.json is then
133+
// parsed and tries to resolve cyclic-a.json, that succeeds (adds cyclic-a).
134+
// On the second round cyclic-a.json tries to resolve cyclic-b.json again
135+
// — but cyclic-b is already in the ParsingContext, so the cycle is detected.
136+
String json = ResourceReaderTestUtils.readResourceFileAsString("json/cyclic/cyclic-a.json");
137+
138+
assertThatThrownBy(() -> enotParser.parse(json, enotContext))
139+
.isInstanceOf(EnotParsingException.class)
140+
.hasMessageContaining("cyclic dependency detected")
141+
.hasMessageContaining("test_resources:json/cyclic/cyclic-b.json");
142+
}
143+
144+
@Test
145+
void testCyclicDependencySelfReference() throws Exception {
146+
// A → A: a template that references itself.
147+
// On the first recursion the identifier is already present in the context.
148+
String json = ResourceReaderTestUtils.readResourceFileAsString("json/cyclic/self-ref.json");
149+
150+
assertThatThrownBy(() -> enotParser.parse(json, enotContext))
151+
.isInstanceOf(EnotParsingException.class)
152+
.hasMessageContaining("cyclic dependency detected")
153+
.hasMessageContaining("test_resources:json/cyclic/self-ref.json");
154+
}
155+
156+
@Test
157+
void testDiamondDependencyNoCycle() throws Exception {
158+
// Diamond: root → left → leaf
159+
// → right → leaf
160+
// Each branch gets an independent ParsingContext copy, so resolving
161+
// "leaf" from two sibling branches does not trigger a cycle error.
162+
String json = ResourceReaderTestUtils.readResourceFileAsString("json/cyclic/diamond-root.json");
163+
164+
List<EnotElement> actual = enotParser.parse(json, enotContext);
165+
166+
// root is an array → two reference elements
167+
assertThat(actual).hasSize(2);
168+
169+
EnotElement leftRef = actual.get(0);
170+
assertThat(leftRef.getAttribute(SystemAttribute.KIND)).isEqualTo("reference");
171+
assertThat(leftRef.getAttribute(SystemAttribute.REFERENCE_IDENTIFIER))
172+
.isEqualTo("json/cyclic/diamond-left.json");
173+
// leftRef.body = [ref-to-leaf]; ref-to-leaf.body = [oid]
174+
assertThat(leftRef.getBody()).isInstanceOf(List.class);
175+
@SuppressWarnings("unchecked")
176+
List<EnotElement> leftBody = (List<EnotElement>) leftRef.getBody();
177+
assertThat(leftBody).hasSize(1);
178+
EnotElement leftLeafRef = leftBody.get(0);
179+
assertThat(leftLeafRef.getAttribute(SystemAttribute.KIND)).isEqualTo("reference");
180+
assertThat(leftLeafRef.getAttribute(SystemAttribute.REFERENCE_IDENTIFIER))
181+
.isEqualTo("json/cyclic/diamond-leaf.json");
182+
assertThat(leftLeafRef.getBody()).isInstanceOf(List.class);
183+
@SuppressWarnings("unchecked")
184+
List<EnotElement> leftLeafBody = (List<EnotElement>) leftLeafRef.getBody();
185+
assertThat(leftLeafBody).hasSize(1);
186+
assertThat(leftLeafBody.get(0).getAttribute(Asn1Attribute.TAG)).isEqualTo("object_identifier");
187+
188+
EnotElement rightRef = actual.get(1);
189+
assertThat(rightRef.getAttribute(SystemAttribute.KIND)).isEqualTo("reference");
190+
assertThat(rightRef.getAttribute(SystemAttribute.REFERENCE_IDENTIFIER))
191+
.isEqualTo("json/cyclic/diamond-right.json");
192+
// rightRef.body = [ref-to-leaf]; ref-to-leaf.body = [oid]
193+
assertThat(rightRef.getBody()).isInstanceOf(List.class);
194+
@SuppressWarnings("unchecked")
195+
List<EnotElement> rightBody = (List<EnotElement>) rightRef.getBody();
196+
assertThat(rightBody).hasSize(1);
197+
EnotElement rightLeafRef = rightBody.get(0);
198+
assertThat(rightLeafRef.getAttribute(SystemAttribute.KIND)).isEqualTo("reference");
199+
assertThat(rightLeafRef.getAttribute(SystemAttribute.REFERENCE_IDENTIFIER))
200+
.isEqualTo("json/cyclic/diamond-leaf.json");
201+
assertThat(rightLeafRef.getBody()).isInstanceOf(List.class);
202+
@SuppressWarnings("unchecked")
203+
List<EnotElement> rightLeafBody = (List<EnotElement>) rightLeafRef.getBody();
204+
assertThat(rightLeafBody).hasSize(1);
205+
assertThat(rightLeafBody.get(0).getAttribute(Asn1Attribute.TAG)).isEqualTo("object_identifier");
206+
}
125207
}

core/src/test/java/com/github/flexca/enot/core/testutil/TestResourcesReferenceResolver.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.github.flexca.enot.core.EnotContext;
44
import com.github.flexca.enot.core.element.EnotElement;
55
import com.github.flexca.enot.core.exception.EnotInvalidArgumentException;
6+
import com.github.flexca.enot.core.parser.context.ParsingContext;
67
import com.github.flexca.enot.core.registry.EnotElementReferenceResolver;
78

89
import java.util.List;
@@ -17,10 +18,10 @@ public String getReferenceType() {
1718
}
1819

1920
@Override
20-
public List<EnotElement> resolve(String referenceIdentifier, EnotContext enotContext) {
21+
public List<EnotElement> resolve(String referenceIdentifier, EnotContext enotContext, ParsingContext parsingContext) {
2122
try {
2223
String json = ResourceReaderTestUtils.readResourceFileAsString(referenceIdentifier);
23-
return enotContext.getEnotParser().parse(json, enotContext);
24+
return enotContext.getEnotParser().parse(json, enotContext, parsingContext);
2425
} catch(Exception e) {
2526
throw new EnotInvalidArgumentException("cannot resolve eNot element with identifier: " + referenceIdentifier
2627
+ ", reson: " + e.getMessage(), e);

0 commit comments

Comments
 (0)