Merge pull request #467 from zalando-incubator/track-old-clients

Tracking old CLI clients in the server.
zalando · Aug 8, 2017 · afdae1b · afdae1b
2 parents 972720b + b760bf0
commit afdae1b
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 48 deletions.
diff --git 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
@@ -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<Violation> 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,29 +72,29 @@ private String retrieveApiDefinition(ApiDefinitionRequest request) {
         }
     }
 
-    private ApiDefinitionResponse buildApiDefinitionResponse(List<Violation> violations) {
+    private ApiDefinitionResponse buildApiDefinitionResponse(List<Violation> 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;
     }
 
     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<String, Integer> buildViolationsCount(List<Violation> 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
@@ -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<String> deprecatedCliAgents = new ArrayList<>();
+    private String releasesPage;
+
+    ServerMessageService(@Value("#{'${zally.cli.deprecatedCliAgents}'.split(',')}") List<String> 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
@@ -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
@@ -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
diff --git 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
diff --git 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
@@ -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("");
+    }
+}