Skip to content

Commit 9325480

Browse files
Fix handling of optional dependencies in java generator.
Java reflection is used to fail smoothly when the extensions aren't found in the classpath. PiperOrigin-RevId: 802766998
1 parent 34d6c85 commit 9325480

File tree

4 files changed

+91
-41
lines changed

4 files changed

+91
-41
lines changed
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.google.protobuf;
2+
3+
/**
4+
* All generated outer classes extend this class. This class implements shared logic that should
5+
* only be called by generated code.
6+
*
7+
* <p>Users should generally ignore this class and use the generated Outer class directly instead.
8+
*
9+
* <p>This class is intended to only be extended by protoc created gencode. It is not intended or
10+
* supported to extend this class, and any protected methods may be removed without it being
11+
* considered a breaking change as long as all supported gencode does not depend on the changed
12+
* methods.
13+
*/
14+
public abstract class GeneratedFile {
15+
16+
protected GeneratedFile() {}
17+
18+
/** Add an optional extension from a file that may or may not be in the classpath. */
19+
protected static void addOptionalExtension(
20+
ExtensionRegistry registry, final String className, final String fieldName) {
21+
try {
22+
GeneratedMessage.GeneratedExtension<?, ?> ext =
23+
(GeneratedMessage.GeneratedExtension<?, ?>)
24+
Class.forName(className).getField(fieldName).get(null);
25+
registry.add(ext);
26+
} catch (ClassNotFoundException e) {
27+
// ignore missing class if it's not in the classpath.
28+
} catch (NoSuchFieldException e) {
29+
// ignore missing field if it's not in the classpath.
30+
} catch (IllegalAccessException e) {
31+
// ignore malformed field if it's not what we expect.
32+
}
33+
}
34+
}

