feat(server): ApiViolationsController always responds including api_d…

…efinition #593
zalando · Apr 15, 2019 · d6a5227 · d6a5227
1 parent c5e29d2
commit d6a5227
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 40 deletions.
diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt
@@ -17,7 +17,6 @@ import org.springframework.web.bind.annotation.PathVariable
 import org.springframework.web.bind.annotation.PostMapping
 import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestHeader
-import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.ResponseBody
 import org.springframework.web.bind.annotation.RestController
 import org.springframework.web.util.UriComponentsBuilder
@@ -62,12 +61,11 @@ class ApiViolationsController(
     @ResponseBody
     @GetMapping("/api-violations/{externalId}")
     fun getExistingViolationResponse(
-        @PathVariable(value = "externalId") externalId: UUID,
-        @RequestParam(value = "include_api_definition", required = false, defaultValue = "false") includeApiDefinition: Boolean
+        @PathVariable(value = "externalId") externalId: UUID
     ): ApiDefinitionResponse {
         val review = apiReviewRepository.findByExternalId(externalId) ?: throw ApiReviewNotFoundException()
 
-        return buildApiDefinitionResponse(review, includeApiDefinition)
+        return buildApiDefinitionResponse(review)
     }
 
     private fun retrieveRulesPolicy(request: ApiDefinitionRequest): RulesPolicy = request.ignoreRules
@@ -83,7 +81,7 @@ class ApiViolationsController(
         throw e
     }
 
-    private fun buildApiDefinitionResponse(review: ApiReview, includeApiDefinition: Boolean = false): ApiDefinitionResponse = ApiDefinitionResponse(
+    private fun buildApiDefinitionResponse(review: ApiReview): ApiDefinitionResponse = ApiDefinitionResponse(
         externalId = review.externalId,
         message = serverMessageService.serverMessage(review.userAgent),
         violations = review.ruleViolations.orEmpty().map {
@@ -104,6 +102,6 @@ class ApiViolationsController(
             Severity.MAY to review.mayViolations,
             Severity.HINT to review.hintViolations
         ).map { it.first.name.toLowerCase() to it.second }.toMap(),
-        apiDefinition = if (includeApiDefinition) review.apiDefinition else null
+        apiDefinition = review.apiDefinition
     )
 }
diff --git a/server/src/main/resources/api/zally-api.yaml b/server/src/main/resources/api/zally-api.yaml
@@ -248,14 +248,6 @@ components:
       schema:
         type: string
         format: uuid
-    IncludeApiDefinition:
-      name: include_api_definition
-      in: query
-      description: Whether to include the API definition in the response
-      required: false
-      schema:
-        type: boolean
-        default: false
 
   schemas:
     LintingRequest:

diff --git a/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt b/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt
@@ -2,9 +2,9 @@ package de.zalando.zally.apireview
 
 import de.zalando.zally.configuration.JacksonObjectMapperConfiguration
 import de.zalando.zally.dto.ApiDefinitionRequest
+import org.hamcrest.CoreMatchers.hasItem
 import org.hamcrest.Matchers.containsString
 import org.hamcrest.Matchers.notNullValue
-import org.hamcrest.Matchers.nullValue
 import org.intellij.lang.annotations.Language
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -18,6 +18,7 @@ import org.springframework.test.web.servlet.MockMvc
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 
@@ -40,7 +41,13 @@ class ApiViolationsControllerTest {
                 .content("{\"api_definition_string\":\"\"}")
         )
             .andExpect(status().isOk)
+            .andExpect(header().exists("Location"))
+            .andExpect(content().contentType(APPLICATION_JSON_UTF8))
             .andExpect(content().string(containsString("https://zalando.github.io/restful-api-guidelines")))
+            .andExpect(jsonPath("$.violations[*].rule_link", hasItem("https://zalando.github.io/restful-api-guidelines/#101")))
+            .andExpect(jsonPath("$.external_id", notNullValue()))
+            .andExpect(jsonPath("$.violations", notNullValue()))
+            .andExpect(jsonPath("$.api_definition", notNullValue()))
     }
 
     @Test
@@ -53,29 +60,7 @@ class ApiViolationsControllerTest {
     }
 
     @Test
-    fun `getExistingViolationResponse with existing responds Ok without api_definition`() {
-
-        val location = mvc.perform(
-            post("/api-violations")
-                .contentType("application/json")
-                .content("{\"api_definition_string\":\"\"}")
-        )
-            .andExpect(status().isOk)
-            .andReturn().response.getHeaderValue("Location")!!
-
-        mvc.perform(
-            get(location.toString())
-                .accept("application/json")
-        )
-            .andExpect(status().isOk)
-            .andExpect(content().contentType(APPLICATION_JSON_UTF8))
-            .andExpect(jsonPath("$.external_id", notNullValue()))
-            .andExpect(jsonPath("$.violations", notNullValue()))
-            .andExpect(jsonPath("$.api_definition", nullValue()))
-    }
-
-    @Test
-    fun `getExistingViolationResponse with existing and include_api_definition responds Ok with api_definition`() {
+    fun `getExistingViolationResponse with existing responds Ok`() {
 
         val location = mvc.perform(
             post("/api-violations")
@@ -87,7 +72,6 @@ class ApiViolationsControllerTest {
 
         mvc.perform(
             get(location.toString())
-                .param("include_api_definition", "true")
                 .accept("application/json")
         )
             .andExpect(status().isOk)

diff --git a/web-ui/src/client/app/services/rest.js b/web-ui/src/client/app/services/rest.js
@@ -46,7 +46,7 @@ export const RestService = {
       .fetch(
         `${
           window.env.ZALLY_API_URL
-        }/api-violations/${externalId}?include_api_definition=true`,
+        }/api-violations/${externalId}`,
         options
       )
       .then(response => response.json())