port: gz-msgs9 to gz-msgs10 (#429)

Signed-off-by: Michael Carroll <[email protected]>

core/src/DynamicFactory.cc

@@ -87,58 +87,60 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths)
   std::vector<std::string> descDirs =
     split(_paths, kEnvironmentVariableSeparator);
 
-
-  for (const std::string &descDir : descDirs)
+  auto loadDescFile = [this](const std::string &descFile)
   {
-    for (auto const &dirIter : std::filesystem::directory_iterator{descDir})
+    // Ignore files without the .desc extension.
+    if (descFile.rfind(".desc") == std::string::npos &&
+        descFile.rfind(".gz_desc") == std::string::npos &&
+        descFile.rfind(".proto") == std::string::npos &&
+        descFile.rfind(".proto.bin") == std::string::npos)
+      return;
+
+    // Parse the .desc file.
+    std::ifstream ifs(descFile);
+    if (!ifs.is_open())
     {
-      // Ignore files without the .desc extension.
-      if (dirIter.path().extension() != ".desc" &&
-          dirIter.path().extension() != ".gz_desc")
-        continue;
-
-      std::ifstream ifs(dirIter.path().string(), std::ifstream::in);
-      if (!ifs.is_open())
+      std::cerr << "DynamicFactory(): Unable to open [" << descFile << "]"
+        << std::endl;
+        return;
+    }
+    google::protobuf::FileDescriptorSet fileDescriptorSet;
+    if (!fileDescriptorSet.ParseFromIstream(&ifs))
+    {
+      std::cerr << "DynamicFactory(): Unable to parse descriptor set from ["
+                << descFile << "]" << std::endl;
+      return;
+    }
+    // Place the real descriptors in the descriptor pool.
+    for (const google::protobuf::FileDescriptorProto &fileDescriptorProto :
+         fileDescriptorSet.file())
+    {
+      if (!this->pool.BuildFile(fileDescriptorProto))
       {
-        std::cerr << "DynamicFactory(): Unable to open ["
-                  << dirIter.path() << "]"
-                  << std::endl;
-        continue;
+        std::cerr << "DynamicFactory(). Unable to place descriptors from ["
+                  << descFile << "] in the descriptor pool" << std::endl;
       }
-
-      google::protobuf::FileDescriptorSet fileDescriptorSet;
-      if (!fileDescriptorSet.ParseFromIstream(&ifs))
+      else
       {
-        std::cerr << "DynamicFactory(): Unable to parse descriptor set from ["
-                  << dirIter.path() << "]" << std::endl;
-        continue;
+        this->db.Add(fileDescriptorProto);
       }
+    }
+  };
 
-      for (const google::protobuf::FileDescriptorProto &fileDescriptorProto :
-           fileDescriptorSet.file())
+  for (const std::string &descDir : descDirs)
+  {
+    if (!std::filesystem::is_directory(descDir))
+    {
+      loadDescFile(descDir);
+    }
+    else
+    {
+      for (auto const &dirIter : std::filesystem::directory_iterator{descDir})
       {
-        // If the descriptor already exists in the database, then skip it.
-        // This may happen as gz_desc files can potentially contain the
-        // transitive message definitions
-        google::protobuf::FileDescriptorProto checkDescriptorProto;
-        if (this->db.FindFileByName(
-          fileDescriptorProto.name(), &checkDescriptorProto))
-        {
-          continue;
-        }
-
-        if (!static_cast<bool>(pool.BuildFile(fileDescriptorProto)))
-        {
-          std::cerr << "DynamicFactory(). Unable to place descriptors from ["
-                    << dirIter.path()
-                    << "] in the descriptor pool" << std::endl;
-        }
-
-        this->db.Add(fileDescriptorProto);
+        loadDescFile(dirIter.path().string());
       }
     }
   }
-
 }
 
 //////////////////////////////////////////////////

test/desc/testing.invalid

