refactor: Reduced copy/paste in statement execute methods

redis-field-engineering · Jun 1, 2022 · 8bd7bd5 · 8bd7bd5
1 parent 37742bd
commit 8bd7bd5
Show file tree

Hide file tree

Showing 8 changed files with 160 additions and 209 deletions.
diff --git a/...jects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/AbstractResultSetCache.java b/...jects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/AbstractResultSetCache.java
@@ -1,7 +1,9 @@
 package com.redis.sidecar.core;
 
 import java.sql.ResultSet;
-import java.sql.SQLException;
+import java.util.Optional;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.Metrics;
@@ -10,45 +12,44 @@
 
 abstract class AbstractResultSetCache implements ResultSetCache {
 
-	private final Timer getTimer = Metrics.timer("gets");
-	private final Timer putTimer = Metrics.timer("puts");
-	private final Counter missCounter = Metrics.counter("gets", Tags.of("result", "miss"));
-	private final Counter hitCounter = Metrics.counter("gets", Tags.of("result", "hit"));
+	private static final Logger log = Logger.getLogger(AbstractResultSetCache.class.getName());
+
+	private final Timer getTimer = Metrics.timer("cache.gets");
+	private final Timer putTimer = Metrics.timer("cache.puts");
+	private final Counter missCounter = Metrics.counter("cache.gets", Tags.of("result", "miss"));
+	private final Counter hitCounter = Metrics.counter("cache.gets", Tags.of("result", "hit"));
 
 	@Override
-	public ResultSet get(String key) throws SQLException {
+	public Optional<ResultSet> get(String key) {
 		ResultSet resultSet;
 		try {
 			resultSet = getTimer.recordCallable(() -> doGet(key));
-		} catch (SQLException e) {
-			throw e;
 		} catch (Exception e) {
-			throw new SQLException(e);
+			log.log(Level.SEVERE, "Could not retrieve key " + key, e);
+			return Optional.empty();
 		}
 		if (resultSet == null) {
 			missCounter.increment();
-		} else {
-			hitCounter.increment();
+			return Optional.empty();
 		}
-		return resultSet;
+		hitCounter.increment();
+		return Optional.of(resultSet);
 	}
 
-	protected abstract ResultSet doGet(String key) throws SQLException;
+	protected abstract ResultSet doGet(String key) throws Exception;
 
 	@Override
-	public void put(String key, long ttl, ResultSet resultSet) throws SQLException {
+	public void put(String key, long ttl, ResultSet resultSet) {
 		if (ttl == Config.TTL_NO_CACHE) {
 			return;
 		}
 		try {
 			putTimer.recordCallable(() -> doPut(key, ttl, resultSet));
-		} catch (SQLException e) {
-			throw e;
 		} catch (Exception e) {
-			throw new SQLException(e);
+			log.log(Level.SEVERE, "Could not store key " + key, e);
 		}
 	}
 
-	protected abstract ResultSet doPut(String key, long ttl, ResultSet resultSet) throws SQLException;
+	protected abstract ResultSet doPut(String key, long ttl, ResultSet resultSet) throws Exception;
 
 }
diff --git a/subprojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/ResultSetCache.java b/subprojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/ResultSetCache.java
@@ -2,16 +2,18 @@
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Optional;
 
 public interface ResultSetCache extends AutoCloseable {
 
 	/**
 	 * 
 	 * @param key the key to get the ResultSet for.
-	 * @return the ResultSet that was retrieved from cache or null if none found.
+	 * @return optional with the ResultSet that was retrieved from cache or empty
+	 *         optional if none found.
 	 * @throws SQLException if the ResultSet could not be retrieved
 	 */
-	ResultSet get(String key) throws SQLException;
+	Optional<ResultSet> get(String key);
 
 	/**
 	 * Adds a ResultSet to the cache.
@@ -21,6 +23,6 @@ public interface ResultSetCache extends AutoCloseable {
 	 * @param resultSet the ResultSet to store under the key.
 	 * @throws SQLException if an error occurred while storing the ResultSet
 	 */
-	void put(String key, long ttl, ResultSet resultSet) throws SQLException;
+	void put(String key, long ttl, ResultSet resultSet);
 
 }
diff --git a/...rojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/StringResultSetCache.java b/...rojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/core/StringResultSetCache.java
@@ -1,8 +1,6 @@
 package com.redis.sidecar.core;
 
-import java.io.IOException;
 import java.sql.ResultSet;
-import java.sql.SQLException;
 import java.util.function.Function;
 
 import org.apache.commons.pool2.impl.GenericObjectPool;
@@ -30,25 +28,17 @@ public void close() throws Exception {
 	}
 
 	@Override
-	protected ResultSet doGet(String key) throws SQLException {
+	protected ResultSet doGet(String key) throws Exception {
 		try (StatefulConnection<String, ResultSet> connection = pool.borrowObject()) {
 			return sync.apply(connection).get(key);
-		} catch (IOException e) {
-			throw new SQLException("Could not decode ResultSet", e);
-		} catch (Exception e) {
-			throw new SQLException("Could not get cache connection", e);
 		}
 	}
 
 	@Override
-	protected ResultSet doPut(String key, long ttl, ResultSet resultSet) throws SQLException {
+	protected ResultSet doPut(String key, long ttl, ResultSet resultSet) throws Exception {
 		try (StatefulConnection<String, ResultSet> connection = pool.borrowObject()) {
 			RedisStringCommands<String, ResultSet> commands = sync.apply(connection);
 			commands.setex(key, ttl, resultSet);
-		} catch (IOException e) {
-			throw new SQLException("Could not encode ResultSet", e);
-		} catch (Exception e) {
-			throw new SQLException("Could not get cache connection", e);
 		}
 		return resultSet;
 	}

diff --git a/subprojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/jdbc/ParsedSQL.java b/subprojects/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/jdbc/ParsedSQL.java
diff --git a/...cts/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/jdbc/SidecarPreparedStatement.java b/...cts/redis-sidecar-jdbc/src/main/java/com/redis/sidecar/jdbc/SidecarPreparedStatement.java
@@ -25,40 +25,49 @@
 
 public class SidecarPreparedStatement extends SidecarStatement implements PreparedStatement {
 
+	private static final String METHOD_CANNOT_BE_USED = "Query methods that take a query string cannot be used on a PreparedStatement";
 	private final SortedMap<Integer, String> parameters = new TreeMap<>();
 	private final PreparedStatement statement;
 
 	public SidecarPreparedStatement(SidecarConnection connection, PreparedStatement statement, String sql) {
-		super(connection, statement, sql);
+		super(connection, statement);
 		this.statement = statement;
+		this.sql = sql;
 	}
 
 	@Override
-	protected String key() {
-		return appendParameters(new StringBuilder(super.key())).toString();
+	public ResultSet executeQuery(String sql) throws SQLException {
+		throw new SQLException(METHOD_CANNOT_BE_USED);
 	}
 
-	protected StringBuilder appendParameters(StringBuilder stringBuilder) {
-		parameters.forEach((k, v) -> appendParameter(stringBuilder, v));
-		return stringBuilder;
+	@Override
+	public int executeUpdate(String sql) throws SQLException {
+		throw new SQLException(METHOD_CANNOT_BE_USED);
 	}
 
-	protected final StringBuilder appendParameter(StringBuilder stringBuilder, String parameter) {
-		return stringBuilder.append(connection.getConfig().getKeySeparator()).append(parameter);
+	@Override
+	public void addBatch(String sql) throws SQLException {
+		throw new SQLException(METHOD_CANNOT_BE_USED);
 	}
 
 	@Override
-	public ResultSet executeQuery() throws SQLException {
-		return recordQuery(this::doExecuteQuery);
+	public boolean execute(String sql) throws SQLException {
+		throw new SQLException(METHOD_CANNOT_BE_USED);
+	}
+
+	@Override
+	protected String key(String sql) {
+		return appendParameters(new StringBuilder(super.key(sql))).toString();
 	}
 
-	private ResultSet doExecuteQuery() throws SQLException {
-		resultSet = get();
-		if (resultSet == null) {
-			ResultSet databaseResultSet = recordDatabase(() -> statement.executeQuery());
-			resultSet = cache(databaseResultSet);
-		}
-		return resultSet;
+	protected StringBuilder appendParameters(StringBuilder stringBuilder) {
+		parameters.forEach((k, v) -> appendParameter(stringBuilder, v));
+		return stringBuilder;
+	}
+
+	@Override
+	public ResultSet executeQuery() throws SQLException {
+		return executeQuery(statement::executeQuery);
 	}
 
 	@Override
@@ -187,15 +196,7 @@ public void setObject(int parameterIndex, Object x) throws SQLException {
 
 	@Override
 	public boolean execute() throws SQLException {
-		return recordQuery(this::doExecute);
-	}
-
-	private boolean doExecute() throws SQLException {
-		resultSet = get();
-		if (resultSet == null) {
-			return recordDatabase(() -> statement.execute());
-		}
-		return true;
+		return execute(statement::execute);
 	}
 
 	@Override