Merge pull request #1245 from zalando/fix-rule-240

Improve validation for Rule 240

server/zally-core/src/main/kotlin/org/zalando/zally/core/util/OpenApiUtil.kt

@@ -47,19 +47,21 @@ fun OpenAPI.getAllHeaders(): Set<HeaderElement> {
  */
 fun OpenAPI.getAllSchemas(): Collection<Schema<Any>> = this.components?.schemas.orEmpty().values +
     this.components?.parameters.orEmpty().mapNotNull { it.value.schema } +
-    this.components?.responses.orEmpty().values.flatMap { it.content.orEmpty().values.mapNotNull { it.schema } } +
-    this.components?.requestBodies.orEmpty().values.flatMap { it.content.orEmpty().values.mapNotNull { it.schema } } +
+    this.components?.responses.orEmpty().values.flatMap { it.content.orEmpty().values.mapNotNull { v -> v.schema } } +
+    this.components?.requestBodies.orEmpty().values.flatMap { it.content.orEmpty().values.mapNotNull { v -> v.schema } } +
     this.paths.orEmpty().flatMap {
-        it.value?.readOperations().orEmpty().flatMap { it.parameters.orEmpty().mapNotNull { it.schema } }
+        it.value?.readOperations().orEmpty()
+            .flatMap { operation -> operation.parameters.orEmpty().mapNotNull { parameter -> parameter.schema } }
     } +
     this.paths.orEmpty().flatMap {
-        it.value?.readOperations().orEmpty().flatMap {
-            it.responses.orEmpty().flatMap { it.value.content.orEmpty().values.mapNotNull { it.schema } }
+        it.value?.readOperations().orEmpty().flatMap { operation ->
+            operation.responses.orEmpty()
+                .flatMap { response -> response.value.content.orEmpty().values.mapNotNull { v -> v.schema } }
         }
     } +
     this.paths.orEmpty().flatMap {
         it.value?.readOperations().orEmpty()
-            .flatMap { it.requestBody?.content.orEmpty().values.mapNotNull { it.schema } }
+            .flatMap { readOps -> readOps.requestBody?.content.orEmpty().values.mapNotNull { v -> v.schema } }
     }
 
 /**
@@ -158,7 +160,7 @@ fun Schema<Any>.isEnum(): Boolean = this.enum?.isNotEmpty() ?: false
 fun Schema<Any>.isExtensibleEnum(): Boolean =
     this.extensions?.containsKey("x-extensible-enum") ?: false
 
-fun Schema<Any>.extensibleEnum(): List<String> =
+fun Schema<Any>.extensibleEnum(): List<Any?> =
     if (this.isExtensibleEnum()) {
-        (this.extensions["x-extensible-enum"] as List<*>).map { it.toString() }
-    } else emptyList()
+        (this.extensions["x-extensible-enum"] as List<Any?>)
+    } else emptyList<Any?>()

server/zally-ruleset-zalando/src/main/kotlin/org/zalando/zally/ruleset/zalando/UpperCaseEnums.kt

@@ -26,32 +26,33 @@ class UpperCaseEnums {
 
     @Check(severity = Severity.SHOULD)
     fun validate(context: Context): List<Violation> =
-        context.api.getAllSchemas()
-            .filter { it.type == "string" }
-            .filter { it.isExtensibleEnum() || it.isEnum() }
-            .flatMap {
-                if (it.isExtensibleEnum()) {
-                    validateExtensibleEnum(it)
-                } else {
-                    validateEnum(it)
-                }.map { enumValue -> context.violation("$description. Incorrect value: $enumValue", it) }
-            } +
-            context.api.getAllProperties()
-                .filter { it.value.type == "string" }
-                .filter { it.value.isExtensibleEnum() || it.value.isEnum() }
-                .flatMap { (propName, property) ->
-                    if (property.isExtensibleEnum()) {
-                        validateExtensibleEnum(property)
-                    } else {
-                        validateEnum(property)
-                    }.map {
-                        context.violation("$propName: $description. Incorrect value: $it", it)
-                    }
-                }
-
-    private fun validateExtensibleEnum(property: Schema<Any>): List<String> =
-        property.extensibleEnum().filter { !pattern.matches(it) }
-
-    private fun validateEnum(property: Schema<Any>): List<String> =
-        property.enum.map { it.toString() }.filter { !pattern.matches(it) }
+        validatePrimitiveSchemas(context) + validateAllProperties(context)
+
+    private fun validateAllProperties(context: Context): List<Violation> = context.api.getAllProperties()
+        .filter { it.value.type == "string" }
+        .filter { it.value.isExtensibleEnum() || it.value.isEnum() }
+        .flatMap { (_, property) ->
+            validateEnum(property, context)
+        }
+
+    private fun validatePrimitiveSchemas(context: Context): List<Violation> = context.api.getAllSchemas()
+        .filter { it.type == "string" }
+        .filter { it.isExtensibleEnum() || it.isEnum() }
+        .flatMap { validateEnum(it, context) }
+
+    private fun validateEnum(scheme: Schema<Any>, context: Context): List<Violation> {
+        val enumValues = if (scheme.isExtensibleEnum()) {
+            scheme.extensibleEnum()
+        } else scheme.enum
+
+        return enumValues.filterNotNull().filterNot { it is String }.map {
+            context.violation(
+                "${scheme.name}: Enum value type ${it.javaClass.simpleName} is not valid. The expected type is string",
+                scheme
+            )
+        } +
+            enumValues.filter { it is String }.map { it.toString() }.filterNot { pattern.matches(it) }.map {
+                context.violation("${scheme.name}: $description. Incorrect value: $it", scheme)
+            }
+    }
 }

server/zally-ruleset-zalando/src/test/kotlin/org/zalando/zally/ruleset/zalando/UpperCaseEnumsTest.kt

@@ -96,19 +96,12 @@ class UpperCaseEnumsTest {
         val spec = """
             openapi: 3.0.1
             paths:
-              /article:
+              '/job-statistics/latest':
                 get:
-                  parameters:
-                    - name: country
-                      in: query
-                      schema:
-                        type: string
-                        enum:
-                          - GERMANY
-                          - SWEDEN
-                          - PUT_YOUR_COUNTRY_HERE
-                          - COUNTRY_1
-                  responses: 
+                  summary: get API monitoring statistics
+                  description: Returns all statistics about monitored API endpoints.
+                  operationId: 'getMonitoringStatistics'
+                  responses:
                     200:
                       description: The identifiers associated with the source id.
                       content: 
@@ -129,4 +122,40 @@ class UpperCaseEnumsTest {
         val violations = rule.validate(context)
         assertThat(violations).isEmpty()
     }
+
+    @Test
+    fun `should return violations if the type of the property doesn't match with enum type values`() {
+        val spec = """
+            openapi: 3.0.1
+            paths:
+              '/job-statistics/latest':
+                get:
+                  summary: get API monitoring statistics
+                  description: Returns all statistics about monitored API endpoints.
+                  operationId: 'getMonitoringStatistics'
+                  responses:
+                    200:
+                      description: The identifiers associated with the source id.
+                      content: 
+                        application/json:
+                          schema:
+                            type: object
+                            properties:
+                              invalid-string-prop-1:
+                                type: string
+                                x-extensible-enum:
+                                  - ON
+                                  - OFF                                 
+                              invalid-string-prop-2:
+                                type: string
+                                x-extensible-enum:
+                                  - 1
+                                  - 2                                 
+        """.trimIndent()
+
+        val context = DefaultContextFactory().getOpenApiContext(spec)
+
+        val violations = rule.validate(context)
+        assertThat(violations).hasSize(4)
+    }
 }