From 97f1c639a3556725ee7205718f6e16f40f3753d3 Mon Sep 17 00:00:00 2001 From: Ilya Kuramshin Date: Wed, 28 Sep 2022 14:08:17 +0400 Subject: [PATCH] 1. fixing infinite recursion with cyclic references (#2640) 2. using record names in unions to make possible to use several record 3. tests refactored Co-authored-by: iliax --- .../jsonschema/AvroJsonSchemaConverter.java | 70 ++++-- .../AvroJsonSchemaConverterTest.java | 229 ++++++++++++------ 2 files changed, 201 insertions(+), 98 deletions(-) diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverter.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverter.java index 61f742e721..8c70352613 100644 --- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverter.java +++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverter.java @@ -4,7 +4,6 @@ import java.net.URI; import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.stream.Collectors; import org.apache.avro.Schema; @@ -22,7 +21,7 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { builder.type(type); Map definitions = new HashMap<>(); - final FieldSchema root = convertSchema("root", schema, definitions, false); + final FieldSchema root = convertSchema(schema, definitions, true); builder.definitions(definitions); if (type.getType().equals(JsonType.Type.OBJECT)) { @@ -36,11 +35,11 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { private FieldSchema convertField(Schema.Field field, Map definitions) { - return convertSchema(field.name(), field.schema(), definitions, true); + return convertSchema(field.schema(), definitions, false); } - private FieldSchema convertSchema(String name, Schema schema, - Map definitions, boolean ref) { + private FieldSchema convertSchema(Schema schema, + Map definitions, boolean isRoot) { if (!schema.isUnion()) { JsonType type = convertType(schema); switch (type.getType()) { @@ -53,12 +52,12 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { return new SimpleFieldSchema(type); case OBJECT: if (schema.getType().equals(Schema.Type.MAP)) { - return new MapFieldSchema(convertSchema(name, schema.getValueType(), definitions, ref)); + return new MapFieldSchema(convertSchema(schema.getValueType(), definitions, isRoot)); } else { - return createObjectSchema(name, schema, definitions, ref); + return createObjectSchema(schema, definitions, isRoot); } case ARRAY: - return createArraySchema(name, schema, definitions); + return createArraySchema(schema, definitions); default: throw new RuntimeException("Unknown type"); } @@ -67,20 +66,26 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { } } - private FieldSchema createUnionSchema(Schema schema, Map definitions) { + // this method formats json-schema field in a way + // to fit avro-> json encoding rules (https://avro.apache.org/docs/1.11.1/specification/_print/#json-encoding) + private FieldSchema createUnionSchema(Schema schema, Map definitions) { final boolean nullable = schema.getTypes().stream() .anyMatch(t -> t.getType().equals(Schema.Type.NULL)); final Map fields = schema.getTypes().stream() .filter(t -> !t.getType().equals(Schema.Type.NULL)) - .map(f -> Tuples.of( - f.getType().getName().toLowerCase(Locale.ROOT), - convertSchema( - f.getType().getName().toLowerCase(Locale.ROOT), - f, definitions, true - ) - )).collect(Collectors.toMap( + .map(f -> { + String oneOfFieldName; + if (f.getType().equals(Schema.Type.RECORD)) { + // for records using full record name + oneOfFieldName = f.getFullName(); + } else { + // for primitive types - using type name + oneOfFieldName = f.getType().getName().toLowerCase(); + } + return Tuples.of(oneOfFieldName, convertSchema(f, definitions, false)); + }).collect(Collectors.toMap( Tuple2::getT1, Tuple2::getT2 )); @@ -97,8 +102,16 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { } } - private FieldSchema createObjectSchema(String name, Schema schema, - Map definitions, boolean ref) { + private FieldSchema createObjectSchema(Schema schema, + Map definitions, + boolean isRoot) { + var definitionName = schema.getFullName(); + if (definitions.containsKey(definitionName)) { + return createRefField(definitionName); + } + // adding stub record, need to avoid infinite recursion + definitions.put(definitionName, new ObjectFieldSchema(Map.of(), List.of())); + final Map fields = schema.getFields().stream() .map(f -> Tuples.of(f.name(), convertField(f, definitions))) .collect(Collectors.toMap( @@ -110,19 +123,26 @@ public class AvroJsonSchemaConverter implements JsonSchemaConverter { .filter(f -> !f.schema().isNullable()) .map(Schema.Field::name).collect(Collectors.toList()); - if (ref) { - String definitionName = String.format("Record%s", schema.getName()); - definitions.put(definitionName, new ObjectFieldSchema(fields, required)); - return new RefFieldSchema(String.format("#/definitions/%s", definitionName)); + var objectSchema = new ObjectFieldSchema(fields, required); + if (isRoot) { + // replacing stub with self-reference (need for usage in json-schema's oneOf) + definitions.put(definitionName, new RefFieldSchema("#")); + return objectSchema; } else { - return new ObjectFieldSchema(fields, required); + // replacing stub record with actual object structure + definitions.put(definitionName, objectSchema); + return createRefField(definitionName); } } - private ArrayFieldSchema createArraySchema(String name, Schema schema, + private RefFieldSchema createRefField(String definitionName) { + return new RefFieldSchema(String.format("#/definitions/%s", definitionName)); + } + + private ArrayFieldSchema createArraySchema(Schema schema, Map definitions) { return new ArrayFieldSchema( - convertSchema(name, schema.getElementType(), definitions, true) + convertSchema(schema.getElementType(), definitions, false) ); } diff --git a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverterTest.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverterTest.java index d78426d48a..24d4daf7d9 100644 --- a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverterTest.java +++ b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/util/jsonschema/AvroJsonSchemaConverterTest.java @@ -1,27 +1,29 @@ package com.provectus.kafka.ui.util.jsonschema; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import com.github.fge.jsonschema.core.exceptions.ProcessingException; -import com.github.fge.jsonschema.core.report.ProcessingReport; -import com.github.fge.jsonschema.main.JsonSchemaFactory; -import io.confluent.kafka.schemaregistry.avro.AvroSchemaUtils; -import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import lombok.SneakyThrows; import org.apache.avro.Schema; -import org.apache.avro.generic.GenericData; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class AvroJsonSchemaConverterTest { - @Test - public void avroConvertTest() throws URISyntaxException, JsonProcessingException { - final AvroJsonSchemaConverter converter = new AvroJsonSchemaConverter(); - URI basePath = new URI("http://example.com/"); +class AvroJsonSchemaConverterTest { - Schema recordSchema = (new Schema.Parser()).parse( - " {" + private AvroJsonSchemaConverter converter; + private URI basePath; + + @BeforeEach + void init() throws URISyntaxException { + converter = new AvroJsonSchemaConverter(); + basePath = new URI("http://example.com/"); + } + + @Test + void avroConvertTest() { + String avroSchema = + " {" + " \"type\": \"record\"," + " \"name\": \"Message\"," + " \"namespace\": \"com.provectus.kafka\"," @@ -76,45 +78,59 @@ public class AvroJsonSchemaConverterTest { + " }" + " }" + " ]" - + " }" - ); + + " }"; + String expectedJsonSchema = "{ " + + " \"$id\" : \"http://example.com/Message\", " + + " \"$schema\" : \"https://json-schema.org/draft/2020-12/schema\", " + + " \"type\" : \"object\", " + + " \"properties\" : { " + + " \"record\" : { \"$ref\" : \"#/definitions/com.provectus.kafka.InnerMessage\" } " + + " }, " + + " \"required\" : [ \"record\" ], " + + " \"definitions\" : { " + + " \"com.provectus.kafka.Message\" : { \"$ref\" : \"#\" }, " + + " \"com.provectus.kafka.InnerMessage\" : { " + + " \"type\" : \"object\", " + + " \"properties\" : { " + + " \"long_text\" : { " + + " \"oneOf\" : [ { " + + " \"type\" : \"null\" " + + " }, { " + + " \"type\" : \"object\", " + + " \"properties\" : { " + + " \"string\" : { " + + " \"type\" : \"string\" " + + " } " + + " } " + + " } ] " + + " }, " + + " \"array\" : { " + + " \"type\" : \"array\", " + + " \"items\" : { \"type\" : \"string\" } " + + " }, " + + " \"id\" : { \"type\" : \"integer\" }, " + + " \"text\" : { \"type\" : \"string\" }, " + + " \"map\" : { " + + " \"type\" : \"object\", " + + " \"additionalProperties\" : { \"type\" : \"integer\" } " + + " }, " + + " \"order\" : { " + + " \"enum\" : [ \"SPADES\", \"HEARTS\", \"DIAMONDS\", \"CLUBS\" ], " + + " \"type\" : \"string\" " + + " } " + + " }, " + + " \"required\" : [ \"id\", \"text\", \"order\", \"array\", \"map\" ] " + + " } " + + " } " + + "}"; - String expected = "{\"$id\":\"http://example.com/Message\"," - + "\"$schema\":\"https://json-schema.org/draft/2020-12/schema\"," - + "\"type\":\"object\",\"properties\":{\"record\":" - + "{\"$ref\":\"#/definitions/RecordInnerMessage\"}}," - + "\"required\":[\"record\"],\"definitions\":" - + "{\"RecordInnerMessage\":{\"type\":\"object\",\"" - + "properties\":{\"long_text\":{\"oneOf\":[{\"type\":\"null\"}," - + "{\"type\":\"object\",\"properties\":{\"string\":" - + "{\"type\":\"string\"}}}]},\"array\":{\"type\":\"array\",\"items\":" - + "{\"type\":\"string\"}},\"id\":{\"type\":\"integer\"},\"text\":" - + "{\"type\":\"string\"},\"map\":{\"type\":\"object\"," - + "\"additionalProperties\":{\"type\":\"integer\"}}," - + "\"order\":{\"enum\":[\"SPADES\",\"HEARTS\",\"DIAMONDS\",\"CLUBS\"]," - + "\"type\":\"string\"}}," - + "\"required\":[\"id\",\"text\",\"order\",\"array\",\"map\"]}}}"; - - final JsonSchema convertRecord = converter.convert(basePath, recordSchema); - - ObjectMapper om = new ObjectMapper(); - Assertions.assertEquals( - om.readTree(expected), - om.readTree( - convertRecord.toJson() - ) - ); - + convertAndCompare(expectedJsonSchema, avroSchema); } @Test - public void testNullableUnions() throws URISyntaxException, IOException, ProcessingException { - final AvroJsonSchemaConverter converter = new AvroJsonSchemaConverter(); - URI basePath = new URI("http://example.com/"); - final ObjectMapper objectMapper = new ObjectMapper(); - - Schema recordSchema = (new Schema.Parser()).parse( + void testNullableUnions() { + String avroSchema = " {" + " \"type\": \"record\"," + " \"name\": \"Message\"," @@ -138,38 +154,105 @@ public class AvroJsonSchemaConverterTest { + " \"default\": null" + " }" + " ]" - + " }" - ); + + " }"; - final GenericData.Record record = new GenericData.Record(recordSchema); - record.put("text", "Hello world"); - record.put("value", 100L); - byte[] jsonBytes = AvroSchemaUtils.toJson(record); - String serialized = new String(jsonBytes); - - String expected = + String expectedJsonSchema = "{\"$id\":\"http://example.com/Message\"," + "\"$schema\":\"https://json-schema.org/draft/2020-12/schema\"," + "\"type\":\"object\",\"properties\":{\"text\":" + "{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\"," + "\"properties\":{\"string\":{\"type\":\"string\"}}}]},\"value\":" + "{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\"," - + "\"properties\":{\"string\":{\"type\":\"string\"},\"long\":{\"type\":\"integer\"}}}]}}}"; + + "\"properties\":{\"string\":{\"type\":\"string\"},\"long\":{\"type\":\"integer\"}}}]}}," + + "\"definitions\" : { \"com.provectus.kafka.Message\" : { \"$ref\" : \"#\" }}}"; - final JsonSchema convert = converter.convert(basePath, recordSchema); - Assertions.assertEquals( - objectMapper.readTree(expected), - objectMapper.readTree(convert.toJson()) - ); - - - final ProcessingReport validate = - JsonSchemaFactory.byDefault().getJsonSchema( - objectMapper.readTree(expected) - ).validate( - objectMapper.readTree(serialized) - ); - - Assertions.assertTrue(validate.isSuccess()); + convertAndCompare(expectedJsonSchema, avroSchema); } + + @Test + void testRecordReferences() { + String avroSchema = + "{\n" + + " \"type\": \"record\", " + + " \"namespace\": \"n.s\", " + + " \"name\": \"RootMsg\", " + + " \"fields\":\n" + + " [ " + + " { " + + " \"name\": \"inner1\", " + + " \"type\": { " + + " \"type\": \"record\", " + + " \"name\": \"Inner\", " + + " \"fields\": [ { \"name\": \"f1\", \"type\": \"double\" } ] " + + " } " + + " }, " + + " { " + + " \"name\": \"inner2\", " + + " \"type\": { " + + " \"type\": \"record\", " + + " \"namespace\": \"n.s2\", " + + " \"name\": \"Inner\", " + + " \"fields\": " + + " [ { \"name\": \"f1\", \"type\": \"double\" } ] " + + " } " + + " }, " + + " { " + + " \"name\": \"refField\", " + + " \"type\": [ \"null\", \"Inner\", \"n.s2.Inner\", \"RootMsg\" ] " + + " } " + + " ] " + + "}"; + + String expectedJsonSchema = "{ " + + " \"$id\" : \"http://example.com/RootMsg\", " + + " \"$schema\" : \"https://json-schema.org/draft/2020-12/schema\", " + + " \"type\" : \"object\", " + + " \"properties\" : { " + + " \"inner1\" : { \"$ref\" : \"#/definitions/n.s.Inner\" }, " + + " \"inner2\" : { \"$ref\" : \"#/definitions/n.s2.Inner\" }, " + + " \"refField\" : { " + + " \"oneOf\" : [ " + + " { " + + " \"type\" : \"null\" " + + " }, " + + " { " + + " \"type\" : \"object\", " + + " \"properties\" : { " + + " \"n.s.RootMsg\" : { \"$ref\" : \"#/definitions/n.s.RootMsg\" }, " + + " \"n.s2.Inner\" : { \"$ref\" : \"#/definitions/n.s2.Inner\" }, " + + " \"n.s.Inner\" : { \"$ref\" : \"#/definitions/n.s.Inner\" } " + + " } " + + " } ] " + + " } " + + " }, " + + " \"required\" : [ \"inner1\", \"inner2\" ], " + + " \"definitions\" : { " + + " \"n.s.RootMsg\" : { \"$ref\" : \"#\" }, " + + " \"n.s2.Inner\" : { " + + " \"type\" : \"object\", " + + " \"properties\" : { \"f1\" : { \"type\" : \"number\" } }, " + + " \"required\" : [ \"f1\" ] " + + " }, " + + " \"n.s.Inner\" : { " + + " \"type\" : \"object\", " + + " \"properties\" : { \"f1\" : { \"type\" : \"number\" } }, " + + " \"required\" : [ \"f1\" ] " + + " } " + + " } " + + "}"; + + convertAndCompare(expectedJsonSchema, avroSchema); + } + + @SneakyThrows + private void convertAndCompare(String expectedJsonSchema, String sourceAvroSchema) { + var parseAvroSchema = new Schema.Parser().parse(sourceAvroSchema); + var converted = converter.convert(basePath, parseAvroSchema).toJson(); + var objectMapper = new ObjectMapper(); + Assertions.assertEquals( + objectMapper.readTree(expectedJsonSchema), + objectMapper.readTree(converted) + ); + } + } \ No newline at end of file