#867 FbAttachment.close() shouldn't throw an exception if not connected or already closed

Author: mrotteveel

jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java

@@ -69,12 +69,17 @@ protected void setDetachedJna() {
     }
 
     @Override
-    protected void checkConnected() throws SQLException {
-        if (!isAttached()) {
+    protected final void checkConnected() throws SQLException {
+        if (!isConnected()) {
             throw FbExceptionBuilder.toException(JaybirdErrorCodes.jb_notAttachedToDatabase);
         }
     }
 
+    @Override
+    protected final boolean isConnected() {
+        return isAttached();
+    }
+
     @Override
     protected void internalDetach() throws SQLException {
         try (LockCloseable ignored = withLock()) {

jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2015-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2015-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng.jna;
 
@@ -71,6 +71,11 @@ protected void checkConnected() throws SQLException {
         }
     }
 
+    @Override
+    protected boolean isConnected() {
+        return isAttached();
+    }
+
     @Override
     public byte[] getServiceInfo(ServiceParameterBuffer serviceParameterBuffer,
             ServiceRequestBuffer serviceRequestBuffer, int maxBufferLength) throws SQLException {

src/docs/asciidoc/release_notes.adoc

@@ -589,6 +589,8 @@ This means there may be undocumented changes to internal API.
 If you are confronted with such a change, let us know on {firebird-java}[firebird-java], so we can take this into account when documenting future changes.
 ====
 
+* `FbAttachment`
+** The `close()` method should no longer throw an exception if already closed, or not connected/attached.
 * `FbWireAsynchronousChannel`
 ** `connect(String, int, int)` was replaced by `connect(String, int)`
 * `FbWireOperations`

src/jna-test/org/firebirdsql/gds/ng/jna/JnaDatabaseTest.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2014-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2014-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng.jna;
 
@@ -163,19 +163,15 @@ void testDrop_NotAttached() throws Exception {
 
     @SuppressWarnings("resource")
     @Test
-    void testDetach_NotConnected() throws Exception {
+    void testClose_notConnected() throws Exception {
         JnaDatabase db = factory.connect(connectionInfo);
 
-        SQLException exception = assertThrows(SQLException.class, db::close);
-        // Note: the error is different from the one in the pure java implementation as we cannot discern between
-        // not connected and not attached
-        assertThat(exception, allOf(
-                message(startsWith("The connection is not attached to a database")),
-                sqlStateEquals(SQLStateConstants.SQL_STATE_CONNECTION_ERROR)));
+        // Close for not connected should work (no-op)
+        assertDoesNotThrow(db::close);
     }
 
     @Test
-    void testBasicDetach() throws Exception {
+    void testBasicClose() throws Exception {
         usesDatabase.createDefaultDatabase();
         JnaDatabase db = factory.connect(connectionInfo);
         try {
@@ -190,7 +186,7 @@ void testBasicDetach() throws Exception {
     }
 
     @Test
-    void testDetach_openTransactions() throws Exception {
+    void testClose_openTransactions() throws Exception {
         usesDatabase.createDefaultDatabase();
         JnaDatabase db = factory.connect(connectionInfo);
         FbTransaction transaction = null;

src/main/org/firebirdsql/gds/ng/AbstractFbAttachment.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2015-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2015-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng;
 
@@ -152,6 +152,18 @@ public final void removeExceptionListener(ExceptionListener listener) {
      */
     protected abstract void checkConnected() throws SQLException;
 
+    /**
+     * Returns if this attachment is connected as checked by {@link #checkConnected()}.
+     * <p>
+     * In general, {@link #isAttached()} should be used. An attachment might be connected (e.g. TCP/IP connection
+     * established to the server), but not (yet) attached to a database or service.
+     * </p>
+     *
+     * @return {@code true} if connected
+     * @since 7
+     */
+    protected abstract boolean isConnected();
+
     /**
      * Performs {@link #close()} suppressing any exception.
      */

src/main/org/firebirdsql/gds/ng/AbstractFbDatabase.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2013-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2013-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng;
 
@@ -152,7 +152,7 @@ public final void removeDatabaseListener(DatabaseListener listener) {
     @Override
     public final void close() throws SQLException {
         try (LockCloseable ignored = withLock()) {
-            checkConnected();
+            if (!isConnected()) return;
             int activeTransactionCount = getActiveTransactionCount();
             if (activeTransactionCount > 0) {
                 // Throw open transactions as exception, fbclient doesn't disconnect with outstanding (unprepared)

src/main/org/firebirdsql/gds/ng/AbstractFbService.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2015-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2015-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng;
 
@@ -76,7 +76,7 @@ public final void removeServiceListener(ServiceListener listener) {
     @Override
     public final void close() throws SQLException {
         try (LockCloseable ignored = withLock()) {
-            checkConnected();
+            if (!isConnected()) return;
             serviceListenerDispatcher.detaching(this);
             try {
                 internalDetach();

src/main/org/firebirdsql/gds/ng/FbAttachment.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2013-2022 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2013-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later OR BSD-3-Clause
 package org.firebirdsql.gds.ng;
 
@@ -29,9 +29,12 @@ public interface FbAttachment extends AutoCloseable, ExceptionListenable {
 
     /**
      * Detaches and closes the connection.
+     * <p>
+     * If the attachment is already closed or otherwise not connected, this is a no-op.
+     * </p>
      *
      * @throws SQLException
-     *         If not currently connected, or another problem occurred detaching.
+     *         if a problem occurred detaching.
      */
     @Override
     void close() throws SQLException;

src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireDatabase.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2013-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2013-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng.wire;
 
@@ -107,22 +107,32 @@ public final FbWireOperations getWireOperations() {
 
     @Override
     public final boolean isAttached() {
-        return super.isAttached() && connection.isConnected();
+        return super.isAttached() && isConnected();
     }
 
     /**
      * Checks if a physical connection to the server is established.
      *
      * @throws SQLException
-     *         If not connected.
+     *         if not connected
      */
     @Override
     protected final void checkConnected() throws SQLException {
-        if (!connection.isConnected()) {
+        if (!isConnected()) {
             throw FbExceptionBuilder.toNonTransientConnectionException(jb_notConnectedToServer);
         }
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * @return {@code true} if a physical connection to the server is established
+     */
+    @Override
+    protected final boolean isConnected() {
+        return connection.isConnected();
+    }
+
     /**
      * Checks if a physical connection to the server is established and if the
      * connection is attached to a database.

src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireService.java

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2015-2024 Mark Rotteveel
+// SPDX-FileCopyrightText: Copyright 2015-2025 Mark Rotteveel
 // SPDX-License-Identifier: LGPL-2.1-or-later
 package org.firebirdsql.gds.ng.wire;
 
@@ -78,7 +78,7 @@ public final ServiceRequestBuffer createServiceRequestBuffer() {
 
     @Override
     public final boolean isAttached() {
-        return super.isAttached() && connection.isConnected();
+        return super.isAttached() && isConnected();
     }
 
     /**
@@ -88,11 +88,21 @@ public final boolean isAttached() {
      *         If not connected.
      */
     protected final void checkConnected() throws SQLException {
-        if (!connection.isConnected()) {
+        if (!isConnected()) {
             throw FbExceptionBuilder.toNonTransientConnectionException(JaybirdErrorCodes.jb_notConnectedToServer);
         }
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * @return {@code true} if a physical connection to the server is established
+     */
+    @Override
+    protected final boolean isConnected() {
+        return connection.isConnected();
+    }
+
     /**
      * Checks if a physical connection to the server is established and if the
      * connection is attached to a database.