feat(rest): Create a new endpoint for database Sanitation. #2223

nikkuma7 · 2023-12-01T06:04:21Z

Please provide a summary of your changes here.

Which issue is this pull request belonging to and how is it solving it? (Refer to issue here)

Did you add or update any new dependencies that are required for your change?

Issue: #2224

Suggest Reviewer

You can suggest reviewers here with an @mention.

How To Test?

http://localhost:8080/resource/api/datasanitation/searchDublicate

How should these changes be tested by the reviewer?
Have you implemented any additional tests?

Checklist

Must:

All related issues are referenced in commit messages and in PR

smrutis1 · 2024-01-03T10:41:14Z

rest/resource-server/src/docs/asciidoc/databaseSanitation.adoc

@@ -0,0 +1,20 @@
+//
+// Copyright Siemens AG, 2017. Part of the SW360 Portal Project.


Please update the copyright year.

smrutis1 · 2024-01-03T11:09:14Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+        resource.add(linkTo(DatabaseSanitationController.class).slash("api" + DATABASESANITATION_URL).withRel("dataSanitation"));
+        return resource;
+    }
+    @RequestMapping(value = DATABASESANITATION_URL + "/searchDublicate", method = RequestMethod.GET)


Please correct the spelling /searchDublicate to /searchDuplicate or /findDuplicate

smrutis1 · 2024-01-03T11:10:36Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+        resource.add(linkTo(DatabaseSanitationController.class).slash("api" + DATABASESANITATION_URL).withRel("dataSanitation"));
+        return resource;
+    }
+    @RequestMapping(value = DATABASESANITATION_URL + "/searchDublicate", method = RequestMethod.GET)


This endpoint should be only visible to user with admin privileges. Now it seems visible to all. Please add @PreAuthorize("hasAuthority('ADMIN')") at controller level.

smrutis1 · 2024-01-03T11:13:55Z

...org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/Sw360DatabaseSanitationService.java

+    @Value("${sw360.thrift-server-url:http://localhost:8080}")
+    private String thriftServerUrl;
+
+    public Map<String, Object> dublicateIdentifiers(User sw360User, HttpServletRequest request) throws TException {


parameters sw360User and request never used. Kindly remove those.

smrutis1 · 2024-01-03T11:17:01Z

...org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/Sw360DatabaseSanitationService.java

+            }
+        }
+        if (oneIsNull(duplicateComponents, duplicateReleases, duplicateProjects, duplicateReleaseSources)) {
+            throw new HttpMessageNotReadableException("Some duplicates are null");


HttpMessageNotReadableException is used for client side error. Seems like this is not applicable here and same in line 75

smrutis1 · 2024-01-03T11:20:00Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+        } catch (Exception e) {
+            throw new TException(e.getMessage());
+        }
+        HttpStatus status = request == null ? HttpStatus.NO_CONTENT : HttpStatus.OK;


Request is always non null here. This seems to be not valid check

smrutis1 · 2024-01-03T11:22:09Z

...src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/databaseSanitationSpecTest.java

+
+    @MockBean
+    ProjectService.Iface projectClient;
+private HttpServletRequest request;


this variable request is never used. Please remove this.

