Skip to content

Commit 90a49ea

Browse files
OptimumCodeOptimumCode
authored
Rework the way the circled references are determine to avoid false-positive results (#63)
1 parent e867a0c commit 90a49ea

File tree

4 files changed

+263
-100
lines changed

4 files changed

+263
-100
lines changed

‎src/commonMain/kotlin/io/github/optimumcode/json/pointer/internal/extensions.kt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,15 @@ internal fun JsonPointer.dropLast(): JsonPointer? {
3232
return EmptyPointer
3333
}
3434
return JsonPointer.compile(fullPath.substring(0, lastPathPart))
35+
}
36+
37+
internal fun JsonPointer.lastSegment(): String? {
38+
var cur: JsonPointer? = this
39+
while (cur != EmptyPointer) {
40+
if (cur is SegmentPointer && cur.next is EmptyPointer) {
41+
return cur.propertyName
42+
}
43+
cur = cur?.next
44+
}
45+
return null
3546
}

‎src/commonMain/kotlin/io/github/optimumcode/json/schema/internal/ReferenceValidator.kt

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ package io.github.optimumcode.json.schema.internal
22

33
import com.eygraber.uri.Uri
44
import io.github.optimumcode.json.pointer.JsonPointer
5-
import io.github.optimumcode.json.pointer.contains
5+
import io.github.optimumcode.json.pointer.internal.dropLast
6+
import io.github.optimumcode.json.pointer.internal.lastSegment
67
import io.github.optimumcode.json.pointer.startsWith
78

