RSDK-9299 - refactor get_resource_name to be a non-proto method (#329)

src/viam/sdk/common/utils.cpp

@@ -4,6 +4,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include <boost/algorithm/string.hpp>
 #include <boost/blank.hpp>
 #include <boost/log/core.hpp>
 #include <boost/log/expressions.hpp>
@@ -183,6 +184,26 @@ bool from_dm_from_extra(const ProtoStruct& extra) {
     }
     return false;
 }
+std::pair<std::string, std::string> long_name_to_remote_and_short(const std::string& long_name) {
+    std::vector<std::string> name_parts;
+    // boost::split causes a clang-tidy false positive, see
+    // https://bugs.llvm.org/show_bug.cgi?id=41141
+    //
+    // NOLINTNEXTLINE
+    boost::split(name_parts, long_name, boost::is_any_of(":"));
+    auto name = name_parts.back();
+    name_parts.pop_back();
+    auto remote_name = name_parts.empty()
+                           ? ""
+                           : std::accumulate(std::next(name_parts.begin()),
+                                             name_parts.end(),
+                                             *name_parts.begin(),
+                                             [](const std::string& a, const std::string& b) {
+                                                 return a + ":" + b;
+                                             });
+
+    return {std::move(remote_name), std::move(name)};
+}
 
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/common/utils.hpp

@@ -61,6 +61,10 @@ class ClientContext {
     grpc::ClientContext wrapped_context_;
 };
 
+/// @brief Given a fully qualified resource name, returns remote name (or "" if no remote name
+/// exists) and short name
+std::pair<std::string, std::string> long_name_to_remote_and_short(const std::string& long_name);
+
 /// @brief Returns a new `ProtoStruct` with a random key for server-side debug logging
 ProtoStruct debug_map();
 

src/viam/sdk/components/component.cpp

@@ -2,26 +2,17 @@
 
 #include <string>
 
-#include <google/protobuf/struct.pb.h>
-
-#include <viam/api/common/v1/common.pb.h>
-
 #include <viam/sdk/common/utils.hpp>
