Merge pull request #821 from zalando/714-migrate-limit-number-of-sub-…

…resources-rule

feat(server): migrate limit number of subresources rule to context object to support openapi 3

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

@@ -0,0 +1,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 org.springframework.beans.factory.annotation.Autowired
+
+@Rule(
+    ruleSet = ZalandoRuleSet::class,
+    id = "147",
+    severity = Severity.SHOULD,
+    title = "Limit number of Sub-resources level"
+)
+class LimitNumberOfSubResourcesRule(@Autowired rulesConfig: Config) {
+    private val subResourcesLimit = rulesConfig.getConfig(javaClass.simpleName).getInt("subresources_limit")
+    private val description = "Number of sub-resources should not exceed $subResourcesLimit"
+
+    @Check(severity = Severity.SHOULD)
+    fun checkNumberOfSubResources(context: Context): List<Violation> =
+        context.api.paths.orEmpty().entries
+            .map { (path, pathObj) -> Pair(path.split("/").filter { it.isNotEmpty() && !PatternUtil.isPathVariable(it) }.size - 1, pathObj) }
+            .filter { (numberOfSubResources, _) -> numberOfSubResources > subResourcesLimit }
+            .map { (_, pathObj) -> context.violation(description, pathObj) }
+}

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

@@ -18,7 +18,7 @@ class SuccessResponseAsJsonObjectRule {
 
     @Check(severity = Severity.MUST)
     fun checkJSONObjectIsUsedAsSuccessResponseType(context: Context): List<Violation> =
-        context.api.paths.values
+        context.api.paths.orEmpty().values
             .flatMap {
                 it.readOperations().orEmpty()
                     .flatMap { it.responses.filter { (resCode, _) -> isSuccess(resCode) }.values }

server/src/main/resources/rules-config.conf

@@ -22,7 +22,7 @@ LimitNumberOfResourcesRule {
   resource_types_limit: 8
 }
 
-LimitNumberOfSubresourcesRule {
+LimitNumberOfSubResourcesRule {
   subresources_limit: 3
 }
 

server/src/test/java/de/zalando/zally/TestUtil.kt

@@ -7,13 +7,9 @@ import de.zalando.zally.rule.ContentParseResult
 import de.zalando.zally.rule.DefaultContext
 import de.zalando.zally.rule.ObjectTreeReader
 import de.zalando.zally.rule.api.Context
-import io.swagger.models.ModelImpl
-import io.swagger.models.Operation
 import io.swagger.models.Path
-import io.swagger.models.Response
 import io.swagger.models.Swagger
 import io.swagger.models.parameters.HeaderParameter
-import io.swagger.models.properties.StringProperty
 import io.swagger.parser.SwaggerParser
 import io.swagger.parser.util.ClasspathHelper
 import io.swagger.v3.oas.models.OpenAPI
@@ -65,7 +61,7 @@ fun getOpenApiContextFromContent(content: String): Context {
             throw RuntimeException("Parsed with violations:$errors")
         }
         is ContentParseResult.NotApplicable -> {
-            throw RuntimeException("Missing the 'OpenAPI' property.")
+            throw RuntimeException("Missing the 'openapi' property.")
         }
     }
 }
@@ -101,27 +97,6 @@ fun swaggerWithHeaderParams(vararg names: String) =
         }.toMap()
     }
 
-fun swaggerWithDefinitions(vararg defs: Pair<String, List<String>>): Swagger =
-    Swagger().apply {
-        definitions = defs.map { def ->
-            def.first to ModelImpl().apply {
-                properties = def.second.map { prop -> prop to StringProperty() }.toMap()
-            }
-        }.toMap()
-    }
-
-fun swaggerWithOperations(operations: Map<String, Iterable<String>>): Swagger =
-    Swagger().apply {
-        val path = Path()
-        operations.forEach { method, statuses ->
-            val operation = Operation().apply {
-                statuses.forEach { addResponse(it, Response()) }
-            }
-            path.set(method, operation)
-        }
-        paths = mapOf("/test" to path)
-    }
-
 fun openApiWithOperations(operations: Map<String, Iterable<String>>): OpenAPI =
     OpenAPI().apply {
         val pathItem = PathItem()

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

@@ -0,0 +1,44 @@
+package de.zalando.zally.rule.zalando
+
+import de.zalando.zally.getOpenApiContextFromContent
+import de.zalando.zally.testConfig
+import org.assertj.core.api.Assertions.assertThat
+import org.intellij.lang.annotations.Language
+import org.junit.Test
+
+class LimitNumberOfSubResourcesRuleTest {
+
+    private val rule = LimitNumberOfSubResourcesRule(testConfig)
+
+    @Test
+    fun `checkNumberOfSubResources should return violation if number of sub resources exceeds the limit`() {
+        @Language("YAML")
+        val spec = """
+              openapi: "3.0.0"
+              paths:
+                /worlds/{world-id}/countries/{country-id}/states/{state-id}/cities/{city-id}/streets/{street-id}: {}
+              """.trimIndent()
+        val context = getOpenApiContextFromContent(spec)
+
+        val violations = rule.checkNumberOfSubResources(context)
+
+        assertThat(violations).isNotEmpty
+        assertThat(violations[0].description).containsPattern("Number of sub-resources should not exceed")
+        assertThat(violations[0].pointer.toString()).isEqualTo("/paths/~1worlds~1{world-id}~1countries~1{country-id}~1states~1{state-id}~1cities~1{city-id}~1streets~1{street-id}")
+    }
+
+    @Test
+    fun `checkNumberOfSubResources should return no violation if number of sub resources doesn not exceed the limit`() {
+        @Language("YAML")
+        val spec = """
+            openapi: 3.0.1
+            paths:
+              /articles: {}
+        """.trimIndent()
+        val context = getOpenApiContextFromContent(spec)
+
+        val violations = rule.checkNumberOfSubResources(context)
+
+        assertThat(violations).isEmpty()
+    }
+}