Skip to content

Commit

Permalink
MSEARCH-557: Call number browse with call number type shows irrelevan…
Browse files Browse the repository at this point in the history 
…t result fix (#416)

* MSEARCH-557: Small fix

* MSEARCH-557: Removed unnecessary parts of logic
  • Loading branch information
@hasanepam
hasanepam authored Sep 14, 2023
1 parent 07ad34e commit 1518543
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 76 deletions.
42 changes: 6 additions & 36 deletions src/main/java/org/folio/search/utils/CallNumberUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,9 @@
import org.apache.commons.lang3.RegExUtils;
import org.apache.commons.lang3.StringUtils;
import org.folio.search.domain.dto.CallNumberBrowseItem;
import org.folio.search.domain.dto.Holding;
import org.folio.search.domain.dto.Item;
import org.folio.search.model.types.CallNumberType;
import org.jetbrains.annotations.NotNull;
import org.marc4j.callnum.DeweyCallNumber;
import org.marc4j.callnum.LCCallNumber;
import org.marc4j.callnum.NlmCallNumber;
import org.springframework.util.Assert;

@UtilityClass
Expand Down Expand Up @@ -178,19 +174,13 @@ public static List<CallNumberBrowseItem> excludeIrrelevantResultItems(String cal
return records;
}

records.forEach(r -> {
r.getInstance().setItems(getItemsFiltered(callNumberType, r));
r.getInstance().setHoldings(getHoldingsFiltered(callNumberType, r));
});
return records;
}

@NotNull
private static List<@Valid Holding> getHoldingsFiltered(String callNumberType, CallNumberBrowseItem item) {
return item.getInstance().getHoldings()
records.forEach(r -> r.getInstance().setItems(getItemsFiltered(callNumberType, r)));
return records
.stream()
.filter(h -> isInGivenType(callNumberType, h.getCallNumber()))
.toList();
.filter(r -> r.getInstance().getItems()
.stream()
.anyMatch(i -> r.getFullCallNumber() == null
|| i.getEffectiveCallNumberComponents().getCallNumber().equals(r.getFullCallNumber()))).toList();
}

@NotNull
Expand All @@ -202,26 +192,6 @@ public static List<CallNumberBrowseItem> excludeIrrelevantResultItems(String cal
.toList();
}

private boolean isInGivenType(String callNumberType, String callNumber) {
switch (callNumberType) {
case "dewey" -> {
return new DeweyCallNumber(callNumber).isValid();
}
case "lc" -> {
return new LCCallNumber(callNumber).isValid();
}
case "nlm" -> {
return new NlmCallNumber(callNumber).isValid();
}
default -> {
return false;
}
// case "sudoc" -> {
// return new SuDocCallNumber(callNumber).isValid();
// };
}
}

