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

…s-via-json

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

@@ -1,33 +1,41 @@
 package de.zalando.zally.rule.zally
 
+import com.fasterxml.jackson.core.JsonPointer
+import de.zalando.zally.rule.api.Context
 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 io.swagger.models.Swagger
 
 @Rule(
-        ruleSet = ZallyRuleSet::class,
-        id = "H001",
-        severity = Severity.HINT,
-        title = "Base path can be extracted"
+    ruleSet = ZallyRuleSet::class,
+    id = "H001",
+    severity = Severity.HINT,
+    title = "Base path can be extracted"
 )
 class ExtractBasePathRule {
 
-    private val description = "All paths start with prefix '%s'. This prefix could be part of base path."
-
     @Check(severity = Severity.HINT)
-    fun validate(swagger: Swagger): Violation? {
-        val paths = swagger.paths.orEmpty().keys
+    fun validate(context: Context): List<Violation> {
+        val paths = context.api.paths?.keys.orEmpty()
         if (paths.size < 2) {
-            return null
+            return emptyList()
+        }
+        val prefix = paths.reduce { s1, s2 -> findCommonPrefix(s1, s2) }
+        return when {
+            prefix.isEmpty() -> emptyList()
+            context.isOpenAPI3() -> violations(prefix, "servers' urls")
+            else -> violations(prefix, "basePath")
         }
-        val commonPrefix = paths.reduce { s1, s2 -> findCommonPrefix(s1, s2) }
-        return if (commonPrefix.isNotEmpty())
-            Violation(description.format(commonPrefix), emptyList())
-        else null
     }
 
+    private fun violations(prefix: String, target: String) = listOf(
+        Violation(
+            "All paths start with prefix '$prefix' which could be part of $target.",
+            JsonPointer.compile("/paths")
+        )
+    )
+
     private fun findCommonPrefix(s1: String, s2: String): String {
         val parts1 = s1.split("/")
         val parts2 = s2.split("/")

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

@@ -1,81 +1,123 @@
 package de.zalando.zally.rule.zally
 
-import de.zalando.zally.getFixture
-import de.zalando.zally.rule.api.Violation
-import de.zalando.zally.swaggerWithPaths
-import io.swagger.models.Swagger
-import org.assertj.core.api.Assertions.assertThat
+import de.zalando.zally.getOpenApiContextFromContent
+import de.zalando.zally.getSwaggerContextFromContent
+import de.zalando.zally.rule.ZallyAssertions
+import org.intellij.lang.annotations.Language
 import org.junit.Test
 
 class ExtractBasePathRuleTest {
-    val DESC_PATTERN = "All paths start with prefix '%s'. This prefix could be part of base path."
 
     private val rule = ExtractBasePathRule()
 
     @Test
-    fun validateEmptyPath() {
-        assertThat(rule.validate(Swagger())).isNull()
-    }
+    fun `validate swagger with no paths returns no violations`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            """.trimIndent())
 
-    @Test
-    fun simplePositiveCase() {
-        val swagger = swaggerWithPaths("/orders/{order_id}", "/orders/{updates}", "/merchants")
-        assertThat(rule.validate(swagger)).isNull()
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .isEmpty()
     }
 
     @Test
-    fun singlePathShouldPass() {
-        val swagger = swaggerWithPaths("/orders/{order_id}")
-        assertThat(rule.validate(swagger)).isNull()
+    fun `validate swagger with no common first segments returns no violations`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            paths:
+              /orders/{order_id}: {}
+              /orders/{updates}: {}
+              /merchants: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .isEmpty()
     }
 
     @Test
-    fun simpleNegativeCase() {
-        val swagger = swaggerWithPaths(
-            "/shipment/{shipment_id}",
-            "/shipment/{shipment_id}/status",
-            "/shipment/{shipment_id}/details"
-        )
-        val rule = rule
-        val expected = Violation(DESC_PATTERN.format("/shipment"),
-                emptyList())
-        assertThat(rule.validate(swagger)).isEqualTo(expected)
+    fun `validate swagger with single path returns no violations`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            paths:
+              /orders/{order_id}: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .isEmpty()
     }
 
     @Test
-    fun multipleResourceNegativeCase() {
-        val swagger = swaggerWithPaths(
-            "/queue/models/configs/{config-id}",
-            "/queue/models/",
-            "/queue/models/{model-id}",
-            "/queue/models/summaries"
-        )
-        val rule = rule
-        val expected = Violation(DESC_PATTERN.format("/queue/models"),
-                emptyList())
-        assertThat(rule.validate(swagger)).isEqualTo(expected)
+    fun `validate swagger with common first segment returns violation`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            paths:
+              /shipment/{shipment_id}: {}
+              /shipment/{shipment_id}/status: {}
+              /shipment/{shipment_id}/details: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .descriptionsEqualTo("All paths start with prefix '/shipment' which could be part of basePath.")
+            .pointersEqualTo("/paths")
     }
 
     @Test
-    fun shouldMatchWholeSubresource() {
-        val swagger = swaggerWithPaths(
-            "/api/{api_id}/deployments",
-            "/api/{api_id}/",
-            "/applications/{app_id}",
-            "/applications/"
-        )
-        assertThat(rule.validate(swagger)).isNull()
+    fun `validate swagger with multiple common first segments returns violation`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            paths:
+              /queue/models/configs/{config-id}: {}
+              /queue/models/: {}
+              /queue/models/{model-id}: {}
+              /queue/models/summaries: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .descriptionsEqualTo("All paths start with prefix '/queue/models' which could be part of basePath.")
+            .pointersEqualTo("/paths")
     }
 
     @Test
-    fun positiveCaseSpp() {
-        val swagger = getFixture("api_spp.json")
-        assertThat(rule.validate(swagger)).isNull()
+    fun `validate swagger with common prefix but no common first segments returns no violations`() {
+        @Language("YAML")
+        val context = getSwaggerContextFromContent("""
+            swagger: 2.0
+            paths:
+              /api/{api_id}/deployments: {}
+              /api/{api_id}/: {}
+              /applications/{app_id}: {}
+              /applications/: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .isEmpty()
     }
 
     @Test
-    fun positiveCaseTinbox() {
-        val swagger = getFixture("api_tinbox.yaml")
-        assertThat(rule.validate(swagger)).isNull()
+    fun `validate openapi with common first segment returns violation`() {
+        @Language("YAML")
+        val context = getOpenApiContextFromContent("""
+            openapi: 3.0.1
+            paths:
+              /shipment/{shipment_id}: {}
+              /shipment/{shipment_id}/status: {}
+              /shipment/{shipment_id}/details: {}
+            """.trimIndent())
+
+        ZallyAssertions
+            .assertThat(rule.validate(context))
+            .descriptionsEqualTo("All paths start with prefix '/shipment' which could be part of servers' urls.")
+            .pointersEqualTo("/paths")
     }
 }