Skip to content

Commit

Permalink
addressing code quality issues #EA-3597
Browse files Browse the repository at this point in the history
  • Loading branch information
GordeaS authored and GordeaS committed Nov 13, 2023
1 parent 20888ee commit 159ea90
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,14 +23,15 @@
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;
import org.springframework.data.redis.serializer.StringRedisSerializer;
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;
Expand Down Expand Up @@ -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()));

Check warning

Code scanning / SonarCloud

This API (java/io/File.(Ljava/lang/String;)V) reads a file whose location might be specified by user input See more on SonarCloud Warning

This API (java/io/File.(Ljava/lang/String;)V) reads a file whose location might be specified by user input See more on SonarCloud
SslOptions sslOptions = SslOptions.builder().jdkSslProvider()
.truststore(new File(translationConfig.getTruststorePath()),
.truststore(truststore,
translationConfig.getTruststorePass())
.build();

Expand All @@ -180,9 +183,9 @@ private LettuceConnectionFactory getRedisConnectionFactory() throws IOException
return new LettuceConnectionFactory(redisConf, lettuceClientConfiguration);
}

private RedisTemplate<String, RedisCacheTranslation> getRedisTemplate(
LettuceConnectionFactory redisConnectionFactory) throws IOException {
RedisTemplate<String, RedisCacheTranslation> redisTemplate = new RedisTemplate<>();
private RedisTemplate<String, CachedTranslation> getRedisTemplate(
RedisConnectionFactory redisConnectionFactory) {
RedisTemplate<String, CachedTranslation> redisTemplate = new RedisTemplate<>();
redisTemplate.setConnectionFactory(redisConnectionFactory);
redisTemplate.setKeySerializer(new StringRedisSerializer());
redisTemplate.setValueSerializer(new JsonRedisSerializer());
Expand All @@ -192,10 +195,10 @@ private RedisTemplate<String, RedisCacheTranslation> 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<String, RedisCacheTranslation> redisTemplate =
RedisTemplate<String, CachedTranslation> redisTemplate =
getRedisTemplate(redisConnectionFactory);

return new RedisCacheService(redisTemplate);
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,6 +72,10 @@ public void setText(List<String> 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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Object> {
private final ObjectMapper om;

/**
* Default constructor initializing the json object mapper
*/
public JsonRedisSerializer() {
this.om = new ObjectMapper();
}
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, RedisCacheTranslation> redisTemplate;
private final RedisTemplate<String, CachedTranslation> redisTemplate;

/**
* Service for remote invocation of redis caching system
* @param redisTemplate the template for communicating with redis system
*/
public RedisCacheService(RedisTemplate<String, RedisCacheTranslation> redisTemplate) {
public RedisCacheService(RedisTemplate<String, CachedTranslation> redisTemplate) {
this.redisTemplate = redisTemplate;
}

Expand All @@ -30,13 +30,13 @@ public RedisCacheService(RedisTemplate<String, RedisCacheTranslation> redisTempl
*/
public List<String> getCachedTranslations(String sourceLang, String targetLang, List<String> texts) {
List<String> keys = new ArrayList<>();
texts.stream().forEach(text -> {
for(String text : texts){
keys.add(generateRedisKey(text, sourceLang, targetLang));
});
}

List<RedisCacheTranslation> redisResponse = redisTemplate.opsForValue().multiGet(keys);
List<String> resp = new ArrayList<String>();
for(RedisCacheTranslation respElem : redisResponse) {
List<CachedTranslation> redisResponse = redisTemplate.opsForValue().multiGet(keys);
List<String> resp = new ArrayList<>();
for(CachedTranslation respElem : redisResponse) {
if(respElem!=null) {
resp.add(respElem.getTranslation());
}
Expand All @@ -55,10 +55,10 @@ public List<String> getCachedTranslations(String sourceLang, String targetLang,
* @param translations the translations of the input text in the targetLang
*/
public void saveRedisCache(String sourceLang, String targetLang, List<String> inputText, List<String> translations) {
Map<String, RedisCacheTranslation> valueMap = new HashMap<>(inputText.size());
Map<String, CachedTranslation> valueMap = new HashMap<>(inputText.size());
for(int i=0;i<inputText.size();i++) {
String key = generateRedisKey(inputText.get(i), sourceLang, targetLang);
RedisCacheTranslation value = new RedisCacheTranslation();
CachedTranslation value = new CachedTranslation();
value.setOriginal(inputText.get(i));
value.setTranslation(translations.get(i));
valueMap.put(key, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@
public class TranslationWebService extends BaseWebService {

@Autowired
private TranslationServiceProvider translationServiceProvider;
private final TranslationServiceProvider translationServiceProvider;

@Autowired(required = false)
private RedisCacheService redisCacheService;

private final Logger logger = LogManager.getLogger(getClass());

@Autowired
public TranslationWebService(TranslationServiceProvider translationServiceProvider) {
this.translationServiceProvider = translationServiceProvider;
}

public TranslationResponse translate(TranslationRequest translationRequest) throws EuropeanaI18nApiException {
if(translationRequest.useCaching() && isCachingEnabled()) {
//only if the request requires caching and the caching service is enabled
Expand All @@ -42,7 +46,7 @@ public TranslationResponse translate(TranslationRequest translationRequest) thro
}

private boolean isCachingEnabled() {
return redisCacheService != null;
return getRedisCacheService() != null;
}

private TranslationResponse getTranslatedResults(TranslationRequest translationRequest) throws EuropeanaI18nApiException {
Expand Down Expand Up @@ -88,7 +92,7 @@ private TranslationResponse getTranslatedResults(TranslationRequest translationR

private TranslationResponse getCombinedCachedAndTranslatedResults(TranslationRequest translRequest) throws EuropeanaI18nApiException {
TranslationResponse result=null;
List<String> redisResp = redisCacheService.getCachedTranslations(translRequest.getSource(), translRequest.getTarget(), translRequest.getText());
List<String> redisResp = getRedisCacheService().getCachedTranslations(translRequest.getSource(), translRequest.getTarget(), translRequest.getText());
if(Collections.frequency(redisResp, null)>0) {

TranslationRequest newTranslReq = new TranslationRequest();
Expand All @@ -97,9 +101,9 @@ private TranslationResponse getCombinedCachedAndTranslatedResults(TranslationReq
newTranslReq.setService(translRequest.getService());
newTranslReq.setFallback(translRequest.getFallback());
newTranslReq.setCaching(translRequest.getCaching());
newTranslReq.setText(new ArrayList<String>(translRequest.getText()));
newTranslReq.setText(new ArrayList<>(translRequest.getText()));

List<String> newText = new ArrayList<String>();
List<String> newText = new ArrayList<>();
int counter=0;
for(String redisRespElem : redisResp) {
if(redisRespElem==null) {
Expand All @@ -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<String> finalText=new ArrayList<String>(redisResp);
List<String> finalText=new ArrayList<>(redisResp);
int counterTranslated = 0;
for(int i=0;i<finalText.size();i++) {
if(finalText.get(i)==null) {
Expand Down Expand Up @@ -227,4 +231,13 @@ public void close() {
service.close();
}
}

public RedisCacheService getRedisCacheService() {
return redisCacheService;
}

@Autowired(required = false)
public void setRedisCacheService(RedisCacheService redisCacheService) {
this.redisCacheService = redisCacheService;
}
}

0 comments on commit 159ea90

Please sign in to comment.