Convert rule AvoidLinkHeaderRule to the Context object (#714)

zalando · Jul 24, 2018 · 0c9fcc7 · 0c9fcc7
1 parent bfb483c
commit 0c9fcc7
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 45 deletions.
diff --git a/server/src/main/java/de/zalando/zally/rule/HttpHeadersRule.kt b/server/src/main/java/de/zalando/zally/rule/HttpHeadersRule.kt
@@ -2,37 +2,49 @@ package de.zalando.zally.rule
 
 import com.typesafe.config.Config
 import de.zalando.zally.rule.api.Violation
-import io.swagger.models.Response
-import io.swagger.models.Swagger
-import io.swagger.models.parameters.Parameter
+import io.swagger.v3.oas.models.parameters.Parameter
+import io.swagger.v3.oas.models.responses.ApiResponse
 
 abstract class HttpHeadersRule(rulesConfig: Config) {
 
     private val headersWhitelist = rulesConfig.getStringList(HttpHeadersRule::class.simpleName + ".whitelist").toSet()
 
-    abstract fun createViolation(paths: List<String>): Violation
+    abstract fun createViolation(context: Context, header: HeaderElement): Violation
 
-    abstract fun isViolation(header: String): Boolean
+    abstract fun isViolation(header: HeaderElement): Boolean
 
-    open fun validate(swagger: Swagger): Violation? {
-        fun Collection<Parameter>?.extractHeaders(path: String) =
-            orEmpty().filter { it.`in` == "header" }.map { path to it.name }
+    data class HeaderElement(
+        val name: String,
+        val element: Any
+    )
 
-        fun Collection<Response>?.extractHeaders(path: String) =
-            orEmpty().flatMap { it.headers?.keys.orEmpty() }.map { path to it }
+    open fun validate(context: Context): List<Violation> {
 
-        val fromParams = swagger.parameters.orEmpty().values.extractHeaders("parameters")
-        val fromPaths = swagger.paths.orEmpty().entries.flatMap { (name, path) ->
-            path.parameters.extractHeaders(name) + path.operations.flatMap { operation ->
-                operation.parameters.extractHeaders(name) + operation.responses.values.extractHeaders(name)
+        fun Collection<Parameter>?.extractHeaders() = orEmpty()
+            .filter { it.`in` == "header" }
+            .map { HeaderElement(it.name, it) }
+            .toSet()
+
+        fun Collection<ApiResponse>?.extractHeaders() = orEmpty()
+            .flatMap { it.headers.orEmpty().entries }
+            .map { HeaderElement(it.key, it.value) }
+            .toSet()
+
+        val fromParams = context.api.components.parameters.orEmpty().values.extractHeaders()
+
+        val fromPaths = context.api.paths.orEmpty().flatMap { (_, path) ->
+            val fromPathParameters = path.parameters.extractHeaders()
+            val fromOperations = path.readOperations().flatMap { operation ->
+                val fromOpParams = operation.parameters.extractHeaders()
+                val fromOpResponses = operation.responses.orEmpty().values.extractHeaders()
+                fromOpParams + fromOpResponses
             }
+            fromPathParameters + fromOperations
         }
+
         val allHeaders = fromParams + fromPaths
-        val paths = allHeaders
-            .filter { it.second !in headersWhitelist && isViolation(it.second) }
-            .map { "${it.first} ${it.second}" }
-            .toSet()
-            .toList()
-        return if (paths.isNotEmpty()) createViolation(paths) else null
+        return allHeaders
+            .filter { it.name !in headersWhitelist && isViolation(it) }
+            .map { createViolation(context, it) }
     }
 }
diff --git a/server/src/main/java/de/zalando/zally/rule/zalando/AvoidLinkHeadersRule.kt b/server/src/main/java/de/zalando/zally/rule/zalando/AvoidLinkHeadersRule.kt
@@ -1,31 +1,31 @@
 package de.zalando.zally.rule.zalando
 
 import com.typesafe.config.Config
+import de.zalando.zally.rule.Context
 import de.zalando.zally.rule.HttpHeadersRule
 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.rule.api.Rule
-import io.swagger.models.Swagger
 import org.springframework.beans.factory.annotation.Autowired
 
 @Rule(
-        ruleSet = ZalandoRuleSet::class,
-        id = "166",
-        severity = Severity.MUST,
-        title = "Avoid Link in Header Rule"
+    ruleSet = ZalandoRuleSet::class,
+    id = "166",
+    severity = Severity.MUST,
+    title = "Avoid Link in Header Rule"
 )
 class AvoidLinkHeadersRule(@Autowired rulesConfig: Config) : HttpHeadersRule(rulesConfig) {
     private val description = "Do Not Use Link Headers with JSON entities"
 
     @Check(severity = Severity.MUST)
-    override fun validate(swagger: Swagger): Violation? {
-        return super.validate(swagger)
+    override fun validate(context: Context): List<Violation> {
+        return super.validate(context)
     }
 
-    override fun isViolation(header: String) = header == "Link"
+    override fun isViolation(header: HeaderElement) = header.name == "Link"
+
+    override fun createViolation(context: Context, header: HeaderElement): Violation =
+        context.violation(description, header.element)
 
-    override fun createViolation(paths: List<String>): Violation {
-        return Violation(description, paths)
-    }
 }
diff --git a/server/src/test/java/de/zalando/zally/TestUtil.kt b/server/src/test/java/de/zalando/zally/TestUtil.kt
@@ -26,9 +26,11 @@ val testConfig: Config by lazy {
 
 fun getFixture(fileName: String): Swagger = SwaggerParser().read("fixtures/$fileName")
 
-fun getContextFromFixture(fileName: String): Context? {
+fun getContextFromFixture(fileName: String): Context {
     val content = getResourceContent(fileName)
-    return Context.createOpenApiContext(content) ?: Context.createSwaggerContext(content)
+    return Context.createOpenApiContext(content)
+        ?: Context.createSwaggerContext(content)
+        ?: throw RuntimeException("Unable to create context.")
 }
 
 fun getResourceContent(fileName: String): String = ClasspathHelper.loadFileFromClasspath("fixtures/$fileName")

diff --git a/server/src/test/java/de/zalando/zally/rule/zalando/AvoidLinkHeadersRuleTest.kt b/server/src/test/java/de/zalando/zally/rule/zalando/AvoidLinkHeadersRuleTest.kt
@@ -1,6 +1,8 @@
 package de.zalando.zally.rule.zalando
 
-import de.zalando.zally.getFixture
+import com.fasterxml.jackson.core.JsonPointer
+import de.zalando.zally.getContextFromFixture
+import de.zalando.zally.rule.api.Violation
 import de.zalando.zally.testConfig
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Test
@@ -11,21 +13,30 @@ class AvoidLinkHeadersRuleTest {
 
     @Test
     fun positiveCaseSpp() {
-        val swagger = getFixture("api_spp.json")
-        assertThat(rule.validate(swagger)).isNull()
+        val context = getContextFromFixture("api_spp.json")
+        val violations = rule.validate(context)
+        assertThat(violations).isEmpty()
     }
 
     @Test
     fun positiveCaseSpa() {
-        val swagger = getFixture("api_spa.yaml")
-        assertThat(rule.validate(swagger)).isNull()
+        val context = getContextFromFixture("api_spa.yaml")
+        val violations = rule.validate(context)
+        assertThat(violations).isEmpty()
     }
 
     @Test
     fun negativeCase() {
-        val swagger = getFixture("avoidLinkHeaderRuleInvalid.json")
-        val violation = rule.validate(swagger)!!
-        assertThat(violation.paths).hasSameElementsAs(
-            listOf("/product-put-requests/{product_path} Link", "/products Link"))
+        val context = getContextFromFixture("avoidLinkHeaderRuleInvalid.json")
+        val violations = rule.validate(context)
+        assertThat(violations).hasSameElementsAs(listOf(
+            v("/paths/~1products/get/parameters/4"),
+            v("/paths/~1product-put-requests~1{product_path}/post/responses/202/headers/Link")
+        ))
     }
+
+    private fun v(pointer: String) = Violation(
+        description = "Do Not Use Link Headers with JSON entities",
+        pointer = JsonPointer.compile(pointer)
+    )
 }
diff --git a/server/src/test/java/de/zalando/zally/rule/zalando/MediaTypesRuleTest.kt b/server/src/test/java/de/zalando/zally/rule/zalando/MediaTypesRuleTest.kt
@@ -120,7 +120,7 @@ class MediaTypesRuleTest {
 
     @Test
     fun `the SPP API generates violations`() {
-        val context = getContextFromFixture("api_spp.json")!!
+        val context = getContextFromFixture("api_spp.json")
         val result = rule.validate(context)
         assertThat(result).hasSameElementsAs(listOf(
             // --- consumes ---
@@ -137,7 +137,7 @@ class MediaTypesRuleTest {
 
     @Test
     fun `the SPA API generates no violations`() {
-        val context = getContextFromFixture("api_spa.yaml")!!
+        val context = getContextFromFixture("api_spa.yaml")
         assertThat(rule.validate(context)).isEmpty()
     }