[MODCON - 65] - Implement endpoint to save shared setting uuid in all tenants #69

azizbekxm · 2023-07-14T12:35:50Z

Purpose

https://issues.folio.org/browse/MODCON-65

Approach

I will implement

Endpoint contract:
POST /consortia/<consortiumId>/sharing/settings
{
    "settingId": "<UUID>",
    "url": "string",
    "payload": <<object>>
}
Response:
{
    "createSettingsPcId": "<UUID>",
    "updateSettingsPcId": "<UUID>"
}

based on this diagram

TODOS and Open Questions

Learning

Pre-Merge Checklist:

Before merging this PR, please go through the following list and take appropriate actions.

Does this PR meet or exceed the expected quality standards?
- Code coverage on new code is 80% or greater
- Duplications on new code is 3% or less
- There are no major code smells or security issues
Does this introduce breaking changes?
- Were any API paths or methods changed, added or removed?
- Were there any schema changes?
- Did any of the interface versions change?
- Were permissions changed, added or removed?
- Are there new interface dependencies?
- There are no breaking changes in this PR.

If there are breaking changes, please STOP and consider the following:

What other modules will these changes impact?
Do JIRAs exist to update the impacted modules?
- If not, please create them
- Do they contain the appropriate level of detail? Which endpoints/schemas changed, etc.
- Do they have all the appropriate links to blocked/related issues?
Are the JIRAs under active development?
- If not, contact the project's PO and make sure they're aware of the urgency.
Do PRs exist for these changes?
- If so, have they been approved?
Did you modify code to call some additional endpoints?
- If so, do you check that necessary module permission added in ModuleDescriptor-template.yaml file?

Ideally, all the PRs involved in breaking changes would be merged on the same day
to avoid breaking the folio-testing environment.
Communication is paramount if that is to be achieved,
especially as the number of inter-module and inter-team dependencies increase.

While it's helpful for reviewers to help identify potential problems,
ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.

… tenants

src/main/java/org/folio/consortia/controller/SharingSettingController.java

azizbekxm · 2023-07-19T13:13:50Z

Request

Database changes

before sharing_setting table

after request sharing_setting table

logs

15:23:11 http-nio-8081-exec-5 [363206/consortia] [diku] [4f53dd76-d8e3-5cf0-a4e2-b51550acb037] [mod-consortia] INFO  ngSettingServiceImpl start:: tenants with size: 2 successfully added to appropriate publication request
15:23:11 http-nio-8081-exec-5 [363206/consortia] [diku] [4f53dd76-d8e3-5cf0-a4e2-b51550acb037] [mod-consortia] INFO  ngSettingServiceImpl start:: set source as a consortium
15:23:11 http-nio-8081-exec-5 [] [diku] [] [mod-consortia] INFO  AuthService          Attempt login with url=http://localhost:9130 tenant=diku username=consortia-system-user.
15:23:11 http-nio-8081-exec-5 [] [diku] [] [mod-consortia] INFO  AuthService          Logged in as consortia-system-user in tenant diku
15:23:22 http-nio-8081-exec-5 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  blicationServiceImpl Created publication record bdfd4259-56d1-42ee-9640-6f696d7ee2fc
15:23:24 ConsortiaAsync-2 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:24 pool-3-thread-1 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:24 pool-3-thread-1 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:25 ConsortiaAsync-2 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:25 ConsortiaAsync-2 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  blicationServiceImpl updatePublicationsStatus:: updated publication record bdfd4259-56d1-42ee-9640-6f696d7ee2fc with status ERROR
15:23:25 http-nio-8081-exec-5 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  blicationServiceImpl Created publication record d3cf7066-02ee-4926-a37b-7e30ac552554
15:23:28 ConsortiaAsync-3 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:28 pool-4-thread-1 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant testenant7664803557551864676
15:23:28 pool-4-thread-1 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:28 ConsortiaAsync-3 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  TenantContextUtils   FOLIO context initialized with tenant diku
15:23:28 ConsortiaAsync-3 [] [diku] [5c780098-751b-4477-9b61-0f75bc452a73] [mod-consortia] INFO  blicationServiceImpl updatePublicationsStatus:: updated publication record d3cf7066-02ee-4926-a37b-7e30ac552554 with status ERROR

… tenants

src/main/java/org/folio/consortia/controller/SharingSettingController.java

src/main/java/org/folio/consortia/domain/entity/SharingSettingEntity.java

src/main/java/org/folio/consortia/service/impl/SharingSettingServiceImpl.java

src/test/java/org/folio/consortia/service/SharingSettingServiceTest.java

src/main/java/org/folio/consortia/service/impl/SharingSettingServiceImpl.java

… tenants

okolawole-ebsco · 2023-07-20T13:13:51Z

