Project import generated by Copybara.

GitOrigin-RevId: d4a11282d20fe4d2e137f9032cf349750030dcb9

mediapipe/framework/tool/options_util.cc

@@ -65,9 +65,9 @@ absl::Status CopyLiteralOptions(CalculatorGraphConfig::Node parent_node,
 
   OptionsSyntaxUtil syntax_util;
   for (auto& node : *config->mutable_node()) {
-    FieldData node_data = options_field_util::AsFieldData(node);
-
     for (const std::string& option_def : node.option_value()) {
+      FieldData node_data = options_field_util::AsFieldData(node);
+
       std::vector<absl::string_view> tag_and_name =
           syntax_util.StrSplitTags(option_def);
       std::string graph_tag = syntax_util.OptionFieldsTag(tag_and_name[1]);
@@ -96,6 +96,7 @@ absl::Status CopyLiteralOptions(CalculatorGraphConfig::Node parent_node,
       status.Update(MergeField(node_path, packet_data, &node_options));
       options_field_util::SetOptionsMessage(node_options, &node);
     }
+    node.clear_option_value();
   }
   return status;
 }

mediapipe/framework/tool/options_util_test.cc

@@ -137,7 +137,6 @@ TEST_F(OptionsUtilTest, CopyLiteralOptions) {
   NightLightCalculatorOptions expected_node_options;
   expected_node_options.add_num_lights(8);
   expected_node.add_node_options()->PackFrom(expected_node_options);
-  *expected_node.add_option_value() = "num_lights:options/chain_length";
   EXPECT_THAT(actual_node, EqualsProto(expected_node));
 
   MP_EXPECT_OK(graph.StartRun({}));

mediapipe/framework/tool/subgraph_expansion_test.cc

@@ -656,7 +656,6 @@ TEST(SubgraphExpansionTest, SimpleSubgraphOptionsUsage) {
               chain_length: 3
             }
           }
-          option_value: "chain_length:options/chain_length"
         }
         type: "MoonSubgraph"
         graph_options {
@@ -666,5 +665,84 @@ TEST(SubgraphExpansionTest, SimpleSubgraphOptionsUsage) {
   EXPECT_THAT(moon_subgraph, mediapipe::EqualsProto(expected_graph));
 }
 
+// Shows ExpandSubgraphs applied twice. "option_value" fields are evaluated
+// and removed on the first ExpandSubgraphs call.  If "option_value" fields
+// are not removed during ExpandSubgraphs, they evaluate incorrectly on the
+// second ExpandSubgraphs call and this test fails on "expected_node_options".
+TEST(SubgraphExpansionTest, SimpleSubgraphOptionsTwice) {
+  GraphRegistry graph_registry;
+
+  // Register a simple-subgraph that accepts graph options.
+  auto moon_subgraph =
+      mediapipe::ParseTextProtoOrDie<CalculatorGraphConfig>(R"pb(
+        type: "MoonSubgraph"
+        graph_options: {
+          [type.googleapis.com/mediapipe.NodeChainSubgraphOptions] {}
+        }
+        node: {
+          calculator: "MoonCalculator"
+          node_options: {
+            [type.googleapis.com/mediapipe.NodeChainSubgraphOptions] {}
+          }
+          option_value: "chain_length:options/chain_length"
+        }
+      )pb");
+  graph_registry.Register("MoonSubgraph", moon_subgraph);
+
+  // Invoke the simple-subgraph with graph options.
+  // The empty NodeChainSubgraphOptions below allows "option_value" fields
+  // on "MoonCalculator" to evaluate incorrectly, if not removed.
+  auto sky_graph = mediapipe::ParseTextProtoOrDie<CalculatorGraphConfig>(R"pb(
+    graph_options: {
+      [type.googleapis.com/mediapipe.NodeChainSubgraphOptions] {}
+    }
+    node: {
+      calculator: "MoonSubgraph"
+      options: {
+        [mediapipe.NodeChainSubgraphOptions.ext] {
+          node_type: "DoubleIntCalculator"
+          chain_length: 3
+        }
+      }
+    }
+  )pb");
+
+  // The first ExpandSubgraphs call evaluates and removes "option_value" fields.
+  MP_ASSERT_OK(tool::ExpandSubgraphs(&sky_graph, &graph_registry));
+  auto expanded_1 = sky_graph;
+
+  // The second ExpandSubgraphs call has no effect on the expanded graph.
+  MP_ASSERT_OK(tool::ExpandSubgraphs(&sky_graph, &graph_registry));
+
+  // Validate the expected node_options for the "MoonSubgraph".
+  // If the "option_value" fields are not removed during ExpandSubgraphs,
+  // this test fails with an incorrect value for "chain_length".
+  auto expected_node_options =
+      mediapipe::ParseTextProtoOrDie<mediapipe::NodeChainSubgraphOptions>(
+          "chain_length: 3");
+  mediapipe::NodeChainSubgraphOptions node_options;
+  sky_graph.node(0).node_options(0).UnpackTo(&node_options);
+  ASSERT_THAT(node_options, mediapipe::EqualsProto(expected_node_options));
+
+  // Validate the results from both ExpandSubgraphs() calls.
+  CalculatorGraphConfig expected_graph =
+      mediapipe::ParseTextProtoOrDie<CalculatorGraphConfig>(R"pb(
+        graph_options {
+          [type.googleapis.com/mediapipe.NodeChainSubgraphOptions] {}
+        }
+        node {
+          name: "moonsubgraph__MoonCalculator"
+          calculator: "MoonCalculator"
+          node_options {
+            [type.googleapis.com/mediapipe.NodeChainSubgraphOptions] {
+              chain_length: 3
+            }
+          }
+        }
+      )pb");
+  EXPECT_THAT(expanded_1, mediapipe::EqualsProto(expected_graph));
+  EXPECT_THAT(sky_graph, mediapipe::EqualsProto(expected_graph));
+}
+
 }  // namespace
 }  // namespace mediapipe