MODBULKOPS-337 - Stage 3b - FQM Performance (FQL): switching matched records flow to use mod-bulk-operations capabilities #423

obozhko-folio · 2025-05-29T11:46:08Z

MODBULKOPS-337 - Stage 3b - FQM Performance (FQL): switching matched records flow to use mod-bulk-operations capabilities

Purpose

Fix bug with JSON from FQM.

Approach

Use getters/setters for json properties, JsonIgnore for objects which are absent in FQM response.

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 they 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?

Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule 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.

khandramai · 2025-06-02T07:31:29Z

src/main/java/org/folio/bulkops/util/FqmContentFetcher.java

+          var jsonb = json.get(getContentJsonKey(entityType));
+          try {
+            ObjectNode extendedRecordWrapper = objectMapper.createObjectNode();
+            extendedRecordWrapper.set("entity", objectMapper.readTree(jsonb.toString()));


Strings can be used as constants.

khandramai · 2025-06-02T07:32:38Z

src/main/java/org/folio/bulkops/util/FqmContentFetcher.java

+            log.error("Error processing JSON content for entity type {}: {}", entityType, e.getMessage());
+            throw new FqmFetcherException("Error processing JSON content", e);
+          }
+        }).collect(Collectors.joining("\n"))


'\n' new line separator is already introduced as constant (or it makes sense to introduce such constant).

khandramai · 2025-06-02T07:33:41Z

src/main/java/org/folio/bulkops/util/FqmContentFetcher.java

