use springboot checkNotModified helper for etags and fix unit tests

admin-ch · Jun 14, 2021 · 18b8ad4 · 18b8ad4
1 parent 57e478b
commit 18b8ad4
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 39 deletions.
diff --git a/...main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/KeyController.java b/...main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/KeyController.java
@@ -26,10 +26,10 @@
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.CrossOrigin;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.context.request.WebRequest;
 
 @Controller
 @RequestMapping("trust/v1/keys")
@@ -97,17 +97,16 @@ private HttpHeaders getKeysUpdatesHeaders(List<ClientCert> dscs) {
     @CrossOrigin(origins = {"https://editor.swagger.io"})
     @GetMapping(value = "list")
     public @ResponseBody ResponseEntity<ActiveCertsResponse> getActiveSignerCertKeyIds(
-            @RequestHeader(value = HttpHeaders.ETAG, required = false) String etag) {
+            WebRequest request) {
         List<String> activeKeyIds = verifierDataService.findActiveDscKeyIds();
 
         // check etag
         String currentEtag = String.valueOf(EtagUtil.getUnsortedListHashcode(activeKeyIds));
-        if (currentEtag.equals(etag)) {
+        if (request.checkNotModified(currentEtag)) {
             return ResponseEntity.status(HttpStatus.NOT_MODIFIED).build();
         }
 
         return ResponseEntity.ok()
-                .header(HttpHeaders.ETAG, currentEtag)
                 .cacheControl(CacheControl.maxAge(CacheUtil.KEYS_LIST_MAX_AGE))
                 .body(new ActiveCertsResponse(activeKeyIds));
     }

diff --git a/...h/admin/bag/covidcertificate/backend/verifier/ws/controller/RevocationListController.java b/...h/admin/bag/covidcertificate/backend/verifier/ws/controller/RevocationListController.java
@@ -29,12 +29,12 @@
 import org.springframework.web.bind.annotation.CrossOrigin;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.client.HttpStatusCodeException;
 import org.springframework.web.client.RestTemplate;
+import org.springframework.web.context.request.WebRequest;
 import org.springframework.web.util.UriComponentsBuilder;
 
 @Controller
@@ -62,21 +62,19 @@ public RevocationListController(String revokedCertsBaseUrl) {
             responseHeaders = {"ETag:etag to set for next request:string"})
     @CrossOrigin(origins = {"https://editor.swagger.io"})
     @GetMapping(value = "/revocationList")
-    public @ResponseBody ResponseEntity<RevocationResponse> getCerts(
-            @RequestHeader(value = HttpHeaders.ETAG, required = false) String etag)
+    public @ResponseBody ResponseEntity<RevocationResponse> getCerts(WebRequest request)
             throws HttpStatusCodeException {
         final var response = new RevocationResponse();
         List<String> revokedCerts = getRevokedCerts();
         response.setRevokedCerts(revokedCerts);
 
         // check etag
         String currentEtag = String.valueOf(EtagUtil.getUnsortedListHashcode(revokedCerts));
-        if (currentEtag.equals(etag)) {
+        if (request.checkNotModified(currentEtag)) {
             return ResponseEntity.status(HttpStatus.NOT_MODIFIED).build();
         }
 
         return ResponseEntity.ok()
-                .header(HttpHeaders.ETAG, currentEtag)
                 .cacheControl(CacheControl.maxAge(CacheUtil.REVOCATION_LIST_MAX_AGE))
                 .body(response);
     }

diff --git a/...ava/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/ValueSetsController.java b/...ava/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/ValueSetsController.java
@@ -23,7 +23,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import org.apache.http.HttpHeaders;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.core.io.ClassPathResource;
@@ -32,9 +31,9 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.context.request.WebRequest;
 
 @Controller
 @RequestMapping("trust/v1")
@@ -97,13 +96,11 @@ private Map readFileAsMap(String path) throws IOException {
             responses = {"200 => value sets", "304 => no changes since last request"},
             responseHeaders = {"ETag:etag to set for next request:string"})
     @GetMapping(value = "/metadata")
-    public @ResponseBody ResponseEntity<ValueSets> getVerificationRules(
-            @RequestHeader(value = HttpHeaders.ETAG, required = false) String etag) {
-        if (valueSetsEtag.equals(etag)) {
+    public @ResponseBody ResponseEntity<ValueSets> getValueSets(WebRequest request) {
+        if (request.checkNotModified(valueSetsEtag)) {
             return ResponseEntity.status(HttpStatus.NOT_MODIFIED).build();
         }
         return ResponseEntity.ok()
-                .header(HttpHeaders.ETAG, valueSetsEtag)
                 .cacheControl(CacheControl.maxAge(CacheUtil.VALUE_SETS_MAX_AGE))
                 .body(valueSets);
     }

diff --git a/...dmin/bag/covidcertificate/backend/verifier/ws/controller/VerificationRulesController.java b/...dmin/bag/covidcertificate/backend/verifier/ws/controller/VerificationRulesController.java
@@ -18,7 +18,6 @@
 import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
 import java.util.Map;
-import org.apache.http.HttpHeaders;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.core.io.ClassPathResource;
@@ -27,9 +26,9 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.context.request.WebRequest;
 
 @Controller
 @RequestMapping("trust/v1")
@@ -55,13 +54,11 @@ public VerificationRulesController() throws IOException, NoSuchAlgorithmExceptio
             },
             responseHeaders = {"ETag:etag to set for next request:string"})
     @GetMapping(value = "/verificationRules")
-    public @ResponseBody ResponseEntity<Map> getVerificationRules(
-            @RequestHeader(value = HttpHeaders.ETAG, required = false) String etag) {
-        if (verificationRulesEtag.equals(etag)) {
+    public @ResponseBody ResponseEntity<Map> getVerificationRules(WebRequest request) {
+        if (request.checkNotModified(verificationRulesEtag)) {
             return ResponseEntity.status(HttpStatus.NOT_MODIFIED).build();
         }
         return ResponseEntity.ok()
-                .header(HttpHeaders.ETAG, verificationRulesEtag)
                 .cacheControl(CacheControl.maxAge(CacheUtil.VERIFICATION_RULES_MAX_AGE))
                 .body(verificationRules);
     }

diff --git a/...min/bag/covidcertificate/backend/verifier/ws/controller/RevocationListControllerTest.java b/...min/bag/covidcertificate/backend/verifier/ws/controller/RevocationListControllerTest.java
@@ -85,29 +85,28 @@ private MockRestServiceServer setupExternalRevocationListMock(int expectedCallCo
 
     @Test
     public void notModifiedTest() throws Exception {
-        String expectedEtag =
-                String.valueOf(EtagUtil.getUnsortedListHashcode(List.of(REVOKED_CERT)));
+        String expectedEtag = "\"" + EtagUtil.getUnsortedListHashcode(List.of(REVOKED_CERT)) + "\"";
 
         // get current etag
         setupExternalRevocationListMock(2);
         MockHttpServletResponse response =
                 mockMvc.perform(
                                 get(revocationListUrl)
                                         .accept(acceptMediaType)
-                                        .header(HttpHeaders.ETAG, "random"))
+                                        .header(HttpHeaders.IF_NONE_MATCH, "random"))
                         .andExpect(status().is2xxSuccessful())
                         .andReturn()
                         .getResponse();
 
         // verify etag
-        String etag = response.getHeader(HttpHeaders.ETAG).replace("\"", "");
+        String etag = response.getHeader(HttpHeaders.ETAG);
         assertEquals(expectedEtag, etag);
 
         // test not modified
         mockMvc.perform(
                         get(revocationListUrl)
                                 .accept(acceptMediaType)
-                                .header(HttpHeaders.ETAG, etag))
+                                .header(HttpHeaders.IF_NONE_MATCH, etag))
                 .andExpect(status().isNotModified())
                 .andReturn()
                 .getResponse();

diff --git a/...ch/admin/bag/covidcertificate/backend/verifier/ws/controller/ValueSetsControllerTest.java b/...ch/admin/bag/covidcertificate/backend/verifier/ws/controller/ValueSetsControllerTest.java
@@ -104,25 +104,30 @@ public void notModifiedTest() throws Exception {
                         .map(p -> "classpath:" + p)
                         .collect(Collectors.toList());
         String expectedEtag =
-                EtagUtil.getSha1HashForFiles(
-                        pathsToValueSets.toArray(new String[pathsToValueSets.size()]));
+                "\""
+                        + EtagUtil.getSha1HashForFiles(
+                                pathsToValueSets.toArray(new String[pathsToValueSets.size()]))
+                        + "\"";
 
         // get current etag
         MockHttpServletResponse response =
                 mockMvc.perform(
                                 get(valueSetsUrl)
                                         .accept(acceptMediaType)
-                                        .header(HttpHeaders.ETAG, "random"))
+                                        .header(HttpHeaders.IF_NONE_MATCH, "random"))
                         .andExpect(status().is2xxSuccessful())
                         .andReturn()
                         .getResponse();
 
         // verify etag
-        String etag = response.getHeader(HttpHeaders.ETAG).replace("\"", "");
+        String etag = response.getHeader(HttpHeaders.ETAG);
         assertEquals(expectedEtag, etag);
 
         // test not modified
-        mockMvc.perform(get(valueSetsUrl).accept(acceptMediaType).header(HttpHeaders.ETAG, etag))
+        mockMvc.perform(
+                        get(valueSetsUrl)
+                                .accept(acceptMediaType)
+                                .header(HttpHeaders.IF_NONE_MATCH, etag))
                 .andExpect(status().isNotModified())
                 .andReturn()
                 .getResponse();

diff --git a/.../bag/covidcertificate/backend/verifier/ws/controller/VerificationRulesControllerTest.java b/.../bag/covidcertificate/backend/verifier/ws/controller/VerificationRulesControllerTest.java
@@ -49,27 +49,28 @@ public void verificationRulesTest() throws Exception {
 
     @Test
     public void notModifiedTest() throws Exception {
-        String expectedEtag = EtagUtil.getSha1HashForFiles(PATH_TO_VERIFICATION_RULES);
+        String expectedEtag =
+                "\"" + EtagUtil.getSha1HashForFiles(PATH_TO_VERIFICATION_RULES) + "\"";
 
         // get current etag
         MockHttpServletResponse response =
                 mockMvc.perform(
                                 get(verificationRulesUrl)
                                         .accept(acceptMediaType)
-                                        .header(HttpHeaders.ETAG, "random"))
+                                        .header(HttpHeaders.IF_NONE_MATCH, "random"))
                         .andExpect(status().is2xxSuccessful())
                         .andReturn()
                         .getResponse();
 
         // verify etag
-        String etag = response.getHeader(HttpHeaders.ETAG).replace("\"", "");
+        String etag = response.getHeader(HttpHeaders.ETAG);
         assertEquals(expectedEtag, etag);
 
         // test not modified
         mockMvc.perform(
                         get(verificationRulesUrl)
                                 .accept(acceptMediaType)
-                                .header(HttpHeaders.ETAG, etag))
+                                .header(HttpHeaders.IF_NONE_MATCH, etag))
                 .andExpect(status().isNotModified())
                 .andReturn()
                 .getResponse();

diff --git a/...verifier/ch-covidcertificate-backend-verifier-ws/src/test/resources/http/verifier-ws.http b/...verifier/ch-covidcertificate-backend-verifier-ws/src/test/resources/http/verifier-ws.http
@@ -10,23 +10,23 @@ Authorization: Bearer {{apiKey}}
 ### get active cert key ids
 GET {{baseUrl}}/v1/keys/list
 Accept: application/json
-ETag: -720957702
+If-None-Match: "-720957702"
 Authorization: Bearer {{apiKey}}
 
 ### get revocation list
 GET {{baseUrl}}/v1/revocationList
 Accept: application/json
-ETag: -103666649
+If-None-Match: "1089905096"
 Authorization: Bearer {{apiKey}}
 
 ### get verification rules
 GET {{baseUrl}}/v1/verificationRules
 Accept: application/json
-ETag: f4915f1428328eb6c239693fc8e6541f68a3f72e
+If-None-Match: "011ec25ca7a4d0c95fe8fd7c33cdeff3654d7bf9"
 Authorization: Bearer {{apiKey}}
 
 ### get value sets
 GET {{baseUrl}}/v1/metadata
 Accept: application/json
-ETag: f4915f1428328eb6c239693fc8e6541f68a3f72e
+If-None-Match: "e8832aef5f37843e52a5acb78c522fa36576a62e"
 Authorization: Bearer {{apiKey}}