Redis connection did not support custom port

.github/workflows/RedisTests.yml

@@ -0,0 +1,56 @@
+name: Redis Cache Tests
+
+on:
+  workflow_dispatch:
+  pull_request:
+    branches:
+      - 'master'
+      - 'release-*'
+  push:
+    branches:
+      - 'master'
+      - 'beta'
+      - 'release-*'
+
+jobs:
+ test-redis:
+    name: Test Redis Cache
+    env:
+      GIT_REF: ${{ github.ref }}
+      GIT_SHA: ${{ github.sha }}
+      POM_PATH: ./pom.xml
+
+    runs-on: ubuntu-latest
+    strategy:
+        matrix:
+            redis-version: [ 6 ]
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v2
+      with:
+        fetch-depth: 1
+
+    - name: Setup Java JDK
+      uses: actions/setup-java@v1.4.3
+      with:
+        java-version: 1.9
+
+    - name: Setup Maven settings
+      uses: whelk-io/maven-settings-xml-action@v14
+      with:
+        repositories: '[{ "id": "github-genexuslabs", "url": "https://maven.pkg.github.com/genexuslabs/Private-Maven-for-GX",  "releases": { "enabled": "true" }, "snapshots": { "enabled": "true" } }]'
+        servers: '[{ "id": "github-genexuslabs", "username": "genexusbot", "password": "${{ secrets.SECURE_TOKEN }}" }]'
+
+    - name: Install
+      run: mvn -B install --file $POM_PATH
+
+    - name: Start Redis
+      uses: supercharge/redis-github-action@1.4.0
+      with:
+        redis-version: ${{ matrix.redis-version }}
+
+    - name: Test Redis
+      run: |
+        export EXECUTE_REDIS_TESTS=true        
+        mvn -B -pl java test --file $POM_PATH

java/src/main/java/com/genexus/cache/redis/RedisClient.java

@@ -3,6 +3,7 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -26,105 +27,96 @@
 import redis.clients.jedis.Pipeline;
 
 
