[GridDyna] Add some batch CRUD endpoints #111

thangqp · 2024-05-07T13:50:17Z

Related PRs:

#111

gridsuite/dynamic-simulation-server#95

gridsuite/dynamic-mapping-server#96

PR powsybl-dynawo:

https://github.com/powsybl/powsybl-dynawo/pull/349 (released 2.5.0-alpha-1)

…int_

Mathieu-Deharbe

I did not finished, not tested yet.

src/main/java/org/gridsuite/filter/server/FilterController.java

etiennehomer · 2024-07-01T09:39:19Z

src/main/java/org/gridsuite/filter/server/FilterController.java

    @PostMapping(value = "/filters", params = "duplicateFrom")
    @Operation(summary = "Duplicate a filter")
-    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The filter has been successfully created"),
+    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The uuid of the new filter has been successfully duplicated"),


Suggested change

@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The uuid of the new filter has been successfully duplicated"),

@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The filter with uuid filterId has been successfully duplicated"),

Done, I have a slight change from your suggestion to "The filter with given id has been successfully duplicated"

etiennehomer · 2024-07-01T12:11:02Z

src/main/java/org/gridsuite/filter/server/FilterController.java

@@ -85,8 +95,19 @@ public ResponseEntity<UUID> duplicateFilter(@RequestParam("duplicateFrom") UUID
                .orElse(ResponseEntity.notFound().build());
    }

+    @PostMapping(value = "/filters/batch/duplicate")
+    @Operation(summary = "Duplicate filters from provided uuids")
+    @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The map of sourceUuid and newUuid of duplicated filters"),


Suggested change

@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The map of sourceUuid and newUuid of duplicated filters"),

@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Filters have been successfully duplicated."),

Done, I have a slight change from your suggestion to "Filters of given ids have been successfully duplicated"

src/main/java/org/gridsuite/filter/server/FilterController.java

etiennehomer · 2024-07-01T14:17:12Z

src/main/java/org/gridsuite/filter/server/FilterService.java

@@ -205,13 +219,70 @@ public <F extends AbstractFilter> void changeFilter(UUID id, F newFilter, String
        notificationService.emitElementUpdated(id, userId);
    }

