fix(conformance): make namespace cleanup failures visible (#52)

Author: magnusp

libsql-conformance/src/test/java/com/github/magnusp/libsql/conformance/NamespaceClosingHandlerTest.java

@@ -0,0 +1,121 @@
+package com.github.magnusp.libsql.conformance;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.lang.reflect.Proxy;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import org.junit.jupiter.api.Test;
+
+class NamespaceClosingHandlerTest {
+
+    @Test
+    void closeReleasesNamespaceOnlyOnce() throws SQLException {
+        List<String> released = new ArrayList<>();
+        List<String> calls = new ArrayList<>();
+        Connection connection = proxy(connection(calls, false, false), released::add);
+
+        connection.close();
+        connection.close();
+
+        assertThat(released).containsExactly("ns");
+        assertThat(calls).containsExactly("close");
+    }
+
+    @Test
+    void closingAnUnwrappedConnectionReleasesNamespace() throws SQLException {
+        List<String> released = new ArrayList<>();
+        List<String> calls = new ArrayList<>();
+        Connection connection = proxy(connection(calls, false, false), released::add);
+
+        Connection unwrapped = connection.unwrap(Connection.class);
+        assertThat(unwrapped).isSameAs(connection);
+
+        unwrapped.close();
+
+        assertThat(released).containsExactly("ns");
+        assertThat(calls).containsExactly("close");
+    }
+
+    @Test
+    void abortReleasesNamespace() throws SQLException {
+        List<String> released = new ArrayList<>();
+        List<String> calls = new ArrayList<>();
+        Connection connection = proxy(connection(calls, false, false), released::add);
+
+        connection.abort(Runnable::run);
+
+        assertThat(released).containsExactly("ns");
+        assertThat(calls).containsExactly("abort");
+    }
+
+    @Test
+    void closeFailureStillReleasesNamespace() {
+        List<String> released = new ArrayList<>();
+        List<String> calls = new ArrayList<>();
+        Connection connection = proxy(connection(calls, true, false), released::add);
+
+        assertThatThrownBy(connection::close)
+                .isInstanceOf(SQLException.class)
+                .hasMessage("close failed");
+
+        assertThat(released).containsExactly("ns");
+    }
+
+    @Test
+    void abortFailureStillReleasesNamespace() {
+        List<String> released = new ArrayList<>();
+        List<String> calls = new ArrayList<>();
+        Connection connection = proxy(connection(calls, false, true), released::add);
+
+        assertThatThrownBy(() -> connection.abort(Runnable::run))
+                .isInstanceOf(SQLException.class)
+                .hasMessage("abort failed");
+
+        assertThat(released).containsExactly("ns");
+    }
+
+    private static Connection proxy(Connection connection, Consumer<String> release) {
+        return (Connection)
+                Proxy.newProxyInstance(
+                        Connection.class.getClassLoader(),
+                        new Class<?>[] {Connection.class},
+                        new XerialConformanceDriver.NamespaceClosingHandler(
+                                connection, "ns", release));
+    }
+
+    private static Connection connection(
+            List<String> calls, boolean failClose, boolean failAbort) {
+        return (Connection)
+                Proxy.newProxyInstance(
+                        Connection.class.getClassLoader(),
+                        new Class<?>[] {Connection.class},
+                        (proxy, method, args) ->
+                                switch (method.getName()) {
+                                    case "close" -> {
+                                        calls.add("close");
+                                        if (failClose) {
+                                            throw new SQLException("close failed");
+                                        }
+                                        yield null;
+                                    }
+                                    case "abort" -> {
+                                        calls.add("abort");
+                                        if (failAbort) {
+                                            throw new SQLException("abort failed");
+                                        }
+                                        yield null;
+                                    }
+                                    case "toString" -> "fake connection";
+                                    case "hashCode" -> System.identityHashCode(proxy);
+                                    case "equals" -> proxy == args[0];
+                                    default ->
+                                            throw new UnsupportedOperationException(
+                                                    method.getName());
+                                });
+    }
+}

libsql-conformance/src/test/java/com/github/magnusp/libsql/conformance/XerialConformanceDriver.java

@@ -15,6 +15,8 @@
 import java.sql.SQLFeatureNotSupportedException;
 import java.util.Properties;
 import java.util.UUID;
+import java.util.concurrent.Executor;
+import java.util.function.Consumer;
 import java.util.logging.Logger;
 
 public final class XerialConformanceDriver implements Driver {
@@ -43,7 +45,7 @@ public Connection connect(String url, Properties info) throws SQLException {
         try {
             connection = delegate.connect(target.url(), info);
         } catch (SQLException e) {
-            XerialTestServer.releaseNamespace(target.namespace());
+            releaseAfterConnectFailure(target.namespace(), e);
             throw e;
         }
         if (connection == null) {
@@ -57,7 +59,11 @@ public Connection connect(String url, Properties info) throws SQLException {
                             new Class<?>[] {Connection.class},
                             new NamespaceClosingHandler(connection, target.namespace()));
         } catch (RuntimeException e) {
-            XerialTestServer.releaseNamespace(target.namespace());
+            try {
+                XerialTestServer.releaseNamespace(target.namespace());
+            } catch (RuntimeException cleanup) {
+                e.addSuppressed(cleanup);
+            }
             throw e;
         }
     }
@@ -95,6 +101,14 @@ public Logger getParentLogger() throws SQLFeatureNotSupportedException {
         return delegate.getParentLogger();
     }
 
+    private static void releaseAfterConnectFailure(String namespace, SQLException failure) {
+        try {
+            XerialTestServer.releaseNamespace(namespace);
+        } catch (RuntimeException cleanup) {
+            failure.addSuppressed(cleanup);
+        }
+    }
+
     private static Target rewrite(String url) {
         String target = url.substring(PREFIX.length());
         String namespace;
@@ -112,15 +126,22 @@ private static Target rewrite(String url) {
 
     private record Target(String url, String namespace) {}
 
-    private static final class NamespaceClosingHandler implements InvocationHandler {
+    static final class NamespaceClosingHandler implements InvocationHandler {
 
         private final Connection connection;
         private final String namespace;
+        private final Consumer<String> release;
         private boolean closed;
 
-        private NamespaceClosingHandler(Connection connection, String namespace) {
+        NamespaceClosingHandler(Connection connection, String namespace) {
+            this(connection, namespace, XerialTestServer::releaseNamespace);
+        }
+
+        NamespaceClosingHandler(
+                Connection connection, String namespace, Consumer<String> release) {
             this.connection = connection;
             this.namespace = namespace;
+            this.release = release;
         }
 
         @Override
@@ -129,6 +150,16 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
                 close();
                 return null;
             }
+            if ("abort".equals(method.getName()) && method.getParameterCount() == 1) {
+                abort((Executor) args[0]);
+                return null;
+            }
+            if ("unwrap".equals(method.getName()) && method.getParameterCount() == 1) {
+                Class<?> iface = (Class<?>) args[0];
+                if (iface != null && iface.isInstance(proxy)) {
+                    return proxy;
+                }
+            }
             try {
                 return method.invoke(connection, args);
             } catch (InvocationTargetException e) {
@@ -143,8 +174,42 @@ private synchronized void close() throws SQLException {
             closed = true;
             try {
                 connection.close();
-            } finally {
-                XerialTestServer.releaseNamespace(namespace);
+            } catch (SQLException e) {
+                releaseWithSuppressed(e);
+                throw e;
+            } catch (RuntimeException e) {
+                releaseWithSuppressed(e);
+                throw e;
+            }
+            releaseNamespace();
+        }
+
+        private synchronized void abort(Executor executor) throws SQLException {
+            if (closed) {
+                return;
+            }
+            closed = true;
+            try {
+                connection.abort(executor);
+            } catch (SQLException e) {
+                releaseWithSuppressed(e);
+                throw e;
+            } catch (RuntimeException e) {
+                releaseWithSuppressed(e);
+                throw e;
+            }
+            releaseNamespace();
+        }
+
+        private void releaseNamespace() {
+            release.accept(namespace);
+        }
+
+        private void releaseWithSuppressed(Throwable failure) {
+            try {
+                releaseNamespace();
+            } catch (RuntimeException cleanup) {
+                failure.addSuppressed(cleanup);
             }
         }
     }