diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt b/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt index fc5b20f05..ce48cdaa2 100644 --- a/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt +++ b/server/src/main/java/de/zalando/zally/apireview/ApiReview.kt @@ -25,7 +25,7 @@ import javax.persistence.OneToMany class ApiReview( request: ApiDefinitionRequest, val userAgent: String = "", - @Suppress("CanBeParameter") private val apiDefinition: String, + @Suppress("CanBeParameter") val apiDefinition: String, violations: List = emptyList(), val name: String? = OpenApiHelper.extractApiName(apiDefinition), val apiId: String? = OpenApiHelper.extractApiId(apiDefinition), diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt index 0acd700ab..61b6ab80e 100644 --- a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt +++ b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.kt @@ -101,6 +101,7 @@ class ApiViolationsController( Severity.SHOULD to review.shouldViolations, Severity.MAY to review.mayViolations, Severity.HINT to review.hintViolations - ).map { it.first.name.toLowerCase() to it.second }.toMap() + ).map { it.first.name.toLowerCase() to it.second }.toMap(), + apiDefinition = review.apiDefinition ) } diff --git a/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt b/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt index 4cbdf22f9..d393433a4 100644 --- a/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt +++ b/server/src/main/java/de/zalando/zally/dto/ApiDefinitionResponse.kt @@ -6,5 +6,6 @@ data class ApiDefinitionResponse( val externalId: UUID? = null, val message: String? = null, val violations: List = emptyList(), - val violationsCount: Map = emptyMap() + val violationsCount: Map = emptyMap(), + val apiDefinition: String? ) diff --git a/server/src/main/resources/api/zally-api.yaml b/server/src/main/resources/api/zally-api.yaml index 3f31a4413..4a2d73155 100644 --- a/server/src/main/resources/api/zally-api.yaml +++ b/server/src/main/resources/api/zally-api.yaml @@ -288,6 +288,9 @@ components: description: Custom server message violations_count: $ref: '#/components/schemas/ViolationsCount' + api_definition: + type: string + description: Specification object in OpenAPI format SupportedRulesResponse: type: object diff --git a/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt b/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt index e3aec4d12..dfa9809f4 100644 --- a/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt +++ b/server/src/test/java/de/zalando/zally/apireview/ApiViolationsControllerTest.kt @@ -2,19 +2,24 @@ 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.intellij.lang.annotations.Language import org.junit.Test import org.junit.runner.RunWith import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc import org.springframework.boot.test.context.SpringBootTest +import org.springframework.http.MediaType.APPLICATION_JSON_UTF8 import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.junit4.SpringRunner 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 @RunWith(SpringRunner::class) @@ -36,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 @@ -49,7 +60,7 @@ class ApiViolationsControllerTest { } @Test - fun `getExistingViolationResponse with existing responds NotFound`() { + fun `getExistingViolationResponse with existing responds Ok`() { val location = mvc.perform( post("/api-violations") @@ -64,7 +75,10 @@ class ApiViolationsControllerTest { .accept("application/json") ) .andExpect(status().isOk) - .andExpect(content().string(containsString("https://zalando.github.io/restful-api-guidelines"))) + .andExpect(content().contentType(APPLICATION_JSON_UTF8)) + .andExpect(jsonPath("$.external_id", notNullValue())) + .andExpect(jsonPath("$.violations", notNullValue())) + .andExpect(jsonPath("$.api_definition", notNullValue())) } /** diff --git a/web-ui/src/client/app/components/__tests__/violations.test.jsx b/web-ui/src/client/app/components/__tests__/violations.test.jsx index 1eebe29a1..fa69aaa53 100644 --- a/web-ui/src/client/app/components/__tests__/violations.test.jsx +++ b/web-ui/src/client/app/components/__tests__/violations.test.jsx @@ -52,7 +52,6 @@ describe('Violations component', () => { test('should render empty list of violation', () => { const component = shallow(); expect(component.find('Violation')).toHaveLength(0); - expect(component.find('h3')).toHaveLength(0); }); }); diff --git a/web-ui/src/client/app/components/violations.jsx b/web-ui/src/client/app/components/violations.jsx index ac783e999..794c51ccb 100644 --- a/web-ui/src/client/app/components/violations.jsx +++ b/web-ui/src/client/app/components/violations.jsx @@ -3,13 +3,21 @@ import { If } from './util.jsx'; import { Msg } from './msg.jsx'; import { RuleType } from './rules.jsx'; import FluidContainer from './fluid-container.jsx'; +import { Link } from 'react-router-dom'; export function Violations(props) { return (
- {props.violations.length ?

VIOLATIONS

: ''} +

+ VIOLATIONS + + + + + +

@@ -136,6 +144,7 @@ export function ViolationsResult(props) { dataTestId="if-violations" > diff --git a/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js b/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js index 2c6639a9c..b99873aaf 100644 --- a/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js +++ b/web-ui/src/client/app/containers/__tests__/violations-tab.test.jsx.js @@ -43,6 +43,16 @@ describe('ViolationsTab container component', () => { expect(component.find('Editor')).toHaveLength(1); expect(component.find('Rules')).toHaveLength(0); }); + test('render the Editor tab on /editor/:externalId', () => { + const component = mount( + + + + ); + expect(component.find('URL')).toHaveLength(0); + expect(component.find('Editor')).toHaveLength(1); + expect(component.find('Rules')).toHaveLength(0); + }); test('render the Rules tab on /rules', () => { const component = mount( diff --git a/web-ui/src/client/app/containers/app.jsx b/web-ui/src/client/app/containers/app.jsx index bcefe2f5c..c81322d2d 100644 --- a/web-ui/src/client/app/containers/app.jsx +++ b/web-ui/src/client/app/containers/app.jsx @@ -14,6 +14,7 @@ export function App(props) { Storage, getApiViolationsByURL, getApiViolationsBySchema, + getApiViolationsByExternalId, getSupportedRules, getFile, } = props; @@ -58,6 +59,7 @@ export function App(props) { getSupportedRules={getSupportedRules} getApiViolationsByURL={getApiViolationsByURL} getApiViolationsBySchema={getApiViolationsBySchema} + getApiViolationsByExternalId={getApiViolationsByExternalId} getFile={getFile} Storage={Storage} {...props} diff --git a/web-ui/src/client/app/containers/editor.jsx b/web-ui/src/client/app/containers/editor.jsx index 6f58d5c9e..41e1e8393 100644 --- a/web-ui/src/client/app/containers/editor.jsx +++ b/web-ui/src/client/app/containers/editor.jsx @@ -4,6 +4,7 @@ import { Violations } from './violations.jsx'; import { ViolationsResult } from '../components/violations.jsx'; import { EditorInputForm } from '../components/editor.jsx'; import { Dialog } from '../components/dialog.jsx'; +import { id } from 'brace/worker/json'; export const editorErrorToAnnotations = error => { if (!error || !error.mark) { @@ -22,15 +23,58 @@ export const editorErrorToAnnotations = error => { export class Editor extends Violations { constructor(props) { super(props); + if ( + props && + props.match && + props.match.params && + props.match.params.externalId + ) { + this.state.externalId = props.match.params.externalId; + } else { + this.state.externalId = null; + } this.state.editorDirty = true; - this.state.editorValue = this.Storage.getItem('editor-value') || ''; + this.state.editorValue = + (!this.state.externalId && this.Storage.getItem('editor-value')) || ''; this.handleOnInputValueChange = this.handleOnInputValueChange.bind(this); this.handleFormSubmit = this.handleFormSubmit.bind(this); this.handleHideOverlay = this.handleHideOverlay.bind(this); } componentDidMount() { - this.updateInputValue(this.state.editorValue); + if (this.state.externalId) { + this.getApiViolationsByExternalId(this.state.externalId) + .then(response => { + this.setState({ + pending: false, + ajaxComplete: true, + externalId: response.external_id, + violations: response.violations, + violationsCount: response.violations_count, + editorValue: response.api_definition, + }); + this.Storage.setItem('editor-value', this.state.editorValue); + this.updateInputValue(this.state.editorValue); + }) + .catch(error => { + console.error(error); // eslint-disable-line no-console + this.setState({ + pending: false, + ajaxComplete: true, + error: error.detail || Violations.DEFAULT_ERROR_MESSAGE, + violations: [], + violationsCount: { + could: 0, + hint: 0, + must: 0, + should: 0, + }, + }); + return Promise.reject(error); + }); + } else { + this.updateInputValue(this.state.editorValue); + } } handleFormSubmit(event) { @@ -95,6 +139,7 @@ export class Editor extends Violations { pending={this.state.pending} complete={this.state.ajaxComplete} errorMsgText={this.state.error} + externalId={this.state.externalId} violations={this.state.violations} successMsgTitle={this.state.successMsgTitle} successMsgText={this.state.successMsgText} @@ -107,6 +152,7 @@ export class Editor extends Violations { pending={this.state.pending} complete={this.state.ajaxComplete} errorMsgText={this.state.error} + externalId={this.state.externalId} violations={this.state.violations} successMsgTitle={this.state.successMsgTitle} successMsgText={this.state.successMsgText} diff --git a/web-ui/src/client/app/containers/root.jsx b/web-ui/src/client/app/containers/root.jsx index a92360c84..5215f97c2 100644 --- a/web-ui/src/client/app/containers/root.jsx +++ b/web-ui/src/client/app/containers/root.jsx @@ -18,6 +18,9 @@ export function Root(props) { getApiViolationsBySchema={props.RestService.getApiViolationsBySchema.bind( props.RestService )} + getApiViolationsByExternalId={props.RestService.getApiViolationsByExternalId.bind( + props.RestService + )} getFile={props.RestService.getFile.bind(props.RestService)} Storage={props.Storage} user={props.user} diff --git a/web-ui/src/client/app/containers/url.jsx b/web-ui/src/client/app/containers/url.jsx index 20db0e4a7..4f8fea860 100644 --- a/web-ui/src/client/app/containers/url.jsx +++ b/web-ui/src/client/app/containers/url.jsx @@ -60,6 +60,7 @@ export class URL extends Violations { pending={this.state.pending} complete={this.state.ajaxComplete} errorMsgText={this.state.error} + externalId={this.state.externalId} violations={this.state.violations} successMsgTitle={this.state.successMsgTitle} successMsgText={this.state.successMsgText} diff --git a/web-ui/src/client/app/containers/violations-tab.jsx b/web-ui/src/client/app/containers/violations-tab.jsx index 36b3f76dd..8c79c0543 100644 --- a/web-ui/src/client/app/containers/violations-tab.jsx +++ b/web-ui/src/client/app/containers/violations-tab.jsx @@ -10,6 +10,7 @@ export function ViolationsTab({ authenticated, getApiViolationsByURL, getApiViolationsBySchema, + getApiViolationsByExternalId, getSupportedRules, getFile, Storage, @@ -82,6 +83,18 @@ export function ViolationsTab({ render={props => ( + )} + /> + ( + diff --git a/web-ui/src/client/app/containers/violations.jsx b/web-ui/src/client/app/containers/violations.jsx index 52794b165..a058c9f2e 100644 --- a/web-ui/src/client/app/containers/violations.jsx +++ b/web-ui/src/client/app/containers/violations.jsx @@ -6,12 +6,14 @@ export class Violations extends Component { this.Storage = this.props.Storage; this.getApiViolations = this.props.getApiViolations; + this.getApiViolationsByExternalId = this.props.getApiViolationsByExternalId; this.state = { error: null, pending: false, ajaxComplete: false, inputValue: '', + externalId: null, violations: [], violationsCount: { could: 0, @@ -34,6 +36,7 @@ export class Violations extends Component { this.setState({ pending: false, ajaxComplete: true, + externalId: response.external_id, violations: response.violations, violationsCount: response.violations_count, }); diff --git a/web-ui/src/client/app/services/rest.js b/web-ui/src/client/app/services/rest.js index 49908dd1b..a3e950d03 100644 --- a/web-ui/src/client/app/services/rest.js +++ b/web-ui/src/client/app/services/rest.js @@ -34,6 +34,25 @@ export const RestService = { }); }, + getApiViolationsByExternalId(externalId) { + const options = { + method: 'GET', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + }, + }; + return client + .fetch( + `${ + window.env.ZALLY_API_URL + }/api-violations/${externalId}`, + options + ) + .then(response => response.json()) + .catch(response => response.json().then(body => Promise.reject(body))); + }, + getSupportedRules() { const options = { method: 'GET',