This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-java.git
The following commit(s) were added to refs/heads/main by this push:
new 6b6d16a42 GH-139: [Flight] Stop return null from
MetadataAdapter.getAll(String) and getAllByte(String) (#1016)
6b6d16a42 is described below
commit 6b6d16a42fdcb46dae728ef0795559e1606c2372
Author: Aleksei Starikov <[email protected]>
AuthorDate: Sun Feb 22 14:41:02 2026 +0100
GH-139: [Flight] Stop return null from MetadataAdapter.getAll(String) and
getAllByte(String) (#1016)
## What's Changed
`CallHeaders` has 3 implementations:
- FlightCallHeaders
- ErrorFlightMetadata
- MetadataAdapter
**Before this change:**
`MetadataAdapter` could return `null` from `getAll(String)` and
`getAllByte(String)` when there were no values for the key, because
gRPC’s `Metadata.getAll()` returns `null` in that case. This was
undocumented and forced callers to null-check.
**After this change:**
All 3 implementations consistently return an `empty iterable` (never
`null`) when the key is absent or has no values. The contract is
documented on the interface and covered by tests for each
implementation.
---
**This contains breaking changes.**
`MetadataAdapter.getAll(String)` and `getAllByte(String)` return empty
iterator instead of null.
---
Closes #139.
---
.../java/org/apache/arrow/flight/CallHeaders.java | 14 +++++++--
.../arrow/flight/ServerSessionMiddleware.java | 26 ++++++++--------
.../flight/client/ClientCookieMiddleware.java | 5 +--
.../apache/arrow/flight/grpc/MetadataAdapter.java | 9 ++++--
.../org/apache/arrow/flight/TestCallOptions.java | 11 +++++++
.../org/apache/arrow/flight/TestErrorMetadata.java | 11 +++++++
.../arrow/flight/grpc/TestMetadataAdapter.java | 36 ++++++++++++++++++++++
7 files changed, 90 insertions(+), 22 deletions(-)
diff --git
a/flight/flight-core/src/main/java/org/apache/arrow/flight/CallHeaders.java
b/flight/flight-core/src/main/java/org/apache/arrow/flight/CallHeaders.java
index f4f6486a3..0939d232c 100644
--- a/flight/flight-core/src/main/java/org/apache/arrow/flight/CallHeaders.java
+++ b/flight/flight-core/src/main/java/org/apache/arrow/flight/CallHeaders.java
@@ -26,10 +26,20 @@ public interface CallHeaders {
/** Get the value of a metadata key. If multiple values are present, then
get the last one. */
byte[] getByte(String key);
- /** Get all values present for the given metadata key. */
+ /**
+ * Get all values present for the given metadata key.
+ *
+ * @param key the metadata key
+ * @return an iterable of all values for the key. Returns an empty iterable
if no value to return.
+ */
Iterable<String> getAll(String key);
- /** Get all values present for the given metadata key. */
+ /**
+ * Get all values present for the given metadata key.
+ *
+ * @param key the metadata key
+ * @return an iterable of all values for the key. Returns an empty iterable
if no value to return.
+ */
Iterable<byte[]> getAllByte(String key);
/**
diff --git
a/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
b/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
index 47fd6f136..5ec01b9c8 100644
---
a/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
+++
b/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
@@ -80,20 +80,18 @@ public class ServerSessionMiddleware implements
FlightServerMiddleware {
String sessionId = null;
final Iterable<String> it = incomingHeaders.getAll("cookie");
- if (it != null) {
- findIdCookie:
- for (final String headerValue : it) {
- for (final String cookie : headerValue.split(" ;")) {
- final String[] cookiePair = cookie.split("=");
- if (cookiePair.length != 2) {
- // Soft failure: Ignore invalid cookie list field
- break;
- }
-
- if (sessionCookieName.equals(cookiePair[0]) &&
cookiePair[1].length() > 0) {
- sessionId = cookiePair[1];
- break findIdCookie;
- }
+ findIdCookie:
+ for (final String headerValue : it) {
+ for (final String cookie : headerValue.split(" ;")) {
+ final String[] cookiePair = cookie.split("=");
+ if (cookiePair.length != 2) {
+ // Soft failure: Ignore invalid cookie list field
+ break;
+ }
+
+ if (sessionCookieName.equals(cookiePair[0]) &&
cookiePair[1].length() > 0) {
+ sessionId = cookiePair[1];
+ break findIdCookie;
}
}
}
diff --git
a/flight/flight-core/src/main/java/org/apache/arrow/flight/client/ClientCookieMiddleware.java
b/flight/flight-core/src/main/java/org/apache/arrow/flight/client/ClientCookieMiddleware.java
index e5eb93400..b33e6b7ec 100644
---
a/flight/flight-core/src/main/java/org/apache/arrow/flight/client/ClientCookieMiddleware.java
+++
b/flight/flight-core/src/main/java/org/apache/arrow/flight/client/ClientCookieMiddleware.java
@@ -100,10 +100,7 @@ public class ClientCookieMiddleware implements
FlightClientMiddleware {
@Override
public void onHeadersReceived(CallHeaders incomingHeaders) {
- final Iterable<String> setCookieHeaders =
incomingHeaders.getAll(SET_COOKIE_HEADER);
- if (setCookieHeaders != null) {
- factory.updateCookies(setCookieHeaders);
- }
+ factory.updateCookies(incomingHeaders.getAll(SET_COOKIE_HEADER));
}
@Override
diff --git
a/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java
b/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java
index a1de16ede..64a0769d6 100644
---
a/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java
+++
b/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java
@@ -18,6 +18,7 @@ package org.apache.arrow.flight.grpc;
import io.grpc.Metadata;
import java.nio.charset.StandardCharsets;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;
@@ -53,13 +54,17 @@ public class MetadataAdapter implements CallHeaders {
@Override
public Iterable<String> getAll(String key) {
- return this.metadata.getAll(Metadata.Key.of(key,
Metadata.ASCII_STRING_MARSHALLER));
+ final Iterable<String> all =
+ this.metadata.getAll(Metadata.Key.of(key,
Metadata.ASCII_STRING_MARSHALLER));
+ return all != null ? all : Collections.emptyList();
}
@Override
public Iterable<byte[]> getAllByte(String key) {
if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
- return this.metadata.getAll(Metadata.Key.of(key,
Metadata.BINARY_BYTE_MARSHALLER));
+ final Iterable<byte[]> all =
+ this.metadata.getAll(Metadata.Key.of(key,
Metadata.BINARY_BYTE_MARSHALLER));
+ return all != null ? all : Collections.emptyList();
}
return StreamSupport.stream(getAll(key).spliterator(), false)
.map(String::getBytes)
diff --git
a/flight/flight-core/src/test/java/org/apache/arrow/flight/TestCallOptions.java
b/flight/flight-core/src/test/java/org/apache/arrow/flight/TestCallOptions.java
index a54ce6981..8aef9c69a 100644
---
a/flight/flight-core/src/test/java/org/apache/arrow/flight/TestCallOptions.java
+++
b/flight/flight-core/src/test/java/org/apache/arrow/flight/TestCallOptions.java
@@ -21,6 +21,7 @@ import static
org.apache.arrow.flight.Location.forGrpcInsecure;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -110,6 +111,16 @@ public class TestCallOptions {
testHeaders(headers);
}
+ @Test
+ public void getAllReturnsEmptyIterableForMissingKey() {
+ FlightCallHeaders headers = new FlightCallHeaders();
+
+ assertNotNull(headers.getAll("missing"));
+ assertFalse(headers.getAll("missing").iterator().hasNext());
+ assertNotNull(headers.getAllByte("missing-bin"));
+ assertFalse(headers.getAllByte("missing-bin").iterator().hasNext());
+ }
+
private void testHeaders(CallHeaders headers) {
try (BufferAllocator a = new RootAllocator(Long.MAX_VALUE);
HeaderProducer producer = new HeaderProducer();
diff --git
a/flight/flight-core/src/test/java/org/apache/arrow/flight/TestErrorMetadata.java
b/flight/flight-core/src/test/java/org/apache/arrow/flight/TestErrorMetadata.java
index a9a3e355b..214614def 100644
---
a/flight/flight-core/src/test/java/org/apache/arrow/flight/TestErrorMetadata.java
+++
b/flight/flight-core/src/test/java/org/apache/arrow/flight/TestErrorMetadata.java
@@ -20,6 +20,7 @@ import static
org.apache.arrow.flight.FlightTestUtil.LOCALHOST;
import static org.apache.arrow.flight.Location.forGrpcInsecure;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -119,6 +120,16 @@ public class TestErrorMetadata {
}
}
+ @Test
+ public void getAllReturnsEmptyIterableForMissingKey() {
+ ErrorFlightMetadata metadata = new ErrorFlightMetadata();
+
+ assertNotNull(metadata.getAll("missing"));
+ assertFalse(metadata.getAll("missing").iterator().hasNext());
+ assertNotNull(metadata.getAllByte("missing-bin"));
+ assertFalse(metadata.getAllByte("missing-bin").iterator().hasNext());
+ }
+
private static class StatusRuntimeExceptionProducer extends
NoOpFlightProducer {
private final PerfOuterClass.Perf perf;
diff --git
a/flight/flight-core/src/test/java/org/apache/arrow/flight/grpc/TestMetadataAdapter.java
b/flight/flight-core/src/test/java/org/apache/arrow/flight/grpc/TestMetadataAdapter.java
new file mode 100644
index 000000000..b0f5dcfcf
--- /dev/null
+++
b/flight/flight-core/src/test/java/org/apache/arrow/flight/grpc/TestMetadataAdapter.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.flight.grpc;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import io.grpc.Metadata;
+import org.junit.jupiter.api.Test;
+
+public class TestMetadataAdapter {
+
+ @Test
+ public void getAllReturnsEmptyIterableForMissingKey() {
+ MetadataAdapter headers = new MetadataAdapter(new Metadata());
+
+ assertNotNull(headers.getAll("missing"));
+ assertFalse(headers.getAll("missing").iterator().hasNext());
+ assertNotNull(headers.getAllByte("missing-bin"));
+ assertFalse(headers.getAllByte("missing-bin").iterator().hasNext());
+ }
+}
