changed RpcClient signature to use method UUri only when calling

invokeMethod

Author: MaximilianToe

include/up-cpp/communication/RpcClient.h

@@ -48,8 +48,7 @@ struct RpcClient {
 	/// For guidance on the permeission_level and token parameters, see:
 	/// https://github.com/eclipse-uprotocol/up-spec/blob/main/basics/permissions.adoc
 	explicit RpcClient(std::shared_ptr<transport::UTransport> transport,
-	                   v1::UUri&& method, v1::UPriority priority,
-	                   std::chrono::milliseconds ttl,
+	                   v1::UPriority priority, std::chrono::milliseconds ttl,
 	                   std::optional<v1::UPayloadFormat> payload_format = {},
 	                   std::optional<uint32_t> permission_level = {},
 	                   std::optional<std::string> token = {});
@@ -100,6 +99,7 @@ struct RpcClient {
 
 	/// @brief Invokes an RPC method by sending a request message.
 	///
+	/// @param The method that will be invoked
 	/// @param A Payload builder containing the payload to be sent with the
 	///        request.
 	/// @param A callback that will be called with the result.
@@ -111,11 +111,13 @@ struct RpcClient {
 	///       * A UStatus based on the commstatus received in the response
 	///         message (if not OK).
 	///       * A UMessage containing the response from the RPC target.
-	[[nodiscard]] InvokeHandle invokeMethod(datamodel::builder::Payload&&,
+	[[nodiscard]] InvokeHandle invokeMethod(const v1::UUri&,
+	                                        datamodel::builder::Payload&&,
 	                                        Callback&&);
 
 	/// @brief Invokes an RPC method by sending a request message.
 	///
+	/// @param The method that will be invoked
 	/// @param A Payload builder containing the payload to be sent with the
 	///        request.
 	///
@@ -128,13 +130,15 @@ struct RpcClient {
 	///          * A UStatus based on the commstatus received in the response
 	///            message (if not OK).
 	///          * A UMessage containing the response from the RPC target.
-	[[nodiscard]] InvokeFuture invokeMethod(datamodel::builder::Payload&&);
+	[[nodiscard]] InvokeFuture invokeMethod(const v1::UUri&,
+	                                        datamodel::builder::Payload&&);
 
 	/// @brief Invokes an RPC method by sending a request message.
 	///
 	/// Request is sent with an empty payload. Can only be called if no payload
 	/// format was provided at construction time.
 	///
+	/// @param The method that will be invoked
 	/// @param A callback that will be called with the result.
 	///
 	/// @post The provided callback will be called with one of:
@@ -144,13 +148,14 @@ struct RpcClient {
 	///       * A UStatus based on the commstatus received in the response
 	///         message (if not OK).
 	///       * A UMessage containing the response from the RPC target.
-	[[nodiscard]] InvokeHandle invokeMethod(Callback&&);
+	[[nodiscard]] InvokeHandle invokeMethod(const v1::UUri&, Callback&&);
 
 	/// @brief Invokes an RPC method by sending a request message.
 	///
 	/// Request is sent with an empty payload. Can only be called if no payload
 	/// format was provided at construction time.
 	///
+	/// @param The method that will be invoked
 	/// @remarks This is a wrapper around the callback form of invokeMethod.
 	///
 	/// @returns A promised future that can resolve to one of:
@@ -160,7 +165,7 @@ struct RpcClient {
 	///          * A UStatus based on the commstatus received in the response
 	///            message (if not OK).
 	///          * A UMessage containing the response from the RPC target.
-	[[nodiscard]] InvokeFuture invokeMethod();
+	[[nodiscard]] InvokeFuture invokeMethod(const v1::UUri&);
 
 	/// @brief Default move constructor (defined in RpcClient.cpp)
 	RpcClient(RpcClient&&) noexcept;

include/up-cpp/datamodel/builder/UMessage.h

@@ -23,8 +23,13 @@
 #include <string>
 #include <vector>
 
+#include "up-cpp/communication/NotificationSink.h"
 #include "up-cpp/datamodel/builder/Uuid.h"
 
+namespace uprotocol::communication {
+struct RpcClient;
+}
+
 namespace uprotocol::datamodel::builder {
 
 /// @brief Interface for composing UMessage objects
@@ -38,6 +43,7 @@ namespace uprotocol::datamodel::builder {
 ///          instance can be held and reused by calling .build(payload)
 ///          for each new set of message data.
 struct UMessageBuilder {
+	friend struct communication::RpcClient;
 	/// @brief Pre-populates a message builder with the attributes of a
 	///        "publish" type message.
 	///
@@ -103,6 +109,17 @@ struct UMessageBuilder {
 	/// @returns UMessageBuilder configured to build a "response" message
 	static UMessageBuilder response(const v1::UMessage& request);
 
+	/// @brief Set the method attribute for built messages.
+	///
+	/// @param The method to use when building messages.
+	///
+	/// @throws std::out_of_range when setting a priority lower than CS4
+	///                           for "request" or "response" messages.
+	/// @throws std::out_of_range if the value is outside of the range of
+	///                           v1::UPriority
+	/// @returns A reference to this UMessageBuilder
+	UMessageBuilder& withMethod(const v1::UUri&);
+
 	/// @brief Set the message priority attribute for built messages.
 	///
 	/// If not called, the default value as specified in

src/client/usubscription/v3/Consumer.cpp

@@ -91,8 +91,7 @@ v1::UStatus Consumer::subscribe(
     v1::UPriority priority, std::chrono::milliseconds subscription_request_ttl,
     ListenCallback&& callback) {
 	rpc_client_ = std::make_unique<communication::RpcClient>(
-	    transport_, uSubscriptionUUriBuilder_.getServiceUriWithResourceId(1),
-	    priority, subscription_request_ttl);
+	    transport_, priority, subscription_request_ttl);
 
 	auto on_response = [this](const auto& maybe_response) {
 		if (maybe_response.has_value() &&
@@ -110,8 +109,9 @@ v1::UStatus Consumer::subscribe(
 	SubscriptionRequest const subscription_request = buildSubscriptionRequest();
 	auto payload = datamodel::builder::Payload(subscription_request);
 
-	rpc_handle_ =
-	    rpc_client_->invokeMethod(std::move(payload), std::move(on_response));
+	rpc_handle_ = rpc_client_->invokeMethod(
+	    uSubscriptionUUriBuilder_.getServiceUriWithResourceId(1),
+	    std::move(payload), std::move(on_response));
 
 	// Create a L2 subscription
 	auto result = communication::Subscriber::subscribe(
@@ -135,8 +135,7 @@ UnsubscribeRequest Consumer::buildUnsubscriptionRequest() {
 void Consumer::unsubscribe(v1::UPriority priority,
                            std::chrono::milliseconds request_ttl) {
 	rpc_client_ = std::make_unique<communication::RpcClient>(
-	    transport_, uSubscriptionUUriBuilder_.getServiceUriWithResourceId(2),
-	    priority, request_ttl);
+	    transport_, priority, request_ttl);
 
 	auto on_response = [](const auto& maybe_response) {
 		if (!maybe_response.has_value()) {
@@ -147,8 +146,9 @@ void Consumer::unsubscribe(v1::UPriority priority,
 	UnsubscribeRequest const unsubscribe_request = buildUnsubscriptionRequest();
 	auto payload = datamodel::builder::Payload(unsubscribe_request);
 
-	rpc_handle_ =
-	    rpc_client_->invokeMethod(std::move(payload), std::move(on_response));
+	rpc_handle_ = rpc_client_->invokeMethod(
+	    uSubscriptionUUriBuilder_.getServiceUriWithResourceId(2),
+	    std::move(payload), std::move(on_response));
 
 	subscriber_.reset();
 }

src/communication/RpcClient.cpp

@@ -99,17 +99,19 @@ struct RpcClient::ExpireService {
 
 ////////////////////////////////////////////////////////////////////////////////
 RpcClient::RpcClient(std::shared_ptr<transport::UTransport> transport,
-                     v1::UUri&& method, v1::UPriority priority,
-                     std::chrono::milliseconds ttl,
+                     v1::UPriority priority, std::chrono::milliseconds ttl,
                      std::optional<v1::UPayloadFormat> payload_format,
                      std::optional<uint32_t> permission_level,
                      std::optional<std::string> token)
     : transport_(std::move(transport)),
       ttl_(ttl),
-      builder_(datamodel::builder::UMessageBuilder::request(
-          std::move(method), v1::UUri(transport_->getEntityUri()), priority,
-          ttl_)),
+      builder_(datamodel::builder::UMessageBuilder(
+          v1::UMESSAGE_TYPE_REQUEST, v1::UUri(transport_->getEntityUri()),
+          v1::UUri{})),
       expire_service_(std::make_unique<ExpireService>()) {
+	builder_.withPriority(priority);
+	builder_.withTtl(ttl);
+
 	if (payload_format) {
 		builder_.withPayloadFormat(*payload_format);
 	}
@@ -204,41 +206,44 @@ RpcClient::InvokeHandle RpcClient::invokeMethod(v1::UMessage&& request,
 }
 
 RpcClient::InvokeHandle RpcClient::invokeMethod(
-    datamodel::builder::Payload&& payload, Callback&& callback) {
-	return invokeMethod(builder_.build(std::move(payload)),
+    const v1::UUri& method, datamodel::builder::Payload&& payload,
+    Callback&& callback) {
+	return invokeMethod(builder_.withMethod(method).build(std::move(payload)),
 	                    std::move(callback));
 }
 
-RpcClient::InvokeHandle RpcClient::invokeMethod(Callback&& callback) {
-	return invokeMethod(builder_.build(), std::move(callback));
+RpcClient::InvokeHandle RpcClient::invokeMethod(const v1::UUri& method,
+                                                Callback&& callback) {
+	return invokeMethod(builder_.withMethod(method).build(),
+	                    std::move(callback));
 }
 
 RpcClient::InvokeFuture RpcClient::invokeMethod(
-    datamodel::builder::Payload&& payload) {
+    const v1::UUri& method, datamodel::builder::Payload&& payload) {
 	// Note: functors need to be copy constructable. We work around this by
 	// wrapping the promise in a shared_ptr. Unique access to it will be
 	// assured by the implementation at the core of invokeMethod - it only
 	// allows exactly one call to the callback via std::call_once.
 	auto promise = std::make_shared<std::promise<MessageOrStatus>>();
 	auto future = promise->get_future();
 	auto handle =
-	    invokeMethod(std::move(payload),
+	    invokeMethod(method, std::move(payload),
 	                 [promise](const MessageOrStatus& maybe_message) mutable {
 		                 promise->set_value(maybe_message);
 	                 });
 
 	return {std::move(future), std::move(handle)};
 }
 
-RpcClient::InvokeFuture RpcClient::invokeMethod() {
+RpcClient::InvokeFuture RpcClient::invokeMethod(const v1::UUri& method) {
 	// Note: functors need to be copy constructable. We work around this by
 	// wrapping the promise in a shared_ptr. Unique access to it will be
 	// assured by the implementation at the core of invokeMethod - it only
 	// allows exactly one call to the callback via std::call_once.
 	auto promise = std::make_shared<std::promise<MessageOrStatus>>();
 	auto future = promise->get_future();
-	auto handle =
-	    invokeMethod([promise](const MessageOrStatus& maybe_message) mutable {
+	auto handle = invokeMethod(
+	    method, [promise](const MessageOrStatus& maybe_message) mutable {
 		    promise->set_value(maybe_message);
 	    });
 

src/datamodel/builder/UMessage.cpp

@@ -137,6 +137,18 @@ UMessageBuilder& UMessageBuilder::withPriority(v1::UPriority priority) {
 	return *this;
 }
 
+UMessageBuilder& UMessageBuilder::withMethod(const v1::UUri& method) {
+	auto [isValid, reason] = validator::uri::isValidRpcMethod(method);
+	if (!isValid) {
+		throw std::invalid_argument(
+		    "The UUri provided is not a valid RpcMethod");
+	}
+
+	*attributes_.mutable_sink() = method;
+
+	return *this;
+}
+
 UMessageBuilder& UMessageBuilder::withTtl(std::chrono::milliseconds ttl) {
 	if ((ttl.count() <= 0) ||
 	    (ttl.count() > std::numeric_limits<uint32_t>::max())) {