Skip to content

Commit 6bda3b8

Browse files
viacheslavkolviacheslavkol
authored
feat(specification-sync): prefer non-obsolete entities no matter the order (#152)
Closes: MRSPECS-95
1 parent b848f67 commit 6bda3b8

3 files changed

Lines changed: 145 additions & 52 deletions

File tree

‎NEWS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Refactor specification sync from URL to local copy ([MRSPECS-87](https://folio-org.atlassian.net/browse/MRSPECS-87))
1111

1212
### Bug fixes
13-
* Description ([ISSUE](https://folio-org.atlassian.net/browse/ISSUE))
13+
* Fix non-obsolete subfield choosing logic ([MRSPECS-95](https://folio-org.atlassian.net/browse/MRSPECS-95))
1414

1515
### Tech Dept
1616
* Description ([ISSUE](https://folio-org.atlassian.net/browse/ISSUE))

‎mod-record-specifications-server/src/main/java/org/folio/rspec/service/sync/SpecificationSyncService.java‎

Lines changed: 38 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@
1717
import com.fasterxml.jackson.databind.JsonNode;
1818
import com.fasterxml.jackson.databind.node.ArrayNode;
1919
import java.util.ArrayList;
20-
import java.util.Collection;
2120
import java.util.Collections;
2221
import java.util.HashMap;
2322
import java.util.HashSet;
2423
import java.util.List;
2524
import java.util.Map;
2625
import java.util.Set;
2726
import java.util.UUID;
27+
import java.util.function.BinaryOperator;
28+
import java.util.function.Predicate;
2829
import lombok.RequiredArgsConstructor;
2930
import org.folio.rspec.domain.dto.Scope;
3031
import org.folio.rspec.domain.entity.Field;
@@ -54,7 +55,7 @@ public void sync(Specification specification) {
5455
var specificationMetadata = metadataService.getSpecificationMetadata(specification.getId());
5556
var fields = evaluatorFetcher(specification, specificationMetadata);
5657
metadataService.saveSpecificationMetadata(specificationMetadata);
57-
specificationFieldService.syncFields(specification, cleanupFields(specification, fields));
58+
specificationFieldService.syncFields(specification, fields);
5859
}
5960

6061
private List<Field> evaluatorFetcher(Specification specification, SpecificationMetadata specificationMetadata) {
@@ -69,19 +70,22 @@ private List<Field> evaluatorFetcher(Specification specification, SpecificationM
6970
private List<Field> syncFields(Specification specification, ArrayNode specificationFields,
7071
Map<String, FieldMetadata> fieldsMetadata,
7172
SpecificationMetadata specificationMetadata) {
72-
List<Field> fields = new ArrayList<>();
73+
var fieldMap = new HashMap<String, Field>();
74+
7375
for (var fieldElement : specificationFields) {
7476
var field = populateField(fieldElement, fieldsMetadata, specificationMetadata, specification);
75-
fields.add(field);
77+
fieldMap.merge(field.getTag(), field, preferNonDeprecated(Field::isDeprecated));
7678
}
7779

78-
for (FieldMetadata value : fieldsMetadata.values()) {
80+
for (var value : fieldsMetadata.values()) {
7981
if (Boolean.TRUE.equals(value.defaultValue())) {
80-
fields.add(toField(value));
82+
var field = toField(value);
83+
field.setSpecification(specification);
84+
fieldMap.merge(field.getTag(), field, preferNonDeprecated(Field::isDeprecated));
8185
}
8286
}
8387

84-
return fields;
88+
return new ArrayList<>(fieldMap.values());
8589
}
8690

8791
private Field populateField(JsonNode fieldElement, Map<String, FieldMetadata> fieldsMetadata,
@@ -113,32 +117,6 @@ private Field populateField(JsonNode fieldElement, Map<String, FieldMetadata> fi
113117
}
114118
}
115119

116-
/**
117-
* Merge fields, indicator codes, subfields if there is a duplicate by skipping deprecated records.
118-
*/
119-
private Collection<Field> cleanupFields(Specification specification, List<Field> fields) {
120-
Map<String, Field> fieldByTags = new HashMap<>();
121-
for (Field field : fields) {
122-
fieldByTags.merge(field.getTag(), field, (field1, field2) -> field1.isDeprecated() ? field2 : field1);
123-
field.setSpecification(specification);
124-
Map<String, Subfield> subfields = new HashMap<>();
125-
for (Subfield subfield : field.getSubfields()) {
126-
subfields.merge(subfield.getCode(), subfield,
127-
(subfield1, subfield2) -> subfield1.isDeprecated() ? subfield2 : subfield1);
128-
}
129-
field.setSubfields(new HashSet<>(subfields.values()));
130-
for (Indicator indicator : field.getIndicators()) {
131-
Map<String, IndicatorCode> indicatorCodes = new HashMap<>();
132-
for (IndicatorCode indicatorCode : indicator.getCodes()) {
133-
indicatorCodes.merge(indicatorCode.getCode(), indicatorCode,
134-
(indicatorCode1, indicatorCode2) -> indicatorCode1.isDeprecated() ? indicatorCode2 : indicatorCode1);
135-
}
136-
indicator.setCodes(new ArrayList<>(indicatorCodes.values()));
137-
}
138-
}
139-
return fieldByTags.values();
140-
}
141-
142120
private Field toField(FieldMetadata fieldMetadata) {
143121
var defaultField = new Field();
144122
defaultField.setId(UUID.fromString(fieldMetadata.id()));
@@ -150,9 +128,9 @@ private Field toField(FieldMetadata fieldMetadata) {
150128
defaultField.setRepeatable(fieldMetadata.repeatable());
151129
defaultField.setRequired(fieldMetadata.required());
152130

153-
Set<Subfield> subfields = new HashSet<>();
131+
var subfields = new HashSet<Subfield>();
154132
if (fieldMetadata.subfields() != null) {
155-
for (SubfieldMetadata subfieldMetadata : fieldMetadata.subfields().values()) {
133+
for (var subfieldMetadata : fieldMetadata.subfields().values()) {
156134
if (Boolean.TRUE.equals(subfieldMetadata.defaultValue())) {
157135
subfields.add(toSubfield(subfieldMetadata));
158136
}
@@ -161,7 +139,7 @@ private Field toField(FieldMetadata fieldMetadata) {
161139
}
162140

163141
if (fieldMetadata.indicators() != null) {
164-
List<Indicator> indicators = new ArrayList<>();
142+
var indicators = new ArrayList<Indicator>();
165143
populateDefaultIndicators(fieldMetadata, indicators);
166144
defaultField.setIndicators(indicators);
167145
}
@@ -172,35 +150,39 @@ private Set<Subfield> prepareSubfields(JsonNode jsonNode, FieldMetadata fieldMet
172150
if (jsonNode == null || jsonNode.isEmpty()) {
173151
return Collections.emptySet();
174152
}
175-
Set<Subfield> subfields = new HashSet<>();
176153

154+
var subfieldMap = new HashMap<String, Subfield>();
177155
var subfieldsMetadata = fieldMetadata.subfields() == null
178156
? new HashMap<String, SubfieldMetadata>()
179157
: fieldMetadata.subfields();
180-
for (JsonNode subfieldElement : jsonNode) {
158+
for (var subfieldElement : jsonNode) {
181159
var code = getText(subfieldElement, CODE_PROP);
182160
var subfieldMetadata = subfieldsMetadata.computeIfAbsent(code,
183161
definedCode -> new SubfieldMetadata(code, Scope.STANDARD.name()));
184-
subfields.add(toSubfield(subfieldElement, subfieldMetadata));
162+
var subfield = toSubfield(subfieldElement, subfieldMetadata);
163+
164+
subfieldMap.merge(subfield.getCode(), subfield, preferNonDeprecated(Subfield::isDeprecated));
185165
}
186166

187-
for (SubfieldMetadata subfieldMetadata : subfieldsMetadata.values()) {
167+
for (var subfieldMetadata : subfieldsMetadata.values()) {
188168
if (Boolean.TRUE.equals(subfieldMetadata.defaultValue())) {
189-
subfields.add(toSubfield(subfieldMetadata));
169+
var subfield = toSubfield(subfieldMetadata);
170+
subfieldMap.merge(subfield.getCode(), subfield, preferNonDeprecated(Subfield::isDeprecated));
190171
}
191172
}
192-
return subfields;
173+
174+
return new HashSet<>(subfieldMap.values());
193175
}
194176

195177
private List<Indicator> prepareIndicators(JsonNode jsonNode, FieldMetadata fieldMetadata) {
196178
if (jsonNode == null || jsonNode.isEmpty() || !jsonNode.isArray()) {
197179
return Collections.emptyList();
198180
}
199-
List<Indicator> indicators = new ArrayList<>();
181+
var indicators = new ArrayList<Indicator>();
200182
var indicatorsMetadata = fieldMetadata.indicators() == null
201183
? new HashMap<String, IndicatorMetadata>()
202184
: fieldMetadata.indicators();
203-
for (JsonNode indicatorElement : jsonNode) {
185+
for (var indicatorElement : jsonNode) {
204186
var order = String.valueOf(getInt(indicatorElement, ORDER_PROP));
205187
var indicatorMetadata = indicatorsMetadata.computeIfAbsent(order, IndicatorMetadata::new);
206188
var indicator = new Indicator();
@@ -216,14 +198,14 @@ private List<Indicator> prepareIndicators(JsonNode jsonNode, FieldMetadata field
216198
}
217199

218200
private void populateDefaultIndicators(FieldMetadata fieldMetadata, List<Indicator> indicators) {
219-
for (IndicatorMetadata indicatorMetadata : fieldMetadata.indicators().values()) {
201+
for (var indicatorMetadata : fieldMetadata.indicators().values()) {
220202
if (Boolean.TRUE.equals(indicatorMetadata.defaultValue())) {
221203
var indicator = new Indicator();
222204
indicator.setId(UUID.fromString(indicatorMetadata.id()));
223205
indicator.setOrder(indicatorMetadata.order());
224206
indicator.setLabel(indicatorMetadata.label());
225-
List<IndicatorCode> indicatorCodes = new ArrayList<>();
226-
for (IndicatorCodeMetadata indicatorCodeMetadata : indicatorMetadata.codes().values()) {
207+
var indicatorCodes = new ArrayList<IndicatorCode>();
208+
for (var indicatorCodeMetadata : indicatorMetadata.codes().values()) {
227209
if (Boolean.TRUE.equals(indicatorCodeMetadata.defaultValue())) {
228210
var indicatorCode = new IndicatorCode();
229211
indicatorCode.setId(UUID.fromString(indicatorCodeMetadata.id()));
@@ -245,9 +227,9 @@ private List<IndicatorCode> toIndicatorCodes(JsonNode indicatorElement, Indicato
245227
if (codesElement == null || codesElement.isEmpty() || !codesElement.isArray()) {
246228
return Collections.emptyList();
247229
}
248-
List<IndicatorCode> codes = new ArrayList<>();
249230

250-
for (JsonNode codeElement : codesElement) {
231+
var indicatorCodeMap = new HashMap<String, IndicatorCode>();
232+
for (var codeElement : codesElement) {
251233
var indicatorCodeMetadata = indicatorMetadata.codes().computeIfAbsent(getText(codeElement, CODE_PROP),
252234
code -> new IndicatorCodeMetadata(code, Scope.STANDARD.name()));
253235
var indicatorCode = new IndicatorCode();
@@ -256,10 +238,11 @@ private List<IndicatorCode> toIndicatorCodes(JsonNode indicatorElement, Indicato
256238
indicatorCode.setLabel(getText(codeElement, LABEL_PROP));
257239
indicatorCode.setDeprecated(getBoolean(codeElement, DEPRECATED_PROP));
258240
indicatorCode.setScope(Scope.valueOf(indicatorCodeMetadata.scope()));
259-
codes.add(indicatorCode);
241+
242+
indicatorCodeMap.merge(indicatorCode.getCode(), indicatorCode, preferNonDeprecated(IndicatorCode::isDeprecated));
260243
}
261244

262-
return codes;
245+
return new ArrayList<>(indicatorCodeMap.values());
263246
}
264247

265248
private Subfield toSubfield(SubfieldMetadata subfieldMetadata) {
@@ -293,4 +276,8 @@ private boolean isRequired(JsonNode fieldElement, SubfieldMetadata subfieldMetad
293276
private boolean isRequired(JsonNode fieldElement, FieldMetadata fieldMetadata) {
294277
return fieldMetadata.required() != null ? fieldMetadata.required() : getBoolean(fieldElement, REQUIRED_PROP);
295278
}
279+
280+
private static <T> BinaryOperator<T> preferNonDeprecated(Predicate<T> isDeprecatedPredicate) {
281+
return (existing, incoming) -> existing != null && isDeprecatedPredicate.test(existing) ? incoming : existing;
282+
}
296283
}

‎mod-record-specifications-server/src/test/java/org/folio/rspec/service/sync/SpecificationSyncServiceTest.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,63 @@ void sync_fetchesAndSyncsFields() {
7575
assertThat(metadata.getFields()).containsOnlyKeys("111", "222", "333");
7676
}
7777

78+
@Test
79+
void sync_shouldPreferNonObsoleteEntitiesOverObsoleteOnes() {
80+
final var specId = randomUUID();
81+
final var metadata = prepareMetadata();
82+
83+
var specification = new Specification();
84+
specification.setId(specId);
85+
specification.setFamily(Family.MARC);
86+
specification.setProfile(FamilyProfile.BIBLIOGRAPHIC);
87+
88+
var fieldsArray = prepareFieldsWithDuplicates();
89+
ArgumentCaptor<Collection<Field>> fieldsCaptor = ArgumentCaptor.captor();
90+
91+
when(metadataService.getSpecificationMetadata(specId)).thenReturn(metadata);
92+
when(specificationFetcher.fetch(Family.MARC, FamilyProfile.BIBLIOGRAPHIC)).thenReturn(fieldsArray);
93+
doNothing().when(specificationFieldService).syncFields(any(), fieldsCaptor.capture());
94+
95+
specificationSyncService.sync(specification);
96+
97+
var syncedFields = fieldsCaptor.getValue();
98+
99+
// Verify duplicate fields - should keep non-obsolete field
100+
var fields856 = syncedFields.stream()
101+
.filter(field -> "856".equals(field.getTag()))
102+
.toList();
103+
104+
assertThat(fields856).hasSize(1);
105+
var field856 = fields856.getFirst();
106+
assertThat(field856.isDeprecated()).isFalse();
107+
assertThat(field856.getLabel()).isEqualTo("Electronic Location and Access (Non-obsolete)");
108+
109+
// Verify duplicate subfields - should keep non-obsolete subfield
110+
var subfieldsH = field856.getSubfields().stream()
111+
.filter(subfield -> "h".equals(subfield.getCode()))
112+
.toList();
113+
114+
assertThat(subfieldsH).hasSize(1);
115+
var subfieldH = subfieldsH.getFirst();
116+
assertThat(subfieldH.isDeprecated()).isFalse();
117+
assertThat(subfieldH.getLabel()).isEqualTo("Non-functioning Uniform Resource Identifier");
118+
119+
// Verify duplicate indicator codes - should keep non-obsolete indicator code
120+
var firstIndicator = field856.getIndicators().stream()
121+
.filter(indicator -> 1 == indicator.getOrder())
122+
.findFirst()
123+
.orElseThrow(() -> new AssertionError("First indicator not found"));
124+
125+
var indicatorCodes0 = firstIndicator.getCodes().stream()
126+
.filter(code -> "0".equals(code.getCode()))
127+
.toList();
128+
129+
assertThat(indicatorCodes0).hasSize(1);
130+
var indicatorCode0 = indicatorCodes0.getFirst();
131+
assertThat(indicatorCode0.isDeprecated()).isFalse();
132+
assertThat(indicatorCode0.getLabel()).isEqualTo("Email (Non-obsolete)");
133+
}
134+
78135
private ArrayNode prepareFetchedFields() {
79136
var fieldNode1 = prepareFieldNode("222", "label1", false, true, false);
80137
var fieldNode2 = prepareFieldNode("333", "label2", true, false, true);
@@ -104,4 +161,53 @@ private SpecificationMetadata prepareMetadata() {
104161
metadata.setUrlFormat("format");
105162
return metadata;
106163
}
164+
165+
private ArrayNode prepareFieldsWithDuplicates() {
166+
var field856 = prepareFieldNode("856", "Electronic Location and Access (Non-obsolete)", false, true, false);
167+
var subfieldsArray = JsonNodeFactory.instance.arrayNode();
168+
169+
// Add duplicate subfields: obsolete first, non-obsolete second
170+
subfieldsArray.add(prepareSubfieldNode("Processor of request (NR) [OBSOLETE]", true, false));
171+
subfieldsArray.add(prepareSubfieldNode("Non-functioning Uniform Resource Identifier", false, true));
172+
field856.set("subfields", subfieldsArray);
173+
174+
var indicatorNode1 = JsonNodeFactory.instance.objectNode();
175+
indicatorNode1.put("order", 1);
176+
indicatorNode1.put("label", "Access method");
177+
178+
var indicatorCodesArray = JsonNodeFactory.instance.arrayNode();
179+
// Add duplicate indicator codes: obsolete first, non-obsolete second
180+
indicatorCodesArray.add(prepareIndicatorCodeNode("Email (Obsolete)", true));
181+
indicatorCodesArray.add(prepareIndicatorCodeNode("Email (Non-obsolete)", false));
182+
indicatorNode1.set("codes", indicatorCodesArray);
183+
184+
var indicatorsArray = JsonNodeFactory.instance.arrayNode();
185+
indicatorsArray.add(indicatorNode1);
186+
field856.set("indicators", indicatorsArray);
187+
188+
var fieldsArray = JsonNodeFactory.instance.arrayNode();
189+
var obsoleteField856 = prepareFieldNode("856", "Electronic Location and Access (Obsolete)", true, true, false);
190+
fieldsArray.add(obsoleteField856);
191+
fieldsArray.add(field856);
192+
return fieldsArray;
193+
}
194+
195+
private ObjectNode prepareSubfieldNode(String label, boolean deprecated,
196+
boolean repeatable) {
197+
var subfieldNode = JsonNodeFactory.instance.objectNode();
198+
subfieldNode.put("code", "h");
199+
subfieldNode.put("label", label);
200+
subfieldNode.put("deprecated", deprecated);
201+
subfieldNode.put("repeatable", repeatable);
202+
subfieldNode.put("required", false);
203+
return subfieldNode;
204+
}
205+
206+
private ObjectNode prepareIndicatorCodeNode(String label, boolean deprecated) {
207+
var codeNode = JsonNodeFactory.instance.objectNode();
208+
codeNode.put("code", "0");
209+
codeNode.put("label", label);
210+
codeNode.put("deprecated", deprecated);
211+
return codeNode;
212+
}
107213
}

0 commit comments

Comments
 (0)