Ensure uniqueness of diagnosis keys (#388)

* wip

* overwriting default save is working, needs some cleanup still.

* save test data again

* fix tests + write tests for duplicate detection

* Add NativeNamedQuery

* Stuff that werks!

* create migration scripts

* make key_data not nullable

* test

* Add Javadoc

* Add static modifier

* Fix failing test

Co-authored-by: Michael Burwig <michael.burwig@sap.com>

common/persistence/pom.xml

@@ -78,7 +78,7 @@
     <dependency>
       <groupId>com.h2database</groupId>
       <artifactId>h2</artifactId>
-      <version>1.4.199</version>
+      <version>1.4.200</version>
       <scope>runtime</scope>
     </dependency>
     <dependency>

common/persistence/src/main/java/app/coronawarn/server/common/persistence/domain/DiagnosisKey.java

@@ -29,9 +29,8 @@
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.Set;
+import javax.persistence.Column;
 import javax.persistence.Entity;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
 import javax.validation.ConstraintViolation;
@@ -57,10 +56,8 @@ public class DiagnosisKey {
   private static final Validator VALIDATOR = Validation.buildDefaultValidatorFactory().getValidator();
 
   @Id
-  @GeneratedValue(strategy = GenerationType.IDENTITY)
-  private Long id;
-
   @Size(min = 16, max = 16, message = "Key data must be a byte array of length 16.")
+  @Column(unique = true)
   private byte[] keyData;
 
   @ValidRollingStartIntervalNumber
@@ -100,10 +97,6 @@ public static Builder builder() {
     return new DiagnosisKeyBuilder();
   }
 
-  public Long getId() {
-    return id;
-  }
-
   /**
    * Returns the diagnosis key.
    */
@@ -189,14 +182,13 @@ public boolean equals(Object o) {
         && rollingPeriod == that.rollingPeriod
         && transmissionRiskLevel == that.transmissionRiskLevel
         && submissionTimestamp == that.submissionTimestamp
-        && Objects.equals(id, that.id)
         && Arrays.equals(keyData, that.keyData);
   }
 
   @Override
   public int hashCode() {
     int result = Objects
-        .hash(id, rollingStartIntervalNumber, rollingPeriod, transmissionRiskLevel, submissionTimestamp);
+        .hash(rollingStartIntervalNumber, rollingPeriod, transmissionRiskLevel, submissionTimestamp);
     result = 31 * result + Arrays.hashCode(keyData);
     return result;
   }

common/persistence/src/main/java/app/coronawarn/server/common/persistence/repository/DiagnosisKeyRepository.java

@@ -22,6 +22,8 @@
 
 import app.coronawarn.server.common.persistence.domain.DiagnosisKey;
 import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.data.jpa.repository.Query;
 import org.springframework.stereotype.Repository;
 
 @Repository
@@ -33,4 +35,22 @@ public interface DiagnosisKeyRepository extends JpaRepository<DiagnosisKey, Long
    * @param submissionTimestamp the submission timestamp up to which entries will be deleted.
    */
   void deleteBySubmissionTimestampIsLessThanEqual(long submissionTimestamp);
+
+  /**
+   * Attempts to write the specified diagnosis key information into the database. If a row with the specified key data
+   * already exists, no data is inserted.
+   *
+   * @param keyData                    The key data of the diagnosis key.
+   * @param rollingStartIntervalNumber The rolling start interval number of the diagnosis key.
+   * @param rollingPeriod              The rolling period of the diagnosis key.
+   * @param submissionTimestamp        The submission timestamp of the diagnosis key.
+   * @param transmissionRisk           The transmission risk level of the diagnosis key.
+   */
+  @Modifying
+  @Query(nativeQuery = true, value =
+      "INSERT INTO diagnosis_key"
+          + "(key_data, rolling_start_interval_number, rolling_period, submission_timestamp, transmission_risk_level)"
+          + " VALUES(?, ?, ?, ?, ?) ON CONFLICT DO NOTHING;")
+  void saveDoNothingOnConflict(byte[] keyData, int rollingStartIntervalNumber, int rollingPeriod,
+      long submissionTimestamp, int transmissionRisk);
 }

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

@@ -50,13 +50,19 @@ public DiagnosisKeyService(DiagnosisKeyRepository keyRepository) {
   }
 
   /**
-   * Persists the specified collection of {@link DiagnosisKey} instances.
+   * Persists the specified collection of {@link DiagnosisKey} instances. If the key data of a particular diagnosis key
+   * already exists in the database, this diagnosis key is not persisted.
    *
    * @param diagnosisKeys must not contain {@literal null}.
    * @throws IllegalArgumentException in case the given collection contains {@literal null}.
    */
+  @Transactional
   public void saveDiagnosisKeys(Collection<DiagnosisKey> diagnosisKeys) {
-    keyRepository.saveAll(diagnosisKeys);
+    for (DiagnosisKey diagnosisKey : diagnosisKeys) {
+      keyRepository.saveDoNothingOnConflict(
+          diagnosisKey.getKeyData(), diagnosisKey.getRollingStartIntervalNumber(), diagnosisKey.getRollingPeriod(),
+          diagnosisKey.getSubmissionTimestamp(), diagnosisKey.getTransmissionRiskLevel());
+    }
   }
 
   /**

common/persistence/src/main/resources/db/migration/h2/V3__makeKeyDataPrimaryKey.sql

@@ -0,0 +1,3 @@
+ALTER TABLE diagnosis_key DROP COLUMN id;
+ALTER TABLE diagnosis_key ALTER COLUMN key_data SET NOT NULL;
+ALTER TABLE diagnosis_key ADD PRIMARY KEY (key_data);

common/persistence/src/main/resources/db/migration/postgres/V3__makeKeyDataPrimaryKey.sql

@@ -0,0 +1,3 @@
+ALTER TABLE diagnosis_key DROP COLUMN id;
+ALTER TABLE diagnosis_key ALTER COLUMN key_data SET NOT NULL;
+ALTER TABLE diagnosis_key ADD PRIMARY KEY (key_data);

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

@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Random;
 import org.assertj.core.util.Lists;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.DisplayName;
@@ -149,9 +150,34 @@ void testNoPersistOnValidationError() {
     assertDiagnosisKeysEqual(Lists.emptyList(), actKeys);
   }
 
+  @Test
+  void shouldNotUpdateExistingKey() {
+    var keyData = "1234567890123456";
+    var keys = List.of(DiagnosisKey.builder()
+            .withKeyData(keyData.getBytes())
+            .withRollingStartIntervalNumber(600)
+            .withTransmissionRiskLevel(2)
+            .withSubmissionTimestamp(0L).build(),
+        DiagnosisKey.builder()
+            .withKeyData(keyData.getBytes())
+            .withRollingStartIntervalNumber(600)
+            .withTransmissionRiskLevel(3)
+            .withSubmissionTimestamp(0L).build());
+
+    diagnosisKeyService.saveDiagnosisKeys(keys);
+
+    var actKeys = diagnosisKeyService.getDiagnosisKeys();
+
+    assertThat(actKeys.size()).isEqualTo(1);
+    assertThat(actKeys.iterator().next().getTransmissionRiskLevel()).isEqualTo(2);
+  }
+
   public static DiagnosisKey buildDiagnosisKeyForSubmissionTimestamp(long submissionTimeStamp) {
+    byte[] randomBytes = new byte[16];
+    Random random = new Random(submissionTimeStamp);
+    random.nextBytes(randomBytes);
     return DiagnosisKey.builder()
-        .withKeyData(new byte[16])
+        .withKeyData(randomBytes)
         .withRollingStartIntervalNumber(600)
         .withTransmissionRiskLevel(2)
         .withSubmissionTimestamp(submissionTimeStamp).build();

common/persistence/src/test/resources/application.yaml

@@ -1,5 +1,8 @@
 ---
 spring:
+  test:
+    database:
+      replace: none
   flyway:
     enabled: false
   jpa:
@@ -8,6 +11,10 @@ spring:
     properties:
       hibernate:
         show_sql: false
+  datasource:
+    url: jdbc:h2:mem:test;MODE=PostgreSQL
+    driverClassName: org.h2.Driver
+    platform: h2
   main:
     banner-mode: off
 logging:

services/submission/src/test/resources/application.yaml

@@ -11,7 +11,13 @@ spring:
     enabled: true
     # default case is H2 - value will be overwritten by profile cloud or postgres
     locations: classpath:db/migration/h2
-
+  datasource:
+    url: jdbc:h2:mem:test;MODE=PostgreSQL
+    driverClassName: org.h2.Driver
+    platform: h2
+  test:
+    database:
+      replace: none
   jpa:
     hibernate:
       ddl-auto: validate