diff --git a/src/main/org/firebirdsql/jdbc/FBConnection.java b/src/main/org/firebirdsql/jdbc/FBConnection.java
index 93d874d03..e4684b5e8 100644
--- a/src/main/org/firebirdsql/jdbc/FBConnection.java
+++ b/src/main/org/firebirdsql/jdbc/FBConnection.java
@@ -858,13 +858,13 @@ public Savepoint setSavepoint() throws SQLException {
* Set the savepoint on the server.
*
* @param savepoint
- * savepoint to set.
+ * savepoint to set
* @throws SQLException
- * if something went wrong.
+ * if something went wrong
*/
private void setSavepoint(FBSavepoint savepoint) throws SQLException {
if (getAutoCommit()) {
- throw new SQLException("Connection.setSavepoint() method cannot be used in auto-commit mode.",
+ throw new SQLException("Connection.setSavepoint() method cannot be used in auto-commit mode",
SQL_STATE_INVALID_TX_STATE);
}
@@ -874,10 +874,7 @@ private void setSavepoint(FBSavepoint savepoint) throws SQLException {
txCoordinator.ensureTransaction();
- StringBuilder setSavepoint = new StringBuilder("SAVEPOINT ");
- getQuoteStrategy().appendQuoted(savepoint.getServerSavepointId(), setSavepoint);
-
- getGDSHelper().executeImmediate(setSavepoint.toString());
+ getGDSHelper().executeImmediate(savepoint.toSavepointStatement(getQuoteStrategy()));
savepoints.add(savepoint);
}
@@ -921,26 +918,14 @@ public void rollback(Savepoint savepoint) throws SQLException {
try (LockCloseable ignored = withLock()) {
checkValidity();
if (getAutoCommit()) {
- throw new SQLException("Connection.rollback(Savepoint) method cannot be used in auto-commit mode.",
+ throw new SQLException("Connection.rollback(Savepoint) method cannot be used in auto-commit mode",
SQL_STATE_INVALID_TX_STATE);
- }
-
- // TODO The error message and actual condition do not match
- if (!(savepoint instanceof FBSavepoint fbSavepoint)) {
- throw new SQLException("Specified savepoint was not obtained from this connection.");
- }
-
- if (mc.inDistributedTransaction()) {
+ } else if (mc.inDistributedTransaction()) {
throw new SQLException("Connection enlisted in distributed transaction", SQL_STATE_INVALID_TX_STATE);
}
- if (!fbSavepoint.isValid()) {
- throw new SQLException("Savepoint is no longer valid.");
- }
-
- StringBuilder rollbackSavepoint = new StringBuilder("ROLLBACK TO ");
- getQuoteStrategy().appendQuoted(fbSavepoint.getServerSavepointId(), rollbackSavepoint);
- getGDSHelper().executeImmediate(rollbackSavepoint.toString());
+ FBSavepoint fbSavepoint = validateSavepoint(savepoint);
+ getGDSHelper().executeImmediate(fbSavepoint.toRollbackStatement(getQuoteStrategy()));
}
}
@@ -949,24 +934,12 @@ public void releaseSavepoint(Savepoint savepoint) throws SQLException {
try (LockCloseable ignored = withLock()) {
checkValidity();
if (getAutoCommit()) {
- throw new SQLException("Connection.releaseSavepoint() method cannot be used in auto-commit mode.",
+ throw new SQLException("Connection.releaseSavepoint() method cannot be used in auto-commit mode",
SQL_STATE_INVALID_TX_STATE);
}
- // TODO The error message and actual condition do not match
- if (!(savepoint instanceof FBSavepoint fbSavepoint)) {
- throw new SQLException("Specified savepoint was not obtained from this connection.");
- }
-
- if (!fbSavepoint.isValid()) {
- throw new SQLException("Savepoint is no longer valid.");
- }
-
- StringBuilder rollbackSavepoint = new StringBuilder("RELEASE SAVEPOINT ");
- getQuoteStrategy().appendQuoted(fbSavepoint.getServerSavepointId(), rollbackSavepoint).append(" ONLY");
- getGDSHelper().executeImmediate(rollbackSavepoint.toString());
-
- fbSavepoint.invalidate();
+ FBSavepoint fbSavepoint = validateSavepoint(savepoint);
+ getGDSHelper().executeImmediate(fbSavepoint.toReleaseStatement(getQuoteStrategy()));
savepoints.remove(fbSavepoint);
}
@@ -977,14 +950,23 @@ public void releaseSavepoint(Savepoint savepoint) throws SQLException {
*/
protected void invalidateSavepoints() {
try (LockCloseable ignored = withLock()) {
- for (FBSavepoint savepoint : savepoints) {
- savepoint.invalidate();
- }
-
savepoints.clear();
}
}
+ FBSavepoint validateSavepoint(Savepoint savepoint) throws SQLException {
+ // TODO The error message and actual condition do not match
+ if (!(savepoint instanceof FBSavepoint fbSavepoint)) {
+ throw new SQLException("Specified savepoint was not obtained from this connection");
+ }
+
+ if (!savepoints.contains(fbSavepoint)) {
+ throw new SQLException("Savepoint is no longer valid");
+ }
+
+ return fbSavepoint;
+ }
+
/**
* Returns a FBLocalTransaction instance that enables a component to
* demarcate resource manager local transactions on this connection.
diff --git a/src/main/org/firebirdsql/jdbc/FBSavepoint.java b/src/main/org/firebirdsql/jdbc/FBSavepoint.java
index 1ef45e275..ba82bcad2 100644
--- a/src/main/org/firebirdsql/jdbc/FBSavepoint.java
+++ b/src/main/org/firebirdsql/jdbc/FBSavepoint.java
@@ -23,123 +23,89 @@
/**
* Savepoint implementation.
*
+ * @param savepointId
+ * numeric savepoint id (must be non-{@code null} if {@code name} is {@code null}).
+ * @param name
+ * savepoint name (must be non-{@code null} if {@code savepointId} is {@code null})
* @author Roman Rokytskyy
+ * @author Mark Rotteveel
*/
-public final class FBSavepoint implements FirebirdSavepoint {
+public record FBSavepoint(Integer savepointId, String name) implements FirebirdSavepoint {
private static final String SAVEPOINT_ID_PREFIX = "SVPT";
- private final boolean named;
- private final int savepointId;
- private final String name;
- private boolean valid = true;
+ public FBSavepoint {
+ if (savepointId == null && name == null) {
+ throw new NullPointerException("savepointId and name cannot both be null");
+ } else if (name == null) {
+ name = generateSavepointName(savepointId);
+ } else if (savepointId != null) {
+ throw new IllegalArgumentException("savepointId cannot be non-null if name is not null");
+ } else if (name.isBlank()) {
+ throw new IllegalArgumentException("name must be non-blank");
+ }
+ }
/**
- * Create instance of this class.
+ * Create an unnamed savepoint.
*
* @param savepointId
- * ID of the savepoint.
+ * ID of the savepoint
*/
public FBSavepoint(int savepointId) {
- this(false, savepointId, getSavepointServerId(savepointId));
+ this(savepointId, null);
}
/**
- * Create instance of this class for the specified name.
+ * Create a named savepoint.
*
* @param name
- * name of the savepoint.
+ * name of the savepoint
*/
public FBSavepoint(String name) {
- this(true, -1, name);
- }
-
- private FBSavepoint(boolean named, int savepointId, String name) {
- this.named = named;
- this.savepointId = savepointId;
- this.name = name;
+ this(null, name);
}
/**
* Generate a savepoint name for the specified savepoint id.
*
* @param savePointId
- * savepoint id.
- * @return valid savepoint name.
+ * savepoint id
+ * @return valid savepoint name
*/
- private static String getSavepointServerId(int savePointId) {
+ private static String generateSavepointName(int savePointId) {
if (savePointId >= 0) {
return SAVEPOINT_ID_PREFIX + savePointId;
}
return SAVEPOINT_ID_PREFIX + '_' + Math.abs(savePointId);
}
- /**
- * Get server savepoint name.
- * <p>
- * This method generates correct name for the savepoint that can be used in the SQL statement after
- * dialect-appropriate quoting.
- * </p>
- *
- * @return valid server-side name for the savepoint.
- */
- String getServerSavepointId() {
- return name;
- }
-
@Override
public int getSavepointId() throws SQLException {
- if (named) {
- throw new SQLException("Savepoint is named.");
+ if (savepointId == null) {
+ throw new SQLException("Savepoint is named");
}
return savepointId;
}
@Override
public String getSavepointName() throws SQLException {
- if (!named) {
- throw new SQLException("Savepoint is unnamed.");
+ if (savepointId != null) {
+ throw new SQLException("Savepoint is unnamed");
}
return name;
}
- /**
- * Check if the savepoint is valid.
- *
- * @return {@code true} if savepoint is valid.
- */
- boolean isValid() {
- return valid;
- }
-
- /**
- * Make this savepoint invalid.
- */
- void invalidate() {
- this.valid = false;
+ String toSavepointStatement(QuoteStrategy quoteStrategy) {
+ return "SAVEPOINT " + quoteStrategy.quoteObjectName(name);
}
- /**
- * Check if objects are equal. For unnamed savepoints their IDs are checked, otherwise their names.
- *
- * @param obj
- * object to test.
- * @return {@code true} if {@code obj} is equal to this object.
- */
- @Override
- public boolean equals(Object obj) {
- if (obj == this) return true;
- if (!(obj instanceof FBSavepoint)) return false;
-
- FBSavepoint that = (FBSavepoint) obj;
-
- return this.named == that.named &&
- this.name.equals(that.name);
+ String toRollbackStatement(QuoteStrategy quoteStrategy) {
+ return "ROLLBACK TO " + quoteStrategy.quoteObjectName(name);
}
- @Override
- public int hashCode() {
- return name.hashCode();
+ String toReleaseStatement(QuoteStrategy quoteStrategy) {
+ return "RELEASE SAVEPOINT " + quoteStrategy.quoteObjectName(name) + " ONLY";
}
}
diff --git a/src/test/org/firebirdsql/jdbc/FBSavepointTest.java b/src/test/org/firebirdsql/jdbc/FBSavepointTest.java
index acda17d8b..f40dab701 100644
--- a/src/test/org/firebirdsql/jdbc/FBSavepointTest.java
+++ b/src/test/org/firebirdsql/jdbc/FBSavepointTest.java
@@ -77,7 +77,7 @@ void testSavePointCreationNotAllowedInAutoCommit() throws Exception {
connection.setAutoCommit(true);
SQLException exception = assertThrows(SQLException.class, connection::setSavepoint);
- assertThat(exception, message(equalTo("Connection.setSavepoint() method cannot be used in auto-commit mode.")));
+ assertThat(exception, message(equalTo("Connection.setSavepoint() method cannot be used in auto-commit mode")));
}
@Test
@@ -94,7 +94,7 @@ void getSavePointNameOnUnnamedSavePointThrowsException() throws Exception {
Savepoint savepoint = connection.setSavepoint();
SQLException exception = assertThrows(SQLException.class, savepoint::getSavepointName);
- assertThat(exception, message(equalTo("Savepoint is unnamed.")));
+ assertThat(exception, message(equalTo("Savepoint is unnamed")));
}
/**
@@ -136,7 +136,7 @@ void getSavePointIdOnNamedSavePointThrowsException() throws Exception {
Savepoint savepoint = connection.setSavepoint("named");
SQLException exception = assertThrows(SQLException.class, savepoint::getSavepointId);
- assertThat(exception, message(equalTo("Savepoint is named.")));
+ assertThat(exception, message(equalTo("Savepoint is named")));
}
/**