From 3c88c882e26f4f01457261c0c6f5257ac2c80c4d Mon Sep 17 00:00:00 2001
From: Mark Rotteveel <mark@lawinegevaar.nl>
Date: Tue, 31 Oct 2023 14:21:53 +0100
Subject: [PATCH] Delete various TODOs

---
 .../org/firebirdsql/gds/ng/jna/JnaDatabase.java    | 14 ++++----------
 .../org/firebirdsql/gds/ng/jna/JnaService.java     |  2 --
 .../org/firebirdsql/gds/impl/GDSServerVersion.java |  4 ++--
 .../firebirdsql/gds/ng/AbstractFbStatement.java    |  1 -
 .../org/firebirdsql/gds/ng/FbExceptionBuilder.java |  1 -
 .../org/firebirdsql/gds/ng/SqlCountProcessor.java  |  1 -
 .../org/firebirdsql/gds/ng/tz/TimeZoneMapping.java |  1 -
 .../gds/ng/wire/auth/ClientAuthBlock.java          |  1 -
 .../gds/ng/wire/version12/V12Database.java         |  1 -
 .../gds/ng/wire/version13/V13WireOperations.java   |  2 --
 .../firebirdsql/jdbc/escape/ConvertFunction.java   |  4 ++--
 .../jdbc/metadata/FbMetadataConstants.java         |  2 --
 12 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java b/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java
