fix: supported references for problem+json (#1274)

zalando · Jul 6, 2021 · 0246c86 · 0246c86
1 parent 34594a2
commit 0246c86
Show file tree

Hide file tree

Showing 4 changed files with 856 additions and 542 deletions.
diff --git a/...ndo/src/main/kotlin/org/zalando/zally/ruleset/zalando/JsonProblemAsDefaultResponseRule.kt b/...ndo/src/main/kotlin/org/zalando/zally/ruleset/zalando/JsonProblemAsDefaultResponseRule.kt
@@ -17,23 +17,35 @@ import io.swagger.v3.oas.models.responses.ApiResponse
 )
 class JsonProblemAsDefaultResponseRule {
     private val validRefs = listOf(
-        "https://zalando.github.io/problem/schema.yaml#/Problem",
-        "https://opensource.zalando.com/problem/schema.yaml#/Problem"
+        "https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.1.yaml#/Problem",
+        "https://opensource.zalando.com/restful-api-guidelines/problem-1.0.1.yaml#/Problem",
+        "https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.0.yaml#/Problem",
+        "https://opensource.zalando.com/restful-api-guidelines/problem-1.0.0.yaml#/Problem",
+        "https://opensource.zalando.com/problem/schema.yaml#/Problem",
+        "https://zalando.github.io/problem/schema.yaml#/Problem"
     )
     private val validContentTypes = listOf("application/json", "application/problem+json")
 
-    @Check(severity = Severity.MAY)
+    @Check(severity = Severity.SHOULD)
     fun checkContainsDefaultResponse(context: Context): List<Violation> = responsesPerOperation(context)
         .filterNot { "default" in it.second.keys }
-        .map { context.violation("operation has to contain the default response", it.first) }
+        .map { context.violation("operation should contain the default response", it.first) }
 
-    @Check(severity = Severity.MAY)
-    fun checkDefaultResponseIsProblemJson(context: Context): List<Violation> = responsesPerOperation(context)
+    @Check(severity = Severity.SHOULD)
+    fun checkDefaultResponseIsProblemJsonMediaType(context: Context): List<Violation> = responsesPerOperation(context)
+        .filter { "default" in it.second.keys }
+        .flatMap { it.second.getValue("default").content.orEmpty().entries }
+        .filterNot { (contentType, _) -> contentType in validContentTypes }
+        .map { context.violation("media-type application/problem+json should be used as default response", it.value) }
+
+    @Check(severity = Severity.SHOULD)
+    fun checkDefaultResponseIsProblemJsonSchema(context: Context): List<Violation> = responsesPerOperation(context)
         .filter { "default" in it.second.keys }
         .flatMap { it.second.getValue("default").content.orEmpty().entries }
         .filter { (contentType, _) -> contentType in validContentTypes }
-        .filterNot { it.value?.schema?.`$ref` in validRefs || isProblemJsonSchema(it.value?.schema) }
-        .map { context.violation("problem json has to be used as default response (${validRefs[0]})", it.value) }
+        .filterNot { it.value?.schema?.`$ref` in validRefs }
+        .filterNot { isProblemJsonSchema(it.value?.schema) }
+        .map { context.violation("problem+json should be used as default response", it.value) }
 
     private fun responsesPerOperation(context: Context): Collection<Pair<Operation, Map<String, ApiResponse>>> =
         context.api.paths?.values
@@ -45,10 +57,13 @@ class JsonProblemAsDefaultResponseRule {
 
     private fun isProblemJsonSchema(schema: Schema<*>?): Boolean {
         val props = schema?.properties.orEmpty()
-        return props["type"]?.type == "string" && props["type"]?.format == "uri" &&
-            props["title"]?.type == "string" &&
-            props["status"]?.type == "integer" && props["status"]?.format == "int32" &&
-            props["detail"]?.type == "string" &&
-            props["instance"]?.type == "string" && props["instance"]?.format == "uri"
+        return props["type"]?.type == "string" &&
+                (props["type"]?.format == "uri" || props["type"]?.format == "uri-reference") &&
+                props["title"]?.type == "string" &&
+                props["status"]?.type == "integer" &&
+                props["status"]?.format == "int32" &&
+                props["detail"]?.type == "string" &&
+                props["instance"]?.type == "string" &&
+                (props["instance"]?.format == "uri" || props["instance"]?.format == "uri-reference")
     }
 }
diff --git a/...src/test/kotlin/org/zalando/zally/ruleset/zalando/JsonProblemAsDefaultResponseRuleTest.kt b/...src/test/kotlin/org/zalando/zally/ruleset/zalando/JsonProblemAsDefaultResponseRuleTest.kt
@@ -16,7 +16,7 @@ class JsonProblemAsDefaultResponseRuleTest {
             """
             openapi: 3.0.1
             paths:
-              '/pets':
+              '/resources':
                 get:
                   responses:
         """.trimIndent()
@@ -25,44 +25,115 @@ class JsonProblemAsDefaultResponseRuleTest {
         val violations = rule.checkContainsDefaultResponse(context)
 
         assertThat(violations).isNotEmpty
-        assertThat(violations[0].description).containsPattern(".*has to contain the default response.*")
-        assertThat(violations[0].pointer.toString()).isEqualTo("/paths/~1pets/get")
+        assertThat(violations[0].description).containsPattern(".*should contain the default response.*")
+        assertThat(violations[0].pointer.toString()).isEqualTo("/paths/~1resources/get")
     }
 
     @Test
-    fun `checkDefaultResponseIsProblemJson should return violation if not problem json is set as default response`() {
+    fun `checkContainsDefaultResponse should not return violation for empty specification`() {
+        @Language("YAML")
+        val context = DefaultContextFactory().getOpenApiContext(
+            """
+            openapi: 3.0.1
+        """
+        )
+
+        assertThat(rule.checkContainsDefaultResponse(context)).isEmpty()
+    }
+
+    @Test
+    fun `checkDefaultResponseIsProblemJsonMediaType should not return violation for application-problem+json`() {
         @Language("YAML")
         val context = DefaultContextFactory().getOpenApiContext(
             """
             openapi: 3.0.1
             paths:
-              '/pets':
+              '/resources':
                 get:
                   responses:
                     default:
                       content:
                         application/problem+json:
                           schema:
                             ${'$'}ref: 'http://example.com'
+        """
+        )
+
+        assertThat(rule.checkDefaultResponseIsProblemJsonMediaType(context)).isEmpty()
+    }
+
+    @Test
+    fun `checkDefaultResponseIsProblemJsonMediaType should return violation if not application-problem+json is set as default response`() {
+        @Language("YAML")
+        val context = DefaultContextFactory().getOpenApiContext(
+            """
+            openapi: 3.0.1
+            paths:
+              '/resources':
+                get:
+                  responses:
+                    default:
+                      content:
+                        application/gzip:
+                          schema:
+                            ${'$'}ref: 'http://example.com'
         """.trimIndent()
         )
 
-        val violations = rule.checkDefaultResponseIsProblemJson(context)
+        val violations = rule.checkDefaultResponseIsProblemJsonMediaType(context)
 
         assertThat(violations).isNotEmpty
-        assertThat(violations[0].description).containsPattern(".*problem json has to be used as default response.*")
+        assertThat(violations[0].description).isEqualTo("media-type application/problem+json should be used as default response")
         assertThat(violations[0].pointer.toString())
-            .isEqualTo("/paths/~1pets/get/responses/default/content/application~1problem+json")
+            .isEqualTo("/paths/~1resources/get/responses/default/content/application~1gzip")
+    }
+
+    @Test
+    fun `checkDefaultResponseIsProblemJsonMediaType should not return violation for empty specification`() {
+        @Language("YAML")
+        val context = DefaultContextFactory().getOpenApiContext(
+            """
+            openapi: 3.0.1
+        """
+        )
+
+        assertThat(rule.checkDefaultResponseIsProblemJsonMediaType(context)).isEmpty()
     }
 
     @Test
-    fun `checkDefaultResponseIsProblemJson should not return violation if problem json is set as default response`() {
+    fun `checkDefaultResponseIsProblemJsonSchema should return violation if incorrect problem+json schema reference is set as default response`() {
         @Language("YAML")
         val context = DefaultContextFactory().getOpenApiContext(
             """
             openapi: 3.0.1
             paths:
-              '/pets':
+              '/resources':
+                get:
+                  responses:
+                    default:
+                      content:
+                        application/problem+json:
+                          schema:
+                            ${'$'}ref: 'http://example.com'
+        """.trimIndent()
+        )
+
+        val violations = rule.checkDefaultResponseIsProblemJsonSchema(context)
+
+        assertThat(violations).isNotEmpty
+        assertThat(violations[0].description).isEqualTo("problem+json should be used as default response")
+        assertThat(violations[0].pointer.toString())
+            .isEqualTo("/paths/~1resources/get/responses/default/content/application~1problem+json")
+    }
+
+    @Test
+    fun `checkDefaultResponseIsProblemJsonSchema should return no violation if last problem+json schema reference is set as default response`() {
+        @Language("YAML")
+        val context = DefaultContextFactory().getOpenApiContext(
+            """
+            openapi: 3.0.1
+            paths:
+              '/resources':
                 get:
                   responses:
                     default:
@@ -73,21 +144,43 @@ class JsonProblemAsDefaultResponseRuleTest {
         """.trimIndent()
         )
 
-        val violations = rule.checkDefaultResponseIsProblemJson(context)
+        val violations = rule.checkDefaultResponseIsProblemJsonSchema(context)
 
         assertThat(violations).isEmpty()
     }
 
     @Test
-    fun `(checkDefaultResponseIsProblemJson|checkContainsDefaultResponse) should not return violation for empty specification`() {
+    fun `checkDefaultResponseIsProblemJsonSchema should return no violation if first problem+json schema reference is set as default response`() {
+        @Language("YAML")
+        val context = DefaultContextFactory().getOpenApiContext(
+            """
+            openapi: 3.0.1
+            paths:
+              '/resources':
+                get:
+                  responses:
+                    default:
+                      content:
+                        application/problem+json:
+                          schema:
+                            ${'$'}ref: 'https://opensource.zalando.com/restful-api-guidelines/models/problem-1.0.1.yaml#/Problem'
+        """.trimIndent()
+        )
+
+        val violations = rule.checkDefaultResponseIsProblemJsonSchema(context)
+
+        assertThat(violations).isEmpty()
+    }
+
+    @Test
+    fun `checkDefaultResponseIsProblemJsonSchema should not return violation for empty specification`() {
         @Language("YAML")
         val context = DefaultContextFactory().getOpenApiContext(
             """
             openapi: 3.0.1
         """
         )
 
-        assertThat(rule.checkContainsDefaultResponse(context)).isEmpty()
-        assertThat(rule.checkDefaultResponseIsProblemJson(context)).isEmpty()
+        assertThat(rule.checkDefaultResponseIsProblemJsonSchema(context)).isEmpty()
     }
 }
diff --git a/server/zally-test/src/main/resources/fixtures/api_spa.yaml b/server/zally-test/src/main/resources/fixtures/api_spa.yaml
@@ -98,13 +98,18 @@ paths:
         404:
           description: Partner not found
 
-  /reports/jobs:
+  /reports/{report-id}/jobs:
     get:
       summary: List job reports
       tags:
         - report
       parameters:
         - $ref: "#/parameters/Authorization"
+        - name: report-id
+          in: path
+          description: uniquely identifies report within this API
+          type: string
+          required: true
         - name: from
           in: query
           description: only list jobs started after this date-time (RFC-3339)
@@ -129,13 +134,18 @@ paths:
             items:
               $ref: "#/definitions/JobSummary"
 
-  /reports/jobs/{job-id}:
+  /reports/{report-id}/jobs/{job-id}:
     get:
       summary: Job details
       tags:
         - report
       parameters:
         - $ref: "#/parameters/Authorization"
+        - name: report-id
+          in: path
+          description: uniquely identifies report within this API
+          type: string
+          required: true
         - name: job-id
           in: path
           description: uniquely identifies job within this API