Skip to content

Commit eb6e608

Browse files
olovyolovy
authored
fix(search): _site parameter (#1665)
The _site parameter didn't work because it was cast to String. Java port: Fix type of queryParameters everywhere.
1 parent 0976786 commit eb6e608

File tree

3 files changed

+42
-73
lines changed

3 files changed

+42
-73
lines changed

‎rest/src/main/groovy/whelk/rest/api/SearchUtils.java‎

Lines changed: 28 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ public SearchUtils(JsonLd jsonld) {
6767
}
6868
}
6969

70-
public Map<String, Object> doSearch(Map<String, Object> queryParameters) throws InvalidQueryException, IOException {
70+
public Map<String, Object> doSearch(Map<String, String[]> queryParameters) throws InvalidQueryException, IOException {
7171
if (whelk.elastic == null) {
7272
throw new WhelkRuntimeException("ElasticSearch not configured.");
7373
}
7474

75-
List<?> predicates = (List<?>) queryParameters.get("p");
75+
String[] predicates = queryParameters.get("p");
7676
String object = getReservedQueryParameter("o", queryParameters);
7777
String value = getReservedQueryParameter("value", queryParameters);
7878
String query = getReservedQueryParameter("q", queryParameters);
@@ -96,7 +96,9 @@ public Map<String, Object> doSearch(Map<String, Object> queryParameters) throws
9696
int offset = limitAndOffset.getV2();
9797

9898
Map<String, Object> pageParams = new LinkedHashMap<>();
99-
pageParams.put("p", predicates);
99+
if (predicates != null) {
100+
pageParams.put("p", Arrays.asList(predicates));
101+
}
100102
pageParams.put("value", value);
101103
pageParams.put("q", query);
102104
pageParams.put("o", object);
@@ -126,7 +128,7 @@ private Map<String, Object> applyLens(Map<String, Object> framedThing, String le
126128
};
127129
}
128130

129-
private Map<String, Object> queryElasticSearch(Map<String, Object> queryParameters,
131+
private Map<String, Object> queryElasticSearch(Map<String, String[]> queryParameters,
130132
Map<String, Object> pageParams,
131133
int limit,
132134
int offset,
@@ -146,9 +148,9 @@ private Map<String, Object> queryElasticSearch(Map<String, Object> queryParamete
146148
// consistent across search paths
147149
//
148150
// TODO Only manipulate `_limit` in one place
149-
queryParameters.put("_limit", List.of(String.valueOf(limit)));
151+
queryParameters.put("_limit", new String[]{String.valueOf(limit)});
150152

151-
Map<String, Object> esResult = (Map<String, Object>) esQuery.doQuery((Map) queryParameters, suggest, spell);
153+
Map<String, Object> esResult = (Map<String, Object>) esQuery.doQuery(queryParameters, suggest, spell);
152154
Lookup lookup = new Lookup();
153155

154156
List<Map<String, Object>> mappings = new ArrayList<>();
@@ -858,7 +860,7 @@ public Map<String, Object> makePaginationLinks(SearchType st, Map<String, Object
858860
* Use default values if not in query.
859861
*
860862
*/
861-
public Tuple2<Integer, Integer> getLimitAndOffset(Map<String, Object> queryParams) throws InvalidQueryException {
863+
public Tuple2<Integer, Integer> getLimitAndOffset(Map<String, String[]> queryParams) throws InvalidQueryException {
862864
int limit = parseIntFromQueryParams("_limit", queryParams, DEFAULT_LIMIT);
863865
// don't let users get carried away.
864866
if (limit > MAX_LIMIT) {
@@ -884,36 +886,21 @@ public Tuple2<Integer, Integer> getLimitAndOffset(Map<String, Object> queryParam
884886
* Use default value if key not found.
885887
*
886888
*/
887-
private int parseIntFromQueryParams(String key, Map<String, Object> queryParams, int defaultValue) {
889+
private int parseIntFromQueryParams(String key, Map<String, String[]> queryParams, int defaultValue) {
888890
if (queryParams.containsKey(key)) {
889-
Object value = queryParams.get(key);
890-
891891
// if someone supplies multiple values, we just pick the
892892
// first one and discard the rest.
893-
if (value instanceof List || value instanceof String[]) {
894-
if (value instanceof List && !((List<?>) value).isEmpty()) {
895-
value = ((List<?>) value).getFirst();
896-
} else if (value instanceof String[] && ((String[]) value).length > 0) {
897-
value = ((String[]) value)[0];
898-
} else {
899-
return defaultValue;
900-
}
901-
}
902-
903-
if (value instanceof String strValue) {
893+
var value = queryParams.get(key);
894+
if (value.length > 0) {
904895
try {
905-
return Integer.parseInt(strValue);
896+
return Integer.parseInt(value[0]);
906897
} catch (NumberFormatException e) {
907898
return defaultValue;
908899
}
909-
} else if (value instanceof Number numValue) {
910-
return numValue.intValue();
911-
} else {
912-
return defaultValue;
913900
}
914-
} else {
915-
return defaultValue;
916901
}
902+
903+
return defaultValue;
917904
}
918905

919906
/*
@@ -923,43 +910,33 @@ private int parseIntFromQueryParams(String key, Map<String, Object> queryParams,
923910
* filtered out.
924911
*
925912
*/
926-
private Tuple2<List<Map<String, Object>>, Map<String, Object>> mapParams(Lookup lookup, Map<String, Object> params, Set<String> multiSelectable) {
913+
private Tuple2<List<Map<String, Object>>, Map<String, Object>> mapParams(Lookup lookup, Map<String, String[]> params, Set<String> multiSelectable) {
927914
List<Map<String, Object>> result = new ArrayList<>();
928915
Map<String, Object> pageParams = new HashMap<>();
929916
List<String> reservedParams = getReservedParameters();
930917

931-
for (Map.Entry<String, Object> entry : params.entrySet()) {
918+
for (var entry : params.entrySet()) {
932919
String param = entry.getKey();
933-
Object paramValue = entry.getValue();
934920

935921
if (param.startsWith("_") || reservedParams.contains(param)) {
936922
continue;
937923
}
938924

939-
List<?> paramValues;
940-
if (paramValue instanceof List) {
941-
paramValues = (List<?>) paramValue;
942-
} else if (paramValue instanceof Object[]) {
943-
paramValues = Arrays.asList((Object[]) paramValue);
944-
} else {
945-
paramValues = List.of(paramValue);
946-
}
947-
948-
for (Object val : paramValues) {
925+
for (String val : entry.getValue()) {
949926
String valueProp;
950927
String termKey;
951928
Object value;
952929

953930
if (param.equals(JsonLd.TYPE_KEY) || param.equals(JsonLd.ID_KEY)) {
954931
valueProp = "object";
955932
termKey = param;
956-
Map<String, Object> chip = lookup.chip((String) val);
933+
Map<String, Object> chip = lookup.chip(val);
957934
chip.put(JsonLd.ID_KEY, val);
958935
value = chip;
959936
} else if (param.endsWith("." + JsonLd.ID_KEY)) {
960937
valueProp = "object";
961938
termKey = param.substring(0, param.length() - ("." + JsonLd.ID_KEY).length());
962-
Map<String, Object> chip = lookup.chip((String) val);
939+
Map<String, Object> chip = lookup.chip(val);
963940
chip.put(JsonLd.ID_KEY, val);
964941
value = chip;
965942
} else {
@@ -980,10 +957,9 @@ private Tuple2<List<Map<String, Object>>, Map<String, Object>> mapParams(Lookup
980957
}
981958
result.add(mapping);
982959

983-
if (!pageParams.containsKey(param)) {
984-
pageParams.put(param, new ArrayList<>());
985-
}
986-
((List<Object>) pageParams.get(param)).add(val);
960+
@SuppressWarnings("unchecked")
961+
var l = (List<Object>) pageParams.computeIfAbsent(param, k -> new ArrayList<>());
962+
l.add(val);
987963
}
988964
}
989965

@@ -1005,23 +981,14 @@ private List<String> getReservedParameters() {
1005981
* the String[] if found, null otherwise.
1006982
*
1007983
*/
1008-
private String getReservedQueryParameter(String name, Map<String, Object> queryParameters) {
984+
private String getReservedQueryParameter(String name, Map<String, String[]> queryParameters) {
1009985
if (queryParameters.containsKey(name)) {
1010-
Object value = queryParameters.get(name);
1011-
// For reserved parameters, we assume only one value
1012-
if (value instanceof List && !((List<?>) value).isEmpty()) {
1013-
return (String) ((List<?>) value).getFirst();
1014-
} else if (value instanceof String[] arr) {
1015-
if (arr.length > 0) {
1016-
return arr[0];
1017-
}
1018-
} else if (value instanceof String) {
1019-
return (String) value;
986+
var value = queryParameters.get(name);
987+
if (value != null && value.length > 0) {
988+
return value[0];
1020989
}
1021-
return null;
1022-
} else {
1023-
return null;
1024990
}
991+
return null;
1025992
}
1026993

1027994
private Object escapeQueryParam(Object input) {

‎rest/src/main/groovy/whelk/rest/api/SiteSearch.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ boolean isSearchResource(String path) {
8787
path.equals("/data") || path.startsWith("/data.");
8888
}
8989

90-
protected String determineActiveSite(Map queryParameters, String baseUri) {
90+
protected String determineActiveSite(Map<String, String[]> queryParameters, String baseUri) {
9191
// If ?_site=<foo> has been specified (and <foo> is a valid site) it takes precedence
92-
String paramSite = (String) queryParameters.get("_site");
92+
String paramSite = queryParameters.containsKey("_site") ? queryParameters.get("_site")[0] : "";
9393
if (searchStatsReprs.containsKey(paramSite)) {
9494
log.debug("Active site set by _site request parameter: {}", paramSite);
9595
return paramSite;
@@ -102,7 +102,7 @@ protected String determineActiveSite(Map queryParameters, String baseUri) {
102102
return activeSite;
103103
}
104104

105-
Map findData(Map queryParameters, String baseUri, String path) throws InvalidQueryException, IOException {
105+
Map findData(Map<String, String[]> queryParameters, String baseUri, String path) throws InvalidQueryException, IOException {
106106
String activeSite = determineActiveSite(queryParameters, baseUri);
107107

108108
Map searchSettings = searchStatsReprs.get(activeSite);
@@ -136,7 +136,7 @@ Map findData(Map queryParameters, String baseUri, String path) throws InvalidQue
136136
}
137137
}
138138

139-
Map toDataIndexDescription(Map appDesc, Map queryParameters) throws InvalidQueryException, IOException {
139+
Map toDataIndexDescription(Map appDesc, Map<String, String[]> queryParameters) throws InvalidQueryException, IOException {
140140
queryParameters.computeIfAbsent("_limit", k -> new String[]{"0"});
141141
queryParameters.computeIfAbsent("q", k -> new String[]{"*"});
142142
Map searchResults = search.doSearch(queryParameters);

‎rest/src/test/groovy/whelk/rest/api/SearchUtilsSpec.groovy‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,21 +141,23 @@ class SearchUtilsSpec extends Specification {
141141
where:
142142
params | result
143143
[:] | new Tuple2(SearchUtils.DEFAULT_LIMIT, SearchUtils.DEFAULT_OFFSET)
144-
['_limit': '5'] | new Tuple2(5, SearchUtils.DEFAULT_OFFSET)
145-
['_limit': ['5']] | new Tuple2(5, SearchUtils.DEFAULT_OFFSET)
146144
['_limit': ['5'] as String[]] | new Tuple2(5, SearchUtils.DEFAULT_OFFSET)
147-
['_limit': '100000000'] | new Tuple2(SearchUtils.DEFAULT_LIMIT, SearchUtils.DEFAULT_OFFSET)
148-
['_offset': '5'] | new Tuple2(SearchUtils.DEFAULT_LIMIT, 5)
149-
['_limit': '5', 'foo': 'bar', '_offset': '5'] | new Tuple2(5, 5)
150-
['_limit': 'foo'] | new Tuple2(SearchUtils.DEFAULT_LIMIT, SearchUtils.DEFAULT_OFFSET)
145+
['_limit': ['100000000'] as String[]] | new Tuple2(SearchUtils.DEFAULT_LIMIT, SearchUtils.DEFAULT_OFFSET)
146+
['_offset': ['5'] as String[]] | new Tuple2(SearchUtils.DEFAULT_LIMIT, 5)
147+
[
148+
'_limit': ['5'] as String[],
149+
'foo': ['bar'] as String[],
150+
'_offset': ['5'] as String[]
151+
] | new Tuple2(5, 5)
152+
['_limit': ['foo'] as String []] | new Tuple2(SearchUtils.DEFAULT_LIMIT, SearchUtils.DEFAULT_OFFSET)
151153
152154
}
153155
154156
def "Should throw on negative limit"() {
155157
given:
156158
157159
when:
158-
search.getLimitAndOffset(['_limit': '-1'])
160+
search.getLimitAndOffset(['_limit': ['-1'] as String[]])
159161
160162
then:
161163
thrown InvalidQueryException
@@ -165,7 +167,7 @@ class SearchUtilsSpec extends Specification {
165167
given:
166168
167169
when:
168-
search.getLimitAndOffset(['_offset': '-1'])
170+
search.getLimitAndOffset(['_offset': ['-1'] as String[]])
169171
170172
then:
171173
thrown InvalidQueryException

0 commit comments

Comments
 (0)