performance: improve tabular modification loading by accessing direct…

…ly concerned repository (#435)


Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>

src/main/java/org/gridsuite/modification/server/repositories/GeneratorModificationRepository.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2024, RTE (http://www.rte-france.com)
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.gridsuite.modification.server.repositories;
+
+import org.gridsuite.modification.server.entities.equipment.modification.GeneratorModificationEntity;
+import org.springframework.data.jpa.repository.EntityGraph;
+import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.stereotype.Repository;
+
+import java.util.List;
+import java.util.UUID;
+
+/**
+ * @author Joris Mancini <joris.mancini_externe at rte-france.com>
+ */
+@Repository
+public interface GeneratorModificationRepository extends JpaRepository<GeneratorModificationEntity, UUID> {
+
+    @EntityGraph(attributePaths = {"reactiveCapabilityCurvePoints"}, type = EntityGraph.EntityGraphType.LOAD)
+    List<GeneratorModificationEntity> findAllReactiveCapabilityCurvePointsByIdIn(List<UUID> ids);
+
+    @EntityGraph(attributePaths = {"properties"}, type = EntityGraph.EntityGraphType.LOAD)
+    List<GeneratorModificationEntity> findAllPropertiesByIdIn(List<UUID> ids);
+}

src/main/java/org/gridsuite/modification/server/repositories/ModificationRepository.java

@@ -13,9 +13,7 @@
 
 import org.gridsuite.modification.server.entities.ModificationEntity;
 import org.gridsuite.modification.server.entities.TabularCreationEntity;
-import org.gridsuite.modification.server.entities.TabularModificationEntity;
 import org.gridsuite.modification.server.entities.equipment.creation.GeneratorCreationEntity;
-import org.gridsuite.modification.server.entities.equipment.modification.GeneratorModificationEntity;
 import org.springframework.data.jpa.repository.EntityGraph;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Query;
@@ -37,11 +35,8 @@ public interface ModificationRepository extends JpaRepository<ModificationEntity
     @Query(value = "SELECT new ModificationEntity(m.id, m.type) FROM ModificationEntity m WHERE m.id IN (?1)")
     List<ModificationEntity> findMetadataIn(List<UUID> uuids);
 
-    @EntityGraph(attributePaths = {"modifications", "modifications.reactiveCapabilityCurvePoints"}, type = EntityGraph.EntityGraphType.LOAD)
-    Optional<TabularModificationEntity> findTabularModificationWithReactiveCapabilityCurvePointsById(UUID id);
-
-    @EntityGraph(attributePaths = {"reactiveCapabilityCurvePoints"}, type = EntityGraph.EntityGraphType.LOAD)
-    Set<GeneratorModificationEntity> findAllModificationsWithReactiveCapabilityCurvePointsByIdIn(List<UUID> ids);
+    @Query(value = "SELECT cast(modifications_id AS VARCHAR) FROM tabular_modification_modifications WHERE tabular_modification_entity_id = :uuid ORDER BY modifications_order", nativeQuery = true)
+    List<UUID> findSubModificationIdsByTabularModificationIdOrderByModificationsOrder(UUID uuid);
 
     @EntityGraph(attributePaths = {"creations", "creations.reactiveCapabilityCurvePoints"}, type = EntityGraph.EntityGraphType.LOAD)
     Optional<TabularCreationEntity> findTabularCreationWithReactiveCapabilityCurvePointsById(UUID id);

src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java

@@ -11,10 +11,12 @@
 import org.gridsuite.modification.server.NetworkModificationException;
 import org.gridsuite.modification.server.dto.ModificationInfos;
 import org.gridsuite.modification.server.dto.ModificationMetadata;
+import org.gridsuite.modification.server.dto.TabularModificationInfos;
 import org.gridsuite.modification.server.entities.ModificationEntity;
 import org.gridsuite.modification.server.entities.ModificationGroupEntity;
 import org.gridsuite.modification.server.entities.TabularCreationEntity;
 import org.gridsuite.modification.server.entities.TabularModificationEntity;
+import org.gridsuite.modification.server.entities.equipment.modification.GeneratorModificationEntity;
 import org.springframework.stereotype.Repository;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -35,11 +37,14 @@ public class NetworkModificationRepository {
 
     private final ModificationRepository modificationRepository;
 
+    private final GeneratorModificationRepository generatorModificationRepository;
+
     private static final String MODIFICATION_NOT_FOUND_MESSAGE = "Modification (%s) not found";
 
-    public NetworkModificationRepository(ModificationGroupRepository modificationGroupRepository, ModificationRepository modificationRepository) {
+    public NetworkModificationRepository(ModificationGroupRepository modificationGroupRepository, ModificationRepository modificationRepository, GeneratorModificationRepository generatorModificationRepository) {
         this.modificationGroupRepository = modificationGroupRepository;
         this.modificationRepository = modificationRepository;
+        this.generatorModificationRepository = generatorModificationRepository;
     }
 
     @Transactional // To have the 2 delete in the same transaction (atomic)
@@ -174,19 +179,37 @@ public List<ModificationInfos> getModificationsMetadata(UUID groupUuid, boolean
         }
     }
 
-    public TabularModificationEntity loadTabularModificationSubEntities(ModificationEntity modificationEntity) {
+    public TabularModificationInfos loadTabularModificationSubEntities(ModificationEntity modificationEntity) {
         TabularModificationEntity tabularModificationEntity = (TabularModificationEntity) modificationEntity;
         switch (tabularModificationEntity.getModificationType()) {
             case GENERATOR_MODIFICATION:
-                tabularModificationEntity = modificationRepository.findTabularModificationWithReactiveCapabilityCurvePointsById(modificationEntity.getId()).orElseThrow(() ->
-                        new NetworkModificationException(MODIFICATION_NOT_FOUND, String.format(MODIFICATION_NOT_FOUND_MESSAGE, modificationEntity.getId()))
-                );
-                modificationRepository.findAllModificationsWithReactiveCapabilityCurvePointsByIdIn(tabularModificationEntity.getModifications().stream().map(ModificationEntity::getId).toList());
-                break;
+                List<UUID> subModificationsUuids = modificationRepository.findSubModificationIdsByTabularModificationIdOrderByModificationsOrder(modificationEntity.getId());
+                // We retrieve generator modifications by generatorModificationRepository and store them as a map by IDs to re-order them later on
+                Map<UUID, GeneratorModificationEntity> generatorModifications = generatorModificationRepository
+                    .findAllReactiveCapabilityCurvePointsByIdIn(subModificationsUuids)
+                    .stream()
+                    .collect(Collectors.toMap(
+                        ModificationEntity::getId,
+                        Function.identity()
+                    ));
+                // We load properties on the generators, it uses hibernate first-level cache to fill them up directly in the map
+                generatorModificationRepository.findAllPropertiesByIdIn(subModificationsUuids);
+                // Then we can re-order the list of GeneratorModificationEntity based on ordered list of IDs
+                List<GeneratorModificationEntity> orderedGeneratorModifications = subModificationsUuids
+                    .stream()
+                    .map(generatorModifications::get)
+                    .toList();
+                return TabularModificationInfos.builder()
+                    .uuid(tabularModificationEntity.getId())
+                    .date(tabularModificationEntity.getDate())
+                    .stashed(tabularModificationEntity.getStashed())
+                    .modificationType(tabularModificationEntity.getModificationType())
+                    .modifications(orderedGeneratorModifications.stream().map(GeneratorModificationEntity::toModificationInfos).map(m -> (ModificationInfos) m).toList())
+                    .build();
             default:
                 break;
         }
-        return tabularModificationEntity;
+        return tabularModificationEntity.toModificationInfos();
     }
 
     public TabularCreationEntity loadTabularCreationSubEntities(ModificationEntity modificationEntity) {
@@ -206,7 +229,7 @@ public TabularCreationEntity loadTabularCreationSubEntities(ModificationEntity m
 
     public ModificationInfos getModificationInfos(ModificationEntity modificationEntity) {
         if (modificationEntity instanceof TabularModificationEntity) {
-            return loadTabularModificationSubEntities(modificationEntity).toModificationInfos();
+            return loadTabularModificationSubEntities(modificationEntity);
         } else if (modificationEntity instanceof TabularCreationEntity) {
             return loadTabularCreationSubEntities(modificationEntity).toModificationInfos();
         }
@@ -281,7 +304,7 @@ public int deleteModifications(UUID groupUuid, List<UUID> uuids) {
             Optional<ModificationEntity> optionalModificationWithGroup = modifications.stream().filter(m -> m.getGroup() != null).findFirst();
             if (optionalModificationWithGroup.isPresent()) {
                 throw new NetworkModificationException(MODIFICATION_DELETION_ERROR, String.format("%s is owned by group %s",
-                    optionalModificationWithGroup.get().getId().toString(), optionalModificationWithGroup.get().getGroup().getId().toString()));
+                    optionalModificationWithGroup.get().getId().toString(), optionalModificationWithGroup.get().getGroup().getId()));
             }
         }
         int count = modifications.size();

src/test/java/org/gridsuite/modification/server/modifications/AbstractNetworkModificationTest.java

@@ -18,6 +18,7 @@
 import com.powsybl.network.store.iidm.impl.NetworkImpl;
 import org.gridsuite.modification.server.dto.ModificationInfos;
 import org.gridsuite.modification.server.dto.NetworkModificationResult;
+import org.gridsuite.modification.server.entities.ModificationEntity;
 import org.gridsuite.modification.server.repositories.NetworkModificationRepository;
 import org.gridsuite.modification.server.service.ReportService;
 import org.gridsuite.modification.server.utils.NetworkCreation;
@@ -245,8 +246,9 @@ protected void testNetworkModificationsCount(UUID groupUuid, int actualSize) thr
 
     /** Save a network modification into the repository and return its UUID. */
     protected UUID saveModification(ModificationInfos modificationInfos) {
-        modificationRepository.saveModifications(TEST_GROUP_ID, List.of(modificationInfos.toEntity()));
-        return modificationRepository.getModifications(TEST_GROUP_ID, true, true).get(0).getUuid();
+        ModificationEntity entity = modificationInfos.toEntity();
+        modificationRepository.saveModifications(TEST_GROUP_ID, List.of(entity));
+        return entity.getId();
     }
 
     protected Network getNetwork() {

src/test/java/org/gridsuite/modification/server/modifications/tabularcreations/TabularGeneratorCreationsTest.java

@@ -191,7 +191,7 @@ public void testCheckSqlRequestsCount() throws Exception {
                         status().isOk(), content().contentType(MediaType.APPLICATION_JSON))
                 .andReturn();
         // We check that the request count is not dependent on the number of sub creations of the tabular creation (the JPA N+1 problem is correctly solved)
-        assertSelectCount(8);
+        assertSelectCount(4);
         reset();
 
         // We get the modifications of the group (so the 2 tabular creations)

src/test/java/org/gridsuite/modification/server/modifications/tabularmodifications/TabularGeneratorModificationsTest.java

@@ -13,23 +13,26 @@
 import org.gridsuite.modification.server.ModificationType;
 import org.gridsuite.modification.server.dto.*;
 import org.gridsuite.modification.server.modifications.AbstractNetworkModificationTest;
+import org.gridsuite.modification.server.utils.ApiUtils;
+import org.gridsuite.modification.server.utils.ModificationCreation;
 import org.gridsuite.modification.server.utils.NetworkCreation;
 import org.junit.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Tag;
 import org.springframework.http.MediaType;
+import org.testcontainers.shaded.org.apache.commons.lang3.tuple.Pair;
 
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import java.util.stream.IntStream;
 
 import static com.vladmihalcea.sql.SQLStatementCountValidator.assertSelectCount;
 import static com.vladmihalcea.sql.SQLStatementCountValidator.reset;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.gridsuite.modification.server.utils.TestUtils.assertLogMessage;
 import static org.junit.Assert.assertEquals;
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 /**
@@ -87,46 +90,38 @@ protected void assertAfterNetworkModificationDeletion() {
     }
 
     @Test
-    public void testCheckSqlRequestsCount() throws Exception {
-        List<ModificationInfos> modifications = List.of(
-                GeneratorModificationInfos.builder().equipmentId("v6generator").maxActivePower(new AttributeModification<>(300., OperationType.SET)).build()
-        );
-        ModificationInfos modificationInfos = TabularModificationInfos.builder()
-                .modificationType(ModificationType.GENERATOR_MODIFICATION)
-                .modifications(modifications)
-                .build();
-        UUID modificationUuid = saveModification(modificationInfos);
+    public void testSqlRequestsCountOnGetModification() throws Exception {
+        Pair<UUID, ModificationInfos> tabularWith1Modification = createTabularGeneratorModification(1);
         reset();
-
-        mockMvc.perform(get("/v1/network-modifications/{uuid}", modificationUuid)).andExpectAll(
-                        status().isOk(), content().contentType(MediaType.APPLICATION_JSON))
-                .andReturn();
-        assertSelectCount(4);
-
-        modifications = List.of(
-                GeneratorModificationInfos.builder().equipmentId("idGenerator").maxActivePower(new AttributeModification<>(300., OperationType.SET)).build(),
-                GeneratorModificationInfos.builder().equipmentId("v5generator").maxActivePower(new AttributeModification<>(300., OperationType.SET)).build(),
-                GeneratorModificationInfos.builder().equipmentId("v6generator").maxActivePower(new AttributeModification<>(300., OperationType.SET)).build()
-        );
-        modificationInfos = TabularModificationInfos.builder()
-                .modificationType(ModificationType.GENERATOR_MODIFICATION)
-                .modifications(modifications)
-                .build();
-        modificationUuid = saveModification(modificationInfos);
+        ModificationInfos tabularWith1ModificationInfos = ApiUtils.getModification(mockMvc, tabularWith1Modification.getLeft()); // Getting one tabular modification with one sub-modification
+        assertSelectCount(4); // 4 before improvements
+        assertThat(tabularWith1Modification.getRight())
+            .usingRecursiveComparison()
+            .ignoringFields("uuid", "date", "modifications.uuid", "modifications.date")
+            .isEqualTo(tabularWith1ModificationInfos);
+
+        Pair<UUID, ModificationInfos> tabularWith3Modification = createTabularGeneratorModification(3);
         reset();
+        ModificationInfos tabularWith3ModificationInfos = ApiUtils.getModification(mockMvc, tabularWith3Modification.getLeft()); // Getting one tabular modification with three sub-modifications
+        assertSelectCount(4); // 6 before improvements
+        assertThat(tabularWith3Modification.getRight())
+            .usingRecursiveComparison()
+            .ignoringFields("uuid", "date", "modifications.uuid", "modifications.date")
+            .isEqualTo(tabularWith3ModificationInfos);
+    }
 
-        mockMvc.perform(get("/v1/network-modifications/{uuid}", modificationUuid)).andExpectAll(
-                        status().isOk(), content().contentType(MediaType.APPLICATION_JSON))
-                .andReturn();
-        // We check that the request count is not dependent on the number of sub modifications of the tabular modification (the JPA N+1 problem is correctly solved)
-        assertSelectCount(4);
-        reset();
+    @Test
+    public void testSqlRequestsCountOnGetGroupModifications() throws Exception {
+        Pair<UUID, ModificationInfos> tabularWith1Modification = createTabularGeneratorModification(1);
+        Pair<UUID, ModificationInfos> tabularWith3Modification = createTabularGeneratorModification(3);
 
-        // We get the modifications of the group (so the 2 tabular modifications)
-        mockMvc.perform(get("/v1/groups/{groupUuid}/network-modifications", getGroupId()))
-                .andExpect(status().isOk());
-        // We check that the request count is not dependent on the number of sub modifications of the tabular modification (the JPA N+1 problem is correctly solved)
-        assertSelectCount(10);
+        reset();
+        List<ModificationInfos> tabularModifications = ApiUtils.getGroupModifications(mockMvc, getGroupId()); // Getting two tabular modifications with respectively one and three sub-modifications
+        assertSelectCount(8); // 10 before improvements
+        assertThat(List.of(tabularWith1Modification.getRight(), tabularWith3Modification.getRight()))
+            .usingRecursiveComparison()
+            .ignoringFields("uuid", "date", "modifications.uuid", "modifications.date")
+            .isEqualTo(tabularModifications);
     }
 
     @Test
@@ -163,4 +158,31 @@ protected void testUpdateModificationMessage(ModificationInfos modificationInfos
         Map<String, String> updatedValues = mapper.readValue(modificationInfos.getMessageValues(), new TypeReference<>() { });
         Assertions.assertEquals(ModificationType.GENERATOR_MODIFICATION.name(), updatedValues.get("tabularModificationType"));
     }
+
+    private Pair<UUID, ModificationInfos> createTabularGeneratorModification(int qty) {
+        ModificationInfos tabularModification = TabularModificationInfos.builder()
+            .modificationType(ModificationType.GENERATOR_MODIFICATION)
+            .modifications(createGeneratorModificationList(qty))
+            .build();
+        UUID uuid = saveModification(tabularModification);
+        tabularModification.setUuid(uuid);
+        return Pair.of(uuid, tabularModification);
+    }
+
+    private List<ModificationInfos> createGeneratorModificationList(int qty) {
+        return IntStream.range(0, qty)
+            .mapToObj(i ->
+                (ModificationInfos) GeneratorModificationInfos.builder()
+                    .equipmentId(UUID.randomUUID().toString())
+                    .maxActivePower(new AttributeModification<>(300., OperationType.SET))
+                    .properties(List.of(
+                        ModificationCreation.getFreeProperty(),
+                        ModificationCreation.getFreeProperty("test", "value")))
+                    .reactiveCapabilityCurvePoints(List.of(
+                        ReactiveCapabilityCurveModificationInfos.builder().p(10.).oldP(15.).build(),
+                        ReactiveCapabilityCurveModificationInfos.builder().qmaxP(12.).oldQmaxP(17.).build(),
+                        ReactiveCapabilityCurveModificationInfos.builder().qminP(5.).qmaxP(5.).p(5.).build()))
+                    .build())
+            .toList();
+    }
 }