diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index 98bbfcc7b1ec..f74762f2bd13 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -17,6 +17,7 @@ package io.grpc.internal; import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; import com.google.errorprone.annotations.DoNotCall; import io.grpc.BinaryLog; import io.grpc.ClientInterceptor; @@ -42,6 +43,14 @@ public abstract class AbstractManagedChannelImplBuilder > extends ManagedChannelBuilder { + /** + * Added for ABI compatibility. + * + *

See details in {@link #maxInboundMessageSize(int)}. + * TODO(sergiitk): move back to concrete classes as a private field, when this class is removed. + */ + protected int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; + /** * The default constructor. */ @@ -161,7 +170,32 @@ public T idleTimeout(long value, TimeUnit unit) { @Override public T maxInboundMessageSize(int max) { - delegate().maxInboundMessageSize(max); + /* + Why this method is not delegating, as the rest of the methods? + + As part of refactoring described in issue #7211, the implementation of this method, + and its corresponding field was pulled down from internal AbstractManagedChannelImplBuilder + to concrete classes that actually enforce this setting. That's the right place for + method's implementation, so it wasn't ported to ManagedChannelImplBuilder too. + + Then AbstractManagedChannelImplBuilder was brought to fix ABI backward compatibility, + and temporarily turned into a ForwardingChannelBuilder, see PR #7564. Eventually it will + be deleted, after a period with "bridge" solution added in #7834. + + However, this bringing AbstractManagedChannelImplBuilder back fix unintentionally + made this method's ABI backward incompatible: pre-refactoring builds expect + maxInboundMessageSize() to be a method of AbstractManagedChannelImplBuilder, and not + concrete classes, see https://gist.github.com/sergiitk/39583f20906df1813f5e170317a35dc4#v1322. + This problem was caught in #8313. + + Since the end goal is to keep only this method in concrete classes the need it, + to fix its ABI issue, we temporary reintroduce it to the original layer it was removed from, + AbstractManagedChannelImplBuilder. This class is also temporary, and its only intention + is a ABI compatibility. Once we move forward with dropping ABI compatibility (with #7834), + this fix is also no longer necessary, and will go away with AbstractManagedChannelImplBuilder. + */ + Preconditions.checkArgument(max >= 0, "negative max"); + maxInboundMessageSize = max; return thisT(); } diff --git a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java index 8643165eec69..81c50b19fbf9 100644 --- a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java @@ -21,12 +21,12 @@ import static org.mockito.Mockito.mock; import com.google.common.base.Defaults; +import com.google.common.collect.ImmutableSet; import io.grpc.ForwardingTestUtil; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.Collections; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -53,7 +53,8 @@ public void allMethodsForwarded() throws Exception { ManagedChannelBuilder.class, mockDelegate, testChannelBuilder, - Collections.emptyList(), + // maxInboundMessageSize is the only method that shouldn't forward. + ImmutableSet.of(ManagedChannelBuilder.class.getMethod("maxInboundMessageSize", int.class)), new ForwardingTestUtil.ArgumentProvider() { @Override public Object get(Method method, int argPos, Class clazz) { @@ -66,6 +67,15 @@ public Object get(Method method, int argPos, Class clazz) { }); } + @Test + public void testMaxInboundMessageSize() { + assertThat(testChannelBuilder.maxInboundMessageSize) + .isEqualTo(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE); + + testChannelBuilder.maxInboundMessageSize(42); + assertThat(testChannelBuilder.maxInboundMessageSize).isEqualTo(42); + } + @Test public void allBuilderMethodsReturnThis() throws Exception { for (Method method : ManagedChannelBuilder.class.getDeclaredMethods()) { diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java index 809c94f12ce8..d25e7fa467ce 100644 --- a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java @@ -99,7 +99,6 @@ public final class NettyChannelBuilder extends private ObjectPool eventLoopGroupPool = DEFAULT_EVENT_LOOP_GROUP_POOL; private boolean autoFlowControl = DEFAULT_AUTO_FLOW_CONTROL; private int flowControlWindow = DEFAULT_FLOW_CONTROL_WINDOW; - private int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; private int maxHeaderListSize = GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE; private long keepAliveTimeNanos = KEEPALIVE_TIME_NANOS_DISABLED; private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS; @@ -522,6 +521,10 @@ public NettyChannelBuilder maxInboundMessageSize(int max) { return this; } + public int maxInboundMessageSize() { + return maxInboundMessageSize; + } + @CheckReturnValue ClientTransportFactory buildTransportFactory() { assertEventLoopAndChannelType(); diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index f7d0d973802b..af5ebe2886cc 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -174,7 +174,6 @@ public static OkHttpChannelBuilder forTarget(String target, ChannelCredentials c private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS; private int flowControlWindow = DEFAULT_FLOW_CONTROL_WINDOW; private boolean keepAliveWithoutCalls; - private int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; private int maxInboundMetadataSize = Integer.MAX_VALUE; /**