fix: change requestId type for method invocation

the unsigned type suites better the request id type, id should not be
negative.
Fixes the overflow issue, as unsigned int behavior is defined.

Author: dorotaphanSiili

src/olink/clientnode.cpp

@@ -9,7 +9,6 @@ namespace ApiGear { namespace ObjectLink {
 ClientNode::ClientNode(ClientRegistry& registry)
     : BaseNode()
     , m_registry(registry)
-    , m_nextRequestId(0)
 {
 }
 
@@ -50,7 +49,7 @@ void ClientNode::invokeRemote(const std::string& methodId, const nlohmann::json&
     static const std::string invokeRemoteLog = "ClientNode.invokeRemote: ";
     emitLog(LogLevel::Info, invokeRemoteLog, methodId);
     std::unique_lock<std::mutex> lock(m_pendingInvokesMutex);
-    int requestId = nextRequestId();
+    auto requestId = nextRequestId();
     m_invokesPending[requestId] = func;
     lock.unlock();
     nlohmann::json msg = Protocol::invokeMessage(requestId, methodId, args);
@@ -113,7 +112,7 @@ void ClientNode::handlePropertyChange(const std::string& propertyId, const nlohm
     }
 }
 
-void ClientNode::handleInvokeReply(int requestId, const std::string& methodId, const nlohmann::json& value)
+void ClientNode::handleInvokeReply(unsigned int requestId, const std::string& methodId, const nlohmann::json& value)
 {
     static const std::string handleInvokeLog = "ClientNode.handleInvokeReply: ";
     emitLogWithPayload(LogLevel::Info, value, handleInvokeLog, methodId);
@@ -154,12 +153,9 @@ void ClientNode::handleError(int msgType, int requestId, const std::string& erro
     emitLog(LogLevel::Info, errorLog, std::to_string(msgType), std::to_string(requestId), error);
 }
 
-int ClientNode::nextRequestId()
+unsigned int ClientNode::nextRequestId()
 {
     m_nextRequestId++;
-    if (m_nextRequestId < 0){
-        m_nextRequestId = 0;
-    }
     return m_nextRequestId;
 }
 

src/olink/clientnode.h

@@ -74,7 +74,7 @@ class OLINK_EXPORT ClientNode : public BaseNode, public IClientNode, public std:
     /** IProtocolListener::handlePropertyChange implementation */
     void handlePropertyChange(const std::string& propertyId, const nlohmann::json& value) override;
     /** IProtocolListener::handleInvokeReply implementation */
-    void handleInvokeReply(int requestId, const std::string& methodId, const nlohmann::json& value) override;
+    void handleInvokeReply(unsigned int requestId, const std::string& methodId, const nlohmann::json& value) override;
     /** IProtocolListener::handleSignal implementation */
     void handleSignal(const std::string& signalId, const nlohmann::json& args) override;
     /** IProtocolListener::handleError implementation */
@@ -84,17 +84,17 @@ class OLINK_EXPORT ClientNode : public BaseNode, public IClientNode, public std:
      * Returns a request id for outgoing messages.
      * @return a unique, non negative id.
      */
-    int nextRequestId();
+    unsigned int nextRequestId();
 private:
     /* The registry in which client is registered and which provides sinks connected with this node*/
     ClientRegistry& m_registry;
     /* Id of this node in registry.*/
     unsigned long m_nodeId;
 
     /* Value of last request id.*/
-    std::atomic<int> m_nextRequestId;
+    std::atomic<unsigned int> m_nextRequestId = {0};
     /** Collection of callbacks for method replies that client is waiting for associated with the id for invocation request message.*/
-    std::map<int,InvokeReplyFunc> m_invokesPending;
+    std::map<unsigned int,InvokeReplyFunc> m_invokesPending;
     std::mutex m_pendingInvokesMutex;
 };
 

src/olink/core/basenode.cpp

@@ -44,7 +44,7 @@ void BaseNode::handleUnlink(const std::string& objectId)
     emitLog(LogLevel::Warning, notImplementedLog, std::string(__func__), objectId);
 }
 
-void BaseNode::handleInvoke(int, const std::string& methodId, const nlohmann::json& args)
+void BaseNode::handleInvoke(unsigned int, const std::string& methodId, const nlohmann::json& args)
 {
     emitLogWithPayload(LogLevel::Warning, args, notImplementedLog, std::string(__func__), methodId, " args ");
 }
@@ -59,7 +59,7 @@ void BaseNode::handleInit(const std::string& objectId, const nlohmann::json& pro
     emitLogWithPayload(LogLevel::Warning, props, notImplementedLog, std::string(__func__), objectId, " props ");
 }
 
-void BaseNode::handleInvokeReply(int requestId, const std::string& methodId, const nlohmann::json& value)
+void BaseNode::handleInvokeReply(unsigned int requestId, const std::string& methodId, const nlohmann::json& value)
 {
     emitLog(LogLevel::Warning, notImplementedLog, std::string(__func__), methodId, " requestId ", std::to_string(requestId), " value ", value);
 }

src/olink/core/basenode.h

@@ -42,13 +42,13 @@ class OLINK_EXPORT BaseNode: public LoggerBase,
     // Empty, logging only implementation of IProtocolListener::handleUnlink, should be overwritten on server side.
     void handleUnlink(const std::string& objectId) override;
     // Empty, logging only implementation of IProtocolListener::handleInvoke, should be overwritten on server side.
-    void handleInvoke(int requestId, const std::string& methodId, const nlohmann::json& args) override;
+    void handleInvoke(unsigned int requestId, const std::string& methodId, const nlohmann::json& args) override;
     // Empty, logging only implementation of IProtocolListener::handleSetProperty, should be overwritten on server side.
     void handleSetProperty(const std::string& propertyId, const nlohmann::json& value) override;
     // Empty, logging only implementation of IProtocolListener::handleInit, should be overwritten on client side.
     void handleInit(const std::string& objectId, const nlohmann::json& props) override;
     // Empty, logging only implementation of IProtocolListener::handleInvokeReply, should be overwritten on client side.
-    void handleInvokeReply(int requestId, const std::string& methodId, const nlohmann::json& value) override;
+    void handleInvokeReply(unsigned int requestId, const std::string& methodId, const nlohmann::json& value) override;
     // Empty, logging only implementation of IProtocolListener::handleSignal, should be overwritten on client side.
     void handleSignal(const std::string& signalId, const nlohmann::json& args) override;
     // Empty, logging only implementation of IProtocolListener::handlePropertyChange, should be overwritten on client side.

src/olink/core/protocol.cpp

@@ -65,14 +65,14 @@ nlohmann::json Protocol::propertyChangeMessage(const std::string& propertyId, co
                 );
 }
 
