OTR(Backend & Frontend) OPHOTRKEH-248: NPE/Enum korjaus kun ONR-tiedo…

…t muuttuneet (#587) * OTR(Backend) OPHOTRKEH-248: NPE fix when interpreter is not found from cache * OTR(Backend) OPHOTRKEH-248: NPE fix when not found from cache. Also fix unknown enum in contact source * OTR(Backend) OPHOTRKEH-248: test for ONR response deserialization * OTR(Backend) OPHOTRKEH-248: test for ONR response deserialization * OTR(Frontend) OPHOTRKEH-248: show alert if interpreters are not synchronized between OTR and ONR [deploy] * OTR(Frontend) OPHOTRKEH-248: mock-up endpoint for missing interpreters [deploy] * OTR(Frontend) OPHOTRKEH-248: error handler for missing interpreters [deploy]
Opetushallitus · Oct 25, 2023 · fb9e894 · fb9e894
1 parent ffb7383
commit fb9e894
Show file tree

Hide file tree

Showing 15 changed files with 338 additions and 49 deletions.
diff --git a/backend/otr/src/main/java/fi/oph/otr/api/clerk/ClerkInterpreterController.java b/backend/otr/src/main/java/fi/oph/otr/api/clerk/ClerkInterpreterController.java
@@ -42,6 +42,12 @@ public List<ClerkInterpreterDTO> listInterpreters() {
     return clerkInterpreterService.list();
   }
 
+  @GetMapping(path = "/missing")
+  @Operation(tags = TAG_INTERPRETER, summary = "List interpreters that are missing from ONR")
+  public List<String> listMissingInterpreters() {
+    return clerkInterpreterService.listMissing();
+  }
+
   @Operation(tags = TAG_INTERPRETER, summary = "Create new interpreter")
   @PostMapping(consumes = APPLICATION_JSON_VALUE)
   @ResponseStatus(HttpStatus.CREATED)

diff --git a/backend/otr/src/main/java/fi/oph/otr/onr/OnrOperationApiImpl.java b/backend/otr/src/main/java/fi/oph/otr/onr/OnrOperationApiImpl.java
@@ -1,6 +1,7 @@
 package fi.oph.otr.onr;
 
 import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import fi.oph.otr.config.Constants;
 import fi.oph.otr.onr.dto.ContactDetailsGroupDTO;
@@ -12,7 +13,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
-import lombok.RequiredArgsConstructor;
 import net.minidev.json.JSONArray;
 import org.asynchttpclient.Request;
 import org.asynchttpclient.RequestBuilder;
@@ -21,7 +21,6 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 
-@RequiredArgsConstructor
 public class OnrOperationApiImpl implements OnrOperationApi {
 
   private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
@@ -30,6 +29,13 @@ public class OnrOperationApiImpl implements OnrOperationApi {
 
   private final String onrServiceUrl;
 
+  public OnrOperationApiImpl(final CasClient onrClient, final String onrServiceUrl) {
+    this.onrClient = onrClient;
+    this.onrServiceUrl = onrServiceUrl;
+
+    OBJECT_MAPPER.configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL, true);
+  }
+
   @Override
   public Map<String, PersonalData> fetchPersonalDatas(final List<String> onrIds) throws Exception {
     // /henkilo/masterHenkilosByOidList might be usable as an endpoint for fetching master person data for persons

diff --git a/backend/otr/src/main/java/fi/oph/otr/onr/dto/ContactDetailsGroupSource.java b/backend/otr/src/main/java/fi/oph/otr/onr/dto/ContactDetailsGroupSource.java
@@ -1,7 +1,9 @@
 package fi.oph.otr.onr.dto;
 
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
+@JsonIgnoreProperties(ignoreUnknown = true)
 public enum ContactDetailsGroupSource {
   @JsonProperty("alkupera1")
   VTJ,

diff --git a/backend/otr/src/main/java/fi/oph/otr/onr/dto/ContactDetailsGroupType.java b/backend/otr/src/main/java/fi/oph/otr/onr/dto/ContactDetailsGroupType.java
@@ -1,7 +1,9 @@
 package fi.oph.otr.onr.dto;
 
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
+@JsonIgnoreProperties(ignoreUnknown = true)
 public enum ContactDetailsGroupType {
   @JsonProperty("yhteystietotyyppi1")
   KOTIOSOITE,

diff --git a/backend/otr/src/main/java/fi/oph/otr/service/ClerkInterpreterService.java b/backend/otr/src/main/java/fi/oph/otr/service/ClerkInterpreterService.java
@@ -34,10 +34,13 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import lombok.RequiredArgsConstructor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -69,13 +72,24 @@ public class ClerkInterpreterService {
   @Resource
   private final AuditService auditService;
 
+  private static final Logger LOG = LoggerFactory.getLogger(ClerkInterpreterService.class);
+
   @Transactional(readOnly = true)
   public List<ClerkInterpreterDTO> list() {
     final List<ClerkInterpreterDTO> result = listWithoutAudit();
     auditService.logOperation(OtrOperation.LIST_INTERPRETERS);
     return result;
   }
 
+  @Transactional(readOnly = true)
+  public List<String> listMissing() {
+    final List<Interpreter> interpreters = interpreterRepository.findExistingInterpreters();
+    final Map<String, PersonalData> personalDatas = onrService.getCachedPersonalDatas();
+    auditService.logOperation(OtrOperation.LIST_INTERPRETERS);
+
+    return interpreters.stream().map(Interpreter::getOnrId).filter(onrId -> personalDatas.get(onrId) == null).toList();
+  }
+
   private List<ClerkInterpreterDTO> listWithoutAudit() {
     final Map<Long, List<InterpreterRegionProjection>> interpreterRegionProjections = regionRepository
       .listInterpreterRegionProjections()
@@ -105,16 +119,22 @@ private List<ClerkInterpreterDTO> listWithoutAudit() {
 
         return createClerkInterpreterDTO(interpreter, personalData, qualifications, regionProjections);
       })
+      .filter(Optional::isPresent)
+      .map(Optional::get)
       .sorted(Comparator.comparing(ClerkInterpreterDTO::lastName).thenComparing(ClerkInterpreterDTO::nickName))
       .toList();
   }
 
-  private ClerkInterpreterDTO createClerkInterpreterDTO(
+  private Optional<ClerkInterpreterDTO> createClerkInterpreterDTO(
     final Interpreter interpreter,
     final PersonalData personalData,
     final List<Qualification> qualifications,
     final List<InterpreterRegionProjection> regionProjections
   ) {
+    if (personalData == null) {
+      LOG.error("Personal data by onr id {} not found", interpreter.getOnrId());
+      return Optional.empty();
+    }
     final List<String> regions = regionProjections.stream().map(InterpreterRegionProjection::code).toList();
 
     final List<ClerkQualificationDTO> qualificationDTOs = qualifications
@@ -127,30 +147,32 @@ private ClerkInterpreterDTO createClerkInterpreterDTO(
       .toList();
     final ClerkInterpreterQualificationsDTO interpreterQualificationsDTO = splitQualificationDTOs(qualificationDTOs);
 
-    return ClerkInterpreterDTO
-      .builder()
-      .id(interpreter.getId())
-      .version(interpreter.getVersion())
-      .isIndividualised(personalData.getIndividualised())
-      .hasIndividualisedAddress(personalData.getHasIndividualisedAddress())
-      .identityNumber(personalData.getIdentityNumber())
-      .lastName(personalData.getLastName())
-      .firstName(personalData.getFirstName())
-      .nickName(personalData.getNickName())
-      .email(personalData.getEmail())
-      .permissionToPublishEmail(interpreter.isPermissionToPublishEmail())
-      .phoneNumber(personalData.getPhoneNumber())
-      .permissionToPublishPhone(interpreter.isPermissionToPublishPhone())
-      .otherContactInfo(interpreter.getOtherContactInformation())
-      .permissionToPublishOtherContactInfo(interpreter.isPermissionToPublishOtherContactInfo())
-      .street(personalData.getStreet())
-      .postalCode(personalData.getPostalCode())
-      .town(personalData.getTown())
-      .country(personalData.getCountry())
-      .extraInformation(interpreter.getExtraInformation())
-      .regions(regions)
-      .qualifications(interpreterQualificationsDTO)
-      .build();
+    return Optional.of(
+      ClerkInterpreterDTO
+        .builder()
+        .id(interpreter.getId())
+        .version(interpreter.getVersion())
+        .isIndividualised(personalData.getIndividualised())
+        .hasIndividualisedAddress(personalData.getHasIndividualisedAddress())
+        .identityNumber(personalData.getIdentityNumber())
+        .lastName(personalData.getLastName())
+        .firstName(personalData.getFirstName())
+        .nickName(personalData.getNickName())
+        .email(personalData.getEmail())
+        .permissionToPublishEmail(interpreter.isPermissionToPublishEmail())
+        .phoneNumber(personalData.getPhoneNumber())
+        .permissionToPublishPhone(interpreter.isPermissionToPublishPhone())
+        .otherContactInfo(interpreter.getOtherContactInformation())
+        .permissionToPublishOtherContactInfo(interpreter.isPermissionToPublishOtherContactInfo())
+        .street(personalData.getStreet())
+        .postalCode(personalData.getPostalCode())
+        .town(personalData.getTown())
+        .country(personalData.getCountry())
+        .extraInformation(interpreter.getExtraInformation())
+        .regions(regions)
+        .qualifications(interpreterQualificationsDTO)
+        .build()
+    );
   }
 
   private ClerkQualificationDTO createQualificationDTO(final Qualification qualification) {
@@ -341,7 +363,7 @@ public ClerkInterpreterDTO getInterpreter(final long interpreterId) {
 
   private ClerkInterpreterDTO getInterpreterWithoutAudit(final long interpreterId) {
     // This could be optimized, by fetching only one interpreter and it's data, but is it worth of the programming work?
-    for (ClerkInterpreterDTO i : listWithoutAudit()) {
+    for (final ClerkInterpreterDTO i : listWithoutAudit()) {
       if (i.id() == interpreterId) {
         return i;
       }

diff --git a/backend/otr/src/main/java/fi/oph/otr/service/PublicInterpreterService.java b/backend/otr/src/main/java/fi/oph/otr/service/PublicInterpreterService.java
@@ -15,8 +15,11 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -36,6 +39,8 @@ public class PublicInterpreterService {
   @Resource
   private final OnrService onrService;
 
+  private static final Logger LOG = LoggerFactory.getLogger(PublicInterpreterService.class);
+
   @Transactional(readOnly = true)
   public List<InterpreterDTO> list() {
     final Map<Long, List<InterpreterRegionProjection>> interpreterRegionProjections = regionRepository
@@ -65,37 +70,45 @@ public List<InterpreterDTO> list() {
 
         return toDTO(interpreter, personalData, regionProjections, qualificationProjections);
       })
+      .filter(Optional::isPresent)
+      .map(Optional::get)
       .collect(Collectors.toCollection(ArrayList::new));
 
     Collections.shuffle(interpreterDTOS);
     return interpreterDTOS;
   }
 
-  private InterpreterDTO toDTO(
+  private Optional<InterpreterDTO> toDTO(
     final Interpreter interpreter,
     final PersonalData personalData,
     final List<InterpreterRegionProjection> regionProjections,
     final List<InterpreterQualificationProjection> qualificationProjections
   ) {
+    if (personalData == null) {
+      LOG.error("Personal data by onr id {} not found", interpreter.getOnrId());
+      return Optional.empty();
+    }
     final List<String> regions = regionProjections.stream().map(InterpreterRegionProjection::code).toList();
 
     final List<LanguagePairDTO> languagePairs = qualificationProjections
       .stream()
       .map(qp -> LanguagePairDTO.builder().from(qp.fromLang()).to(qp.toLang()).build())
       .toList();
 
-    return InterpreterDTO
-      .builder()
-      .id(interpreter.getId())
-      .firstName(personalData.getNickName())
-      .lastName(personalData.getLastName())
-      .email(interpreter.isPermissionToPublishEmail() ? personalData.getEmail() : null)
-      .phoneNumber(interpreter.isPermissionToPublishPhone() ? personalData.getPhoneNumber() : null)
-      .otherContactInfo(
-        interpreter.isPermissionToPublishOtherContactInfo() ? interpreter.getOtherContactInformation() : null
-      )
-      .regions(regions)
-      .languages(languagePairs)
-      .build();
+    return Optional.of(
+      InterpreterDTO
+        .builder()
+        .id(interpreter.getId())
+        .firstName(personalData.getNickName())
+        .lastName(personalData.getLastName())
+        .email(interpreter.isPermissionToPublishEmail() ? personalData.getEmail() : null)
+        .phoneNumber(interpreter.isPermissionToPublishPhone() ? personalData.getPhoneNumber() : null)
+        .otherContactInfo(
+          interpreter.isPermissionToPublishOtherContactInfo() ? interpreter.getOtherContactInformation() : null
+        )
+        .regions(regions)
+        .languages(languagePairs)
+        .build()
+    );
   }
 }
diff --git a/backend/otr/src/main/java/fi/oph/otr/service/email/ClerkEmailService.java b/backend/otr/src/main/java/fi/oph/otr/service/email/ClerkEmailService.java
@@ -90,7 +90,7 @@ private void createQualificationExpiryData(final Qualification qualification) {
 
       createQualificationReminder(qualification, email);
     } else {
-      LOG.warn("Personal data by onr id {} not found", interpreter.getOnrId());
+      LOG.error("Personal data by onr id {} not found", interpreter.getOnrId());
     }
   }
 

diff --git a/backend/otr/src/test/java/fi/oph/otr/onr/OnrRequestTest.java b/backend/otr/src/test/java/fi/oph/otr/onr/OnrRequestTest.java
@@ -0,0 +1,43 @@
+package fi.oph.otr.onr;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import fi.oph.otr.onr.model.PersonalData;
+import fi.vm.sade.javautils.nio.cas.CasClient;
+import java.io.IOException;
+import java.util.Optional;
+import org.asynchttpclient.Response;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.security.test.context.support.WithMockUser;
+
+@WithMockUser
+@DataJpaTest
+class OnrRequestTest {
+
+  @Value("classpath:json/henkilo-hetu-response.json")
+  private org.springframework.core.io.Resource hetuRequestResponse;
+
+  @Test
+  void testFindPersonByIdentityNumberDeserializes() throws Exception {
+    final Response response = mock(Response.class);
+    final CasClient casClient = mock(CasClient.class);
+    final OnrOperationApiImpl onrOperationApi = new OnrOperationApiImpl(casClient, "http://localhost");
+
+    when(casClient.executeBlocking(any())).thenReturn(response);
+    when(response.getStatusCode()).thenReturn(200);
+    when(response.getResponseBody()).thenReturn(getHetuMockJsonResponse());
+
+    final Optional<PersonalData> personalDataOptional = onrOperationApi.findPersonalDataByIdentityNumber("54321-54312");
+
+    assertTrue(personalDataOptional.isPresent());
+  }
+
+  private String getHetuMockJsonResponse() throws IOException {
+    return new String(hetuRequestResponse.getInputStream().readAllBytes());
+  }
+}