Skip to content

Commit

Permalink
np-47469: allow special characters for organization query
Browse files Browse the repository at this point in the history
  • Loading branch information
torarnet committed Sep 4, 2024
1 parent 0fa970c commit 4535036
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class ErrorMessages {
"Upstream returned 400 (Bad Request). That might indicate bad query parameters";
public static final String ERROR_MESSAGE_BACKEND_FAILED_WITH_EXCEPTION =
"Remote service responded with error when client called uri: %s. Exception from upstream returned: %s";
public static final String ERROR_MESSAGE_REQUIRED_PARAM_MISSING =
"Required param '%s' is missing";

/**
* Formats and emits a message with required parameter names.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import nva.commons.core.Environment;

import java.util.List;
import java.util.Optional;
import java.util.Set;

import static no.unit.nva.cristin.common.ErrorMessages.ALPHANUMERIC_CHARACTERS_DASH_COMMA_PERIOD_AND_WHITESPACE;
Expand All @@ -17,7 +16,6 @@
import static no.unit.nva.cristin.model.Constants.FIRST_PAGE;
import static no.unit.nva.cristin.model.JsonPropertyNames.NAME;
import static no.unit.nva.cristin.model.JsonPropertyNames.NUMBER_OF_RESULTS;
import static no.unit.nva.cristin.model.JsonPropertyNames.ORGANIZATION;
import static no.unit.nva.cristin.model.JsonPropertyNames.PAGE;
import static no.unit.nva.cristin.model.JsonPropertyNames.QUERY;

Expand Down Expand Up @@ -66,11 +64,6 @@ protected String getValidName(RequestInfo requestInfo) throws BadRequestExceptio
invalidQueryParametersMessage(NAME, ALPHANUMERIC_CHARACTERS_DASH_COMMA_PERIOD_AND_WHITESPACE)));
}

protected Optional<String> getValidOrganization(RequestInfo requestInfo) {
return requestInfo.getQueryParameterOpt(ORGANIZATION)
.filter(this::isValidQueryString);
}

protected boolean isValidQueryString(String str) {
for (Character c : str.toCharArray()) {
if (isUnsupportedCharacter(c)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import static no.unit.nva.cristin.common.ErrorMessages.ERROR_MESSAGE_REQUIRED_PARAM_MISSING;
import static no.unit.nva.cristin.common.ErrorMessages.validQueryParameterNamesMessage;
import static no.unit.nva.client.ClientProvider.VERSION;
import static no.unit.nva.cristin.model.Constants.FULL_TREE;
Expand Down Expand Up @@ -92,6 +93,12 @@ private ConcurrentHashMap<String, String> extractQueryParameters(RequestInfo req
return requestQueryParams;
}

protected String getValidQuery(RequestInfo requestInfo) throws BadRequestException {
return requestInfo.getQueryParameterOpt(QUERY)
.orElseThrow(() -> new BadRequestException(
String.format(ERROR_MESSAGE_REQUIRED_PARAM_MISSING, QUERY)));
}

private Optional<String> getSort(RequestInfo requestInfo) {
return requestInfo.getQueryParameterOpt(SORT);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Feature: API tests for Cristin Organization retrieve and search
Then status 400
And match response.title == 'Bad Request'
And match response.status == 400
And match response.detail == 'Parameter \'query\' has invalid value. May only contain alphanumeric characters, dash, comma, period, colon, semicolon and whitespace'
And match response.detail == 'Required param \'query\' is missing'

Scenario: GET returns 404 status Not found when requesting unknown organization identifier
Given path '/organization/' + nonExistingOrganizationId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
import static java.net.HttpURLConnection.HTTP_OK;
import static no.unit.nva.cristin.common.ErrorMessages.ALPHANUMERIC_CHARACTERS_DASH_COMMA_PERIOD_AND_WHITESPACE;
import static no.unit.nva.cristin.common.ErrorMessages.ERROR_MESSAGE_DEPTH_INVALID;
import static no.unit.nva.cristin.common.ErrorMessages.invalidQueryParametersMessage;
import static no.unit.nva.client.ClientProvider.VERSION_2023_05_26;
import static no.unit.nva.client.ClientProvider.VERSION_ONE;
import static no.unit.nva.cristin.model.Constants.FULL;
Expand Down Expand Up @@ -88,6 +86,7 @@ class QueryCristinOrganizationHandlerTest {
public static final String MEDICAL_BIOCHEMISTRY = "Department of Medical Biochemistry";
public static final String EMPTY_OBJECT = "{}";
public static final String NOT_FOUND_LOG_MESSAGE = "Organization from search result could not be found in upstream";
public static final String PARAM_QUERY_IS_MISSING = "Required param 'query' is missing";

private QueryCristinOrganizationHandler queryCristinOrganizationHandler;
private DefaultOrgQueryClientProvider clientProvider;
Expand Down Expand Up @@ -126,8 +125,7 @@ void shouldReturnBadRequestResponseOnMissingQueryParam() throws IOException {
var gatewayResponse = GatewayResponse.fromOutputStream(output,Problem.class);
var actualDetail = getProblemDetail(gatewayResponse);
assertEquals(HTTP_BAD_REQUEST, gatewayResponse.getStatusCode());
assertThat(actualDetail, containsString(invalidQueryParametersMessage(
QUERY, ALPHANUMERIC_CHARACTERS_DASH_COMMA_PERIOD_AND_WHITESPACE)));
assertThat(actualDetail, containsString(PARAM_QUERY_IS_MISSING));
}

@Test
Expand Down Expand Up @@ -399,6 +397,17 @@ void shouldHaveCorrectEncodingOfNameParam() throws Exception {
assertThat(gatewayResponse.getStatusCode(), equalTo(HTTP_OK));
}

@ParameterizedTest
@MethodSource("specialCharacterQueryParamProvider")
void shouldAllowSpecialCharactersForQueryString(String query) throws Exception {
var input = generateHandlerRequestWithQuery(query);
queryCristinOrganizationHandler.handleRequest(input, output, context);

var gatewayResponse = GatewayResponse.fromOutputStream(output, SearchResponse.class);

assertThat(gatewayResponse.getStatusCode(), equalTo(HTTP_OK));
}

private List<Organization> convertHitsToProperFormat(SearchResponse<?> searchResponse) {
return OBJECT_MAPPER.convertValue(searchResponse.getHits(), new TypeReference<>() {});
}
Expand Down Expand Up @@ -517,4 +526,20 @@ private FetchCristinOrgClient20230526 mockFetchClientWithOneHitMissingInUpstream
return fetchClient;
}

private InputStream generateHandlerRequestWithQuery(String query) throws JsonProcessingException {
return new HandlerRequestBuilder<InputStream>(restApiMapper)
.withHeaders(Map.of(CONTENT_TYPE, APPLICATION_JSON_LD.type()))
.withQueryParameters(Map.of(QUERY, query))
.build();
}

private static Stream<Arguments> specialCharacterQueryParamProvider() {
return Stream.of(
Arguments.of("Wyższa Szkoła Informatyki i Zarządzania \"COPERNICUS\" we Wrocławiu"),
Arguments.of("Yale School of Medicine - Yale Cancer Center (YCC)"),
Arguments.of("Hogeschool IPABO Amsterdam/Alkmaar"),
Arguments.of("University of Technology, Sydney - Branch: St. Leonards Campus)")
);
}

}

0 comments on commit 4535036

Please sign in to comment.