-nlohmann::json Protocol::invokeMessage(int requestId, const std::string& methodId, const nlohmann::json& args)
+nlohmann::json Protocol::invokeMessage(unsigned int requestId, const std::string& methodId, const nlohmann::json& args)
 {
     return nlohmann::json::array(
                 { MsgType::Invoke, requestId, methodId, args }
                 );
 }
 
-nlohmann::json Protocol::invokeReplyMessage(int requestId, const std::string& methodId, const nlohmann::json& value)
+nlohmann::json Protocol::invokeReplyMessage(unsigned int requestId, const std::string& methodId, const nlohmann::json& value)
 {
     return nlohmann::json::array(
                 { MsgType::InvokeReply, requestId, methodId, value }
@@ -131,14 +131,14 @@ bool Protocol::handleMessage(const nlohmann::json& msg, IProtocolListener& liste
         break;
     }
     case int(MsgType::Invoke): {
-        const auto& id = msg[1].get<int>();
+        const auto& id = msg[1].get<unsigned int>();
         const auto& methodId = msg[2].get<std::string>();
         const auto& args = msg[3].get<nlohmann::json>();
         listener.handleInvoke(id, methodId, args);
         break;
     }
     case int(MsgType::InvokeReply): {
-        const auto& id = msg[1].get<int>();
+        const auto& id = msg[1].get<unsigned int>();
         const auto& methodId = msg[2].get<std::string>();
         const auto& value = msg[3].get<nlohmann::json>();
         listener.handleInvokeReply(id, methodId, value);

src/olink/core/protocol.h

@@ -79,15 +79,15 @@ class OLINK_EXPORT IProtocolListener
      * @param methodId Unambiguously describes method in object for which invoke message was received.
      * @param args Arguments with which method should be invoked.
      */
-    virtual void handleInvoke(int requestId, const std::string& methodId, const nlohmann::json& args) = 0;
+    virtual void handleInvoke(unsigned int requestId, const std::string& methodId, const nlohmann::json& args) = 0;
     /**
      * Client side handler, handles invokeReply message.
      * @param requestId Identifier of a request with which the client requested method invocation.
      *    should be used to deliver the result to a caller.
      * @param methodId Unambiguously describes method in object for which invokeReply message was received.
      * @param value Method's result value.
      */
-    virtual void handleInvokeReply(int requestId, const std::string& methodId, const nlohmann::json& value) = 0;
+    virtual void handleInvokeReply(unsigned int requestId, const std::string& methodId, const nlohmann::json& value) = 0;
     /**
      * Client side handler, handles signal message.
      * @param signalId Unambiguously describes signal in object for which signal message was received.
@@ -165,7 +165,7 @@ class OLINK_EXPORT Protocol : public LoggerBase
     * @param args Arguments with which method should be invoked.
     * @return Composed invokeMessage in json format.
     */
-    static nlohmann::json invokeMessage(int requestId, const std::string& methodId, const nlohmann::json& args);
+    static nlohmann::json invokeMessage(unsigned int requestId, const std::string& methodId, const nlohmann::json& args);
     /**
     * Method message.
     * Composes a response to a method invocation message for a methodId.
@@ -175,7 +175,7 @@ class OLINK_EXPORT Protocol : public LoggerBase
     * @param value Value that is an outcome of method invocation.
     * @return Composed invokeReplyMessage in json format.
     */
-    static nlohmann::json invokeReplyMessage(int requestId, const std::string& methodId, const nlohmann::json& value);
+    static nlohmann::json invokeReplyMessage(unsigned int requestId, const std::string& methodId, const nlohmann::json& value);
     /**
     * Signal message.
     * Composes a notification message for signal emitted for signalId.

src/olink/remotenode.cpp

@@ -85,7 +85,7 @@ void RemoteNode::handleSetProperty(const std::string& propertyId, const nlohmann
     }
 }
 
-void RemoteNode::handleInvoke(int requestId, const std::string& methodId, const nlohmann::json& args)
+void RemoteNode::handleInvoke(unsigned int requestId, const std::string& methodId, const nlohmann::json& args)
 {
     auto objectId = ApiGear::ObjectLink::Name::getObjectId(methodId);
     auto source = m_registry.getSource(objectId).lock();

src/olink/remotenode.h

@@ -81,7 +81,7 @@ class OLINK_EXPORT RemoteNode: public BaseNode, public IRemoteNode, public std::
     /** IProtocolListener::handleSetProperty implementation. */
     void handleSetProperty(const std::string& propertyId, const nlohmann::json& value) override;
     /** IProtocolListener::handleInvoke implementation. */
-    void handleInvoke(int requestId, const std::string& methodId, const nlohmann::json& args) override;
+    void handleInvoke(unsigned int requestId, const std::string& methodId, const nlohmann::json& args) override;
 
     /** IRemoteNode::notifyPropertyChange implementation. */
     void notifyPropertyChange(const std::string& propertyId, const nlohmann::json& value) override;

tests/test_protocol.cpp

@@ -16,7 +16,7 @@ TEST_CASE("protocol")
     json props = {{ "count", 0 }};
     int value = 1;
     json args = {1, 2};
-    int requestId = 1;
+    unsigned int requestId = 1;
     MsgType msgType = MsgType::Invoke;
     std::string error = "failed";