From ead59309cacedc27fdf7c96098e7388cd3172a41 Mon Sep 17 00:00:00 2001 From: Vibhatha Lakmal Abeykoon Date: Tue, 9 Jan 2024 13:45:12 +0530 Subject: [PATCH] fixing warnings v3 --- .../arrow/flight/CancelFlightInfoResult.java | 2 +- .../arrow/flight/ErrorFlightMetadata.java | 2 +- .../arrow/flight/FlightCallHeaders.java | 11 +++++--- .../arrow/flight/FlightRuntimeException.java | 2 +- .../org/apache/arrow/flight/FlightServer.java | 28 +++++++++++-------- .../apache/arrow/flight/FlightService.java | 22 ++++++++------- .../arrow/flight/auth/AuthConstants.java | 4 +-- .../auth2/BearerTokenAuthenticator.java | 1 - .../arrow/flight/grpc/AddWritableBuffer.java | 2 +- .../flight/grpc/ClientInterceptorAdapter.java | 4 +-- .../arrow/flight/grpc/GetReadableBuffer.java | 2 +- .../arrow/flight/grpc/MetadataAdapter.java | 20 +++++++------ .../flight/grpc/ServerInterceptorAdapter.java | 2 +- .../apache/arrow/flight/grpc/StatusUtils.java | 3 +- 14 files changed, 58 insertions(+), 47 deletions(-) diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CancelFlightInfoResult.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CancelFlightInfoResult.java index eff5afdeeb788..165afdff553df 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CancelFlightInfoResult.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CancelFlightInfoResult.java @@ -105,7 +105,7 @@ public boolean equals(Object o) { if (this == o) { return true; } - if (o == null || getClass() != o.getClass()) { + if (!(o instanceof CancelFlightInfoResult)) { return false; } CancelFlightInfoResult that = (CancelFlightInfoResult) o; diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ErrorFlightMetadata.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ErrorFlightMetadata.java index 6669ce4655010..6e19d2750cb67 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ErrorFlightMetadata.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ErrorFlightMetadata.java @@ -61,7 +61,7 @@ public Iterable getAllByte(String key) { @Override public void insert(String key, String value) { - metadata.put(key, value.getBytes()); + metadata.put(key, value.getBytes(StandardCharsets.UTF_8)); } @Override diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightCallHeaders.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightCallHeaders.java index dd26d190872ac..6b730fb841dad 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightCallHeaders.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightCallHeaders.java @@ -17,6 +17,8 @@ package org.apache.arrow.flight; +import static java.nio.charset.StandardCharsets.UTF_8; + import java.util.Collection; import java.util.Set; import java.util.stream.Collectors; @@ -46,7 +48,7 @@ public String get(String key) { } if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { - return new String((byte[]) Iterables.get(values, 0)); + return new String((byte[]) Iterables.get(values, 0), UTF_8); } return (String) Iterables.get(values, 0); @@ -63,13 +65,13 @@ public byte[] getByte(String key) { return (byte[]) Iterables.get(values, 0); } - return ((String) Iterables.get(values, 0)).getBytes(); + return ((String) Iterables.get(values, 0)).getBytes(UTF_8); } @Override public Iterable getAll(String key) { if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { - return this.keysAndValues.get(key).stream().map(o -> new String((byte[]) o)).collect(Collectors.toList()); + return this.keysAndValues.get(key).stream().map(o -> new String((byte[]) o, UTF_8)).collect(Collectors.toList()); } return (Collection) (Collection) this.keysAndValues.get(key); } @@ -79,7 +81,7 @@ public Iterable getAllByte(String key) { if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { return (Collection) (Collection) this.keysAndValues.get(key); } - return this.keysAndValues.get(key).stream().map(o -> ((String) o).getBytes()).collect(Collectors.toList()); + return this.keysAndValues.get(key).stream().map(o -> ((String) o).getBytes(UTF_8)).collect(Collectors.toList()); } @Override @@ -105,6 +107,7 @@ public boolean containsKey(String key) { return this.keysAndValues.containsKey(key); } + @Override public String toString() { return this.keysAndValues.toString(); } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java index 76d3349a2c37a..a22b4614cbf5d 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java @@ -39,7 +39,7 @@ public CallStatus status() { } @Override - public String toString() { + public String getMessage() { String s = getClass().getName(); return String.format("%s: %s: %s", s, status.code(), status.description()); } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java index 234c9bdcaacc1..d873f7d2828d0 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java @@ -21,6 +21,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; @@ -134,6 +135,7 @@ public boolean awaitTermination(final long timeout, final TimeUnit unit) throws } /** Shutdown the server, waits for up to 6 seconds for successful shutdown before returning. */ + @Override public void close() throws InterruptedException { shutdown(); final boolean terminated = awaitTermination(3000, TimeUnit.MILLISECONDS); @@ -146,7 +148,7 @@ public void close() throws InterruptedException { server.shutdownNow(); int count = 0; - while (!server.isTerminated() & count < 30) { + while (!server.isTerminated() && count < 30) { count++; logger.debug("Waiting for termination"); Thread.sleep(100); @@ -216,22 +218,23 @@ public FlightServer build() { try { try { // Linux - builder.channelType( - (Class) Class - .forName("io.netty.channel.epoll.EpollServerDomainSocketChannel")); - final EventLoopGroup elg = (EventLoopGroup) Class.forName("io.netty.channel.epoll.EpollEventLoopGroup") - .newInstance(); + builder.channelType(Class + .forName("io.netty.channel.epoll.EpollServerDomainSocketChannel") + .asSubclass(ServerChannel.class)); + final EventLoopGroup elg = Class.forName("io.netty.channel.epoll.EpollEventLoopGroup") + .asSubclass(EventLoopGroup.class).getConstructor().newInstance(); builder.bossEventLoopGroup(elg).workerEventLoopGroup(elg); } catch (ClassNotFoundException e) { // BSD builder.channelType( - (Class) Class - .forName("io.netty.channel.kqueue.KQueueServerDomainSocketChannel")); - final EventLoopGroup elg = (EventLoopGroup) Class.forName("io.netty.channel.kqueue.KQueueEventLoopGroup") - .newInstance(); + Class.forName("io.netty.channel.kqueue.KQueueServerDomainSocketChannel") + .asSubclass(ServerChannel.class)); + final EventLoopGroup elg = Class.forName("io.netty.channel.kqueue.KQueueEventLoopGroup") + .asSubclass(EventLoopGroup.class).getConstructor().newInstance(); builder.bossEventLoopGroup(elg).workerEventLoopGroup(elg); } - } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) { + } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | NoSuchMethodException | + InvocationTargetException e) { throw new UnsupportedOperationException( "Could not find suitable Netty native transport implementation for domain socket address."); } @@ -342,7 +345,8 @@ private void closeInputStreamIfNotNull(InputStream stream) { if (stream != null) { try { stream.close(); - } catch (IOException ignored) { + } catch (IOException expected) { + // stream closes gracefully, doesn't expect an exception. } } } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightService.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightService.java index 5231a7aaf76e4..5f4d803aacd0b 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightService.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightService.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.Map; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import java.util.function.BooleanSupplier; import java.util.function.Consumer; @@ -142,7 +143,7 @@ public void listActions(Flight.Empty request, StreamObserver } private static class GetListener extends OutboundStreamListenerImpl implements ServerStreamListener { - private ServerCallStreamObserver responseObserver; + private final ServerCallStreamObserver serverCallResponseObserver; private final Consumer errorHandler; private Runnable onCancelHandler = null; private Runnable onReadyHandler = null; @@ -152,10 +153,10 @@ public GetListener(ServerCallStreamObserver responseObserver, Cons super(null, responseObserver); this.errorHandler = errorHandler; this.completed = false; - this.responseObserver = responseObserver; - this.responseObserver.setOnCancelHandler(this::onCancel); - this.responseObserver.setOnReadyHandler(this::onReady); - this.responseObserver.disableAutoInboundFlowControl(); + this.serverCallResponseObserver = responseObserver; + this.serverCallResponseObserver.setOnCancelHandler(this::onCancel); + this.serverCallResponseObserver.setOnReadyHandler(this::onReady); + this.serverCallResponseObserver.disableAutoInboundFlowControl(); } private void onCancel() { @@ -183,7 +184,7 @@ public void setOnReadyHandler(Runnable handler) { @Override public boolean isCancelled() { - return responseObserver.isCancelled(); + return serverCallResponseObserver.isCancelled(); } @Override @@ -228,7 +229,7 @@ public StreamObserver doPutCustom(final StreamObserver observer = fs.asObserver(); - executors.submit(() -> { + Future unused = executors.submit(() -> { try { producer.acceptPut(makeContext(responseObserver), fs, ackStream).run(); } catch (Throwable ex) { @@ -277,7 +278,8 @@ public void pollFlightInfo(Flight.FlightDescriptor request, StreamObserver, FlightServerMiddleware> middleware = ServerInterceptorAdapter.SERVER_MIDDLEWARE_KEY.get(); + final Map, FlightServerMiddleware> middleware = ServerInterceptorAdapter + .SERVER_MIDDLEWARE_KEY.get(); if (middleware == null || middleware.isEmpty()) { logger.error("Uncaught exception in Flight method body", t); return; @@ -377,7 +379,7 @@ public StreamObserver doExchangeCustom(StreamObserver observer = fs.asObserver(); try { - executors.submit(() -> { + Future unused = executors.submit(() -> { try { producer.doExchange(makeContext(responseObserver), fs, listener); } catch (Exception ex) { @@ -416,7 +418,7 @@ public boolean isCancelled() { } @Override - public T getMiddleware(Key key) { + public T getMiddleware(FlightServerMiddleware.Key key) { final Map, FlightServerMiddleware> middleware = ServerInterceptorAdapter.SERVER_MIDDLEWARE_KEY.get(); if (middleware == null) { return null; diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth/AuthConstants.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth/AuthConstants.java index ac55872e5b18b..e3ccdc626d71b 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth/AuthConstants.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth/AuthConstants.java @@ -20,8 +20,8 @@ import org.apache.arrow.flight.FlightConstants; import io.grpc.Context; +import io.grpc.Metadata; import io.grpc.Metadata.BinaryMarshaller; -import io.grpc.Metadata.Key; import io.grpc.MethodDescriptor; /** @@ -32,7 +32,7 @@ public final class AuthConstants { public static final String HANDSHAKE_DESCRIPTOR_NAME = MethodDescriptor .generateFullMethodName(FlightConstants.SERVICE, "Handshake"); public static final String TOKEN_NAME = "Auth-Token-bin"; - public static final Key TOKEN_KEY = Key.of(TOKEN_NAME, new BinaryMarshaller() { + public static final Metadata.Key TOKEN_KEY = Metadata.Key.of(TOKEN_NAME, new BinaryMarshaller() { @Override public byte[] toBytes(byte[] value) { diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthenticator.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthenticator.java index 2006e0a2b1241..5eb5863e792d4 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthenticator.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthenticator.java @@ -55,7 +55,6 @@ public AuthResult authenticate(CallHeaders incomingHeaders) { * Validate the bearer token. * @param bearerToken The bearer token to validate. * @return A successful AuthResult if validation succeeded. - * @throws Exception If the token validation fails. */ protected abstract AuthResult validateBearer(String bearerToken); diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java index 4a99ab22842e2..a4cacc713270e 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java @@ -72,7 +72,7 @@ public class AddWritableBuffer { tmpBufChainOut = tmpBufChainOut2; } catch (Exception ex) { - new RuntimeException("Failed to initialize AddWritableBuffer, falling back to slow path", ex).printStackTrace(); + throw new RuntimeException("Failed to initialize AddWritableBuffer, falling back to slow path", ex); } bufConstruct = tmpConstruct; diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ClientInterceptorAdapter.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ClientInterceptorAdapter.java index ae11e52605623..dccda26bbacaf 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ClientInterceptorAdapter.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ClientInterceptorAdapter.java @@ -46,9 +46,9 @@ */ public class ClientInterceptorAdapter implements ClientInterceptor { - private final List factories; + private final List factories; - public ClientInterceptorAdapter(List factories) { + public ClientInterceptorAdapter(List factories) { this.factories = factories; } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java index 82cfd7f39c91c..bd2e780a0cef1 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java @@ -51,7 +51,7 @@ public class GetReadableBuffer { tmpField = f; tmpClazz = clazz; } catch (Exception e) { - new RuntimeException("Failed to initialize GetReadableBuffer, falling back to slow path", e).printStackTrace(); + throw new RuntimeException("Failed to initialize GetReadableBuffer, falling back to slow path", e); } READABLE_BUFFER = tmpField; BUFFER_INPUT_STREAM = tmpClazz; diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java index 4327f0ca85b0d..25e4fb0197f95 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/MetadataAdapter.java @@ -17,6 +17,7 @@ package org.apache.arrow.flight.grpc; +import java.nio.charset.StandardCharsets; import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; @@ -42,26 +43,26 @@ public MetadataAdapter(Metadata metadata) { @Override public String get(String key) { - return this.metadata.get(Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); + return this.metadata.get(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); } @Override public byte[] getByte(String key) { if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { - return this.metadata.get(Key.of(key, Metadata.BINARY_BYTE_MARSHALLER)); + return this.metadata.get(Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER)); } - return get(key).getBytes(); + return get(key).getBytes(StandardCharsets.UTF_8); } @Override public Iterable getAll(String key) { - return this.metadata.getAll(Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); + return this.metadata.getAll(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); } @Override public Iterable getAllByte(String key) { if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { - return this.metadata.getAll(Key.of(key, Metadata.BINARY_BYTE_MARSHALLER)); + return this.metadata.getAll(Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER)); } return StreamSupport.stream(getAll(key).spliterator(), false) .map(String::getBytes).collect(Collectors.toList()); @@ -69,12 +70,12 @@ public Iterable getAllByte(String key) { @Override public void insert(String key, String value) { - this.metadata.put(Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + this.metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); } @Override public void insert(String key, byte[] value) { - this.metadata.put(Key.of(key, Metadata.BINARY_BYTE_MARSHALLER), value); + this.metadata.put(Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER), value); } @Override @@ -85,13 +86,14 @@ public Set keys() { @Override public boolean containsKey(String key) { if (key.endsWith("-bin")) { - final Key grpcKey = Key.of(key, Metadata.BINARY_BYTE_MARSHALLER); + final Metadata.Key grpcKey = Metadata.Key.of(key, Metadata.BINARY_BYTE_MARSHALLER); return this.metadata.containsKey(grpcKey); } - final Key grpcKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER); + final Metadata.Key grpcKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); return this.metadata.containsKey(grpcKey); } + @Override public String toString() { return this.metadata.toString(); } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ServerInterceptorAdapter.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ServerInterceptorAdapter.java index 9b038b9d49272..70c667df56020 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ServerInterceptorAdapter.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/ServerInterceptorAdapter.java @@ -61,7 +61,7 @@ public static class KeyFactory { private final FlightServerMiddleware.Key key; private final FlightServerMiddleware.Factory factory; - public KeyFactory(Key key, Factory factory) { + public KeyFactory(FlightServerMiddleware.Key key, FlightServerMiddleware.Factory factory) { this.key = key; this.factory = factory; } diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/StatusUtils.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/StatusUtils.java index 55e8418642d36..7f0dcf2da3f0d 100644 --- a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/StatusUtils.java +++ b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/StatusUtils.java @@ -17,6 +17,7 @@ package org.apache.arrow.flight.grpc; +import java.nio.charset.StandardCharsets; import java.util.Iterator; import java.util.Objects; import java.util.function.Function; @@ -171,7 +172,7 @@ private static ErrorFlightMetadata parseTrailers(Metadata trailers) { if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { metadata.insert(key, trailers.get(keyOfBinary(key))); } else { - metadata.insert(key, Objects.requireNonNull(trailers.get(keyOfAscii(key))).getBytes()); + metadata.insert(key, Objects.requireNonNull(trailers.get(keyOfAscii(key))).getBytes(StandardCharsets.UTF_8)); } } return metadata;