-#include <viam/sdk/resource/resource.hpp>
 
 namespace viam {
 namespace sdk {
 
-using viam::common::v1::ResourceName;
-
-Component::Component() : Resource("component"){};
+Component::Component() : Resource("component") {}
 
-Component::Component(std::string name) : Resource(std::move(name)){};
+Component::Component(std::string name) : Resource(std::move(name)) {}
 
-ResourceName Component::get_resource_name(std::string name) const {
-    auto r = this->Resource::get_resource_name(name);
-    *r.mutable_type() = kComponent;
-    return r;
+Name Component::get_resource_name() const {
+    return Resource::get_resource_name(kComponent);
 }
 
 }  // namespace sdk

src/viam/sdk/components/component.hpp

@@ -2,10 +2,6 @@
 
 #include <string>
 
-#include <google/protobuf/struct.pb.h>
-
-#include <viam/api/common/v1/common.pb.h>
-
 #include <viam/sdk/resource/resource.hpp>
 
 namespace viam {
@@ -14,7 +10,7 @@ namespace sdk {
 class Component : public Resource {
    public:
     Component();
-    viam::common::v1::ResourceName get_resource_name(std::string name) const override;
+    Name get_resource_name() const override;
 
    protected:
     explicit Component(std::string name);

src/viam/sdk/registry/registry.cpp

@@ -162,7 +162,7 @@ Registry::registered_models() {
 // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 Status ModelRegistration::create_status(const std::shared_ptr<Resource>& resource) const {
     Status status;
-    *status.mutable_name() = resource->get_resource_name(resource->name());
+    *status.mutable_name() = resource->get_resource_name().to_proto();
     *status.mutable_status() = google::protobuf::Struct();
     return status;
 }

src/viam/sdk/resource/resource.cpp

@@ -1,7 +1,5 @@
 #include <viam/sdk/resource/resource.hpp>
 
-#include <grpcpp/support/status.h>
-
 #include <viam/sdk/common/proto_value.hpp>
 #include <viam/sdk/common/utils.hpp>
 #include <viam/sdk/registry/registry.hpp>
@@ -10,23 +8,23 @@
 namespace viam {
 namespace sdk {
 
-using common::v1::ResourceName;
-
 Resource::~Resource() = default;
 Resource::Resource(std::string name) : name_(std::move(name)) {}
 
 std::string Resource::name() const {
     return name_;
 }
 
-ResourceName Resource::get_resource_name(std::string name) const {
-    ResourceName r;
-    *r.mutable_namespace_() = kRDK;
-    *r.mutable_type() = kResource;
-    *r.mutable_subtype() = this->api().resource_subtype();
-    *r.mutable_name() = std::move(name);
+Name Resource::get_resource_name(const std::string& type) const {
+    auto api_ = api();
+    api_.set_resource_type(type);
+
+    auto name_parts = long_name_to_remote_and_short(name_);
+    return {api_, name_parts.first, name_parts.second};
+}
 
-    return r;
+Name Resource::get_resource_name() const {
+    return get_resource_name(kResource);
 }
 
 }  // namespace sdk

src/viam/sdk/resource/resource.hpp

@@ -2,9 +2,6 @@
 
 #include <unordered_map>
 
-#include <grpcpp/impl/service_type.h>
-#include <grpcpp/support/status.h>
-
 #include <viam/sdk/common/proto_value.hpp>
 #include <viam/sdk/config/resource.hpp>
 #include <viam/sdk/resource/resource_api.hpp>
@@ -22,14 +19,17 @@ class Resource {
     /// @brief Returns the `API` associated with a particular resource.
     virtual API api() const = 0;
 
-    /// @brief Returns a `ResourceName` for a particular resource name.
-    virtual viam::common::v1::ResourceName get_resource_name(std::string name) const;
+    /// @brief Returns the `Name` for a particular resource.
+    virtual Name get_resource_name() const;
 
     /// @brief Return the resource's name.
     virtual std::string name() const;
 
    private:
     std::string name_;
+
+   protected:
+    Name get_resource_name(const std::string& type) const;
 };
 
 template <>

src/viam/sdk/resource/resource_api.cpp

@@ -32,7 +32,7 @@ std::string APIType::to_string() const {
 }
 
 APIType::APIType(std::string namespace_, std::string resource_type)
-    : namespace_(std::move(namespace_)), resource_type_(std::move(resource_type)){};
+    : namespace_(std::move(namespace_)), resource_type_(std::move(resource_type)) {}
 
 std::string API::to_string() const {
     return APIType::to_string() + ":" + resource_subtype_;
@@ -126,21 +126,10 @@ viam::common::v1::ResourceName Name::to_proto() const {
 }
 
 Name Name::from_proto(const viam::common::v1::ResourceName& proto) {
-    const API api(proto.namespace_(), proto.type(), proto.subtype());
-    std::vector<std::string> name_parts;
-    boost::split(name_parts, proto.name(), boost::is_any_of(":"));
-    auto name = name_parts.back();
-    name_parts.pop_back();
-    auto remote_name = name_parts.empty()
-                           ? ""
-                           : std::accumulate(std::next(name_parts.begin()),
-                                             name_parts.end(),
-                                             *name_parts.begin(),
-                                             [](const std::string& a, const std::string& b) {
-                                                 return a + ":" + b;
-                                             });
-
-    return Name({proto.namespace_(), proto.type(), proto.subtype()}, remote_name, name);
+    auto name_parts = long_name_to_remote_and_short(proto.name());
+
+    return Name(
+        {proto.namespace_(), proto.type(), proto.subtype()}, name_parts.first, name_parts.second);
 };
 
 Name Name::from_string(std::string name) {

src/viam/sdk/robot/service.cpp

@@ -82,7 +82,7 @@ std::vector<Status> RobotService_::generate_status_(
             const std::shared_ptr<const ModelRegistration> registration = kv.second;
             if (registration->api().resource_subtype() == resource->api().resource_subtype()) {
                 bool resource_present = false;
-                const ResourceName name = resource->get_resource_name(resource->name());
+                const ResourceName name = resource->get_resource_name().to_proto();
                 for (const auto& resource_name : resource_names) {
                     if (name.SerializeAsString() == resource_name.SerializeAsString()) {
                         resource_present = true;
@@ -195,7 +195,7 @@ ::grpc::Status RobotService_::StopAll(::grpc::ServerContext*,
 
     for (const auto& r : resource_manager()->resources()) {
         const std::shared_ptr<Resource> resource = r.second;
-        const ResourceName rn = resource->get_resource_name(resource->name());
+        const ResourceName rn = resource->get_resource_name().to_proto();
         const std::string rn_ = rn.SerializeAsString();
         if (extra.find(rn_) != extra.end()) {
             try {

src/viam/sdk/services/service.cpp

@@ -2,21 +2,16 @@
 
 #include <string>
 
-#include <viam/api/common/v1/common.pb.h>
-
 #include <viam/sdk/common/utils.hpp>
-#include <viam/sdk/resource/resource.hpp>
 
 namespace viam {
 namespace sdk {
 
-common::v1::ResourceName Service::get_resource_name(std::string name) const {
-    auto r = this->Resource::get_resource_name(name);
-    *r.mutable_type() = kService;
-    return r;
+Name Service::get_resource_name() const {
+    return Resource::get_resource_name(kService);
 }
 
-Service::Service() : Resource("service"){};
+Service::Service() : Resource("service") {}
 
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/services/service.hpp

@@ -2,20 +2,18 @@
 
 #include <string>
 
-#include <viam/api/common/v1/common.pb.h>
-
 #include <viam/sdk/resource/resource.hpp>
 
 namespace viam {
 namespace sdk {
 
 class Service : public Resource {
    public:
-    viam::common::v1::ResourceName get_resource_name(std::string name) const override;
+    Name get_resource_name() const override;
     Service();
 
    protected:
-    explicit Service(std::string name) : Resource(std::move(name)){};
+    explicit Service(std::string name) : Resource(std::move(name)) {}
 };
 
 }  // namespace sdk

src/viam/sdk/tests/test_common.cpp

@@ -123,6 +123,23 @@ BOOST_AUTO_TEST_CASE(test_version_metadata) {
     BOOST_CHECK_EQUAL(version_constructed, version);
 }
 
+BOOST_AUTO_TEST_CASE(test_name_conversion) {
+    std::string long_name1 = "foo:bar";
+    auto res1 = long_name_to_remote_and_short(long_name1);
+    BOOST_CHECK_EQUAL(res1.first, "foo");
+    BOOST_CHECK_EQUAL(res1.second, "bar");
+
+    std::string long_name2 = "foo:bar:baz";
+    auto res2 = long_name_to_remote_and_short(long_name2);
+    BOOST_CHECK_EQUAL(res2.first, "foo:bar");
+    BOOST_CHECK_EQUAL(res2.second, "baz");
+
+    std::string long_name3 = "foo";
+    auto res3 = long_name_to_remote_and_short(long_name3);
+    BOOST_CHECK_EQUAL(res3.first, "");
+    BOOST_CHECK_EQUAL(res3.second, "foo");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }  // namespace sdktests