Force descriptor initialization of dependencies *before* internalUpdateFileDescriptor(). (#15718)

zhangskz · web-flow · commit 4dec1a7d410c · 2024-02-05T11:55:40.000-05:00
This fixes an edge-case where EnumDescriptor for a custom option may be unresolved if used in the same file, since adding the field to ExtensionRegistry doesn't trigger its static init block if the Enum is imported from a dependency.

Also renames feature resolution methods exposed from gencode. Private resolveAllFeaturesInternal() method may be renamed back to resolveAllFeatures() in a followup change.

PiperOrigin-RevId: 603852391
diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java
@@ -348,7 +348,7 @@ public FieldDescriptor findExtensionByName(String name) {
      * Construct a {@code FileDescriptor}.
      *
      * @param proto the protocol message form of the FileDescriptort
-     * @param dependencies {@code FileDescriptor}s corresponding to all of the file's dependencies
+     * @param dependencies {@code FileDescriptor}s corresponding to all of the file's dependencies.
      * @throws DescriptorValidationException {@code proto} is not a valid descriptor. This can occur
      *     for a number of reasons; for instance, because a field has an undefined type or because
      *     two messages were defined with the same name.
@@ -397,7 +397,9 @@ private static FileDescriptor buildFrom(
       result.crossLink();
       // Skip feature resolution until later for calls from gencode.
       if (!allowUnresolvedFeatures) {
-        result.resolveAllFeatures();
+        // We do not need to force feature resolution for proto1 dependencies
+        // since dependencies from non-gencode should already be fully feature resolved.
+        result.resolveAllFeaturesInternal();
       }
       return result;
     }
@@ -480,19 +482,19 @@ public static FileDescriptor internalBuildGeneratedFileFrom(
       return internalBuildGeneratedFileFrom(descriptorDataParts, dependencies);
     }
 
-    /**
-     * This method is to be called by generated code only. It updates the FileDescriptorProto
-     * associated with the descriptor by parsing it again with the given ExtensionRegistry. This is
-     * needed to recognize custom options.
-     */
-    public static void internalUpdateFileDescriptor(
+    public static void internalUpdateFileDescriptorImmutable(
         FileDescriptor descriptor, ExtensionRegistry registry) {
+      internalUpdateFileDescriptor(descriptor, registry, false);
+    }
+
+    private static void internalUpdateFileDescriptor(
+        FileDescriptor descriptor, ExtensionRegistry registry, boolean mutable) {
       ByteString bytes = descriptor.proto.toByteString();
       try {
         FileDescriptorProto proto = FileDescriptorProto.parseFrom(bytes, registry);
         synchronized (descriptor) {
           descriptor.setProto(proto);
-          descriptor.resolveAllFeatures();
+          descriptor.resolveAllFeaturesImmutable();
         }
       } catch (InvalidProtocolBufferException e) {
         throw new IllegalArgumentException(
@@ -618,11 +620,15 @@ private FileDescriptor(
       pool.addSymbol(message);
     }
 
+    public void resolveAllFeaturesImmutable() {
+      resolveAllFeaturesInternal();
+    }
+
     /**
      * This method is to be called by generated code only. It resolves features for the descriptor
      * and all of its children.
      */
-    public void resolveAllFeatures() {
+    private void resolveAllFeaturesInternal() {
       if (this.features != null) {
         return;
       }
diff --git a/src/google/protobuf/compiler/java/file.cc b/src/google/protobuf/compiler/java/file.cc
@@ -430,7 +430,7 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
   // Feature resolution for Java features uses extension registry
   // which must happen after internalInit() from
   // GenerateNonNestedInitializationCode
-  printer->Print("descriptor.resolveAllFeatures();\n");
+  printer->Print("descriptor.resolveAllFeaturesImmutable();\n");
 
   // Proto compiler builds a DescriptorPool, which holds all the descriptors to
   // generate, when processing the ".proto" files. We call this DescriptorPool
@@ -460,6 +460,17 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
       return field->containing_type()->full_name() == "google.protobuf.FeatureSet";
     });
   }
+
+  // Force descriptor initialization of all dependencies.
+  for (int i = 0; i < file_->dependency_count(); i++) {
+    if (ShouldIncludeDependency(file_->dependency(i), true)) {
+      std::string dependency =
+          name_resolver_->GetImmutableClassName(file_->dependency(i));
+      printer->Print("$dependency$.getDescriptor();\n", "dependency",
+                     dependency);
+    }
+  }
+
   if (!extensions.empty()) {
     // Must construct an ExtensionRegistry containing all existing extensions
     // and use it to parse the descriptor data again to recognize extensions.
@@ -479,17 +490,7 @@ void FileGenerator::GenerateDescriptorInitializationCodeForImmutable(
     }
     printer->Print(
         "com.google.protobuf.Descriptors.FileDescriptor\n"
-        "    .internalUpdateFileDescriptor(descriptor, registry);\n");
-  }
-
-  // Force descriptor initialization of all dependencies.
-  for (int i = 0; i < file_->dependency_count(); i++) {
-    if (ShouldIncludeDependency(file_->dependency(i), true)) {
-      std::string dependency =
-          name_resolver_->GetImmutableClassName(file_->dependency(i));
-      printer->Print("$dependency$.getDescriptor();\n", "dependency",
-                     dependency);
-    }
+        "    .internalUpdateFileDescriptorImmutable(descriptor, registry);\n");
   }
 
   printer->Outdent();
diff --git a/src/google/protobuf/compiler/java/shared_code_generator.cc b/src/google/protobuf/compiler/java/shared_code_generator.cc
@@ -76,9 +76,6 @@ void SharedCodeGenerator::Generate(
                              options_.annotate_code ? info_relative_path : "",
                              options_);
 
-    if (!options_.opensource_runtime) {
-      printer->Print("@com.google.protobuf.Internal.ProtoNonnullApi\n");
-    }
 
     printer->Print(
 
@@ -87,17 +84,12 @@ void SharedCodeGenerator::Generate(
         "returns\n"
         "  * an incomplete descriptor for internal use only. */\n"
         "  public static com.google.protobuf.Descriptors.FileDescriptor\n"
-        "      descriptor;\n"
-        "  /* This method is to be called by generated code only. It returns\n"
-        "  * an incomplete descriptor for internal use only. */\n"
-        "  public static com.google.protobuf.Descriptors.FileDescriptor "
-        "getDescriptor() {\n"
-        "    descriptor.resolveAllFeatures();\n"
-        "    return descriptor;\n"
-        "  }\n"
-        "  static {\n",
+        "      descriptor;\n",
         "classname", classname);
     printer->Annotate("classname", file_->name());
+
+
+    printer->Print("  static {\n");
     printer->Indent();
     printer->Indent();
     GenerateDescriptors(printer.get());
