simplified assumptions about product schemas, added test for product …

…property checking
softwaremill · Apr 22, 2024 · 4c5e88c · 4c5e88c
1 parent 1e291b8
commit 4c5e88c
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 19 deletions.
diff --git a/apispec-model/src/main/scala/sttp/apispec/validation/SchemaComparator.scala b/apispec-model/src/main/scala/sttp/apispec/validation/SchemaComparator.scala
@@ -114,10 +114,7 @@ private class SchemaComparator(
       checkType(writerSchema, readerSchema).toList ++
         checkRequiredProperties(writerSchema, readerSchema).toList ++
         checkDependentRequired(writerSchema, readerSchema) ++
-        propIssues ++
-        checkPropertyNames(writerSchema, readerSchema).toList ++
-        checkAdditionalProperties(writerSchema, readerSchema).toList ++
-        checkMinMaxProperties(writerSchema, readerSchema).toList
+        propIssues
 
     } else if (isCoproductSchema(writerSchema) && isCoproductSchema(readerSchema) &&
       // if readerSchema does not have a discriminator, we fall back to GeneralSchemaMismatch
@@ -167,9 +164,9 @@ private class SchemaComparator(
         prefixItemsIssues
 
     } else if (isMapSchema(writerSchema) && isMapSchema(readerSchema)) {
-      checkAdditionalProperties(writerSchema, readerSchema).toList ++
+      checkType(writerSchema, readerSchema).toList ++
+        checkAdditionalProperties(writerSchema, readerSchema).toList ++
         checkPropertyNames(writerSchema, readerSchema).toList ++
-        checkType(writerSchema, readerSchema).toList ++
         checkMinMaxProperties(writerSchema, readerSchema).toList
 
     } else if (readerSchema == Schema.Nothing) {
@@ -230,7 +227,6 @@ private class SchemaComparator(
       s.prefixItems.isDefined &&
       s == Schema(
         `type` = s.`type`,
-        items = s.items,
         prefixItems = s.prefixItems,
         maxItems = s.maxItems,
         minItems = s.minItems,
@@ -252,13 +248,9 @@ private class SchemaComparator(
       s.properties.nonEmpty &&
       s == Schema(
         `type` = s.`type`,
-        propertyNames = s.propertyNames,
         properties = s.properties,
         required = s.required,
         dependentRequired = s.dependentRequired,
-        additionalProperties = s.additionalProperties,
-        maxProperties = s.maxProperties,
-        minProperties = s.minProperties
       )
 
   // coproduct schema is a schema with `oneOf` or `anyOf` of pure references, with an optional discriminator object
@@ -423,13 +415,13 @@ private class SchemaComparator(
     else Some(MinMaxPropertiesMismatch(writerBounds, readerBounds))
   }
 
-  private def checkRequiredProperties(writerSchema: Schema, readerSchema: Schema): Option[MoreRequiredProperties] = {
+  private def checkRequiredProperties(writerSchema: Schema, readerSchema: Schema): Option[MissingRequiredProperties] = {
     val newRequired = readerSchema.required.toSet -- writerSchema.required.toSet
     if (newRequired.isEmpty) None
-    else Some(MoreRequiredProperties(newRequired))
+    else Some(MissingRequiredProperties(newRequired))
   }
 
-  private def checkDependentRequired(writerSchema: Schema, readerSchema: Schema): List[MoreDependentRequired] =
+  private def checkDependentRequired(writerSchema: Schema, readerSchema: Schema): List[MissingDependentRequiredProperties] =
     readerSchema.dependentRequired.toList.flatMap { case (property, required) =>
       // if the writer schema does not require or define a schema for this property, we assume that it will never
       // be passed (even if it's implicitly allowed by patternProperties/additionalProperties)
@@ -442,7 +434,7 @@ private class SchemaComparator(
       writerRequired.flatMap { wr =>
         val moreRequired = required.toSet -- wr.toSet
         if (moreRequired.isEmpty) None
-        else Some(MoreDependentRequired(property, moreRequired))
+        else Some(MissingDependentRequiredProperties(property, moreRequired))
       }
     }
 

diff --git a/apispec-model/src/main/scala/sttp/apispec/validation/SchemaCompatibilityIssue.scala b/apispec-model/src/main/scala/sttp/apispec/validation/SchemaCompatibilityIssue.scala
@@ -125,13 +125,13 @@ case class MinMaxPropertiesMismatch(
     s"target object size (minProperties/maxProperties) $readerBounds is stricter than $writerBounds"
 }
 
-case class MoreRequiredProperties(
+case class MissingRequiredProperties(
   newRequiredProperties: Set[String]
 ) extends SchemaCompatibilityIssue {
-  def description: String = s"target schema introduced new required properties: ${newRequiredProperties.mkString(", ")}"
+  def description: String = s"target schema requires properties: ${newRequiredProperties.mkString(", ")}"
 }
 
-case class MoreDependentRequired(
+case class MissingDependentRequiredProperties(
   property: String,
   newRequiredProperties: Set[String]
 ) extends SchemaCompatibilityIssue {

diff --git a/apispec-model/src/test/scala/sttp/apispec/validation/SchemaComparatorTest.scala b/apispec-model/src/test/scala/sttp/apispec/validation/SchemaComparatorTest.scala
@@ -9,6 +9,7 @@ class SchemaComparatorTest extends AnyFunSuite {
   private val stringSchema = Schema(SchemaType.String)
   private val integerSchema = Schema(SchemaType.Integer)
   private val numberSchema = Schema(SchemaType.Number)
+  private val booleanSchema = Schema(SchemaType.Boolean)
 
   // A schema with internal structure currently not understood by SchemaComparator.
   // Such Schema can only be compared for equality (this may change in the future as SchemaComparator is improved).
@@ -128,7 +129,7 @@ class SchemaComparatorTest extends AnyFunSuite {
   test("recursive schemas") {
     assert(compare(writerTreeSchema, readerTreeSchema) == Nil)
     assert(compare(writerTreeSchema, strictReaderTreeSchema) == List(
-      MoreRequiredProperties(Set("value"))
+      MissingRequiredProperties(Set("value"))
     ))
   }
 
@@ -328,5 +329,36 @@ class SchemaComparatorTest extends AnyFunSuite {
     ))
   }
 
+  test("product schema properties") {
+    assert(compare(
+      Schema(SchemaType.Object).copy(
+        properties = ListMap(
+          "a" -> stringSchema,
+          "b" -> integerSchema,
+          "c" -> booleanSchema,
+          "d" -> stringSchema,
+        ),
+        required = List("a", "b", "d")
+      ),
+      Schema(SchemaType.Object).copy(
+        properties = ListMap(
+          "a" -> stringSchema,
+          "b" -> numberSchema,
+          "c" -> stringSchema,
+          "e" -> stringSchema,
+          "f" -> stringSchema,
+        ),
+        required = List("a", "b", "e"),
+        dependentRequired = ListMap("c" -> List("f"))
+      ),
+    ) == List(
+      MissingRequiredProperties(Set("e")),
+      MissingDependentRequiredProperties("c", Set("f")),
+      IncompatibleProperty("c", List(
+        TypeMismatch(List(SchemaType.Boolean), List(SchemaType.String))
+      )),
+    ))
+  }
+
   //TODO significantly more tests
 }