From 050ae18e2a53019f5d199427c91e7d519686fc05 Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Fri, 8 Sep 2023 15:20:58 -0700 Subject: [PATCH] core: Remove temporary AbstractServerImplBuilder This breaks the ABI of the classes listed below. Users that recompiled their code using grpc-java [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on Feb 23, 2021) and later, ARE NOT AFFECTED. Users that compiled their source using grpc-java earlier than [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code will fail on runtime with `NoSuchMethodError`. For example, code: ```java NettyServerBuilder.forPort(80).directExecutor(); ``` Will fail with > `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractServerImplBuilder io.grpc.netty.NettyServerBuilder.directExecutor()'` **Affected classes** Class `AbstractServerImplBuilder` is deleted, and no longer in the class hierarchy of the server builders: - `io.grpc.netty.NettyServerBuilder` - `io.grpc.inprocess.InProcessServerBuilder` --- core/build.gradle | 5 - .../internal/AbstractServerImplBuilder.java | 233 ------------------ .../AbstractServerImplBuilderTest.java | 89 ------- .../inprocess/InProcessServerBuilder.java | 5 +- .../io/grpc/netty/NettyServerBuilder.java | 4 +- 5 files changed, 4 insertions(+), 332 deletions(-) delete mode 100644 core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java delete mode 100644 core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java diff --git a/core/build.gradle b/core/build.gradle index a2b519c386..14ad2e748f 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -113,11 +113,6 @@ plugins.withId("java") { "io/grpc/internal/AbstractManagedChannelImplBuilder.class").get().getAsFile(), ";>Lio/grpc/ManagedChannelBuilder;", ";>Lio/grpc/ManagedChannelBuilder;"); - project.replaceConstant( - destinationDirectory.file( - "io/grpc/internal/AbstractServerImplBuilder.class").get().getAsFile(), - ";>Lio/grpc/ServerBuilder;", - ";>Lio/grpc/ServerBuilder;"); } } } diff --git a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java deleted file mode 100644 index 574117d046..0000000000 --- a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java +++ /dev/null @@ -1,233 +0,0 @@ -/* - * Copyright 2020 The gRPC Authors - * - * Licensed 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 io.grpc.internal; - -import com.google.common.base.MoreObjects; -import com.google.errorprone.annotations.DoNotCall; -import io.grpc.BinaryLog; -import io.grpc.BindableService; -import io.grpc.CompressorRegistry; -import io.grpc.DecompressorRegistry; -import io.grpc.HandlerRegistry; -import io.grpc.Server; -import io.grpc.ServerBuilder; -import io.grpc.ServerCallExecutorSupplier; -import io.grpc.ServerInterceptor; -import io.grpc.ServerServiceDefinition; -import io.grpc.ServerStreamTracer; -import io.grpc.ServerTransportFilter; -import java.io.File; -import java.io.InputStream; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; - -/** - * A {@link ServerBuilder} that delegates all its builder method to another builder by default. - * - *

Temporarily duplicates io.grpc.ForwardingServerBuilder (temporarily package-private) - * to fix ABI backward compatibility. - - * @param The concrete type of this builder. - * @see grpc/grpc-java#7211 - */ -public abstract class AbstractServerImplBuilder - > extends ServerBuilder { - - /** The default constructor. */ - protected AbstractServerImplBuilder() {} - - /** - * This method serves to force sub classes to "hide" this static factory. - */ - @DoNotCall("Unsupported") - public static ServerBuilder forPort(int port) { - throw new UnsupportedOperationException("Subclass failed to hide static factory"); - } - - /** - * Returns the delegated {@code ServerBuilder}. - */ - protected abstract ServerBuilder delegate(); - - @Override - public T directExecutor() { - delegate().directExecutor(); - return thisT(); - } - - @Override - public T callExecutor(ServerCallExecutorSupplier executorSupplier) { - delegate().callExecutor(executorSupplier); - return thisT(); - } - - @Override - public T executor(@Nullable Executor executor) { - delegate().executor(executor); - return thisT(); - } - - @Override - public T addService(ServerServiceDefinition service) { - delegate().addService(service); - return thisT(); - } - - @Override - public T addService(BindableService bindableService) { - delegate().addService(bindableService); - return thisT(); - } - - @Override - public T intercept(ServerInterceptor interceptor) { - delegate().intercept(interceptor); - return thisT(); - } - - @Override - public T addTransportFilter(ServerTransportFilter filter) { - delegate().addTransportFilter(filter); - return thisT(); - } - - @Override - public T addStreamTracerFactory(ServerStreamTracer.Factory factory) { - delegate().addStreamTracerFactory(factory); - return thisT(); - } - - @Override - public T fallbackHandlerRegistry(@Nullable HandlerRegistry fallbackRegistry) { - delegate().fallbackHandlerRegistry(fallbackRegistry); - return thisT(); - } - - @Override - public T useTransportSecurity(File certChain, File privateKey) { - delegate().useTransportSecurity(certChain, privateKey); - return thisT(); - } - - @Override - public T useTransportSecurity(InputStream certChain, InputStream privateKey) { - delegate().useTransportSecurity(certChain, privateKey); - return thisT(); - } - - @Override - public T decompressorRegistry(@Nullable DecompressorRegistry registry) { - delegate().decompressorRegistry(registry); - return thisT(); - } - - @Override - public T compressorRegistry(@Nullable CompressorRegistry registry) { - delegate().compressorRegistry(registry); - return thisT(); - } - - @Override - public T handshakeTimeout(long timeout, TimeUnit unit) { - delegate().handshakeTimeout(timeout, unit); - return thisT(); - } - - @Override - public T keepAliveTime(long keepAliveTime, TimeUnit timeUnit) { - delegate().keepAliveTime(keepAliveTime, timeUnit); - return thisT(); - } - - @Override - public T keepAliveTimeout(long keepAliveTimeout, TimeUnit timeUnit) { - delegate().keepAliveTimeout(keepAliveTimeout, timeUnit); - return thisT(); - } - - @Override - public T maxConnectionIdle(long maxConnectionIdle, TimeUnit timeUnit) { - delegate().maxConnectionIdle(maxConnectionIdle, timeUnit); - return thisT(); - } - - @Override - public T maxConnectionAge(long maxConnectionAge, TimeUnit timeUnit) { - delegate().maxConnectionAge(maxConnectionAge, timeUnit); - return thisT(); - } - - @Override - public T maxConnectionAgeGrace(long maxConnectionAgeGrace, TimeUnit timeUnit) { - delegate().maxConnectionAgeGrace(maxConnectionAgeGrace, timeUnit); - return thisT(); - } - - @Override - public T permitKeepAliveTime(long keepAliveTime, TimeUnit timeUnit) { - delegate().permitKeepAliveTime(keepAliveTime, timeUnit); - return thisT(); - } - - @Override - public T permitKeepAliveWithoutCalls(boolean permit) { - delegate().permitKeepAliveWithoutCalls(permit); - return thisT(); - } - - @Override - public T maxInboundMessageSize(int bytes) { - delegate().maxInboundMessageSize(bytes); - return thisT(); - } - - @Override - public T maxInboundMetadataSize(int bytes) { - delegate().maxInboundMetadataSize(bytes); - return thisT(); - } - - @Override - public T setBinaryLog(BinaryLog binaryLog) { - delegate().setBinaryLog(binaryLog); - return thisT(); - } - - /** - * Returns the {@link Server} built by the delegate by default. Overriding method can return - * different value. - */ - @Override - public Server build() { - return delegate().build(); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString(); - } - - /** - * Returns the correctly typed version of the builder. - */ - protected final T thisT() { - @SuppressWarnings("unchecked") - T thisT = (T) this; - return thisT; - } -} diff --git a/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java b/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java deleted file mode 100644 index 34cc3a3de5..0000000000 --- a/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2020 The gRPC Authors - * - * Licensed 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 io.grpc.internal; - -import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; - -import com.google.common.base.Defaults; -import io.grpc.ForwardingTestUtil; -import io.grpc.Server; -import io.grpc.ServerBuilder; -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; - -/** - * Unit tests for {@link AbstractServerImplBuilderTest}. - */ -@RunWith(JUnit4.class) -public class AbstractServerImplBuilderTest { - private final ServerBuilder mockDelegate = mock(ServerBuilder.class); - - private final AbstractServerImplBuilder testServerBuilder = new TestBuilder(); - - private final class TestBuilder extends AbstractServerImplBuilder { - @Override - protected ServerBuilder delegate() { - return mockDelegate; - } - } - - @Test - public void allMethodsForwarded() throws Exception { - ForwardingTestUtil.testMethodsForwarded( - ServerBuilder.class, - mockDelegate, - testServerBuilder, - Collections.emptyList()); - } - - @Test - public void allBuilderMethodsReturnThis() throws Exception { - for (Method method : ServerBuilder.class.getDeclaredMethods()) { - if (Modifier.isStatic(method.getModifiers()) - || Modifier.isPrivate(method.getModifiers()) - || Modifier.isFinal(method.getModifiers())) { - continue; - } - if (method.getName().equals("build")) { - continue; - } - Class[] argTypes = method.getParameterTypes(); - Object[] args = new Object[argTypes.length]; - for (int i = 0; i < argTypes.length; i++) { - args[i] = Defaults.defaultValue(argTypes[i]); - } - - Object returnedValue = method.invoke(testServerBuilder, args); - - assertThat(returnedValue).isSameInstanceAs(testServerBuilder); - } - } - - @Test - public void buildReturnsDelegateBuildByDefault() { - Server server = mock(Server.class); - doReturn(server).when(mockDelegate).build(); - - assertThat(testServerBuilder.build()).isSameInstanceAs(server); - } -} diff --git a/inprocess/src/main/java/io/grpc/inprocess/InProcessServerBuilder.java b/inprocess/src/main/java/io/grpc/inprocess/InProcessServerBuilder.java index 4c5d46dc54..190f67603c 100644 --- a/inprocess/src/main/java/io/grpc/inprocess/InProcessServerBuilder.java +++ b/inprocess/src/main/java/io/grpc/inprocess/InProcessServerBuilder.java @@ -22,10 +22,10 @@ import com.google.common.base.Preconditions; import com.google.errorprone.annotations.DoNotCall; import io.grpc.Deadline; import io.grpc.ExperimentalApi; +import io.grpc.ForwardingServerBuilder; import io.grpc.Internal; import io.grpc.ServerBuilder; import io.grpc.ServerStreamTracer; -import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.internal.FixedObjectPool; import io.grpc.internal.GrpcUtil; import io.grpc.internal.InternalServer; @@ -73,8 +73,7 @@ import java.util.concurrent.TimeUnit; * */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1783") -public final class InProcessServerBuilder extends - AbstractServerImplBuilder { +public final class InProcessServerBuilder extends ForwardingServerBuilder { /** * Create a server builder that will bind with the given name. * diff --git a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java index 3e8674e840..9411a979ed 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java @@ -30,11 +30,11 @@ import com.google.errorprone.annotations.CheckReturnValue; import com.google.errorprone.annotations.InlineMe; import io.grpc.Attributes; import io.grpc.ExperimentalApi; +import io.grpc.ForwardingServerBuilder; import io.grpc.Internal; import io.grpc.ServerBuilder; import io.grpc.ServerCredentials; import io.grpc.ServerStreamTracer; -import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.internal.FixedObjectPool; import io.grpc.internal.GrpcUtil; import io.grpc.internal.InternalServer; @@ -67,7 +67,7 @@ import javax.net.ssl.SSLException; */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1784") @CheckReturnValue -public final class NettyServerBuilder extends AbstractServerImplBuilder { +public final class NettyServerBuilder extends ForwardingServerBuilder { // 1MiB public static final int DEFAULT_FLOW_CONTROL_WINDOW = 1024 * 1024;