private static long callNumberToLong(String callNumber, long startVal, int maxChars) {
var cleanCallNumber = cleanCallNumber(callNumber, maxChars);
if (StringUtils.isBlank(cleanCallNumber)) {
Expand Down
108 changes: 78 additions & 30 deletions src/test/java/org/folio/search/controller/BrowseTypedCallNumberIrrelevantResultIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static org.folio.search.utils.TestUtils.randomId;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -56,28 +57,49 @@ void browseByCallNumber_browsingAroundWithEnabledIntermediateValues() {
.param("precedingRecordsCount", "5")
.param("expandAll", "true");
var actual = parseResponse(doGet(request), CallNumberBrowseResult.class);
final CallNumberBrowseResult expected = new CallNumberBrowseResult()
var expected = new CallNumberBrowseResult()
.totalRecords(18)
.prev("3308 H975")
.next("3308 H981")
.items(List.of(
cnBrowseItem(INSTANCE_MAP.get("instance #18"), "308 H975"),
cnBrowseItem(INSTANCE_MAP.get("instance #01"), "308 H976"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H972"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H973"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H974"),
cnBrowseItem(INSTANCE_MAP.get("instance #09"), "308 H977", true),
cnBrowseItem(INSTANCE_MAP.get("instance #01"), "308 H978"),
cnBrowseItem(INSTANCE_MAP.get("instance #10"), "308 H979"),
cnBrowseItem(INSTANCE_MAP.get("instance #02"), "308 H980"),
cnBrowseItem(INSTANCE_MAP.get("instance #08"), "308 H981")
.items(Arrays.asList(
cnBrowseItem(instance("instance #18"), "308 H975"),
cnBrowseItem(instance("instance #01"), "308 H976"),
cnBrowseItem(instance("instance #11"), "308 H972"),
cnBrowseItem(instance("instance #11"), "308 H973"),
cnBrowseItem(instance("instance #11"), "308 H974"),
cnBrowseItem(instance("instance #09"), "308 H977", true),
cnBrowseItem(instance("instance #01"), "308 H978"),
cnBrowseItem(instance("instance #10"), "308 H979"),
cnBrowseItem(instance("instance #02"), "308 H980"),
cnBrowseItem(instance("instance #08"), "308 H981")
));
expected.setItems(CallNumberUtils.excludeIrrelevantResultItems("dewey", expected.getItems()));
assertThat(actual).isEqualTo(expected);
}

@Test
void browseByCallNumber_browsingAroundWithDisabledIntermediateValuesAndLowLimit() {
void browseByCallNumber_browsingAroundWithoutExpandAll() {
var request = get(instanceCallNumberBrowsePath())
.param("callNumberType", "lc")
.param("query", prepareQuery("typedCallNumber >= {value} or typedCallNumber < {value}", "Z669.R360 1975"))
.param("limit", "10")
.param("highlightMatch", "true")
.param("precedingRecordsCount", "5");
var actual = parseResponse(doGet(request), CallNumberBrowseResult.class);
var expected = new CallNumberBrowseResult()
.totalRecords(4)
.items(Arrays.asList(
cnBrowseItem(instance("instance #10"), "Z669.R360 1975", true),
cnBrowseItem(instance("instance #02"), "Z669.R360 1976"),
cnBrowseItem(instance("instance #10"), "Z669.R360 1977")
));
expected.setItems(CallNumberUtils.excludeIrrelevantResultItems("lc", expected.getItems()));
leaveOnlyBasicProps(expected);
assertThat(actual).isEqualTo(expected);
}

@Test
void browseByCallNumber_browsingAroundWithLowLimit() {
var limit = 12;
var request = get(instanceCallNumberBrowsePath())
.param("callNumberType", "dewey")
Expand All @@ -87,31 +109,42 @@ void browseByCallNumber_browsingAroundWithDisabledIntermediateValuesAndLowLimit(
.param("precedingRecordsCount", "7")
.param("expandAll", "true");
var result = parseResponse(doGet(request), CallNumberBrowseResult.class);
final CallNumberBrowseResult expected = new CallNumberBrowseResult()
var expected = new CallNumberBrowseResult()
.next("3308 H981")
.totalRecords(18)
.items(
List.of(
cnBrowseItem(INSTANCE_MAP.get("instance #10"), "308 H970"),
cnBrowseItem(INSTANCE_MAP.get("instance #01"), "308 H971"),
cnBrowseItem(INSTANCE_MAP.get("instance #18"), "308 H975"),
cnBrowseItem(INSTANCE_MAP.get("instance #01"), "308 H976"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H972"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H973"),
cnBrowseItem(INSTANCE_MAP.get("instance #11"), "308 H974"),
cnBrowseItem(INSTANCE_MAP.get("instance #09"), "308 H977", true),
cnBrowseItem(INSTANCE_MAP.get("instance #01"), "308 H978"),
cnBrowseItem(INSTANCE_MAP.get("instance #10"), "308 H979"),
cnBrowseItem(INSTANCE_MAP.get("instance #02"), "308 H980"),
cnBrowseItem(INSTANCE_MAP.get("instance #08"), "308 H981")
)
);
.items(Arrays.asList(
cnBrowseItem(instance("instance #10"), "308 H970"),
cnBrowseItem(instance("instance #01"), "308 H971"),
cnBrowseItem(instance("instance #18"), "308 H975"),
cnBrowseItem(instance("instance #01"), "308 H976"),
cnBrowseItem(instance("instance #11"), "308 H972"),
cnBrowseItem(instance("instance #11"), "308 H973"),
cnBrowseItem(instance("instance #11"), "308 H974"),
cnBrowseItem(instance("instance #09"), "308 H977", true),
cnBrowseItem(instance("instance #01"), "308 H978"),
cnBrowseItem(instance("instance #10"), "308 H979"),
cnBrowseItem(instance("instance #02"), "308 H980"),
cnBrowseItem(instance("instance #08"), "308 H981")
));
expected.setItems(CallNumberUtils.excludeIrrelevantResultItems("dewey", expected.getItems()));

assertThat(result.getItems()).hasSizeLessThanOrEqualTo(limit);
assertThat(result).isEqualTo(expected);
}

private void leaveOnlyBasicProps(CallNumberBrowseResult expected) {
expected.getItems().forEach(i -> {
Instance instance = i.getInstance();
instance.setTenantId(null);
instance.setShared(null);
instance.getItems().forEach(item -> {
item.setId(null);
item.setTenantId(null);
item.setDiscoverySuppress(null);
});
});
}

private static Instance[] instances() {
List<List<String>> instanceData = callNumberBrowseInstanceData();
Map<String, List<List<String>>> collectedByTitle = instanceData
Expand All @@ -127,6 +160,21 @@ private static Instance[] instances() {
return instanceArray;
}

private static Instance instance(String title) {
Instance instance1 = INSTANCE_MAP.get(title);

Instance instance = new Instance();
instance.setId(instance1.getId());
instance.setTitle(title);
instance.setItems(new ArrayList<>(instance1.getItems()));
instance.setTenantId(instance1.getTenantId());
instance.setShared(instance1.getShared());
instance.staffSuppress(instance1.getStaffSuppress());
instance.discoverySuppress(instance1.getDiscoverySuppress());
instance.isBoundWith(instance1.getIsBoundWith());
return instance;
}

private static Instance instance(List<List<String>> data, String title) {
var items = data.stream().map(d -> new Item()
.id(randomId())
Expand Down
10 changes: 0 additions & 10 deletions src/test/java/org/folio/search/utils/CallNumberUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,6 @@ void excludeIrrelevantResultItems(String callNumberType, CallNumberBrowseItem gi
assertThat(unchangedItems).isEqualTo(List.of(given));
}

@ParameterizedTest(name = "[{index}] callNumber={0}, records={1}, expected={2}")
@MethodSource("eliminateIrrelevantItemsOnCallNumberBrowsingDataHoldings")
void excludeIrrelevantResultHoldings(String callNumberType, CallNumberBrowseItem given,
CallNumberBrowseItem expected) {
var items = CallNumberUtils.excludeIrrelevantResultItems(callNumberType, List.of(given));
assertThat(items).isEqualTo(List.of(expected));
var unchangedItems = CallNumberUtils.excludeIrrelevantResultItems("", List.of(given));
assertThat(unchangedItems).isEqualTo(List.of(given));
}

private static Stream<Arguments> supportedCharactersDataset() {
return StreamEx.<Arguments>empty()
.append(letterCharacterDataProvider())
Expand Down

0 comments on commit 1518543

Please sign in to comment.