From 1d9526e0449a3d7d84ebbb8c3fe28e59d4bc9afe Mon Sep 17 00:00:00 2001 From: Ruslan Lavrov Date: Wed, 1 Jul 2026 12:24:00 +0300 Subject: [PATCH 1/4] Fixed extraction and check for 999ff$i field existence in a record --- .../consumers/MarcBibUpdateKafkaHandler.java | 6 +++-- .../actions/CreateInstanceEventHandler.java | 9 ++++++-- .../dataimport/util/AdditionalFieldsUtil.java | 23 ++++++++++--------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/folio/inventory/dataimport/consumers/MarcBibUpdateKafkaHandler.java b/src/main/java/org/folio/inventory/dataimport/consumers/MarcBibUpdateKafkaHandler.java index c2c009074..87e331cd9 100644 --- a/src/main/java/org/folio/inventory/dataimport/consumers/MarcBibUpdateKafkaHandler.java +++ b/src/main/java/org/folio/inventory/dataimport/consumers/MarcBibUpdateKafkaHandler.java @@ -6,6 +6,7 @@ import static org.folio.inventory.dataimport.handlers.matching.util.EventHandlingUtil.OKAPI_REQUEST_ID; import static org.folio.inventory.dataimport.handlers.matching.util.EventHandlingUtil.OKAPI_USER_ID; import static org.folio.inventory.dataimport.handlers.matching.util.EventHandlingUtil.constructContext; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; import static org.folio.inventory.dataimport.util.MappingConstants.MARC_BIB_RECORD_TYPE; @@ -212,8 +213,9 @@ private KafkaProducer createProducer(String eventType, KafkaConf } private LinkUpdateReport mapToLinkReport(MarcBibUpdate marcBibUpdate, String errMessage) { - var instanceId = AdditionalFieldsUtil.getValue(marcBibUpdate.getRecord(), TAG_999, SUBFIELD_I) - .orElse(null); + var instanceId = + AdditionalFieldsUtil.getValueFromDataField(marcBibUpdate.getRecord(), TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I) + .orElse(null); return new LinkUpdateReport() .withJobId(marcBibUpdate.getJobId()) .withInstanceId(instanceId) diff --git a/src/main/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandler.java b/src/main/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandler.java index bfc784deb..80d873a81 100644 --- a/src/main/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandler.java +++ b/src/main/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandler.java @@ -10,6 +10,7 @@ import static org.folio.DataImportEventTypes.DI_INVENTORY_INSTANCE_CREATED_READY_FOR_POST_PROCESSING; import static org.folio.inventory.dataimport.handlers.matching.util.EventHandlingUtil.OKAPI_REQUEST_ID; import static org.folio.inventory.dataimport.handlers.matching.util.EventHandlingUtil.PAYLOAD_USER_ID; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.reorderMarcRecordFields; @@ -61,7 +62,7 @@ public class CreateInstanceEventHandler extends AbstractInstanceEventHandler { private static final String PAYLOAD_HAS_NO_DATA_MSG = "Failed to handle event payload - event payload context does not contain MARC_BIBLIOGRAPHIC data"; static final String ACTION_HAS_NO_MAPPING_MSG = "Action profile to create an Instance requires a mapping profile by jobExecutionId: '%s' and recordId: '%s'"; private static final String MAPPING_PARAMETERS_NOT_FOUND_MSG = "MappingParameters snapshot was not found by jobExecutionId: '%s'. RecordId: '%s', chunkId: '%s' "; - private static final String INSTANCE_CREATION_999_ERROR_MESSAGE = "A new Instance was not created because the incoming record already contained a 999ff$s or 999ff$i field"; + private static final String INSTANCE_CREATION_999_ERROR_MESSAGE = "A new Instance was not created because the incoming record already contains a 999ff$s or 999ff$i field"; private static final String RECORD_ID_HEADER = "recordId"; private static final String CHUNK_ID_HEADER = "chunkId"; public static final String DISCOVERY_SUPPRESS_PROPERTY = "discoverySuppress"; @@ -102,7 +103,7 @@ public CompletableFuture handle(DataImportEventPayload d Record targetRecord = Json.decodeValue(payloadContext.get(EntityType.MARC_BIBLIOGRAPHIC.value()), Record.class); var sourceContent = targetRecord.getParsedRecord().getContent().toString(); - if (!Boolean.parseBoolean(payloadContext.get("acceptInstanceId")) && AdditionalFieldsUtil.getValue(targetRecord, TAG_999, SUBFIELD_I).isPresent()) { + if (!Boolean.parseBoolean(payloadContext.get("acceptInstanceId")) && contains999ffSubfieldIValue(targetRecord)) { LOGGER.error("handle:: {} jobExecutionId: {} recordId: {} ", INSTANCE_CREATION_999_ERROR_MESSAGE, jobExecutionId, recordId); return CompletableFuture.failedFuture(new EventProcessingException(INSTANCE_CREATION_999_ERROR_MESSAGE)); } @@ -183,6 +184,10 @@ public CompletableFuture handle(DataImportEventPayload d return future; } + private boolean contains999ffSubfieldIValue(Record targetRecord) { + return AdditionalFieldsUtil.getValueFromDataField(targetRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I).isPresent(); + } + private JsonObject prepareInstance(DataImportEventPayload dataImportEventPayload, String instanceId, String jobExecutionId) { JsonObject instanceAsJson = new JsonObject(dataImportEventPayload.getContext().get(INSTANCE.value())); if (instanceAsJson.getJsonObject(INSTANCE_PATH) != null) { diff --git a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java index ee9dab257..0569720c8 100644 --- a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java +++ b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java @@ -68,7 +68,7 @@ public final class AdditionalFieldsUtil { public static final char TAG_035_SUB = 'a'; private static final char TAG_035_IND = ' '; private static final String ANY_STRING = "*"; - private static final char INDICATOR = 'f'; + public static final char INDICATOR_F = 'f'; public static final char SUBFIELD_I = 'i'; public static final char SUBFIELD_L = 'l'; private static final String HR_ID_FIELD = "hrid"; @@ -150,14 +150,14 @@ public static boolean addFieldToMarcRecord(Record recordForUpdate, String field, VariableField variableField = getSingleFieldByIndicators(marcRecord.getVariableFields(field)); DataField dataField; if (variableField != null - && ((DataField) variableField).getIndicator1() == INDICATOR - && ((DataField) variableField).getIndicator2() == INDICATOR + && ((DataField) variableField).getIndicator1() == INDICATOR_F + && ((DataField) variableField).getIndicator2() == INDICATOR_F ) { dataField = (DataField) variableField; marcRecord.removeVariableField(variableField); dataField.removeSubfield(dataField.getSubfield(subfield)); } else { - dataField = factory.newDataField(field, INDICATOR, INDICATOR); + dataField = factory.newDataField(field, INDICATOR_F, INDICATOR_F); } dataField.addSubfield(factory.newSubfield(subfield, value)); marcRecord.addVariableField(dataField); @@ -338,12 +338,13 @@ public static String getValueFromControlledField(Record srcRecord, String tag) { return null; } - public static Optional getValue(Record srcRecord, String tag, char subfield) { - return Optional.ofNullable(computeMarcRecord(srcRecord)) - .stream() - .flatMap(marcRecord -> marcRecord.getVariableFields(tag).stream()) - .flatMap(field -> getFieldValue(field, subfield).stream()) - .findFirst(); + public static Optional getValueFromDataField(Record srcRecord, String tag, char ind1, char ind2, char subfield) { + return Optional.ofNullable(computeMarcRecord(srcRecord)) + .stream() + .flatMap(marcRecord -> marcRecord.getVariableFields(tag).stream()) + .filter(f -> f instanceof DataField df && df.getIndicator1() == ind1 && df.getIndicator2() == ind2) + .flatMap(field -> getFieldValue(field, subfield).stream()) + .findFirst(); } private static Optional getFieldValue(VariableField field, char subfield) { @@ -366,7 +367,7 @@ private static VariableField getSingleFieldByIndicators(List list return null; } return list.stream() - .filter(f -> ((DataField) f).getIndicator1() == INDICATOR && ((DataField) f).getIndicator2() == INDICATOR) + .filter(f -> ((DataField) f).getIndicator1() == INDICATOR_F && ((DataField) f).getIndicator2() == INDICATOR_F) .findFirst() .orElse(null); } From 46ba6312125e907fe5de802618a261b46e6e7ca1 Mon Sep 17 00:00:00 2001 From: Ruslan Lavrov Date: Wed, 1 Jul 2026 12:29:17 +0300 Subject: [PATCH 2/4] Added tests --- .../MarcBibUpdateKafkaHandlerTest.java | 3 +- .../CreateInstanceEventHandlerTest.java | 7 ++ .../util/AdditionalFieldsUtilTest.java | 96 ++++++++++++++++--- ...teInstanceIngressEventHandlerUnitTest.java | 7 +- ...teInstanceIngressEventHandlerUnitTest.java | 9 +- 5 files changed, 103 insertions(+), 19 deletions(-) diff --git a/src/test/java/org/folio/inventory/dataimport/handlers/MarcBibUpdateKafkaHandlerTest.java b/src/test/java/org/folio/inventory/dataimport/handlers/MarcBibUpdateKafkaHandlerTest.java index f1b059153..b7ed314db 100644 --- a/src/test/java/org/folio/inventory/dataimport/handlers/MarcBibUpdateKafkaHandlerTest.java +++ b/src/test/java/org/folio/inventory/dataimport/handlers/MarcBibUpdateKafkaHandlerTest.java @@ -2,6 +2,7 @@ import static org.folio.inventory.EntityLinksKafkaTopic.LINKS_STATS; import static org.folio.inventory.KafkaUtility.checkKafkaEventSent; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; import static org.folio.inventory.dataimport.util.MappingConstants.MARC_BIB_RECORD_TYPE; @@ -258,7 +259,7 @@ public void shouldNotSendSuccessLinkReportEvent(TestContext context) { public void shouldSendFailedLinkReportEvent(TestContext context) { // given var async = context.async(); - var instanceId = AdditionalFieldsUtil.getValue(record, TAG_999, SUBFIELD_I) + var instanceId = AdditionalFieldsUtil.getValueFromDataField(record, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I) .orElse(null); record.setId(INVALID_INSTANCE_ID); diff --git a/src/test/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandlerTest.java b/src/test/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandlerTest.java index 82c630891..19e95856e 100644 --- a/src/test/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandlerTest.java +++ b/src/test/java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandlerTest.java @@ -128,6 +128,7 @@ public class CreateInstanceEventHandlerTest { private static final String PARSED_CONTENT_999ffi = "{\"leader\":\"01314nam 22003851a 4500\",\"fields\":[{\"001\":\"ybp7406411\"},{\"245\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"a\":\"titleValue\"}]}},{\"336\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"b\":\"b6698d38-149f-11ec-82a8-0242ac130003\"}]}},{\"780\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"Houston oil directory\"}]}},{\"785\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"SAIS review of international affairs\"},{\"x\":\"1945-4724\"}]}},{\"500\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Adaptation of Xi xiang ji by Wang Shifu.\"}]}},{\"520\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\"}]}}]}"; private static final String PARSED_CONTENT_WITH_005 = "{\"leader\":\"01314nam 22003851a 4500\",\"fields\":[{\"001\":\"ybp7406411\"},{\"005\":\"20141107001016.0\"},{\"245\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"a\":\"titleValue\"}]}},{\"336\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"b\":\"b6698d38-149f-11ec-82a8-0242ac130003\"}]}},{\"780\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"Houston oil directory\"}]}},{\"785\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"SAIS review of international affairs\"},{\"x\":\"1945-4724\"}]}},{\"500\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Adaptation of Xi xiang ji by Wang Shifu.\"}]}},{\"520\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\"}]}}]}"; private static final String PARSED_CONTENT_WITH_999fi = "{\"leader\":\"01314nam 22003851a 4500\",\"fields\":[{\"001\":\"ybp7406411\"},{\"003\":\"in001\"},{\"245\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"a\":\"titleValue\"}]}},{\"336\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"b\":\"b6698d38-149f-11ec-82a8-0242ac130003\"}]}},{\"780\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"Houston oil directory\"}]}},{\"785\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"SAIS review of international affairs\"},{\"x\":\"1945-4724\"}]}},{\"500\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Adaptation of Xi xiang ji by Wang Shifu.\"}]}},{\"520\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\"}]}} , {\"999\": {\"ind1\":\"f\", \"ind2\":\"f\", \"subfields\":[ { \"i\": \"957985c6-97e3-4038-b0e7-343ecd0b8120\"} ] } }]}"; + private static final String PARSED_CONTENT_WITH_999_NON_FF_I = "{\"leader\":\"01314nam 22003851a 4500\",\"fields\":[{\"001\":\"ybp7406411\"},{\"003\":\"in001\"},{\"245\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"a\":\"titleValue\"}]}},{\"336\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"b\":\"b6698d38-149f-11ec-82a8-0242ac130003\"}]}},{\"780\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"Houston oil directory\"}]}},{\"785\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"SAIS review of international affairs\"},{\"x\":\"1945-4724\"}]}},{\"500\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Adaptation of Xi xiang ji by Wang Shifu.\"}]}},{\"520\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\"}]}} , {\"999\": {\"ind1\":\" \", \"ind2\":\" \", \"subfields\":[ { \"i\": \"957985c6-97e3-4038-b0e7-343ecd0b8120\"} ] } }]}"; private static final String PARSED_CONTENT_WITH_DELETED_05 = "{\"leader\":\"01314dam 22003851a 4500\",\"fields\":[{\"001\":\"ybp7406411\"},{\"003\":\"in001\"},{\"245\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"a\":\"titleValue\"}]}},{\"336\":{\"ind1\":\"1\",\"ind2\":\"0\",\"subfields\":[{\"b\":\"b6698d38-149f-11ec-82a8-0242ac130003\"}]}},{\"780\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"Houston oil directory\"}]}},{\"785\":{\"ind1\":\"0\",\"ind2\":\"0\",\"subfields\":[{\"t\":\"SAIS review of international affairs\"},{\"x\":\"1945-4724\"}]}},{\"500\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Adaptation of Xi xiang ji by Wang Shifu.\"}]}},{\"520\":{\"ind1\":\" \",\"ind2\":\" \",\"subfields\":[{\"a\":\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\"}]}}]}"; private static final String MAPPING_RULES_PATH = "src/test/resources/handlers/bib-rules.json"; private static final String MAPPING_METADATA_URL = "/mapping-metadata"; @@ -378,6 +379,12 @@ public void shouldProcessEventWith999AndAcceptedInstanceId() throws InterruptedE shouldProcessEvent(PARSED_CONTENT_WITH_999fi, Boolean.TRUE.toString()); } + @Test + public void shouldProcessEventAndCreateInstanceIfRecordContains999WithSubfieldIWithNonFfIndicators() + throws InterruptedException, ExecutionException, TimeoutException { + shouldProcessEvent(PARSED_CONTENT_WITH_999_NON_FF_I, Boolean.FALSE.toString()); + } + public void shouldProcessEvent(String content, String acceptInstanceId) throws InterruptedException, ExecutionException, TimeoutException { Reader fakeReader = Mockito.mock(Reader.class); diff --git a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java index a2182ce55..12a137365 100644 --- a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java +++ b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java @@ -1,21 +1,24 @@ package org.folio.inventory.dataimport.util; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_001; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_005; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.addControlledFieldToMarcRecord; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.addDataFieldToMarcRecord; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.addFieldToMarcRecord; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.dateTime005Formatter; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.getCacheStats; -import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.getValue; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.getValueFromControlledField; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.getValueFromDataField; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.isFieldExist; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.removeField; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.github.benmanes.caffeine.cache.stats.CacheStats; @@ -656,14 +659,13 @@ public void shouldUpdate005Field() { } @Test - public void shouldReturnFields() { + public void shouldReturnValueFromDataField() { // given var id = UUID.randomUUID().toString(); - var hrId = "in001"; var parsedContent = """ { "fields": [ - {"001": "%s"}, + {"001": "in001"}, {"999": { "ind1": "f", "ind2": "f", @@ -673,23 +675,93 @@ public void shouldReturnFields() { }} ] } - """.formatted(hrId, id); + """.formatted(id); var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); // when - var parsedHrId = getValue(marcRecord, TAG_001, ' '); - var parsedId = getValue(marcRecord, TAG_999, 'i'); - var missingField = getValue(marcRecord, TAG_005, ' '); + var parsedId = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); // then - assertTrue(parsedHrId.isPresent()); assertTrue(parsedId.isPresent()); - assertTrue(missingField.isEmpty()); - - assertEquals(hrId, parsedHrId.get()); assertEquals(id, parsedId.get()); } + @Test + public void shouldReturnEmptyOptionalFromDataFieldWhenIndicatorsDoNotMatch() { + // given + var parsedContent = """ + { + "fields": [ + {"999": { + "ind1": " ", + "ind2": " ", + "subfields": [ + {"i": "test"} + ] + }} + ] + } + """; + var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); + + // when + var result = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); + + // then + assertTrue(result.isEmpty()); + } + + @Test + public void shouldReturnEmptyOptionalFromDataFieldWhenSubfieldIsAbsent() { + // given + var parsedContent = """ + { + "fields": [ + {"999": { + "ind1": "f", + "ind2": "f", + "subfields": [ + {"s": "test"} + ] + }} + ] + } + """; + var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); + + // when + var result = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); + + // then + assertTrue(result.isEmpty()); + } + + @Test + public void shouldReturnEmptyOptionalFromDataFieldWhenTagIsAbsent() { + // given + var parsedContent = """ + { + "fields": [ + {"001": "ybp7406411"}, + {"245": { + "ind1": "1", + "ind2": "0", + "subfields": [ + {"a": "titleValue"} + ] + }} + ] + } + """; + var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); + + // when + var result = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); + + // then + assertTrue(result.isEmpty()); + } + @Test public void shouldExistControlField() throws IOException { String parsedRecordContent = TestUtil.readFileFromPath(PARSED_MARC_RECORD_PATH); diff --git a/src/test/java/org/folio/inventory/instanceingress/handler/CreateInstanceIngressEventHandlerUnitTest.java b/src/test/java/org/folio/inventory/instanceingress/handler/CreateInstanceIngressEventHandlerUnitTest.java index 5082c61de..51cf51130 100644 --- a/src/test/java/org/folio/inventory/instanceingress/handler/CreateInstanceIngressEventHandlerUnitTest.java +++ b/src/test/java/org/folio/inventory/instanceingress/handler/CreateInstanceIngressEventHandlerUnitTest.java @@ -6,6 +6,7 @@ import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; import static org.folio.inventory.TestUtil.buildHttpResponseWithBuffer; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_L; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; @@ -389,8 +390,10 @@ public void shouldReturnSucceededFuture_ifProcessFinishedCorrectly() throws IOEx var recordSentToSRS = recordCaptor.getValue(); assertThat(recordSentToSRS.getId()).isEqualTo(event.getEventPayload().getSourceRecordIdentifier()); assertThat(recordSentToSRS.getRecordType()).isEqualTo(Record.RecordType.MARC_BIB); - assertThat(AdditionalFieldsUtil.getValue(recordSentToSRS, TAG_999, SUBFIELD_I)).hasValue(instance.getId()); - assertThat(AdditionalFieldsUtil.getValue(recordSentToSRS, TAG_999, SUBFIELD_L)).hasValue(linkedDataId); + assertThat(AdditionalFieldsUtil.getValueFromDataField(recordSentToSRS, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I)) + .hasValue(instance.getId()); + assertThat(AdditionalFieldsUtil.getValueFromDataField(recordSentToSRS, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_L)) + .hasValue(linkedDataId); } private String metadataCacheKey() { diff --git a/src/test/java/org/folio/inventory/instanceingress/handler/UpdateInstanceIngressEventHandlerUnitTest.java b/src/test/java/org/folio/inventory/instanceingress/handler/UpdateInstanceIngressEventHandlerUnitTest.java index 7551f3e38..609a2a793 100644 --- a/src/test/java/org/folio/inventory/instanceingress/handler/UpdateInstanceIngressEventHandlerUnitTest.java +++ b/src/test/java/org/folio/inventory/instanceingress/handler/UpdateInstanceIngressEventHandlerUnitTest.java @@ -6,9 +6,11 @@ import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; import static org.folio.inventory.TestUtil.buildHttpResponseWithBuffer; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INDICATOR_F; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_L; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_999; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.getValueFromDataField; import static org.folio.inventory.dataimport.util.MappingConstants.MARC_BIB_RECORD_TYPE; import static org.folio.rest.jaxrs.model.InstanceIngressPayload.SourceType.LINKED_DATA; import static org.junit.Assert.assertEquals; @@ -39,7 +41,6 @@ import org.folio.inventory.dataimport.cache.MappingMetadataCache; import org.folio.inventory.dataimport.handlers.actions.PrecedingSucceedingTitlesHelper; import org.folio.inventory.dataimport.services.SnapshotService; -import org.folio.inventory.dataimport.util.AdditionalFieldsUtil; import org.folio.inventory.domain.instances.Instance; import org.folio.inventory.domain.instances.InstanceCollection; import org.folio.inventory.instanceingress.InstanceIngressEventConsumer; @@ -574,9 +575,9 @@ public void shouldReturnSucceededFuture_ifProcessFinishedCorrectly() throws IOEx assertThat(recordSentToSRS.getId()).isNotEqualTo(initialSrsId); assertThat(recordSentToSRS.getMatchedId()).isEqualTo(initialSrsId); assertThat(recordSentToSRS.getRecordType()).isEqualTo(Record.RecordType.MARC_BIB); - assertThat(AdditionalFieldsUtil.getValue(recordSentToSRS, TAG_999, SUBFIELD_I)).hasValue(instance.getId()); - assertThat(AdditionalFieldsUtil.getValue(recordSentToSRS, TAG_999, SUBFIELD_L)).hasValue(linkedDataId); - assertThat(AdditionalFieldsUtil.getValue(recordSentToSRS, TAG_999, 's')).hasValue(initialSrsId); + assertThat(getValueFromDataField(recordSentToSRS, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I)).hasValue(instance.getId()); + assertThat(getValueFromDataField(recordSentToSRS, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_L)).hasValue(linkedDataId); + assertThat(getValueFromDataField(recordSentToSRS, TAG_999, INDICATOR_F, INDICATOR_F, 's')).hasValue(initialSrsId); } private String metadataCacheKey() { From a1cbbef438d16b01d0448b659d09687190bc8d2f Mon Sep 17 00:00:00 2001 From: Ruslan Lavrov Date: Wed, 1 Jul 2026 14:54:27 +0300 Subject: [PATCH 3/4] Refactoring --- .../dataimport/util/AdditionalFieldsUtil.java | 28 +++++++++---------- .../util/AdditionalFieldsUtilTest.java | 14 +++++++++- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java index 0569720c8..aa46482d0 100644 --- a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java +++ b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java @@ -51,6 +51,7 @@ import org.marc4j.marc.MarcFactory; import org.marc4j.marc.Subfield; import org.marc4j.marc.VariableField; +import org.marc4j.marc.impl.Verifier; /** * Util to work with additional fields @@ -77,6 +78,7 @@ public final class AdditionalFieldsUtil { private static final String OCLC_PREFIX = "(OCoLC)"; private static final ObjectMapper objectMapper = new ObjectMapper(); public static final String FIELDS = "fields"; + static final String INVALID_DATA_FIELD_MSG = "Field '%s' is not a data field."; static { // this function is executed when creating a new item to be saved in the cache. @@ -339,22 +341,18 @@ public static String getValueFromControlledField(Record srcRecord, String tag) { } public static Optional getValueFromDataField(Record srcRecord, String tag, char ind1, char ind2, char subfield) { + if (Verifier.isControlField(tag)) { + String msg = INVALID_DATA_FIELD_MSG.formatted(tag); + LOGGER.warn("getValueFromDataField:: {}", msg); + throw new IllegalArgumentException(msg); + } + return Optional.ofNullable(computeMarcRecord(srcRecord)) .stream() - .flatMap(marcRecord -> marcRecord.getVariableFields(tag).stream()) - .filter(f -> f instanceof DataField df && df.getIndicator1() == ind1 && df.getIndicator2() == ind2) - .flatMap(field -> getFieldValue(field, subfield).stream()) - .findFirst(); - } - - private static Optional getFieldValue(VariableField field, char subfield) { - if (field instanceof DataField dataField) { - return dataField.getSubfields(subfield).stream().findFirst().map(Subfield::getData); - } else if (field instanceof ControlField controlField) { - return Optional.ofNullable(controlField.getData()); - } else { - return Optional.empty(); - } + .flatMap(marcRecord -> marcRecord.getDataFields().stream()) + .filter(df -> df.getTag().equals(tag) && df.getIndicator1() == ind1 && df.getIndicator2() == ind2) + .findFirst() + .flatMap(df -> df.getSubfields(subfield).stream().findFirst().map(Subfield::getData)); } private static MarcReader buildMarcReader(Record srcRecord) { @@ -388,7 +386,7 @@ private static boolean isField005NeedToUpdate(Record srcRecord, MappingParameter org.marc4j.marc.Record marcRecord = reader.next(); List variableFields = marcRecord.getVariableFields(TAG_005); if(!variableFields.isEmpty()) { - VariableField field = variableFields.get(0); + VariableField field = variableFields.getFirst(); needToUpdate = isNotProtected(fieldProtectionSettings, (ControlField) field); } } diff --git a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java index 12a137365..b5051e999 100644 --- a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java +++ b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java @@ -1,5 +1,6 @@ package org.folio.inventory.dataimport.util; +import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.INVALID_DATA_FIELD_MSG; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.SUBFIELD_I; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_001; import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.TAG_005; @@ -18,7 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import com.github.benmanes.caffeine.cache.stats.CacheStats; @@ -762,6 +763,17 @@ public void shouldReturnEmptyOptionalFromDataFieldWhenTagIsAbsent() { assertTrue(result.isEmpty()); } + @Test + public void shouldThrowIllegalArgumentExceptionOnGetValueFromDataFieldCallWithNonDataFieldTag() { + // given + var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent("{}")); + + // when / then + var exception = assertThrows(IllegalArgumentException.class, + () -> getValueFromDataField(marcRecord, TAG_001, INDICATOR_F, INDICATOR_F, SUBFIELD_I)); + assertEquals(INVALID_DATA_FIELD_MSG.formatted(TAG_001), exception.getMessage()); + } + @Test public void shouldExistControlField() throws IOException { String parsedRecordContent = TestUtil.readFileFromPath(PARSED_MARC_RECORD_PATH); From fe24f21cb558e21558430f4646f0e7977533eb3e Mon Sep 17 00:00:00 2001 From: Ruslan Lavrov Date: Fri, 3 Jul 2026 02:46:23 +0300 Subject: [PATCH 4/4] news --- NEWS.md | 1 + .../dataimport/util/AdditionalFieldsUtil.java | 20 ++++++++++++++-- .../util/AdditionalFieldsUtilTest.java | 24 ++++++------------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1c17693ef..d58e3cf14 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ ## 22.1.0-SNAPSHOT 2026-mm-dd * Data Import profiles will map invalid statistical codes [MODINV-1391](https://folio-org.atlassian.net/browse/MODINV-1391) +* Importing Records from BNE z39.50 - record not available error [MODINV-1395](https://folio-org.atlassian.net/browse/MODINV-1395) ## 22.0.0 2026-04-16 * ECS: Improve logs in move holdings operation for [MODINV-1276](https://folio-org.atlassian.net/browse/MODINV-1276) diff --git a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java index aa46482d0..8ba4c5c03 100644 --- a/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java +++ b/src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java @@ -340,6 +340,22 @@ public static String getValueFromControlledField(Record srcRecord, String tag) { return null; } + /** + * Reads the value of a subfield from the first matching data field in a + * MARC record, identified by {@code tag} and indicator values. + * + * @param srcRecord record containing the parsed MARC content to retrieve data + * @param tag three-character MARC tag of the data field (must not be + * a control field tag, i.e. must not start with "00") + * @param ind1 first indicator + * @param ind2 second indicator + * @param subfield subfield code whose data value should be returned + * @return {@link Optional} containing the data of the first matching + * subfield, or an empty {@link Optional} if no matching field or + * subfield is found + * @throws IllegalArgumentException if {@code tag} identifies a control + * instead of a data field + */ public static Optional getValueFromDataField(Record srcRecord, String tag, char ind1, char ind2, char subfield) { if (Verifier.isControlField(tag)) { String msg = INVALID_DATA_FIELD_MSG.formatted(tag); @@ -518,8 +534,8 @@ private static boolean removeFirstFoundFieldByName(org.marc4j.marc.Record marcRe */ private static boolean isFieldContainsValue(VariableField field, char subfield, String value) { boolean isContains = false; - if (field instanceof DataField) { - for (Subfield sub : ((DataField) field).getSubfields(subfield)) { + if (field instanceof DataField dataField) { + for (Subfield sub : dataField.getSubfields(subfield)) { if (isNotEmpty(sub.getData()) && sub.getData().contains(value.trim())) { isContains = true; break; diff --git a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java index b5051e999..6db817066 100644 --- a/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java +++ b/src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java @@ -689,7 +689,6 @@ public void shouldReturnValueFromDataField() { @Test public void shouldReturnEmptyOptionalFromDataFieldWhenIndicatorsDoNotMatch() { - // given var parsedContent = """ { "fields": [ @@ -703,18 +702,11 @@ public void shouldReturnEmptyOptionalFromDataFieldWhenIndicatorsDoNotMatch() { ] } """; - var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); - - // when - var result = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); - - // then - assertTrue(result.isEmpty()); + shouldReturnEmptyOptional(parsedContent); } @Test public void shouldReturnEmptyOptionalFromDataFieldWhenSubfieldIsAbsent() { - // given var parsedContent = """ { "fields": [ @@ -728,18 +720,11 @@ public void shouldReturnEmptyOptionalFromDataFieldWhenSubfieldIsAbsent() { ] } """; - var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); - - // when - var result = getValueFromDataField(marcRecord, TAG_999, INDICATOR_F, INDICATOR_F, SUBFIELD_I); - - // then - assertTrue(result.isEmpty()); + shouldReturnEmptyOptional(parsedContent); } @Test public void shouldReturnEmptyOptionalFromDataFieldWhenTagIsAbsent() { - // given var parsedContent = """ { "fields": [ @@ -754,6 +739,11 @@ public void shouldReturnEmptyOptionalFromDataFieldWhenTagIsAbsent() { ] } """; + shouldReturnEmptyOptional(parsedContent); + } + + private void shouldReturnEmptyOptional(String parsedContent) { + // given var marcRecord = new Record().withParsedRecord(new ParsedRecord().withContent(parsedContent)); // when