get Composite Modification endpoint #541

Mathieu-Deharbe · 2024-10-08T12:44:14Z

No description provided.

Signed-off-by: Mathieu DEHARBE <[email protected]>

ghazwarhili · 2024-10-29T09:17:11Z

src/main/java/org/gridsuite/modification/server/NetworkModificationController.java

+    @GetMapping(value = "/network-composite-modification/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
+    @Operation(summary = "Get the content of a composite modification via its id")
+    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Content of the composite modification"),
+        @ApiResponse(responseCode = "404", description = "This composite modification does not exist")}


404 code is for not found response
for readability reasons I suggest
@ApiResponses(value = { @ApiResponse(responseCode = "200", description = "Content of the composite modification"), @ApiResponse(responseCode = "404", description = "Composite modification not found") })

If the id is not found in the database, an exception should be thrown by networkModificationService
Currently, it returns 200 OK even if the id doesn’t exist !...
fix it (with try-catch or another solution)

Message changed. Remobved that 404.

ghazwarhili · 2024-10-29T09:30:24Z

src/main/java/org/gridsuite/modification/server/NetworkModificationController.java

+    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Content of the composite modification"),
+        @ApiResponse(responseCode = "404", description = "This composite modification does not exist")}
+    )
+    public ResponseEntity<List<ModificationInfos>> getCompositeModificationContent(@PathVariable("id") UUID compositeModificationId) {


@PathVariable(value = "id", required = true) UUID compositeModificationId

to ensure that the id is mandatory

For PathVariable boolean required() default true

If I add a true, intellij calls it a redundant parameter.

ghazwarhili · 2024-10-29T09:43:20Z

src/main/java/org/gridsuite/modification/server/NetworkModificationController.java

@@ -220,6 +220,18 @@ public ResponseEntity<UUID> createNetworkCompositeModification(@RequestBody List
        return ResponseEntity.ok().body(networkModificationService.createNetworkCompositeModification(modificationUuids));
    }

+    @GetMapping(value = "/network-composite-modification/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
+    @Operation(summary = "Get the content of a composite modification via its id")
+    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Content of the composite modification"),


Content is not clear enough and it is generic description
maybe "The modifications list of the composite"

I kind of updated all of this to be closer to @GetMapping(value = "/groups/{groupUuid}/network-modifications", produces = MediaType.APPLICATION_JSON_VALUE)

I think it was clearer that my previous endpoint.

ghazwarhili · 2024-10-29T09:45:15Z

src/main/java/org/gridsuite/modification/server/NetworkModificationController.java

+    public ResponseEntity<List<ModificationInfos>> getCompositeModificationContent(@PathVariable("id") UUID compositeModificationId) {
+        return ResponseEntity.ok()
+                .contentType(MediaType.APPLICATION_JSON)
+                .body(networkModificationService.getCompositeModificationContentMetadata(compositeModificationId)


Renamed getNetworkModificationsFromComposite

I am still not very satisfied of it, but still better than metadata indeed.

ghazwarhili · 2024-10-29T09:54:47Z

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

    @Transactional(readOnly = true)
-    public List<ModificationInfos> getCompositeModificationsInfos(@NonNull List<UUID> uuids) {
+    public List<ModificationInfos> getCompositeModificationsContentInfos(@NonNull List<UUID> uuids, boolean onlyCommonData) {


don't need to rename this

Ok but I don't think this is a good name : it sounds like we are getting the infos of the composite modifications but we are not. We get the infos of the network modifications that are inside the composite modifications and don't get those of the composite modifications themselves.

It sent me in the wrong direction when I was looking at functions.

ghazwarhili · 2024-10-29T10:02:20Z

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

@@ -394,15 +394,28 @@ public List<ModificationInfos> getModificationsInfos(@NonNull List<UUID> uuids)
        return uuids.stream().map(entities::get).filter(Objects::nonNull).map(this::getModificationInfos).toList();
    }

+    /**
+     * @param onlyCommonData if true, only returns the basic data common to all the modifications. If false, returns complete modifications


Could you explain the basic of the data and what do you mean by onlyCommonData?
It doesn't make sense to me

All the network modifications use the ModificationInfos. The data inside this class is the common data to all the network modifications.

But they also have specific data extending this data like BatteryModificationInfos or LineCreationInfos.

ghazwarhili · 2024-10-29T10:07:42Z

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

-                    .stream()
-                    .map(this::getModificationInfo)
-                    .toList();
+            List<UUID> networkModificationsUuids = modificationRepository.findModificationIdsByCompositeModificationId(uuid);


foundEntities

Do you mean renaming networkModificationsUuids ? But those are not entities.

ghazwarhili · 2024-10-29T10:08:29Z

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

+            List<UUID> networkModificationsUuids = modificationRepository.findModificationIdsByCompositeModificationId(uuid);
+            List<ModificationInfos> orderedModifications;
+            if (onlyCommonData) {
+                List<ModificationEntity> networkModifications = modificationRepository.findBaseDataByIdIn(networkModificationsUuids);


networkModificationsEntities

ghazwarhili · 2024-10-29T10:22:30Z

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

+                        .stream()
+                        .map(this::getModificationInfo)
+                        .toList();
+            }


if (uuids.isEmpty()) { return Collections.emptyList(); }

and we can do better by fetching all modification uuids in one go like

.collect(Collectors.toMap( uuid -> uuid, modificationRepository::findModificationIdsByCompositeModificationId)); }

I don't understand, but I completely changed this funciton anyway so I don't think this is still needed.

ghazwarhili · 2024-10-29T10:23:30Z

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

+                        .stream()
+                        .map(this::getModificationInfo)
+                        .toList();
+            }
            entities.addAll(orderedModifications);
        }


for (UUID uuid : uuids) { List<UUID> networkModificationsUuids = networkModificationsMap.get(uuid); if (onlyMetadata) { List<ModificationEntity> networkModifications = modificationRepository.findBaseDataByIdIn(networkModificationsUuids); entities.addAll(networkModifications.stream() .map(this::getModificationInfos) .toList()); } else { entities.addAll(networkModificationsUuids.stream() .map(this::getModificationInfo) .toList()); } } return Collections.unmodifiableList(entities); }

Same answer than the previous comment.

etiennehomer · 2024-10-29T12:14:59Z

src/main/java/org/gridsuite/modification/server/NetworkModificationController.java

@@ -220,6 +220,18 @@ public ResponseEntity<UUID> createNetworkCompositeModification(@RequestBody List
        return ResponseEntity.ok().body(networkModificationService.createNetworkCompositeModification(modificationUuids));
    }

+    @GetMapping(value = "/network-composite-modification/{id}", produces = MediaType.APPLICATION_JSON_VALUE)


Suggested change

@GetMapping(value = "/network-composite-modification/{id}", produces = MediaType.APPLICATION_JSON_VALUE)

@GetMapping(value = "/network-composite-modification/{uuid}", produces = MediaType.APPLICATION_JSON_VALUE)

OK done and updated with a /network-modifications. I think this is clearer.

etiennehomer · 2024-10-29T13:23:28Z

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

    @Transactional(readOnly = true)
-    public List<ModificationInfos> getCompositeModificationsInfos(@NonNull List<UUID> uuids) {
+    public List<ModificationInfos> getCompositeModificationsContentInfos(@NonNull List<UUID> uuids, boolean onlyCommonData) {


I'm not sure this method should return wether a list of modification base infos or a modification full infos. You could rather implement 2 two methods

Yes, it gets too confusing. I separated them.

Signed-off-by: Mathieu DEHARBE <[email protected]>

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

Signed-off-by: Mathieu DEHARBE <[email protected]>

sonarqubecloud · 2024-11-13T10:22:45Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

draft getCompositeModification endpoint

190e807

Signed-off-by: Mathieu DEHARBE <[email protected]>

This was referenced Oct 8, 2024

get composite modification content endpoint gridsuite/explore-server#102

Merged

Visualize composite modifications gridsuite/gridexplore-app#538

Merged

Mathieu-Deharbe and others added 6 commits October 9, 2024 17:02

metadata instead of complete ModificationInfos

94c049d

Signed-off-by: Mathieu DEHARBE <[email protected]>

optimize database requests

0dcc187

Signed-off-by: Mathieu DEHARBE <[email protected]>

Merge branch 'main' into get-composite-modification-content

b4fe982

Merge branch 'main' into get-composite-modification-content

04d95b4

TUs

959bfd4

Signed-off-by: Mathieu DEHARBE <[email protected]>

pleasing sonar

659dfb2

Signed-off-by: Mathieu DEHARBE <[email protected]>

Mathieu-Deharbe changed the title ~~draft get Composite Modification endpoint~~ get Composite Modification endpoint Oct 24, 2024

Mathieu-Deharbe and others added 3 commits October 24, 2024 13:25

cleaning + order by

b001a7f

Signed-off-by: Mathieu DEHARBE <[email protected]>

Merge branch 'main' into get-composite-modification-content

6886fb4

clean post merge conflicts

d890755

Signed-off-by: Mathieu DEHARBE <[email protected]>

Mathieu-Deharbe requested review from etiennehomer and ghazwarhili October 24, 2024 16:31

ghazwarhili requested changes Oct 29, 2024

View reviewed changes

etiennehomer reviewed Oct 29, 2024

View reviewed changes

Mathieu-Deharbe and others added 3 commits October 31, 2024 16:44

post reviews

002e663

Signed-off-by: Mathieu DEHARBE <[email protected]>

removed unhandled 404

6277098

Signed-off-by: Mathieu DEHARBE <[email protected]>

Merge branch 'main' into get-composite-modification-content

8dbf573

Mathieu-Deharbe requested a review from ghazwarhili October 31, 2024 16:19

etiennehomer reviewed Nov 5, 2024

View reviewed changes

src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java Outdated Show resolved Hide resolved

etiennehomer reviewed Nov 5, 2024

View reviewed changes

src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java Outdated Show resolved Hide resolved

etiennehomer reviewed Nov 5, 2024

View reviewed changes

src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java Outdated Show resolved Hide resolved

etiennehomer approved these changes Nov 5, 2024

View reviewed changes

removed empty lines

5443632

Signed-off-by: Mathieu DEHARBE <[email protected]>

ghazwarhili approved these changes Nov 8, 2024

View reviewed changes

Merge branch 'main' into get-composite-modification-content

4d88cac

Mathieu-Deharbe merged commit 4276dda into main Nov 13, 2024
3 checks passed

Mathieu-Deharbe deleted the get-composite-modification-content branch November 13, 2024 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

get Composite Modification endpoint #541

get Composite Modification endpoint #541

Mathieu-Deharbe commented Oct 8, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024 •

edited

Loading

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

ghazwarhili Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

etiennehomer Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

etiennehomer Oct 29, 2024

Mathieu-Deharbe Oct 31, 2024

sonarqubecloud bot commented Nov 13, 2024

	@GetMapping(value = "/network-composite-modification/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
	@GetMapping(value = "/network-composite-modification/{uuid}", produces = MediaType.APPLICATION_JSON_VALUE)

get Composite Modification endpoint #541

get Composite Modification endpoint #541

Conversation

Mathieu-Deharbe commented Oct 8, 2024

Choose a reason for hiding this comment

Mathieu-Deharbe Oct 31, 2024 • edited Loading

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 13, 2024

Quality Gate passed

Mathieu-Deharbe Oct 31, 2024 •

edited

Loading