Remove unnecessary abstraction FBTpb and make sure copies of the TPB …

…are returned/used

src/docs/asciidoc/release_notes.adoc

@@ -1575,6 +1575,8 @@ this package is internal API only, and not exported from the module (see also ea
 *** `getException()` now always returns `SQLException`
 ** Constructor `SQLExceptionChainBuilder(SQLException)` was removed, as in practice this was never used;
 replacement is `new SQLExceptionChainBuilder().append(exception)`
+* `FBTpb` was removed, and its usages were replaced with `TransactionParameterBuffer`
+* `ParameterBuffer` now extends `Serializable`, as all implementations are serializable, and some usages expect serializable behaviour even when the interfaces were used (though in practice, these objects are hardly ever serialized)
 
 [#breaking-changes-unlikely]
 === Unlikely breaking changes

src/main/org/firebirdsql/gds/ParameterBuffer.java

@@ -29,11 +29,11 @@
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.Serializable;
 import java.util.Iterator;
 
 /**
- * Instance of this interface represents a Parameter Buffer it is extended
- * by various parameter buffer interfaces.
+ * Instance of this interface represents a Parameter Buffer it is extended by various parameter buffer interfaces.
  *
  * @author Mark Rotteveel
  * @see org.firebirdsql.gds.ParameterBuffer
@@ -42,7 +42,7 @@
  * @see org.firebirdsql.gds.ServiceParameterBuffer
  * @since 3.0
  */
-public interface ParameterBuffer extends Iterable<Parameter> {
+public interface ParameterBuffer extends Iterable<Parameter>, Serializable {
 
     /**
      * @return The parameter buffer type identifier

src/main/org/firebirdsql/gds/TransactionParameterBuffer.java

@@ -24,6 +24,10 @@
  */
 package org.firebirdsql.gds;
 
+import static org.firebirdsql.jaybird.fb.constants.TpbItems.isc_tpb_autocommit;
+import static org.firebirdsql.jaybird.fb.constants.TpbItems.isc_tpb_read;
+import static org.firebirdsql.jaybird.fb.constants.TpbItems.isc_tpb_write;
+
 /**
  * Instances of this interface represent Transaction Parameter Buffer from the Firebird API.
  */
@@ -47,4 +51,58 @@ default void copyTo(TransactionParameterBuffer destination) {
             parameter.copyTo(destination, null);
         }
     }
+
+    /**
+     * Set the read-only flag ({@code isc_tpb_read}) or read/write flag ({@code isc_tpb_write}) on this TPB.
+     *
+     * @param readOnly
+     *         if {@code true}, this TPB will be set to read-only, otherwise it will be read/write
+     * @since 6
+     */
+    default void setReadOnly(boolean readOnly) {
+        removeArgument(isc_tpb_read);
+        removeArgument(isc_tpb_write);
+        addArgument(readOnly ? isc_tpb_read : isc_tpb_write);
+    }
+
+    /**
+     * Determine whether this TPB is set to read-only.
+     *
+     * @return {@code true} if this TPB is read-only, otherwise {@code false}
+     * @since 6
+     */
+    default boolean isReadOnly() {
+        return hasArgument(isc_tpb_read);
+    }
+
+    /**
+     * Sets the Firebird auto-commit flag on this TPB.
+     * <p>
+     * This shouldn't be confused with the normal JDBC auto-commit behavior. Effectively, setting this to {@code true}
+     * will result in Firebird using commit retain after each executed statement.
+     * </p>
+     *
+     * @param autoCommit
+     *         {@code true} add the auto-commit flag, otherwise remove it
+     * @see #isAutoCommit()
+     * @since 6
+     */
+    default void setAutoCommit(boolean autoCommit) {
+        removeArgument(isc_tpb_autocommit);
+        if (autoCommit) {
+            addArgument(isc_tpb_autocommit);
+        }
+    }
+
+    /**
+     * Returns if this TPB has the auto-commit flag set.
+     *
+     * @return {@code true} if this TPB has the auto-commit flag, {@code false} otherwise
+     * @see #setAutoCommit(boolean)
+     * @since 6
+     */
+    default boolean isAutoCommit() {
+        return hasArgument(isc_tpb_autocommit);
+    }
+
 }

src/main/org/firebirdsql/gds/impl/ParameterBufferBase.java

@@ -49,7 +49,6 @@ public abstract class ParameterBufferBase implements ParameterBuffer, Serializab
     private final List<Argument> arguments = new ArrayList<>();
 
     private final String defaultEncodingName;
-    @SuppressWarnings("java:S1948")
     private final ParameterBufferMetaData parameterBufferMetaData;
     private transient Encoding defaultEncoding;
 
@@ -262,6 +261,7 @@ public void write(final XdrOutputStream outputStream) throws IOException {
         }
     }
 
+    @Serial
     private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
         in.defaultReadObject();
 

src/main/org/firebirdsql/gds/impl/ParameterBufferMetaData.java

@@ -21,13 +21,15 @@
 import org.firebirdsql.gds.ParameterBuffer;
 import org.firebirdsql.gds.impl.argument.ArgumentType;
 
+import java.io.Serializable;
+
 /**
  * Additional metadata for parameter buffer behavior.
  *
  * @author Mark Rotteveel
  * @since 3.0
  */
-public interface ParameterBufferMetaData {
+public interface ParameterBufferMetaData extends Serializable {
 
     /**
      * Parameter buffer type (this usually is the version of the parameter buffer).

src/main/org/firebirdsql/jaybird/xca/FBManagedConnection.java

@@ -90,7 +90,7 @@ public final class FBManagedConnection implements ExceptionListener {
     private XAResource xaResource;
     private final FBConnectionRequestInfo cri;
     private FBTpbMapper transactionMapping;
-    private FBTpb tpb;
+    private TransactionParameterBuffer tpb;
     private int transactionIsolation;
 
     private volatile boolean managedEnvironment = true;
@@ -552,7 +552,7 @@ private void forget(Xid id) throws XAException {
         try {
             // find XID
             // TODO: Is there a reason why this piece of code can't use the JDBC Statement class?
-            FbTransaction trHandle2 = database.startTransaction(tpb.getTransactionParameterBuffer());
+            FbTransaction trHandle2 = database.startTransaction(tpb);
             try (FbStatement stmtHandle2 = database.createStatement(trHandle2)) {
                 var gdsHelper2 = new GDSHelper(database);
                 gdsHelper2.setCurrentTransaction(trHandle2);
@@ -589,7 +589,7 @@ private void forget(Xid id) throws XAException {
 
         try {
             // delete XID
-            FbTransaction trHandle2 = database.startTransaction(tpb.getTransactionParameterBuffer());
+            FbTransaction trHandle2 = database.startTransaction(tpb);
             try (FbStatement stmtHandle2 = database.createStatement(trHandle2)) {
                 stmtHandle2.prepare(getXidQueries().forgetDelete() + inLimboId);
                 stmtHandle2.execute(RowValue.EMPTY_ROW_VALUE);
@@ -695,7 +695,7 @@ private Xid[] recover(int flags) throws javax.transaction.xa.XAException {
 
             var xids = new ArrayList<FBXid>();
 
-            FbTransaction trHandle = database.startTransaction(tpb.getTransactionParameterBuffer());
+            FbTransaction trHandle = database.startTransaction(tpb);
             try (FbStatement stmtHandle = database.createStatement(trHandle)) {
                 var gdsHelper = new GDSHelper(database);
                 gdsHelper.setCurrentTransaction(trHandle);
@@ -756,7 +756,7 @@ private static FBXid extractXid(byte[] xidData, long txId) {
      */
     Xid findSingleXid(Xid externalXid) throws javax.transaction.xa.XAException {
         try {
-            FbTransaction trHandle = database.startTransaction(tpb.getTransactionParameterBuffer());
+            FbTransaction trHandle = database.startTransaction(tpb);
             try (FbStatement stmtHandle = database.createStatement(trHandle)) {
                 var gdsHelper = new GDSHelper(database);
                 gdsHelper.setCurrentTransaction(trHandle);
@@ -1042,15 +1042,27 @@ public FBConnectionRequestInfo getConnectionRequestInfo() {
         return cri;
     }
 
+    /**
+     * @return a copy of the current TPB
+     */
     public TransactionParameterBuffer getTransactionParameters() {
+        TransactionParameterBuffer currentTpb;
         try (LockCloseable ignored = withLock()) {
-            return tpb.getTransactionParameterBuffer();
+            currentTpb = tpb;
         }
+        return currentTpb.deepCopy();
     }
 
-    public void setTransactionParameters(TransactionParameterBuffer transactionParameters) {
+    /**
+     * Sets the current TPB to a copy of {@code transactionParams}.
+     *
+     * @param transactionParams
+     *         transaction parameters
+     */
+    public void setTransactionParameters(TransactionParameterBuffer transactionParams) {
+        TransactionParameterBuffer copy = transactionParams.deepCopy();
         try (LockCloseable ignored = withLock()) {
-            tpb.setTransactionParameterBuffer(transactionParameters);
+            tpb = copy;
         }
     }
 
@@ -1099,7 +1111,7 @@ private void findIscTrHandle(Xid xid, int flags) throws SQLException, XAExceptio
 
         // new xid for us
         try {
-            registerNewTransaction(xid, database.startTransaction(tpb.getTransactionParameterBuffer()));
+            registerNewTransaction(xid, database.startTransaction(tpb));
         } catch (SQLException e) {
             throw new FBXAException(e.getMessage(), XAException.XAER_RMERR, e);
         }
@@ -1185,9 +1197,7 @@ public void setTransactionIsolation(int isolation) throws SQLException {
         try (LockCloseable ignored = withLock()) {
             transactionIsolation = isolation;
             final FBTpbMapper mapping = transactionMapping;
-            tpb = mapping == null
-                    ? mcf.getTpb(isolation)
-                    : new FBTpb(mapping.getMapping(isolation));
+            tpb = mapping == null ? mcf.getTpb(isolation) : mapping.getMapping(isolation);
         }
     }
 
@@ -1219,6 +1229,34 @@ public boolean isReadOnly() {
         return tpb.isReadOnly();
     }
 
+    /**
+     * Sets the Firebird auto-commit flag on the current TPB. This change is transient and will be reset when the
+     * transaction isolation level is set again or the TPB is otherwise replaced.
+     * <p>
+     * This shouldn't be confused with the normal JDBC auto-commit behavior. Effectively, setting this to {@code true}
+     * will result in Firebird using commit retain after each executed statement.
+     * </p>
+     *
+     * @param autoCommit
+     *         {@code true} add the auto-commit flag, otherwise remove it
+     * @see #isTpbAutoCommit()
+     * @since 6
+     */
+    public void setTpbAutoCommit(boolean autoCommit) {
+        tpb.setAutoCommit(autoCommit);
+    }
+
+    /**
+     * Returns if the current TPB has the auto-commit flag set.
+     *
+     * @return {@code true} if the current TPB has the auto-commit flag, {@code false} otherwise
+     * @see #setTpbAutoCommit(boolean)
+     * @since 6
+     */
+    public boolean isTpbAutoCommit() {
+        return tpb.isAutoCommit();
+    }
+
     @SuppressWarnings("java:S135")
     private void notifyWarning(SQLWarning warning) {
         final FBConnection connection = connectionHandle;

src/main/org/firebirdsql/jaybird/xca/FBManagedConnectionFactory.java

@@ -97,6 +97,7 @@ public final class FBManagedConnectionFactory implements FirebirdConnectionPrope
     // Maps supplied XID to internal transaction handle.
     private final Map<Xid, FBManagedConnection> xidMap = new ConcurrentHashMap<>();
 
+    @SuppressWarnings("java:S1948" /* we're using a serialization proxy */)
     private final Object startLock = new Object();
     private boolean started = false;
     // When a MCF is shared, its configuration has to be stable after first connection/datasource creation
@@ -297,21 +298,21 @@ private int hashCodeImpl() {
     @Override
     public boolean equals(Object other) {
         if (other == this) return true;
-        if (!(other instanceof FBManagedConnectionFactory that)) return false;
-        return this.connectionProperties.equals(that.connectionProperties);
+        return other instanceof FBManagedConnectionFactory that
+               && this.connectionProperties.equals(that.connectionProperties);
     }
 
     public FBConnectionRequestInfo getDefaultConnectionRequestInfo() throws SQLException {
         return new FBConnectionRequestInfo(connectionProperties.asIConnectionProperties().asNewMutable());
     }
 
-    public FBTpb getDefaultTpb() throws SQLException {
-        int defaultTransactionIsolation = connectionProperties.getDefaultTransactionIsolation();
-        return getTpb(defaultTransactionIsolation);
+    public TransactionParameterBuffer getDefaultTpb() throws SQLException {
+        return getTpb(connectionProperties.getDefaultTransactionIsolation());
     }
 
-    public FBTpb getTpb(int isolation) throws SQLException {
-        return new FBTpb(connectionProperties.getMapper().getMapping(isolation));
+    public TransactionParameterBuffer getTpb(int isolation) throws SQLException {
+        // getMapping makes a defensive copy
+        return connectionProperties.getMapper().getMapping(isolation);
     }
 
     /**
@@ -717,7 +718,7 @@ private void tryDeleteTransactionInfo(GDSHelper gdsHelper, long fbTransactionId)
             String query = "delete from rdb$transactions where rdb$transaction_id = " + fbTransactionId;
 
             FbDatabase dbHandle = gdsHelper.getCurrentDatabase();
-            FbTransaction trHandle = dbHandle.startTransaction(getDefaultTpb().getTransactionParameterBuffer());
+            FbTransaction trHandle = dbHandle.startTransaction(getDefaultTpb());
             try (FbStatement stmtHandle = dbHandle.createStatement(trHandle)) {
                 stmtHandle.prepare(query);
                 stmtHandle.execute(RowValue.EMPTY_ROW_VALUE);