index 545fb6bee..8df032b5f 100644
--- a/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java
+++ b/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java
@@ -51,8 +51,6 @@
 public class JnaDatabase extends AbstractFbDatabase<JnaDatabaseConnection>
         implements JnaAttachment, TransactionListener, FbClientFeatureAccess {
 
-    // TODO Find out if there are any exception from JNA that we need to be prepared to handle.
-
     private static final ParameterConverter<JnaDatabaseConnection, ?> PARAMETER_CONVERTER = new JnaParameterConverter();
     public static final int STATUS_VECTOR_SIZE = 20;
     public static final int MAX_STATEMENT_LENGTH = 64 * 1024;
@@ -211,8 +209,7 @@ public void cancelOperation(int kind) throws SQLException {
             if (kind != fb_cancel_abort) {
                 checkConnected();
             }
-            // TODO Test what happens with 2.1 and earlier client library
-            // No synchronization, otherwise cancel will never work; might conflict with sync policy of JNA (TODO: find out)
+            // No synchronization, otherwise cancel will never work; might conflict with sync policy of JNA
             try {
                 clientLibrary.fb_cancel_operation(statusVector, handle, (short) kind);
             } finally {
@@ -395,14 +392,11 @@ protected JnaEventHandle validateEventHandle(EventHandle eventHandle) throws SQL
 
     @Override
     public JnaEventHandle createEventHandle(String eventName, EventHandler eventHandler) throws SQLException {
-        // TODO Any JNA errors we need to track and convert to SQLException here?
         final JnaEventHandle eventHandle = new JnaEventHandle(eventName, eventHandler, getEncoding());
         try (LockCloseable ignored = withLock()) {
-            synchronized (eventHandle) {
-                int size = clientLibrary.isc_event_block(eventHandle.getEventBuffer(), eventHandle.getResultBuffer(),
-                        (short) 1, eventHandle.getEventNameMemory());
-                eventHandle.setSize(size);
-            }
+            int size = clientLibrary.isc_event_block(eventHandle.getEventBuffer(), eventHandle.getResultBuffer(),
+                    (short) 1, eventHandle.getEventNameMemory());
+            eventHandle.setSize(size);
         }
         return eventHandle;
     }
diff --git a/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java b/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java
index 3b8ef2159..dd3215d96 100644
--- a/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java
+++ b/jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java
@@ -47,8 +47,6 @@
  */
 public final class JnaService extends AbstractFbService<JnaServiceConnection> implements JnaAttachment {
 
-    // TODO Find out if there are any exception from JNA that we need to be prepared to handle.
-
     private static final ParameterConverter<?, JnaServiceConnection> PARAMETER_CONVERTER = new JnaParameterConverter();
     public static final int STATUS_VECTOR_SIZE = 20;
 
diff --git a/src/main/org/firebirdsql/gds/impl/GDSServerVersion.java b/src/main/org/firebirdsql/gds/impl/GDSServerVersion.java
index b8331ef3b..2f58e97e8 100644
--- a/src/main/org/firebirdsql/gds/impl/GDSServerVersion.java
+++ b/src/main/org/firebirdsql/gds/impl/GDSServerVersion.java
@@ -159,7 +159,7 @@ public String getFullVersion() {
      */
     public int getProtocolVersion() {
         // We assume the protocol information is in the second version string;
-        // TODO this assumption may be wrong for multi-hop connections
+        // this assumption may be wrong for multi-hop connections
         if (rawVersions.length == 1 || rawVersions[1] == null) return -1;
         Matcher connectionMetadataMatcher = CONNECTION_METADATA_PATTERN.matcher(rawVersions[1]);
         if (!connectionMetadataMatcher.find()) return -1;
@@ -182,7 +182,7 @@ public boolean isWireCompressionUsed() {
 
     private String getConnectionOptions() {
         // We assume the protocol information is in the second version string;
-        // TODO this assumption may be wrong for multi-hop connections
+        // this assumption may be wrong for multi-hop connections
         if (rawVersions.length == 1 || rawVersions[1] == null) return "";
         Matcher connectionMetadataMatcher = CONNECTION_METADATA_PATTERN.matcher(rawVersions[1]);
         if (!connectionMetadataMatcher.find()) return "";
diff --git a/src/main/org/firebirdsql/gds/ng/AbstractFbStatement.java b/src/main/org/firebirdsql/gds/ng/AbstractFbStatement.java
index 6a65663c9..1bde2b9ea 100644
--- a/src/main/org/firebirdsql/gds/ng/AbstractFbStatement.java
+++ b/src/main/org/firebirdsql/gds/ng/AbstractFbStatement.java
@@ -164,7 +164,6 @@ protected final void checkFetchSize(int fetchSize) throws SQLException {
     public void close() throws SQLException {
         if (getState() == StatementState.CLOSED) return;
         try (LockCloseable ignored = withLock()) {
-            // TODO do additional checks (see also old implementation and .NET)
             try {
                 final StatementState currentState = getState();
                 forceState(StatementState.CLOSING);
diff --git a/src/main/org/firebirdsql/gds/ng/FbExceptionBuilder.java b/src/main/org/firebirdsql/gds/ng/FbExceptionBuilder.java
index 6baf30ecb..f81db0fa4 100644
--- a/src/main/org/firebirdsql/gds/ng/FbExceptionBuilder.java
+++ b/src/main/org/firebirdsql/gds/ng/FbExceptionBuilder.java
@@ -744,7 +744,6 @@ private enum Type {
         EXCEPTION(SQLStateConstants.SQL_STATE_GENERAL_ERROR) {
             @Override
             public SQLException createSQLException(final String message, final String sqlState, final int errorCode) {
-                // TODO Replace with a list or chain of processors?
                 if (sqlState != null) {
                     if (sqlState.startsWith(SQLSTATE_FEATURE_NOT_SUPPORTED_PREFIX)) {
                         // Feature not supported by Firebird
diff --git a/src/main/org/firebirdsql/gds/ng/SqlCountProcessor.java b/src/main/org/firebirdsql/gds/ng/SqlCountProcessor.java
index 6a7131dd7..df5ce5eaa 100644
--- a/src/main/org/firebirdsql/gds/ng/SqlCountProcessor.java
+++ b/src/main/org/firebirdsql/gds/ng/SqlCountProcessor.java
@@ -83,7 +83,6 @@ public SqlCountHolder process(byte[] infoResponse) throws SQLException {
             // NOTE: we assume we always use a sufficiently large buffer
         } else if (infoResponse[0] == ISCConstants.isc_info_end) {
             // Happens with statement types that don't have update counts, etc (eg DDL)
-            // TODO Return all -1 instead?
             return new SqlCountHolder(0, 0, 0, 0);
         } else {
             throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_unexpectedInfoResponse)
diff --git a/src/main/org/firebirdsql/gds/ng/tz/TimeZoneMapping.java b/src/main/org/firebirdsql/gds/ng/tz/TimeZoneMapping.java
index 7def56265..930d99592 100644
--- a/src/main/org/firebirdsql/gds/ng/tz/TimeZoneMapping.java
+++ b/src/main/org/firebirdsql/gds/ng/tz/TimeZoneMapping.java
@@ -341,7 +341,6 @@ private static int timeZoneId(final int internalId) {
     }
 
     private static List<String> loadTimeZoneNameById() {
-        // TODO Potential loading issues?
         // TODO Just hard code it instead?
         try {
             final Properties timeZoneMapping = loadTimeZoneMapping();
diff --git a/src/main/org/firebirdsql/gds/ng/wire/auth/ClientAuthBlock.java b/src/main/org/firebirdsql/gds/ng/wire/auth/ClientAuthBlock.java
index 6eef68673..4507113ef 100644
--- a/src/main/org/firebirdsql/gds/ng/wire/auth/ClientAuthBlock.java
+++ b/src/main/org/firebirdsql/gds/ng/wire/auth/ClientAuthBlock.java
@@ -168,7 +168,6 @@ public void resetClient(byte[] serverInfo) throws SQLException {
 
     public void setServerData(byte[] serverData) {
         if (currentPlugin == null) {
-            // TODO Check if this is valid to ignore
             log.log(DEBUG, "Received server data without current plugin");
         } else {
             currentPlugin.setServerData(serverData);
diff --git a/src/main/org/firebirdsql/gds/ng/wire/version12/V12Database.java b/src/main/org/firebirdsql/gds/ng/wire/version12/V12Database.java
index af76bca7c..0d3f4251d 100644
--- a/src/main/org/firebirdsql/gds/ng/wire/version12/V12Database.java
+++ b/src/main/org/firebirdsql/gds/ng/wire/version12/V12Database.java
@@ -64,7 +64,6 @@ public void cancelOperation(int kind) throws SQLException {
                 checkConnected();
                 try {
                     // We circumvent the normal xdrOut to minimize the chance of interleaved writes
-                    // TODO We may still need to do separate write / read synchronization to ensure this works correctly
                     ByteArrayOutputStream out = new ByteArrayOutputStream(8);
                     try (XdrOutputStream xdr = new XdrOutputStream(out, 8)) {
                         xdr.writeInt(WireProtocolConstants.op_cancel);
diff --git a/src/main/org/firebirdsql/gds/ng/wire/version13/V13WireOperations.java b/src/main/org/firebirdsql/gds/ng/wire/version13/V13WireOperations.java
index b357156ce..ffcf39f33 100644
--- a/src/main/org/firebirdsql/gds/ng/wire/version13/V13WireOperations.java
+++ b/src/main/org/firebirdsql/gds/ng/wire/version13/V13WireOperations.java
@@ -84,7 +84,6 @@ public void authReceiveResponse(FbWireAttachment.AcceptPacket acceptPacket, DbCr
                 data = acceptPacket.p_acpt_data;
                 pluginName = acceptPacket.p_acpt_plugin;
                 addServerKeys(acceptPacket.p_acpt_keys);
-                // TODO handle compression
                 acceptPacket = null;
             } else {
                 int operation = readNextOperation();
@@ -115,7 +114,6 @@ public void authReceiveResponse(FbWireAttachment.AcceptPacket acceptPacket, DbCr
                     xdrIn.skipNBytes(4); // skip int: p_acpt_authenticated
                     addServerKeys(xdrIn.readBuffer()); //p_acpt_keys
                 }
-                // TODO handle compression
                 case op_response -> {
                     GenericResponse response = (GenericResponse) readOperationResponse(operation, null);
                     boolean wasAuthComplete = clientAuthBlock.isAuthComplete();
diff --git a/src/main/org/firebirdsql/jdbc/escape/ConvertFunction.java b/src/main/org/firebirdsql/jdbc/escape/ConvertFunction.java
index 40660f457..e56e90b50 100644
--- a/src/main/org/firebirdsql/jdbc/escape/ConvertFunction.java
+++ b/src/main/org/firebirdsql/jdbc/escape/ConvertFunction.java
@@ -41,8 +41,8 @@
  *  <li>{@code LONGVARBINARY}, {@code BLOB} will be cast to {@code BLOB SUB_TYPE BINARY}</li>
  *  <li>{@code TINYINT} is mapped to {@code SMALLINT}</li>
  *  <li>{@code ROWID} is not supported as length of {@code DB_KEY} values depend on the context
- *  TODO: consider cast to CHAR(8) character set binary or maybe multiples of 8?</li>
- *  <li>{@code `(SQL_)DECIMAL`} and {@code `(SQL_)NUMERIC`} without precision and scale are passed as is, in current
+ *  <!-- TODO: consider cast to CHAR(8) character set binary or maybe multiples of 8? --></li>
+ *  <li>{@code (SQL_)DECIMAL} and {@code (SQL_)NUMERIC} without precision and scale are passed as is, in current
  *  Firebird versions, this means the value will be equivalent to {@code DECIMAL(9,0)} (which is equivalent to
  *  {@code INTEGER})</li>
  *  <li>Extension not defined in JDBC: {@code TIME_WITH_TIMEZONE/TIME_WITH_TIME_ZONE} and
diff --git a/src/main/org/firebirdsql/jdbc/metadata/FbMetadataConstants.java b/src/main/org/firebirdsql/jdbc/metadata/FbMetadataConstants.java
index 8fa8d1dc2..0ad01501c 100644
--- a/src/main/org/firebirdsql/jdbc/metadata/FbMetadataConstants.java
+++ b/src/main/org/firebirdsql/jdbc/metadata/FbMetadataConstants.java
@@ -64,8 +64,6 @@ public final class FbMetadataConstants {
     public static final int OBJECT_NAME_LENGTH_V4_0 = 63;
     public static final int OBJECT_NAME_LENGTH = OBJECT_NAME_LENGTH_V4_0;
 
-    // TODO Double check if these are the same as the blr constants or not.
-    // TODO And if they are the same, check missing types (like text2 = 15, varying2 = 38)
     public static final int smallint_type = 7;
     public static final int integer_type = 8;
     public static final int quad_type = 9;