Merge branch 'master' into feature/zalandogh-714-no-unused-definition…

…s-via-json
roxspring · Oct 18, 2018 · c144a36 · c144a36
2 parents 73391d4 + 4ab243c
commit c144a36
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 64 deletions.
diff --git a/server/src/main/java/de/zalando/zally/rule/zally/AvoidXZallyIgnoreRule.kt b/server/src/main/java/de/zalando/zally/rule/zally/AvoidXZallyIgnoreRule.kt
@@ -1,24 +1,24 @@
 package de.zalando.zally.rule.zally
 
+import com.fasterxml.jackson.core.JsonPointer
 import com.fasterxml.jackson.databind.JsonNode
 import de.zalando.zally.rule.api.Check
 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.ast.JsonPointers
 
 /**
  * Rule highlighting that x-zally-ignore should be used sparingly
  */
 @Rule(
-        ruleSet = ZallyRuleSet::class,
-        id = "H002",
-        severity = Severity.HINT,
-        title = "Avoid using x-zally-ignore extension."
+    ruleSet = ZallyRuleSet::class,
+    id = "H002",
+    severity = Severity.HINT,
+    title = "Avoid using x-zally-ignore extension."
 )
 class AvoidXZallyIgnoreRule {
 
-    private val description = "Ignoring rules should be reserved for exceptional temporary circumstances"
-
     private val xZallyIgnore = "x-zally-ignore"
 
     /**
@@ -27,38 +27,40 @@ class AvoidXZallyIgnoreRule {
      * @return Violation iff x-zally-ignore is in use
      */
     @Check(severity = Severity.HINT)
-    fun validate(root: JsonNode): Violation? {
-        val paths = validateTree(root)
-        return when {
-            paths.isEmpty() -> null
-            else -> Violation(description, paths)
-        }
-    }
+    fun validate(root: JsonNode): List<Violation> = validateTree(JsonPointers.EMPTY, root)
 
-    private fun validateTree(node: JsonNode): List<String> {
-        return when {
-            node.isObject -> validateXZallyIgnore(node) + validateChildren(node)
+    private fun validateTree(pointer: JsonPointer, node: JsonNode): List<Violation> =
+        when {
+            node.isArray -> validateArrayNode(pointer, node)
+            node.isObject -> validateObjectNode(pointer, node)
             else -> emptyList()
         }
-    }
 
-    private fun validateXZallyIgnore(node: JsonNode): List<String> {
-        return when {
-            node.has(xZallyIgnore) -> {
-                val ignores = node.get(xZallyIgnore)
-                when {
-                    ignores.isArray -> listOf("ignores rules " + ignores.joinToString(separator = ", ", transform = JsonNode::asText))
-                    ignores.isValueNode -> listOf("invalid ignores, expected list but found single value $ignores")
-                    else -> listOf("invalid ignores, expected list but found $ignores")
-                }
+    private fun validateArrayNode(pointer: JsonPointer, node: JsonNode): List<Violation> =
+        node.asSequence().toList().mapIndexed { index, childNode ->
+            val childPointer = pointer.append(JsonPointer.compile("/$index"))
+            validateTree(childPointer, childNode)
+        }.flatten()
+
+    private fun validateObjectNode(pointer: JsonPointer, node: JsonNode): List<Violation> =
+        node.fields().asSequence().toList().flatMap { (name, childNode) ->
+            val childPointer = pointer.append(JsonPointers.escape(name))
+            when (name) {
+                xZallyIgnore -> validateXZallyIgnore(childPointer, childNode)
+                else -> validateTree(childPointer, childNode)
             }
-            else -> emptyList()
         }
-    }
 
-    private fun validateChildren(node: JsonNode): List<String> {
-        return node.fields().asSequence().toList().flatMap { (name, child) ->
-            validateTree(child).map { "$name: $it" }
-        }
-    }
+    private fun validateXZallyIgnore(pointer: JsonPointer, node: JsonNode): List<Violation> =
+        listOf(Violation(
+            when {
+                node.isArray -> node.joinToString(
+                    prefix = "Ignores rules ",
+                    separator = ", ",
+                    transform = JsonNode::asText
+                )
+                node.isValueNode -> "Invalid ignores, expected list but found single value $node"
+                else -> "Invalid ignores, expected list but found $node"
+            }, pointer
+        ))
 }
diff --git a/server/src/test/java/de/zalando/zally/rule/zally/AvoidXZallyIgnoreRuleTest.kt b/server/src/test/java/de/zalando/zally/rule/zally/AvoidXZallyIgnoreRuleTest.kt
@@ -1,7 +1,9 @@
 package de.zalando.zally.rule.zally
 
 import de.zalando.zally.rule.ObjectTreeReader
+import de.zalando.zally.rule.ZallyAssertions
 import org.assertj.core.api.Assertions.assertThat
+import org.intellij.lang.annotations.Language
 import org.junit.Test
 
 class AvoidXZallyIgnoreRuleTest {
@@ -10,31 +12,24 @@ class AvoidXZallyIgnoreRuleTest {
     private val reader = ObjectTreeReader()
 
     @Test
-    fun validateSpecWithNoIgnores() {
-        val root = reader.read("""
-            swagger: 2.0
-            """.trimIndent())
-
-        val violation = rule.validate(root)
-
-        assertThat(violation).isNull()
-    }
-
-    @Test
-    fun validateSpecWithInlineIgnoresAtRoot() {
+    fun `validate swagger with inline ignores returns violation`() {
+        @Language("YAML")
         val root = reader.read("""
             swagger: 2.0
             x-zally-ignore: [ ONE, TWO, THREE]
             """.trimIndent())
 
-        val violation = rule.validate(root)
+        val violations = rule.validate(root)
 
-        assertThat(violation?.paths!!)
-                .hasSameElementsAs(listOf("ignores rules ONE, TWO, THREE"))
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Ignores rules ONE, TWO, THREE")
+            .pointersEqualTo("/x-zally-ignore")
     }
 
     @Test
-    fun validateSpecWithDashedIgnoresAtRoot() {
+    fun `validate swagger with dashed ignores returns violation`() {
+        @Language("YAML")
         val root = reader.read("""
             swagger: 2.0
             x-zally-ignore:
@@ -43,50 +38,79 @@ class AvoidXZallyIgnoreRuleTest {
               - THREE
             """.trimIndent())
 
-        val violation = rule.validate(root)
+        val violations = rule.validate(root)
 
-        assertThat(violation?.paths!!)
-                .hasSameElementsAs(listOf("ignores rules ONE, TWO, THREE"))
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Ignores rules ONE, TWO, THREE")
+            .pointersEqualTo("/x-zally-ignore")
     }
 
     @Test
-    fun validateSpecWithInlineIgnoresDeeper() {
+    fun `validate swagger with ignores within object returns violation`() {
+        @Language("YAML")
         val root = reader.read("""
             swagger: 2.0
             info:
               x-zally-ignore: [ ONE, TWO, THREE]
             """.trimIndent())
 
-        val violation = rule.validate(root)
+        val violations = rule.validate(root)
+
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Ignores rules ONE, TWO, THREE")
+            .pointersEqualTo("/info/x-zally-ignore")
+    }
+
+    @Test
+    fun `validate openapi with ignores within array returns violation`() {
+        @Language("YAML")
+        val root = reader.read("""
+            openapi: 3.0.0
+            servers:
+              - url: http://example.com
+                x-zally-ignore: [ONE, TWO, THREE]
+            """.trimIndent())
+
+        val violations = rule.validate(root)
 
-        assertThat(violation?.paths!!)
-                .hasSameElementsAs(listOf("info: ignores rules ONE, TWO, THREE"))
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Ignores rules ONE, TWO, THREE")
+            .pointersEqualTo("/servers/0/x-zally-ignore")
     }
 
     @Test
-    fun validateSpecWithInvalidSingleValueIgnores() {
+    fun `validate openapi with invalid ignores string returns violation`() {
+        @Language("YAML")
         val root = reader.read("""
             swagger: 2.0
             x-zally-ignore: INVALID
             """.trimIndent())
 
-        val violation = rule.validate(root)
+        val violations = rule.validate(root)
 
-        assertThat(violation?.paths!!)
-                .hasSameElementsAs(listOf("invalid ignores, expected list but found single value \"INVALID\""))
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Invalid ignores, expected list but found single value \"INVALID\"")
+            .pointersEqualTo("/x-zally-ignore")
     }
 
     @Test
-    fun validateSpecWithInvalidOtherIgnores() {
+    fun `validate openapi with invalid ignores object returns violation`() {
+        @Language("YAML")
         val root = reader.read("""
             swagger: 2.0
             x-zally-ignore:
               invalid: INVALID
             """.trimIndent())
 
-        val violation = rule.validate(root)
+        val violations = rule.validate(root)
 
-        assertThat(violation?.paths!!)
-                .hasSameElementsAs(listOf("invalid ignores, expected list but found {\"invalid\":\"INVALID\"}"))
+        ZallyAssertions
+            .assertThat(violations)
+            .descriptionsEqualTo("Invalid ignores, expected list but found {\"invalid\":\"INVALID\"}")
+            .pointersEqualTo("/x-zally-ignore")
     }
 }