From 2c1dd52ef08febf36c816b917d773345764042b0 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Thu, 18 Sep 2025 11:22:43 +0200 Subject: [PATCH 1/5] Modifications applications: split long running transaction before/after the computations This requires to reorganize the code and the transactions the fully load the data, hence the changes in NetworkModificationRepository with the new intermediate methods. Add testInsertCompositeModifications for symmetry with other code paths duplicate/move/ in the switch/case of ModificationController ?action=... --- .../NetworkModificationRepository.java | 60 +++++++++++++++++- .../service/NetworkModificationService.java | 19 ++---- .../server/service/BuildTest.java | 5 ++ .../service/ModificationIndexationTest.java | 63 ++++++++++++++++++- 4 files changed, 132 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java b/src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java index c560f6438..fdaddc90c 100644 --- a/src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java +++ b/src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java @@ -115,6 +115,11 @@ public List saveModifications(UUID groupUuid, List saveModificationInfos(UUID groupUuid, List modifications) { + return saveModificationInfosNonTransactional(groupUuid, modifications); + } + + private List saveModificationInfosNonTransactional(UUID groupUuid, + List modifications) { List entities = modifications.stream().map(ModificationEntity::fromDTO).toList(); return saveModificationsNonTransactional(groupUuid, entities); @@ -165,6 +170,20 @@ private List saveModificationsNonTransactional(UUID groupUui @Transactional // TODO Remove transaction when errors will no longer be sent to the front public List moveModifications(UUID destinationGroupUuid, UUID originGroupUuid, List modificationsToMoveUUID, UUID referenceModificationUuid) { + return moveModificationsNonTransactional(destinationGroupUuid, originGroupUuid, modificationsToMoveUUID, referenceModificationUuid); + } + + @Transactional + public List moveModificationsFullDto(UUID destinationGroupUuid, UUID originGroupUuid, List modificationsToMoveUUID, UUID referenceModificationUuid) { + List movedModifications = moveModificationsNonTransactional(destinationGroupUuid, originGroupUuid, modificationsToMoveUUID, referenceModificationUuid); + // Force load subentities/collections, needed later when the transaction is closed + // to avoid LazyInitialisationException. Maybe better to refactor to return the dto ? + // And refactor to more efficiently load the data (avoid 1+N) ? + movedModifications.forEach(ModificationEntity::toModificationInfos); + return movedModifications; + } + + private List moveModificationsNonTransactional(UUID destinationGroupUuid, UUID originGroupUuid, List modificationsToMoveUUID, UUID referenceModificationUuid) { // read origin group and modifications ModificationGroupEntity originModificationGroupEntity = getModificationGroup(originGroupUuid); List originModificationEntities = originModificationGroupEntity.getModifications() @@ -443,7 +462,7 @@ public ModificationInfos getModificationInfos(ModificationEntity modificationEnt return modificationEntity.toModificationInfos(); } - public List getModificationsEntities(List groupUuids, boolean onlyStashed) { + private List getModificationsEntitiesNonTransactional(List groupUuids, boolean onlyStashed) { Stream entityStream = groupUuids.stream().flatMap(this::getModificationEntityStream); if (onlyStashed) { return entityStream.filter(m -> m.getStashed() == onlyStashed).toList(); @@ -452,6 +471,21 @@ public List getModificationsEntities(List groupUuids, } } + //TODO ? should be @Transactional(readOnly = true) + public List getModificationsEntities(List groupUuids, boolean onlyStashed) { + return getModificationsEntitiesNonTransactional(groupUuids, onlyStashed); + } + + @Transactional(readOnly = true) + public List getModificationsEntitiesFullDto(List groupUuids, boolean onlyStashed) { + List modificationsEntities = getModificationsEntitiesNonTransactional(groupUuids, onlyStashed); + // Force load subentities/collections, needed later when the transaction is closed + // to avoid LazyInitialisationException. Maybe better to refactor to return the dto ? + // And refactor to more efficiently load the data (avoid 1+N) ? + modificationsEntities.forEach(ModificationEntity::toModificationInfos); + return modificationsEntities; + } + @Transactional(readOnly = true) public ModificationInfos getModificationInfo(UUID modificationUuid) { return getModificationInfos(getModificationEntity(modificationUuid)); @@ -525,6 +559,10 @@ public Integer getModificationsCount(@NonNull UUID groupUuid, boolean stashed) { @Transactional(readOnly = true) public List getModificationsInfos(@NonNull List uuids) { + return getModificationsInfosNonTransactional(uuids); + } + + private List getModificationsInfosNonTransactional(List uuids) { // Spring-data findAllById doc says: the order of elements in the result is not guaranteed Map entities = modificationRepository.findAllById(uuids) .stream() @@ -553,6 +591,10 @@ public List getBasicNetworkModificationsFromComposite(@NonNul @Transactional(readOnly = true) public List getCompositeModificationsInfos(@NonNull List uuids) { + return getCompositeModificationsInfosNonTransactional(uuids); + } + + private List getCompositeModificationsInfosNonTransactional(@NonNull List uuids) { List entities = new ArrayList<>(); uuids.forEach(uuid -> { List foundEntities = modificationRepository.findModificationIdsByCompositeModificationId(uuid); @@ -568,6 +610,10 @@ public List getCompositeModificationsInfos(@NonNull List getActiveModificationsInfos(@NonNull UUID groupUuid) { + return getActiveModificationsInfosNonTransactional(groupUuid); + } + + private List getActiveModificationsInfosNonTransactional(UUID groupUuid) { return getModificationEntityStream(groupUuid).filter(m -> !m.getStashed()).map(this::getModificationInfos).toList(); } @@ -812,4 +858,16 @@ private void deleteTabularModificationSubModifications(TabularModificationEntity throw new UnsupportedOperationException(String.format("No sub-modifications deletion for modification type: %s", tabularModificationType)); } } + + @Transactional + public List saveDuplicateModifications(@NonNull UUID targetGroupUuid, UUID originGroupUuid, @NonNull List modificationsUuids) { + List modificationInfos = originGroupUuid != null ? getActiveModificationsInfosNonTransactional(originGroupUuid) : getModificationsInfosNonTransactional(modificationsUuids); + return saveModificationInfosNonTransactional(targetGroupUuid, modificationInfos); + } + + @Transactional + public List saveCompositeModifications(@NonNull UUID targetGroupUuid, @NonNull List modificationsUuids) { + List modificationInfos = getCompositeModificationsInfosNonTransactional(modificationsUuids); + return saveModificationInfosNonTransactional(targetGroupUuid, modificationInfos); + } } diff --git a/src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java b/src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java index 8daf4cca9..302ed6175 100644 --- a/src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java +++ b/src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java @@ -191,9 +191,6 @@ public void restoreNetworkModifications(UUID groupUuid, @NonNull List modi networkModificationRepository.getModificationsCount(groupUuid, false)); } - // No transactional because we need to save modification in DB also in case of error - // Transaction made in 'saveModifications' method - // TODO Add transaction when errors will no longer be sent to the front public NetworkModificationsResult createNetworkModification(@NonNull UUID groupUuid, @NonNull ModificationInfos modificationInfo, @NonNull List applicationContexts) { List modificationEntities = networkModificationRepository.saveModificationInfos(groupUuid, List.of(modificationInfo)); @@ -237,7 +234,6 @@ public Network cloneNetworkVariant(UUID networkUuid, return network; } - @Transactional public NetworkModificationResult buildVariant(@NonNull UUID networkUuid, @NonNull BuildInfos buildInfos) { // Apply all modifications belonging to the modification groups uuids in buildInfos List modificationGroupsInfos = new ArrayList<>(); @@ -246,7 +242,8 @@ public NetworkModificationResult buildVariant(@NonNull UUID networkUuid, @NonNul Set modificationsToExclude = buildInfos.getModificationUuidsToExclude().get(groupUuid); List modifications = List.of(); try { - modifications = networkModificationRepository.getModificationsEntities(List.of(groupUuid), false) + // FullDto needed for toModificationInfos() after the modifications have been applied + modifications = networkModificationRepository.getModificationsEntitiesFullDto(List.of(groupUuid), false) .stream() .filter(m -> modificationsToExclude == null || !modificationsToExclude.contains(m.getId())) .filter(m -> !m.getStashed()) @@ -287,12 +284,12 @@ public void deleteNetworkModifications(UUID groupUuid, List modificationsU } } - @Transactional public NetworkModificationsResult moveModifications(@NonNull UUID destinationGroupUuid, @NonNull UUID originGroupUuid, UUID beforeModificationUuid, @NonNull List modificationsToMoveUuids, @NonNull List applicationContexts, boolean applyModifications) { // update origin/destinations groups to cut and paste all modificationsToMove - List modificationEntities = networkModificationRepository.moveModifications(destinationGroupUuid, originGroupUuid, modificationsToMoveUuids, beforeModificationUuid); + // FullDto needed for toModificationInfos() after the modifications have been applied + List modificationEntities = networkModificationRepository.moveModificationsFullDto(destinationGroupUuid, originGroupUuid, modificationsToMoveUuids, beforeModificationUuid); List> result = applyModifications && !modificationEntities.isEmpty() ? applyModifications(destinationGroupUuid, modificationEntities, applicationContexts) : List.of(); return new NetworkModificationsResult(modificationEntities.stream().map(ModificationEntity::getId).toList(), result); @@ -333,23 +330,19 @@ private Optional applyModifications(UUID networkUuid, return Optional.empty(); } - @Transactional public NetworkModificationsResult duplicateModifications(@NonNull UUID targetGroupUuid, UUID originGroupUuid, @NonNull List modificationsUuids, @NonNull List applicationContexts) { if (originGroupUuid != null && !modificationsUuids.isEmpty()) { // Duplicate modifications from a group or from a list only throw new NetworkModificationServerException(DUPLICATION_ARGUMENT_INVALID); } - List modificationInfos = originGroupUuid != null ? networkModificationRepository.getActiveModificationsInfos(originGroupUuid) : networkModificationRepository.getModificationsInfos(modificationsUuids); - List duplicateModifications = networkModificationRepository.saveModificationInfos(targetGroupUuid, modificationInfos); + List duplicateModifications = networkModificationRepository.saveDuplicateModifications(targetGroupUuid, originGroupUuid, modificationsUuids); return new NetworkModificationsResult( duplicateModifications.stream().map(ModificationEntity::getId).toList(), applyModifications(targetGroupUuid, duplicateModifications, applicationContexts) ); } - @Transactional public NetworkModificationsResult insertCompositeModifications(@NonNull UUID targetGroupUuid, @NonNull List modificationsUuids, @NonNull List applicationContexts) { - List modificationInfos = networkModificationRepository.getCompositeModificationsInfos(modificationsUuids); - List modificationEntities = networkModificationRepository.saveModificationInfos(targetGroupUuid, modificationInfos); + List modificationEntities = networkModificationRepository.saveCompositeModifications(targetGroupUuid, modificationsUuids); return new NetworkModificationsResult(modificationEntities.stream().map(ModificationEntity::getId).toList(), applyModifications(targetGroupUuid, modificationEntities, applicationContexts)); } diff --git a/src/test/java/org/gridsuite/modification/server/service/BuildTest.java b/src/test/java/org/gridsuite/modification/server/service/BuildTest.java index 3df4d22ff..552ce35d8 100644 --- a/src/test/java/org/gridsuite/modification/server/service/BuildTest.java +++ b/src/test/java/org/gridsuite/modification/server/service/BuildTest.java @@ -362,6 +362,11 @@ void testIndexationAfterBuild(final MockWebServer server) { .couplingDevices(Arrays.asList(CouplingDeviceInfos.builder().busbarSectionId1("vl9_1_1").busbarSectionId2("vl9_2_1").build())) .build())); // add new Load + // Need an entity with a lazyloaded subentity/collection to test that all required + // data has been loaded during the initial completed transaction before applying modifications: + // LoadCreationInfos and other modifications in this test do the job because it has the free properties Collection. + // Is there a good way to add in this test that we have 2 short transactions + // instead of one long idle transaction ? equipmentsToAdd.add(ModificationEntity.fromDTO(LoadCreationInfos.builder() .equipmentId("newLoad") .equipmentName("newLoad") diff --git a/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java b/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java index a5ce01bda..5d844e95e 100644 --- a/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java +++ b/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java @@ -182,6 +182,11 @@ void testDuplicateModifications() { Create first modification then apply it on group 1 */ String newEquipmentId = "newLoad"; + // Need an entity with a lazyloaded subentity/collection to test that all required + // data has been loaded during the completed transaction before applying modifications: + // LoadCreationInfos does the job because it has the free properties Collection. + // Is there a good way to add in this test that we have 2 short transactions + // instead of one long idle transaction ? LoadCreationInfos loadCreationInfos = createLoadCreationInfos(newEquipmentId); UUID groupUuid1 = UUID.randomUUID(); List entities = modificationRepository.saveModifications(groupUuid1, List.of(ModificationEntity.fromDTO(loadCreationInfos))); @@ -225,10 +230,14 @@ void testMoveModifications() { Create first modification then apply it on group 1 */ String newEquipmentId = "newLoad"; + // Need an entity with a lazyloaded subentity/collection to test that all required + // data has been loaded during the completed transaction before applying modifications: + // LoadCreationInfos does the job because it has the free properties Collection. + // Is there a good way to add in this test that we have 2 short transactions + // instead of one long idle transaction ? LoadCreationInfos loadCreationInfos = createLoadCreationInfos(newEquipmentId); UUID groupUuid1 = UUID.randomUUID(); List entities = modificationRepository.saveModifications(groupUuid1, List.of(ModificationEntity.fromDTO(loadCreationInfos))); - NetworkModificationResult result = networkModificationApplicator.applyModifications(new ModificationApplicationGroup(groupUuid1, entities, reportInfos), networkInfos); assertNotNull(result); @@ -264,6 +273,58 @@ void testMoveModifications() { modificationApplicationInfos.forEach(applicationInfo -> assertEquals(newEquipmentId, applicationInfo.getCreatedEquipmentIds().iterator().next())); } + @Test + void testInsertCompositeModifications() { + /* + Create first modification then apply it on group 1 + */ + String newEquipmentId = "newLoad"; + // Need an entity with a lazyloaded subentity/collection to test that all required + // data has been loaded during the completed transaction before applying modifications: + // LoadCreationInfos does the job because it has the free properties Collection. + // Is there a good way to add in this test that we have 2 short transactions + // instead of one long idle transaction ? + LoadCreationInfos loadCreationInfos = createLoadCreationInfos(newEquipmentId); + UUID groupUuid1 = UUID.randomUUID(); + List entities = modificationRepository.saveModifications(groupUuid1, List.of(ModificationEntity.fromDTO(loadCreationInfos))); + + NetworkModificationResult result = networkModificationApplicator.applyModifications(new ModificationApplicationGroup(groupUuid1, entities, reportInfos), networkInfos); + assertNotNull(result); + + // Create the composite modification to pass later to ?action=insert + UUID compositeUuid = networkModificationService.createNetworkCompositeModification( + entities.stream().map(ModificationEntity::getId).toList() + ); + + // Need to remove the listener created in the last modifications application + ((NetworkImpl) networkInfos.getNetwork()).getListeners().clear(); + + /* + Insert as composite this modification to group 2, variant 2 + */ + UUID groupUuid2 = UUID.randomUUID(); + NetworkModificationsResult modificationsResult = networkModificationService.insertCompositeModifications( + groupUuid2, + List.of(compositeUuid), + List.of(new ModificationApplicationContext(networkInfos.getNetworkUuuid(), variant2, UUID.randomUUID(), UUID.randomUUID())) + ); + + /* + check results in database and in elasticsearch + */ + List expectedModificationUuids = List.of(entities.getFirst().getId(), modificationsResult.modificationUuids().getFirst()); + List expectedGroupUuids = List.of(groupUuid1, groupUuid2); + + List modificationApplicationEntities = modificationApplicationRepository.findAll(); + List modificationApplicationInfos = IterableUtils.toList(modificationApplicationInfosRepository.findAll()); + + assertThat(modificationApplicationEntities.stream().map(m -> m.getModification().getId()).toList()).usingRecursiveComparison().isEqualTo(expectedModificationUuids); + assertThat(modificationApplicationInfos.stream().map(ModificationApplicationInfos::getModificationUuid).toList()).usingRecursiveComparison().isEqualTo(expectedModificationUuids); + + assertThat(modificationApplicationInfos.stream().map(ModificationApplicationInfos::getGroupUuid).toList()).usingRecursiveComparison().isEqualTo(expectedGroupUuids); + modificationApplicationInfos.forEach(applicationInfo -> assertEquals(newEquipmentId, applicationInfo.getCreatedEquipmentIds().iterator().next())); + } + @Test void testDeleteModifications() { /* From 802cda87d30d7f94e0a83bee90553adce879eb5e Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Wed, 15 Oct 2025 10:12:49 +0200 Subject: [PATCH 2/5] Apply same fix as f2745ae48 after merge --- .../modification/server/service/ModificationIndexationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java b/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java index 2c418dd0a..1efeba283 100644 --- a/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java +++ b/src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java @@ -325,7 +325,7 @@ void testInsertCompositeModifications() { assertThat(modificationApplicationInfos.stream().map(ModificationApplicationInfos::getModificationUuid).toList()).usingRecursiveComparison().isEqualTo(expectedModificationUuids); assertThat(modificationApplicationInfos.stream().map(ModificationApplicationInfos::getGroupUuid).toList()).usingRecursiveComparison().isEqualTo(expectedGroupUuids); - modificationApplicationInfos.forEach(applicationInfo -> assertEquals(newEquipmentId, applicationInfo.getCreatedEquipmentIds().iterator().next())); + modificationApplicationInfos.forEach(applicationInfo -> assertTrue(applicationInfo.getCreatedEquipmentIds().contains(newEquipmentId))); } @Test From 733119441f0142f6225a6d14a4d484c1461e329c Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Wed, 24 Sep 2025 12:32:38 +0200 Subject: [PATCH 3/5] Modification flush: use the same thread as for apply (bounded thread pool for large modifications) --- .../NetworkModificationApplicator.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java index 3d9a06003..c1e63538f 100644 --- a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java +++ b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java @@ -106,15 +106,18 @@ public NetworkModificationResult applyModifications(ModificationApplicationGroup .orElse(PreloadingStrategy.NONE); NetworkStoreListener listener = NetworkStoreListener.create(networkInfos.getNetwork(), networkInfos.getNetworkUuuid(), networkStoreService, equipmentInfosService, applicationInfosService, collectionThreshold); - ApplicationStatus groupApplicationStatus; if (preloadingStrategy == PreloadingStrategy.ALL_COLLECTIONS_NEEDED_FOR_BUS_VIEW) { - groupApplicationStatus = largeNetworkModificationExecutionService - .supplyAsync(() -> apply(modificationInfosGroup, listener)) + return largeNetworkModificationExecutionService + .supplyAsync(() -> applyAndFlush(modificationInfosGroup, listener)) .join(); } else { - groupApplicationStatus = apply(modificationInfosGroup, listener); + return applyAndFlush(modificationInfosGroup, listener); } + } + private NetworkModificationResult applyAndFlush(ModificationApplicationGroup modificationInfosGroup, + NetworkStoreListener listener) { + ApplicationStatus groupApplicationStatus = apply(modificationInfosGroup, listener); return flushModificationApplications(groupApplicationStatus, listener); } @@ -147,15 +150,18 @@ public NetworkModificationResult applyModifications(List groupsApplicationStatuses; if (preloadingStrategy == PreloadingStrategy.ALL_COLLECTIONS_NEEDED_FOR_BUS_VIEW) { - groupsApplicationStatuses = largeNetworkModificationExecutionService - .supplyAsync(() -> apply(modificationInfosGroups, listener)) + return largeNetworkModificationExecutionService + .supplyAsync(() -> applyAndFlush(modificationInfosGroups, listener)) .join(); } else { - groupsApplicationStatuses = apply(modificationInfosGroups, listener); + return applyAndFlush(modificationInfosGroups, listener); } + } + private NetworkModificationResult applyAndFlush(List modificationInfosGroups, + NetworkStoreListener listener) { + List groupsApplicationStatuses = apply(modificationInfosGroups, listener); return flushModificationApplications(groupsApplicationStatuses, listener); } From 7d5828e92943467dc87e490ad9cf00bf437189c4 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Wed, 15 Oct 2025 10:48:54 +0200 Subject: [PATCH 4/5] Review remove variable --- .../server/modifications/NetworkModificationApplicator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java index c1e63538f..537340d41 100644 --- a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java +++ b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java @@ -117,8 +117,7 @@ public NetworkModificationResult applyModifications(ModificationApplicationGroup private NetworkModificationResult applyAndFlush(ModificationApplicationGroup modificationInfosGroup, NetworkStoreListener listener) { - ApplicationStatus groupApplicationStatus = apply(modificationInfosGroup, listener); - return flushModificationApplications(groupApplicationStatus, listener); + return flushModificationApplications(apply(modificationInfosGroup, listener), listener); } private NetworkModificationResult flushModificationApplications(ApplicationStatus groupApplicationStatus, NetworkStoreListener listener) { From a7fa993073411e53d8554084721e67e856b3e622 Mon Sep 17 00:00:00 2001 From: HARPER Jon Date: Wed, 15 Oct 2025 15:17:35 +0200 Subject: [PATCH 5/5] more review remove variable --- .../server/modifications/NetworkModificationApplicator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java index 537340d41..a06a6f0b7 100644 --- a/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java +++ b/src/main/java/org/gridsuite/modification/server/modifications/NetworkModificationApplicator.java @@ -160,8 +160,7 @@ public NetworkModificationResult applyModifications(List modificationInfosGroups, NetworkStoreListener listener) { - List groupsApplicationStatuses = apply(modificationInfosGroups, listener); - return flushModificationApplications(groupsApplicationStatuses, listener); + return flushModificationApplications(apply(modificationInfosGroups, listener), listener); } // This method is used when building a variant