From 90090bff37b9af01e7eec1d5336086644e449ae6 Mon Sep 17 00:00:00 2001
From: uyqhv_engel <uyqhv@student.kit.edu>
Date: Thu, 2 May 2024 00:28:48 +0200
Subject: [PATCH] all methods in SkillManagerComponentPlugin have Result return
 tzpe

---
 .../libraries/skills/core/error/Exception.h   | 44 ++++++++++
 .../manager/SkillManagerComponentPlugin.cpp   | 84 ++++++++++---------
 .../manager/SkillManagerComponentPlugin.h     | 22 ++---
 .../SkillManagerComponentPluginUser.cpp       |  7 +-
 4 files changed, 104 insertions(+), 53 deletions(-)

diff --git a/source/RobotAPI/libraries/skills/core/error/Exception.h b/source/RobotAPI/libraries/skills/core/error/Exception.h
index 9e01205b2..df43682ed 100644
--- a/source/RobotAPI/libraries/skills/core/error/Exception.h
+++ b/source/RobotAPI/libraries/skills/core/error/Exception.h
@@ -97,4 +97,48 @@ namespace armarx::skills::error
         {
         }
     };
+
+    class MutexError : public SkillException
+    {
+    public:
+        MutexError() = delete;
+
+            MutexError(const std::string& prettymethod) :
+            SkillException(prettymethod, "This method is not yet implemented!")
+        {
+        }
+    };
+
+   class ProfileNotFoundException: public SkillException
+    {
+    public:
+        ProfileNotFoundException() = delete;
+
+            ProfileNotFoundException (const std::string& prettymethod) :
+            SkillException(prettymethod, "This method is not yet implemented!")
+        {
+        }
+    };
+
+       class ProviderNotFoundException: public SkillException
+    {
+    public:
+        ProviderNotFoundException() = delete;
+
+            ProviderNotFoundException (const std::string& prettymethod) :
+            SkillException(prettymethod, "This method is not yet implemented!")
+        {
+        }
+    };
+
+    class SkillExecutionException: public SkillException
+    {
+    public:
+        SkillExecutionException() = delete;
+
+            SkillExecutionException (const std::string& prettymethod) :
+            SkillException(prettymethod, "This method is not yet implemented!")
+        {
+        }
+    };
 } // namespace armarx::skills::error
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
index fb11ced49..448c9c071 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
@@ -718,14 +718,14 @@ namespace armarx::plugins
         }
     }
 
-    std::optional<std::vector<skills::FluxioSkillStatusUpdate>>
+    skills::Result<std::vector<skills::FluxioSkillStatusUpdate>, skills::error::SkillException>
     SkillManagerComponentPlugin::getFluxioSkillExecutionStatus(const std::string& executionId)
     {
         const auto& executorIt = fluxioDC.fluxioExecutors.find(executionId);
         if (executorIt == fluxioDC.fluxioExecutors.end())
         {
             ARMARX_WARNING << "Execution with id '" << executionId << "' not found.";
-            return std::nullopt;
+            return {skills::error::SkillExecutionException("Execution with id" + executionId + " not found.")};
         }
 
         FluxioExecutor* executorPtr = executorIt->second;
@@ -739,17 +739,17 @@ namespace armarx::plugins
                 {
                     ARMARX_WARNING << "Executor with id '" << executionId
                                    << "' is not a FluxioNativeExecutor.";
-                    return std::nullopt;
+                    return {skills::error::SkillExecutionException("Executor with id" + executionId + "is not a FluxioNativeExecutor")};
                 }
             }
             catch (const std::bad_cast& e)
             {
                 ARMARX_WARNING << "Error while getting execution runner for fluxio skill with id "
                                << executionId << ": " << e.what();
-                return std::nullopt;
+                return {skills::error::SkillExecutionException("Error while getting execution runner for fluxio skill with id " + executionId + ": " + e.what())};
             }
-
-            return castedExecutor->getStatusUpdate();
+           
+            return {(castedExecutor->getStatusUpdate()).value()};
         }
 
         FluxioCompositeExecutor* castedExecutor = nullptr;
