Skip to content

Commit ce5eb54

Browse files
committed
fix: Resolve merge conflicts and fix TimeUnit ambiguity
- Resolve merge conflicts from trunk merge (buildFlightClient extraction) - Fix TimeUnit import ambiguity: fully qualify java.util.concurrent.TimeUnit at keepAlive call sites to avoid conflict with Arrow's TimeUnit - Add closed guard to reset() — throws IllegalStateException after close() - Make close() idempotent with early return on already-closed client - Update testResetAfterClose to expect IllegalStateException
1 parent 7724c13 commit ce5eb54

2 files changed

Lines changed: 17 additions & 7 deletions

File tree

src/main/java/ai/spice/SpiceClient.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ of this software and associated documentation files (the "Software"), to deal
3939
import java.util.List;
4040
import java.util.Map;
4141
import java.util.concurrent.ExecutionException;
42-
import java.util.concurrent.TimeUnit;
4342

4443
import org.apache.arrow.adbc.core.AdbcConnection;
4544
import org.apache.arrow.adbc.core.AdbcDatabase;
@@ -154,6 +153,7 @@ public class SpiceClient implements AutoCloseable {
154153
private FlightSqlClient flightClient;
155154
private CredentialCallOption authCallOptions = null;
156155
private BufferAllocator allocator;
156+
private boolean closed = false;
157157

158158
// Cached retryers (immutable, thread-safe)
159159
private Retryer<ArrowReader> adbcRetryer;
@@ -268,8 +268,8 @@ private synchronized void buildFlightClient() {
268268
}
269269
channelBuilder
270270
// HTTP/2 keep-alive to detect dead/idle connections behind load balancers
271-
.keepAliveTime(30, TimeUnit.SECONDS)
272-
.keepAliveTimeout(10, TimeUnit.SECONDS)
271+
.keepAliveTime(30, java.util.concurrent.TimeUnit.SECONDS)
272+
.keepAliveTimeout(10, java.util.concurrent.TimeUnit.SECONDS)
273273
.keepAliveWithoutCalls(true)
274274
.maxInboundMessageSize(Integer.MAX_VALUE)
275275
.maxInboundMetadataSize(Integer.MAX_VALUE);
@@ -347,6 +347,9 @@ private synchronized void ensureFlightClient() {
347347
* }</pre>
348348
*/
349349
public synchronized void reset() {
350+
if (closed) {
351+
throw new IllegalStateException("Cannot reset a closed SpiceClient");
352+
}
350353
logger.info("Resetting SpiceClient transport");
351354

352355
// Close ADBC resources (they maintain a separate Flight connection)
@@ -1011,6 +1014,10 @@ private boolean shouldRetry(CallStatus status) {
10111014

10121015
@Override
10131016
public void close() throws Exception {
1017+
if (closed) {
1018+
return;
1019+
}
1020+
closed = true;
10141021
logger.debug("Closing SpiceClient");
10151022
List<Exception> exceptions = new ArrayList<>();
10161023

src/test/java/ai/spice/ResetTest.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,14 +170,17 @@ public void testResetQueryResetQueryCycle() throws Exception {
170170
}
171171

172172
/**
173-
* reset() after close() should not throw.
174-
* (The flight client is already null after close, reset handles that.)
173+
* reset() after close() should throw IllegalStateException.
175174
*/
176175
public void testResetAfterClose() throws Exception {
177176
SpiceClient client = SpiceClient.builder().build();
178177
client.close();
179-
// reset on an already-closed client should be safe
180-
client.reset();
178+
try {
179+
client.reset();
180+
fail("Expected IllegalStateException when resetting a closed client");
181+
} catch (IllegalStateException e) {
182+
assertTrue(e.getMessage().contains("closed"));
183+
}
181184
}
182185

183186
/**

0 commit comments

Comments
 (0)