From ca4857b1b50cfc995e6792655529da3edadf9507 Mon Sep 17 00:00:00 2001 From: srishti Date: Tue, 10 Dec 2024 22:47:45 +0100 Subject: [PATCH] EA-4015 review comments --- set-client/pom.xml | 15 -- .../set/client/UserSetApiClient.java | 4 +- .../client/connection/BaseApiConnection.java | 145 ++++++++++-------- .../connection/UserSetApiConnection.java | 6 +- .../set/client}/json/AgentDeserializer.java | 5 +- .../set/client/json/UserSetDeserializer.java | 51 ++++++ .../set/client/web/SearchUserSetApi.java | 2 +- .../set/client/web/WebUserSetApi.java | 2 +- set-definitions/pom.xml | 14 -- .../definitions/model/BaseWebResource.java | 3 - .../model/agent/impl/BaseAgent.java | 2 - .../definitions/model/impl/BaseUserSet.java | 31 +--- 12 files changed, 141 insertions(+), 139 deletions(-) rename {set-definitions/src/main/java/eu/europeana/set/definitions => set-client/src/main/java/eu/europeana/set/client}/json/AgentDeserializer.java (89%) create mode 100644 set-client/src/main/java/eu/europeana/set/client/json/UserSetDeserializer.java diff --git a/set-client/pom.xml b/set-client/pom.xml index 0d4a7008..8055cad6 100644 --- a/set-client/pom.xml +++ b/set-client/pom.xml @@ -37,13 +37,6 @@ ${version.jettison} - - - jakarta.ws.rs - jakarta.ws.rs-api - 4.0.0 - - @@ -52,14 +45,6 @@ 3.1.2 - - - - - - - - org.junit.jupiter diff --git a/set-client/src/main/java/eu/europeana/set/client/UserSetApiClient.java b/set-client/src/main/java/eu/europeana/set/client/UserSetApiClient.java index 004588e4..5ae1d68b 100644 --- a/set-client/src/main/java/eu/europeana/set/client/UserSetApiClient.java +++ b/set-client/src/main/java/eu/europeana/set/client/UserSetApiClient.java @@ -61,7 +61,7 @@ public UserSet updateUserSet(String identifier, String set, String profile) thro } @Override - public List getPaginationUserSet(String identifier, String sort, String sortOrder, int page, int pageSize, String profile) throws SetApiClientException { + public List getPaginationUserSet(String identifier, String sort, String sortOrder, String page, String pageSize, String profile) throws SetApiClientException { return getApiConnection().getPaginationUserSet(identifier, sort, sortOrder, page, pageSize, profile); } } @@ -70,7 +70,7 @@ private class SearchUserSetClient implements SearchUserSetApi { @Override public List searchUserSet(String query, String[] qf, - String sort, int page, int pageSize, String facet, int facetLimit, String profile) throws SetApiClientException { + String sort, String page, String pageSize, String facet, int facetLimit, String profile) throws SetApiClientException { return getApiConnection().searchUserSet(query, qf, sort, page, pageSize, facet, facetLimit, profile); } } diff --git a/set-client/src/main/java/eu/europeana/set/client/connection/BaseApiConnection.java b/set-client/src/main/java/eu/europeana/set/client/connection/BaseApiConnection.java index c3083048..f9b435e4 100644 --- a/set-client/src/main/java/eu/europeana/set/client/connection/BaseApiConnection.java +++ b/set-client/src/main/java/eu/europeana/set/client/connection/BaseApiConnection.java @@ -1,28 +1,34 @@ package eu.europeana.set.client.connection; import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; import eu.europeana.api.commons.definitions.search.result.impl.ResultsPageImpl; import eu.europeana.set.client.exception.SetApiClientException; +import eu.europeana.set.client.json.RecordPreviewDeserializer; +import eu.europeana.set.client.json.UserSetDeserializer; import eu.europeana.set.client.model.result.AbstractUserSetApiResponse; import eu.europeana.set.client.model.result.RecordPreview; import eu.europeana.set.common.http.HttpConnection; import eu.europeana.set.definitions.model.UserSet; -import eu.europeana.set.definitions.model.impl.BaseUserSet; +import eu.europeana.set.definitions.model.agent.Agent; +import eu.europeana.set.client.json.AgentDeserializer; import eu.europeana.set.definitions.model.vocabulary.ProfileConstants; import eu.europeana.set.definitions.model.vocabulary.WebUserSetFields; -import jakarta.ws.rs.core.UriBuilder; import org.apache.commons.lang3.StringUtils; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.ParseException; import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.net.URIBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -54,6 +60,16 @@ public BaseApiConnection(String setServiceUri, String apiKey, String regularUser this.setServiceUri = setServiceUri; this.apiKey = apiKey; this.regularUserAuthorizationValue = regularUserAuthorizationValue; + + // set object mapper + SimpleModule module = new SimpleModule(); + mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + module.addDeserializer(Agent.class, new AgentDeserializer()); + module.addDeserializer(UserSet.class, new UserSetDeserializer()); + module.addDeserializer(RecordPreview.class, new RecordPreviewDeserializer()); + + mapper.registerModule(module); + mapper.findAndRegisterModules(); } public HttpConnection getHttpConnection() { @@ -143,7 +159,7 @@ protected String deleteURL(String url, String authorizationHeaderValue) throws S private UserSet parseSetApiResponse(CloseableHttpResponse response, List statusToCheckList) throws SetApiClientException, IOException, ParseException { String responseBody = EntityUtils.toString(response.getEntity()); if (statusToCheckList.contains(response.getCode())) { - return mapper.readValue(responseBody, BaseUserSet.class); + return mapper.readValue(responseBody, UserSet.class); } else { AbstractUserSetApiResponse errorResponse = mapper.readValue(responseBody, AbstractUserSetApiResponse.class); if (LOGGER.isDebugEnabled()) { @@ -171,15 +187,15 @@ protected List getUserSetPaginatedResponse(String url, String aut CloseableHttpResponse response = getHttpConnection().get(url, ContentType.APPLICATION_JSON.getMimeType(), authorizationHeaderValue); String responseBody = EntityUtils.toString(response.getEntity()); if (response.getCode() == HttpStatus.SC_OK) { - if (StringUtils.equals(profile, ProfileConstants.VALUE_PARAM_ITEMS)) { - TypeReference> typeRef = new TypeReference<>() {}; - List recordIds = mapper.readValue(responseBody, typeRef).getItems(); - List records = new ArrayList<>(); - for (String id: recordIds) { - records.add(new RecordPreview(id)); - } - return records; - } +// if (StringUtils.equals(profile, ProfileConstants.VALUE_PARAM_ITEMS)) { +// TypeReference> typeRef = new TypeReference<>() {}; +// List recordIds = mapper.readValue(responseBody, typeRef).getItems(); +// List records = new ArrayList<>(); +// for (String id: recordIds) { +// records.add(new RecordPreview(id)); +// } +// return records; +// } TypeReference> typeRef = new TypeReference<>() {}; return mapper.readValue(responseBody, typeRef).getItems(); @@ -199,9 +215,6 @@ protected List getUserSetPaginatedResponse(String url, String aut /** * Fetches the user Set search api response * - * Check profile for the deserialization - * META - empty page (no items) , ITEMS_META - only set descriptions, not item descriptions , ITEMS - items set as set ids - * ITEMS_META is the default profile value * @param url * @param authorizationHeaderValue * @return @@ -213,18 +226,7 @@ protected List getSearchUserSetResponse(String url, String au CloseableHttpResponse response = getHttpConnection().get(url, "application/json", authorizationHeaderValue); String responseBody = EntityUtils.toString(response.getEntity()); if (response.getCode() == HttpStatus.SC_OK) { - if (StringUtils.equals(profile, ProfileConstants.VALUE_PARAM_ITEMS)) { - TypeReference> typeRef = new TypeReference<>() {}; - List setIds = mapper.readValue(responseBody, typeRef).getItems(); - List userSets = new ArrayList<>(); - for (String id: setIds) { - BaseUserSet userSet = new BaseUserSet(); - userSet.setIdentifier(StringUtils.substringAfterLast(id, "/")); - userSets.add(userSet); - } - return userSets; - } - TypeReference> typeRef = new TypeReference<>() {}; + TypeReference> typeRef = new TypeReference<>() {}; return mapper.readValue(responseBody, typeRef).getItems(); } else { @@ -258,12 +260,16 @@ public StringBuilder getUserSetServiceUri() { * @param profile * @return */ - public static URI buildGetUrls(String path, String profile) { - UriBuilder builder = UriBuilder.newInstance().path(path); - if (profile != null) { - builder.queryParam(QUERY_PARAM_PROFILE, profile); + public static URI buildGetUrls(String path, String profile) throws SetApiClientException{ + try { + URIBuilder builder = new URIBuilder(path); + if (profile != null) { + builder.addParameter(QUERY_PARAM_PROFILE, profile); + } + return builder.build(); + } catch (URISyntaxException e) { + throw new SetApiClientException("Error creating Get url for " +path); } - return builder.build(); } /** @@ -277,21 +283,25 @@ public static URI buildGetUrls(String path, String profile) { * @return */ public static URI buildPaginatedGetUrls(String path, String sort, - String sortOrder, int page, int pageSize, String profile) { - UriBuilder builder = UriBuilder.newInstance().path(path) - .queryParam(QUERY_PARAM_PAGE, page) - .queryParam(QUERY_PARAM_PAGE_SIZE, pageSize); + String sortOrder, String page, String pageSize, String profile) throws SetApiClientException { + try { + URIBuilder builder = new URIBuilder(path) + .addParameter(QUERY_PARAM_PAGE, page) + .addParameter(QUERY_PARAM_PAGE_SIZE, pageSize); - if (sort != null) { - builder.queryParam(QUERY_PARAM_SORT, sort); - } - if (sortOrder != null) { - builder.queryParam(PARAM_SORT_ORDER, sortOrder); - } - if (profile != null) { - builder.queryParam(QUERY_PARAM_PROFILE, profile); + if (sort != null) { + builder.addParameter(QUERY_PARAM_SORT, sort); + } + if (sortOrder != null) { + builder.addParameter(PARAM_SORT_ORDER, sortOrder); + } + if (profile != null) { + builder.addParameter(QUERY_PARAM_PROFILE, profile); + } + return builder.build(); + } catch (URISyntaxException e) { + throw new SetApiClientException("Error creating Paginated Get Urls url for " +path); } - return builder.build(); } /** @@ -307,27 +317,32 @@ public static URI buildPaginatedGetUrls(String path, String sort, * @param profile * @return search url */ - public static URI buildSearchUrl(String query, String[] qf, String sort, int page, - int pageSize, String facet, int facetLimit, - String profile) { - UriBuilder builder = UriBuilder.newInstance().path(SEARCH_PATH) - .queryParam(QUERY_PARAM_QUERY, query) - .queryParam(QUERY_PARAM_PAGE, page) - .queryParam(QUERY_PARAM_PAGE_SIZE, pageSize); - if (qf != null) { - builder.queryParam(QUERY_PARAM_QF, qf); - } - if (sort != null) { - builder.queryParam(QUERY_PARAM_SORT, sort); - } - if (facet != null) { - builder.queryParam(QUERY_PARAM_FACET, facet); - builder.queryParam("facet.limit", facetLimit); - } - if (profile != null) { - builder.queryParam(QUERY_PARAM_PROFILE, profile); + public static URI buildSearchUrl(String query, String[] qf, String sort, String page, + String pageSize, String facet, int facetLimit, + String profile) throws SetApiClientException { + try { + URIBuilder builder = new URIBuilder(SEARCH_PATH) + .addParameter(QUERY_PARAM_QUERY, query) + .addParameter(QUERY_PARAM_PAGE, page) + .addParameter(QUERY_PARAM_PAGE_SIZE, pageSize); + + if (qf != null) { + builder.addParameter(QUERY_PARAM_QF, String.valueOf(qf)); + } + if (sort != null) { + builder.addParameter(QUERY_PARAM_SORT, sort); + } + if (facet != null) { + builder.addParameter(QUERY_PARAM_FACET, facet); + builder.addParameter("facet.limit", String.valueOf(facetLimit)); + } + if (profile != null) { + builder.addParameter(QUERY_PARAM_PROFILE, profile); + } + return builder.build(); + } catch (URISyntaxException e) { + throw new SetApiClientException("Error creating Search Urls url for " +SEARCH_PATH); } - return builder.build(); } public String getApiKey() { diff --git a/set-client/src/main/java/eu/europeana/set/client/connection/UserSetApiConnection.java b/set-client/src/main/java/eu/europeana/set/client/connection/UserSetApiConnection.java index cadce5b9..8dc2382d 100644 --- a/set-client/src/main/java/eu/europeana/set/client/connection/UserSetApiConnection.java +++ b/set-client/src/main/java/eu/europeana/set/client/connection/UserSetApiConnection.java @@ -103,7 +103,7 @@ public String deleteUserSet(String identifier) throws SetApiClientException { * @throws SetApiClientException */ public List getPaginationUserSet(String identifier, String sort, - String sortOrder, int page, int pageSize, String profile) throws SetApiClientException { + String sortOrder, String page, String pageSize, String profile) throws SetApiClientException { StringBuilder urlBuilder = getUserSetServiceUri().append( buildPaginatedGetUrls(identifier + WebUserSetFields.JSON_LD_REST, sort, sortOrder, page, pageSize, profile)); return getUserSetPaginatedResponse(urlBuilder.toString(), regularUserAuthorizationValue, profile); @@ -123,8 +123,8 @@ public List getPaginationUserSet(String identifier, String sort, * @return * @throws IOException */ - public List searchUserSet(String query, String[] qf, String sort, int page, - int pageSize, String facet, int facetLimit, + public List searchUserSet(String query, String[] qf, String sort, String page, + String pageSize, String facet, int facetLimit, String profile) throws SetApiClientException { StringBuilder urlBuilder = getUserSetServiceUri().append(buildSearchUrl(query, qf, sort, page, pageSize, facet, facetLimit, profile)); diff --git a/set-definitions/src/main/java/eu/europeana/set/definitions/json/AgentDeserializer.java b/set-client/src/main/java/eu/europeana/set/client/json/AgentDeserializer.java similarity index 89% rename from set-definitions/src/main/java/eu/europeana/set/definitions/json/AgentDeserializer.java rename to set-client/src/main/java/eu/europeana/set/client/json/AgentDeserializer.java index d01e9025..688c9238 100644 --- a/set-definitions/src/main/java/eu/europeana/set/definitions/json/AgentDeserializer.java +++ b/set-client/src/main/java/eu/europeana/set/client/json/AgentDeserializer.java @@ -1,6 +1,5 @@ -package eu.europeana.set.definitions.json; +package eu.europeana.set.client.json; -import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; @@ -17,7 +16,7 @@ public class AgentDeserializer extends JsonDeserializer { @Override - public Agent deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException, JacksonException { + public Agent deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec(); ObjectNode root = mapper.readTree(jsonParser); diff --git a/set-client/src/main/java/eu/europeana/set/client/json/UserSetDeserializer.java b/set-client/src/main/java/eu/europeana/set/client/json/UserSetDeserializer.java new file mode 100644 index 00000000..274db2be --- /dev/null +++ b/set-client/src/main/java/eu/europeana/set/client/json/UserSetDeserializer.java @@ -0,0 +1,51 @@ +package eu.europeana.set.client.json; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import eu.europeana.set.definitions.model.UserSet; +import eu.europeana.set.definitions.model.impl.BaseUserSet; +import org.apache.commons.lang3.StringUtils; + +import java.io.IOException; + +/** + * Used for Deserializing UserSet. + *

+ * Field - {@link UserSet#getIdentifier()} + * As Identifier is always null, we fetch the value from the 'id' field in the response + * exmaple : 'id' : http://data.europeana.eu/set/xyz , identifier : xyz + * + * @author srishti singh + * @since 9 December 2024 + */ +public class UserSetDeserializer extends JsonDeserializer { + + @Override + public UserSet deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { + ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec(); + if (mapper.readTree(jsonParser).isObject()) { + ObjectNode root = mapper.readTree(jsonParser); + BaseUserSet set = mapper.readValue(root.toString(), BaseUserSet.class); + if (root.has("id")) { + String id = root.get("id").asText(); + String identifier = set.getIdentifier(); + if (identifier == null) { + set.setIdentifier(StringUtils.substringAfterLast(id, "/")); + } + } + return set; + } else { + // there are profiles where only id value is returned instead of UserSet object + // hence we will form a userset object with 'id' value and return that + JsonNode root = mapper.readTree(jsonParser); + String id = mapper.readValue(root.toString(), String.class); + UserSet set = new BaseUserSet(); + set.setIdentifier(StringUtils.substringAfterLast(id, "/")); + return set; + } + } +} diff --git a/set-client/src/main/java/eu/europeana/set/client/web/SearchUserSetApi.java b/set-client/src/main/java/eu/europeana/set/client/web/SearchUserSetApi.java index b5464321..cd8ebd54 100644 --- a/set-client/src/main/java/eu/europeana/set/client/web/SearchUserSetApi.java +++ b/set-client/src/main/java/eu/europeana/set/client/web/SearchUserSetApi.java @@ -24,6 +24,6 @@ public interface SearchUserSetApi { * @param profile * @return */ - List searchUserSet(String query, String[] qf, String sort, int page, int pageSize, + List searchUserSet(String query, String[] qf, String sort, String page, String pageSize, String facet, int facetLimit, String profile) throws SetApiClientException; } diff --git a/set-client/src/main/java/eu/europeana/set/client/web/WebUserSetApi.java b/set-client/src/main/java/eu/europeana/set/client/web/WebUserSetApi.java index ca3e073d..56bb6a63 100644 --- a/set-client/src/main/java/eu/europeana/set/client/web/WebUserSetApi.java +++ b/set-client/src/main/java/eu/europeana/set/client/web/WebUserSetApi.java @@ -58,5 +58,5 @@ String deleteUserSet( * @return * @throws SetApiClientException */ - List getPaginationUserSet(String identifier, String sort, String sortOrder, int page, int pageSize, String profile) throws SetApiClientException; + List getPaginationUserSet(String identifier, String sort, String sortOrder, String page, String pageSize, String profile) throws SetApiClientException; } \ No newline at end of file diff --git a/set-definitions/pom.xml b/set-definitions/pom.xml index 728162b8..b159f6b3 100644 --- a/set-definitions/pom.xml +++ b/set-definitions/pom.xml @@ -29,19 +29,5 @@ commons-definitions ${version.commonsApi} - - - - com.fasterxml.jackson.core - jackson-annotations - ${jackson.version} - - - - com.fasterxml.jackson.core - jackson-databind - ${jackson.version} - diff --git a/set-definitions/src/main/java/eu/europeana/set/definitions/model/BaseWebResource.java b/set-definitions/src/main/java/eu/europeana/set/definitions/model/BaseWebResource.java index 19d9b0cd..932b8eba 100644 --- a/set-definitions/src/main/java/eu/europeana/set/definitions/model/BaseWebResource.java +++ b/set-definitions/src/main/java/eu/europeana/set/definitions/model/BaseWebResource.java @@ -1,10 +1,7 @@ package eu.europeana.set.definitions.model; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; - import java.util.Objects; -@JsonIgnoreProperties(ignoreUnknown = true) public class BaseWebResource { public static final String TYPE = "WebResource"; diff --git a/set-definitions/src/main/java/eu/europeana/set/definitions/model/agent/impl/BaseAgent.java b/set-definitions/src/main/java/eu/europeana/set/definitions/model/agent/impl/BaseAgent.java index 829f8b7d..29ebba22 100644 --- a/set-definitions/src/main/java/eu/europeana/set/definitions/model/agent/impl/BaseAgent.java +++ b/set-definitions/src/main/java/eu/europeana/set/definitions/model/agent/impl/BaseAgent.java @@ -1,10 +1,8 @@ package eu.europeana.set.definitions.model.agent.impl; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import eu.europeana.set.definitions.model.agent.Agent; import eu.europeana.set.definitions.model.vocabulary.AgentTypes; -@JsonIgnoreProperties(ignoreUnknown = true) public abstract class BaseAgent implements Agent { private String httpUrl; diff --git a/set-definitions/src/main/java/eu/europeana/set/definitions/model/impl/BaseUserSet.java b/set-definitions/src/main/java/eu/europeana/set/definitions/model/impl/BaseUserSet.java index 05b5951d..12d51e94 100644 --- a/set-definitions/src/main/java/eu/europeana/set/definitions/model/impl/BaseUserSet.java +++ b/set-definitions/src/main/java/eu/europeana/set/definitions/model/impl/BaseUserSet.java @@ -4,26 +4,18 @@ import java.util.List; import java.util.Map; -import com.fasterxml.jackson.annotation.JsonGetter; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import eu.europeana.set.definitions.json.AgentDeserializer; import eu.europeana.set.definitions.model.BaseWebResource; import eu.europeana.set.definitions.model.UserSet; import eu.europeana.set.definitions.model.agent.Agent; import eu.europeana.set.definitions.model.vocabulary.UserSetTypes; import eu.europeana.set.definitions.model.vocabulary.VisibilityTypes; import eu.europeana.set.definitions.model.vocabulary.WebUserSetModelFields; -import org.apache.commons.lang3.StringUtils; /** * Europeana Sets API Specification * - * @JsonIgnoreProperties - to ignore "@context" while parsing the results in the Client side code - * - * @author GrafR Modified by Srishti Singh 2-2-2021 + * @author GrafR */ -@JsonIgnoreProperties(ignoreUnknown = true) public class BaseUserSet extends BasePageInfo implements UserSet { // EDM Collection Profile @@ -82,11 +74,6 @@ public BaseUserSet() { // Provenance information - /** - * A reference to the user agent that gathers objects together following - * implicit or explicit criteria or accrual policy. - */ - @JsonDeserialize(using = AgentDeserializer.class) private Agent creator; /** @@ -119,23 +106,7 @@ public BaseUserSet() { private Provider provider; - /** - * For deserialization - - * Adding @JsonGetter, as this field is ignored in the json responses. - * Will build the identifier value from the field "id". - * - * Only if identifier is present and has a baseUrl (which means it is fetched via ID, during deserialization) - * In other cases, return the actual identifier value - * - * exmaple : 'id' : http://data.europeana.eu/set/xyz , identifier : xyz - * - * @return identifier of the set - */ - @JsonGetter(WebUserSetModelFields.ID) public String getIdentifier() { - if (identifier != null && StringUtils.contains(identifier, "/")) { - return StringUtils.substringAfterLast(identifier, "/"); - } return identifier; }