Skip to content

Commit c388129

Browse files
authored
chore: refactor code to comply with Checkstyle method length limit (max 25 lines) (#869)
Closes: MSEARCH-1110
1 parent b832ba3 commit c388129

File tree

63 files changed

+1873
-1893
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+1873
-1893
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
* Update contributors.name search option type to full-text in documentation ([MSEARCH-814](https://folio-org.atlassian.net/browse/MSEARCH-814))
7373
* Process item/instance in batches, add stale sub-resource lock release logic ([MSEARCH-1097](https://folio-org.atlassian.net/browse/MSEARCH-1097))
7474
* Refactor system user handling and update dependencies in ModuleDescriptor ([MSEARCH-1111](https://folio-org.atlassian.net/browse/MSEARCH-1111))
75+
* Refactor code to comply with Checkstyle method length limit (max 25 lines) ([MSEARCH-1110](https://folio-org.atlassian.net/browse/MSEARCH-1110))
7576

7677
### Dependencies
7778
* Bump `LIB_NAME` from `OLD_VERSION` to `NEW_VERSION`

pom.xml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -459,9 +459,6 @@
459459
<logViolationsToConsole>true</logViolationsToConsole>
460460
<configLocation>folio-checkstyle/checkstyle.xml</configLocation>
461461
<cacheFile>${basedir}/target/cachefile</cacheFile>
462-
<propertyExpansion>
463-
checkstyle.method-length.max=300
464-
</propertyExpansion>
465462
</configuration>
466463
</plugin>
467464

src/main/java/org/folio/search/cql/CqlTermQueryConverter.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,7 @@ public QueryBuilder getQuery(CQLTermNode termNode, ResourceType resource) {
8585
var searchTerm = getSearchTerm(termNode.getTerm(), optionalPlainFieldByPath);
8686
var comparator = isWildcardQuery(searchTerm) ? WILDCARD_OPERATOR : lowerCase(termNode.getRelation().getBase());
8787

88-
var termQueryBuilder = termQueryBuilders.get(comparator);
89-
if (termQueryBuilder == null) {
90-
throw new UnsupportedOperationException(String.format(
91-
"Failed to parse CQL query. Comparator '%s' is not supported.", comparator));
92-
}
88+
var termQueryBuilder = getTermQueryBuilderOrFail(comparator);
9389

9490
var modifiers = termNode.getRelation().getModifiers().stream()
9591
.map(Modifier::getType)
@@ -109,6 +105,15 @@ public QueryBuilder getQuery(CQLTermNode termNode, ResourceType resource) {
109105
: termQueryBuilder.getTermLevelQuery(searchTerm, fieldName, resource, index);
110106
}
111107

108+
private TermQueryBuilder getTermQueryBuilderOrFail(String comparator) {
109+
var termQueryBuilder = termQueryBuilders.get(comparator);
110+
if (termQueryBuilder == null) {
111+
throw new UnsupportedOperationException(String.format(
112+
"Failed to parse CQL query. Comparator '%s' is not supported.", comparator));
113+
}
114+
return termQueryBuilder;
115+
}
116+
112117
private Object getSearchTerm(String term, Optional<PlainFieldDescription> plainFieldDescription) {
113118
var normalizedTerm = term.replace("\\\\", "\\");
114119
return plainFieldDescription
@@ -139,17 +144,21 @@ private static Map<String, TermQueryBuilder> getTermQueryProvidersAsMap(List<Ter
139144
}
140145
}
141146

147+
throwIfErrorsExist(errors);
148+
149+
return unmodifiableMap(queryBuildersMap);
150+
}
151+
152+
private static void throwIfErrorsExist(LinkedHashMap<String, List<TermQueryBuilder>> errors) {
142153
if (MapUtils.isNotEmpty(errors)) {
143154
var stringJoiner = new StringJoiner(", ");
144155
errors.forEach((c, v) -> {
145156
var buildersAsString = v.stream().map(e -> e.getClass().getSimpleName()).collect(joining(", "));
146157
stringJoiner.add(String.format("comparator '%s': %s", c, buildersAsString));
147158
});
148159
throw new IllegalStateException(String.format("Multiple TermQueryBuilder objects cannot be responsible "
149-
+ "for the same comparator. Found issues: [%s]", stringJoiner));
160+
+ "for the same comparator. Found issues: [%s]", stringJoiner));
150161
}
151-
152-
return unmodifiableMap(queryBuildersMap);
153162
}
154163

155164
private void validateIndexFormat(String index, CQLTermNode termNode) {

src/main/java/org/folio/search/cql/FacetQueryBuilder.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,7 @@ private static Pair<BoolQueryBuilder, List<String>> getFilterQueryAndFacetTerms(
118118
var facetTerms = new ArrayList<String>();
119119

120120
List<QueryBuilder> filters = new ArrayList<>(((BoolQueryBuilder) query).filter());
121-
var musts = ((BoolQueryBuilder) query).must();
122-
for (var must : musts) {
123-
if (must instanceof NestedQueryBuilder nestedQueryBuilder
124-
&& nestedQueryBuilder.query() instanceof BoolQueryBuilder boolQueryBuilder) {
125-
filters.addAll(boolQueryBuilder.filter());
126-
}
127-
}
121+
addNestedQueryFilters(((BoolQueryBuilder) query).must(), filters);
128122
for (var filterQuery : filters) {
129123
if (isFilterQuery(filterQuery, field::equals)) {
130124
getValueFromFilerQuery(filterQuery).ifPresent(facetTerms::add);
@@ -142,6 +136,15 @@ private static Pair<BoolQueryBuilder, List<String>> getFilterQueryAndFacetTerms(
142136
return isNotEmpty(facetFilterQuery.filter()) ? Pair.of(facetFilterQuery, facetTerms) : Pair.of(null, facetTerms);
143137
}
144138

139+
private static void addNestedQueryFilters(List<QueryBuilder> musts, List<QueryBuilder> filters) {
140+
for (var must : musts) {
141+
if (must instanceof NestedQueryBuilder nestedQueryBuilder
142+
&& nestedQueryBuilder.query() instanceof BoolQueryBuilder boolQueryBuilder) {
143+
filters.addAll(boolQueryBuilder.filter());
144+
}
145+
}
146+
}
147+
145148
private static AggregationBuilder getFilterAggregation(
146149
Pair<BoolQueryBuilder, List<String>> filterAndTerms, Facet facet) {
147150
var filterAggregation = filter(facet.aggregationName(), filterAndTerms.getFirst());

src/main/java/org/folio/search/model/context/FolioExecutionContextBuilder.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public Builder(FolioModuleMetadata moduleMetadata) {
5050
this.okapiHeaders = new HashMap<>();
5151
}
5252

53+
@SuppressWarnings("checkstyle:MethodLength")
5354
public FolioExecutionContext build() {
5455
return new FolioExecutionContext() {
5556
@Override

src/main/java/org/folio/search/service/browse/AbstractShelvingOrderBrowseServiceBySearchAfter.java

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import static org.opensearch.search.sort.SortOrder.ASC;
1212
import static org.opensearch.search.sort.SortOrder.DESC;
1313

14+
import java.util.List;
15+
import java.util.UUID;
1416
import lombok.extern.log4j.Log4j2;
1517
import org.apache.commons.collections4.CollectionUtils;
1618
import org.folio.search.domain.dto.BrowseConfig;
@@ -20,6 +22,7 @@
2022
import org.folio.search.service.consortium.BrowseConfigServiceDecorator;
2123
import org.folio.search.service.consortium.ConsortiumSearchHelper;
2224
import org.folio.search.utils.ShelvingOrderCalculationHelper;
25+
import org.opensearch.index.query.BoolQueryBuilder;
2326
import org.opensearch.index.query.QueryBuilder;
2427
import org.opensearch.index.query.TermQueryBuilder;
2528
import org.opensearch.search.builder.SearchSourceBuilder;
@@ -88,21 +91,14 @@ protected SearchSourceBuilder getSearchQuery(BrowseRequest req, BrowseContext ct
8891

8992
private QueryBuilder getQuery(BrowseContext ctx, BrowseConfig config, TermQueryBuilder anchorQuery) {
9093
var typeIds = config.getTypeIds();
91-
var typeIdsEmpty = CollectionUtils.isEmpty(config.getTypeIds());
92-
if (typeIdsEmpty && ctx.getFilters().isEmpty()) {
94+
if (CollectionUtils.isEmpty(typeIds) && ctx.getFilters().isEmpty()) {
9395
if (anchorQuery != null) {
9496
return anchorQuery;
9597
}
9698
return matchAllQuery();
9799
} else {
98100
var boolQueryMain = boolQuery();
99-
if (!typeIdsEmpty) {
100-
var boolQuery = boolQuery();
101-
for (var typeId : typeIds) {
102-
boolQuery.should(termQuery(getTypeIdField(), typeId.toString()));
103-
}
104-
boolQueryMain.must(boolQuery);
105-
}
101+
includeTypeIdConditions(typeIds, boolQueryMain);
106102
if (!ctx.getFilters().isEmpty()) {
107103
ctx.getFilters().forEach(boolQueryMain::filter);
108104
}
@@ -113,6 +109,18 @@ private QueryBuilder getQuery(BrowseContext ctx, BrowseConfig config, TermQueryB
113109
}
114110
}
115111

112+
private void includeTypeIdConditions(List<UUID> typeIds, BoolQueryBuilder boolQueryMain) {
113+
if (CollectionUtils.isNotEmpty(typeIds)) {
114+
var boolQuery = boolQuery();
115+
for (var typeId : typeIds) {
116+
if (typeId != null) {
117+
boolQuery.should(termQuery(getTypeIdField(), typeId.toString()));
118+
}
119+
}
120+
boolQueryMain.must(boolQuery);
121+
}
122+
}
123+
116124
private static String getBrowseField(BrowseConfig config) {
117125
return BROWSE_FIELDS_MAP.getOrDefault(config.getShelvingAlgorithm(), DEFAULT_SHELVING_ORDER_BROWSING_FIELD);
118126
}

0 commit comments

Comments
 (0)