[JN-474] Various clean-ups and fixes based on integration testing with Pepper (#539)

breilly2 · web-flow · commit f5a8fa504e5b · 2023-09-12T19:08:57.000Z
diff --git a/api-admin/src/main/java/bio/terra/pearl/api/admin/controller/kit/KitController.java b/api-admin/src/main/java/bio/terra/pearl/api/admin/controller/kit/KitController.java
@@ -53,7 +53,8 @@ public ResponseEntity<Object> requestKit(
       String kitType) {
     AdminUser adminUser = authUtilService.requireAdminUser(request);
     try {
-      KitRequest sampleKit = kitExtService.requestKit(adminUser, enrolleeShortcode, kitType);
+      KitRequest sampleKit =
+          kitExtService.requestKit(adminUser, studyShortcode, enrolleeShortcode, kitType);
       return ResponseEntity.ok(sampleKit);
     } catch (PepperException e) {
       log.error("Error requesting sample kit from Pepper", e);
diff --git a/api-admin/src/main/java/bio/terra/pearl/api/admin/service/kit/KitExtService.java b/api-admin/src/main/java/bio/terra/pearl/api/admin/service/kit/KitExtService.java
@@ -37,7 +37,7 @@ public List<KitRequest> requestKits(
     authUtilService.authUserToStudy(adminUser, portalShortcode, studyShortcode);
 
     return enrolleeShortcodes.stream()
-        .map(enrolleeShortcode -> requestKit(adminUser, enrolleeShortcode, kitType))
+        .map(enrolleeShortcode -> requestKit(adminUser, studyShortcode, enrolleeShortcode, kitType))
         .toList();
   }
 
@@ -54,9 +54,10 @@ public Collection<KitRequest> getKitRequestsByStudyEnvironment(
     return kitRequestService.getSampleKitsByStudyEnvironment(studyEnvironment);
   }
 
-  public KitRequest requestKit(AdminUser adminUser, String enrolleeShortcode, String kitTypeName) {
+  public KitRequest requestKit(
+      AdminUser adminUser, String studyShortcode, String enrolleeShortcode, String kitTypeName) {
     Enrollee enrollee = authUtilService.authAdminUserToEnrollee(adminUser, enrolleeShortcode);
-    return kitRequestService.requestKit(adminUser, enrollee, kitTypeName);
+    return kitRequestService.requestKit(adminUser, studyShortcode, enrollee, kitTypeName);
   }
 
   public Collection<KitRequest> getKitRequests(AdminUser adminUser, String enrolleeShortcode) {
diff --git a/api-admin/src/main/resources/application.yml b/api-admin/src/main/resources/application.yml
@@ -22,7 +22,8 @@ env:
     deploymentZone: ${DEPLOYMENT_ZONE:}
     tdrExportEnabled: ${TDR_EXPORT_ENABLED:false}
   dsm:
-    basePath: ${DSM_ADDRESS:https://dsm-dev.datadonationplatform.org/ddp}
+    basePath: ${DSM_ADDRESS:https://dsm-dev.datadonationplatform.org/dsm}
+    issuerClaim: ${DSM_JWT_ISSUER:admin-d2p.ddp-dev.envs.broadinstitute.org}
     secret: ${DSM_JWT_SIGNING_SECRET:}
   populate:
     populate-from-classpath: true
diff --git a/core/build.gradle b/core/build.gradle
@@ -25,6 +25,7 @@ dependencies {
     implementation group: 'org.jdbi', name: 'jdbi3-postgres', version: '3.34.0'
     implementation 'org.springframework.boot:spring-boot-starter-data-jdbc:2.7.10'
     implementation 'org.springframework.boot:spring-boot-starter-webflux:2.7.10'
+    implementation 'org.springframework.boot:spring-boot-starter-validation:2.7.14'
     implementation 'org.springframework.retry:spring-retry:1.3.4'
     implementation 'org.apache.commons:commons-text:1.10.0'
     implementation 'commons-beanutils:commons-beanutils:1.9.4'
@@ -48,6 +49,7 @@ dependencies {
     testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.1'
     testImplementation 'org.springframework.boot:spring-boot-starter-test:2.7.10'
     testImplementation 'com.squareup.okhttp3:mockwebserver:4.11.0'
+    testImplementation 'com.github.seregamorph:hamcrest-more-matchers:0.1'
 
     testFixturesImplementation 'org.springframework.boot:spring-boot-starter-test:2.7.10'
     testFixturesImplementation 'org.apache.commons:commons-text:1.10.0'
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/KitRequestService.java b/core/src/main/java/bio/terra/pearl/core/service/kit/KitRequestService.java
@@ -72,15 +72,15 @@ public KitRequestService(KitRequestDao dao,
      * transaction to help avoid that potential inconsistency.
      */
     @Transactional
-    public KitRequest requestKit(AdminUser adminUser, Enrollee enrollee, String kitTypeName) {
+    public KitRequest requestKit(AdminUser adminUser, String studyShortcode, Enrollee enrollee, String kitTypeName) {
         // create and save kit request
         Profile profile = profileService.loadWithMailingAddress(enrollee.getProfileId())
                 .orElseThrow(() -> new RuntimeException("Missing profile for enrollee: " + enrollee.getShortcode()));
         PepperKitAddress pepperKitAddress = makePepperKitAddress(profile);
         var kitRequest = createKitRequest(adminUser, enrollee, pepperKitAddress, kitTypeName);
 
         // send kit request to DSM
-        var result = pepperDSMClient.sendKitRequest(enrollee, kitRequest, pepperKitAddress);
+        var result = pepperDSMClient.sendKitRequest(studyShortcode, enrollee, kitRequest, pepperKitAddress);
 
         // save DSM response/status with Juniper KitRequest
         kitRequest.setDsmStatus(result);
@@ -163,7 +163,7 @@ public void syncAllKitStatusesFromPepper() {
             var pepperKitStatuses = pepperDSMClient.fetchKitStatusByStudy(study.getShortcode());
             var pepperStatusFetchedAt = Instant.now();
             var pepperKitStatusByKitId = pepperKitStatuses.stream().collect(
-                    Collectors.toMap(PepperKitStatus::getKitId, Function.identity()));
+                    Collectors.toMap(PepperKitStatus::getJuniperKitId, Function.identity()));
 
             var studyEnvironments = studyEnvironmentService.findByStudy(study.getId());
             for (StudyEnvironment studyEnvironment : studyEnvironments) {
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/LivePepperDSMClient.java b/core/src/main/java/bio/terra/pearl/core/service/kit/LivePepperDSMClient.java
@@ -14,35 +14,41 @@
 import org.springframework.web.reactive.function.client.WebClient;
 import reactor.core.publisher.Mono;
 
+import javax.validation.Validator;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.UUID;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 @Component
 public class LivePepperDSMClient implements PepperDSMClient {
     private final PepperDSMConfig pepperDSMConfig;
     private final WebClient webClient;
     private final ObjectMapper objectMapper;
+    private final Validator validator;
 
     public LivePepperDSMClient(PepperDSMConfig pepperDSMConfig,
                                WebClient.Builder webClientBuilder,
-                               ObjectMapper objectMapper) {
+                               ObjectMapper objectMapper,
+                               Validator validator) {
         this.pepperDSMConfig = pepperDSMConfig;
         this.webClient = webClientBuilder.build();
         this.objectMapper = objectMapper;
+        this.validator = validator;
     }
 
     @Override
-    public String sendKitRequest(Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
-        var request = buildAuthedPostRequest("shipKit", makeKitRequestBody(enrollee, kitRequest, address));
+    public String sendKitRequest(String studyShortcode, Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
+        var request = buildAuthedPostRequest("shipKit", makeKitRequestBody(studyShortcode, enrollee, kitRequest, address));
         return retrieveAndDeserializeResponse(request, String.class);
     }
 
     @Override
     public PepperKitStatus fetchKitStatus(UUID kitRequestId) {
-        var request = buildAuthedGetRequest("kitStatus/kit/%s".formatted(kitRequestId));
+        // TODO: change "juniperKit" to "juniperkit" after DSM updates this path
+        var request = buildAuthedGetRequest("kitstatus/juniperKit/%s".formatted(kitRequestId));
         var response = retrieveAndDeserializeResponse(request, PepperKitStatusResponse.class);
         if (response.getKits().length != 1) {
             throw new PepperException("Expected a single result from fetchKitStatus by ID (%s), got %d".formatted(
@@ -53,7 +59,7 @@ public PepperKitStatus fetchKitStatus(UUID kitRequestId) {
 
     @Override
     public Collection<PepperKitStatus> fetchKitStatusByStudy(String studyShortcode) {
-        var request = buildAuthedGetRequest("kitStatus/study/%s".formatted(makePepperStudyName(studyShortcode)));
+        var request = buildAuthedGetRequest("kitstatus/study/%s".formatted(makePepperStudyName(studyShortcode)));
         var response = retrieveAndDeserializeResponse(request, PepperKitStatusResponse.class);
         return Arrays.asList(response.getKits());
     }
@@ -62,15 +68,15 @@ private String makePepperStudyName(String studyShortcode) {
         return "juniper-" + studyShortcode;
     }
 
-    private String makeKitRequestBody(Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
+    private String makeKitRequestBody(String studyShortcode, Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
         var juniperKitRequest = PepperDSMKitRequest.JuniperKitRequest.builderWithAddress(address)
                 .juniperKitId(kitRequest.getId().toString())
                 .juniperParticipantId(enrollee.getShortcode())
                 .build();
         var pepperDSMKitRequest = PepperDSMKitRequest.builder()
                 .juniperKitRequest(juniperKitRequest)
                 .kitType(kitRequest.getKitType().getName())
-                .juniperStudyId("Juniper-mock-guid")
+                .juniperStudyId("juniper-%s".formatted(studyShortcode))
                 .build();
 
         try {
@@ -108,6 +114,7 @@ private String generateDsmJwt() {
         var now = System.currentTimeMillis();
         var fifteenMinutes = 15 * 60 * 1000;
         return JWT.create()
+                .withIssuer(pepperDSMConfig.getIssuerClaim())
                 .withExpiresAt(Instant.ofEpochMilli(now + fifteenMinutes))
                 .sign(Algorithm.HMAC256(pepperDSMConfig.getSecret()));
     }
@@ -154,23 +161,43 @@ private <T> Function<String, Mono<T>> deserializeTo(Class<T> clazz) {
                 return Mono.just(clazz.cast(body));
             }
             try {
-                return Mono.just(objectMapper.readValue(body, clazz));
+                var object = objectMapper.readValue(body, clazz);
+                validate(object, clazz, body);
+                return Mono.just(object);
             } catch (JsonProcessingException e) {
                 return Mono.error(new PepperException(
                         "Unable to parse response from Pepper as %s: %s".formatted(clazz.getName(), body), e));
             }
         };
     }
 
+    /** Validate returned object based on javax.validation annotations. */
+    private <T> void validate(Object object, Class<T> clazz, String body) {
+        var violations = validator.validate(object);
+        if (!violations.isEmpty()) {
+            throw new PepperException(
+                    "Unexpected %s from Pepper: %s; %s".formatted(
+                            clazz.getName(),
+                            body,
+                            violations.stream()
+                                    .map(violation -> "%s %s".formatted(
+                                            violation.getPropertyPath(),
+                                            violation.getMessage()))
+                                    .collect(Collectors.joining(", "))));
+        }
+    }
+
     @Component
     @Getter
     @Setter
     public static class PepperDSMConfig {
         private String basePath;
+        private String issuerClaim;
         private String secret;
 
         public PepperDSMConfig(Environment environment) {
             this.basePath = environment.getProperty("env.dsm.basePath");
+            this.issuerClaim = environment.getProperty("env.dsm.issuerClaim");
             this.secret = environment.getProperty("env.dsm.secret");
         }
     }
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/PepperDSMClient.java b/core/src/main/java/bio/terra/pearl/core/service/kit/PepperDSMClient.java
@@ -10,13 +10,14 @@ public interface PepperDSMClient {
     /**
      * Sends a sample kit request to Pepper.
      *
-     * @param enrollee   the enrollee to receive the sample kit
-     * @param kitRequest sample kit request details
-     * @param address    mailing address for the sample kit
+     * @param studyShortcode    the shortcode of the Juniper study
+     * @param enrollee          the enrollee to receive the sample kit
+     * @param kitRequest        sample kit request details
+     * @param address           mailing address for the sample kit
      * @return status result from Pepper
      * @throws PepperException on error from Pepper or failure to process the Pepper response
      */
-    String sendKitRequest(Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address);
+    String sendKitRequest(String studyShortcode, Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address);
     PepperKitStatus fetchKitStatus(UUID kitRequestId);
     Collection<PepperKitStatus> fetchKitStatusByStudy(String studyShortcode);
 }
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/PepperErrorResponse.java b/core/src/main/java/bio/terra/pearl/core/service/kit/PepperErrorResponse.java
@@ -1,15 +1,18 @@
 package bio.terra.pearl.core.service.kit;
 
-import com.fasterxml.jackson.annotation.JsonProperty;
 import lombok.*;
 import lombok.experimental.SuperBuilder;
 
+import javax.validation.constraints.NotNull;
+
 @Getter @Setter
 @SuperBuilder @NoArgsConstructor
-@EqualsAndHashCode
+@EqualsAndHashCode(callSuper = true)
 @ToString
-public class PepperErrorResponse {
+public class PepperErrorResponse extends PepperResponse {
+    @NotNull
     private String errorMessage;
+    @NotNull
     private String juniperKitId;
     private String value;
 }
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/PepperKitStatus.java b/core/src/main/java/bio/terra/pearl/core/service/kit/PepperKitStatus.java
@@ -1,14 +1,16 @@
 package bio.terra.pearl.core.service.kit;
 
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import lombok.*;
 import lombok.experimental.SuperBuilder;
 
 @Getter @Setter
 @SuperBuilder @NoArgsConstructor
 @EqualsAndHashCode
 @ToString
+@JsonIgnoreProperties(ignoreUnknown = true)
 public class PepperKitStatus {
-    private String kitId;
+    private String juniperKitId;
     private String currentStatus;
     private String labelDate;
     private String scanDate;
@@ -17,4 +19,5 @@ public class PepperKitStatus {
     private String returnTrackingNumber;
     private String errorMessage;
     private String errorDate;
+    private Boolean error;
 }
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/PepperKitStatusResponse.java b/core/src/main/java/bio/terra/pearl/core/service/kit/PepperKitStatusResponse.java
@@ -7,8 +7,8 @@
 @Setter
 @SuperBuilder
 @NoArgsConstructor
-@EqualsAndHashCode
+@EqualsAndHashCode(callSuper = true)
 @ToString
-public class PepperKitStatusResponse {
+public class PepperKitStatusResponse extends PepperResponse {
     private PepperKitStatus[] kits;
 }
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/PepperResponse.java b/core/src/main/java/bio/terra/pearl/core/service/kit/PepperResponse.java
@@ -0,0 +1,19 @@
+package bio.terra.pearl.core.service.kit;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import lombok.*;
+import lombok.experimental.SuperBuilder;
+
+import javax.validation.constraints.NotNull;
+
+@Getter @Setter
+@SuperBuilder @NoArgsConstructor
+@EqualsAndHashCode
+@ToString
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class PepperResponse {
+    @NotNull
+    @JsonProperty("isError")
+    private Boolean isError;
+}
diff --git a/core/src/main/java/bio/terra/pearl/core/service/kit/StubPepperDSMClient.java b/core/src/main/java/bio/terra/pearl/core/service/kit/StubPepperDSMClient.java
@@ -27,9 +27,9 @@ public StubPepperDSMClient(@Lazy KitRequestService kitRequestService,
     }
 
     @Override
-    public String sendKitRequest(Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
+    public String sendKitRequest(String studyShortcode, Enrollee enrollee, KitRequest kitRequest, PepperKitAddress address) {
         var statusBuilder = PepperKitStatus.builder()
-                .kitId(kitRequest.getId().toString())
+                .juniperKitId(kitRequest.getId().toString())
                 .currentStatus("CREATED");
         try {
             return objectMapper.writeValueAsString(statusBuilder.build());
@@ -41,7 +41,7 @@ public String sendKitRequest(Enrollee enrollee, KitRequest kitRequest, PepperKit
     @Override
     public PepperKitStatus fetchKitStatus(UUID kitRequestId) {
         return PepperKitStatus.builder()
-                .kitId(kitRequestId.toString())
+                .juniperKitId(kitRequestId.toString())
                 .currentStatus("SHIPPED")
                 .build();
     }
@@ -51,7 +51,7 @@ public Collection<PepperKitStatus> fetchKitStatusByStudy(String studyShortcode)
         var studyEnvironment = studyEnvironmentDao.findByStudy(studyShortcode, EnvironmentName.sandbox).get();
         return kitRequestService.findIncompleteKits(studyEnvironment.getId()).stream().map(kit -> {
             PepperKitStatus status = PepperKitStatus.builder()
-                    .kitId(kit.getId().toString())
+                    .juniperKitId(kit.getId().toString())
                     .currentStatus("SHIPPED")
                     .build();
             return status;
diff --git a/core/src/test/java/bio/terra/pearl/core/service/kit/KitRequestServiceTest.java b/core/src/test/java/bio/terra/pearl/core/service/kit/KitRequestServiceTest.java
diff --git a/core/src/test/java/bio/terra/pearl/core/service/kit/LivePepperDSMClientIntegrationTest.java b/core/src/test/java/bio/terra/pearl/core/service/kit/LivePepperDSMClientIntegrationTest.java
diff --git a/core/src/test/java/bio/terra/pearl/core/service/kit/LivePepperDSMClientTest.java b/core/src/test/java/bio/terra/pearl/core/service/kit/LivePepperDSMClientTest.java
diff --git a/core/src/test/java/bio/terra/pearl/core/service/kit/StubPepperDSMClientTest.java b/core/src/test/java/bio/terra/pearl/core/service/kit/StubPepperDSMClientTest.java
diff --git a/core/src/test/resources/application.yml b/core/src/test/resources/application.yml
diff --git a/core/src/testFixtures/java/bio/terra/pearl/core/factory/kit/KitRequestFactory.java b/core/src/testFixtures/java/bio/terra/pearl/core/factory/kit/KitRequestFactory.java

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public List<KitRequest> requestKits(`
`37`	`37`	`authUtilService.authUserToStudy(adminUser, portalShortcode, studyShortcode);`
`38`	`38`
`39`	`39`	`return enrolleeShortcodes.stream()`
`40`		`- .map(enrolleeShortcode -> requestKit(adminUser, enrolleeShortcode, kitType))`
	`40`	`+ .map(enrolleeShortcode -> requestKit(adminUser, studyShortcode, enrolleeShortcode, kitType))`
`41`	`41`	`.toList();`
`42`	`42`	`}`
`43`	`43`
`@@ -54,9 +54,10 @@ public Collection<KitRequest> getKitRequestsByStudyEnvironment(`
`54`	`54`	`return kitRequestService.getSampleKitsByStudyEnvironment(studyEnvironment);`
`55`	`55`	`}`
`56`	`56`
`57`		`- public KitRequest requestKit(AdminUser adminUser, String enrolleeShortcode, String kitTypeName) {`
	`57`	`+ public KitRequest requestKit(`
	`58`	`+ AdminUser adminUser, String studyShortcode, String enrolleeShortcode, String kitTypeName) {`
`58`	`59`	`Enrollee enrollee = authUtilService.authAdminUserToEnrollee(adminUser, enrolleeShortcode);`
`59`		`- return kitRequestService.requestKit(adminUser, enrollee, kitTypeName);`
	`60`	`+ return kitRequestService.requestKit(adminUser, studyShortcode, enrollee, kitTypeName);`
`60`	`61`	`}`
`61`	`62`
`62`	`63`	`public Collection<KitRequest> getKitRequests(AdminUser adminUser, String enrolleeShortcode) {`