Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Root network deletion #643

Merged
merged 18 commits into from
Dec 12, 2024
Merged

Root network deletion #643

merged 18 commits into from
Dec 12, 2024

Conversation

klesaulnier
Copy link
Contributor

No description provided.

LE SAULNIER Kevin added 12 commits November 29, 2024 14:14
feat: root network deletion implementation
2b5dc58 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: remove unnecessary service dependencies in computation services …
3193ad4 
…to prevent circular dependencies

Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: tests
bd5d988 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: add comments
b2d7355 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
test: deletion test
cf7d3b5 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: obsolete comment
7219124 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: checkstyle
8abe55f 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: checkstyle
752f816 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: clean code
7eb99f0 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: when getting root network creationg message from consumer, if no…
d221460 
… creation request -> deletion

Signed-off-by: LE SAULNIER Kevin <[email protected]>
fix: checkstyle
8d2d1ef 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
Merge remote-tracking branch 'origin/root-network-creation' into root…
9cf9774 
…-network-delete
@SlimaneAmar SlimaneAmar self-requested a review December 9, 2024 13:03
SlimaneAmar
SlimaneAmar reviewed Dec 11, 2024
View reviewed changes
src/main/java/org/gridsuite/study/server/RestResponseEntityExceptionHandler.java Outdated Show resolved Hide resolved
src/main/java/org/gridsuite/study/server/repository/rootnetwork/RootNetworkEntity.java Outdated
.reportUuid(this.reportUuid)
.build();

if (this.rootNetworkNodeInfos != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to initialize with empty list et remove test
private List<RootNetworkNodeInfoEntity> rootNetworkNodeInfos = new ArrayList<>();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to do it but it's breaking tests, causing NPE when calling "addRootNetworkNodeInfo"

src/main/java/org/gridsuite/study/server/service/StudyService.java Outdated
@@ -292,7 +295,8 @@ public RootNetworkCreationRequestInfos createRootNetwork(UUID studyUuid, UUID ca
try {
networkConversionService.persistNetwork(caseUuid, studyUuid, rootNetworkUuid, null, userId, importReportUuid, caseFormat, importParameters, CaseImportAction.ROOT_NETWORK_CREATION);
} catch (Exception e) {
//TODO: implement rootNetworkService.deleteRootNetworkIfNotCreationInProgress(rootNetworkCreationRequestEntity.getId(), userId);
//TODO: don't throw another error if deletion fails ?
rootNetworkService.delete(rootNetworkUuid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteCreationRequest ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I'll add a test for this case, a failing creation should not stay in database

Copy link
Contributor Author

@klesaulnier klesaulnier Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
I even removed the previous delete
it would not have done anything since no rootNetworkEntity will be in database if the http call fails

src/main/java/org/gridsuite/study/server/service/StudyService.java
@@ -283,6 +279,13 @@ public BasicStudyInfos createStudy(UUID caseUuid, String userId, UUID studyUuid,
}

@Transactional
public void deleteRootNetwork(UUID studyUuid, UUID rootNetworkUuid, String userId) {
assertIsStudyExist(studyUuid);
rootNetworkService.assertIsRootNetworkInStudy(rootNetworkUuid, studyUuid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need assertIsRootNetworkInStudy ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we make the same assertion when deleting network modifications, but not when deleting nodes
but why do we need "studyUuid" in endpoint if we don't even use it ?

src/main/java/org/gridsuite/study/server/service/RootNetworkService.java Show resolved Hide resolved
src/test/java/org/gridsuite/study/server/RootNetworkTest.java
}

@Test
void testDeleteRootNetwork() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to test remote services with wiremock ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no remote service called here, all services supposed to make http calls to remote services are mocked and tested with Mockito

src/main/java/org/gridsuite/study/server/service/RootNetworkService.java Show resolved Hide resolved
fix: PR remarks
b90d812 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
13 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@klesaulnier klesaulnier merged commit 7e9ea57 into root-network-creation Dec 12, 2024
4 checks passed
@klesaulnier klesaulnier deleted the root-network-delete branch December 12, 2024 12:31
klesaulnier added a commit that referenced this pull request Dec 12, 2024
@klesaulnier
Root network creation and deletion (#642 - #643)
57d5583 
Signed-off-by: LE SAULNIER Kevin <[email protected]>
Co-authored-by: Slimane AMAR <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@souissimai souissimai Awaiting requested review from souissimai

@SlimaneAmar SlimaneAmar Awaiting requested review from SlimaneAmar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@klesaulnier @SlimaneAmar