From 4bc0c95d0b835c343adfe357d66aa0fc2ec5a9a0 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 8 Jan 2018 15:08:56 -0800 Subject: [PATCH] Update ErrorProne to 2.1.3 and fix failures The fixes could have subtle side-effects, but I did take care when making them. --- .../java/io/grpc/benchmarks/qps/OpenLoopClient.java | 2 +- build.gradle | 5 +++-- core/src/main/java/io/grpc/Metadata.java | 4 ++-- .../io/grpc/internal/ApplicationThreadDeframer.java | 4 ++-- .../main/java/io/grpc/internal/ChannelExecutor.java | 5 +++-- .../src/test/java/io/grpc/MethodDescriptorTest.java | 13 ++++++++----- .../grpc/testing/integration/TestServiceImpl.java | 6 +++--- .../io/grpc/netty/AbstractHttp2HeadersTest.java | 4 ++-- .../java/io/grpc/okhttp/OkHttpClientTransport.java | 1 + .../protobuf/services/ProtoReflectionService.java | 6 +++--- .../src/test/java/io/grpc/stub/ClientCallsTest.java | 2 +- 11 files changed, 29 insertions(+), 23 deletions(-) diff --git a/benchmarks/src/main/java/io/grpc/benchmarks/qps/OpenLoopClient.java b/benchmarks/src/main/java/io/grpc/benchmarks/qps/OpenLoopClient.java index b977355ca3..07b4be5fd1 100644 --- a/benchmarks/src/main/java/io/grpc/benchmarks/qps/OpenLoopClient.java +++ b/benchmarks/src/main/java/io/grpc/benchmarks/qps/OpenLoopClient.java @@ -137,7 +137,7 @@ public class OpenLoopClient { stub = BenchmarkServiceGrpc.newStub(checkNotNull(channel, "channel")); this.request = checkNotNull(request, "request"); this.targetQps = targetQps; - numRpcs = targetQps * duration; + numRpcs = (long) targetQps * duration; rnd = new Random(); } diff --git a/build.gradle b/build.gradle index 8d9768e6f6..80e0debc67 100644 --- a/build.gradle +++ b/build.gradle @@ -35,7 +35,7 @@ subprojects { dependencies { // The ErrorProne plugin defaults to the latest, which would break our // build if error prone releases a new version with a new check - errorprone 'com.google.errorprone:error_prone_core:2.0.21' + errorprone 'com.google.errorprone:error_prone_core:2.1.3' apt 'com.google.guava:guava-beta-checker:1.0' } } else { @@ -76,7 +76,8 @@ subprojects { compileTestJava { // serialVersionUID is basically guaranteed to be useless in our tests - options.compilerArgs += ["-Xlint:-serial"] + // LinkedList doesn't hurt much in tests and has lots of usages + options.compilerArgs += ["-Xlint:-serial", "-Xep:JdkObsolete:OFF"] } jar.manifest { diff --git a/core/src/main/java/io/grpc/Metadata.java b/core/src/main/java/io/grpc/Metadata.java index 2b368512e0..9778ab180f 100644 --- a/core/src/main/java/io/grpc/Metadata.java +++ b/core/src/main/java/io/grpc/Metadata.java @@ -24,13 +24,13 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.io.BaseEncoding; import java.nio.ByteBuffer; +import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; @@ -347,7 +347,7 @@ public final class Metadata { List ret = null; for (; readIdx < size; readIdx++) { if (bytesEqual(key.asciiName(), name(readIdx))) { - ret = ret != null ? ret : new LinkedList(); + ret = ret != null ? ret : new ArrayList(); ret.add(key.parseBytes(value(readIdx))); continue; } diff --git a/core/src/main/java/io/grpc/internal/ApplicationThreadDeframer.java b/core/src/main/java/io/grpc/internal/ApplicationThreadDeframer.java index c70ebe54b9..71c540f297 100644 --- a/core/src/main/java/io/grpc/internal/ApplicationThreadDeframer.java +++ b/core/src/main/java/io/grpc/internal/ApplicationThreadDeframer.java @@ -20,7 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import io.grpc.Decompressor; import java.io.InputStream; -import java.util.LinkedList; +import java.util.ArrayDeque; import java.util.Queue; import javax.annotation.Nullable; @@ -41,7 +41,7 @@ public class ApplicationThreadDeframer implements Deframer, MessageDeframer.List private final TransportExecutor transportExecutor; /** Queue for messages returned by the deframer when deframing in the application thread. */ - private final Queue messageReadQueue = new LinkedList(); + private final Queue messageReadQueue = new ArrayDeque(); ApplicationThreadDeframer( MessageDeframer.Listener listener, diff --git a/core/src/main/java/io/grpc/internal/ChannelExecutor.java b/core/src/main/java/io/grpc/internal/ChannelExecutor.java index 5a2b23a4b1..af62cedced 100644 --- a/core/src/main/java/io/grpc/internal/ChannelExecutor.java +++ b/core/src/main/java/io/grpc/internal/ChannelExecutor.java @@ -19,7 +19,8 @@ package io.grpc.internal; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; -import java.util.LinkedList; +import java.util.ArrayDeque; +import java.util.Queue; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.concurrent.GuardedBy; @@ -39,7 +40,7 @@ final class ChannelExecutor { private final Object lock = new Object(); @GuardedBy("lock") - private final LinkedList queue = new LinkedList(); + private final Queue queue = new ArrayDeque(); @GuardedBy("lock") private boolean draining; diff --git a/core/src/test/java/io/grpc/MethodDescriptorTest.java b/core/src/test/java/io/grpc/MethodDescriptorTest.java index 9ede2663d4..4f2ca1d3ef 100644 --- a/core/src/test/java/io/grpc/MethodDescriptorTest.java +++ b/core/src/test/java/io/grpc/MethodDescriptorTest.java @@ -21,7 +21,9 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import io.grpc.MethodDescriptor.MethodType; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -30,6 +32,9 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class MethodDescriptorTest { + @Rule + public final ExpectedException thrown = ExpectedException.none(); + @Test public void createMethodDescriptor() { @SuppressWarnings("deprecation") // MethodDescriptor.create @@ -80,7 +85,7 @@ public class MethodDescriptorTest { assertEquals("package.service/method", newDescriptor.getFullMethodName()); } - @Test(expected = IllegalArgumentException.class) + @Test public void safeAndNonUnary() { MethodDescriptor descriptor = MethodDescriptor.newBuilder() .setType(MethodType.SERVER_STREAMING) @@ -89,10 +94,8 @@ public class MethodDescriptorTest { .setResponseMarshaller(new StringMarshaller()) .build(); - - MethodDescriptor discard = descriptor.toBuilder().setSafe(true).build(); - // Never reached - assert discard == null; + thrown.expect(IllegalArgumentException.class); + MethodDescriptor unused = descriptor.toBuilder().setSafe(true).build(); } @Test diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java index 7c3e1abc5c..40dd78751a 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java @@ -39,9 +39,9 @@ import io.grpc.testing.integration.Messages.StreamingOutputCallRequest; import io.grpc.testing.integration.Messages.StreamingOutputCallResponse; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Queue; import java.util.Random; @@ -209,7 +209,7 @@ public class TestServiceImpl extends TestServiceGrpc.TestServiceImplBase { @Override public StreamObserver halfDuplexCall( final StreamObserver responseObserver) { - final Queue chunks = new LinkedList(); + final Queue chunks = new ArrayDeque(); return new StreamObserver() { @Override public void onNext(StreamingOutputCallRequest request) { @@ -371,7 +371,7 @@ public class TestServiceImpl extends TestServiceGrpc.TestServiceImplBase { * Breaks down the request and creates a queue of response chunks for the given request. */ public Queue toChunkQueue(StreamingOutputCallRequest request) { - Queue chunkQueue = new LinkedList(); + Queue chunkQueue = new ArrayDeque(); int offset = 0; boolean compressable = compressableResponse(request.getResponseType()); for (ResponseParameters params : request.getResponseParametersList()) { diff --git a/netty/src/test/java/io/grpc/netty/AbstractHttp2HeadersTest.java b/netty/src/test/java/io/grpc/netty/AbstractHttp2HeadersTest.java index 1d9f36bb08..7eef877827 100644 --- a/netty/src/test/java/io/grpc/netty/AbstractHttp2HeadersTest.java +++ b/netty/src/test/java/io/grpc/netty/AbstractHttp2HeadersTest.java @@ -50,8 +50,8 @@ public class AbstractHttp2HeadersTest { } catch (InvocationTargetException ex) { assertEquals("For method: " + method, UnsupportedOperationException.class, ex.getCause().getClass()); - } catch (Throwable t) { - throw new RuntimeException("Failure with method: " + method, t); + } catch (Exception ex) { + throw new AssertionError("Failure with method: " + method, ex); } } } diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 717c06ac78..df20daeae9 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -165,6 +165,7 @@ class OkHttpClientTransport implements ConnectionClientTransport { private Socket socket; @GuardedBy("lock") private int maxConcurrentStreams = 0; + @SuppressWarnings("JdkObsolete") // Usage is bursty; want low memory usage when empty @GuardedBy("lock") private LinkedList pendingStreams = new LinkedList(); private final ConnectionSpec connectionSpec; diff --git a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java index eb986f8338..387c2bc4ec 100644 --- a/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java +++ b/services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java @@ -42,10 +42,10 @@ import io.grpc.reflection.v1alpha.ServerReflectionResponse; import io.grpc.reflection.v1alpha.ServiceResponse; import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; +import java.util.ArrayDeque; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Queue; @@ -300,7 +300,7 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef FileDescriptorResponse.Builder fdRBuilder = FileDescriptorResponse.newBuilder(); Set seenFiles = new HashSet(); - Queue frontier = new LinkedList(); + Queue frontier = new ArrayDeque(); seenFiles.add(fd.getName()); frontier.add(fd); while (!frontier.isEmpty()) { @@ -408,7 +408,7 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef new HashMap>(); FileDescriptorIndex(List services) { - Queue fileDescriptorsToProcess = new LinkedList(); + Queue fileDescriptorsToProcess = new ArrayDeque(); Set seenFiles = new HashSet(); for (ServerServiceDefinition service : services) { io.grpc.ServiceDescriptor serviceDescriptor = service.getServiceDescriptor(); diff --git a/stub/src/test/java/io/grpc/stub/ClientCallsTest.java b/stub/src/test/java/io/grpc/stub/ClientCallsTest.java index 6cbe70e829..db1a0a1530 100644 --- a/stub/src/test/java/io/grpc/stub/ClientCallsTest.java +++ b/stub/src/test/java/io/grpc/stub/ClientCallsTest.java @@ -557,7 +557,7 @@ public class ClientCallsTest { try { iter.next(); fail("Should fail"); - } catch (Throwable e) { + } catch (Exception e) { Status status = Status.fromThrowable(e); assertEquals(Status.INTERNAL, status); Metadata metadata = Status.trailersFromThrowable(e);