From 8a18a3b889f8ef2fad069b6cc5150f1fe791d94d Mon Sep 17 00:00:00 2001 From: Loose Date: Tue, 24 Sep 2024 14:30:07 -0300 Subject: [PATCH 1/6] W-16571553 - Back to 5.7.0-SNAPSHOT --- amf-apicontract.versions | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/amf-apicontract.versions b/amf-apicontract.versions index 80bb5e28a1..03e685d2ce 100644 --- a/amf-apicontract.versions +++ b/amf-apicontract.versions @@ -1,5 +1,5 @@ -amf.apicontract=5.6.1 -amf.aml=6.6.0 +amf.apicontract=5.7.0-SNAPSHOT +amf.aml=6.7.0-SNAPSHOT amf.model=3.9.0 antlr4Version=0.7.25 amf.validation.profile.dialect=1.6.0 From 6e0758cf44a1f5fd969c85722df4eec10fd9cc43 Mon Sep 17 00:00:00 2001 From: Loose Date: Tue, 24 Sep 2024 19:15:20 -0300 Subject: [PATCH 2/6] W-16772136 - Minor changes in Avro ADR due last library changes (naming) --- adrs/0014-avro-support.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/adrs/0014-avro-support.md b/adrs/0014-avro-support.md index 249f6c1c82..b7979acf32 100644 --- a/adrs/0014-avro-support.md +++ b/adrs/0014-avro-support.md @@ -67,7 +67,7 @@ We don't support AVRO Schemas: - because we can't determine if it's an AVRO Schema or any other schema ### AVRO Validation -We'll use the Apache official libraries for JVM and JS. +We'll use the Apache official libraries for JVM and JS, in the version 1.11.3, due Java8 binary compatibility requirements. ## Consequences / Constraints @@ -82,5 +82,5 @@ The validation plugins differ in interfaces and implementations, and each has so ### Both JVM & JS validation plugins - `"default"` values are not being validated when the type is `bytes`, `map`, or `array` - the validator treats as invalid an empty array as the default value for arrays (`"default": []`) even though the [Avro Schema Specification](https://avro.apache.org/docs/1.12.0/specification) has some examples with it -- if an avro record has a field that is a union that includes the root record itself (recursive reference) we fail to validate it correctly because we treat that shape as an unresolved/undefined shape -- in the future we'll try to ignore the cases that we are now failing and/or show a warning instead +- if an avro record has a field that is a union that includes the root record itself (recursive reference) we fail to validate payloads (default value or payload validation) against that 'recursive union field' correctly. The root record works correctly. In the future we'll try to ignore the cases that we are now failing and/or show a warning instead. +- the avro libraries are very restrictives with the allowed characters in naming definition (names of records for example). They only allow letters, numbers and `_` chars. We are not modifying this behavior. \ No newline at end of file From bdbec895e8553d005e332fa18052fac265efe878 Mon Sep 17 00:00:00 2001 From: Loose Date: Wed, 25 Sep 2024 17:59:55 -0300 Subject: [PATCH 3/6] W-16772136 - Fix nested union validation error in Avro. Fixed parsing of union as array items --- .../parser/domain/AvroArrayShapeParser.scala | 4 +- ...arser.scala => AvroInlineTypeParser.scala} | 9 +- .../parser/domain/AvroUnionShapeParser.scala | 26 ++++- .../union-nullable-record-invalid.report.js | 14 +++ .../union-nullable-record-invalid.report.jvm | 14 +++ .../union-simple-array-invalid.report.js | 14 +++ .../union-simple-array-invalid.report.jvm | 14 +++ .../union-simple-record-invalid.report.js | 14 +++ .../union-simple-record-invalid.report.jvm | 14 +++ .../union-nullable-record-invalid.json | 12 +++ .../schemas/union-nullable-record-valid.json | 12 +++ .../schemas/union-simple-array-invalid.json | 5 + .../schemas/union-simple-array-valid.json | 5 + .../schemas/union-simple-record-invalid.json | 12 +++ .../schemas/union-simple-record-valid.json | 12 +++ .../amf/avro/AvroSchemaValidationTest.scala | 49 ++++++++++ .../validation/PayloadValidationTest.scala | 37 +++++++- .../avro/BaseAvroSchemaPayloadValidator.scala | 94 +++++++++++-------- 18 files changed, 312 insertions(+), 49 deletions(-) rename amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/{AvroTextParser.scala => AvroInlineTypeParser.scala} (59%) create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.js create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.jvm create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.js create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.jvm create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.js create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.jvm create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-valid.json create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-valid.json create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-valid.json diff --git a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroArrayShapeParser.scala b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroArrayShapeParser.scala index 82c3235403..db5d4d8daf 100644 --- a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroArrayShapeParser.scala +++ b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroArrayShapeParser.scala @@ -11,7 +11,9 @@ case class AvroArrayShapeParser(map: YMap)(implicit ctx: AvroSchemaContext) override def setMembers(anyShape: AnyShape): Unit = shape.setWithoutId(ArrayShapeModel.Items, anyShape, Annotations.inferred()) - override def parseMembers(e: YMapEntry): AnyShape = AvroTextParser(e.value).parse() + override def parseMembers(e: YMapEntry): AnyShape = { + AvroInlineTypeParser(e.value).parse() + } override def parseSpecificFields(): Unit = {} } diff --git a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroTextParser.scala b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroInlineTypeParser.scala similarity index 59% rename from amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroTextParser.scala rename to amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroInlineTypeParser.scala index 5b3fcbe680..4d2a1db1e1 100644 --- a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroTextParser.scala +++ b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroInlineTypeParser.scala @@ -2,9 +2,9 @@ package amf.apicontract.internal.spec.avro.parser.domain import amf.apicontract.internal.spec.avro.parser.context.AvroSchemaContext import amf.shapes.client.scala.model.domain.AnyShape -import org.yaml.model.{YMap, YNode, YScalar} +import org.yaml.model.{YMap, YNode, YScalar, YSequence} -case class AvroTextParser(node: YNode)(implicit ctx: AvroSchemaContext) extends AvroKeyExtractor { +case class AvroInlineTypeParser(node: YNode)(implicit ctx: AvroSchemaContext) extends AvroKeyExtractor { def parse(): AnyShape = { node.value match { case scalar: YScalar => @@ -12,7 +12,10 @@ case class AvroTextParser(node: YNode)(implicit ctx: AvroSchemaContext) extends val parsedShape = AvroTextTypeParser(scalar.text, None).parse() annotatedAvroShape(parsedShape, avroType, node) case map: YMap => // todo: putting an empty AnyShape when the union type is incorrect is kinda weird behavior - new AvroShapeParser(map).parse().getOrElse(AnyShape()) + new AvroShapeParser(map).parse().getOrElse(AnyShape(map)) + case seq: YSequence => // The only valid case here for a seq is a Union + AvroInlineUnionShapeParser(seq).parse() + case _ => AnyShape(node) } } } diff --git a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroUnionShapeParser.scala b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroUnionShapeParser.scala index 5d3ce1726a..3338e4a9d0 100644 --- a/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroUnionShapeParser.scala +++ b/amf-api-contract/shared/src/main/scala/amf/apicontract/internal/spec/avro/parser/domain/AvroUnionShapeParser.scala @@ -3,16 +3,34 @@ package amf.apicontract.internal.spec.avro.parser.domain import amf.apicontract.internal.spec.avro.parser.context.AvroSchemaContext import amf.core.client.scala.model.domain.AmfArray import amf.core.internal.parser.domain.Annotations -import amf.shapes.client.scala.model.domain.UnionShape +import amf.shapes.client.scala.model.domain.{AnyShape, UnionShape} import amf.shapes.internal.domain.metamodel.UnionShapeModel -import org.yaml.model.{YMap, YNode} +import org.yaml.model.{YMap, YNode, YSequence, YValue} case class AvroUnionShapeParser(members: Seq[YNode], node: YNode)(implicit ctx: AvroSchemaContext) - extends AvroComplexShapeParser(node.as[YMap]) { + extends AvroComplexShapeParser(node.as[YMap]) + with AvroUnionLikeShapeParser { override val shape: UnionShape = UnionShape(node).withSynthesizeName("union") override def parseSpecificFields(): Unit = { - val parsedMembers = members.map(node => AvroTextParser(node).parse()) + val parsedMembers = parseMembers(members) shape.setWithoutId(UnionShapeModel.AnyOf, AmfArray(parsedMembers, Annotations(node)), Annotations(node)) } } + +case class AvroInlineUnionShapeParser(seq: YSequence)(implicit ctx: AvroSchemaContext) + extends AvroUnionLikeShapeParser { + + def parse(): AnyShape = { + val shape: UnionShape = UnionShape(seq).withSynthesizeName("union") + val parsedMembers = parseMembers(seq.nodes) + shape.setWithoutId(UnionShapeModel.AnyOf, AmfArray(parsedMembers, Annotations(seq)), Annotations(seq)) + } + +} + +trait AvroUnionLikeShapeParser { + def parseMembers(members: Seq[YNode])(implicit ctx: AvroSchemaContext): Seq[AnyShape] = { + members.map(node => AvroInlineTypeParser(node).parse()) + } +} diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.js b/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.js new file mode 100644 index 0000000000..5874bd0a1c --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.js @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: Internal error during Avro validation: Error: invalid "null": 1234 + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json#/shape/unionNullableRecord + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.jvm b/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.jvm new file mode 100644 index 0000000000..015bd5cae9 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-nullable-record-invalid.report.jvm @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: invalid schema type: Invalid default for field nullableUnion: 1234 not a ["null","string"] + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json#/shape/unionNullableRecord + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.js b/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.js new file mode 100644 index 0000000000..d2aa6da282 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.js @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json +Profile: Avro +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/validation#example-validation-error + Message: '132' is not a valid value (of type '"string"') for '0.string' + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json#/array/default-array/array_1 + Property: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json#/array/default-array/array_1 + Range: [(4,13)-(4,49)] + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.jvm b/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.jvm new file mode 100644 index 0000000000..7311d7b0be --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-simple-array-invalid.report.jvm @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json +Profile: Avro +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/validation#schema-exception + Message: invalid schema type: Expected string. Got VALUE_NUMBER_INT + Severity: Violation + Target: null + Property: Expected string. Got VALUE_NUMBER_INT + Range: [(1,0)-(5,1)] + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.js b/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.js new file mode 100644 index 0000000000..bfddd527e8 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.js @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: Internal error during Avro validation: Error: invalid "string": true + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json#/shape/unionSimpleRecord + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.jvm b/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.jvm new file mode 100644 index 0000000000..1e18d0decb --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-simple-record-invalid.report.jvm @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: invalid schema type: Invalid default for field simpleUnion: true not a ["string","int"] + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json#/shape/unionSimpleRecord + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json new file mode 100644 index 0000000000..d971335406 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-invalid.json @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "unionNullableRecord", + "namespace": "root", + "fields": [ + { + "name": "nullableUnion", + "type": [ "null", "string"], + "default": 1234 + } + ] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-valid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-valid.json new file mode 100644 index 0000000000..202b4163e0 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-nullable-record-valid.json @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "unionNullableRecord", + "namespace": "root", + "fields": [ + { + "name": "nullableUnion", + "type": [ "null", "string"], + "default": null + } + ] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json new file mode 100644 index 0000000000..3f31ef2aaf --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-invalid.json @@ -0,0 +1,5 @@ +{ + "type": "array", + "items": [ "string", "int"], + "default": [{"string": 132}, {"string": "b"}] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-valid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-valid.json new file mode 100644 index 0000000000..22ba625473 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-array-valid.json @@ -0,0 +1,5 @@ +{ + "type": "array", + "items": [ "string", "int"], + "default": [{"string": "a"}, {"string": "b"}] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json new file mode 100644 index 0000000000..9138f29b56 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-invalid.json @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "unionSimpleRecord", + "namespace": "root", + "fields": [ + { + "name": "simpleUnion", + "type": [ "string", "int"], + "default": true + } + ] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-valid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-valid.json new file mode 100644 index 0000000000..82c22992e2 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-valid.json @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "unionSimpleRecord", + "namespace": "root", + "fields": [ + { + "name": "simpleUnion", + "type": [ "string", "int"], + "default": "something" + } + ] +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala b/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala index a99ac9dcf8..b1e0197d9e 100644 --- a/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala +++ b/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala @@ -165,6 +165,55 @@ class AvroSchemaValidationTest extends MultiPlatformReportGenTest { ) } + test("validate simple union in record - valid") { + validate( + "union-simple-record-valid.json", + None, + configOverride = Some(config) + ) + } + + test("validate simple union in record - invalid") { + validate( + "union-simple-record-invalid.json", + Some("union-simple-record-invalid.report"), + configOverride = Some(config) + ) + } + + test("validate nullable union in record - valid") { + validate( + "union-nullable-record-valid.json", + None, + configOverride = Some(config) + ) + } + + test("validate nullable union in record - invalid") { + validate( + "union-nullable-record-invalid.json", + Some("union-nullable-record-invalid.report"), + configOverride = Some(config) + ) + } + + test("validate simple union in array - valid") { + validate( + "union-simple-array-valid.json", + None, + configOverride = Some(config) + ) + } + + test("validate simple union in array - invalid") { + validate( + "union-simple-array-invalid.json", + Some("union-simple-array-invalid.report"), + configOverride = Some(config) + ) + } + + // TODO We need to see how implement this in with AVRO 1.11.3 // if (platform.name == "jvm") { // We were able only to change this behavior in JVM validator. JS one is still strict (only letter, numbers and '_') diff --git a/amf-cli/shared/src/test/scala/amf/client/validation/PayloadValidationTest.scala b/amf-cli/shared/src/test/scala/amf/client/validation/PayloadValidationTest.scala index c3fe703565..4f94de3ad0 100644 --- a/amf-cli/shared/src/test/scala/amf/client/validation/PayloadValidationTest.scala +++ b/amf-cli/shared/src/test/scala/amf/client/validation/PayloadValidationTest.scala @@ -1,8 +1,8 @@ package amf.client.validation +import amf.apicontract.client.scala.AvroConfiguration import amf.cli.internal.convert.NativeOps -import amf.client.validation.AvroTestSchemas.recordSchema -import amf.core.client.common.validation.{ScalarRelaxedValidationMode, StrictValidationMode} +import amf.core.client.common.validation.{ScalarRelaxedValidationMode, SeverityLevels, StrictValidationMode} import amf.core.client.platform.model.DataTypes import amf.core.client.scala.AMFGraphConfiguration import amf.core.client.scala.model.domain.extensions.PropertyShape @@ -11,6 +11,7 @@ import amf.core.client.scala.validation.AMFValidationReport import amf.core.client.scala.validation.payload.{AMFShapePayloadValidationPlugin, AMFShapePayloadValidator} import amf.core.internal.remote.Mimes._ import amf.shapes.client.scala.ShapesConfiguration +import amf.shapes.client.scala.model.document.AvroSchemaDocument import amf.shapes.client.scala.model.domain._ import amf.shapes.client.scala.plugin.FailFastJsonSchemaPayloadValidationPlugin import amf.shapes.internal.annotations.{AVRORawSchema, AVROSchemaType} @@ -325,6 +326,36 @@ trait PayloadValidationTest extends AsyncFunSuite with NativeOps with Matchers w } } + test("avro union validation should return a warning") { + val schema = "file://amf-cli/shared/src/test/resources/avro/schemas/union-simple-record-valid.json" + val payload = "{}" + + val client = AvroConfiguration.Avro().baseUnitClient() + + for { + parseResult <- client.parse(schema) + validationResult <- client.validate(parseResult.baseUnit) + } yield { + // Warning in the unit validation report are filtered by UnitPayloadsValidation, so no warning here + assert(parseResult.conforms) + assert(parseResult.results.isEmpty) + assert(validationResult.conforms) + assert(parseResult.results.isEmpty) + + val doc = parseResult.baseUnit.asInstanceOf[AvroSchemaDocument] + val record = doc.encodes.asInstanceOf[NodeShape] + val union = record.properties.head.range + assert(union.isInstanceOf[UnionShape] && union.asInstanceOf[AnyShape].avroSchemaType.contains("union")) + + val validator = payloadValidator(union, `application/json`) + val report = validator.syncValidate(payload) + assert(report.conforms) + assert(report.results.size == 1) + assert(report.results.head.severityLevel == SeverityLevels.WARNING) + assert(report.results.head.message.contains("Cannot validate union schema kind payloads")) + } + } + protected def makeAvroShape(raw: String, kind: String, base: AnyShape): AnyShape = { base.annotations += AVROSchemaType(kind) base.annotations += AVRORawSchema(raw) @@ -365,7 +396,7 @@ object AvroTestSchemas { |} """.stripMargin - val int = + val int: String = """ |{ | "type": "int", diff --git a/amf-shapes/shared/src/main/scala/amf/shapes/internal/validation/avro/BaseAvroSchemaPayloadValidator.scala b/amf-shapes/shared/src/main/scala/amf/shapes/internal/validation/avro/BaseAvroSchemaPayloadValidator.scala index 7c99032aa7..b087778d67 100644 --- a/amf-shapes/shared/src/main/scala/amf/shapes/internal/validation/avro/BaseAvroSchemaPayloadValidator.scala +++ b/amf-shapes/shared/src/main/scala/amf/shapes/internal/validation/avro/BaseAvroSchemaPayloadValidator.scala @@ -145,48 +145,66 @@ abstract class BaseAvroSchemaPayloadValidator( payload: (Option[LoadedObj], Option[PayloadParsingResult]), validationProcessor: ValidationProcessor ): AMFValidationReport = { - payload match { - case (_, Some(result)) if result.hasError => validationProcessor.processResults(result.results) - case (Some(obj), resultOption) => - val fragmentOption = resultOption.map(_.fragment) - try { - { - resultOption match { - case _ if shape.isInstanceOf[AnyShape] => - getOrCreateSchema(shape.asInstanceOf[AnyShape], validationProcessor) - case _ => - Left( - validationProcessor.processResults( - Seq( - AMFValidationResult( - "Cannot validate shape that is not an any shape", - defaultSeverity, - "", - Some(shape.id), - ExampleValidationErrorSpecification.id, - shape.position(), - shape.location(), - null + shape match { + case s: AnyShape if s.avroSchemaType.contains("union") => + validationProcessor.processResults( + Seq( + AMFValidationResult( + "Cannot validate union schema kind payloads", + SeverityLevels.WARNING, + "", + Some(shape.id), + ExampleValidationErrorSpecification.id, + shape.position(), + shape.location(), + null + ) + ) + ) + case _ => + payload match { + case (_, Some(result)) if result.hasError => validationProcessor.processResults(result.results) + case (Some(obj), resultOption) => + val fragmentOption = resultOption.map(_.fragment) + try { + { + resultOption match { + case _ if shape.isInstanceOf[AnyShape] => + getOrCreateSchema(shape.asInstanceOf[AnyShape], validationProcessor) + case _ => + Left( + validationProcessor.processResults( + Seq( + AMFValidationResult( + "Cannot validate shape that is not an any shape", + defaultSeverity, + "", + Some(shape.id), + ExampleValidationErrorSpecification.id, + shape.position(), + shape.location(), + null + ) + ) ) ) - ) - ) + } + } match { + case Right(Some(schema)) => // Schema obtained successfully, calling validator with it + callValidator(schema, obj, fragmentOption, validationProcessor) + case Left(result) => // Error occurred during schema generation, returning that result + result + case _ => // No schema or payload error, returning empty results + validationProcessor.processResults(Nil) + } + } catch { + case e: Exception => + validationProcessor.processException(e, fragmentOption.map(_.encodes)) } - } match { - case Right(Some(schema)) => // Schema obtained successfully, calling validator with it - callValidator(schema, obj, fragmentOption, validationProcessor) - case Left(result) => // Error occurred during schema generation, returning that result - result - case _ => // No schema or payload error, returning empty results - validationProcessor.processResults(Nil) - } - } catch { - case e: Exception => - validationProcessor.processException(e, fragmentOption.map(_.encodes)) - } - case _ => - validationProcessor.processResults(Nil) // ignore + case _ => + validationProcessor.processResults(Nil) // ignore + } } } From 25a12eb53d5f1f2692a5f4eda7c3892d29e02342 Mon Sep 17 00:00:00 2001 From: Loose Date: Wed, 25 Sep 2024 18:46:30 -0300 Subject: [PATCH 4/6] W-16772136 - Add test to validate that Avro Unions at root level are invalid --- .../avro/reports/union-root-invalid.report.js | 14 ++++++++++++++ .../avro/reports/union-root-invalid.report.jvm | 14 ++++++++++++++ .../resources/avro/schemas/union-root-invalid.json | 5 +++++ .../scala/amf/avro/AvroSchemaValidationTest.scala | 8 ++++++++ 4 files changed, 41 insertions(+) create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.js create mode 100644 amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.jvm create mode 100644 amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.js b/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.js new file mode 100644 index 0000000000..f71cbd7da9 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.js @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: Internal error during Avro validation: Error: unknown type: ["string","int"] + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json#/union/simpleUnion + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.jvm b/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.jvm new file mode 100644 index 0000000000..3bb298ed61 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/reports/union-root-invalid.report.jvm @@ -0,0 +1,14 @@ +ModelId: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json +Profile: +Conforms: false +Number of results: 1 + +Level: Violation + +- Constraint: http://a.ml/vocabularies/amf/parser#invalid-avro-schema + Message: No type: {"name":"simpleUnion","type":["string","int"],"default":"something"} + Severity: Violation + Target: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json#/union/simpleUnion + Property: + Range: + Location: file://amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json diff --git a/amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json b/amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json new file mode 100644 index 0000000000..d62998f373 --- /dev/null +++ b/amf-cli/shared/src/test/resources/avro/schemas/union-root-invalid.json @@ -0,0 +1,5 @@ +{ + "name": "simpleUnion", + "type": [ "string", "int"], + "default": "something" +} \ No newline at end of file diff --git a/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala b/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala index b1e0197d9e..0aae235df7 100644 --- a/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala +++ b/amf-cli/shared/src/test/scala/amf/avro/AvroSchemaValidationTest.scala @@ -213,6 +213,14 @@ class AvroSchemaValidationTest extends MultiPlatformReportGenTest { ) } + test("validate union at root level is invalid") { + validate( + "union-root-invalid.json", + Some("union-root-invalid.report"), + configOverride = Some(config) + ) + } + // TODO We need to see how implement this in with AVRO 1.11.3 From fc5cc882bf6f5cf7c108123ca9271960900d7de8 Mon Sep 17 00:00:00 2001 From: Loose Date: Wed, 25 Sep 2024 18:47:22 -0300 Subject: [PATCH 5/6] W-16772136 - Updates in avro adr based on last changes --- adrs/0014-avro-support.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/adrs/0014-avro-support.md b/adrs/0014-avro-support.md index b7979acf32..0889ff9fdb 100644 --- a/adrs/0014-avro-support.md +++ b/adrs/0014-avro-support.md @@ -10,7 +10,7 @@ Accepted ## Context -Async 2.x supports AVRO Schemas, and we currently don't. +Async 2.x supports AVRO Schemas, and we currently don't. We want to add support of AVRO Schemas: - inside Async APIs - as a standalone document @@ -54,12 +54,12 @@ We've also added 3 AVRO-specific fields to the `AnyShape` Model via the `AvroFie ### Where we support and DON'T support AVRO Schemas We Support AVRO Schemas (inline or inside a `$ref`): -- as a standalone document or file - - we encourage users to use the `.avsc` file type to indicate that's an avro file, for better suggestions and so on in the platform) +- as a standalone document or file + - we encourage users to use the `.avsc` file type to indicate that's an avro file - must use the specific `AvroConfiguration` - inside a message payload in an AsyncAPI - the key `schemaFormat` MUST be declared and specify it's an AVRO payload - - we only support avro schema version 1.9.0 + - **we only support avro schema version 1.9.0** - the avro specific document `AvroSchemaDocument` can only be parsed with the specific `AvroConfiguration` We don't support AVRO Schemas: @@ -71,16 +71,16 @@ We'll use the Apache official libraries for JVM and JS, in the version 1.11.3, d ## Consequences / Constraints -The validation plugins differ in interfaces and implementations, and each has some constraints: +The validation libraries differ in interfaces and implementations, and each has some constraints: -### JVM avro validation plugin +### JVM avro validation library - validation per se is not supported, we try to parse an avro schema and throw parsing results if there are any - this means it's difficult to have location of where the error is thrown, we may give an approximate location from our end post-validation - when a validation is thrown, the rest of the file is not being searched for more validations - this is particularly important in large avro schemas, where many errors can be found but only one is shown -### Both JVM & JS validation plugins +### Both JVM & JS validation libraries - `"default"` values are not being validated when the type is `bytes`, `map`, or `array` - the validator treats as invalid an empty array as the default value for arrays (`"default": []`) even though the [Avro Schema Specification](https://avro.apache.org/docs/1.12.0/specification) has some examples with it -- if an avro record has a field that is a union that includes the root record itself (recursive reference) we fail to validate payloads (default value or payload validation) against that 'recursive union field' correctly. The root record works correctly. In the future we'll try to ignore the cases that we are now failing and/or show a warning instead. +- avro schemas of type union are not valid at the root level of the schema, but they are accepted as field types in a record or items in an array. For this reason, it is not possible to generate a `PayloadValidator` for an avro union shape. - the avro libraries are very restrictives with the allowed characters in naming definition (names of records for example). They only allow letters, numbers and `_` chars. We are not modifying this behavior. \ No newline at end of file From e991687ec93074f53a8d6f466efc080f8f67f3cb Mon Sep 17 00:00:00 2001 From: Loose Date: Wed, 25 Sep 2024 19:17:32 -0300 Subject: [PATCH 6/6] W-16837997 - Publish 5.6.2 --- amf-apicontract.versions | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/amf-apicontract.versions b/amf-apicontract.versions index 03e685d2ce..fa06bb2b94 100644 --- a/amf-apicontract.versions +++ b/amf-apicontract.versions @@ -1,5 +1,5 @@ -amf.apicontract=5.7.0-SNAPSHOT -amf.aml=6.7.0-SNAPSHOT +amf.apicontract=5.6.2 +amf.aml=6.6.0 amf.model=3.9.0 antlr4Version=0.7.25 amf.validation.profile.dialect=1.6.0