Skip to content

Commit

Permalink
Misc code cleanup/suppress Sonar warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrotteveel
mrotteveel committed Jan 13, 2024
1 parent e5156b2 commit 2752f1f
Show file tree
Hide file tree
Showing 13 changed files with 233 additions and 238 deletions.
77 changes: 40 additions & 37 deletions src/main/org/firebirdsql/jaybird/xca/FBManagedConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.firebirdsql.gds.impl.GDSHelper;
import org.firebirdsql.gds.impl.GDSServerVersion;
import org.firebirdsql.gds.ng.*;
import org.firebirdsql.gds.ng.fields.RowDescriptor;
import org.firebirdsql.gds.ng.fields.RowValue;
import org.firebirdsql.gds.ng.listeners.DatabaseListener;
import org.firebirdsql.gds.ng.listeners.ExceptionListener;
Expand All @@ -37,7 +38,6 @@
import javax.transaction.xa.XAException;
import javax.transaction.xa.XAResource;
import javax.transaction.xa.Xid;
import java.io.IOException;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.SQLNonTransientConnectionException;
Expand Down Expand Up @@ -294,6 +294,7 @@ private void forceDisassociateConnections() {
* @throws SQLException
* generic exception if operation fails
*/
@SuppressWarnings("java:S2095")
public FBConnection getConnection() throws SQLException {
disassociateConnections();

Expand Down Expand Up @@ -513,9 +514,8 @@ void internalEnd(Xid xid, int flags) throws XAException {
if (isCurrentTransaction(endingTr)) {
clearCurrentTransaction();
} else {
throw new FBXAException(
"You are trying to %s a transaction that is not the current transaction"
.formatted(flags == XAResource.TMSUSPEND ? "suspend" : "end"),
throw new FBXAException("You are trying to %s a transaction that is not the current transaction"
.formatted(flags == XAResource.TMSUSPEND ? "suspend" : "end"),
XAException.XAER_INVAL);
}
}
Expand Down Expand Up @@ -547,45 +547,31 @@ private void forget(Xid id) throws XAException {
// 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());
FbStatement stmtHandle2 = database.createStatement(trHandle2);

var gdsHelper2 = new GDSHelper(database);
gdsHelper2.setCurrentTransaction(trHandle2);

stmtHandle2.prepare(getXidQueries().forgetFindQuery());

var dataProvider = new DataProvider(stmtHandle2);
stmtHandle2.addStatementListener(dataProvider);

var field0 = FBField.createField(stmtHandle2.getRowDescriptor().getFieldDescriptor(0), dataProvider.asFieldDataProvider(0), gdsHelper2, false);
var field1 = FBField.createField(stmtHandle2.getRowDescriptor().getFieldDescriptor(1), dataProvider.asFieldDataProvider(1), gdsHelper2, false);
try (FbStatement stmtHandle2 = database.createStatement(trHandle2)) {
var gdsHelper2 = new GDSHelper(database);
gdsHelper2.setCurrentTransaction(trHandle2);

while (dataProvider.next()) {
long inLimboTxId = field0.getLong();
byte[] inLimboMessage = field1.getBytes();
stmtHandle2.prepare(getXidQueries().forgetFindQuery());

try {
var xid = new FBXid(inLimboMessage, inLimboTxId);
var dataProvider = new DataProvider(stmtHandle2);
stmtHandle2.addStatementListener(dataProvider);

boolean gtridEquals = Arrays.equals(xid.getGlobalTransactionId(), id.getGlobalTransactionId());
boolean bqualEquals = Arrays.equals(xid.getBranchQualifier(), id.getBranchQualifier());
RowDescriptor rowDescriptor = stmtHandle2.getRowDescriptor();
var field0 = FBField.createField(rowDescriptor.getFieldDescriptor(0),
dataProvider.asFieldDataProvider(0), gdsHelper2, false);
var field1 = FBField.createField(rowDescriptor.getFieldDescriptor(1),
dataProvider.asFieldDataProvider(1), gdsHelper2, false);

if (gtridEquals && bqualEquals) {
while (dataProvider.next()) {
long inLimboTxId = field0.getLong();
if (matchesXid(id, inLimboTxId, field1.getBytes())) {
inLimboId = inLimboTxId;
break;
}
} catch (FBIncorrectXidException ex) {
if (log.isLoggable(WARNING)) {
String message =
"incorrect XID format in RDB$TRANSACTIONS where RDB$TRANSACTION_ID=" + inLimboTxId;
log.log(WARNING, message + "; see debug level for stacktrace", ex);
log.log(DEBUG, message, ex);
}
}
} finally {
trHandle2.commit();
}

stmtHandle2.close();
trHandle2.commit();
} catch (SQLException ex) {
log.log(DEBUG, "can't perform query to fetch xids", ex);
throw new FBXAException(XAException.XAER_RMFAIL, ex);
Expand All @@ -609,6 +595,22 @@ private void forget(Xid id) throws XAException {
}
}

private static boolean matchesXid(Xid id, long fbTxId, byte[] fbTxMessage) {
try {
var xid = new FBXid(fbTxMessage, fbTxId);
return Arrays.equals(xid.getGlobalTransactionId(), id.getGlobalTransactionId())
&& Arrays.equals(xid.getBranchQualifier(), id.getBranchQualifier());
} catch (FBIncorrectXidException ex) {
if (log.isLoggable(WARNING)) {
String message = "incorrect XID format in RDB$TRANSACTIONS where RDB$TRANSACTION_ID=%d: %s"
.formatted(fbTxId, ByteArrayHelper.toHexString(fbTxMessage));
log.log(WARNING, message + "; see debug level for stacktrace", ex);
log.log(DEBUG, message, ex);
}
return false;
}
}

private int getTransactionTimeout() throws XAException {
return timeout;
}
Expand Down Expand Up @@ -716,12 +718,12 @@ private Xid[] recover(int flags) throws javax.transaction.xa.XAException {
}

return xids.toArray(new Xid[0]);
} catch (SQLException | IOException e) {
} catch (SQLException e) {
throw new FBXAException("can't perform query to fetch xids", XAException.XAER_RMFAIL, e);
}
}

private static FBXid extractXid(byte[] xidData, long txId) throws IOException {
private static FBXid extractXid(byte[] xidData, long txId) {
try {
return new FBXid(xidData, txId);
} catch (FBIncorrectXidException e) {
Expand Down Expand Up @@ -774,7 +776,7 @@ Xid findSingleXid(Xid externalXid) throws javax.transaction.xa.XAException {
} finally {
trHandle2.commit();
}
} catch (SQLException | IOException e) {
} catch (SQLException e) {
throw new FBXAException("can't perform query to fetch xids", XAException.XAER_RMFAIL, e);
}
}
Expand Down Expand Up @@ -1167,6 +1169,7 @@ public boolean isReadOnly() {
return tpb.isReadOnly();
}

@SuppressWarnings("java:S135")
private void notifyWarning(SQLWarning warning) {
final FBConnection connection = connectionHandle;
if (connection == null) {
Expand Down
39 changes: 19 additions & 20 deletions src/main/org/firebirdsql/jaybird/xca/FBManagedConnectionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ public void recover(FBManagedConnection mc, Xid xid) {
* @throws XAException
* if "in limbo" transaction cannot be completed.
*/
@SuppressWarnings("java:S1141")
private void tryCompleteInLimboTransaction(Xid xid, boolean commit) throws XAException {
try {
FBManagedConnection tempMc = null;
Expand Down Expand Up @@ -571,29 +572,14 @@ private void tryCompleteInLimboTransaction(Xid xid, boolean commit) throws XAExc
throw new FBXAException("unable to remove in limbo transaction from rdb$transactions where rdb$transaction_id = " + fbTransactionId, XAException.XAER_RMERR);
}
}
} catch (SQLException ex) {
/*
* if ex.getIntParam() is 335544353 (transaction is not in limbo) and next ex.getIntParam() is 335544468 (transaction {0} is {1})
* => detected heuristic
*/
// TODO: We may need to parse the exception to get the details (or we need to handle this specific one differently)
int errorCode = XAException.XAER_RMERR;
int sqlError = ex.getErrorCode();
//int nextIntParam = ex.getNext().getIntParam();

if (sqlError == ISCConstants.isc_no_recon /*&& nextIntParam == ISCConstants.isc_tra_state*/) {
if (ex.getMessage().contains("committed")) {
errorCode = XAException.XA_HEURCOM;
} else if (ex.getMessage().contains("rolled back")) {
errorCode = XAException.XA_HEURCOM;
}
}

throw new FBXAException("unable to complete in limbo transaction", errorCode, ex);
} catch (SQLException e) {
throw new FBXAException("unable to complete in limbo transaction",
determineLimboCompletionErrorCode(e), e);
} finally {
try {
if (tempLocalTx != null && tempLocalTx.inTransaction())
if (tempLocalTx != null && tempLocalTx.inTransaction()) {
tempLocalTx.commit();
}
} finally {
if (tempMc != null) tempMc.destroy();
}
Expand All @@ -603,6 +589,19 @@ private void tryCompleteInLimboTransaction(Xid xid, boolean commit) throws XAExc
}
}

private static int determineLimboCompletionErrorCode(SQLException ex) {
/* if ex.getIntParam() is 335544353 (transaction is not in limbo) and next ex.getIntParam() is 335544468
(transaction {0} is {1}) => detected heuristic */
// TODO: We may need to parse the exception to get the details (or we need to handle this specific one differently)
if (ex.getErrorCode() == ISCConstants.isc_no_recon) {
String message = ex.getMessage();
if (message.contains("committed") || message.contains("rolled back")) {
return XAException.XA_HEURCOM;
}
}
return XAException.XAER_RMERR;
}

FBConnection newConnection(FBManagedConnection mc) throws SQLException {
Class<?> connectionClass = GDSFactory.getConnectionClass(getGDSType());
return connectionClass == FBConnection.class ? new FBConnection(mc) : newConnection(mc, connectionClass);
Expand Down
2 changes: 2 additions & 0 deletions src/main/org/firebirdsql/jdbc/FBObjectListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ public interface ResultSetListener {
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.ResultSetListener} that implements all methods as
* empty methods.
*/
@SuppressWarnings("java:S1186")
public static final class NoActionResultSetListener implements ResultSetListener {

private static final ResultSetListener INSTANCE = new NoActionResultSetListener();
Expand Down Expand Up @@ -223,6 +224,7 @@ public interface BlobListener {
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.BlobListener} that implements all methods as
* empty methods.
*/
@SuppressWarnings("java:S1186")
public static final class NoActionBlobListener implements BlobListener {

private static final BlobListener INSTANCE = new NoActionBlobListener();
Expand Down
2 changes: 1 addition & 1 deletion src/main/org/firebirdsql/jdbc/FBPreparedStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,7 @@ public void addBatch(BatchRowValue rowValue) throws SQLException {
batchRowValues.addLast(rowValue);
}

private boolean isEmpty() throws SQLException {
private boolean isEmpty() {
return batchRowValues.isEmpty();
}

Expand Down
30 changes: 9 additions & 21 deletions src/main/org/firebirdsql/jdbc/FBProcedureCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public Object clone() {

return newProcedureCall;
} catch (CloneNotSupportedException e) {
return null;
throw new AssertionError("clone() unexpectedly not supported", e);
}
}

Expand Down Expand Up @@ -311,20 +311,12 @@ public String getSQL(boolean select) throws SQLException {
*/
public void checkParameters() throws SQLException {
for (FBProcedureParam param : inputParams) {
if (param == null) {
continue;
}

// if parameter does not have set value, and is not registered
// as output parameter, throw an exception, otherwise, continue
// to the next one.
if (!param.isValueSet()) {
if (param.isParam()
&& !outputParams.isEmpty()
// if parameter does not have set value, and is not registered as output parameter, throw an exception,
// otherwise, continue to the next one.
if (param != null && !param.isValueSet() && param.isParam() && !outputParams.isEmpty()
&& outputParams.get(param.getPosition()) == null) {
throw new SQLException("Value of parameter %d not set and it was not registered as output parameter"
.formatted(param.getIndex()), SQL_STATE_WRONG_PARAM_NUM);
}
throw new SQLException("Value of parameter %d not set and it was not registered as output parameter"
.formatted(param.getIndex()), SQL_STATE_WRONG_PARAM_NUM);
}
}
}
Expand All @@ -337,18 +329,14 @@ public void checkParameters() throws SQLException {
*/
public boolean equals(Object obj) {
if (obj == this) return true;
if (!(obj instanceof FBProcedureCall other)) return false;
return Objects.equals(name, other.name)
return obj instanceof FBProcedureCall other
&& Objects.equals(name, other.name)
&& inputParams.equals(other.inputParams)
&& outputParams.equals(other.outputParams);
}

public int hashCode() {
int hashCode = 547;
hashCode = 37 * hashCode + (name != null ? name.hashCode() : 0);
hashCode = 37 * hashCode + inputParams.hashCode();
hashCode = 37 * hashCode + outputParams.hashCode();
return hashCode;
return Objects.hash(name, inputParams, outputParams);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/main/org/firebirdsql/jdbc/FBProcedureParam.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ public void setType(int type) {
public boolean equals(Object obj) {
if (obj == this) return true;
if (!(obj instanceof FBProcedureParam other)) return false;
// TODO this condition is flawed, but somehow current code relies on it
return this.position == other.position &&
this.value != null ? this.value.equals(other.value) :
other.value == null;
Expand All @@ -159,7 +160,7 @@ public Object clone() {
try {
return super.clone();
} catch (CloneNotSupportedException e) {
return null;
throw new AssertionError("clone() unexpectedly not supported", e);
}
}
}
Loading

0 comments on commit 2752f1f

Please sign in to comment.