fix NPE issue with http HEAD (#1809)

* feign can't handle empty/null reponse body :-(

* move DB truncation into the same transaction as the insert, otherwise we might run into inconsistency!

* RetentionPolicyDccRevocationTest

* add DccRevocationClientDelegatorTest

common/persistence/src/main/java/app/coronawarn/server/common/persistence/service/DccRevocationListService.java

@@ -60,6 +60,8 @@ public Collection<RevocationEntry> getRevocationListEntries() {
   @Timed
   @Transactional
   public void store(final Collection<RevocationEntry> revocationEntries) {
+    logger.info("Truncate Revocation list...");
+    repository.truncate();
     logger.info("Saving Revocation list entries...");
     for (final RevocationEntry entry : revocationEntries) {
       repository.saveDoNothingOnConflict(entry.getKid(), entry.getType(), entry.getHash());
@@ -79,8 +81,4 @@ public void store(final RevocationEtag etag) {
     }
     etagRepository.save(etag.getPath(), etag.getEtag());
   }
-
-  public void truncate() {
-    repository.truncate();
-  }
 }

common/persistence/src/test/java/app/coronawarn/server/common/persistence/service/DccRevocationListServiceTest.java

@@ -9,7 +9,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.data.jdbc.DataJdbcTest;
@@ -20,11 +19,6 @@ class DccRevocationListServiceTest {
   @Autowired
   private DccRevocationListService service;
 
-  @AfterEach
-  public void tearDown() {
-    service.truncate();
-  }
-
   @Test
   void testStore() {
     Collection<RevocationEntry> some = new ArrayList<>();

runconfigs/Eclipse/spring-boot-dcc_revocation.launch

@@ -31,7 +31,7 @@
         <mapEntry key="POSTGRES_PASSWORD" value="postgres"/>
         <mapEntry key="POSTGRES_USER" value="postgres"/>
         <mapEntry key="SECRET_PRIVATE" value="file:/secrets/private.pem"/>
-        <mapEntry key="SPRING_PROFILES_ACTIVE" value="testdata,disable-ssl-client-postgres,signature-dev,local-json-stats,revocation"/>
+        <mapEntry key="SPRING_PROFILES_ACTIVE" value="testdata,disable-ssl-client-postgres,signature-dev,local-json-stats,revocation,dsc-rev-client-factory"/>
         <mapEntry key="STATISTICS_FILE_ACCESS_KEY_ID" value="fakeAccessKey"/>
         <mapEntry key="STATISTICS_FILE_S3_BUCKET" value="obs-cwa-public-dev"/>
         <mapEntry key="STATISTICS_FILE_S3_ENDPOINT" value="http://localhost:8013"/>

services/distribution/src/main/java/app/coronawarn/server/services/distribution/dcc/CloudDccRevocationFeignHttpClientProvider.java

@@ -60,8 +60,7 @@ public CloudDccRevocationFeignHttpClientProvider(DistributionServiceConfig confi
   @Override
   @Bean
   public Client createDccRevocationFeignClient() {
-    return new ApacheHttpClient(
-        dccHttpClientFactory().createBuilder().build());
+    return new DccRevocationClientDelegator(new ApacheHttpClient(dccHttpClientFactory().createBuilder().build()));
   }
 
   /**
@@ -73,7 +72,7 @@ private SSLContext getSslContext(File trustStorePath, String trustStorePass) {
     logger.info("Instantiating DCC Revocation client - SSL context with truststore: {}", trustStorePath.getName());
     try {
       return SSLContextBuilder.create().loadTrustMaterial(trustStorePath,
-              emptyCharrArrayIfNull(trustStorePass))
+          emptyCharrArrayIfNull(trustStorePass))
           .build();
     } catch (Exception e) {
       logger.error("Problem on creating DCC Revocation client - SSL context with truststore: "

services/distribution/src/main/java/app/coronawarn/server/services/distribution/dcc/DccRevocationClientDelegator.java

@@ -0,0 +1,43 @@
+package app.coronawarn.server.services.distribution.dcc;
+
+import feign.Client;
+import feign.Request;
+import feign.Request.Options;
+import feign.Response;
+import feign.Response.Body;
+import feign.httpclient.ApacheHttpClient;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DccRevocationClientDelegator implements Client {
+
+  private static final Logger logger = LoggerFactory.getLogger(DccRevocationClientDelegator.class);
+
+  private final ApacheHttpClient apacheHttpClient;
+
+  public DccRevocationClientDelegator(final ApacheHttpClient apacheHttpClient) {
+    this.apacheHttpClient = apacheHttpClient;
+  }
+
+  @Override
+  public Response execute(final Request request, final Options options) throws IOException {
+    final Response response = apacheHttpClient.execute(request, options);
+
+    // in case of http HEAD the response is NULL!
+    final Body body = response.body();
+    if (body != null) {
+      return response;
+    } else {
+      logger.info("response body is null for '{}'", request);
+    }
+
+    return Response.builder()
+        .status(response.status())
+        .reason(response.reason())
+        .headers(response.headers())
+        .request(response.request())
+        .body("", StandardCharsets.UTF_8).build();
+  }
+}

services/distribution/src/main/java/app/coronawarn/server/services/distribution/dcc/ProdDccRevocationClient.java

@@ -52,6 +52,7 @@ public String getETag() throws FetchDccListException {
       etag = getETag(dccRevocationFeignClient.head());
       return etag;
     } catch (final Exception e) {
+      logger.error(e.getMessage(), e);
       throw new FetchDccListException("http-HEAD for DCC Revocation List failed", e);
     }
   }
@@ -64,8 +65,8 @@ public String getETag() throws FetchDccListException {
    * @throws NullPointerException if there is no ETag in the {@link ResponseEntity#getHeaders()}.
    */
   public static String getETag(final ResponseEntity<?> response) {
-    final String string = response.getHeaders().getETag().replaceAll("\"", "");
-    logger.info("got DCC Revocation List ETag: {}", string);
+    final String string = response.getHeaders().getETag();
+    logger.info("got DCC Revocation List ETag: '{}'", string);
     return string;
   }
 }

services/distribution/src/main/java/app/coronawarn/server/services/distribution/dcc/TestDccRevocationClient.java

@@ -41,6 +41,6 @@ public Optional<List<RevocationEntry>> getDccRevocationList() throws FetchDccLis
 
   @Override
   public String getETag() throws FetchDccListException {
-    return "62620c23-2953";
+    return "62620c23-test";
   }
 }

services/distribution/src/main/java/app/coronawarn/server/services/distribution/dcc/decode/DccRevocationListDecoder.java

@@ -15,11 +15,15 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
 
 @Component
 public class DccRevocationListDecoder {
 
+  private static final Logger logger = LoggerFactory.getLogger(DccRevocationListDecoder.class);
+
   private DistributionServiceConfig distributionServiceConfig;
 
   public DccRevocationListDecoder(DistributionServiceConfig distributionServiceConfig) {
@@ -58,6 +62,7 @@ public List<RevocationEntry> decode(byte[] data) throws DccRevocationListDecodeE
             revocationEntries.add(new RevocationEntry(kid, type, hash)));
       });
     } catch (Exception e) {
+      logger.error(e.getMessage(), e);
       throw new DccRevocationListDecodeException("DCC revocation list NOT decoded.", e);
     }
     return revocationEntries;

services/distribution/src/main/java/app/coronawarn/server/services/distribution/runner/RetentionPolicy.java

@@ -87,9 +87,9 @@ public void run(ApplicationArguments args) {
         if (dccRevocationListService.etagExists(dccRevocationClient.getETag())) {
           logger.info("DCC Revocation - ETag didn't change, nothing to do, shutting down.");
           SpringApplication.exit(applicationContext);
+          System.exit(0);
           return;
         }
-        dccRevocationListService.truncate();
         s3RetentionPolicy.deleteDccRevocationDir();
       } else {
         diagnosisKeyService.applyRetentionPolicy(retentionDays);

services/distribution/src/test/java/app/coronawarn/server/services/distribution/dcc/DccRevocationClientDelegatorTest.java

@@ -0,0 +1,53 @@
+package app.coronawarn.server.services.distribution.dcc;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import feign.Request;
+import feign.Request.Body;
+import feign.Request.HttpMethod;
+import feign.Request.Options;
+import feign.RequestTemplate;
+import feign.Response;
+import feign.httpclient.ApacheHttpClient;
+import java.util.Collections;
+import org.junit.jupiter.api.Test;
+
+class DccRevocationClientDelegatorTest {
+
+  @Test
+  void nullBodyShouldBeTurnedIntoEmptyString() throws Exception {
+    final ApacheHttpClient client = mock(ApacheHttpClient.class);
+    final Request request = Request.create(HttpMethod.GET, "http://localhost", Collections.emptyMap(), Body.empty(),
+        (RequestTemplate) null);
+    final Response mockResponse = Response.builder().request(request).body((Response.Body) null).build();
+    when(client.execute(any(), any())).thenReturn(mockResponse);
+    final DccRevocationClientDelegator fixture = new DccRevocationClientDelegator(client);
+    assertNull(mockResponse.body());
+    final Response response = fixture.execute(request, new Options());
+    assertNotNull(response.body());
+  }
+
+  @Test
+  void responseIsNotChangedIfBodyIsNotNull() throws Exception {
+    final ApacheHttpClient client = mock(ApacheHttpClient.class);
+    final Request request = Request.create(HttpMethod.GET, "http://localhost", Collections.emptyMap(), Body.empty(),
+        (RequestTemplate) null);
+    final Response mockResponse = Response.builder().request(request).body("foo".getBytes()).build();
+    when(client.execute(any(), any())).thenReturn(mockResponse);
+    final DccRevocationClientDelegator fixture = new DccRevocationClientDelegator(client);
+    assertNotNull(mockResponse.body());
+    final Response response = fixture.execute(request, new Options());
+    assertEquals(mockResponse, response);
+  }
+
+  @Test
+  void testDccRevocationClientDelegator() {
+    final DccRevocationClientDelegator fixture = new DccRevocationClientDelegator(new ApacheHttpClient());
+    assertNotNull(fixture);
+  }
+}

services/distribution/src/test/java/app/coronawarn/server/services/distribution/dcc/DccRevocationClientUnitTest.java

@@ -48,7 +48,7 @@ void coverTestDccRevocationClient2() throws Exception {
 
   @Test
   void coverTestDccRevocationClient3() throws Exception {
-    assertEquals("62620c23-2953", testDccRevocationClient.getETag());
+    assertEquals("62620c23-test", testDccRevocationClient.getETag());
   }
 
   @Test

services/distribution/src/test/java/app/coronawarn/server/services/distribution/dcc/DccRevocationListIT.java

@@ -44,6 +44,6 @@ void should_fetch_dcc_revocation_list() throws FetchDccListException {
   void should_fetch_etag() throws FetchDccListException {
     final String etag = dccRevocationClient.getETag();
     assertThat(etag).isNotBlank();
-    assertThat(etag).doesNotContain("\"");
+    assertThat(etag).doesNotContain("test");
   }
 }

services/distribution/src/test/java/app/coronawarn/server/services/distribution/runner/RetentionPolicyTestRevocation.java → services/distribution/src/test/java/app/coronawarn/server/services/distribution/runner/RetentionPolicyDccRevocationTest.java

@@ -1,30 +1,33 @@
 package app.coronawarn.server.services.distribution.runner;
 
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-
 import app.coronawarn.server.common.persistence.service.DccRevocationListService;
 import app.coronawarn.server.common.persistence.service.DiagnosisKeyService;
 import app.coronawarn.server.common.persistence.service.StatisticsDownloadService;
 import app.coronawarn.server.common.persistence.service.TraceTimeIntervalWarningService;
 import app.coronawarn.server.services.distribution.config.DistributionServiceConfig;
 import app.coronawarn.server.services.distribution.dcc.DccRevocationClient;
+import app.coronawarn.server.services.distribution.dcc.FetchDccListException;
 import app.coronawarn.server.services.distribution.objectstore.S3RetentionPolicy;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.SpringApplication;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
 import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.context.ApplicationContext;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
+import static org.mockito.Mockito.*;
+
 @EnableConfigurationProperties(value = DistributionServiceConfig.class)
 @ExtendWith(SpringExtension.class)
 @ContextConfiguration(classes = { RetentionPolicy.class }, initializers = ConfigDataApplicationContextInitializer.class)
 @ActiveProfiles("revocation")
-class RetentionPolicyTestRevocation {
+class RetentionPolicyDccRevocationTest {
 
   @MockBean
   DiagnosisKeyService diagnosisKeyService;
@@ -50,6 +53,9 @@ class RetentionPolicyTestRevocation {
   @MockBean
   DccRevocationClient dccRevocationClient;
 
+  @MockBean
+  ApplicationContext applicationContext;
+
   @Test
   void shouldCallDatabaseAndS3RetentionRunner() {
     retentionPolicy.run(null);
@@ -60,7 +66,16 @@ void shouldCallDatabaseAndS3RetentionRunner() {
     verify(traceTimeIntervalWarningService, times(0))
         .applyRetentionPolicy(distributionServiceConfig.getRetentionDays());
 
-    verify(dccRevocationListService, times(1)).truncate();
+    verify(s3RetentionPolicy, times(1)).deleteDccRevocationDir();
+  }
+
+  @Test
+  void shouldCheckForEtag() throws FetchDccListException {
+    retentionPolicy.run(null);
+
+    when(dccRevocationListService.etagExists(dccRevocationClient.getETag())).thenReturn(true);
+    Assertions.assertThat(retentionPolicy.isDccRevocation()).isTrue();
+    Assertions.assertThat(SpringApplication.exit(applicationContext));
     verify(s3RetentionPolicy, times(1)).deleteDccRevocationDir();
   }
 }