Skip to content

Commit 1b2ee8f

Browse files
ivangsaivangsa
committed
fix: $RefParser fails to mergeAllOf in json with circular references
closes #30
1 parent 0983757 commit 1b2ee8f

File tree

2 files changed

+141
-58
lines changed

2 files changed

+141
-58
lines changed

‎src/main/java/io/zenwave360/jsonrefparser/$RefParser.java‎

Lines changed: 52 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,7 @@
22

33
import io.zenwave360.jsonrefparser.parser.ExtendedJsonContext;
44
import io.zenwave360.jsonrefparser.parser.Parser;
5-
import io.zenwave360.jsonrefparser.resolver.ClasspathResolver;
6-
import io.zenwave360.jsonrefparser.resolver.FileResolver;
7-
import io.zenwave360.jsonrefparser.resolver.HttpResolver;
8-
import io.zenwave360.jsonrefparser.resolver.RefFormat;
9-
import io.zenwave360.jsonrefparser.resolver.Resolver;
5+
import io.zenwave360.jsonrefparser.resolver.*;
106
import org.apache.commons.lang3.ArrayUtils;
117
import org.apache.commons.lang3.ObjectUtils;
128
import org.apache.commons.lang3.StringUtils;
@@ -143,61 +139,69 @@ public class $RefParser {
143139
return this;
144140
}
145141

142+
private Set<Object> mergeAllOfObjectStack = Collections.newSetFromMap(new IdentityHashMap<>());
143+
146144
public $RefParser mergeAllOf() {
147145
this.refs.addPath(uri);
148146
this.visited.clear();
147+
this.mergeAllOfObjectStack.clear();
149148
mergeAllOf(refs.schema(), new String[0], uri);
150149
return this;
151150
}
152151

