Skip to content

Commit

Permalink
Refactor statement implementations to use connection directly, add @n…
Browse files Browse the repository at this point in the history
…onnull and @nullable annotations where possible
  • Loading branch information
mrotteveel committed Sep 18, 2024
1 parent dcf2151 commit 3e473da
Show file tree
Hide file tree
Showing 15 changed files with 332 additions and 338 deletions.
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ java {
}

dependencies {
api libs.jspecify

// Use JUnit Jupiter API for testing.
testImplementation platform(testLibs.junit.bom)
testImplementation testLibs.bundles.junit
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ dependencyResolutionManagement {
library('jna', 'net.java.dev.jna', 'jna-jpms').version('5.14.0')
library('jakarta.servlet-api', 'jakarta.servlet', 'jakarta.servlet-api').version('5.0.0')
library('bcprov-jdk18on', 'org.bouncycastle', 'bcprov-jdk18on').version('1.78.1')
library('jspecify', 'org.jspecify', 'jspecify').version('1.0.0')
}

testLibs {
Expand Down
10 changes: 10 additions & 0 deletions src/docs/asciidoc/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,16 @@ We also fixed a potential `NullPointerException` in the implementation of these
The methods `getLastExceutionPlan()` and `getLastExplainedExecutionPlan()` which were already defined in `FirebirdStatement` now have a default implementation that call `getExecutionPlan()` and `getExplainedExecutionPlan()`.
These methods have been deprecated with the advice to use the new getters.
For now, we have no plans to remove these methods in a future release.
* Added dependency on `org.jspecify:jspecify` for nullability annotations.
+
We are working on adding nullability information where applicable, but right now annotation of Jaybird is far from complete, and this will generally only be added when we touch a class for other reasons.
The addition of these annotations is intended for making it easier for us to reason about the implementation, and get static analysis warnings about possible programming errors.
Hopefully it will -- in time -- provide some benefits for users of Jaybird's extension interfaces and "`internal`" APIs.
+
If a type is not annotated, consider it nullable unless stated otherwise in the API documentation.
+
In practice, this is an optional dependency, but Maven will pull it in by default.
If the JSpecify JAR is not included on the classpath or modulepath, Jaybird will still work.
[#potentially-breaking-changes]
=== Potentially breaking changes
Expand Down
3 changes: 3 additions & 0 deletions src/main/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
requires transitive java.transaction.xa;
requires transitive java.naming;

// Declare as optional for deployment simplicity
requires static org.jspecify;

exports org.firebirdsql.ds;
exports org.firebirdsql.encodings;
exports org.firebirdsql.event;
Expand Down
42 changes: 27 additions & 15 deletions src/main/org/firebirdsql/jdbc/AbstractStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@
import org.firebirdsql.gds.ng.FbStatement;
import org.firebirdsql.gds.ng.LockCloseable;
import org.firebirdsql.util.InternalApi;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;

import java.sql.SQLException;
import java.sql.SQLNonTransientException;
import java.sql.SQLWarning;
import java.sql.Statement;
import java.util.concurrent.atomic.AtomicInteger;

import static java.util.Objects.requireNonNull;
import static org.firebirdsql.jdbc.SQLStateConstants.SQL_STATE_INVALID_ATTR_VALUE;
import static org.firebirdsql.jdbc.SQLStateConstants.SQL_STATE_INVALID_STATEMENT_ID;

Expand All @@ -48,28 +51,35 @@ public abstract class AbstractStatement implements Statement, FirebirdStatement
private static final AtomicInteger STATEMENT_ID_GENERATOR = new AtomicInteger();

private final int localStatementId = STATEMENT_ID_GENERATOR.incrementAndGet();
private String cursorName;
protected final @NonNull FBConnection connection;
private @Nullable String cursorName;
@SuppressWarnings("java:S3077")
private volatile SQLWarning warning;
private FetchConfig fetchConfig;
private volatile @Nullable SQLWarning warning;
private @NonNull FetchConfig fetchConfig;

private volatile boolean closed;
private boolean poolable;
private boolean closeOnCompletion;

protected AbstractStatement(ResultSetBehavior resultSetBehavior) {
protected AbstractStatement(@NonNull FBConnection connection, @NonNull ResultSetBehavior resultSetBehavior) {
this.connection = requireNonNull(connection, "connection");
fetchConfig = new FetchConfig(resultSetBehavior);
}

@Override
public final @NonNull FBConnection getConnection() throws SQLException {
checkValidity();
return connection;
}

/**
* @return instance of {@link FbStatement} associated with this statement; can be {@code null} if no statement has
* been prepared or executed yet.
* @return instance of {@link FbStatement} associated with this statement; cannot be {@code null}
* @throws SQLException
* if this statement is closed
* @throws java.sql.SQLFeatureNotSupportedException
* if this statement implementation does not use statement handles
*/
protected abstract FbStatement getStatementHandle() throws SQLException;
protected abstract @NonNull FbStatement getStatementHandle() throws SQLException;

/**
* Get the current statement type of this statement.
Expand Down Expand Up @@ -140,7 +150,7 @@ public final void completeStatement() throws SQLException {
* @throws SQLException
* for failures completing this statement
*/
public abstract void completeStatement(CompletionReason reason) throws SQLException;
public abstract void completeStatement(@NonNull CompletionReason reason) throws SQLException;

@Override
public final boolean isPoolable() throws SQLException {
Expand Down Expand Up @@ -192,7 +202,7 @@ protected final void performCloseOnCompletion() throws SQLException {
* @return current fetch config for this statement
* @since 6
*/
protected final FetchConfig fetchConfig() {
protected final @NonNull FetchConfig fetchConfig() {
try (var ignored = withLock()) {
return fetchConfig;
}
Expand All @@ -202,7 +212,7 @@ protected final FetchConfig fetchConfig() {
* @return result set behavior for this statement
* @since 6
*/
protected final ResultSetBehavior resultSetBehavior() {
protected final @NonNull ResultSetBehavior resultSetBehavior() {
return fetchConfig().resultSetBehavior();
}

Expand Down Expand Up @@ -300,7 +310,7 @@ public final void setFetchDirection(int direction) throws SQLException {
}

@Override
public final void setCursorName(String cursorName) throws SQLException {
public final void setCursorName(@Nullable String cursorName) throws SQLException {
checkValidity();
try (var ignored = withLock()) {
this.cursorName = cursorName;
Expand All @@ -311,12 +321,12 @@ public final void setCursorName(String cursorName) throws SQLException {
* @return current value of {@code cursorName}
* @see #setCursorName(String)
*/
protected final String getCursorName() {
protected final @Nullable String getCursorName() {
return cursorName;
}

@Override
public final SQLWarning getWarnings() throws SQLException {
public final @Nullable SQLWarning getWarnings() throws SQLException {
checkValidity();
return warning;
}
Expand All @@ -327,7 +337,7 @@ public final void clearWarnings() throws SQLException {
warning = null;
}

protected final void addWarning(SQLWarning warning) {
protected final void addWarning(@NonNull SQLWarning warning) {
try (var ignored = withLock()) {
SQLWarning currentWarning = this.warning;
if (currentWarning == null) {
Expand Down Expand Up @@ -357,6 +367,8 @@ public final boolean equals(Object other) {
/**
* @see FbAttachment#withLock()
*/
protected abstract LockCloseable withLock();
protected final @NonNull LockCloseable withLock() {
return connection.withLock();
}

}
2 changes: 1 addition & 1 deletion src/main/org/firebirdsql/jdbc/ClientInfoProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private Statement getStatement() throws SQLException {
// even in auto-commit)
var rsBehavior = ResultSetBehavior.of(
ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY, ResultSet.CLOSE_CURSORS_AT_COMMIT);
return this.statement = new FBStatement(connection.getGDSHelper(), rsBehavior, metaDataTransactionCoordinator);
return this.statement = new FBStatement(connection, rsBehavior, metaDataTransactionCoordinator);
}

/**
Expand Down
Loading

0 comments on commit 3e473da

Please sign in to comment.