@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+package testing;
+
+message Bytes
+{
+  bytes data    = 1;
+}

test/desc/testing.proto

@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+package testing;
+
+message Bytes
+{
+  bytes data    = 1;
+}

test/desc/testing.proto.bin

@@ -0,0 +1,16 @@
+
+C
+testing/bytes.prototesting"
+Bytes
+data
 
(Rdatabproto3
+�

+testing/message.prototesting"
+
+FooMessage
+foo
 
(	Rfoo"
+
+BarMessage
+foo
 
(	Rfoo"
+
+BazMessage
+foo
 
(	Rfoobproto3

test/desc/testing/bytes.proto

@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+package testing;
+
+message Bytes
+{
+  bytes data    = 1;
+}

test/desc/testing/message.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+package testing;
+
+message FooMessage {
+    string data = 1;
+}
+
+message BarMessage {
+    FooMessage foo = 1;
+}
+
+message BazMessage {
+    BarMessage bar = 1;
+}

test/integration/CMakeLists.txt

@@ -11,12 +11,17 @@ foreach(test ${test_targets})
   target_link_libraries(${test} ${PROJECT_LIBRARY_TARGET_NAME})
 endforeach()
 
+if(TARGET INTEGRATION_descriptors)
+  target_compile_definitions(INTEGRATION_descriptors PRIVATE
+    "-DGZ_MSGS_TEST_PATH=\"${PROJECT_SOURCE_DIR}/test\"")
+endif()
+
 if(TARGET INTEGRATION_Factory_TEST)
   target_compile_definitions(INTEGRATION_Factory_TEST PRIVATE
     "-DGZ_MSGS_TEST_PATH=\"${PROJECT_SOURCE_DIR}/test\"")
 endif()
 
 if(TARGET INTEGRATION_gz_TEST)
-    target_compile_definitions(INTEGRATION_gz_TEST PUBLIC 
+    target_compile_definitions(INTEGRATION_gz_TEST PUBLIC
       "-DDETAIL_GZ_CONFIG_PATH=\"${CMAKE_BINARY_DIR}/test/conf/$<CONFIG>\"")
 endif()

test/integration/descriptors.cc

@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2024 Open Source Robotics Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <gtest/gtest.h>
+
+#include <filesystem>
+
+#include "gz/msgs/Factory.hh"
+
+static constexpr const char * kMsgsTestPath = GZ_MSGS_TEST_PATH;
+
+TEST(FactoryTest, DynamicFactory)
+{
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+
+  // Test loading an invalid path
+  {
+    std::filesystem::path test_path(kMsgsTestPath);
+    std::string paths = (test_path / "desc" / "does_not_exist.desc").string();
+    gz::msgs::Factory::LoadDescriptors(paths);
+  }
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+
+  // Test loading an incorrect extension
+  {
+    std::filesystem::path test_path(kMsgsTestPath);
+    std::string paths = (test_path / "desc" / "testing.invalid").string();
+    gz::msgs::Factory::LoadDescriptors(paths);
+  }
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+
+  // Test loading a file with invalid content
+  {
+    std::filesystem::path test_path(kMsgsTestPath);
+    std::string paths = (test_path / "desc" / "testing.proto").string();
+    gz::msgs::Factory::LoadDescriptors(paths);
+  }
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+
+  // Load a valid descriptor file with one message type
+  {
+    std::filesystem::path test_path(kMsgsTestPath);
+    std::string paths = (test_path / "desc" / "stringmsg.desc").string();
+    gz::msgs::Factory::LoadDescriptors(paths);
+  }
+
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_EQ(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+
+  // Load a directory
+  {
+    std::filesystem::path test_path(kMsgsTestPath);
+    std::string paths = (test_path / "desc").string();
+    gz::msgs::Factory::LoadDescriptors(paths);
+  }
+
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg"));
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.Bytes"));
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.FooMessage"));
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.BarMessage"));
+  EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.BazMessage"));
+}