+    @Transactional
+    public List<AbstractFilter> changeFilters(Map<UUID, AbstractFilter> filtersToModifyMap) {


Suggested change

public List<AbstractFilter> changeFilters(Map<UUID, AbstractFilter> filtersToModifyMap) {

public List<AbstractFilter> updateFilters(Map<UUID, AbstractFilter> filtersToModifyMap) {

This method is quite complicated. As we can assume only few filters will be updated at one time, you can iterate on filtersToModifyMap and call changeFilter.
And rename changeFilter to updateFilter

Done in 0e8609f

…f directly via 'this'

Mathieu-Deharbe · 2024-07-03T12:40:31Z

src/main/java/org/gridsuite/filter/server/FilterController.java

@@ -85,18 +95,38 @@ public ResponseEntity<UUID> duplicateFilter(@RequestParam("duplicateFrom") UUID
                .orElse(ResponseEntity.notFound().build());
    }

+    @PostMapping(value = "/filters/batch/duplicate")


Adding duplicate looks like a good way to differentiate the two /filters/batch but I don't get the fact that this has not be done for the 2 @PostMapping(value = "/filters") (the creation and the duplication have the same path).

Our rest API best practices are not very precise about extra data like this. But they state that the path should only be used to identify a data.

@etiennehomer shouldn't we modify the filters duplication signature in an other PR ?

We should add "duplicate" in the path of duplicate endpoint, i.e. POST /filters/duplicate but we have to changer also in gridexplorer-app. I think we can change in another TS

In fact we should have :
POST /filters
POST /filters/batch
POST /filters/duplicate
POST /filters/duplicate/batch

At least I can change from /filters/batch/duplicate to /filters/duplicate/batch, what do you think??? @etiennehomer @Mathieu-Deharbe

Ok. I think the API you propose is good Thang. So let's change POST /filters/batch/duplicate to POST /filters/duplicate/batch
And then, in another PR, let's change for unitary duplication endpoint

DONE in 0713dfc

src/main/java/org/gridsuite/filter/server/FilterService.java

...rg/gridsuite/filter/server/repositories/proxies/expertfiler/ExpertFilterRepositoryProxy.java

src/test/java/org/gridsuite/filter/server/FilterEntityControllerTest.java

…int_

sonarqubecloud · 2024-07-10T11:04:18Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

jonenst · 2024-07-11T12:52:38Z

Read through it nothing stands out as blocking

jonenst · 2024-07-11T12:53:10Z

nitpick: /batch endpoints do we have endpoints like this in other parts of the code ?

thangqp · 2024-07-12T16:14:40Z

nitpick: /batch endpoints do we have endpoints like this in other parts of the code ?

NO, I use /batch to distinguish two endpoints 'single' and 'batch' processing for the same HTTP verb POST

thangqp and others added 13 commits May 7, 2024 15:48

[GridDyna] Add some batch CRUD endpoints

fdd3458

Set to current version of filter library

6a29931

Update URI path

3680536

Merge branch 'main' into dynamic_simulation_add_some_batch_crud_endpo…

2549a85

…int_

Merge branch 'main' into dynamic_simulation_add_some_batch_crud_endpo…

db4c530

…int_

Merge from main

588ae16

Add tests for new endpoints

dcebe97

Sonar

c508737

merge from main

4834285

Add modify all endpoint

1190a4b

Add tests

2251b0c

Update to filter library 1.0.8

c0ac2fc

Merge branch 'main' into dynamic_simulation_add_some_batch_crud_endpo…

d508e0f

…int_

Mathieu-Deharbe self-requested a review June 21, 2024 14:04

Mathieu-Deharbe reviewed Jun 21, 2024

View reviewed changes

thangqp added 4 commits June 21, 2024 20:13

Update description for endpoints

f551b4c

Using AssertJ recursive for test expert rules

54201b9

undo set id from dto to entity

e8236a8

merge from main

2f3c535

etiennehomer requested changes Jul 1, 2024

View reviewed changes

thangqp added 4 commits July 1, 2024 20:03

Correct as suggest of Etienne

0e8609f

Tiny correction for description

1f22f0b

Sonar Call transactional methods via an injected dependency instead o…

d11226a

…f directly via 'this'

Enrich test for update in batch with none existing filter id

cb40b78

etiennehomer approved these changes Jul 2, 2024

View reviewed changes

thangqp requested a review from Mathieu-Deharbe July 2, 2024 08:25

This was referenced Jul 2, 2024

[GridDyna] Using gridsuite filter library for mapping gridsuite/dynamic-mapping-server#96

Merged

[GridDyna] Using gridsuite filter library for mapping gridsuite/dynamic-simulation-server#95

Merged

Mathieu-Deharbe reviewed Jul 3, 2024

View reviewed changes

Correct following some comments of Mathieu

6c2ba17

thangqp and others added 2 commits July 5, 2024 02:07

Merge branch 'main' into dynamic_simulation_add_some_batch_crud_endpo…

bd50593

…int_

change url path in the filte-server endpoint for duplication in batch

0713dfc

Mathieu-Deharbe approved these changes Jul 10, 2024

View reviewed changes

jonenst approved these changes Jul 11, 2024

View reviewed changes

thangqp merged commit 1cb6ca5 into main Jul 12, 2024
3 checks passed

thangqp deleted the dynamic_simulation_add_some_batch_crud_endpoint_ branch July 12, 2024 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GridDyna] Add some batch CRUD endpoints #111

[GridDyna] Add some batch CRUD endpoints #111

thangqp commented May 7, 2024 •

edited

Loading

Mathieu-Deharbe left a comment

etiennehomer Jul 1, 2024

thangqp Jul 1, 2024

etiennehomer Jul 1, 2024

thangqp Jul 1, 2024

etiennehomer Jul 1, 2024

etiennehomer Jul 1, 2024

thangqp Jul 1, 2024

Mathieu-Deharbe Jul 3, 2024

thangqp Jul 3, 2024

etiennehomer Jul 10, 2024

thangqp Jul 10, 2024

sonarqubecloud bot commented Jul 10, 2024

jonenst commented Jul 11, 2024

jonenst commented Jul 11, 2024

thangqp commented Jul 12, 2024

	@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The uuid of the new filter has been successfully duplicated"),
	@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The filter with uuid filterId has been successfully duplicated"),

	public List<AbstractFilter> changeFilters(Map<UUID, AbstractFilter> filtersToModifyMap) {
	public List<AbstractFilter> updateFilters(Map<UUID, AbstractFilter> filtersToModifyMap) {

[GridDyna] Add some batch CRUD endpoints #111

[GridDyna] Add some batch CRUD endpoints #111

Conversation

thangqp commented May 7, 2024 • edited Loading

Mathieu-Deharbe 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

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 Jul 10, 2024

Quality Gate passed

jonenst commented Jul 11, 2024

jonenst commented Jul 11, 2024

thangqp commented Jul 12, 2024

thangqp commented May 7, 2024 •

edited

Loading