From 4586e78bf12932abb28af92a7eea31e37ed0ae42 Mon Sep 17 00:00:00 2001 From: GordeaS Date: Mon, 18 Mar 2024 16:36:05 +0100 Subject: [PATCH] addressing code quality issues --- pom.xml | 6 -- set-client/pom.xml | 4 -- .../service/PersistentUserSetServiceImpl.java | 60 ++++++++----------- .../europeana/set/web/config/BeanNames.java | 4 ++ .../europeana/set/web/config/MongoConfig.java | 6 +- .../set/web/config/SpringDocConfig.java | 11 +++- .../set/web/config/UserSetAutoConfig.java | 12 +--- .../exception/GlobalExceptionHandler.java | 4 ++ .../exception/UserSetErrorController.java | 5 ++ 9 files changed, 53 insertions(+), 59 deletions(-) diff --git a/pom.xml b/pom.xml index 3f07c404..eb66fcea 100644 --- a/pom.xml +++ b/pom.xml @@ -332,12 +332,6 @@ org.apache.maven.plugins maven-compiler-plugin 3.10.1 - org.apache.maven.plugins diff --git a/set-client/pom.xml b/set-client/pom.xml index ddec4642..a1ce9370 100644 --- a/set-client/pom.xml +++ b/set-client/pom.xml @@ -94,10 +94,6 @@ org.apache.maven.plugins maven-surefire-plugin - - diff --git a/set-mongo/src/main/java/eu/europeana/set/mongo/service/PersistentUserSetServiceImpl.java b/set-mongo/src/main/java/eu/europeana/set/mongo/service/PersistentUserSetServiceImpl.java index 417b749d..8a3fb1f3 100644 --- a/set-mongo/src/main/java/eu/europeana/set/mongo/service/PersistentUserSetServiceImpl.java +++ b/set-mongo/src/main/java/eu/europeana/set/mongo/service/PersistentUserSetServiceImpl.java @@ -118,14 +118,11 @@ public PersistentUserSet getByIdentifier(String identifier) { public long getDistinct(String field, boolean fieldIsArray, String collectionType) throws UserSetServiceException { long count = 0; - // Cursor is needed in aggregate command -// AggregationOptions aggregationOptions = -// AggregationOptions.builder().outputMode(AggregationOptions. OutputMode.CURSOR).build(); AggregationOptions aggregationOptions = - AggregationOptions.builder().allowDiskUse(true).build(); + AggregationOptions.builder().allowDiskUse(Boolean.TRUE).build(); Cursor cursor = getDao().getCollection().aggregate( getDistinctCountPipeline(field, fieldIsArray, collectionType), aggregationOptions); - if (cursor != null && cursor.hasNext()) { + if (cursor.hasNext()) { // ideally there should be only one value present. count = Long.parseLong(cursor.next().get(UserSetMongoConstants.MONGO_FIELD_COUNT).toString()); @@ -141,18 +138,16 @@ public long getDistinct(String field, boolean fieldIsArray, String collectionTyp @Override public long countItemsInEntitySets() { - // Cursor is needed in aggregate command -// AggregationOptions aggregationOptions = -// AggregationOptions.builder().outputMode(AggregationOptions.OutputMode.CURSOR).build(); AggregationOptions aggregationOptions = - AggregationOptions.builder().allowDiskUse(true).build(); + AggregationOptions.builder().allowDiskUse(Boolean.TRUE).build(); long totalItems = 0; Map groupFieldsAdditional = new ConcurrentHashMap<>(); groupFieldsAdditional.put(UserSetMongoConstants.MONGO_FIELD_COUNT, new BasicDBObject(UserSetMongoConstants.MONGO_SUM, UserSetMongoConstants.MONGO_TOTAL)); Cursor cursor = getDao().getCollection().aggregate( - getAggregatePipeline(UserSetTypes.ENTITYBESTITEMSSET.getJsonValue(), groupFieldsAdditional), aggregationOptions); - if (cursor != null && cursor.hasNext()) { + getAggregatePipeline(UserSetTypes.ENTITYBESTITEMSSET.getJsonValue(), groupFieldsAdditional), + aggregationOptions); + if (cursor.hasNext()) { totalItems = Long.parseLong(cursor.next().get(UserSetMongoConstants.MONGO_FIELD_COUNT).toString()); } @@ -274,21 +269,19 @@ public long count(UserSetQuery query) { public Map getFacets(UserSetFacetQuery facetQuery) { Map valueCountMap = new LinkedHashMap<>(); // Cursor is needed in aggregate command - AggregationOptions aggregationOptions = - AggregationOptions.builder().allowDiskUse(null).build(); + AggregationOptions aggregationOptions = AggregationOptions.builder().allowDiskUse(null).build(); Cursor cursor = getDao().getCollection().aggregate(getFacetPipeline(facetQuery), aggregationOptions); - if (cursor != null) { - while (cursor.hasNext()) { - DBObject object = cursor.next(); - @SuppressWarnings("unchecked") - List facet = (List) object.get(facetQuery.getOutputField()); - for (DBObject o : facet) { - valueCountMap.put(String.valueOf(o.get(UserSetMongoConstants.MONGO_ID)), - Long.parseLong(o.get(UserSetMongoConstants.MONGO_FIELD_COUNT).toString())); - } + while (cursor.hasNext()) { + DBObject object = cursor.next(); + @SuppressWarnings("unchecked") + List facet = (List) object.get(facetQuery.getOutputField()); + for (DBObject o : facet) { + valueCountMap.put(String.valueOf(o.get(UserSetMongoConstants.MONGO_ID)), + Long.parseLong(o.get(UserSetMongoConstants.MONGO_FIELD_COUNT).toString())); } } + return valueCountMap; } @@ -341,7 +334,7 @@ private DBObject getOutputFieldAndStages(UserSetFacetQuery facetQuery) { public long countTotalLikes() { // Cursor is needed in aggregate command AggregationOptions aggregationOptions = - AggregationOptions.builder().allowDiskUse(true).build(); + AggregationOptions.builder().allowDiskUse(Boolean.TRUE).build(); long totalLikes = 0; Map groupFieldsAdditional = new ConcurrentHashMap<>(); @@ -350,13 +343,12 @@ public long countTotalLikes() { Cursor cursor = getDao().getCollection().aggregate( getAggregatePipeline(UserSetTypes.BOOKMARKSFOLDER.getJsonValue(), groupFieldsAdditional), aggregationOptions); - if (cursor != null) { - while (cursor.hasNext()) { - DBObject object = cursor.next(); - totalLikes += - Long.parseLong(String.valueOf(object.get(UserSetMongoConstants.MONGO_TOTAL_LIKES))); - } + while (cursor.hasNext()) { + DBObject object = cursor.next(); + totalLikes += + Long.parseLong(String.valueOf(object.get(UserSetMongoConstants.MONGO_TOTAL_LIKES))); } + return totalLikes; } @@ -423,10 +415,10 @@ private void setPaginationOptions(Query mongoQuery, UserSetQu private Query buildMongoQuery(UserSetQuery query) { Query searchQuery = buildUserConditionsQuery(query); - + setPaginationOptions(searchQuery, query); setOrder(searchQuery, query); - + if (query.isAdmin()) { // admin can see all return searchQuery; @@ -445,7 +437,7 @@ private Query buildMongoQuery(UserSetQuery query) { // private only, user can see only his private sets searchQuery.filter(FIELD_CREATOR, query.getUser()); } - + return searchQuery; } @@ -502,13 +494,13 @@ private void setOrder(Query mongoQuery, UserSetQuery query) { mongoQuery.order(Sort.descending(WebUserSetModelFields.MODIFIED)); return; } - + for (String sortField : query.getSortCriteria()) { // check the score sort first (it can only be in desc order) if (sortField.contains(WebUserSetFields.TEXT_SCORE_SORT)) { mongoQuery.order(Meta.textScore()); mongoQuery.order(Sort.ascending(UserSetMongoConstants.MONGO_ID)); - + } else { if (!sortField.contains(" ")) { mongoQuery.order(Sort.ascending(sortField)); diff --git a/set-web/src/main/java/eu/europeana/set/web/config/BeanNames.java b/set-web/src/main/java/eu/europeana/set/web/config/BeanNames.java index b363e7e9..fd38ccec 100644 --- a/set-web/src/main/java/eu/europeana/set/web/config/BeanNames.java +++ b/set-web/src/main/java/eu/europeana/set/web/config/BeanNames.java @@ -1,9 +1,13 @@ package eu.europeana.set.web.config; +/** + * Abstract class to keep an inventory of bean names + */ public abstract class BeanNames { public static final String BEAN_SET_MONGO_STORE = "set_db_morphia_datastore_set"; public static final String BEAN_SET_PERSITENCE_SERVICE = "set_db_setService"; public static final String BEAN_I18N_SERVICE = "i18nService"; + private BeanNames() {} } diff --git a/set-web/src/main/java/eu/europeana/set/web/config/MongoConfig.java b/set-web/src/main/java/eu/europeana/set/web/config/MongoConfig.java index f21ea841..9bc3a20c 100644 --- a/set-web/src/main/java/eu/europeana/set/web/config/MongoConfig.java +++ b/set-web/src/main/java/eu/europeana/set/web/config/MongoConfig.java @@ -24,13 +24,13 @@ ignoreResourceNotFound = true) public class MongoConfig { - @Value("${mongodb.set.connectionUrl:}") + @Value("${mongodb.set.connectionUrl:''}") private String mongoConnectionUrl; - @Value("${mongodb.set.truststore:}") + @Value("${mongodb.set.truststore:''}") private String mongoTrustStore; - @Value("${mongodb.set.truststorepass:}") + @Value("${mongodb.set.truststorepass:''}") private String mongoTrustStorePass; private static final String[] MODEL_PACKAGES = new String[]{"eu.europeana.set.definitions", "eu.europeana.api.commons.nosql.entity"}; diff --git a/set-web/src/main/java/eu/europeana/set/web/config/SpringDocConfig.java b/set-web/src/main/java/eu/europeana/set/web/config/SpringDocConfig.java index 5f4c4404..bd79c290 100644 --- a/set-web/src/main/java/eu/europeana/set/web/config/SpringDocConfig.java +++ b/set-web/src/main/java/eu/europeana/set/web/config/SpringDocConfig.java @@ -10,22 +10,27 @@ import io.swagger.v3.oas.models.info.Info; import io.swagger.v3.oas.models.info.License; +/** + * Class for configuration of SpringDoc + */ @Configuration @OpenAPIDefinition public class SpringDocConfig { private final BuildProperties buildProperties; - -// private final BuildProperties buildInfo; /** * Initialize SpringDoc with API build information - * @param buildInfo object for retrieving build information + * @param buildProperties object for retrieving build information */ public SpringDocConfig(BuildProperties buildProperties) { this.buildProperties = buildProperties; } + /** + * create OpenAPI bean with required information + * @return the opeApi bean + */ @Bean public OpenAPI userServiceOpenAPI() { return new OpenAPI().info(new Info().title(buildProperties.getName()) diff --git a/set-web/src/main/java/eu/europeana/set/web/config/UserSetAutoConfig.java b/set-web/src/main/java/eu/europeana/set/web/config/UserSetAutoConfig.java index 22e53eaf..8126092f 100644 --- a/set-web/src/main/java/eu/europeana/set/web/config/UserSetAutoConfig.java +++ b/set-web/src/main/java/eu/europeana/set/web/config/UserSetAutoConfig.java @@ -2,17 +2,11 @@ import org.springframework.context.annotation.Configuration; +/** + * CLass to autoconfigure application by instantiating required beans + */ @Configuration() public class UserSetAutoConfig{ -// @Bean("messageSource") -// public MessageSource getMessageSource() { -// ReloadableResourceBundleMessageSource messageSource = -// new ReloadableResourceBundleMessageSource(); -// messageSource.setBasename("classpath:messages"); -// messageSource.setDefaultEncoding("utf-8"); -// messageSource.setDefaultLocale(Locale.ENGLISH); -// return messageSource; -// } } diff --git a/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/GlobalExceptionHandler.java b/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/GlobalExceptionHandler.java index 66267eb8..29970799 100644 --- a/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/GlobalExceptionHandler.java +++ b/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/GlobalExceptionHandler.java @@ -14,11 +14,15 @@ import eu.europeana.set.web.config.BeanNames; import eu.europeana.set.web.service.RequestPathMethodService; +/** + * Controller for handling application exceptions + */ @ControllerAdvice public class GlobalExceptionHandler extends EuropeanaGlobalExceptionHandler { I18nService i18nService; + @Override protected I18nService getI18nService() { return i18nService; } diff --git a/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/UserSetErrorController.java b/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/UserSetErrorController.java index edabef49..5a2e95fc 100644 --- a/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/UserSetErrorController.java +++ b/set-web/src/main/java/eu/europeana/set/web/service/controller/exception/UserSetErrorController.java @@ -21,6 +21,11 @@ public UserSetErrorController(ErrorAttributes errorAttributes) { } + /** + * handle /error requests + * @param request the request object + * @return ,app with default error attributes + */ @GetMapping(value = "/error", produces = {HttpHeaders.CONTENT_TYPE_JSON_UTF8, HttpHeaders.CONTENT_TYPE_JSONLD}) @ResponseBody public Map error(final HttpServletRequest request) {