Optim get tabular modif #422

etiennehomer · 2024-01-22T15:36:52Z

No description provided.

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

…val for all tabulars Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>

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

…k-modification-server into optim_get_tabular_modif

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

sonarqubecloud · 2024-02-06T11:27:48Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
90.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

thangqp · 2024-02-08T11:30:49Z

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

-
-    @EntityGraph(attributePaths = {"reactiveCapabilityCurvePoints"}, type = EntityGraph.EntityGraphType.LOAD)
-    Set<GeneratorModificationEntity> findAllReactiveCapabilityCurvePointsByIdIn(List<UUID> ids);
+    @Query(value = "SELECT cast(modifications_id as varchar) from tabular_modification_modifications where tabular_modification_entity_id= ?1", nativeQuery = true)


Lack a space : tabular_modification_entity_id= ?1 => tabular_modification_entity_id = ?1

Fixed, I also put SQL keywords in capital letters to harmonize

thangqp · 2024-02-08T11:34:49Z

.../modification/server/modifications/tabularmodifications/TabularBatteryModificationsTest.java

+
+    private List<ModificationInfos> createBatteryModificationList(int qty) {
+        List<ModificationInfos> modifications = new ArrayList<>();
+        for (int i = 0; i <= qty; i++) {


IMO, IntStream.range(0, qty).map().toList() is more expressive, less error-prone than using loop with
Idem for others test classes

thangqp · 2024-02-08T11:43:58Z

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

        if (onlyStashed) {
-            return modificationEntity.filter(m -> m.getStashed() == onlyStashed)
+            return modificationEntities.filter(m -> m.getStashed() == onlyStashed)


could be replaced by ModificationEntity::getStashed as line 148???

thangqp · 2024-02-08T12:15:38Z

...a/org/gridsuite/modification/server/repositories/TabularModificationRepositoryException.java

+/**
+ * @author Joris Mancini <joris.mancini_externe at rte-france.com>
+ */
+public class TabularModificationRepositoryException extends RuntimeException {


We have already NetworkModificationException, is necessary to create a new one?

thangqp · 2024-02-08T14:29:42Z

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

-        Optional<ModificationEntity> optionalModificationEntity = modificationRepository.findById(modificationUuid);
-        if (!optionalModificationEntity.isPresent()) {
-            throw new NetworkModificationException(MODIFICATION_NOT_FOUND, modificationUuid.toString());
+    public ModificationInfos getModificationInfos(UUID modificationUuid) {


Should be as following to avoid throw the same exception when getModificationInfos return null

public ModificationInfos getModificationInfos(UUID modificationUuid) { ModificationEntity modificationEntity = modificationRepository.findById(modificationUuid) .orElseThrow(() -> new NetworkModificationException(MODIFICATION_NOT_FOUND, modificationUuid.toString())); return getModificationInfos(modificationEntity); }

klesaulnier

It could be nice to find a way to combine those tests classes
It could make them easier to maintain

klesaulnier · 2024-02-08T09:10:08Z

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

+    @Query(value = "SELECT cast(modifications_id as varchar) from tabular_modification_modifications where tabular_modification_entity_id= ?1", nativeQuery = true)
+    List<UUID> findSubModificationsIds(UUID uuid);


why do we need native query here ? If it is to get "modifications_id" only, we could use JPA projections

As the relation table is not defined explicitly we cannot query it directly through JPA, so either we do it with native query directly on the relation table or we go though JPA but we need to add join because to query on TabularModificationEnity. So, I'd rather keep this solution, though the join solution would be quite equally performant I think.

klesaulnier · 2024-02-08T15:19:15Z

src/test/java/org/gridsuite/modification/server/utils/ApiUtils.java

+        mockMvc.perform(get("/v1/groups/{groupUuid}/network-modifications", groupUuid))
+            .andExpectAll(status().isOk(), content().contentType(MediaType.APPLICATION_JSON))
+            .andReturn();
+    }
+
+    public static void getModification(MockMvc mockMvc, UUID modificationUuid) throws Exception {
+        mockMvc.perform(get("/v1/network-modifications/{uuid}", modificationUuid))
+            .andExpectAll(status().isOk(), content().contentType(MediaType.APPLICATION_JSON))
+            .andReturn();


if we don't get the returned value, we don't need "andReturn()"

klesaulnier · 2024-02-09T08:01:33Z

...te/modification/server/repositories/equipmentmodification/BatteryModificationRepository.java

+public interface BatteryModificationRepository extends JpaRepository<BatteryModificationEntity, UUID>, EagerNetworkModificationRepository<BatteryModificationEntity> {
+
+    @Override
+    @EntityGraph(attributePaths = {"reactiveCapabilityCurvePoints", "properties"}, type = EntityGraph.EntityGraphType.LOAD)


I thought it was not possible to fetch two distinct collections from the same entity in JPA, see below exception
"org.hibernate.loader.MultipleBagFetchException: cannot simultaneously fetch multiple bags"

klesaulnier · 2024-02-09T08:04:24Z

...odification/server/modifications/tabularmodifications/TabularGeneratorModificationsTest.java

-                .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)
+        reset();
+        ApiUtils.getGroupModifications(mockMvc, getGroupId()); // Getting two tabular modifications with respectively one and three sub-modifications


this call grows in SQL count if we create more tabular modifications
it might not be linked to this PR though, we might have this behaviour with other modifications
Same for other concerned tests

klesaulnier · 2024-02-09T08:06:31Z

.../modification/server/modifications/tabularmodifications/TabularBatteryModificationsTest.java

+        List<ModificationInfos> modifications = new ArrayList<>();
+        for (int i = 0; i <= qty; i++) {
+            modifications.add(
+                BatteryModificationInfos.builder()


It could be nice to add some "reactiveCapabilityCurvePoints" to this builder to cover the @entitygraph usage
Same for other concerned tests

etiennehomer added 14 commits January 22, 2024 14:59

Optimize get tabular modification

2629c4f

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

clean

0610ab2

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Clean

737df21

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

clean

030ce7b

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

clean

244b52a

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Extends to all tabular modifs

475b4fd

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

clean

316342d

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Merge branch 'main' into optim_get_tabular_modif

f77a3e0

Merge branch 'main' into optim_get_tabular_modif

6554117

Add cast to native query

6a662ee

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Change requests count in UT

7881819

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Add parametrized type

44f33e4

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

clean

bb9be45

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

Clean

7a5ecc9

Signed-off-by: Etienne Homer <etiennehomer@gmail.com>

etiennehomer changed the title ~~[WIP] Optim get tabular modif~~ Optim get tabular modif Feb 2, 2024

etiennehomer requested review from jonenst and TheMaskedTurtle February 2, 2024 16:10

TheMaskedTurtle force-pushed the optim_get_tabular_modif branch from 4ffe44f to 24d7e72 Compare February 5, 2024 17:14

fix: avoid to perform optimal request twice and complete eager retrie…

4858bea

…val for all tabulars Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>

TheMaskedTurtle force-pushed the optim_get_tabular_modif branch from 24d7e72 to 4858bea Compare February 5, 2024 17:52

TheMaskedTurtle and others added 5 commits February 6, 2024 10:25

Merge branch 'main' into optim_get_tabular_modif

1bec948

fix: add reactiveCapabilityCurvePoints for batteries

9384424

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

fix: add SQL select count tests on tabular modifications

5c8a214

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

Merge branch 'optim_get_tabular_modif' of github.com:gridsuite/networ…

4cb6b5f

…k-modification-server into optim_get_tabular_modif

chore: copyright and author

b7846eb

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

TheMaskedTurtle force-pushed the optim_get_tabular_modif branch from 069f21d to b7846eb Compare February 6, 2024 11:23

klesaulnier self-requested a review February 7, 2024 14:46

thangqp self-requested a review February 8, 2024 08:20

thangqp reviewed Feb 8, 2024

View reviewed changes

klesaulnier reviewed Feb 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optim get tabular modif #422

Optim get tabular modif #422

etiennehomer commented Jan 22, 2024

sonarqubecloud bot commented Feb 6, 2024

thangqp Feb 8, 2024

TheMaskedTurtle Feb 9, 2024

thangqp Feb 8, 2024

thangqp Feb 8, 2024

thangqp Feb 8, 2024

thangqp Feb 8, 2024 •

edited

Loading

klesaulnier left a comment

klesaulnier Feb 8, 2024

TheMaskedTurtle Feb 9, 2024

klesaulnier Feb 8, 2024

klesaulnier Feb 9, 2024

klesaulnier Feb 9, 2024

klesaulnier Feb 9, 2024

		@Query(value = "SELECT cast(modifications_id as varchar) from tabular_modification_modifications where tabular_modification_entity_id= ?1", nativeQuery = true)
		List<UUID> findSubModificationsIds(UUID uuid);

Optim get tabular modif #422

Are you sure you want to change the base?

Optim get tabular modif #422

Conversation

etiennehomer commented Jan 22, 2024

sonarqubecloud bot commented Feb 6, 2024

Quality Gate passed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thangqp Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

klesaulnier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thangqp Feb 8, 2024 •

edited

Loading