Skip to content

Commit

Permalink
Merge pull request #17913 from protocolbuffers/cp-compat-upgrade
Browse files Browse the repository at this point in the history 
Binary compatibility shims for GeneratedMessageV3, SingleFieldBuilder…
  • Loading branch information
@zhangskz
zhangskz authored Aug 22, 2024
2 parents bb6ecb9 + 6bf01c5 commit 13f850d
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 @@ -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()
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 @@ -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([
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 @@ -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();
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.");
}
Expand Down
Loading

0 comments on commit 13f850d

Please sign in to comment.