Skip to content

Commit a8e4345

Browse files
magnuspmagnusp
authored
fix(conformance): make namespace cleanup failures visible (#65)
Closes #52. Part of #54. ## What changed - `XerialTestServer.releaseNamespace` now checks the admin `DELETE` status. Transient responses (I/O, 408, 429, 5xx) are retried up to three times; any non-2xx response other than 404 is surfaced as an `IllegalStateException`. - A namespace whose delete failed is recorded as failed, and `ensureNamespace` refuses to reuse it. A 409 from namespace create is no longer treated as success, so a stale namespace cannot be silently adopted. - Cleanup failures during `connect` are attached as suppressed to the primary connection error. - `NamespaceClosingHandler` now releases on `close`, `abort`, and `unwrap(Connection.class)`. `unwrap` returns the proxy so closing the unwrapped handle still releases, and release still runs if the underlying close/abort throws. - Vendored `JSON1Test` closes its per-test connection via `@AfterEach`; vendored `CachedRowSetTest` closes its connection with try-with-resources. - Updated `upstream-xerial_sqlite-jdbc.manifest` with the two divergence notes and `CHECKSUMS.sha256` with the current hashes, using the mechanism from #53. `tools/export-upstream --verify` passes. ## Contract decision Parity with xerial 3.53.4.0 at the pinned tag. The only deviations are the two vendored test edits, which are test-only and required by the Hrana namespace lifecycle: a sqld namespace lives until its last connection closes, so the upstream tests that leak connections would leak namespaces for the whole run. ## Evidence Failing before the fix (original cleanup behavior, with only the test seam in place): ``` ./mvnw -pl libsql-conformance -am -Dit.test='XerialTestServerCleanupTest,NamespaceClosingHandlerTest' -DfailIfNoTests=false verify ... [ERROR] Tests run: 9, Failures: 5, Errors: 1, Skipped: 0 [INFO] BUILD FAILURE ``` Failures covered failed-delete visibility, retry, 409 reuse, abort release, and unwrap release. Passing after the fix: ``` ./mvnw -pl libsql-conformance -am -Dit.test='XerialTestServerCleanupTest,NamespaceClosingHandlerTest' -DfailIfNoTests=false verify [INFO] Tests run: 9, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Full verification: ``` ./mvnw -pl libsql-jdbc -am test [INFO] BUILD SUCCESS ./mvnw -pl libsql-conformance -am verify [INFO] Tests run: 115, Failures: 0, Errors: 0, Skipped: 1 [INFO] BUILD SUCCESS ./mvnw spotless:apply [INFO] BUILD SUCCESS ./tools/export-upstream --verify Verified upstream-xerial_sqlite-jdbc against upstream-xerial_sqlite-jdbc.manifest ``` ## Deviations The two vendored test edits above are the only deviations from the pinned xerial tag, and both are recorded in `upstream-xerial_sqlite-jdbc.manifest` with updated `CHECKSUMS.sha256` current hashes. Runtime behavior is otherwise unchanged.
1 parent de56052 commit a8e4345

8 files changed

Lines changed: 485 additions & 44 deletions

File tree

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package com.github.magnusp.libsql.conformance;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5+
6+
import java.lang.reflect.Proxy;
7+
import java.sql.Connection;
8+
import java.sql.SQLException;
9+
import java.util.ArrayList;
10+
import java.util.List;
11+
import java.util.function.Consumer;
12+
import org.junit.jupiter.api.Test;
13+
14+
class NamespaceClosingHandlerTest {
15+
16+
@Test
17+
void closeReleasesNamespaceOnlyOnce() throws SQLException {
18+
List<String> released = new ArrayList<>();
19+
List<String> calls = new ArrayList<>();
20+
Connection connection = proxy(connection(calls, false, false), released::add);
21+
22+
connection.close();
23+
connection.close();
24+
25+
assertThat(released).containsExactly("ns");
26+
assertThat(calls).containsExactly("close");
27+
}
28+
29+
@Test
30+
void closingAnUnwrappedConnectionReleasesNamespace() throws SQLException {
31+
List<String> released = new ArrayList<>();
32+
List<String> calls = new ArrayList<>();
33+
Connection connection = proxy(connection(calls, false, false), released::add);
34+
35+
Connection unwrapped = connection.unwrap(Connection.class);
36+
assertThat(unwrapped).isSameAs(connection);
37+
38+
unwrapped.close();
39+
40+
assertThat(released).containsExactly("ns");
41+
assertThat(calls).containsExactly("close");
42+
}
43+
44+
@Test
45+
void abortReleasesNamespace() throws SQLException {
46+
List<String> released = new ArrayList<>();
47+
List<String> calls = new ArrayList<>();
48+
Connection connection = proxy(connection(calls, false, false), released::add);
49+
50+
connection.abort(Runnable::run);
51+
52+
assertThat(released).containsExactly("ns");
53+
assertThat(calls).containsExactly("abort");
54+
}
55+
56+
@Test
57+
void closeFailureStillReleasesNamespace() {
58+
List<String> released = new ArrayList<>();
59+
List<String> calls = new ArrayList<>();
60+
Connection connection = proxy(connection(calls, true, false), released::add);
61+
62+
assertThatThrownBy(connection::close)
63+
.isInstanceOf(SQLException.class)
64+
.hasMessage("close failed");
65+
66+
assertThat(released).containsExactly("ns");
67+
}
68+
69+
@Test
70+
void abortFailureStillReleasesNamespace() {
71+
List<String> released = new ArrayList<>();
72+
List<String> calls = new ArrayList<>();
73+
Connection connection = proxy(connection(calls, false, true), released::add);
74+
75+
assertThatThrownBy(() -> connection.abort(Runnable::run))
76+
.isInstanceOf(SQLException.class)
77+
.hasMessage("abort failed");
78+
79+
assertThat(released).containsExactly("ns");
80+
}
81+
82+
private static Connection proxy(Connection connection, Consumer<String> release) {
83+
return (Connection)
84+
Proxy.newProxyInstance(
85+
Connection.class.getClassLoader(),
86+
new Class<?>[] {Connection.class},
87+
new XerialConformanceDriver.NamespaceClosingHandler(
88+
connection, "ns", release));
89+
}
90+
91+
private static Connection connection(
92+
List<String> calls, boolean failClose, boolean failAbort) {
93+
return (Connection)
94+
Proxy.newProxyInstance(
95+
Connection.class.getClassLoader(),
96+
new Class<?>[] {Connection.class},
97+
(proxy, method, args) ->
98+
switch (method.getName()) {
99+
case "close" -> {
100+
calls.add("close");
101+
if (failClose) {
102+
throw new SQLException("close failed");
103+
}
104+
yield null;
105+
}
106+
case "abort" -> {
107+
calls.add("abort");
108+
if (failAbort) {
109+
throw new SQLException("abort failed");
110+
}
111+
yield null;
112+
}
113+
case "toString" -> "fake connection";
114+
case "hashCode" -> System.identityHashCode(proxy);
115+
case "equals" -> proxy == args[0];
116+
default ->
117+
throw new UnsupportedOperationException(
118+
method.getName());
119+
});
120+
}
121+
}

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

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import java.sql.SQLFeatureNotSupportedException;
1616
import java.util.Properties;
1717
import java.util.UUID;
18+
import java.util.concurrent.Executor;
19+
import java.util.function.Consumer;
1820
import java.util.logging.Logger;
1921

2022
public final class XerialConformanceDriver implements Driver {
@@ -43,7 +45,7 @@ public Connection connect(String url, Properties info) throws SQLException {
4345
try {
4446
connection = delegate.connect(target.url(), info);
4547
} catch (SQLException e) {
46-
XerialTestServer.releaseNamespace(target.namespace());
48+
releaseAfterConnectFailure(target.namespace(), e);
4749
throw e;
4850
}
4951
if (connection == null) {
@@ -57,7 +59,11 @@ public Connection connect(String url, Properties info) throws SQLException {
5759
new Class<?>[] {Connection.class},
5860
new NamespaceClosingHandler(connection, target.namespace()));
5961
} catch (RuntimeException e) {
60-
XerialTestServer.releaseNamespace(target.namespace());
62+
try {
63+
XerialTestServer.releaseNamespace(target.namespace());
64+
} catch (RuntimeException cleanup) {
65+
e.addSuppressed(cleanup);
66+
}
6167
throw e;
6268
}
6369
}
@@ -95,6 +101,14 @@ public Logger getParentLogger() throws SQLFeatureNotSupportedException {
95101
return delegate.getParentLogger();
96102
}
97103

104+
private static void releaseAfterConnectFailure(String namespace, SQLException failure) {
105+
try {
106+
XerialTestServer.releaseNamespace(namespace);
107+
} catch (RuntimeException cleanup) {
108+
failure.addSuppressed(cleanup);
109+
}
110+
}
111+
98112
private static Target rewrite(String url) {
99113
String target = url.substring(PREFIX.length());
100114
String namespace;
@@ -112,15 +126,22 @@ private static Target rewrite(String url) {
112126

113127
private record Target(String url, String namespace) {}
114128

115-
private static final class NamespaceClosingHandler implements InvocationHandler {
129+
static final class NamespaceClosingHandler implements InvocationHandler {
116130

117131
private final Connection connection;
118132
private final String namespace;
133+
private final Consumer<String> release;
119134
private boolean closed;
120135

121-
private NamespaceClosingHandler(Connection connection, String namespace) {
136+
NamespaceClosingHandler(Connection connection, String namespace) {
137+
this(connection, namespace, XerialTestServer::releaseNamespace);
138+
}
139+
140+
NamespaceClosingHandler(
141+
Connection connection, String namespace, Consumer<String> release) {
122142
this.connection = connection;
123143
this.namespace = namespace;
144+
this.release = release;
124145
}
125146

126147
@Override
@@ -129,6 +150,16 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
129150
close();
130151
return null;
131152
}
153+
if ("abort".equals(method.getName()) && method.getParameterCount() == 1) {
154+
abort((Executor) args[0]);
155+
return null;
156+
}
157+
if ("unwrap".equals(method.getName()) && method.getParameterCount() == 1) {
158+
Class<?> iface = (Class<?>) args[0];
159+
if (iface != null && iface.isInstance(proxy)) {
160+
return proxy;
161+
}
162+
}
132163
try {
133164
return method.invoke(connection, args);
134165
} catch (InvocationTargetException e) {
@@ -143,8 +174,42 @@ private synchronized void close() throws SQLException {
143174
closed = true;
144175
try {
145176
connection.close();
146-
} finally {
147-
XerialTestServer.releaseNamespace(namespace);
177+
} catch (SQLException e) {
178+
releaseWithSuppressed(e);
179+
throw e;
180+
} catch (RuntimeException e) {
181+
releaseWithSuppressed(e);
182+
throw e;
183+
}
184+
releaseNamespace();
185+
}
186+
187+
private synchronized void abort(Executor executor) throws SQLException {
188+
if (closed) {
189+
return;
190+
}
191+
closed = true;
192+
try {
193+
connection.abort(executor);
194+
} catch (SQLException e) {
195+
releaseWithSuppressed(e);
196+
throw e;
197+
} catch (RuntimeException e) {
198+
releaseWithSuppressed(e);
199+
throw e;
200+
}
201+
releaseNamespace();
202+
}
203+
204+
private void releaseNamespace() {
205+
release.accept(namespace);
206+
}
207+
208+
private void releaseWithSuppressed(Throwable failure) {
209+
try {
210+
releaseNamespace();
211+
} catch (RuntimeException cleanup) {
212+
failure.addSuppressed(cleanup);
148213
}
149214
}
150215
}

0 commit comments

Comments
 (0)