From 02859e491e1de8ee8f1941e0e7b85ec65a0502af Mon Sep 17 00:00:00 2001 From: "sonarqube-agent[bot]" <210722872+sonarqube-agent[bot]@users.noreply.github.com> Date: Fri, 8 May 2026 09:04:51 +0000 Subject: [PATCH] fix: Address SonarQube issues Generated by SonarQube Agent (task: 567c307a-aa71-427b-aad9-868516c09d2d) --- .../ans/psc/delegate/PsApiDelegateImpl.java | 267 ++++++++++-------- .../psc/pscapimajv2/api/PsOperationTest.java | 68 ++--- 2 files changed, 182 insertions(+), 153 deletions(-) diff --git a/src/main/java/fr/ans/psc/delegate/PsApiDelegateImpl.java b/src/main/java/fr/ans/psc/delegate/PsApiDelegateImpl.java index 9c7b5ae..fe48fe9 100644 --- a/src/main/java/fr/ans/psc/delegate/PsApiDelegateImpl.java +++ b/src/main/java/fr/ans/psc/delegate/PsApiDelegateImpl.java @@ -123,140 +123,169 @@ public ResponseEntity getPsById(String encodedPsId, Boolean includeDeactivat @Override public ResponseEntity createNewPs(Ps ps) { long timestamp = ApiUtils.getInstantTimestamp(); - Ps storedPs; - + Ps storedPs = findStoredPs(ps.getNationalId()); + + // Remove prof - COMMENTED OUT to preserve professions data + // ps.setProfessions(new ArrayList()); + // PS EXISTS, UPDATE AND REACTIVATION + if (storedPs != null) { + return handleExistingPs(ps, storedPs, timestamp); + } + // PS DOES NOT EXIST, PHYSICAL CREATION + else { + return createBrandNewPs(ps, timestamp); + } + } + + private Ps findStoredPs(String nationalId) { try { - storedPs = psRepository.findByIdsContaining(ps.getNationalId()); + return psRepository.findByIdsContaining(nationalId); } catch (org.springframework.dao.IncorrectResultSizeDataAccessException e) { - log.warn("Multiple PS found with nationalId {}, searching for active one using MongoTemplate", ps.getNationalId()); - Query query = new Query(Criteria.where("ids").in(ps.getNationalId())); + log.warn("Multiple PS found with nationalId {}, searching for active one using MongoTemplate", nationalId); + Query query = new Query(Criteria.where("ids").in(nationalId)); List psList = mongoTemplate.find(query, Ps.class); - storedPs = psList.stream() + return psList.stream() .filter(ApiUtils::isPsActivated) .findFirst() .orElse(psList.isEmpty() ? null : psList.get(0)); } - - // Remove prof - COMMENTED OUT to preserve professions data - // ps.setProfessions(new ArrayList()); - // PS EXISTS, UPDATE AND REACTIVATION - if (storedPs != null) { - // DON'T UPDATE IF ALREADY ACTIVATED - if (ApiUtils.isPsActivated(storedPs)) { - // If the stored PS is a PSI (UUID) and the incoming is a non-PSI identifier (RPPS/ADELI/SIRET), - // do a partial update: replace only the professions from this source identifier - if (!storedPs.getNationalId().equals(ps.getNationalId())) { - final String incomingId = ps.getNationalId(); - // Tag incoming professions with sourceId before comparison - if (ps.getProfessions() != null) { - ps.getProfessions().forEach(p -> p.setSourceId(incomingId)); - } - // Skip save if professions for this source haven't changed - if (storedPs.getProfessions() != null) { - List existingForSource = storedPs.getProfessions().stream() - .filter(p -> incomingId.equals(p.getSourceId())) - .collect(java.util.stream.Collectors.toList()); - if (ps.getProfessions() != null && existingForSource.equals(ps.getProfessions())) { - log.info("Professions for source {} in account {} unchanged, skipping save", incomingId, storedPs.getNationalId()); - return new ResponseEntity<>(HttpStatus.OK); - } - } - log.info("Partial update: replacing professions for source {} in account {}", incomingId, storedPs.getNationalId()); - if (storedPs.getProfessions() != null) { - storedPs.getProfessions().removeIf(p -> incomingId.equals(p.getSourceId())); - } else { - storedPs.setProfessions(new ArrayList<>()); - } - if (ps.getProfessions() != null) { - storedPs.getProfessions().addAll(ps.getProfessions()); - } - mongoTemplate.save(storedPs); - log.info("Partial update for source {} in account {} completed", incomingId, storedPs.getNationalId()); - return new ResponseEntity<>(HttpStatus.OK); - } - log.warn("Ps {} already exists and is activated, will not be updated", ps.getNationalId()); - return new ResponseEntity<>(HttpStatus.CONFLICT); - } - // If storedPs is a DEACTIVATED PSI and incoming is a non-UUID source (RPPS/ADELI/SIRET), - // do partial update on professions WITHOUT reactivating - if (!ApiUtils.isPsActivated(storedPs) - && !storedPs.getNationalId().equals(ps.getNationalId())) { - final String incomingId = ps.getNationalId(); - log.info("Partial update on DEACTIVATED PSI: updating professions for source {} in PSI {} without reactivating", - incomingId, storedPs.getNationalId()); - if (storedPs.getProfessions() != null) { - storedPs.getProfessions().removeIf(p -> incomingId.equals(p.getSourceId())); - } else { - storedPs.setProfessions(new ArrayList<>()); - } - if (ps.getProfessions() != null) { - ps.getProfessions().forEach(p -> p.setSourceId(incomingId)); - storedPs.getProfessions().addAll(ps.getProfessions()); - } - mongoTemplate.save(storedPs); + } + + private ResponseEntity handleExistingPs(Ps ps, Ps storedPs, long timestamp) { + // DON'T UPDATE IF ALREADY ACTIVATED + if (ApiUtils.isPsActivated(storedPs)) { + return handleActivatedPs(ps, storedPs); + } + // If storedPs is a DEACTIVATED PSI and incoming is a non-UUID source (RPPS/ADELI/SIRET), + // do partial update on professions WITHOUT reactivating + if (!storedPs.getNationalId().equals(ps.getNationalId())) { + return handleDeactivatedPsPartialUpdate(ps, storedPs); + } + // set mongo _id to avoid error if it's an update + // Then update Ps data + return reactivatePs(ps, storedPs, timestamp); + } + + private ResponseEntity handleActivatedPs(Ps ps, Ps storedPs) { + // If the stored PS is a PSI (UUID) and the incoming is a non-PSI identifier (RPPS/ADELI/SIRET), + // do a partial update: replace only the professions from this source identifier + if (!storedPs.getNationalId().equals(ps.getNationalId())) { + return handleActivatedPsPartialUpdate(ps, storedPs); + } + log.warn("Ps {} already exists and is activated, will not be updated", ps.getNationalId()); + return new ResponseEntity<>(HttpStatus.CONFLICT); + } + + private ResponseEntity handleActivatedPsPartialUpdate(Ps ps, Ps storedPs) { + final String incomingId = ps.getNationalId(); + // Tag incoming professions with sourceId before comparison + if (ps.getProfessions() != null) { + ps.getProfessions().forEach(p -> p.setSourceId(incomingId)); + } + // Skip save if professions for this source haven't changed + if (storedPs.getProfessions() != null) { + List existingForSource = storedPs.getProfessions().stream() + .filter(p -> incomingId.equals(p.getSourceId())) + .collect(java.util.stream.Collectors.toList()); + if (ps.getProfessions() != null && existingForSource.equals(ps.getProfessions())) { + log.info("Professions for source {} in account {} unchanged, skipping save", incomingId, storedPs.getNationalId()); return new ResponseEntity<>(HttpStatus.OK); } - // set mongo _id to avoid error if it's an update - // Then update Ps data - log.info("Ps {} already exists, will be updated", ps.getNationalId()); - ps.set_id(storedPs.get_id()); - // if ids is null or doesn't contain nat id, we take the one from the stored ps - ApiUtils.setAppropriateIds(ps, storedPs); - ps.setActivated(timestamp); - mongoTemplate.save(ps); - log.info("Ps {} successfully stored or updated", ps.getNationalId()); - for (String id : ps.getIds()) { - log.info("Ps {} has been reactivated", id); - } } - // PS DOES NOT EXIST, PHYSICAL CREATION - else { - log.info("PS {} doesn't exist already, will be created", ps.getNationalId()); - // Tag professions with sourceId for non-PSI accounts - if (!ApiUtils.isValidUUID(ps.getNationalId()) && ps.getProfessions() != null) { - final String sourceId = ps.getNationalId(); - ps.getProfessions().forEach(prof -> { - if (prof.getSourceId() == null) { - prof.setSourceId(sourceId); - } - }); - } - // Default usualLastName if not provided : - // - PSI account (UUID nationalId) → fallback sur lastName - // - non-PSI : profession dont sourceId commence par "8" (RPPS) si présente - // - Fallback : lastName du PS (couverture complète, garantit que tout PS créé - // a un usualLastName même sans profession RPPS — ex. FINESS mono-prof) - // Cf. règle de gestion alignée avec migrate-usual-lastname.js. - if (ps.getUsualLastName() == null || ps.getUsualLastName().isEmpty()) { - if (ApiUtils.isValidUUID(ps.getNationalId())) { - ps.setUsualLastName(ps.getLastName()); - } else if (ps.getProfessions() != null && !ps.getProfessions().isEmpty()) { - ps.getProfessions().stream() - .filter(p -> p.getSourceId() != null && p.getSourceId().startsWith("8")) - .map(p -> p.getLastName()) - .filter(name -> name != null && !name.isEmpty()) - .findFirst() - .ifPresent(ps::setUsualLastName); - } - // Fallback ultime : si aucune règle n'a matché, on prend lastName. - if ((ps.getUsualLastName() == null || ps.getUsualLastName().isEmpty()) - && ps.getLastName() != null && !ps.getLastName().isEmpty()) { - ps.setUsualLastName(ps.getLastName()); - } - if (ps.getUsualLastName() != null && !ps.getUsualLastName().isEmpty()) { - log.info("Defaulting usualLastName for new PS {} to '{}'", ps.getNationalId(), ps.getUsualLastName()); - } - } - // if ids is null or doesn't contain nat id, we put nat id in it - ApiUtils.setAppropriateIds(ps, null); - ps.setActivated(timestamp); - mongoTemplate.save(ps); - log.info("Ps {} successfully stored or updated", ps.getNationalId()); + log.info("Partial update: replacing professions for source {} in account {}", incomingId, storedPs.getNationalId()); + replaceProfessionsForSource(ps, storedPs, incomingId); + mongoTemplate.save(storedPs); + log.info("Partial update for source {} in account {} completed", incomingId, storedPs.getNationalId()); + return new ResponseEntity<>(HttpStatus.OK); + } + + private ResponseEntity handleDeactivatedPsPartialUpdate(Ps ps, Ps storedPs) { + final String incomingId = ps.getNationalId(); + log.info("Partial update on DEACTIVATED PSI: updating professions for source {} in PSI {} without reactivating", + incomingId, storedPs.getNationalId()); + if (ps.getProfessions() != null) { + ps.getProfessions().forEach(p -> p.setSourceId(incomingId)); } + replaceProfessionsForSource(ps, storedPs, incomingId); + mongoTemplate.save(storedPs); + return new ResponseEntity<>(HttpStatus.OK); + } + private void replaceProfessionsForSource(Ps ps, Ps storedPs, String sourceId) { + if (storedPs.getProfessions() != null) { + storedPs.getProfessions().removeIf(p -> sourceId.equals(p.getSourceId())); + } else { + storedPs.setProfessions(new ArrayList<>()); + } + if (ps.getProfessions() != null) { + storedPs.getProfessions().addAll(ps.getProfessions()); + } + } + + private ResponseEntity reactivatePs(Ps ps, Ps storedPs, long timestamp) { + log.info("Ps {} already exists, will be updated", ps.getNationalId()); + ps.set_id(storedPs.get_id()); + // if ids is null or doesn't contain nat id, we take the one from the stored ps + ApiUtils.setAppropriateIds(ps, storedPs); + ps.setActivated(timestamp); + mongoTemplate.save(ps); + log.info("Ps {} successfully stored or updated", ps.getNationalId()); + for (String id : ps.getIds()) { + log.info("Ps {} has been reactivated", id); + } return new ResponseEntity<>(HttpStatus.CREATED); } + private ResponseEntity createBrandNewPs(Ps ps, long timestamp) { + log.info("PS {} doesn't exist already, will be created", ps.getNationalId()); + // Tag professions with sourceId for non-PSI accounts + if (!ApiUtils.isValidUUID(ps.getNationalId()) && ps.getProfessions() != null) { + final String sourceId = ps.getNationalId(); + ps.getProfessions().forEach(prof -> { + if (prof.getSourceId() == null) { + prof.setSourceId(sourceId); + } + }); + } + // Default usualLastName if not provided : + // - PSI account (UUID nationalId) → fallback sur lastName + // - non-PSI : profession dont sourceId commence par "8" (RPPS) si présente + // - Fallback : lastName du PS (couverture complète, garantit que tout PS créé + // a un usualLastName même sans profession RPPS — ex. FINESS mono-prof) + // Cf. règle de gestion alignée avec migrate-usual-lastname.js. + defaultUsualLastName(ps); + // if ids is null or doesn't contain nat id, we put nat id in it + ApiUtils.setAppropriateIds(ps, null); + ps.setActivated(timestamp); + mongoTemplate.save(ps); + log.info("Ps {} successfully stored or updated", ps.getNationalId()); + return new ResponseEntity<>(HttpStatus.CREATED); + } + + private void defaultUsualLastName(Ps ps) { + if (ps.getUsualLastName() != null && !ps.getUsualLastName().isEmpty()) { + return; + } + if (ApiUtils.isValidUUID(ps.getNationalId())) { + ps.setUsualLastName(ps.getLastName()); + } else if (ps.getProfessions() != null && !ps.getProfessions().isEmpty()) { + ps.getProfessions().stream() + .filter(p -> p.getSourceId() != null && p.getSourceId().startsWith("8")) + .map(p -> p.getLastName()) + .filter(name -> name != null && !name.isEmpty()) + .findFirst() + .ifPresent(ps::setUsualLastName); + } + // Fallback ultime : si aucune règle n'a matché, on prend lastName. + if ((ps.getUsualLastName() == null || ps.getUsualLastName().isEmpty()) + && ps.getLastName() != null && !ps.getLastName().isEmpty()) { + ps.setUsualLastName(ps.getLastName()); + } + if (ps.getUsualLastName() != null && !ps.getUsualLastName().isEmpty()) { + log.info("Defaulting usualLastName for new PS {} to '{}'", ps.getNationalId(), ps.getUsualLastName()); + } + } + @Override public ResponseEntity updatePs(Ps ps, String existingId) { // check if ps is activated before trying to update it diff --git a/src/test/java/fr/ans/psc/pscapimajv2/api/PsOperationTest.java b/src/test/java/fr/ans/psc/pscapimajv2/api/PsOperationTest.java index 714599f..a3a9abc 100644 --- a/src/test/java/fr/ans/psc/pscapimajv2/api/PsOperationTest.java +++ b/src/test/java/fr/ans/psc/pscapimajv2/api/PsOperationTest.java @@ -215,7 +215,7 @@ public void createNewPs() throws Exception { ResultActions createdPs2 = mockMvc.perform(post("/api/v2/ps").header("Accept", "application/json") .contentType("application/json").content("{\"idType\":\"8\",\"id\":\"00000000002\"," +"\"ids\":[\"800000000002\",\"855e8700-e29b-41d4-a716-44665544111\"],"+ - "\"alternativeIds\":[{\"identifier\":\"800000000002\",\"origine\":\"RPPS\",\"quality\":1},{\"identifier\":\"855e8700-e29b-41d4-a716-44665544111\",\"origine\":\"PSI\",\"quality\":2}]," + "\"quality\":0," + + "\"alternativeIds\":[{\"identifier\":\"800000000002\",\"origine\":\"RPPS\",\"quality\":1},{\"identifier\":\"855e8700-e29b-41d4-a716-44665544111\",\"origine\":\"PSI\",\"quality\":2}]," + "\"quality\":0," + "\"nationalId\":\"855e8700-e29b-41d4-a716-44665544111\",\"lastName\":\"DUPONT\",\"firstNames\":[{\"firstName\":\"JIMMY\",\"order\":1}],\"dateOfBirth\":\"17/12/1983\"," + "\"birthAddressCode\":\"57463\",\"birthCountryCode\":\"99000\",\"birthAddress\":\"METZ\",\"genderCode\":\"M\"," + "\"phone\":\"0601020304\",\"email\":\"toto57@hotmail.fr\",\"salutationCode\":\"MME\",\"professions\":[{\"exProId\":\"50C\"," + @@ -388,10 +388,10 @@ public void deletePsFailed() throws Exception { @DisplayName(value = "Search a PS by identity") @MongoDataSet(value = "/dataset/ps_search_by_identity.json", cleanBefore = true, cleanAfter = true) public void searchPs() throws Exception { - - // Only required criterias (and only one firstName) - ResultActions result = mockMvc.perform( - get("/api/v2/ps/search") + + // Only required criterias (and only one firstName) + ResultActions result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY") @@ -399,13 +399,13 @@ public void searchPs() throws Exception { .param("birthdate", "1983-12-17") ) .andExpect(status().is(200)); - String responseBody = result.andReturn().getResponse().getContentAsString(); + String responseBody = result.andReturn().getResponse().getContentAsString(); assertTrue(responseBody.contains("800000000001") && responseBody.contains("800000000002")); // Only required criterias (and all firstnames) - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -413,13 +413,13 @@ public void searchPs() throws Exception { .param("birthdate", "1983-12-17") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); assertTrue(responseBody.contains("800000000001")); // Only required criterias + birthCountryCode - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -428,13 +428,13 @@ public void searchPs() throws Exception { .param("birthCountryCode","99000") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); assertTrue(responseBody.contains("800000000001")); // Only required criterias + birthCountryCode + birthAddressCode - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -444,13 +444,13 @@ public void searchPs() throws Exception { .param("birthTownCode","57463") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); assertTrue(responseBody.contains("800000000001")); // All criterias (+ optionals birthCountryCode, birthAddressCode, birthAddress) - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -461,13 +461,13 @@ public void searchPs() throws Exception { .param("birthplace","METZ") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); assertTrue(responseBody.contains("800000000001")); // All criterias, fake firstname - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB TOTO") @@ -478,13 +478,13 @@ public void searchPs() throws Exception { .param("birthplace","METZ") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); assertEquals("[]", responseBody); // All criterias, fake birthCountryCode - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -495,13 +495,13 @@ public void searchPs() throws Exception { .param("birthplace","METZ") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); - assertEquals("[]", responseBody); - + assertEquals("[]", responseBody); + // All criterias, fake birthCountryCode - result = mockMvc.perform( - get("/api/v2/ps/search") + result = mockMvc.perform( + get("/api/v2/ps/search") .header("Accept", "application/json") .param("lastName", "DUPONT") .param("firstNames", "JIMMY BOB") @@ -512,9 +512,9 @@ public void searchPs() throws Exception { .param("birthplace","METZ") ) .andExpect(status().is(200)); - responseBody = result.andReturn().getResponse().getContentAsString(); + responseBody = result.andReturn().getResponse().getContentAsString(); - assertEquals("[]", responseBody); + assertEquals("[]", responseBody); } @Test @@ -692,7 +692,7 @@ public void updatePsiWithSamePsiExtraId() throws Exception { @Test @DisplayName(value = "upsertPsActivity: ajoute une nouvelle practice quand le sourceId n'existe pas") @MongoDataSet(value = "/dataset/ps_2_psref_entries.json", cleanBefore = true, cleanAfter = true) - public void upsertPsActivity_addsNewWhenSourceIdAbsent() throws Exception { + void upsertPsActivity_addsNewWhenSourceIdAbsent() throws Exception { // Le dataset initial a 1 practice tagged sourceId=null. On ajoute une practice sourceId=NEW_RPPS. String newProfessionJson = "{" + "\"sourceId\":\"810099999999\"," @@ -716,7 +716,7 @@ public void upsertPsActivity_addsNewWhenSourceIdAbsent() throws Exception { @Test @DisplayName(value = "upsertPsActivity: remplace la practice existante quand le sourceId match") @MongoDataSet(value = "/dataset/ps_with_rpps_practice.json", cleanBefore = true, cleanAfter = true) - public void upsertPsActivity_replacesExistingWhenSourceIdMatches() throws Exception { + void upsertPsActivity_replacesExistingWhenSourceIdMatches() throws Exception { // ps_with_rpps_practice.json a 1 practice tagged sourceId=810000000001 + 1 sourceId=510000000002. String updatedProfessionJson = "{" + "\"sourceId\":\"810000000001\"," @@ -750,7 +750,7 @@ public void upsertPsActivity_replacesExistingWhenSourceIdMatches() throws Except @Test @DisplayName(value = "upsertPsActivity: 400 si sourceId manquant dans le body") @MongoDataSet(value = "/dataset/ps_2_psref_entries.json", cleanBefore = true, cleanAfter = true) - public void upsertPsActivity_rejectsMissingSourceId() throws Exception { + void upsertPsActivity_rejectsMissingSourceId() throws Exception { String noSourceIdJson = "{\"code\":\"99\",\"lastName\":\"DUPONT\"}"; mockMvc.perform(put("/api/v2/ps/800000000001/activity") @@ -882,7 +882,7 @@ public void createNewPsNonPsi_defaultsUsualLastNamePrioritisesRppsPractice() thr @Test @DisplayName(value = "createNewPs non-PSI: usualLastName non fourni et aucune practice RPPS → fallback sur lastName") - public void createNewPsNonPsi_noRppsPractice_fallbackOnLastName() throws Exception { + void createNewPsNonPsi_noRppsPractice_fallbackOnLastName() throws Exception { // Compte FINESS pur (sourceId auto-tagué "3...") sans aucune practice RPPS. // Aucune règle spécifique ne match → fallback final sur lastName. String nonPsiJson = "{"