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 index 08eab4ece..760140b1f 100644 --- 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 = validateTree(JsonPointers.EMPTY, root) - private fun validateTree(node: JsonNode): List { - return when { - node.isObject -> validateXZallyIgnore(node) + validateChildren(node) + private fun validateTree(pointer: JsonPointer, node: JsonNode): List = + when { + node.isArray -> validateArrayNode(pointer, node) + node.isObject -> validateObjectNode(pointer, node) else -> emptyList() } - } - private fun validateXZallyIgnore(node: JsonNode): List { - 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 = + 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 = + 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 { - return node.fields().asSequence().toList().flatMap { (name, child) -> - validateTree(child).map { "$name: $it" } - } - } + private fun validateXZallyIgnore(pointer: JsonPointer, node: JsonNode): List = + 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 index 73b24ca6d..74fd8a3b3 100644 --- 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") } }