Merge pull request #17913 from protocolbuffers/cp-compat-upgrade

Binary compatibility shims for GeneratedMessageV3, SingleFieldBuilder…
protocolbuffers · Aug 22, 2024 · 13f850d · 13f850d
2 parents bb6ecb9 + 6bf01c5
commit 13f850d
Show file tree

Hide file tree

Showing 11 changed files with 1,500 additions and 57 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -238,10 +238,6 @@ http_archive(
     url = "https://github.com/protocolbuffers/protobuf/releases/download/v25.0/protobuf-25.0.tar.gz",
 )
 
-# Needed as a dependency of @com_google_protobuf_v25.x, which was before
-# utf8_range was merged in.
-http_archive(
-    name = "utf8_range",
-    strip_prefix = "utf8_range-d863bc33e15cba6d873c878dcca9e6fe52b2f8cb",
-    url = "https://github.com/protocolbuffers/utf8_range/archive/d863bc33e15cba6d873c878dcca9e6fe52b2f8cb.zip",
-)
+# Needed as a dependency of @com_google_protobuf_v25.0
+load("@com_google_protobuf_v25.0//:protobuf_deps.bzl", protobuf_v25_deps="protobuf_deps")
+protobuf_v25_deps()
diff --git a/compatibility/BUILD.bazel b/compatibility/BUILD.bazel
@@ -6,6 +6,26 @@
 
 load("//compatibility:runtime_conformance.bzl", "java_runtime_conformance")
 
+java_library(
+    name = "v25_test_protos_srcjar",
+    testonly = True,
+    srcs = glob([
+        "v3.25.0/*.srcjar",
+    ]),
+    visibility = ["//java/core:__pkg__"],
+    deps = ["//java/core"],
+)
+
+java_library(
+    name = "v25_test_protos_jar",
+    testonly = True,
+    srcs = glob([
+        "v3.25.0/*.srcjar",
+    ]),
+    visibility = ["//java/core:__pkg__"],
+    deps = ["@com_google_protobuf_v25.0//java/core"],
+)
+
 # main gencode builds with main runtime as a proof of concept.
 java_runtime_conformance(
     name = "java_conformance_main",

diff --git a/compatibility/v3.25.0/generic_test_protos-speed.srcjar b/compatibility/v3.25.0/generic_test_protos-speed.srcjar
diff --git a/compatibility/v3.25.0/java_test_protos-speed.srcjar b/compatibility/v3.25.0/java_test_protos-speed.srcjar
diff --git a/compatibility/v3.25.0/lite_test_protos-speed.srcjar b/compatibility/v3.25.0/lite_test_protos-speed.srcjar
diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel
@@ -589,6 +589,102 @@ junit_tests(
     ],
 )
 