smrutis1 · 2024-01-03T11:25:28Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+    @RequestMapping(value = DATABASESANITATION_URL + "/searchDublicate", method = RequestMethod.GET)
+    public ResponseEntity<CollectionModel<?>> searchDublicateIdentifiers(HttpServletRequest request )throws TException {
+        try {
+            User sw360User = restControllerHelper.getSw360UserFromAuthentication();


Remove this statement as sw360User is never used.

smrutis1 · 2024-01-03T11:37:02Z

...src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/databaseSanitationSpecTest.java

+    private Sw360DatabaseSanitationService sanitationService;
+
+    @MockBean
+    ComponentService.Iface componentClient;


Mocking client is not required as the service call itself is already mocked at line 95. Hence remove all the statements creating mocking of clients and using that.

smrutis1

Please look into the changes requested along with the below.

Please follow the java naming convention for the class name. databaseSanitationSpecTest.java to DatabaseSanitationSpecTest.java

smrutis1 · 2024-01-30T11:00:40Z

rest/resource-server/src/docs/asciidoc/databaseSanitation.adoc

+[[search-duplicate]]
+==== search duplicate identifiers
+
+A `GET` request will fetch the dublicate project and components.


Please correct the spelling dublicate to duplicate

smrutis1 · 2024-01-30T11:01:54Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+ * SPDX-License-Identifier: EPL-2.0
+ */
+
+package org.eclipse.sw360.rest.resourceserver.DatabaseSanitation;


package name should be small. Rename DatabaseSanitation to databasesanitization / databasesanitation

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

smrutis1 · 2024-01-30T11:36:17Z

...a/org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/DatabaseSanitationController.java

+        return resource;
+    }
+    @RequestMapping(value = DATABASESANITATION_URL + "/searchDuplicate", method = RequestMethod.GET)
+    public ResponseEntity<CollectionModel<?>> searchDuplicateIdentifiers()throws TException {


Indentation

Not resolved I think. Can be ignored though.

smrutis1 · 2024-01-30T11:39:06Z

...src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/databaseSanitationSpecTest.java

+    @Test
+    public void should_document_search_duplicate() throws Exception {
+        String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword);
+        mockMvc.perform(get("/api/dataSanitation/searchDuplicate")


Please add the response structure and document as well once the data is added in the response.

smrutis1 · 2024-01-31T06:15:46Z

...org/eclipse/sw360/rest/resourceserver/DatabaseSanitation/Sw360DatabaseSanitationService.java

+                duplicateReleaseSources = componentClient.getDuplicateReleaseSources();
+                duplicateProjects =  projectClient.getDuplicateProjects();
+            } else {
+                throw new RuntimeException("User is not admin");


This will give 500 - Internal Server Error. Appropriate should be 403- Forbidden

Unfortunately, I don't see the changes. Seems like still the runtime exception is thrown which will give a 500 response

rudra-superrr · 2024-03-20T08:13:21Z

Testing was successful.

rudra-superrr · 2024-03-20T08:16:07Z

One thing, getting 500 error code when no duplicates are found.

rudra-superrr · 2024-03-20T09:05:10Z

Either change the description to, used to fetch duplicate projects/components/releases/releaseSources or you can mention, it is used to fetch duplicate identifiers.
Also change search duplicate identifiers --> Search duplicate identifiers.

rudra-superrr · 2024-03-27T12:39:05Z

Display some message when no content is found.

Please change message for the highlighted portion.

nikkuma7 · 2024-03-28T06:18:39Z

Display some message when no content is found.

Please change message for the highlighted portion.

comment addressed.

rudra-superrr · 2024-03-29T05:47:32Z

Testing was successful.

smrutis1 · 2024-04-29T09:31:23Z

...org/eclipse/sw360/rest/resourceserver/databasesanitation/Sw360DatabaseSanitationService.java

+            if (sw360Exp.getErrorCode() == 403) {
+                throw new AccessDeniedException("User has not admin access");
+            } if (sw360Exp.getErrorCode() == 204) {
+                throw new SW360Exception(sw360Exp.getMessage()).setErrorCode(204);


indentation

Signed-off-by: Nikesh kumar <[email protected]>

smrutis1

Code looks ok to me.

rudra-superrr added the WIP work in progress label Dec 1, 2023

nikkuma7 force-pushed the fix/dataBaseSantitation branch 3 times, most recently from 88d3634 to 37e42ff Compare December 6, 2023 09:08

ag4ums added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for and removed WIP work in progress labels Dec 7, 2023

ag4ums assigned smrutis1 Dec 13, 2023

nikkuma7 force-pushed the fix/dataBaseSantitation branch from 37e42ff to b2c45c0 Compare December 14, 2023 07:24

smrutis1 requested changes Jan 3, 2024

View reviewed changes

smrutis1 removed the needs code review label Jan 3, 2024

nikkuma7 force-pushed the fix/dataBaseSantitation branch from b2c45c0 to 1bb6b5f Compare January 5, 2024 06:31

nikkuma7 force-pushed the fix/dataBaseSantitation branch 3 times, most recently from 964696a to 8a99b66 Compare January 16, 2024 07:41

smrutis1 requested changes Jan 31, 2024

View reviewed changes

nikkuma7 force-pushed the fix/dataBaseSantitation branch 2 times, most recently from a387789 to dc17c89 Compare February 6, 2024 06:32

nikkuma7 force-pushed the fix/dataBaseSantitation branch from dc17c89 to 96dabb4 Compare February 25, 2024 11:18

ag4ums added the has merge conflicts label Mar 19, 2024

ag4ums assigned rudra-superrr Mar 19, 2024

nikkuma7 force-pushed the fix/dataBaseSantitation branch from 96dabb4 to c9a7d9a Compare March 19, 2024 09:37

rudra-superrr removed the has merge conflicts label Mar 22, 2024

nikkuma7 force-pushed the fix/dataBaseSantitation branch from c9a7d9a to c4e3d67 Compare March 27, 2024 08:20

heliocastro added the New-UI Level for the API and UI level changes for the new-ui label Mar 27, 2024

nikkuma7 force-pushed the fix/dataBaseSantitation branch from c4e3d67 to a4e37e7 Compare March 28, 2024 06:18

nikkuma7 force-pushed the fix/dataBaseSantitation branch from a4e37e7 to bd2dd9b Compare March 29, 2024 04:52

rudra-superrr removed the needs general test This is general testing, meaning that there is no org specific issue to check for label Mar 29, 2024

smrutis1 reviewed Apr 29, 2024

View reviewed changes

fix(rest): Create a new endpoint for dataBaseSanitation.

6d9f362

Signed-off-by: Nikesh kumar <[email protected]>

nikkuma7 force-pushed the fix/dataBaseSantitation branch from bd2dd9b to 6d9f362 Compare April 29, 2024 10:02

smrutis1 approved these changes Apr 30, 2024

View reviewed changes

rudra-superrr added the ready ready to merge label May 2, 2024

ag4ums merged commit 43a7692 into eclipse-sw360:main May 8, 2024
1 of 2 checks passed

		@@ -0,0 +1,20 @@
		//
		// Copyright Siemens AG, 2017. Part of the SW360 Portal Project.

feat(rest): Create a new endpoint for database Sanitation. #2223

feat(rest): Create a new endpoint for database Sanitation. #2223

Conversation

nikkuma7 commented Dec 1, 2023 • edited Loading

Suggest Reviewer

How To Test?

Checklist

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

smrutis1 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

rudra-superrr commented Mar 20, 2024

rudra-superrr commented Mar 20, 2024

rudra-superrr commented Mar 20, 2024

rudra-superrr commented Mar 27, 2024

nikkuma7 commented Mar 28, 2024

rudra-superrr commented Mar 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smrutis1 left a comment

Choose a reason for hiding this comment

nikkuma7 commented Dec 1, 2023 •

edited

Loading