java/core/src/test/java/com/google/protobuf/ImportOptionTest.java

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ public final class ImportOptionTest {
1818

1919
@Test
2020
public void testImportOption() throws Exception {
21-
// Ensure that UnittestCustomOptions is linked in and referenced.
22-
FileDescriptor unused = UnittestCustomOptions.getDescriptor();
23-
2421
FileDescriptor fileDescriptor = UnittestImportOptionProto.getDescriptor();
2522
Descriptor messageDescriptor = TestMessage.getDescriptor();
2623
FieldDescriptor fieldDescriptor = messageDescriptor.findFieldByName("field1");
@@ -29,26 +26,15 @@ public void testImportOption() throws Exception {
2926
UnknownFieldSet unknownFieldsMessage = messageDescriptor.getOptions().getUnknownFields();
3027
UnknownFieldSet unknownFieldsField = fieldDescriptor.getOptions().getUnknownFields();
3128

32-
// TODO: Currently linked in options also end up in unknown fields.
33-
// TODO: Exclude for open source tests once linked in options are treated
34-
// differently, since `option_deps` are treated as `deps` in Bazel 7.
35-
// assertThat(fileDescriptor.getOptions().getExtension(UnittestCustomOptions.fileOpt1))
36-
// .isEqualTo(1);
37-
// assertThat(messageDescriptor.getOptions().getExtension(UnittestCustomOptions.messageOpt1))
38-
// .isEqualTo(2);
39-
// assertThat(fieldDescriptor.getOptions().getExtension(UnittestCustomOptions.fieldOpt1))
40-
// .isEqualTo(3);
41-
assertThat(unknownFieldsFile.getField(7736974).getVarintList()).containsExactly(1L);
42-
assertThat(unknownFieldsMessage.getField(7739036).getVarintList()).containsExactly(2L);
43-
assertThat(unknownFieldsField.getField(7740936).getFixed64List()).containsExactly(3L);
44-
45-
// Options from import option that are not linked in should be in unknown fields.
46-
assertThat(unknownFieldsFile.getField(7736975).getVarintList()).containsExactly(1L);
47-
assertThat(unknownFieldsMessage.getField(7739037).getVarintList()).containsExactly(2L);
48-
assertThat(unknownFieldsField.getField(7740937).getFixed64List()).containsExactly(3L);
29+
assertThat(fileDescriptor.getOptions().getExtension(UnittestCustomOptions.fileOpt1))
30+
.isEqualTo(1);
31+
assertThat(messageDescriptor.getOptions().getExtension(UnittestCustomOptions.messageOpt1))
32+
.isEqualTo(2);
33+
assertThat(fieldDescriptor.getOptions().getExtension(UnittestCustomOptions.fieldOpt1))
34+
.isEqualTo(3);
4935

50-
assertThat(unknownFieldsFile.asMap()).hasSize(2);
51-
assertThat(unknownFieldsMessage.asMap()).hasSize(2);
52-
assertThat(unknownFieldsField.asMap()).hasSize(2);
36+
// TODO: Since `option_deps` are treated as `deps` in Bazel 7, the unknown
37+
// fields are not empty. Once we drop Bazel 7 support we can test that these are filled with
38+
// unknown fields.
5339
}
5440
}

src/google/protobuf/compiler/java/file.cc

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "google/protobuf/compiler/java/internal_helpers.h"
3232
#include "google/protobuf/compiler/java/lite/generator_factory.h"
3333
#include "google/protobuf/compiler/java/name_resolver.h"
34+
#include "google/protobuf/compiler/java/names.h"
3435
#include "google/protobuf/compiler/java/options.h"
3536
#include "google/protobuf/compiler/java/shared_code_generator.h"
3637
#include "google/protobuf/compiler/retention.h"
@@ -116,8 +117,9 @@ void CollectPublicDependencies(
116117

117118
// Finds all extensions for custom options in the given file descriptor with the
118119
// builder pool which resolves Java features instead of the generated pool.
119-
void CollectExtensions(const FileDescriptor& file,
120-
FieldDescriptorSet* extensions) {
120+
void CollectExtensions(const FileDescriptor& file, const Options& options,
121+
FieldDescriptorSet* extensions,
122+
FieldDescriptorSet* optional_extensions) {
121123
FileDescriptorProto file_proto = StripSourceRetentionOptions(file);
122124
std::string file_data;
123125
file_proto.SerializeToString(&file_data);
@@ -139,15 +141,27 @@ void CollectExtensions(const FileDescriptor& file,
139141
// Unknown extensions are ok and expected in the case of option imports.
140142
CollectExtensions(*dynamic_file_proto, extensions);
141143

142-
// TODO Check against dependencies to remove option dependencies
143-
// polluting the pool. These will be handled as optional dependencies.
144+
if (options.strip_nonfunctional_codegen) {
145+
// Skip feature extensions, which are a visible (but non-functional)
146+
// deviation between editions and legacy syntax.
147+
absl::erase_if(*extensions, [](const FieldDescriptor* field) {
148+
return field->containing_type()->full_name() == "google.protobuf.FeatureSet";
149+
});
150+
}
151+
152+
// Check against dependencies to handle option dependencies polluting pool.
144153
absl::flat_hash_set<const FileDescriptor*> dependencies;
145154
dependencies.insert(&file);
146155
for (int i = 0; i < file.dependency_count(); i++) {
147156
CollectPublicDependencies(file.dependency(i), &dependencies);
148157
}
158+
for (auto* extension : *extensions) {
159+
if (!dependencies.contains(extension->file())) {
160+
optional_extensions->insert(extension);
161+
}
162+
}
149163
absl::erase_if(*extensions, [&](const FieldDescriptor* fieldDescriptor) {
150-
return !dependencies.contains(fieldDescriptor->file());
164+
return optional_extensions->contains(fieldDescriptor);
151165
});
152166
}
153167

@@ -325,11 +339,14 @@ void FileGenerator::Generate(io::Printer* printer) {
325339
printer->Print("@com.google.protobuf.Internal.ProtoNonnullApi\n");
326340
}
327341
printer->Print(
328-
"$deprecation$public final class $classname$ {\n"
342+
"$deprecation$public final class $classname$ $extends${\n"
329343
" private $ctor$() {}\n",
330344
"deprecation",
331345
file_->options().deprecated() ? "@java.lang.Deprecated " : "",
332-
"classname", classname_, "ctor", classname_);
346+
"classname", classname_, "ctor", classname_, "extends",
347+
HasDescriptorMethods(file_, context_->EnforceLite())
348+
? "extends com.google.protobuf.GeneratedFile "
349+
: "");
333350
printer->Annotate("classname", file_->name());
334351
printer->Indent();
335352

@@ -507,15 +524,8 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
507524
// of the FileDescriptor based on the builder-pool, then we can use
508525
// reflections to find all extension fields
509526
FieldDescriptorSet extensions;
510-
CollectExtensions(*file_, &extensions);
511-
512-
if (options_.strip_nonfunctional_codegen) {
513-
// Skip feature extensions, which are a visible (but non-functional)
514-
// deviation between editions and legacy syntax.
515-
absl::erase_if(extensions, [](const FieldDescriptor* field) {
516-
return field->containing_type()->full_name() == "google.protobuf.FeatureSet";
517-
});
518-
}
527+
FieldDescriptorSet optional_extensions;
528+
CollectExtensions(*file_, options_, &extensions, &optional_extensions);
519529

520530
// Force descriptor initialization of all dependencies.
521531
for (int i = 0; i < file_->dependency_count(); i++) {
@@ -527,7 +537,7 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
527537
}
528538
}
529539

530-
if (!extensions.empty()) {
540+
if (!extensions.empty() || !optional_extensions.empty()) {
531541
// Must construct an ExtensionRegistry containing all existing extensions
532542
// and use it to parse the descriptor data again to recognize extensions.
533543
printer->Print(
@@ -544,6 +554,25 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
544554
"private static void _clinit_autosplit_dinit_$method_num$(\n"
545555
" com.google.protobuf.ExtensionRegistry registry) {\n");
546556
}
557+
for (const FieldDescriptor* field : optional_extensions) {
558+
std::unique_ptr<ExtensionGenerator> generator(
559+
generator_factory_->NewExtensionGenerator(field));
560+
printer->Emit({{"scope", field->extension_scope() != nullptr
561+
? name_resolver_->GetImmutableClassName(
562+
field->extension_scope())
563+
: name_resolver_->GetImmutableClassName(
564+
field->file())},
565+
{"name", UnderscoresToCamelCaseCheckReserved(field)}},
566+
R"java(
567+
addOptionalExtension(registry, "$scope$", "$name$");
568+
)java");
569+
bytecode_estimate += 8;
570+
MaybeRestartJavaMethod(
571+
printer, &bytecode_estimate, &method_num,
572+
"_clinit_autosplit_dinit_$method_num$(registry);\n",
573+
"private static void _clinit_autosplit_dinit_$method_num$(\n"
574+
" com.google.protobuf.ExtensionRegistry registry) {\n");
575+
}
547576
printer->Print(
548577
"com.google.protobuf.Descriptors.FileDescriptor\n"
549578
" .internalUpdateFileDescriptor(descriptor, registry);\n");

src/google/protobuf/compiler/java/file.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
#include <vector>
1818

1919
#include "google/protobuf/compiler/java/options.h"
20+
#include "google/protobuf/descriptor.pb.h"
2021
#include "google/protobuf/port.h"
2122

2223
namespace google {
2324
namespace protobuf {
24-
class FileDescriptor; // descriptor.h
2525
namespace io {
2626
class Printer; // printer.h
2727
}
@@ -75,6 +75,7 @@ class FileGenerator {
7575
bool ShouldIncludeDependency(const FileDescriptor* descriptor,
7676
bool immutable_api_);
7777

78+
7879
const FileDescriptor* file_;
7980
std::string java_package_;
8081
std::string classname_;

0 commit comments

Comments
 (0)