From 159ea9047af352773f18a93f5da820f91020409f Mon Sep 17 00:00:00 2001 From: GordeaS Date: Mon, 13 Nov 2023 11:07:47 +0100 Subject: [PATCH] addressing code quality issues #EA-3597 --- .../config/TranslationApiAutoconfig.java | 19 +++++++----- ...ranslation.java => CachedTranslation.java} | 7 ++++- .../translation/model/TranslationRequest.java | 9 ++++++ .../serialization/JsonRedisSerializer.java | 17 ++++++++--- .../web/service/RedisCacheService.java | 20 ++++++------- .../web/service/TranslationWebService.java | 29 ++++++++++++++----- 6 files changed, 70 insertions(+), 31 deletions(-) rename translation-web/src/main/java/eu/europeana/api/translation/model/{RedisCacheTranslation.java => CachedTranslation.java} (78%) diff --git a/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationApiAutoconfig.java b/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationApiAutoconfig.java index bc9fc5cf..04251553 100644 --- a/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationApiAutoconfig.java +++ b/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationApiAutoconfig.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Locale; +import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -22,6 +23,7 @@ import org.springframework.context.annotation.PropertySource; import org.springframework.context.support.ReloadableResourceBundleMessageSource; import org.springframework.data.redis.connection.RedisConfiguration; +import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.data.redis.connection.lettuce.LettuceClientConfiguration; import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; import org.springframework.data.redis.core.RedisTemplate; @@ -29,7 +31,7 @@ import eu.europeana.api.commons.config.i18n.I18nService; import eu.europeana.api.commons.config.i18n.I18nServiceImpl; import eu.europeana.api.commons.oauth2.service.impl.EuropeanaClientDetailsService; -import eu.europeana.api.translation.model.RedisCacheTranslation; +import eu.europeana.api.translation.model.CachedTranslation; import eu.europeana.api.translation.serialization.JsonRedisSerializer; import eu.europeana.api.translation.service.exception.LangDetectionServiceConfigurationException; import eu.europeana.api.translation.service.exception.TranslationServiceConfigurationException; @@ -155,15 +157,16 @@ public TranslationServiceProvider getTranslationServiceProvider() { * bean creation. Otherwise all these methods would need to be called manually which is not the * best solution. */ - private LettuceConnectionFactory getRedisConnectionFactory() throws IOException { + private LettuceConnectionFactory getRedisConnectionFactory() { // in case of integration tests, we do not need the SSL certificate LettuceClientConfiguration.LettuceClientConfigurationBuilder lettuceClientConfigurationBuilder = LettuceClientConfiguration.builder(); // if redis secure protocol is used (rediss vs. redis) boolean sslEnabled = translationConfig.getRedisConnectionUrl().startsWith("rediss"); if (sslEnabled) { + final File truststore = new File(FilenameUtils.normalize(translationConfig.getTruststorePath())); SslOptions sslOptions = SslOptions.builder().jdkSslProvider() - .truststore(new File(translationConfig.getTruststorePath()), + .truststore(truststore, translationConfig.getTruststorePass()) .build(); @@ -180,9 +183,9 @@ private LettuceConnectionFactory getRedisConnectionFactory() throws IOException return new LettuceConnectionFactory(redisConf, lettuceClientConfiguration); } - private RedisTemplate getRedisTemplate( - LettuceConnectionFactory redisConnectionFactory) throws IOException { - RedisTemplate redisTemplate = new RedisTemplate<>(); + private RedisTemplate getRedisTemplate( + RedisConnectionFactory redisConnectionFactory) { + RedisTemplate redisTemplate = new RedisTemplate<>(); redisTemplate.setConnectionFactory(redisConnectionFactory); redisTemplate.setKeySerializer(new StringRedisSerializer()); redisTemplate.setValueSerializer(new JsonRedisSerializer()); @@ -192,10 +195,10 @@ private RedisTemplate getRedisTemplate( @Bean(BeanNames.BEAN_REDIS_CACHE_SERVICE) @ConditionalOnProperty(name = "redis.connection.url") - public RedisCacheService getRedisCacheService() throws IOException { + public RedisCacheService getRedisCacheService() { LettuceConnectionFactory redisConnectionFactory = getRedisConnectionFactory(); redisConnectionFactory.afterPropertiesSet(); - RedisTemplate redisTemplate = + RedisTemplate redisTemplate = getRedisTemplate(redisConnectionFactory); return new RedisCacheService(redisTemplate); diff --git a/translation-web/src/main/java/eu/europeana/api/translation/model/RedisCacheTranslation.java b/translation-web/src/main/java/eu/europeana/api/translation/model/CachedTranslation.java similarity index 78% rename from translation-web/src/main/java/eu/europeana/api/translation/model/RedisCacheTranslation.java rename to translation-web/src/main/java/eu/europeana/api/translation/model/CachedTranslation.java index e65d3f3f..93c38cf0 100644 --- a/translation-web/src/main/java/eu/europeana/api/translation/model/RedisCacheTranslation.java +++ b/translation-web/src/main/java/eu/europeana/api/translation/model/CachedTranslation.java @@ -1,6 +1,11 @@ package eu.europeana.api.translation.model; -public class RedisCacheTranslation { +/** + * object model for the cached translations + * @author GordeaS + * + */ +public class CachedTranslation { private String original; private String translation; public String getOriginal() { diff --git a/translation-web/src/main/java/eu/europeana/api/translation/model/TranslationRequest.java b/translation-web/src/main/java/eu/europeana/api/translation/model/TranslationRequest.java index 59a27c1c..61e421cd 100644 --- a/translation-web/src/main/java/eu/europeana/api/translation/model/TranslationRequest.java +++ b/translation-web/src/main/java/eu/europeana/api/translation/model/TranslationRequest.java @@ -8,6 +8,11 @@ @JsonInclude(value = JsonInclude.Include.NON_EMPTY) @JsonIgnoreProperties(ignoreUnknown = true) +/** + * The representation of translation request body + * @author GordeaS + * + */ public class TranslationRequest { private String source; @@ -67,6 +72,10 @@ public void setText(List text) { this.text = text; } + /** + * Utility method indicating if the caching service should be used for this request + * @return true if request should be served with the caching service, caching is true by default + */ public boolean useCaching() { return caching; } diff --git a/translation-web/src/main/java/eu/europeana/api/translation/serialization/JsonRedisSerializer.java b/translation-web/src/main/java/eu/europeana/api/translation/serialization/JsonRedisSerializer.java index 3e48a079..74ef586a 100644 --- a/translation-web/src/main/java/eu/europeana/api/translation/serialization/JsonRedisSerializer.java +++ b/translation-web/src/main/java/eu/europeana/api/translation/serialization/JsonRedisSerializer.java @@ -1,14 +1,23 @@ package eu.europeana.api.translation.serialization; +import java.io.IOException; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.SerializationException; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import eu.europeana.api.translation.model.RedisCacheTranslation; +import eu.europeana.api.translation.model.CachedTranslation; +/** + * Json Serializer for objects managed with redis cache + * @author GordeaS + * + */ public class JsonRedisSerializer implements RedisSerializer { private final ObjectMapper om; + /** + * Default constructor initializing the json object mapper + */ public JsonRedisSerializer() { this.om = new ObjectMapper(); } @@ -23,13 +32,13 @@ public byte[] serialize(Object t) throws SerializationException { } @Override - public RedisCacheTranslation deserialize(byte[] bytes) throws SerializationException { + public CachedTranslation deserialize(byte[] bytes) throws SerializationException { if(bytes == null){ return null; } try { - return om.readValue(bytes, RedisCacheTranslation.class); - } catch (Exception e) { + return om.readValue(bytes, CachedTranslation.class); + } catch (IOException e) { throw new SerializationException(e.getMessage(), e); } } diff --git a/translation-web/src/main/java/eu/europeana/api/translation/web/service/RedisCacheService.java b/translation-web/src/main/java/eu/europeana/api/translation/web/service/RedisCacheService.java index 0a69c98e..6e69f0f6 100644 --- a/translation-web/src/main/java/eu/europeana/api/translation/web/service/RedisCacheService.java +++ b/translation-web/src/main/java/eu/europeana/api/translation/web/service/RedisCacheService.java @@ -6,17 +6,17 @@ import java.util.Map; import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.data.redis.core.RedisTemplate; -import eu.europeana.api.translation.model.RedisCacheTranslation; +import eu.europeana.api.translation.model.CachedTranslation; public class RedisCacheService { - private final RedisTemplate redisTemplate; + private final RedisTemplate redisTemplate; /** * Service for remote invocation of redis caching system * @param redisTemplate the template for communicating with redis system */ - public RedisCacheService(RedisTemplate redisTemplate) { + public RedisCacheService(RedisTemplate redisTemplate) { this.redisTemplate = redisTemplate; } @@ -30,13 +30,13 @@ public RedisCacheService(RedisTemplate redisTempl */ public List getCachedTranslations(String sourceLang, String targetLang, List texts) { List keys = new ArrayList<>(); - texts.stream().forEach(text -> { + for(String text : texts){ keys.add(generateRedisKey(text, sourceLang, targetLang)); - }); + } - List redisResponse = redisTemplate.opsForValue().multiGet(keys); - List resp = new ArrayList(); - for(RedisCacheTranslation respElem : redisResponse) { + List redisResponse = redisTemplate.opsForValue().multiGet(keys); + List resp = new ArrayList<>(); + for(CachedTranslation respElem : redisResponse) { if(respElem!=null) { resp.add(respElem.getTranslation()); } @@ -55,10 +55,10 @@ public List getCachedTranslations(String sourceLang, String targetLang, * @param translations the translations of the input text in the targetLang */ public void saveRedisCache(String sourceLang, String targetLang, List inputText, List translations) { - Map valueMap = new HashMap<>(inputText.size()); + Map valueMap = new HashMap<>(inputText.size()); for(int i=0;i redisResp = redisCacheService.getCachedTranslations(translRequest.getSource(), translRequest.getTarget(), translRequest.getText()); + List redisResp = getRedisCacheService().getCachedTranslations(translRequest.getSource(), translRequest.getTarget(), translRequest.getText()); if(Collections.frequency(redisResp, null)>0) { TranslationRequest newTranslReq = new TranslationRequest(); @@ -97,9 +101,9 @@ private TranslationResponse getCombinedCachedAndTranslatedResults(TranslationReq newTranslReq.setService(translRequest.getService()); newTranslReq.setFallback(translRequest.getFallback()); newTranslReq.setCaching(translRequest.getCaching()); - newTranslReq.setText(new ArrayList(translRequest.getText())); + newTranslReq.setText(new ArrayList<>(translRequest.getText())); - List newText = new ArrayList(); + List newText = new ArrayList<>(); int counter=0; for(String redisRespElem : redisResp) { if(redisRespElem==null) { @@ -111,10 +115,10 @@ private TranslationResponse getCombinedCachedAndTranslatedResults(TranslationReq result = getTranslatedResults(newTranslReq); //save the translations to the cache - redisCacheService.saveRedisCache(newTranslReq.getSource(), newTranslReq.getTarget(), newTranslReq.getText(), result.getTranslations()); + getRedisCacheService().saveRedisCache(newTranslReq.getSource(), newTranslReq.getTarget(), newTranslReq.getText(), result.getTranslations()); //aggregate the redis and translation responses - List finalText=new ArrayList(redisResp); + List finalText=new ArrayList<>(redisResp); int counterTranslated = 0; for(int i=0;i