Modifications applications: split long running transaction before/after the computations

jonenst · jonenst · commit 2c1dd52ef08f · 2025-09-22T17:54:50.000+02:00
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=...
diff --git 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<ModificationEntity> saveModifications(UUID groupUuid, List<Modificat
     @Transactional // To have all create in the same transaction (atomic)
     // TODO Remove transaction when errors will no longer be sent to the front
     public List<ModificationEntity> saveModificationInfos(UUID groupUuid, List<ModificationInfos> modifications) {
+        return saveModificationInfosNonTransactional(groupUuid, modifications);
+    }
+
+    private List<ModificationEntity> saveModificationInfosNonTransactional(UUID groupUuid,
+            List<ModificationInfos> modifications) {
         List<ModificationEntity> entities = modifications.stream().map(ModificationEntity::fromDTO).toList();
 
         return saveModificationsNonTransactional(groupUuid, entities);
@@ -165,6 +170,20 @@ private List<ModificationEntity> saveModificationsNonTransactional(UUID groupUui
     @Transactional
     // TODO Remove transaction when errors will no longer be sent to the front
     public List<ModificationEntity> moveModifications(UUID destinationGroupUuid, UUID originGroupUuid, List<UUID> modificationsToMoveUUID, UUID referenceModificationUuid) {
+        return moveModificationsNonTransactional(destinationGroupUuid, originGroupUuid, modificationsToMoveUUID, referenceModificationUuid);
+    }
+
+    @Transactional
+    public List<ModificationEntity> moveModificationsFullDto(UUID destinationGroupUuid, UUID originGroupUuid, List<UUID> modificationsToMoveUUID, UUID referenceModificationUuid) {
+        List<ModificationEntity> 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<ModificationEntity> moveModificationsNonTransactional(UUID destinationGroupUuid, UUID originGroupUuid, List<UUID> modificationsToMoveUUID, UUID referenceModificationUuid) {
         // read origin group and modifications
         ModificationGroupEntity originModificationGroupEntity = getModificationGroup(originGroupUuid);
         List<ModificationEntity> originModificationEntities = originModificationGroupEntity.getModifications()
@@ -443,7 +462,7 @@ public ModificationInfos getModificationInfos(ModificationEntity modificationEnt
         return modificationEntity.toModificationInfos();
     }
 
-    public List<ModificationEntity> getModificationsEntities(List<UUID> groupUuids, boolean onlyStashed) {
+    private List<ModificationEntity> getModificationsEntitiesNonTransactional(List<UUID> groupUuids, boolean onlyStashed) {
         Stream<ModificationEntity> entityStream = groupUuids.stream().flatMap(this::getModificationEntityStream);
         if (onlyStashed) {
             return entityStream.filter(m -> m.getStashed() == onlyStashed).toList();
@@ -452,6 +471,21 @@ public List<ModificationEntity> getModificationsEntities(List<UUID> groupUuids,
         }
     }
 
+    //TODO ? should be @Transactional(readOnly = true)
+    public List<ModificationEntity> getModificationsEntities(List<UUID> groupUuids, boolean onlyStashed) {
+        return getModificationsEntitiesNonTransactional(groupUuids, onlyStashed);
+    }
+
+    @Transactional(readOnly = true)
+    public List<ModificationEntity> getModificationsEntitiesFullDto(List<UUID> groupUuids, boolean onlyStashed) {
+        List<ModificationEntity> 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<ModificationInfos> getModificationsInfos(@NonNull List<UUID> uuids) {
+        return getModificationsInfosNonTransactional(uuids);
+    }
+
+    private List<ModificationInfos> getModificationsInfosNonTransactional(List<UUID> uuids) {
         // Spring-data findAllById doc says: the order of elements in the result is not guaranteed
         Map<UUID, ModificationEntity> entities = modificationRepository.findAllById(uuids)
             .stream()
@@ -553,6 +591,10 @@ public List<ModificationInfos> getBasicNetworkModificationsFromComposite(@NonNul
 
     @Transactional(readOnly = true)
     public List<ModificationInfos> getCompositeModificationsInfos(@NonNull List<UUID> uuids) {
+        return getCompositeModificationsInfosNonTransactional(uuids);
+    }
+
+    private List<ModificationInfos> getCompositeModificationsInfosNonTransactional(@NonNull List<UUID> uuids) {
         List<ModificationInfos> entities = new ArrayList<>();
         uuids.forEach(uuid -> {
             List<UUID> foundEntities = modificationRepository.findModificationIdsByCompositeModificationId(uuid);
@@ -568,6 +610,10 @@ public List<ModificationInfos> getCompositeModificationsInfos(@NonNull List<UUID
 
     @Transactional(readOnly = true)
     public List<ModificationInfos> getActiveModificationsInfos(@NonNull UUID groupUuid) {
+        return getActiveModificationsInfosNonTransactional(groupUuid);
+    }
+
+    private List<ModificationInfos> 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<ModificationEntity> saveDuplicateModifications(@NonNull UUID targetGroupUuid, UUID originGroupUuid, @NonNull List<UUID> modificationsUuids) {
+        List<ModificationInfos> modificationInfos = originGroupUuid != null ? getActiveModificationsInfosNonTransactional(originGroupUuid) : getModificationsInfosNonTransactional(modificationsUuids);
+        return saveModificationInfosNonTransactional(targetGroupUuid, modificationInfos);
+    }
+
+    @Transactional
+    public List<ModificationEntity> saveCompositeModifications(@NonNull UUID targetGroupUuid, @NonNull List<UUID> modificationsUuids) {
+        List<ModificationInfos> 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
@@ -191,9 +191,6 @@ public void restoreNetworkModifications(UUID groupUuid, @NonNull List<UUID> 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<ModificationApplicationContext> applicationContexts) {
         List<ModificationEntity> 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<ModificationApplicationGroup> modificationGroupsInfos = new ArrayList<>();
@@ -246,7 +242,8 @@ public NetworkModificationResult buildVariant(@NonNull UUID networkUuid, @NonNul
                 Set<UUID> modificationsToExclude = buildInfos.getModificationUuidsToExclude().get(groupUuid);
                 List<ModificationEntity> 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<UUID> modificationsU
         }
     }
 
-    @Transactional
     public NetworkModificationsResult moveModifications(@NonNull UUID destinationGroupUuid, @NonNull UUID originGroupUuid, UUID beforeModificationUuid,
                                                                        @NonNull List<UUID> modificationsToMoveUuids, @NonNull List<ModificationApplicationContext> applicationContexts,
                                                                        boolean applyModifications) {
         // update origin/destinations groups to cut and paste all modificationsToMove
-        List<ModificationEntity> modificationEntities = networkModificationRepository.moveModifications(destinationGroupUuid, originGroupUuid, modificationsToMoveUuids, beforeModificationUuid);
+        // FullDto needed for toModificationInfos() after the modifications have been applied
+        List<ModificationEntity> modificationEntities = networkModificationRepository.moveModificationsFullDto(destinationGroupUuid, originGroupUuid, modificationsToMoveUuids, beforeModificationUuid);
 
         List<Optional<NetworkModificationResult>> 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<NetworkModificationResult> applyModifications(UUID networkUuid,
         return Optional.empty();
     }
 
-    @Transactional
     public NetworkModificationsResult duplicateModifications(@NonNull UUID targetGroupUuid, UUID originGroupUuid, @NonNull List<UUID> modificationsUuids, @NonNull List<ModificationApplicationContext> applicationContexts) {
         if (originGroupUuid != null && !modificationsUuids.isEmpty()) { // Duplicate modifications from a group or from a list only
             throw new NetworkModificationServerException(DUPLICATION_ARGUMENT_INVALID);
         }
-        List<ModificationInfos> modificationInfos = originGroupUuid != null ? networkModificationRepository.getActiveModificationsInfos(originGroupUuid) : networkModificationRepository.getModificationsInfos(modificationsUuids);
-        List<ModificationEntity> duplicateModifications = networkModificationRepository.saveModificationInfos(targetGroupUuid, modificationInfos);
+        List<ModificationEntity> 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<UUID> modificationsUuids, @NonNull List<ModificationApplicationContext> applicationContexts) {
-        List<ModificationInfos> modificationInfos = networkModificationRepository.getCompositeModificationsInfos(modificationsUuids);
-        List<ModificationEntity> modificationEntities = networkModificationRepository.saveModificationInfos(targetGroupUuid, modificationInfos);
+        List<ModificationEntity> 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
@@ -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
@@ -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<ModificationEntity> 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<ModificationEntity> 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<ModificationEntity> 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<UUID> expectedModificationUuids = List.of(entities.getFirst().getId(), modificationsResult.modificationUuids().getFirst());
+        List<UUID> expectedGroupUuids = List.of(groupUuid1, groupUuid2);
+
+        List<ModificationApplicationEntity> modificationApplicationEntities = modificationApplicationRepository.findAll();
+        List<ModificationApplicationInfos> 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() {
         /*