153152
private void mergeAllOf(Object value, String[] paths, URI currentFileURL) {
154-
// var visitedNodeRef = String.format("%s%s", currentFileURL, jsonPointer(paths));
155-
// log.trace("{}visiting {}", indent(), visitedNodeRef);
156-
// if(visited.contains(value)) {
157-
// log.trace("{}skipping visited {}", indent(), visitedNodeRef);
158-
// return;
159-
// }
160-
// visited.add(value);
161-
if(paths.length > 0 && "allOf".equals(paths[paths.length -1])) {
162-
List allOf = (List) value;
163-
String[] jsonPaths = Arrays.copyOf(paths, paths.length -1);
164-
String jsonPath = jsonPath(jsonPaths);
165-
Map<String, Object> originalAllOfRoot = refs.jsonContext.read(jsonPath);
166-
167-
AllOfObject allOfObject = new AllOfObject();
168-
merge(allOfObject, originalAllOfRoot);
169-
for (int i = 0; i < allOf.size(); i++) {
170-
if(allOf.get(i) instanceof Map) {
171-
Map<String, Object> item = (Map<String, Object>) allOf.get(i);
172-
merge(allOfObject, item);
173-
} else {
174-
throw new RuntimeException("Could not understand allOf: " + allOf.get(i));
153+
// Use object identity to detect actual circular object references
154+
if (mergeAllOfObjectStack.contains(value)) {
155+
log.debug("Detected circular object reference during mergeAllOf at path: {}", Arrays.toString(paths));
156+
return; // Skip to prevent infinite recursion
157+
}
158+
159+
mergeAllOfObjectStack.add(value);
160+
161+
try {
162+
if (paths.length > 0 && "allOf".equals(paths[paths.length -1])) {
163+
List allOf = (List) value;
164+
String[] jsonPaths = Arrays.copyOf(paths, paths.length -1);
165+
String jsonPath = jsonPath(jsonPaths);
166+
Map<String, Object> originalAllOfRoot = refs.jsonContext.read(jsonPath);
167+
168+
AllOfObject allOfObject = new AllOfObject();
169+
merge(allOfObject, originalAllOfRoot);
170+
for (int i = 0; i < allOf.size(); i++) {
171+
if(allOf.get(i) instanceof Map) {
172+
Map<String, Object> item = (Map<String, Object>) allOf.get(i);
173+
merge(allOfObject, item);
174+
} else {
175+
throw new RuntimeException("Could not understand allOf: " + allOf.get(i));
176+
}
175177
}
176-
}
177178

178-
try {
179-
var mergedAllOfObject = allOfObject.buildAllOfObject();
180-
$Ref originalRef = refs.getOriginalRef(originalAllOfRoot);
181-
if (originalRef != null) {
182-
refs.saveOriginalRef(originalRef, mergedAllOfObject);
179+
try {
180+
var mergedAllOfObject = allOfObject.buildAllOfObject();
181+
$Ref originalRef = refs.getOriginalRef(originalAllOfRoot);
182+
if (originalRef != null) {
183+
refs.saveOriginalRef(originalRef, mergedAllOfObject);
184+
}
185+
refs.jsonContext.set(jsonPath, mergedAllOfObject);
186+
refs.saveOriginalAllOf(mergedAllOfObject, allOf);
187+
} catch (Exception e){
188+
log.error("Error setting jsonPath:{} in file:{}", jsonPath, currentFileURL, e);
189+
throw e;
190+
}
191+
} else if(value instanceof Map) {
192+
// visit
193+
((Map<String, Object>) value).entrySet().forEach(e -> {
194+
mergeAllOf(e.getValue(), ArrayUtils.add(paths, e.getKey()), currentFileURL);
195+
});
196+
} else if(value instanceof List) {
197+
// visit
198+
List list = (List) value;
199+
for (int i = 0; i < list.size(); i++) {
200+
mergeAllOf(list.get(i), ArrayUtils.add(paths, i + ""), currentFileURL);
183201
}
184-
refs.jsonContext.set(jsonPath, mergedAllOfObject);
185-
refs.saveOriginalAllOf(mergedAllOfObject, allOf);
186-
} catch (Exception e){
187-
log.error("Error setting jsonPath:{} in file:{}", jsonPath, currentFileURL, e);
188-
throw e;
189-
}
190-
} else if(value instanceof Map) {
191-
// visit
192-
((Map<String, Object>) value).entrySet().forEach(e -> {
193-
mergeAllOf(e.getValue(), ArrayUtils.add(paths, e.getKey()), currentFileURL);
194-
});
195-
} else if(value instanceof List) {
196-
// visit
197-
List list = (List) value;
198-
for (int i = 0; i < list.size(); i++) {
199-
mergeAllOf(list.get(i), ArrayUtils.add(paths, i + ""), currentFileURL);
200202
}
203+
} finally {
204+
mergeAllOfObjectStack.remove(value);
201205
}
202206
}
203207

‎src/test/java/io/zenwave360/jsonrefparser/ParserTest.java‎

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package io.zenwave360.jsonrefparser;
22

33
import com.fasterxml.jackson.annotation.JsonInclude;
4+
import com.fasterxml.jackson.core.JsonGenerator;
45
import com.fasterxml.jackson.core.JsonProcessingException;
5-
import com.fasterxml.jackson.databind.DeserializationFeature;
6-
import com.fasterxml.jackson.databind.ObjectMapper;
7-
import com.fasterxml.jackson.databind.SerializationFeature;
6+
import com.fasterxml.jackson.databind.*;
87
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
8+
import com.fasterxml.jackson.databind.module.SimpleModule;
99
import io.zenwave360.jsonrefparser.$RefParserOptions.OnMissing;
1010
import io.zenwave360.jsonrefparser.resolver.HttpResolver;
1111
import io.zenwave360.jsonrefparser.resolver.Resolver;
@@ -16,8 +16,7 @@
1616
import java.io.File;
1717
import java.io.IOException;
1818
import java.net.URI;
19-
import java.util.List;
20-
import java.util.Map;
19+
import java.util.*;
2120

2221
import static io.zenwave360.jsonrefparser.$RefParserOptions.OnCircular.FAIL;
2322

@@ -27,6 +26,10 @@ public class ParserTest {
2726
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
2827
mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
2928
mapper.setSerializationInclusion(JsonInclude.Include.NON_EMPTY);
29+
30+
SimpleModule module = new SimpleModule();
31+
module.addSerializer((Class<Map<?, ?>>) (Class<?>) Map.class, new CycleBreakingMapSerializer());
32+
mapper.registerModule(module);
3033
}
3134

3235
private void assertNoRefs(Object object) throws JsonProcessingException {
@@ -193,8 +196,34 @@ public void testDereferenceAndMerge_MultipleAllOf() throws IOException {
193196
$Refs refs = parser.dereference().mergeAllOf().getRefs();
194197

195198
assertNoRefs(refs.schema());
199+
assertNoAllOfs(refs.schema());
200+
201+
// Test merged properties from Test1 (test1a, test1b)
202+
var test1Properties = (Map) refs.get("$.components.schemas.Test1.properties");
203+
Assert.assertEquals(2, test1Properties.size());
204+
Assert.assertTrue(test1Properties.containsKey("test1a"));
205+
Assert.assertTrue(test1Properties.containsKey("test1b"));
206+
Assert.assertEquals("string", ((Map) test1Properties.get("test1a")).get("type"));
207+
Assert.assertEquals("string", ((Map) test1Properties.get("test1b")).get("type"));
208+
209+
// Test merged properties from Test2 (test2a, test2b)
210+
var test2Properties = (Map) refs.get("$.components.schemas.Test2.properties");
211+
Assert.assertEquals(2, test2Properties.size());
212+
Assert.assertTrue(test2Properties.containsKey("test2a"));
213+
Assert.assertTrue(test2Properties.containsKey("test2b"));
214+
215+
// Test final merged Test schema contains all 4 properties
196216
var properties = (Map) refs.get("$.components.schemas.Test.properties");
197217
Assert.assertEquals(4, properties.size());
218+
Assert.assertTrue(properties.containsKey("test1a"));
219+
Assert.assertTrue(properties.containsKey("test1b"));
220+
Assert.assertTrue(properties.containsKey("test2a"));
221+
Assert.assertTrue(properties.containsKey("test2b"));
222+
223+
// Verify Test schema maintains its object type
224+
var testSchema = (Map) refs.get("$.components.schemas.Test");
225+
Assert.assertEquals("object", testSchema.get("type"));
226+
Assert.assertFalse(testSchema.containsKey("allOf"));
198227
}
199228

200229
@Test
@@ -204,8 +233,32 @@ public void testDereferenceAndMerge_MultipleAllOf2() throws IOException {
204233
$Refs refs = parser.dereference().mergeAllOf().getRefs();
205234

206235
assertNoRefs(refs.schema());
236+
assertNoAllOfs(refs.schema());
237+
238+
// Test Test1b has merged properties (test1b, test1c)
239+
var test1bProperties = (Map) refs.get("$.components.schemas.Test1b.properties");
240+
Assert.assertEquals(2, test1bProperties.size());
241+
Assert.assertTrue(test1bProperties.containsKey("test1b"));
242+
Assert.assertTrue(test1bProperties.containsKey("test1c"));
243+
244+
// Test Test1 has all 3 properties (test1a from itself + test1b, test1c from Test1b)
245+
var test1Properties = (Map) refs.get("$.components.schemas.Test1.properties");
246+
Assert.assertEquals(3, test1Properties.size());
247+
Assert.assertTrue(test1Properties.containsKey("test1a"));
248+
Assert.assertTrue(test1Properties.containsKey("test1b"));
249+
Assert.assertTrue(test1Properties.containsKey("test1c"));
250+
251+
// Test final Test schema has all 5 properties
207252
var properties = (Map) refs.get("$.components.schemas.Test.properties");
208253
Assert.assertEquals(5, properties.size());
254+
Assert.assertTrue(properties.containsKey("test1a"));
255+
Assert.assertTrue(properties.containsKey("test1b"));
256+
Assert.assertTrue(properties.containsKey("test1c"));
257+
Assert.assertTrue(properties.containsKey("test2a"));
258+
Assert.assertTrue(properties.containsKey("test2b"));
259+
260+
// Verify no circular references detected
261+
Assert.assertFalse(refs.circular);
209262
}
210263

211264
@Test
@@ -358,25 +411,51 @@ public void testDereferenceOnMissingFail() throws IOException {
358411
}
359412

360413
@Test
361-
@Ignore
362414
public void testMergeAllOfRecursive() throws IOException {
363415
File file = new File("src/test/resources/asyncapi/orders-model.yml");
364416
$RefParser parser = new $RefParser(file).parse();
365417
$Refs refs = parser.dereference().mergeAllOf().getRefs();
366-
System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(refs.schema()));
418+
// System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(refs.schema()));
419+
Assert.assertTrue(refs.circular);
367420
assertNoRefs(refs.schema());
368421
assertNoAllOfs(refs.schema());
369422
}
370423

371424
@Test
372-
// @Ignore
373425
public void testDetectCircularRecursive() throws IOException {
374426
File file = new File("src/test/resources/asyncapi/orders-model.yml");
375427
$RefParser parser = new $RefParser(file).parse();
376428
$Refs refs = parser.dereference().getRefs();
377429
Assert.assertTrue(refs.circular);
378-
// System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(refs.schema()));
379-
// assertNoRefs(refs.schema());
430+
// System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(refs.schema()));
431+
assertNoRefs(refs.schema());
380432
}
381433

434+
435+
class CycleBreakingMapSerializer extends JsonSerializer<Map<?, ?>> {
436+
private final ThreadLocal<Set<Object>> SEEN =
437+
ThreadLocal.withInitial(() -> Collections.newSetFromMap(new IdentityHashMap<>()));
438+
439+
@Override
440+
public void serialize(Map<?, ?> map, JsonGenerator gen, SerializerProvider provider)
441+
throws IOException {
442+
Set<Object> seen = SEEN.get();
443+
if (seen.contains(map)) {
444+
gen.writeString("CYCLE"); // O gen.writeNull() para omitir
445+
return;
446+
}
447+
seen.add(map);
448+
try {
449+
gen.writeStartObject();
450+
for (Map.Entry<?, ?> entry : map.entrySet()) {
451+
gen.writeFieldName(entry.getKey().toString());
452+
Object value = entry.getValue();
453+
provider.defaultSerializeValue(value, gen); // Usa serializador recursivo para valores Map
454+
}
455+
gen.writeEndObject();
456+
} finally {
457+
seen.remove(map);
458+
}
459+
}
460+
}
382461
}

0 commit comments

Comments
 (0)