89
internal object ReferenceValidator {
@@ -34,8 +35,8 @@ internal object ReferenceValidator {
3435
checkCircledReferences(usedRef, referencesWithPath)
3536
}
3637

37-
private val alwaysRunAssertions = hashSetOf("allOf", "anyOf", "oneOf")
38-
private val mightNotRun = hashSetOf("properties")
38+
private val alwaysRunInPlaceApplicators = hashSetOf("allOf", "anyOf", "oneOf")
39+
private val definitionProperties = hashSetOf("definitions", "\$defs")
3940

4041
private fun checkCircledReferences(
4142
usedRefs: Set<ReferenceLocation>,
@@ -48,9 +49,12 @@ internal object ReferenceValidator {
4849

4950
val circledReferences = hashSetOf<CircledReference>()
5051

51-
fun checkRunAlways(path: JsonPointer): Boolean {
52-
return alwaysRunAssertions.any { path.contains(it) } && mightNotRun.none { path.contains(it) }
53-
}
52+
val refsByBaseId: Map<Uri, Set<JsonPointer>> =
53+
referencesWithPath
54+
.entries
55+
.groupingBy { it.value.baseId }
56+
.fold(hashSetOf()) { acc, el -> acc.apply { add(el.value.pointer) } }
57+
5458
for ((location, refId) in locationToRef) {
5559
val schemaLocation: PointerWithBaseId = referencesWithPath.getValue(refId)
5660

@@ -64,7 +68,12 @@ internal object ReferenceValidator {
6468
) {
6569
continue
6670
}
67-
if (checkRunAlways(location) && checkRunAlways(otherLocation) && location != otherLocation) {
71+
val refsForBaseId = refsByBaseId[schemaLocation.baseId] ?: emptySet()
72+
if (checkRunAlways(
73+
location,
74+
refsForBaseId,
75+
) && checkRunAlways(otherLocation, refsForBaseId) && location != otherLocation
76+
) {
6877
circledReferences +=
6978
CircledReference(
7079
firstLocation = location,
@@ -83,6 +92,31 @@ internal object ReferenceValidator {
8392
}
8493
}
8594

95+
private fun checkRunAlways(
96+
path: JsonPointer,
97+
schemaLocations: Set<JsonPointer>,
98+
): Boolean {
99+
var curPath: JsonPointer? = path
100+
while (curPath != null) {
101+
val parentPath = curPath.dropLast()
102+
// The idea here is the following:
103+
// 'parentPath in schemaLocations' returns true only if the last segment is a schema keyword.
104+
// If this is the case we should check if this keyword is not applied in-place.
105+
// We also check that this keyword is not a definition as this would be incorrect.
106+
// If this all is 'true' this is not a circled reference
107+
if (
108+
parentPath in schemaLocations &&
109+
curPath.lastSegment()?.let {
110+
it !in alwaysRunInPlaceApplicators && it !in definitionProperties
111+
} == true
112+
) {
113+
return false
114+
}
115+
curPath = parentPath
116+
}
117+
return true
118+
}
119+
86120
private class CircledReference(
87121
val firstLocation: JsonPointer,
88122
val firstRef: JsonPointer,

‎src/commonTest/kotlin/io/github/optimumcode/json/schema/base/JsonSchemaCircledReferencesTest.kt

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
package io.github.optimumcode.json.schema.base
2+
3+
import io.github.optimumcode.json.schema.JsonSchema
4+
import io.kotest.assertions.asClue
5+
import io.kotest.assertions.throwables.shouldNotThrowAny
6+
import io.kotest.assertions.throwables.shouldThrow
7+
import io.kotest.core.spec.style.FunSpec
8+
import io.kotest.matchers.shouldBe
9+
10+
class JsonSchemaCircledReferencesTest : FunSpec() {
11+
init {
12+
13+
listOf(
14+
"oneOf",
15+
"allOf",
16+
"anyOf",
17+
).forEach {
18+
test("reports circled references $it") {
19+
shouldThrow<IllegalArgumentException> {
20+
JsonSchema.fromDefinition(
21+
"""
22+
{
23+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
24+
"definitions": {
25+
"alice": {
26+
"$it": [
27+
{ "${KEY}ref": "#/definitions/bob" }
28+
]
29+
},
30+
"bob": {
31+
"$it": [
32+
{ "${KEY}ref": "#/definitions/alice" }
33+
]
34+
}
35+
},
36+
"properties": {
37+
"alice": {
38+
"${KEY}ref": "#/definitions/alice"
39+
},
40+
"bob": {
41+
"${KEY}ref": "#/definitions/bob"
42+
}
43+
}
44+
}
45+
""".trimIndent(),
46+
)
47+
}.message shouldBe "circled references: /definitions/alice/$it/0 ref to /definitions/bob" +
48+
" and /definitions/bob/$it/0 ref to /definitions/alice"
49+
}
50+
}
51+
52+
test("reports if properties is not the applicator") {
53+
shouldThrow<IllegalArgumentException> {
54+
JsonSchema.fromDefinition(
55+
"""
56+
{
57+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
58+
"definitions": {
59+
"alice": {
60+
"allOf": [
61+
{ "${KEY}ref": "#/definitions/properties" }
62+
]
63+
},
64+
"properties": {
65+
"allOf": [
66+
{ "${KEY}ref": "#/definitions/alice" }
67+
]
68+
}
69+
},
70+
"properties": {
71+
"alice": {
72+
"${KEY}ref": "#/definitions/alice"
73+
},
74+
"bob": {
75+
"${KEY}ref": "#/definitions/properties"
76+
}
77+
}
78+
}
79+
""".trimIndent(),
80+
)
81+
}.message shouldBe "circled references: /definitions/alice/allOf/0 ref to /definitions/properties" +
82+
" and /definitions/properties/allOf/0 ref to /definitions/alice"
83+
}
84+
85+
test("does not report circled references if definitions have similar names") {
86+
shouldNotThrowAny {
87+
JsonSchema.fromDefinition(
88+
"""
89+
{
90+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
91+
"definitions": {
92+
"nonNegativeInteger": {
93+
"type": "integer",
94+
"minimum": 0
95+
},
96+
"nonNegativeIntegerDefault0": {
97+
"allOf": [
98+
{ "${KEY}ref": "#/definitions/nonNegativeInteger" },
99+
{ "default": 0 }
100+
]
101+
}
102+
}
103+
}
104+
""".trimIndent(),
105+
)
106+
}
107+
}
108+
109+
test("does not report circled references if one is defined inside properties applicator") {
110+
"""
111+
{
112+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
113+
"definitions": {
114+
"alice": {
115+
"allOf": [
116+
{ "${KEY}ref": "#/definitions/bob" }
117+
]
118+
},
119+
"bob": {
120+
"properties": {
121+
"test": {
122+
"allOf": [
123+
{ "${KEY}ref": "#/definitions/alice" }
124+
]
125+
}
126+
}
127+
}
128+
},
129+
"properties": {
130+
"alice": {
131+
"${KEY}ref": "#/definitions/alice"
132+
},
133+
"bob": {
134+
"${KEY}ref": "#/definitions/bob"
135+
}
136+
}
137+
}
138+
""".trimIndent().asClue {
139+
shouldNotThrowAny {
140+
JsonSchema.fromDefinition(it)
141+
}
142+
}
143+
144+
"""
145+
{
146+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
147+
"definitions": {
148+
"alice": {
149+
"properties": {
150+
"test": {
151+
"allOf": [
152+
{ "${KEY}ref": "#/definitions/alice" }
153+
]
154+
}
155+
}
156+
},
157+
"bob": {
158+
"allOf": [
159+
{ "${KEY}ref": "#/definitions/bob" }
160+
]
161+
}
162+
},
163+
"properties": {
164+
"alice": {
165+
"${KEY}ref": "#/definitions/alice"
166+
},
167+
"bob": {
168+
"${KEY}ref": "#/definitions/bob"
169+
}
170+
}
171+
}
172+
""".trimIndent().asClue {
173+
shouldNotThrowAny {
174+
JsonSchema.fromDefinition(it)
175+
}
176+
}
177+
}
178+
179+
test("does not report circled references if property match allOf") {
180+
"""
181+
{
182+
"${KEY}schema": "http://json-schema.org/draft-07/schema#",
183+
"definitions": {
184+
"alice": {
185+
"allOf": [
186+
{ "${KEY}ref": "#/definitions/bob" }
187+
]
188+
},
189+
"bob": {
190+
"properties": {
191+
"allOf": { "${KEY}ref": "#/definitions/alice" }
192+
}
193+
}
194+
},
195+
"properties": {
196+
"alice": {
197+
"${KEY}ref": "#/definitions/alice"
198+
},
199+
"bob": {
200+
"${KEY}ref": "#/definitions/bob"
201+
}
202+
}
203+
}
204+
""".trimIndent().asClue {
205+
shouldNotThrowAny {
206+
JsonSchema.fromDefinition(it)
207+
}
208+
}
209+
}
210+
}
211+
}

0 commit comments

Comments
 (0)