@@ -760,18 +760,18 @@ namespace armarx::plugins
             {
                 ARMARX_WARNING << "Executor with id '" << executionId
                                << "' is not a FluxioCompositeExecutor.";
-                return std::nullopt;
+                return {skills::error::SkillExecutionException("Executor not found")};
             }
         }
         catch (const std::bad_cast& e)
         {
             ARMARX_WARNING << "Error while getting execution runner for fluxio skill with id "
                            << executionId << ": " << e.what();
-            return std::nullopt;
+           return {skills::error::SkillExecutionException("Error while getting execution runner for fluxio skill")};
         }
 
 
-        return castedExecutor->getStatusUpdate();
+        return {castedExecutor->getStatusUpdate().value()};
     }
 
     /**
@@ -802,7 +802,7 @@ namespace armarx::plugins
         }
     }
 
-    std::vector<skills::FluxioSkill>
+    skills::Result<std::vector<skills::FluxioSkill>, skills::error::SkillException>
     SkillManagerComponentPlugin::getSkillList()
     {
         std::map<skills::SkillID, skills::SkillDescription> skillDescriptions =
@@ -921,7 +921,7 @@ namespace armarx::plugins
             fluxioDC.skills[s.id] = s;
         }
 
-        return convertMapValuesToVector(fluxioDC.skills);
+        return {convertMapValuesToVector(fluxioDC.skills)};
     }
 
 
@@ -943,7 +943,7 @@ namespace armarx::plugins
         // TODO: does the user need to have the mutex here ?
     }
 
-    bool
+    skills::Result<bool, skills::error::SkillException>
     SkillManagerComponentPlugin::getSkillMutex(const std::string& skillId,
                                                const std::string& userId)
     {
@@ -965,33 +965,33 @@ namespace armarx::plugins
         // TODO: check if mutex is held by user
     }
 
-    std::vector<skills::FluxioProfile>
+    skills::Result<std::vector<skills::FluxioProfile>, skills::error::SkillException>
     SkillManagerComponentPlugin::getProfileList()
     {
-        return convertMapValuesToVector(fluxioDC.profiles);
+        return {convertMapValuesToVector(fluxioDC.profiles)};
     }
 
-    std::optional<skills::FluxioProfile>
+    skills::Result<skills::FluxioProfile, skills::error::ProfileNotFoundException>
     SkillManagerComponentPlugin::getProfile(const std::string& id)
     {
         const auto& profilesEntry = fluxioDC.profiles.find(id);
 
         if (profilesEntry != fluxioDC.profiles.end())
         {
-            return profilesEntry->second;
+            return {profilesEntry->second};
         }
 
-        return std::nullopt;
+        return {skills::error::ProfileNotFoundException("Profile could not be found")}; 
     }
 
-    skills::FluxioProfile
+    skills::Result<skills::FluxioProfile, skills::error::SkillException>
     SkillManagerComponentPlugin::createProfile(const skills::FluxioProfile& profile)
     {
         std::string id = IceUtil::generateUUID(); // TODO: use boost library for uuids
         fluxioDC.profiles[id] = profile; // a copy is created when the profile is added to the map
         fluxioDC.profiles[id].id = id; // this copy can then be modified
 
-        return fluxioDC.profiles[id];
+        return {fluxioDC.profiles[id]};
     }
 
     void
@@ -999,15 +999,17 @@ namespace armarx::plugins
     {
         auto oldProfile = getProfile(profile.id);
 
-        if (oldProfile.has_value() && oldProfile->id == profile.id)
+        if (oldProfile.isSuccess() && oldProfile.getResult().id == profile.id)
         {
             fluxioDC.profiles[profile.id].name = profile.name;
             fluxioDC.profiles[profile.id].description = profile.description;
             fluxioDC.profiles[profile.id].parentPtr = profile.parentPtr;
+        } else {
+         //TODO: Do we throw the error here?
         }
     }
 
-    std::vector<skills::FluxioProvider>
+    skills::Result<std::vector<skills::FluxioProvider>, skills::error::SkillException>
     SkillManagerComponentPlugin::getProviderList()
     {
         for (const auto& [providerName, providerPrx] : skillProviderMap)
@@ -1024,10 +1026,10 @@ namespace armarx::plugins
                                                              providerName.providerName};
         }
 
-        return convertMapValuesToVector(fluxioDC.providers);
+        return {convertMapValuesToVector(fluxioDC.providers)};
     }
 
-    std::optional<skills::FluxioProvider>
+    skills::Result<std::optional<skills::FluxioProvider>, skills::error::SkillException>
     SkillManagerComponentPlugin::getProvider(const std::string& id)
     {
         getProviderList();
@@ -1035,13 +1037,13 @@ namespace armarx::plugins
 
         if (providersEntry != fluxioDC.providers.end())
         {
-            return providersEntry->second;
+            return {providersEntry->second};
         }
 
-        return std::nullopt;
+        return {skills::error::ProviderNotFoundException("Provider could not be found")}; 
     }
 
-    std::optional<std::vector<skills::FluxioSkill>>
+    skills::Result<std::vector<skills::FluxioSkill>, skills::error::SkillException>
     SkillManagerComponentPlugin::getSkillsOfProvider(const std::string& id)
     {
         getProviderList();
@@ -1049,10 +1051,10 @@ namespace armarx::plugins
 
         if (providersEntry == fluxioDC.providers.end())
         {
-            return std::nullopt;
+            return {skills::error::SkillNotFoundException("Skill could not be found","Skill is not in skill list of Provider")};
         }
 
-        std::vector<skills::FluxioSkill> allSkills = getSkillList();
+        std::vector<skills::FluxioSkill> allSkills = getSkillList().getResult();
         std::vector<skills::FluxioSkill> ret;
 
         for (const auto& skill : allSkills)
@@ -1063,10 +1065,11 @@ namespace armarx::plugins
             }
         }
 
-        return ret;
+        return {ret};
     }
 
-    std::optional<skills::FluxioSkill>
+
+    skills::Result<skills::FluxioSkill, skills::error::SkillException>
     SkillManagerComponentPlugin::addSkillToProvider(const std::string& userId,
                                                     const std::string& providerId,
                                                     skills::FluxioSkill& skill)
@@ -1112,7 +1115,7 @@ namespace armarx::plugins
         const auto& providerIt = fluxioDC.providers.find(providerId);
         if (providerIt == fluxioDC.providers.end())
         {
-            return std::nullopt;
+            return {skills::error::ProviderNotFoundException("Provider could not be found")}; 
         }
 
         // set id, lastChanged and store in data collector
@@ -1124,10 +1127,10 @@ namespace armarx::plugins
         // set mutex
         setEditFluxioSkillMutex(true, userId, id);
 
-        return fluxioDC.skills[id];
+        return {fluxioDC.skills[id]};
     }
 
-    bool // TODO: add armarx info messages
+    skills::Result<bool,skills::error::SkillException>
     SkillManagerComponentPlugin::setEditFluxioSkillMutex(bool aquireMutex,
                                                          const std::string& userId,
                                                          const std::string& skillId)
@@ -1141,7 +1144,7 @@ namespace armarx::plugins
             {
                 // skill is free, set mutex
                 fluxioDC.skillMutexMap[skillId] = std::make_tuple(userId, armarx::DateTime::Now());
-                return true;
+                return {true};
             }
 
             // skill is already locked
@@ -1149,7 +1152,7 @@ namespace armarx::plugins
             {
                 // user already holds the mutex -> reset timestamp / prolong mutex
                 std::get<1>(fluxioDC.skillMutexMap[skillId]) = armarx::DateTime::Now();
-                return true;
+                return {true};
             }
 
             // another user holds the mutex -> is it still valid ?
@@ -1159,28 +1162,29 @@ namespace armarx::plugins
             {
                 // mutex invalid, requesting user gets the mutex instead
                 fluxioDC.skillMutexMap[skillId] = std::make_tuple(userId, armarx::DateTime::Now());
-                return true;
+                return {true};
             }
 
             // mutex could not be aquired
-            return false;
+            return {skills::error::MutexError("Mutex could not be acquired")};
         }
 
         // remove mutex
         if (skillMutexIt == fluxioDC.skillMutexMap.end())
         {
             // skill is already free
-            return true;
+            return {true};
         }
 
+
         // check if the user holds the mutex, as only the user that holds the mutex can release it
         if (std::get<0>(fluxioDC.skillMutexMap[skillId]) == userId)
         {
             fluxioDC.skillMutexMap.erase(skillId);
-            return true;
+            return {true}; 
         }
 
         // mutex could not be removed
-        return false;
+        return {skills::error::MutexError("Mutex could not be acquired")};
     }
 } // namespace armarx::plugins
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
index 760395ae9..81f6dd9c4 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
@@ -81,16 +81,16 @@ namespace armarx::plugins
 
         void abortFluxioSkill(const std::string& executionId);
 
-        std::optional<std::vector<skills::FluxioSkillStatusUpdate>>
+        skills::Result<std::vector<skills::FluxioSkillStatusUpdate>, skills::error::SkillException>
         getFluxioSkillExecutionStatus(const std::string& executionId);
 
-        std::vector<skills::FluxioSkill> getSkillList();
+        skills::Result<std::vector<skills::FluxioSkill>, skills::error::SkillException> getSkillList();
 
         skills::Result<skills::FluxioSkill, skills::error::SkillException>getSkill(const std::string& id);
 
         void removeSkill(const std::string& id);
 
-        bool getSkillMutex(const std::string& skillId, const std::string& userId);
+        skills::Result<bool, skills::error::SkillException> getSkillMutex(const std::string& skillId, const std::string& userId);
 
         void deleteSkillMutex(const std::string& skillId, const std::string& userId);
 
@@ -98,21 +98,21 @@ namespace armarx::plugins
                                   const std::string& skillId,
                                   const std::string& parameterId);
 
-        std::vector<skills::FluxioProfile> getProfileList();
+        skills::Result<std::vector<skills::FluxioProfile>, skills::error::SkillException> getProfileList();
 
-        std::optional<skills::FluxioProfile> getProfile(const std::string& id);
+        skills::Result<skills::FluxioProfile, skills::error::ProfileNotFoundException> getProfile(const std::string& id);
 
-        skills::FluxioProfile createProfile(const skills::FluxioProfile& profile);
+        skills::Result<skills::FluxioProfile, skills::error::SkillException> createProfile(const skills::FluxioProfile& profile);
 
         void updateProfile(const skills::FluxioProfile& profile);
 
-        std::vector<skills::FluxioProvider> getProviderList();
+        skills::Result<std::vector<skills::FluxioProvider>, skills::error::SkillException> getProviderList();
 
-        std::optional<skills::FluxioProvider> getProvider(const std::string& id);
+        skills::Result<std::optional<skills::FluxioProvider>, skills::error::SkillException> getProvider(const std::string& id);
 
-        std::optional<std::vector<skills::FluxioSkill>> getSkillsOfProvider(const std::string& id);
+        skills::Result<std::vector<skills::FluxioSkill>, skills::error::SkillException> getSkillsOfProvider(const std::string& id);
 
-        std::optional<skills::FluxioSkill> addSkillToProvider(const std::string& userId,
+        skills::Result<skills::FluxioSkill, skills::error::SkillException> addSkillToProvider(const std::string& userId,
                                                               const std::string& providerId,
                                                               skills::FluxioSkill& skill);
 
@@ -147,7 +147,7 @@ namespace armarx::plugins
         void replaceVectorContent(std::vector<T>& originalVector,
                                   const std::vector<T>& replacementVector);
 
-        bool setEditFluxioSkillMutex(bool aquireMutex,
+        skills::Result<bool, skills::error::SkillException> setEditFluxioSkillMutex(bool aquireMutex,
                                      const std::string& userId,
                                      const std::string& skillId);
 
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
index 94a93cffd..5b674d154 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
@@ -6,6 +6,8 @@
 
 #include "RobotAPI/libraries/skills/core/FluxioProfile.h"
 #include "RobotAPI/libraries/skills/core/FluxioProvider.h"
+#include "RobotAPI/libraries/skills/core/error/Exception.h"
+#include "RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h"
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
@@ -257,10 +259,11 @@ namespace armarx
         }
 
         // Check if the user has the mutex for the skill
-        if (!this->plugin->getSkillMutex(skill.id, userId))
+        auto res = this->plugin->getSkillMutex(skill.id, userId); 
+        if (!res.isSuccess())
         {
             ARMARX_WARNING << "SkillManagerComponentPluginUser::updateSkill: User " << userId
-                           << " does not have the mutex for skill with id " << skill.id;
+                           << res.getError().getReason().append("User does not have Mutex for this Skill") << skill.id;
             return false;
         }
 
-- 
GitLab