src/main/java/org/folio/consortia/service/impl/SharingInstanceServiceImpl.java

@@ -80,8 +77,7 @@ public SharingInstance start(UUID consortiumId, SharingInstance sharingInstance)
      }

      try (var context = new FolioExecutionContextSetter(prepareContextForTenant(targetTenantId, folioModuleMetadata, folioExecutionContext))) {
-        String source = FOLIO_SOURCE_VALUE.equalsIgnoreCase(inventoryInstance.get("source").asText()) ? CONSORTIUM_FOLIO : CONSORTIUM_MARK;


I am not sure if it still stands but it should be CONSORTIUM_MARC not CONSORTIUM_MARK.

okolawole-ebsco · 2023-07-20T13:26:33Z

src/main/java/org/folio/consortia/utils/HelperUtils.java

 public class HelperUtils {

+  private static final String FOLIO_SOURCE_VALUE = "folio";
+  private static final String CONSORTIUM_FOLIO = "CONSORTIUM-FOLIO";
+  private static final String CONSORTIUM_MARK = "CONSORTIUM-MARC";


Change this to CONSORTIUM_MARC

okolawole-ebsco · 2023-07-20T13:29:31Z

src/main/java/org/folio/consortia/utils/HelperUtils.java

@@ -17,4 +25,9 @@ public static String randomString(Integer noOfString) {
    RandomStringGenerator generator = new RandomStringGenerator.Builder().withinRange('a', 'z').build();
    return generator.generate(noOfString);
  }
+
+  public static ObjectNode setSourceAsConsortium(JsonNode payload) {
+    String source = FOLIO_SOURCE_VALUE.equalsIgnoreCase(payload.get("source").asText()) ? CONSORTIUM_FOLIO: CONSORTIUM_MARK;


I think we should be more explicit here to map folio -> CONSORTIUM-FOLIO and marc -> CONSORTIUM-MARC. if other source values are introduced in the future, it would default to CONSORTIUM-MARC and that wont be ideal. Probably should return an error if we find a source value that is not recognized.

Do you mean this structure

if source is folio, then it will be CONSORTIUM-FOLIO

If source is marc, then it will be CONSORTIUM-MARC.

all other type will throw exceptino

What about instance? is it still valid scenario for sharing instance (this code is used by share instance method also)

The screenshot looks incorrect to me because it is saying if source = CONSORTIUM_FOLIO, set source to CONSORTIUM_FOLIO. But the words are what I expect.

I was not aware that this was the case. it was my impression that we would save "consortium" instead of "consortium-folio" for settings. I was thinking about my initial comment in the instance context instead of settings.

Generally, what I am looking for is that CONSORTIUM-MARC is not the default output when source does not match CONSORTIUM-FOLIO.

Yes agree, @azizbekxm for instances if source = "folio" - result should be "CONSORTIUM_FOLIO", if source = "marc" - result should be "CONSORTIUM_MARC" or exception should be thrown
For Settings it always should be "consortium"

@okolawole-ebsco maybe we can use lower case as "consortium_folio" or "consortium_marc" to make it consistent?

@azizbekxm handling such situation using flag isSetting is not approprtiate here, you can leave constants in utility class and move business logic to appropriate services

Fixed big thanks

… tenants

src/main/java/org/folio/consortia/service/impl/SharingInstanceServiceImpl.java

… tenants

src/main/java/org/folio/consortia/service/impl/SharingSettingServiceImpl.java

… tenants

tatsiana-tarhonskaya · 2023-07-21T14:14:59Z

src/main/java/org/folio/consortia/service/impl/SharingSettingServiceImpl.java

+    log.info("start:: The Sharing Settings for settingId '{}' and '{}' unique tenant(s) were successfully saved to the database",
+      sharingSettingRequest.getSettingId(), publicationPostRequest.getTenants().size());
+
+    ObjectMapper objectMapper = new ObjectMapper();


this should be a class field

Moved to class field

This field exists in application context, so can be injected

sonarcloud · 2023-07-24T09:13:00Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

91.4% Coverage
0.0% Duplication

[MODCON - 65] - Implement endpoint to save shared setting uuid in all…

c0a3024

… tenants

SerhiiNosko reviewed Jul 14, 2023

View reviewed changes

src/main/java/org/folio/consortia/controller/SharingSettingController.java Outdated Show resolved Hide resolved

azizbekxm added 2 commits July 18, 2023 17:56

Merge branch 'master' into MODCON-65

91fa05e

Merge branch 'master' into MODCON-65

2304eae

[MODCON - 65] - Implement endpoint to save shared setting uuid in all…

2718ac4

… tenants

azizbekxm marked this pull request as ready for review July 19, 2023 14:43

azizbekxm requested a review from a team July 19, 2023 14:50