From b760bf0534012f82ccfca3d94548d457e8e3d22a Mon Sep 17 00:00:00 2001 From: Maxim Tschumak Date: Fri, 4 Aug 2017 13:47:54 +0200 Subject: [PATCH] tracking unsupported CLI clients in the server (#464) --- cli/zally/integration_test.go | 10 +++++ .../apireview/ApiViolationsController.java | 31 ++++++++-------- .../zally/apireview/ServerMessageService.java | 32 ++++++++++++++++ server/src/main/resources/application-dev.yml | 3 ++ .../src/main/resources/application-test.yml | 5 +++ server/src/main/resources/application.yml | 4 +- .../apireview/RestApiEmptyMessageTest.java | 25 ------------- .../apireview/RestApiViolationsTest.java | 8 +--- .../apireview/ServerMessageServiceTest.java | 37 +++++++++++++++++++ 9 files changed, 107 insertions(+), 48 deletions(-) create mode 100644 server/src/main/java/de/zalando/zally/apireview/ServerMessageService.java delete mode 100644 server/src/test/java/de/zalando/zally/apireview/RestApiEmptyMessageTest.java create mode 100644 server/src/test/java/de/zalando/zally/apireview/ServerMessageServiceTest.java diff --git a/cli/zally/integration_test.go b/cli/zally/integration_test.go index c4b6eb268..e73106312 100644 --- a/cli/zally/integration_test.go +++ b/cli/zally/integration_test.go @@ -153,3 +153,13 @@ func TestIntegrationDisplayRulesList(t *testing.T) { assert.Nil(t, e) }) } + +func TestIntegrationNotReceiveDeprecationMessage(t *testing.T) { + t.Run("notShowDeprecationMessage", func(t *testing.T) { + out, e := RunAppAndCaptureOutput([]string{"", "rules"}) + + assert.NotContains(t, out, "Please update your CLI:") + + assert.Nil(t, e) + }) +} diff --git a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.java b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.java index c66570b89..3191a3498 100644 --- a/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.java +++ b/server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.java @@ -10,11 +10,11 @@ import de.zalando.zally.rule.ApiValidator; import de.zalando.zally.rule.Violation; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.actuate.metrics.dropwizard.DropwizardMetricServices; import org.springframework.web.bind.annotation.CrossOrigin; 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.ResponseBody; import org.springframework.web.bind.annotation.RestController; @@ -33,31 +33,32 @@ public class ApiViolationsController { private final DropwizardMetricServices metricServices; private final ApiDefinitionReader apiDefinitionReader; private final ApiReviewRepository apiReviewRepository; - private final String message; + private final ServerMessageService serverMessageService; @Autowired public ApiViolationsController(ApiValidator rulesValidator, DropwizardMetricServices metricServices, ApiDefinitionReader apiDefinitionReader, ApiReviewRepository apiReviewRepository, - @Value("${zally.message:}") String message) { + ServerMessageService serverMessageService) { this.rulesValidator = rulesValidator; this.metricServices = metricServices; this.apiDefinitionReader = apiDefinitionReader; this.apiReviewRepository = apiReviewRepository; - this.message = message; + this.serverMessageService = serverMessageService; } @ResponseBody @PostMapping("/api-violations") - public ApiDefinitionResponse validate(@RequestBody ApiDefinitionRequest request) { + public ApiDefinitionResponse validate(@RequestBody ApiDefinitionRequest request, + @RequestHeader(value = "User-Agent", required = false) String userAgent) { metricServices.increment("meter.api-reviews.requested"); String apiDefinition = retrieveApiDefinition(request); List violations = rulesValidator.validate(apiDefinition, request.getIgnoreRules()); apiReviewRepository.save(new ApiReview(request, apiDefinition, violations)); - ApiDefinitionResponse response = buildApiDefinitionResponse(violations); + ApiDefinitionResponse response = buildApiDefinitionResponse(violations, userAgent); metricServices.increment("meter.api-reviews.processed"); return response; } @@ -71,9 +72,9 @@ private String retrieveApiDefinition(ApiDefinitionRequest request) { } } - private ApiDefinitionResponse buildApiDefinitionResponse(List violations) { + private ApiDefinitionResponse buildApiDefinitionResponse(List violations, String userAgent) { ApiDefinitionResponse response = new ApiDefinitionResponse(); - response.setMessage(message); + response.setMessage(serverMessageService.serverMessage(userAgent)); response.setViolations(violations.stream().map(this::toDto).collect(toList())); response.setViolationsCount(buildViolationsCount(violations)); return response; @@ -81,19 +82,19 @@ private ApiDefinitionResponse buildApiDefinitionResponse(List violati private ViolationDTO toDto(Violation violation) { return new ViolationDTO( - violation.getTitle(), - violation.getDescription(), - violation.getViolationType(), - violation.getRuleLink(), - violation.getPaths() + violation.getTitle(), + violation.getDescription(), + violation.getViolationType(), + violation.getRuleLink(), + violation.getPaths() ); } private Map buildViolationsCount(List violations) { ViolationsCounter counter = new ViolationsCounter(violations); return Arrays.stream(ViolationType.values()).collect(toMap( - violationType -> violationType.toString().toLowerCase(), - counter::getCounter + violationType -> violationType.toString().toLowerCase(), + counter::getCounter )); } } diff --git a/server/src/main/java/de/zalando/zally/apireview/ServerMessageService.java b/server/src/main/java/de/zalando/zally/apireview/ServerMessageService.java new file mode 100644 index 000000000..eb4709cb2 --- /dev/null +++ b/server/src/main/java/de/zalando/zally/apireview/ServerMessageService.java @@ -0,0 +1,32 @@ +package de.zalando.zally.apireview; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; + +import java.util.ArrayList; +import java.util.List; + +@Service +class ServerMessageService { + + private Logger logger = LoggerFactory.getLogger(ServerMessageService.class); + + private List deprecatedCliAgents = new ArrayList<>(); + private String releasesPage; + + ServerMessageService(@Value("#{'${zally.cli.deprecatedCliAgents}'.split(',')}") List deprecatedCliAgents, + @Value("${zally.cli.releasesPage:}") String releasesPage){ + this.deprecatedCliAgents = deprecatedCliAgents; + this.releasesPage = releasesPage; + } + + protected String serverMessage(String userAgent) { + if (userAgent != null && !userAgent.isEmpty() && deprecatedCliAgents.contains(userAgent)) { + logger.info("received request from user-agent {}", userAgent); + return "Please update your CLI: " + releasesPage; + } + return ""; + } +} diff --git a/server/src/main/resources/application-dev.yml b/server/src/main/resources/application-dev.yml index 5abc7aaee..d4b3289a8 100644 --- a/server/src/main/resources/application-dev.yml +++ b/server/src/main/resources/application-dev.yml @@ -27,3 +27,6 @@ management.port: 8080 zally: ignoreRules: S001,S010 + cli: + releasesPage: https://github.com/zalando-incubator/zally/releases + deprecatedCliAgents: unirest-java/1.3.11,Zally-CLI/1.0 diff --git a/server/src/main/resources/application-test.yml b/server/src/main/resources/application-test.yml index 581a68e23..6991d9d77 100644 --- a/server/src/main/resources/application-test.yml +++ b/server/src/main/resources/application-test.yml @@ -26,3 +26,8 @@ twintip: yaml: "classpath:/api/zally-api.yaml" rules-config-path: "rules-config-test.conf" + +zally: + cli: + releasesPage: https://github.com/zalando-incubator/zally/releases + deprecatedCliAgents: unirest-java/1.3.11,Zally-CLI/1.0 \ No newline at end of file diff --git a/server/src/main/resources/application.yml b/server/src/main/resources/application.yml index 4bda52c88..d03e13f24 100644 --- a/server/src/main/resources/application.yml +++ b/server/src/main/resources/application.yml @@ -36,7 +36,9 @@ rules-config-path: "rules-config.conf" zally: ignoreRules: S001,S010 apiGuidelinesBaseUrl: "http://zalando.github.io/restful-api-guidelines" - + cli: + releasesPage: https://github.com/zalando-incubator/zally/releases + deprecatedCliAgents: unirest-java/1.3.11,Zally-CLI/1.0 --- spring.profiles: local TOKEN_INFO_URI: https://auth.example.com/oauth2/tokeninfo diff --git a/server/src/test/java/de/zalando/zally/apireview/RestApiEmptyMessageTest.java b/server/src/test/java/de/zalando/zally/apireview/RestApiEmptyMessageTest.java deleted file mode 100644 index 46a25a247..000000000 --- a/server/src/test/java/de/zalando/zally/apireview/RestApiEmptyMessageTest.java +++ /dev/null @@ -1,25 +0,0 @@ -package de.zalando.zally.apireview; - -import de.zalando.zally.dto.ApiDefinitionResponse; -import de.zalando.zally.dto.ViolationDTO; -import org.junit.Test; -import org.springframework.test.context.TestPropertySource; - -import java.io.IOException; -import java.util.List; - -import static de.zalando.zally.util.ResourceUtil.readApiDefinition; -import static org.assertj.core.api.Assertions.assertThat; - -@TestPropertySource(properties = "zally.message=") -public class RestApiEmptyMessageTest extends RestApiBaseTest { - - @Test - public void shouldNotContainMessageFieldWhenMessageIsEmpty() throws IOException { - ApiDefinitionResponse response = sendApiDefinition(readApiDefinition("fixtures/api_spp.json")); - - List violations = response.getViolations(); - assertThat(violations).isNotEmpty(); - assertThat(response.getMessage()).isEmpty(); - } -} diff --git a/server/src/test/java/de/zalando/zally/apireview/RestApiViolationsTest.java b/server/src/test/java/de/zalando/zally/apireview/RestApiViolationsTest.java index 6c95b3efe..359c6dcef 100644 --- a/server/src/test/java/de/zalando/zally/apireview/RestApiViolationsTest.java +++ b/server/src/test/java/de/zalando/zally/apireview/RestApiViolationsTest.java @@ -19,7 +19,6 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; -import org.springframework.test.context.TestPropertySource; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.RequestBuilder; @@ -39,11 +38,8 @@ import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.NOT_FOUND; -@TestPropertySource(properties = "zally.message=" + RestApiViolationsTest.TEST_MESSAGE) public class RestApiViolationsTest extends RestApiBaseTest { - public static final String TEST_MESSAGE = "Test message"; - @LocalManagementPort private int managementPort; @@ -69,8 +65,6 @@ public void shouldValidateGivenApiDefinition() throws IOException { assertThat(violations.get(0).getTitle()).isEqualTo("dummy1"); assertThat(violations.get(1).getTitle()).isEqualTo("dummy2"); assertThat(violations.get(2).getTitle()).isEqualTo("schema"); - - assertThat(response.getMessage()).isEqualTo(TEST_MESSAGE); } @Test @@ -218,7 +212,7 @@ public void shouldStoreUnsuccessfulApiReviewRequest() { } @Test - public void shouldAcceptYamlAndRespondwithJson() throws Exception { + public void shouldAcceptYamlAndRespondWithJson() throws Exception { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(wac).build(); RequestBuilder requestBuilder = MockMvcRequestBuilders.post("/api-violations") .contentType(WebMvcConfiguration.MEDIA_TYPE_APP_XYAML) diff --git a/server/src/test/java/de/zalando/zally/apireview/ServerMessageServiceTest.java b/server/src/test/java/de/zalando/zally/apireview/ServerMessageServiceTest.java new file mode 100644 index 000000000..a79898ec7 --- /dev/null +++ b/server/src/test/java/de/zalando/zally/apireview/ServerMessageServiceTest.java @@ -0,0 +1,37 @@ +package de.zalando.zally.apireview; + +import org.junit.Test; + +import java.util.Arrays; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ServerMessageServiceTest { + + private final ServerMessageService serverMessageService = new ServerMessageService( + Arrays.asList("unirest-java/1.3.11", "Zally-CLI/1.0"), + "https://github.com/zalando-incubator/zally/releases"); + + @Test + public void shouldReturnEmptyStringIfUserAgentIsUpToDate() throws Exception { + String message = serverMessageService.serverMessage("Zally-CLI/1.1"); + + assertThat(message).isEqualTo(""); + } + + @Test + public void shouldReturnDeprecationMessageIfUserAgentIsDeprecated() throws Exception { + String message = serverMessageService.serverMessage("Zally-CLI/1.0"); + + assertThat(message).isEqualTo("Please update your CLI: https://github.com/zalando-incubator/zally/releases"); + } + + @Test + public void shouldReturnDeprecationMessageIfUserAgentIsNotSet() throws Exception { + String messageIfNull = serverMessageService.serverMessage(null); + String messageIfEmptyString = serverMessageService.serverMessage(""); + + assertThat(messageIfNull).isEqualTo(""); + assertThat(messageIfEmptyString).isEqualTo(""); + } +}