Skip to content

Commit

Permalink
Binary compatibility shims for GeneratedMessageV3, SingleFieldBuilder…
Browse files Browse the repository at this point in the history 
…V3, RepeatedFieldBuilderV3, and their nested classes to restore binary compatibility with <=v3.x.x generated code built against v3.x.x prior to v4.26.0 breaking release.

3.x.x descriptor.proto generated code is *not* supported with 4.x.x runtime, since this results in an ODR violation with the descriptor.proto built into the 4.x.x runtime. This is expected to result in undefined behavior / failures.

Tested against //java/core:v25_generated_message_test_jar (binary compatibility) and //java/core:v25_generated_message_test_srcjar (source compatibility)

PiperOrigin-RevId: 664819152
  • Loading branch information
@zhangskz
zhangskz committed Aug 23, 2024
1 parent 8c96e55 commit 8dd83da
Show file tree
Hide file tree
Showing 11 changed files with 1,500 additions and 57 deletions.
10 changes: 3 additions & 7 deletions WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,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()
20 changes: 20 additions & 0 deletions compatibility/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Binary file added compatibility/v3.25.0/generic_test_protos-speed.srcjar
View file
Binary file not shown.
Binary file added compatibility/v3.25.0/java_test_protos-speed.srcjar
View file
Binary file not shown.
Binary file added compatibility/v3.25.0/lite_test_protos-speed.srcjar
View file
Binary file not shown.
96 changes: 96 additions & 0 deletions java/core/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,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([
Expand Down
52 changes: 31 additions & 21 deletions java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -1036,7 +1038,8 @@ protected class ExtensionWriter {
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();
}
Expand Down Expand Up @@ -1254,7 +1257,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);
Expand All @@ -1263,7 +1267,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);
Expand All @@ -1273,7 +1278,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);
Expand All @@ -1295,7 +1300,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);
Expand All @@ -1309,7 +1314,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);
Expand All @@ -1322,7 +1327,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);
Expand All @@ -1335,7 +1342,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);
Expand All @@ -1347,7 +1354,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);
Expand Down Expand Up @@ -1563,7 +1571,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);
Expand Down Expand Up @@ -1939,7 +1948,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
Expand Down Expand Up @@ -3136,7 +3146,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.");
}
Expand Down
Loading

0 comments on commit 8dd83da

Please sign in to comment.