Skip to content

Commit

Permalink
#805 Preserve connection read-only state across changes to transactio…
Browse files Browse the repository at this point in the history
…n isolation
  • Loading branch information
mrotteveel committed May 12, 2024
1 parent 0c14f7e commit 9177bae
Showing 12 changed files with 188 additions and 95 deletions.
20 changes: 20 additions & 0 deletions src/docs/asciidoc/release_notes.adoc
Original file line number Diff line number Diff line change
@@ -1034,6 +1034,7 @@ Given this is a non-standard extension, it is advisable to retrieve these column
+
To be clear, Jaybird does not provide statement pooling.
This is change is only about returning and recording the poolable information for JDBC compliance, so it can be used by -- for example -- connection pool implementations that provide statement pooling.
* The state of `Connection.setReadOnly(boolean)` was not retained after calling `Connection.setTransactionIsolation(int)` or other method calls that changed the current transaction parameter buffer (https://github.com/FirebirdSQL/jaybird/issues/805[#805])
[#potentially-breaking-changes]
=== Potentially breaking changes
@@ -1194,6 +1195,21 @@ Switch to using one of the normal execute methods.

See also <<allow-tx-stmts>>.

[#compat-read-only]
=== Read-only behaviour of connections

In previous versions of Jaybird the read-only state of a connection was not retained if the transaction parameter buffer was replaced, for example by calls to `setTransactionIsolation(int)`.

Now this has been corrected, it is possible that your code unexpectedly throws an exception with message "`__attempted update during read-only transaction [SQLState:42000, ISC error code:335544361]__`" (error `isc_read_only_trans`).

You need to make sure to call `setReadOnly(false)` if the connection was previously marked read-only.
If you're using a connection pool, you need to ensure it properly resets the read-only state of the connection when checking in or checking out the connection.
For example, both Apache DBCP and Apache Tomcat connection pools requires the `defaultReadOnly` property to be set (i.e. to `false`), otherwise it will not reset the read-only state.

If overridden transaction mappings are used, and the default isolation level has `isc_tpb_read`, the connection will be marked as read-only.
As a result, switching isolation levels will now also result in read-only transactions, even if the mapping of the other isolation level is defined with `isc_tpb_write`.
You will need to explicitly call `setReadOnly(false)`, or -- better yet -- do not override transaction mappings with a `isc_tpb_read`, but always use `isc_tpb_write`, and control read-only state only through `setReadOnly`.

// TODO Document compatibility issues

[#removal-of-classes-packages-and-methods-without-deprecation]
@@ -1576,7 +1592,11 @@ this package is internal API only, and not exported from the module (see also ea
** 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`
* `FBTpbMapper` no longer implements `Cloneable`, use `FBTpbMapper.copyOf(FBTpbMapper)` instead
* `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)
* `FBManagedConnection`
** `setReadOnly(boolean)` was renamed to `setTpbReadOnly(boolean)` to reflect what it actually does
** `isReadOnly()` was renamed to `isTpbReadOnly()` to reflect what it actually does

[#breaking-changes-unlikely]
=== Unlikely breaking changes
2 changes: 1 addition & 1 deletion src/main/org/firebirdsql/gds/ParameterBuffer.java
Original file line number Diff line number Diff line change
@@ -120,7 +120,7 @@ public interface ParameterBuffer extends Iterable<Parameter>, Serializable {
void addArgument(int argumentType, byte[] content);

/**
* Remove specified argument.
* Remove the first occurrence of the specified argument.
*
* @param argumentType
* type of argument to remove.
22 changes: 17 additions & 5 deletions src/main/org/firebirdsql/gds/TransactionParameterBuffer.java
Original file line number Diff line number Diff line change
@@ -60,9 +60,18 @@ default void copyTo(TransactionParameterBuffer destination) {
* @since 6
*/
default void setReadOnly(boolean readOnly) {
removeArgument(isc_tpb_read);
removeArgument(isc_tpb_write);
addArgument(readOnly ? isc_tpb_read : isc_tpb_write);
if (readOnly) {
ensurePresentAbsent(isc_tpb_read, isc_tpb_write);
} else {
ensurePresentAbsent(isc_tpb_write, isc_tpb_read);
}
}

private void ensurePresentAbsent(int present, int absent) {
if (!hasArgument(present)) {
addArgument(present);
}
removeArgument(absent);
}

/**
@@ -88,9 +97,12 @@ default boolean isReadOnly() {
* @since 6
*/
default void setAutoCommit(boolean autoCommit) {
removeArgument(isc_tpb_autocommit);
if (autoCommit) {
addArgument(isc_tpb_autocommit);
if (!hasArgument(isc_tpb_autocommit)) {
addArgument(isc_tpb_autocommit);
}
} else {
removeArgument(isc_tpb_autocommit);
}
}

81 changes: 39 additions & 42 deletions src/main/org/firebirdsql/gds/impl/ParameterBufferBase.java
Original file line number Diff line number Diff line change
@@ -35,6 +35,7 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;

/**
* Base class for parameter buffers
@@ -45,7 +46,7 @@ public abstract class ParameterBufferBase implements ParameterBuffer, Serializab

@Serial
private static final long serialVersionUID = 8812835147477954476L;

private final List<Argument> arguments = new ArrayList<>();

private final String defaultEncodingName;
@@ -98,72 +99,68 @@ public final void addArgument(int argumentType, String value) {

@Override
public final void addArgument(int argumentType, String value, Encoding encoding) {
getArgumentsList().add(new StringArgument(argumentType, parameterBufferMetaData.getStringArgumentType(argumentType), value, encoding));
addArgument(new StringArgument(
argumentType, parameterBufferMetaData.getStringArgumentType(argumentType), value, encoding));
}

@Override
public void addArgument(int argumentType, byte value) {
getArgumentsList().add(new ByteArgument(argumentType, parameterBufferMetaData.getByteArgumentType(argumentType), value));
public final void addArgument(int argumentType, byte value) {
addArgument(new ByteArgument(argumentType, parameterBufferMetaData.getByteArgumentType(argumentType), value));
}

@Override
public final void addArgument(int argumentType, int value) {
getArgumentsList().add(new NumericArgument(argumentType, parameterBufferMetaData.getIntegerArgumentType(argumentType), value));
addArgument(new NumericArgument(
argumentType, parameterBufferMetaData.getIntegerArgumentType(argumentType), value));
}

@Override
public final void addArgument(int argumentType, long value) {
getArgumentsList().add(new BigIntArgument(argumentType, parameterBufferMetaData.getIntegerArgumentType(argumentType), value));
addArgument(new BigIntArgument(
argumentType, parameterBufferMetaData.getIntegerArgumentType(argumentType), value));
}

@Override
public final void addArgument(int argumentType) {
getArgumentsList().add(new SingleItem(argumentType, parameterBufferMetaData.getSingleArgumentType(argumentType)));
addArgument(new SingleItem(argumentType, parameterBufferMetaData.getSingleArgumentType(argumentType)));
}

@Override
public final void addArgument(int type, byte[] content) {
getArgumentsList().add(new ByteArrayArgument(type, parameterBufferMetaData.getByteArrayArgumentType(type), content));
addArgument(new ByteArrayArgument(type, parameterBufferMetaData.getByteArrayArgumentType(type), content));
}

protected final void addArgument(Argument argument) {
arguments.add(argument);
}

@Override
public final String getArgumentAsString(int type) {
final List<Argument> argumentsList = getArgumentsList();
for (final Argument argument : argumentsList) {
if (argument.getType() == type) {
return argument.getValueAsString();
}
}
return null;
return findFirst(type).map(Argument::getValueAsString).orElse(null);
}

@SuppressWarnings("OptionalIsPresent")
@Override
public final int getArgumentAsInt(int type) {
final List<Argument> argumentsList = getArgumentsList();
for (final Argument argument : argumentsList) {
if (argument.getType() == type) {
return argument.getValueAsInt();
}
}
return 0;
Optional<Argument> argumentOpt = findFirst(type);
return argumentOpt.isPresent() ? argumentOpt.get().getValueAsInt() : 0;
}

@Override
public final boolean hasArgument(int type) {
final List<Argument> argumentsList = getArgumentsList();
for (final Argument argument : argumentsList) {
if (argument.getType() == type) return true;
}
return false;
return findFirst(type).isPresent();
}

protected Optional<Argument> findFirst(int type) {
return arguments.stream().filter(argument -> argument.getType() == type).findFirst();
}

@Override
public final void removeArgument(int type) {
final List<Argument> argumentsList = getArgumentsList();
for (int i = 0, n = argumentsList.size(); i < n; i++) {
final Argument argument = argumentsList.get(i);
if (argument.getType() == type) {
argumentsList.remove(i);
Iterator<Argument> argumentIterator = arguments.iterator();
while (argumentIterator.hasNext()) {
if (argumentIterator.next().getType() == type) {
argumentIterator.remove();
return;
}
}
@@ -175,7 +172,7 @@ public final Iterator<Parameter> iterator() {
}

public final void writeArgumentsTo(OutputStream outputStream) throws IOException {
for (final Argument currentArgument : arguments) {
for (Argument currentArgument : arguments) {
currentArgument.writeTo(outputStream);
}
}
@@ -186,9 +183,8 @@ public final Xdrable toXdrable() {
}

protected final int getLength() {
final List<Argument> argumentsList = getArgumentsList();
int length = 0;
for (final Argument currentArgument : argumentsList) {
for (Argument currentArgument : arguments) {
length += currentArgument.getLength();
}
return length;
@@ -200,7 +196,7 @@ protected final List<Argument> getArgumentsList() {

@Override
public final byte[] toBytes() {
final ByteArrayOutputStream bout = new ByteArrayOutputStream();
var bout = new ByteArrayOutputStream();
try {
writeArgumentsTo(bout);
} catch (IOException e) {
@@ -211,7 +207,7 @@ public final byte[] toBytes() {

@Override
public final byte[] toBytesWithType() {
final ByteArrayOutputStream bout = new ByteArrayOutputStream();
final var bout = new ByteArrayOutputStream();
try {
bout.write(getType());
writeArgumentsTo(bout);
@@ -229,29 +225,30 @@ public final int size() {
@Override
@SuppressWarnings("java:S2097")
public final boolean equals(Object other) {
if (other == null || !(this.getClass().isAssignableFrom(other.getClass())))
if (other == null || !(this.getClass().isAssignableFrom(other.getClass()))) {
return false;
}

final ParameterBufferBase otherServiceBufferBase = (ParameterBufferBase) other;
return otherServiceBufferBase.getArgumentsList().equals(this.getArgumentsList());
return otherServiceBufferBase.arguments.equals(this.arguments);
}

@Override
public final int hashCode() {
return getArgumentsList().hashCode();
return arguments.hashCode();
}

/**
* Default implementation for serializing the parameter buffer to the XDR output stream
*/
private class ParameterBufferXdrable implements Xdrable {
private final class ParameterBufferXdrable implements Xdrable {
@Override
public int getLength() {
return ParameterBufferBase.this.getLength();
}

@Override
public void read(XdrInputStream inputStream, int length) throws IOException {
public void read(XdrInputStream inputStream, int length) {
throw new UnsupportedOperationException();
}

12 changes: 7 additions & 5 deletions src/main/org/firebirdsql/jaybird/xca/FBManagedConnection.java
Original file line number Diff line number Diff line change
@@ -1211,21 +1211,23 @@ public FBManagedConnectionFactory getManagedConnectionFactory() {
}

/**
* Set whether this connection is to be readonly
* Set the current TPB to read-only.
*
* @param readOnly
* If {@code true}, the connection will be set read-only, otherwise it will be writable
* if {@code true}, the connection will be set read-only, otherwise it will be writable
* @since 6
*/
public void setReadOnly(boolean readOnly) {
public void setTpbReadOnly(boolean readOnly) {
tpb.setReadOnly(readOnly);
}

/**
* Retrieve whether this connection is readonly.
* Retrieve whether the current TPB is read-only.
*
* @return {@code true} if this connection is readonly, {@code false} otherwise
* @since 6
*/
public boolean isReadOnly() {
public boolean isTpbReadOnly() {
return tpb.isReadOnly();
}

Original file line number Diff line number Diff line change
@@ -323,7 +323,7 @@ public TransactionParameterBuffer getTpb(int isolation) throws SQLException {
* For errors on obtaining or creating the transaction mapping
*/
FBTpbMapper getTransactionMappingCopy() throws SQLException {
return (FBTpbMapper) connectionProperties.getMapper().clone();
return FBTpbMapper.copyOf(connectionProperties.getMapper());
}

/**
9 changes: 7 additions & 2 deletions src/main/org/firebirdsql/jdbc/FBConnection.java
Original file line number Diff line number Diff line change
@@ -91,6 +91,7 @@ public class FBConnection implements FirebirdConnection {
private StoredProcedureMetaData storedProcedureMetaData;
private GeneratedKeysSupport generatedKeysSupport;
private ClientInfoProvider clientInfoProvider;
private boolean readOnly;

/**
* Create a new FBConnection instance based on a {@link FBManagedConnection}.
@@ -107,6 +108,9 @@ public FBConnection(FBManagedConnection mc) {
resultSetHoldability = props.isDefaultResultSetHoldable()
? ResultSet.HOLD_CURSORS_OVER_COMMIT
: ResultSet.CLOSE_CURSORS_AT_COMMIT;

// Inherit read-only state from the initial TPB
readOnly = mc.isTpbReadOnly();
}

@Override
@@ -603,15 +607,16 @@ public void setReadOnly(boolean readOnly) throws SQLException {
"Calling setReadOnly(boolean) method is not allowed when transaction is already started.",
SQL_STATE_TX_ACTIVE);
}
mc.setReadOnly(readOnly);
this.readOnly = readOnly;
mc.setTpbReadOnly(readOnly);
}
}

@Override
public boolean isReadOnly() throws SQLException {
try (LockCloseable ignored = withLock()) {
checkValidity();
return mc.isReadOnly();
return readOnly;
}
}

2 changes: 1 addition & 1 deletion src/main/org/firebirdsql/jdbc/FBConnectionProperties.java
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ public Object clone() {
clone.properties = (FbConnectionProperties) properties.asNewMutable();
clone.properties.registerPropertyUpdateListener(clone.createPropertyUpdateListener());
clone.customMapping = new HashMap<>(customMapping);
clone.mapper = mapper != null ? (FBTpbMapper) mapper.clone() : null;
clone.mapper = mapper != null ? FBTpbMapper.copyOf(mapper) : null;

return clone;
} catch (CloneNotSupportedException ex) {
Loading

0 comments on commit 9177bae

Please sign in to comment.