+protobuf_java_library(
+    name = "v25_test_util_srcjar",
+    testonly = True,
+    srcs = [
+        "src/test/java/com/google/protobuf/TestUtil.java",
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
+    ],
+    deps = [
+        ":core",
+        "//compatibility:v25_test_protos_srcjar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
+    ],
+)
+
+# Tests source compatibility against v25 gencode jar compiled against current runtime
+junit_tests(
+    name = "v25_core_tests_srcjar",
+    size = "small",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = [
+            # Depends on test protos or API changes added in v4.x.x (e.g. editions)
+            "src/test/java/com/google/protobuf/TextFormatTest.java",
+            "src/test/java/com/google/protobuf/DescriptorsTest.java",
+            "src/test/java/com/google/protobuf/DebugFormatTest.java",
+            "src/test/java/com/google/protobuf/CodedOutputStreamTest.java",
+            "src/test/java/com/google/protobuf/ProtobufToStringOutputTest.java",
+            # Excluded in core_tests
+            "src/test/java/com/google/protobuf/DecodeUtf8Test.java",
+            "src/test/java/com/google/protobuf/IsValidUtf8Test.java",
+            "src/test/java/com/google/protobuf/TestUtil.java",
+            "src/test/java/com/google/protobuf/TestUtilLite.java",
+            "src/test/java/com/google/protobuf/RuntimeVersionTest.java",
+        ],
+    ),
+    test_prefix = "v25SrcJar",
+    deps = [
+        ":core",
+        ":v25_test_util_srcjar",
+        "//compatibility:v25_test_protos_srcjar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_mockito_mockito_core",
+    ],
+)
+
+protobuf_java_library(
+    name = "v25_test_util_jar",
+    testonly = True,
+    srcs = [
+        "src/test/java/com/google/protobuf/TestUtil.java",
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
+    ],
+    deps = [
+        ":core",
+        "//compatibility:v25_test_protos_jar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
+    ],
+)
+
+# Tests binary compatibility against v25 gencode ja compiled against v25 runtime
+junit_tests(
+    name = "v25_core_tests_jar",
+    size = "small",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = [
+            # Depends on test protos or API changes added in v4.x.x (e.g. editions)
+            "src/test/java/com/google/protobuf/TextFormatTest.java",
+            "src/test/java/com/google/protobuf/DescriptorsTest.java",
+            "src/test/java/com/google/protobuf/DebugFormatTest.java",
+            "src/test/java/com/google/protobuf/CodedOutputStreamTest.java",
+            "src/test/java/com/google/protobuf/ProtobufToStringOutputTest.java",
+            # Excluded in core_tests
+            "src/test/java/com/google/protobuf/DecodeUtf8Test.java",
+            "src/test/java/com/google/protobuf/IsValidUtf8Test.java",
+            "src/test/java/com/google/protobuf/TestUtil.java",
+            "src/test/java/com/google/protobuf/TestUtilLite.java",
+            "src/test/java/com/google/protobuf/RuntimeVersionTest.java",
+        ],
+    ),
+    test_prefix = "v25Jar",
+    deps = [
+        ":core",
+        ":v25_test_util_jar",
+        "//compatibility:v25_test_protos_jar",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_mockito_mockito_core",
+    ],
+)
+
 pkg_files(
     name = "dist_files",
     srcs = glob([

diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
@@ -886,16 +886,16 @@ public interface ExtendableMessageOrBuilder<MessageT extends ExtendableMessage<M
     Message getDefaultInstanceForType();
 
     /** Check if a singular extension is present. */
-    <T> boolean hasExtension(ExtensionLite<MessageT, T> extension);
+    <T> boolean hasExtension(ExtensionLite<? extends MessageT, T> extension);
 
     /** Get the number of elements in a repeated extension. */
-    <T> int getExtensionCount(ExtensionLite<MessageT, List<T>> extension);
+    <T> int getExtensionCount(ExtensionLite<? extends MessageT, List<T>> extension);
 
     /** Get the value of an extension. */
-    <T> T getExtension(ExtensionLite<MessageT, T> extension);
+    <T> T getExtension(ExtensionLite<? extends MessageT, T> extension);
 
     /** Get one element of a repeated extension. */
-    <T> T getExtension(ExtensionLite<MessageT, List<T>> extension, int index);
+    <T> T getExtension(ExtensionLite<? extends MessageT, List<T>> extension, int index);
   }
 
   /**
@@ -946,7 +946,7 @@ protected ExtendableMessage(ExtendableBuilder<MessageT, ?> builder) {
       this.extensions = builder.buildExtensions();
     }
 
-    private void verifyExtensionContainingType(final Extension<MessageT, ?> extension) {
+    private void verifyExtensionContainingType(final Extension<? extends MessageT, ?> extension) {
       if (extension.getDescriptor().getContainingType() != getDescriptorForType()) {
         // This can only happen if someone uses unchecked operations.
         throw new IllegalArgumentException(
@@ -960,7 +960,8 @@ private void verifyExtensionContainingType(final Extension<MessageT, ?> extensio
 
     /** Check if a singular extension is present. */
     @Override
-    public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> boolean hasExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -969,7 +970,8 @@ public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extension
 
     /** Get the number of elements in a repeated extension. */
     @Override
-    public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> extensionLite) {
+    public final <T> int getExtensionCount(
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -980,7 +982,7 @@ public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> ex
     /** Get the value of an extension. */
     @Override
     @SuppressWarnings("unchecked")
-    public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> T getExtension(final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1003,7 +1005,7 @@ public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite)
     @Override
     @SuppressWarnings("unchecked")
     public final <T> T getExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final int index) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1054,7 +1056,8 @@ protected class ExtensionWriter implements ExtensionSerializer {
       private Map.Entry<FieldDescriptor, Object> next;
       private final boolean messageSetWireFormat;
 
-      private ExtensionWriter(final boolean messageSetWireFormat) {
+      // TODO: Should be marked private in v5.x.x once GeneratedMessageV3 is removed.
+      protected ExtensionWriter(final boolean messageSetWireFormat) {
         if (iter.hasNext()) {
           next = iter.next();
         }
@@ -1298,7 +1301,8 @@ private void verifyExtensionContainingType(final Extension<MessageT, ?> extensio
 
     /** Check if a singular extension is present. */
     @Override
-    public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> boolean hasExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1307,7 +1311,8 @@ public final <T> boolean hasExtension(final ExtensionLite<MessageT, T> extension
 
     /** Get the number of elements in a repeated extension. */
     @Override
-    public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> extensionLite) {
+    public final <T> int getExtensionCount(
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1317,7 +1322,7 @@ public final <T> int getExtensionCount(final ExtensionLite<MessageT, List<T>> ex
 
     /** Get the value of an extension. */
     @Override
-    public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> T getExtension(final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1339,7 +1344,7 @@ public final <T> T getExtension(final ExtensionLite<MessageT, T> extensionLite)
     /** Get one element of a repeated extension. */
     @Override
     public final <T> T getExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final int index) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1353,7 +1358,7 @@ public final <T> T getExtension(
 
     /** Set the value of an extension. */
     public final <T> BuilderT setExtension(
-        final ExtensionLite<MessageT, T> extensionLite, final T value) {
+        final ExtensionLite<? extends MessageT, T> extensionLite, final T value) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1366,7 +1371,9 @@ public final <T> BuilderT setExtension(
 
     /** Set the value of one element of a repeated extension. */
     public final <T> BuilderT setExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final int index, final T value) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite,
+        final int index,
+        final T value) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1379,7 +1386,7 @@ public final <T> BuilderT setExtension(
 
     /** Append a value to a repeated extension. */
     public final <T> BuilderT addExtension(
-        final ExtensionLite<MessageT, List<T>> extensionLite, final T value) {
+        final ExtensionLite<? extends MessageT, List<T>> extensionLite, final T value) {
       Extension<MessageT, List<T>> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1391,7 +1398,8 @@ public final <T> BuilderT addExtension(
     }
 
     /** Clear an extension. */
-    public final <T> BuilderT clearExtension(final ExtensionLite<MessageT, T> extensionLite) {
+    public final <T> BuilderT clearExtension(
+        final ExtensionLite<? extends MessageT, T> extensionLite) {
       Extension<MessageT, T> extension = checkNotLite(extensionLite);
 
       verifyExtensionContainingType(extension);
@@ -1607,7 +1615,8 @@ public Message.Builder newBuilderForField(final FieldDescriptor field) {
       }
     }
 
-    protected final void mergeExtensionFields(final ExtendableMessage<?> other) {
+    // TODO: Should be marked final in v5.x.x once GeneratedMessageV3 is removed.
+    protected void mergeExtensionFields(final ExtendableMessage<?> other) {
       if (other.extensions != null) {
         ensureExtensionsIsMutable();
         extensions.mergeFrom(other.extensions);
@@ -1983,7 +1992,8 @@ protected MapField internalGetMapField(int fieldNumber) {
    * Users should ignore this class. This class provides the implementation with access to the
    * fields of a message object using Java reflection.
    */
-  public static final class FieldAccessorTable {
+  // TODO: Should be marked final in v5.x.x once GeneratedMessageV3 is removed.
+  public static class FieldAccessorTable {
 
     /**
      * Construct a FieldAccessorTable for a particular message class. Only one FieldAccessorTable
@@ -3176,7 +3186,7 @@ protected Object writeReplace() throws ObjectStreamException {
    * Checks that the {@link Extension} is non-Lite and returns it as a {@link GeneratedExtension}.
    */
   private static <MessageT extends ExtendableMessage<MessageT>, T>
-      Extension<MessageT, T> checkNotLite(ExtensionLite<MessageT, T> extension) {
+      Extension<MessageT, T> checkNotLite(ExtensionLite<? extends MessageT, T> extension) {
     if (extension.isLite()) {
       throw new IllegalArgumentException("Expected non-lite extension.");
     }