Skip to content

feat: identify array of single type #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 25, 2021
Merged

feat: identify array of single type #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e46a02f
feat: identify array of single type
Aug 20, 2021
4ddbe39
apply reviews
Aug 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 19 additions & 40 deletions .../kotlin/com/epages/restdocs/apispec/jsonschema/JsonSchemaFromFieldDescriptorsGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class JsonSchemaFromFieldDescriptorsGenerator {
if (schema is ObjectSchema) {
val groups = groupFieldsByFirstRemainingPathSegment(emptyList(), jsonFieldPaths)
if (groups.keys.size == 1 && groups.keys.contains("[]")) {
val descriptor = jsonFieldPaths.find { it.fieldDescriptor.path == "[]" }?.fieldDescriptor
return ArraySchema.builder().allItemSchema(schema.propertySchemas["[]"]).applyConstraints(descriptor).title(schema.title).build()
return jsonFieldPaths.find { it.fieldDescriptor.path == "[]" }?.fieldDescriptor?.jsonSchemaType()
?: ArraySchema.builder().allItemSchema(schema.propertySchemas["[]"]).title(schema.title).build()
}
}
return schema
Expand Down Expand Up @@ -148,6 +148,7 @@ class JsonSchemaFromFieldDescriptorsGenerator {
traversedSegments.add(remainingSegments[0])
builder.addPropertySchema(
propertyName,

ArraySchema.builder()
.allItemSchema(traverse(traversedSegments, fields, ObjectSchema.builder()))
.applyConstraints(propertyField?.fieldDescriptor)
Expand All @@ -167,37 +168,11 @@ class JsonSchemaFromFieldDescriptorsGenerator {
}

private fun handleEndOfPath(builder: ObjectSchema.Builder, propertyName: String, fieldDescriptor: FieldDescriptorWithSchemaType) {

if (fieldDescriptor.ignored) {
// We don't need to render anything
} else {
if (!fieldDescriptor.ignored) {
if (isRequired(fieldDescriptor)) {
builder.addRequiredProperty(propertyName)
}
if (propertyName == "[]") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this case obsolete now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not obsolete, it looks like fieldDescriptor.jsonSchemaType() can handle this case. Now all the tests are still green. I think the tests cover this part well but maybe I miss something,. If removing this part creates bugs, then some tests might be missing.

Copy link
Contributor

@ozscheyge ozscheyge Aug 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's keep it removed. If it creates a regression, we get to know what wasn't tested and can write the test. 😅

builder.addPropertySchema(
propertyName,
createSchemaWithArrayContent(ObjectSchema.builder().build(), depthOfArrayPath(fieldDescriptor.path), fieldDescriptor)
)
} else {
builder.addPropertySchema(propertyName, fieldDescriptor.jsonSchemaType())
}
}
}

private fun depthOfArrayPath(path: String): Int {
return path.split("]")
.filter { it.isNotEmpty() }
.size - 1
}

private fun createSchemaWithArrayContent(schema: Schema, level: Int, fieldDescriptor: FieldDescriptorWithSchemaType): Schema {
return if (schema is ObjectSchema && level < 1) {
schema
} else if (level <= 1) {
ArraySchema.builder().addItemSchema(schema).applyConstraints(fieldDescriptor).build()
} else {
createSchemaWithArrayContent(ArraySchema.builder().addItemSchema(schema).applyConstraints(fieldDescriptor).build(), level - 1, fieldDescriptor)
builder.addPropertySchema(propertyName, fieldDescriptor.jsonSchemaType())
}
}

Expand Down Expand Up @@ -243,16 +218,7 @@ class JsonSchemaFromFieldDescriptorsGenerator {
"null" -> NullSchema.builder()
"empty" -> EmptySchema.builder()
"object" -> ObjectSchema.builder()
"array" -> ArraySchema.builder().applyConstraints(this).allItemSchema(
CombinedSchema.oneOf(
listOf(
ObjectSchema.builder().build(),
BooleanSchema.builder().build(),
StringSchema.builder().build(),
NumberSchema.builder().build()
)
).build()
)
"array" -> ArraySchema.builder().applyConstraints(this).allItemSchema(arrayItemsSchema())
"boolean" -> BooleanSchema.builder()
"number" -> NumberSchema.builder()
"string" -> StringSchema.builder()
Expand All @@ -267,6 +233,19 @@ class JsonSchemaFromFieldDescriptorsGenerator {
else -> throw IllegalArgumentException("unknown field type $type")
}

private fun arrayItemsSchema(): Schema {
return attributes.itemsType
?.let { typeToSchema(it.toLowerCase()).build() }
?: CombinedSchema.oneOf(
listOf(
ObjectSchema.builder().build(),
BooleanSchema.builder().build(),
StringSchema.builder().build(),
NumberSchema.builder().build()
)
).build()
}

fun equalsOnPathAndType(f: FieldDescriptorWithSchemaType): Boolean =
(
this.path == f.path &&
Expand Down
90 changes: 90 additions & 0 deletions ...lin/com/epages/restdocs/apispec/jsonschema/JsonSchemaFromFieldDescriptorsGeneratorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import com.jayway.jsonpath.JsonPath
import org.assertj.core.api.BDDAssertions.then
import org.assertj.core.api.BDDAssertions.thenThrownBy
import org.everit.json.schema.ArraySchema
import org.everit.json.schema.BooleanSchema
import org.everit.json.schema.CombinedSchema
import org.everit.json.schema.CombinedSchema.ONE_CRITERION
import org.everit.json.schema.EnumSchema
import org.everit.json.schema.NumberSchema
import org.everit.json.schema.ObjectSchema
import org.everit.json.schema.Schema
import org.everit.json.schema.StringSchema
Expand Down Expand Up @@ -138,6 +141,15 @@ class JsonSchemaFromFieldDescriptorsGeneratorTest {
whenSchemaGenerated()

then(schema).isInstanceOf(ArraySchema::class.java)
then((schema as ArraySchema).allItemSchema).isInstanceOf(CombinedSchema::class.java)
val combinedSchema = ((schema as ArraySchema).allItemSchema) as CombinedSchema
then(combinedSchema.criterion).isEqualTo(ONE_CRITERION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ONE_CRITERION seems to be the oneOf criterion:
https://github.com/everit-org/json-schema/blob/master/core/src/main/java/org/everit/json/schema/CombinedSchema.java#L119

This also matches current behavior when generating JSON schema for arrays with unknown items type:
#147

then(combinedSchema.subschemas).extracting("class").containsExactlyInAnyOrder(
ObjectSchema::class.java,
BooleanSchema::class.java,
StringSchema::class.java,
NumberSchema::class.java
)
thenSchemaIsValid()
thenSchemaValidatesJson("""[{"id": "some"}]""")
}
Expand All @@ -149,6 +161,17 @@ class JsonSchemaFromFieldDescriptorsGeneratorTest {
whenSchemaGenerated()

then(schema).isInstanceOf(ArraySchema::class.java)
then((schema as ArraySchema).allItemSchema).isInstanceOf(ArraySchema::class.java)
val arrayOfArraySchema = ((schema as ArraySchema).allItemSchema) as ArraySchema
then(arrayOfArraySchema.allItemSchema).isInstanceOf(CombinedSchema::class.java)
val combinedSchema = arrayOfArraySchema.allItemSchema as CombinedSchema
then(combinedSchema.criterion).isEqualTo(ONE_CRITERION)
then(combinedSchema.subschemas).extracting("class").containsExactlyInAnyOrder(
ObjectSchema::class.java,
BooleanSchema::class.java,
StringSchema::class.java,
NumberSchema::class.java
)
thenSchemaIsValid()
thenSchemaValidatesJson("""[[{"id": "some"}]]""")
}
Expand Down Expand Up @@ -289,6 +312,51 @@ class JsonSchemaFromFieldDescriptorsGeneratorTest {
thenSchemaIsValid()
}

@Test
fun should_specify_accurate_items_type_in_array_when_descriptor_contains_itemsType_in_additionalParameters() {
givenFieldDescriptorWithArrayOfSingleType()

whenSchemaGenerated()

then(schema).isInstanceOf(ArraySchema::class.java)
then((schema as ArraySchema).allItemSchema).isInstanceOf(StringSchema::class.java)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relevant part of JSON schema spec:
https://json-schema.org/draft/2020-12/json-schema-core.html#items

thenSchemaIsValid()
}

@Test
fun should_specify_accurate_items_type_in_array_of_array_when_descriptor_contains_itemsType_in_additionalParameters() {
givenFieldDescriptorWithTopLevelArrayOfArrayOfSingleType()

whenSchemaGenerated()

then(schema).isInstanceOf(ArraySchema::class.java)
then((schema as ArraySchema).allItemSchema).isInstanceOf(ArraySchema::class.java)
val arrayOfArraySchema = (schema as ArraySchema).allItemSchema as ArraySchema
then(arrayOfArraySchema.allItemSchema).isInstanceOf(StringSchema::class.java)
thenSchemaIsValid()
}

@Test
fun should_create_objectSchema_in_arraySchema_when_items_of_array_are_object() {
fieldDescriptors = listOf(
FieldDescriptor("thisIsAnArray", "I'm an array", "ARRAY"),
FieldDescriptor("thisIsAnArray[].numberItem", "I'm a number", "NUMBER"),
FieldDescriptor("thisIsAnArray[].objectItem", "I'm an object", "OBJECT")
)

whenSchemaGenerated()

then(schema).isInstanceOf(ObjectSchema::class.java)
then((schema as ObjectSchema).definesProperty("thisIsAnArray")).isTrue
then((schema as ObjectSchema).propertySchemas["thisIsAnArray"]).isInstanceOf(ArraySchema::class.java)
val objectInArray = ((schema as ObjectSchema).propertySchemas["thisIsAnArray"] as ArraySchema).allItemSchema as ObjectSchema
then(objectInArray.definesProperty("numberItem")).isTrue
then(objectInArray.propertySchemas["numberItem"]).isInstanceOf(NumberSchema::class.java)
then(objectInArray.definesProperty("objectItem")).isTrue
then(objectInArray.propertySchemas["objectItem"]).isInstanceOf(ObjectSchema::class.java)
thenSchemaIsValid()
}

private fun thenSchemaIsValid() {

val report = JsonSchemaFactory.byDefault()
Expand Down Expand Up @@ -331,10 +399,32 @@ class JsonSchemaFromFieldDescriptorsGeneratorTest {
fieldDescriptors = listOf(FieldDescriptor("[]", "some", "ARRAY"))
}

private fun givenFieldDescriptorWithArrayOfSingleType() {
fieldDescriptors = listOf(
FieldDescriptor(
"[]",
"some",
"ARRAY",
attributes = Attributes(itemsType = "string")
)
)
}

private fun givenFieldDescriptorWithTopLevelArrayOfArrayOfAny() {
fieldDescriptors = listOf(FieldDescriptor("[][]", "some", "ARRAY"))
}

private fun givenFieldDescriptorWithTopLevelArrayOfArrayOfSingleType() {
fieldDescriptors = listOf(
FieldDescriptor(
"[][]",
"some",
"ARRAY",
attributes = Attributes(itemsType = "string")
)
)
}

private fun givenFieldDescriptorUnspecifiedArrayItems() {
fieldDescriptors = listOf(FieldDescriptor("some[]", "some", "ARRAY"))
}
Expand Down
3 changes: 2 additions & 1 deletion restdocs-api-spec-model/src/main/kotlin/com/epages/restdocs/apispec/model/ResourceModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ open class FieldDescriptor(

data class Attributes(
val validationConstraints: List<Constraint> = emptyList(),
val enumValues: List<String> = emptyList()
val enumValues: List<String> = emptyList(),
val itemsType: String? = null
)

data class Constraint(
Expand Down