feat(server): migrate snake_case in prop names rule to context object to support openapi 3

cli/zally/integration_test.go

@@ -53,13 +53,13 @@ func TestIntegrationWithLocalYamlFile(t *testing.T) {
 	t.Run("integrationWithLocalYamlFile", func(t *testing.T) {
 		out, e := RunAppAndCaptureOutput([]string{"", "lint", "../../server/src/test/resources/fixtures/api_spa.yaml"})
 
-		assert.Contains(t, out, "MUST violations: 12")
+		assert.Contains(t, out, "MUST violations: 24")
 		assert.Contains(t, out, "SHOULD violations: 1")
 		assert.Contains(t, out, "MAY violations: 6")
 		assert.Contains(t, out, "HINT violations: 0")
 
 		assert.NotNil(t, e)
-		assert.Equal(t, "Failing because: 12 must violation(s) found", e.Error())
+		assert.Equal(t, "Failing because: 24 must violation(s) found", e.Error())
 	})
 }
 
@@ -96,7 +96,7 @@ func TestIntegrationWithRemoteYamlFile(t *testing.T) {
 		defer ts.Close()
 		out, e := RunAppAndCaptureOutput([]string{"", "lint", ts.URL + "/api_spa.yaml"})
 
-		assert.Contains(t, out, "MUST violations: 12")
+		assert.Contains(t, out, "MUST violations: 24")
 		assert.Contains(t, out, "SHOULD violations: 1")
 		assert.Contains(t, out, "MAY violations: 6")
 		assert.Contains(t, out, "HINT violations: 0")

server/src/main/java/de/zalando/zally/rule/zalando/SnakeCaseInPropNameRule.kt

@@ -2,35 +2,28 @@ package de.zalando.zally.rule.zalando
 
 import com.typesafe.config.Config
 import de.zalando.zally.rule.api.Check
+import de.zalando.zally.rule.api.Context
 import de.zalando.zally.rule.api.Rule
 import de.zalando.zally.rule.api.Severity
 import de.zalando.zally.rule.api.Violation
 import de.zalando.zally.util.PatternUtil
-import de.zalando.zally.util.getAllJsonObjects
-import io.swagger.models.Swagger
+import de.zalando.zally.util.getAllProperties
 import org.springframework.beans.factory.annotation.Autowired
 
 @Rule(
-        ruleSet = ZalandoRuleSet::class,
-        id = "118",
-        severity = Severity.MUST,
-        title = "snake_case property names"
+    ruleSet = ZalandoRuleSet::class,
+    id = "118",
+    severity = Severity.MUST,
+    title = "Property Names Must be ASCII snake_case"
 )
 class SnakeCaseInPropNameRule(@Autowired rulesConfig: Config) {
-    private val description = "Property names must be snake_case: "
+    private val description = "Property name has to be snake_case"
 
     private val whitelist = rulesConfig.getStringList(SnakeCaseInPropNameRule::class.simpleName + ".whitelist").toSet()
 
     @Check(severity = Severity.MUST)
-    fun validate(swagger: Swagger): Violation? {
-        val result = swagger.getAllJsonObjects().flatMap { (def, path) ->
-            val badProps = def.keys.filterNot { PatternUtil.isSnakeCase(it) || whitelist.contains(it) }
-            if (badProps.isNotEmpty()) listOf(badProps to path) else emptyList()
-        }
-        return if (result.isNotEmpty()) {
-            val (props, paths) = result.unzip()
-            val properties = props.flatten().toSet().joinToString(", ")
-            Violation(description + properties, paths)
-        } else null
-    }
+    fun checkPropertyNames(context: Context): List<Violation> =
+        context.api.getAllProperties()
+            .filterNot { (name, _) -> PatternUtil.isSnakeCase(name) || whitelist.contains(name) }
+            .map { context.violation(description, it.value) }
 }

server/src/main/java/de/zalando/zally/util/OpenApiUtil.kt

@@ -79,6 +79,30 @@ fun OpenAPI.getAllTransitiveSchemas(): Set<Schema<Any>> {
     return collector
 }
 
+/**
+ * Traverses the schemas and returns all included properties and their names
+ * @return a map (name -> schema) of all transitive properties
+ */
+fun OpenAPI.getAllProperties(): Map<String, Schema<Any>> {
+    fun isPrimitive(schema: Schema<Any>): Boolean = schema.properties.orEmpty().isEmpty()
+    val collector = mutableMapOf<String, Schema<Any>>()
+
+    tailrec fun traverseSchemas(schemasToScan: Collection<Schema<Any>>) {
+        if (schemasToScan.isEmpty()) return
+
+        val properties = schemasToScan.flatMap { it.properties.orEmpty().entries }
+        val primitiveSchemas = properties.filter { isPrimitive(it.value) }
+        val nonPrimitiveSchemas = properties.filterNot { isPrimitive(it.value) }
+
+        collector.putAll(primitiveSchemas.associateBy({ it.key }, { it.value }))
+        traverseSchemas(nonPrimitiveSchemas.flatMap { it.value.properties.orEmpty().values })
+    }
+
+    traverseSchemas(this.getAllSchemas())
+
+    return collector
+}
+
 /**
  * Returns all defined parameters of an API specification
  * @return a collection of parameters

server/src/test/java/de/zalando/zally/rule/zalando/SnakeCaseInPropNameRuleTest.kt

@@ -1,66 +1,52 @@
 package de.zalando.zally.rule.zalando
 
-import de.zalando.zally.swaggerWithDefinitions
+import de.zalando.zally.getOpenApiContextFromContent
 import de.zalando.zally.testConfig
-import io.swagger.models.Swagger
 import org.assertj.core.api.Assertions.assertThat
+import org.intellij.lang.annotations.Language
 import org.junit.Test
 
 class SnakeCaseInPropNameRuleTest {
 
     private val rule = SnakeCaseInPropNameRule(testConfig)
 
     @Test
-    fun emptySwagger() {
-        assertThat(rule.validate(Swagger())).isNull()
+    fun `checkPropertyNames should return violation if a property name is not snake_case`() {
+        @Language("YAML")
+        val spec = """
+            openapi: '3.0.1'
+            components:
+              schemas:
+                article:
+                  properties:
+                    superMegaLaserTurboArticle:
+                      type: boolean
+            """.trimIndent()
+        val context = getOpenApiContextFromContent(spec)
+
+        val violations = rule.checkPropertyNames(context)
+
+        assertThat(violations).isNotEmpty
+        assertThat(violations[0].description).isEqualTo("Property name has to be snake_case")
+        assertThat(violations[0].pointer.toString()).isEqualTo("/components/schemas/article/properties/superMegaLaserTurboArticle")
     }
 
     @Test
-    fun validateNormalProperty() {
-        val swagger = swaggerWithDefinitions("ExampleDefinition" to listOf("test_property"))
-        assertThat(rule.validate(swagger)).isNull()
-    }
-
-    @Test
-    fun validateMultipleNormalProperties() {
-        val swagger = swaggerWithDefinitions("ExampleDefinition" to listOf("test_property", "test_property_two"))
-        assertThat(rule.validate(swagger)).isNull()
-    }
-
-    @Test
-    fun validateMultipleNormalPropertiesInMultipleDefinitions() {
-        val swagger = swaggerWithDefinitions(
-            "ExampleDefinition" to listOf("test_property"),
-            "ExampleDefinitionTwo" to listOf("test_property_two")
-        )
-        assertThat(rule.validate(swagger)).isNull()
-    }
-
-    @Test
-    fun validateFalseProperty() {
-        val swagger = swaggerWithDefinitions("ExampleDefinition" to listOf("TEST_PROPERTY"))
-        val result = rule.validate(swagger)!!
-        assertThat(result.description).contains("TEST_PROPERTY")
-        assertThat(result.paths).hasSameElementsAs(listOf("/definitions/ExampleDefinition"))
-    }
-
-    @Test
-    fun validateMultipleFalsePropertiesInMultipleDefinitions() {
-        val swagger = swaggerWithDefinitions(
-            "ExampleDefinition" to listOf("TEST_PROPERTY"),
-            "ExampleDefinitionTwo" to listOf("test_property_TWO")
-        )
-        val result = rule.validate(swagger)!!
-        assertThat(result.description).contains("TEST_PROPERTY", "test_property_TWO")
-        assertThat(result.paths).hasSameElementsAs(listOf(
-            "/definitions/ExampleDefinition",
-            "/definitions/ExampleDefinitionTwo")
-        )
-    }
-
-    @Test
-    fun notFireOnWhitelistedProperty() {
-        val swagger = swaggerWithDefinitions("ExampleDefinition" to listOf("_links"))
-        assertThat(rule.validate(swagger)).isNull()
+    fun `checkPropertyNames should return no violation if only snake_case properties are used`() {
+        @Language("YAML")
+        val spec = """
+            openapi: '3.0.1'
+            components:
+              schemas:
+                article:
+                  properties:
+                    article_title:
+                      type: string
+            """.trimIndent()
+        val context = getOpenApiContextFromContent(spec)
+
+        val violations = rule.checkPropertyNames(context)
+
+        assertThat(violations).isEmpty()
     }
 }