Skip to content

Commit

Permalink
Clean up/refactor savepoint handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrotteveel
mrotteveel committed Dec 5, 2023
1 parent 17fb8bd commit a979fd1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 115 deletions.
66 changes: 24 additions & 42 deletions src/main/org/firebirdsql/jdbc/FBConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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()));
}
}

Expand All @@ -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);
}
Expand All @@ -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.
Expand Down
106 changes: 36 additions & 70 deletions src/main/org/firebirdsql/jdbc/FBSavepoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

}
6 changes: 3 additions & 3 deletions src/test/org/firebirdsql/jdbc/FBSavepointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")));
}

/**
Expand Down Expand Up @@ -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")));
}

/**
Expand Down

0 comments on commit a979fd1

Please sign in to comment.