-public class RedisClient implements ICacheService2, Closeable{
+public class RedisClient implements ICacheService2, Closeable {
 	public static final ILogger logger = LogManager.getLogger(RedisClient.class);
 	private String keyPattern = "%s_%s_%s"; //Namespace_KEY
-	private static int UNDEFINED_PORT = -1;
 	private static int REDIS_DEFAULT_PORT = 6379;
 	private JedisPool pool;
-	private ObjectMapper objMapper; 	
-	public RedisClient() throws IOException {
+	private ObjectMapper objMapper;
+
+	public RedisClient() throws URISyntaxException {
 		initCache();
 	}
 
-	private void initCache() throws IOException {
-		objMapper = new ObjectMapper();
-		objMapper.setVisibility(PropertyAccessor.FIELD, Visibility.ANY);
-		objMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
-		objMapper.enable(SerializationFeature.INDENT_OUTPUT);
+	public RedisClient(String hostOrRedisURL, String password, String cacheKeyPattern) throws URISyntaxException {
+		initCache(hostOrRedisURL, password, cacheKeyPattern);
+	}
 
+	public JedisPool getConnection() {
+		return pool;
+	}
+
+	private void initCache() throws URISyntaxException {
 		GXService providerService = Application.getGXServices().get(GXServices.CACHE_SERVICE);
 		String addresses = providerService.getProperties().get("CACHE_PROVIDER_ADDRESS");
 		String cacheKeyPattern = providerService.getProperties().get("CACHE_PROVIDER_KEYPATTERN");
 		String password = providerService.getProperties().get("CACHE_PROVIDER_PASSWORD");
+		initCache(addresses, password, cacheKeyPattern);
+	}
 
-		if (!isNullOrEmpty(cacheKeyPattern))
-			keyPattern = cacheKeyPattern;
-
-		if (!isNullOrEmpty(addresses)){
+	private void initCache(String hostOrRedisURL, String password, String cacheKeyPattern) throws URISyntaxException {
+		keyPattern = isNullOrEmpty(cacheKeyPattern) ? keyPattern : cacheKeyPattern;
+		String host = "127.0.0.1";
+		hostOrRedisURL = isNullOrEmpty(hostOrRedisURL) ? host: hostOrRedisURL;
+		int port = REDIS_DEFAULT_PORT;
 
-			if (!isNullOrEmpty(password)) {
+		boolean isRedisURIScheme = hostOrRedisURL.startsWith("redis://");
+		String sRedisURI = isRedisURIScheme ? hostOrRedisURL : "redis://" + hostOrRedisURL;
 
-				addresses = "redis://:" + password.trim() + "@" + addresses.trim();
-				try {
-					URI redisUri = new URI(addresses);
-					if (redisUri.getPort()==UNDEFINED_PORT){
-						redisUri = new URI(addresses + ":" + REDIS_DEFAULT_PORT);
-					}
-					pool = new JedisPool(new JedisPoolConfig(), redisUri);
-				}catch (java.net.URISyntaxException ex){
-					logger.error("Invalid redis uri " + addresses, ex);
-				}
+		try {
+			URI redisURI = new URI(sRedisURI);
+			host = redisURI.getHost();
+			if (redisURI.getPort() > 0) {
+				port = redisURI.getPort();
 			}
-		}else{
-			addresses ="127.0.0.1:" + REDIS_DEFAULT_PORT;
+		} catch (URISyntaxException e) {
+			logger.error(String.format("Could not parse Redis URL. Check for supported URLs: %s" , sRedisURI), e);
+			throw e;
 		}
-		if (pool == null)
-			pool = new JedisPool(new JedisPoolConfig(), addresses);
+
+		password = (!isNullOrEmpty(password)) ? password : null;
+
+		pool = new JedisPool(new JedisPoolConfig(), host, port, redis.clients.jedis.Protocol.DEFAULT_TIMEOUT, password);
+
+		objMapper = new ObjectMapper();
+		objMapper.setVisibility(PropertyAccessor.FIELD, Visibility.ANY);
+		objMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+		objMapper.enable(SerializationFeature.INDENT_OUTPUT);
 	}
-	
+
 	private boolean isNullOrEmpty(String s) {
 		return s == null || s.trim().length() == 0;
 	}
 
 	private Boolean containsKey(String key) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();			
+		try (Jedis jedis = pool.getResource()) {
 			return jedis.exists(key);
 		} catch (Exception e) {
 			logger.error("Contains failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 		return false;
 	}
 
-	private void close(Jedis jedis) {
-		if (jedis != null)
-		{
-			jedis.close();
-		}
-	}
-
 	private <T> void set(String key, T value) {
 		set(key, value, 0);
 	}
 
 	private <T> void set(String key, T value, int expirationSeconds) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
+		try (Jedis jedis = pool.getResource()) {
 			String valueJSON = objMapper.writeValueAsString(value);
 			if (expirationSeconds > 0)
 				jedis.setex(key, expirationSeconds, valueJSON);
 			else
 				jedis.set(key, valueJSON);
 		} catch (Exception e) {
 			logger.error("Set with TTL failed", e);
-		}	
-		finally {
-			close(jedis);
 		}
 	}
 
 	public <T> void setAll(String cacheid, String[] keys, T[] values, int expirationSeconds) {
-		Jedis jedis = null;
-		try {
-			if (keys!=null && values!=null && keys.length == values.length) {
+		try (Jedis jedis = pool.getResource()) {
+			if (keys != null && values != null && keys.length == values.length) {
 				String[] prefixedKeys = getKey(cacheid, keys);
-				jedis = pool.getResource();
 				Pipeline p = jedis.pipelined();
 				int idx = 0;
 				for (String key : prefixedKeys) {
@@ -140,56 +132,41 @@ public <T> void setAll(String cacheid, String[] keys, T[] values, int expiration
 		} catch (Exception e) {
 			logger.error("SetAll with TTL failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 	}
 
 	private <T> T get(String key, Class<T> type) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
+		try (Jedis jedis = pool.getResource()) {
 			String json = jedis.get(key);
-			if (StringUtils.isNotEmpty(json))
-			{
-				return  objMapper.readValue(json, type); 
-			}
-			else
-			{
+			if (StringUtils.isNotEmpty(json)) {
+				return objMapper.readValue(json, type);
+			} else {
 				return null;
 			}
 		} catch (Exception e) {
 			logger.error("Get Item failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 		return null;
 	}
-	public <T> List<T> getAll(String cacheid, String[] keys, Class<T> type){
+
+	public <T> List<T> getAll(String cacheid, String[] keys, Class<T> type) {
 		List<T> result = null;
-		Jedis jedis = null;
-		try {
+		try (Jedis jedis = pool.getResource()) {
 			String[] prefixedKeys = getKey(cacheid, keys);
-			jedis = pool.getResource();	
 			List<String> json = jedis.mget(prefixedKeys);
 			result = new ArrayList<T>();
-			for (String val: json) {
+			for (String val : json) {
 				if (val != null)
-					result.add(objMapper.readValue(val, type)); 
+					result.add(objMapper.readValue(val, type));
 				else
 					result.add(null);
 			}
 			return result;
 		} catch (Exception e) {
 			logger.error("Get Item failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 		return null;
 	}
-	
+
 
 	public boolean containtsKey(String cacheid, String key) {
 		return containsKey(getKey(cacheid, key));
@@ -198,7 +175,7 @@ public boolean containtsKey(String cacheid, String key) {
 	public <T> T get(String cacheid, String key, Class<T> type) {
 		return get(getKey(cacheid, key), type);
 	}
-	
+
 
 	public <T> void set(String cacheid, String key, T value) {
 		set(getKey(cacheid, key), value);
@@ -209,72 +186,51 @@ public <T> void set(String cacheid, String key, T value, int duration) {
 	}
 
 	public void clear(String cacheid, String key) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
+		try (Jedis jedis = pool.getResource()) {
 			jedis.del(getKey(cacheid, key));
 		} catch (Exception e) {
 			logger.error("Remove Item failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 	}
 
 	public void clearCache(String cacheid) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
-			jedis.incr(cacheid);			
+		try (Jedis jedis = pool.getResource()) {
+			jedis.incr(cacheid);
 		} catch (Exception e) {
 			logger.error("clearCache failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 	}
 
 	public void clearKey(String key) {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
-			 jedis.del(key);
+		try (Jedis jedis = pool.getResource()) {
+			jedis.del(key);
 		} catch (Exception e) {
 			logger.error("Remove Item failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 	}
 
 	public void clearAllCaches() {
-		Jedis jedis = null;
-		try {
-			jedis = pool.getResource();	
-			 jedis.flushAll();
+		try (Jedis jedis = pool.getResource()) {
+			jedis.flushAll();
 		} catch (Exception e) {
 			logger.error("Clear All Caches failed", e);
 		}
-		finally {
-			close(jedis);
-		}
 	}
 
 	private String getKey(String cacheid, String key) {
 		return String.format(keyPattern, cacheid, getKeyPrefix(cacheid), com.genexus.CommonUtil.getHash(key));
 	}
 
-	private String[] getKey(String cacheid, String[] keys)
-	{
+	private String[] getKey(String cacheid, String[] keys) {
 		Long prefix = getKeyPrefix(cacheid);
 		String[] prefixedKeys = new String[keys.length];
-		for (int idx =0; idx<keys.length; idx++){
+		for (int idx = 0; idx < keys.length; idx++) {
 			prefixedKeys[idx] = formatKey(cacheid, keys[idx], prefix);
 		}
 		return prefixedKeys;
 	}
-	private String formatKey(String cacheid, String key, Long prefix)
-	{
+
+	private String formatKey(String cacheid, String key, Long prefix) {
 		return String.format(keyPattern, cacheid, prefix, com.genexus.CommonUtil.getHash(key));
 	}
 
@@ -286,8 +242,9 @@ private Long getKeyPrefix(String cacheid) {
 		}
 		return prefix;
 	}
+
 	@Override
-	public void close()  throws IOException {
+	public void close() throws IOException {
 		if (pool != null)
 			pool.destroy();
 	}