Fix CategoryPlugin, MetadataPlugin

Refactor codes for readability.

Change-Id: I4363feb0ead8c35f3aab418a7fd913c6ddd56eb9
Signed-off-by: Sangyoon Jang <jeremy.jang@samsung.com>

3 files changed, 115 insertions, 92 deletions

diff --git a/src/common/plugins/category_plugin.cc b/src/common/plugins/category_plugin.cc
index 01b82771..f0596878 100644
--- a/src/common/plugins/category_plugin.cc
+++ b/src/common/plugins/category_plugin.cc
@@ -25,6 +25,35 @@ void ClearCategoryDetail(gpointer data) {
   free(category);
 }
 
+GList* GetCategoryListForKey(GList* list, const std::string& key) {
+  // pack all categories starting with key to list that will
+  // be sent to the plugin.
+  // e.g. all http://tizen.org/category/antivirus/*
+  //   will be packed for http://tizen.org/category/antivirus
+  GList* category_list = nullptr;
+  for (const char* category : GListRange<char*>(list)) {
+    if (std::string(category).find(key) != 0)
+      continue;
+
+    __category_t* c = reinterpret_cast<__category_t*>(
+        calloc(1, sizeof(__category_t)));
+    if (!c) {
+      LOG(ERROR) << "Out of memory";
+      g_list_free_full(category_list, &ClearCategoryDetail);
+      return nullptr;
+    }
+    c->name = strdup(category);
+    if (!c->name) {
+      LOG(ERROR) << "Out of memory";
+      free(c);
+      g_list_free_full(category_list, &ClearCategoryDetail);
+      return nullptr;
+    }
+    category_list = g_list_append(category_list, c);
+  }
+  return category_list;
+}
+
 }  // namespace
 
 namespace common_installer {
@@ -74,6 +103,35 @@ std::string CategoryPlugin::GetFunctionName(ActionType action) const {
   return pos->second;
 }
 
+bool CategoryPlugin::ExecutePlugin(const std::string& name,
+    const char* pkgid, const char* appid, GList* category_list) {
+  int result = 0;
+  Exec(name, &result, pkgid, appid, category_list);
+  if (result) {
+    LOG(ERROR) << "Function: " << name << " of plugin "
+        << plugin_info_.path() << " failed";
+    return false;
+  }
+  return true;
+}
+
+bool CategoryPlugin::LoadPluginInfo(manifest_x* manifest) {
+  if (pkgmgrinfo_plugininfo_foreach_plugininfo(manifest->package,
+      CategoryPlugin::kType,
+      plugin_info_.name().c_str(),
+      [](const char*, const char* appid, const char*,
+          const char*, void* user_data) -> int {
+        auto* set = static_cast<std::set<std::string>*>(user_data);
+        set->emplace(std::string(appid));
+        return PMINFO_R_OK;
+      },
+      &appid_set_) != PMINFO_R_OK) {
+    LOG(ERROR) << "Failed to get previous execution info";
+    return false;
+  }
+  return true;
+}
+
 bool CategoryPlugin::Run(xmlDocPtr /*doc_ptr*/, manifest_x* manifest,
          ActionType action_type) {
   std::string name;
@@ -81,93 +139,46 @@ bool CategoryPlugin::Run(xmlDocPtr /*doc_ptr*/, manifest_x* manifest,
   if (tag.empty())
     return false;
 
-  std::set<std::string> appid_list;
   if (action_type == ActionType::Upgrade) {
-    if (pkgmgrinfo_plugininfo_foreach_plugininfo(manifest->package,
-        CategoryPlugin::kType,
-        plugin_info_.name().c_str(),
-        [](const char*, const char* appid, const char*,
-            const char*, void* user_data) -> int {
-          auto* list = static_cast<std::set<std::string>*>(user_data);
-          list->emplace(std::string(appid));
-          return PMINFO_R_OK;
-        },
-        &appid_list) != PMINFO_R_OK) {
-      LOG(ERROR) << "Failed to get previous execution info";
+    // Load previous execution info. If some element left in appid_set_
+    // at the end of this method, it means that the plugin was not executed for
+    // this app. (need to invoke PKGMGR_MDPARSER_PLUGIN_REMOVED)
+    if (!LoadPluginInfo(manifest))
       return false;
-    }
   }
 
+  bool result;
   for (application_x* app : GListRange<application_x*>(manifest->application)) {
     // pack all categories starting with key to list that will
     // be sent to the plugin.
     // e.g. all http://tizen.org/category/antivirus/*
     //   will be packed for http://tizen.org/category/antivirus
-    GList* category_list = nullptr;
-    for (const char* category : GListRange<char*>(app->category)) {
-      const std::string& sub_key_prefix = plugin_info_.name();
-      if (std::string(category).find(sub_key_prefix) == 0) {
-        __category_t* c = reinterpret_cast<__category_t*>(
-            calloc(1, sizeof(__category_t)));
-        if (!c) {
-          LOG(ERROR) << "Out of memory";
-          g_list_free_full(category_list, &ClearCategoryDetail);
-          return false;
-        }
-        c->name = strdup(category);
-        if (!c->name) {
-          LOG(ERROR) << "Out of memory";
-          free(c);
-          g_list_free_full(category_list, &ClearCategoryDetail);
-          return false;
-        }
-        category_list = g_list_append(category_list, c);
-      }
-    }
-
+    GList* category_list = GetCategoryListForKey(app->category,
+        plugin_info_.name());
     // skip application if it has no given category
     if (!category_list) {
-      if (action_type != ActionType::Upgrade)
-        continue;
-      auto iter = appid_list.find(app->appid);
-      if (iter != appid_list.end()) {
-        name = GetFunctionName(ActionType::Removed);
-        appid_list.erase(iter);
-        category_list = nullptr;
-      } else {
-        continue;
-      }
+      continue;
     } else {
       name = GetFunctionName(action_type);
       if (!AddPluginInfo(manifest, app->appid)) {
         g_list_free_full(category_list, &ClearCategoryDetail);
         return false;
       }
-      appid_list.erase(app->appid);
-    }
-    int result = 0;
-    Exec(name, &result, manifest->package, app->appid, category_list);
-    if (result) {
-      LOG(ERROR) << "Function: " << name << " of plugin "
-                 << plugin_info_.path() << " failed";
-      g_list_free_full(category_list, &ClearCategoryDetail);
-      return false;
+      appid_set_.erase(app->appid);
     }
+
+    result = ExecutePlugin(name, manifest->package, app->appid,
+        category_list);
     g_list_free_full(category_list, &ClearCategoryDetail);
+    if (!result)
+      return false;
   }
 
-  if (action_type != ActionType::Upgrade)
-    return true;
-
   name = GetFunctionName(ActionType::Removed);
-  for (const auto& appid : appid_list) {
-    int result = 0;
-    Exec(name, &result, manifest->package, appid, nullptr);
-    if (result) {
-      LOG(ERROR) << "Function: " << name << " of plugin "
-                << plugin_info_.path() << " failed";
+  for (const auto& appid : appid_set_) {
+    result = ExecutePlugin(name, manifest->package, appid.c_str(), nullptr);
+    if (!result)
       return false;
-    }
   }
   return true;
 }
diff --git a/src/common/plugins/category_plugin.h b/src/common/plugins/category_plugin.h
index 883d4a98..33ce14aa 100644
--- a/src/common/plugins/category_plugin.h
+++ b/src/common/plugins/category_plugin.h
@@ -6,6 +6,7 @@
 #define COMMON_PLUGINS_CATEGORY_PLUGIN_H_
 
 #include <memory>
+#include <set>
 #include <string>
 
 #include "common/plugins/plugin.h"
@@ -21,9 +22,14 @@ class CategoryPlugin : public Plugin {
            ActionType action_type) override;
 
  private:
+  bool LoadPluginInfo(manifest_x* manifest);
+  bool ExecutePlugin(const std::string& name, const char* pkgid,
+      const char* appid, GList* category_list);
   bool AddPluginInfo(manifest_x* manifest, const char* appid);
   std::string GetFunctionName(ActionType action) const;
 
+  std::set<std::string> appid_set_;
+
   using Plugin::Plugin;
 
   SCOPE_LOG_TAG(CategoryPlugin)
diff --git a/src/common/plugins/metadata_plugin.cc b/src/common/plugins/metadata_plugin.cc
index c11a577a..cf2952af 100644
--- a/src/common/plugins/metadata_plugin.cc
+++ b/src/common/plugins/metadata_plugin.cc
@@ -30,37 +30,39 @@ void ClearMetadataDetail(gpointer data) {
 }
 
 GList* GetMetadataListForKey(GList* list, const std::string& key) {
-  // pack all metadata starting with key to list that will
-  // be sent to the plugin.
-  // e.g. all http://developer.samsung.com/tizen/metadata/profile/*
-  //   will be packed for http://developer.samsung.com/tizen/metadata/profile
   GList* md_list = nullptr;
   for (metadata_x* meta : GListRange<metadata_x*>(list)) {
-    if (meta->key && meta->value &&
-        std::string(meta->key).find(key) == 0) {
-      __metadata_t* md = reinterpret_cast<__metadata_t*>(
-          calloc(1, sizeof(__metadata_t)));
-      if (!md) {
-        LOG(ERROR) << "Out of memory";
-        g_list_free_full(md_list, &ClearMetadataDetail);
-        return nullptr;
-      }
-      md->key = strdup(meta->key);
-      if (!md->key) {
-        LOG(ERROR) << "Out of memory";
-        free(md);
-        g_list_free_full(md_list, &ClearMetadataDetail);
-        return nullptr;
-      }
-      md->value = strdup(meta->value);
-      if (!md->value) {
-        LOG(ERROR) << "Out of memory";
-        ClearMetadataDetail(md);
-        g_list_free_full(md_list, &ClearMetadataDetail);
-        return nullptr;
-      }
-      md_list = g_list_append(md_list, md);
+    // key and val should not be null (at least empty string)
+    if (!meta->key || !meta->val) {
+      LOG(ERROR) << "Metadata key or val is null";
+      continue;
+    }
+
+    if (std::string(meta->key).find(key) != 0)
+      continue;
+
+    __metadata_t* md = reinterpret_cast<__metadata_t*>(
+        calloc(1, sizeof(__metadata_t)));
+    if (!md) {
+      LOG(ERROR) << "Out of memory";
+      g_list_free_full(md_list, &ClearMetadataDetail);
+      return nullptr;
+    }
+    md->key = strdup(meta->key);
+    if (!md->key) {
+      LOG(ERROR) << "Out of memory";
+      free(md);
+      g_list_free_full(md_list, &ClearMetadataDetail);
+      return nullptr;
     }
+    md->value = strdup(meta->value);
+    if (!md->value) {
+      LOG(ERROR) << "Out of memory";
+      ClearMetadataDetail(md);
+      g_list_free_full(md_list, &ClearMetadataDetail);
+      return nullptr;
+    }
+    md_list = g_list_append(md_list, md);
   }
   return md_list;
 }
@@ -169,6 +171,10 @@ bool MetadataPlugin::Run(xmlDocPtr /*doc_ptr*/, manifest_x* manifest,
   }
 
   bool result;
+  // pack all metadata starting with key to list that will
+  // be sent to the plugin.
+  // e.g. all http://developer.samsung.com/tizen/metadata/profile/*
+  //   will be packed for http://developer.samsung.com/tizen/metadata/profile
   GList* md_list = GetMetadataListForKey(manifest->metadata,
       plugin_info_.name());
   if (md_list) {