+
+  private String getContentTenantKey(EntityType entityType) {
+    return switch(entityType) {
+      case USER -> null;


Maybe empty string will be safer then null?

khandramai · 2025-06-02T07:35:01Z

src/main/java/org/folio/bulkops/client/MaterialTypeClient.java

 import org.springframework.web.bind.annotation.RequestParam;

 @FeignClient(name = "material-types", configuration = FeignClientConfiguration.class)
 public interface MaterialTypeClient {

  @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE)
  MaterialTypeCollection getByQuery(@RequestParam String query);
+
+  @GetMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
+  MaterialType getById(@PathVariable String id);


Can we use here UUID type instead of String?

Probably yes, but in all other clients related to reference data we use String.

Let's stay as is.

khandramai · 2025-06-02T07:38:23Z

src/main/java/org/folio/bulkops/domain/converter/LoanTypeConverter.java

+
+  @Override
+  public LoanType convertToObject(String value) {
+    return ItemReferenceHelper.service().getLoanTypeByName(value);


TBC - Looks, there is no flow to restore LoanType from csv representation to object. Why do we need this method?

khandramai · 2025-06-02T07:39:12Z

src/main/java/org/folio/bulkops/domain/deserializer/LoanTypeDeserializer.java

+public class LoanTypeDeserializer extends JsonDeserializer<LoanType> {
+    @Override
+    public LoanType deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
+        System.out.println("Deserializing LoanType from JSON");


Logging should be used here instead of sout (if applicable).

LoanTypeDeserializer is not needed, removed.

khandramai · 2025-06-02T07:40:13Z

src/main/java/org/folio/bulkops/domain/deserializer/LoanTypeDeserializer.java

+import com.fasterxml.jackson.databind.JsonDeserializer;
+import org.folio.bulkops.domain.bean.LoanType;
+
+public class LoanTypeDeserializer extends JsonDeserializer<LoanType> {


Renaming for code readability: JsonParser p -> JsonParser parser.

LoanTypeDeserializer is not needed, removed.

khandramai · 2025-06-02T07:41:24Z

src/main/java/org/folio/bulkops/processor/folio/ItemUpdateProcessor.java

@@ -26,6 +26,7 @@ public class ItemUpdateProcessor extends FolioAbstractUpdateProcessor<ExtendedIt
  private final FolioExecutionContext folioExecutionContext;
  private final ConsortiaService consortiaService;
  private final PermissionsValidator permissionsValidator;
+  private final ObjectMapper objectMapper;


Is it used?

khandramai · 2025-06-02T07:42:12Z

src/main/java/org/folio/bulkops/processor/folio/ItemUpdateProcessor.java

@@ -40,7 +41,7 @@ public void updateRecord(ExtendedItem extendedItem) {
    } else {
      permissionsValidator.checkIfBulkEditWritePermissionExists(folioExecutionContext.getTenantId(), EntityType.ITEM,
        NO_ITEM_WRITE_PERMISSIONS_TEMPLATE + folioExecutionContext.getTenantId());
-      itemClient.updateItem(item.withHoldingsData(null), item.getId());
+        itemClient.updateItem(item.withHoldingsData(null), item.getId());


Inline: roll-back format.

khandramai · 2025-06-02T07:43:05Z

src/test/java/org/folio/bulkops/processor/ItemDataProcessorTest.java

@@ -142,7 +142,7 @@ void testClearItemLocationAndLoanType() {
      .withHoldingsRecordId(holdingsId)
      .withPermanentLocation(new ItemLocation().withId(UUID.randomUUID().toString()).withName("Permanent location"))
      .withTemporaryLocation(new ItemLocation().withId(UUID.randomUUID().toString()).withName("Temporary location"))
-      .withPermanentLoanType(new LoanType().withId(UUID.randomUUID().toString()).withName("Permanent loan type"));
+            .withPermanentLoanType(new LoanType().withId(UUID.randomUUID().toString()).withName("Permanent loan type"));


Inline: roll-back format.

khandramai · 2025-06-02T07:43:21Z

src/test/java/org/folio/bulkops/processor/ItemDataProcessorTest.java

@@ -174,7 +174,7 @@ void testUpdateItemAndLoanTypeLocation() {
    var item = new Item()
      .withPermanentLocation(new ItemLocation().withId(UUID.randomUUID().toString()).withName("Permanent location"))
      .withTemporaryLocation(new ItemLocation().withId(UUID.randomUUID().toString()).withName("Temporary location"))
-      .withPermanentLoanType(new LoanType().withId(UUID.randomUUID().toString()).withName("Permanent loan type"))
+            .withPermanentLoanType(new LoanType().withId(UUID.randomUUID().toString()).withName("Permanent loan type"))


Inline: roll-back format.

sonarqubecloud · 2025-06-03T20:28:11Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

obozhko-folio added 8 commits May 28, 2025 11:45

MODBULKOPS-333 Test loan type

c337f6f

MODBULKOPS-333 Test loan type

735fc45

MODBULKOPS-333 Test loan type

362e2b3

MODBULKOPS-333 Test loan type

91b8eca

MODBULKOPS-333 Use deserializer

5ff6148

MODBULKOPS-333 Use getter/setter

a947c39

MODBULKOPS-333 Fixed tests

4e244d2

MODBULKOPS-333 Fixed missing reference data names

f597197

obozhko-folio self-assigned this May 30, 2025

obozhko-folio and others added 5 commits May 30, 2025 15:12

Merge branch 'master' into MODBULKOPS-333

b550414

MODBULKOPS-333 Fixed used tenants for ui

b0a5806

MODBULKOPS-333 Fixed tests

c46cddb

MODBULKOPS-333 Added tests

829240b

MODBULKOPS-333 Added tests

59169d8

obozhko-folio requested a review from a team May 30, 2025 20:07

khandramai reviewed Jun 2, 2025

View reviewed changes

obozhko-folio and others added 4 commits June 2, 2025 12:11

MODBULKOPS-333 Fixed according to comments

47031f7

Merge branch 'master' into MODBULKOPS-333

4e49e24

MODBULKOPS-333 Refactored

c6496ff

Merge remote-tracking branch 'origin/MODBULKOPS-333' into MODBULKOPS-333

4abbb6e

obozhko-folio added 6 commits June 3, 2025 15:16

MODBULKOPS-333 Logging issue

8eb5fc4

MODBULKOPS-333 Logging issue

02a5c25

MODBULKOPS-333 Added property order

ae9c8c5

MODBULKOPS-333 Removed property order, added constructor

2431e15

MODBULKOPS-333 Fixed minor

7ecb1f7

MODBULKOPS-333 Fixed error file name

9a61d39

MODBULKOPS-337 - Stage 3b - FQM Performance (FQL): switching matched records flow to use mod-bulk-operations capabilities #423

Are you sure you want to change the base?

MODBULKOPS-337 - Stage 3b - FQM Performance (FQL): switching matched records flow to use mod-bulk-operations capabilities #423

Uh oh!

Conversation

obozhko-folio commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Approach

TODOS and Open Questions

Learning

Pre-Merge Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 3, 2025

Quality Gate passed

Uh oh!

Uh oh!

obozhko-folio commented May 29, 2025 •

edited

Loading