Cleanup tpk-backend

- Improve readability.
- Reduce indentation depth.
- Extract codes to make lighter functions.
- Remove incorrect comments.

Change-Id: I05447a4dcf975e6069f5e1b11277d6509edd9557
Signed-off-by: Junghyun Yeon <jungh.yeon@samsung.com>

diff --git a/src/lib/tpk_archive_info.cc b/src/lib/tpk_archive_info.cc
index 34e317eae3138aa58d23cdfc712a18ee6ab3a1cd..8b613d4e4e4a1ac079903673dc646bc0ed5093c2 100644 (file)
--- a/src/lib/tpk_archive_info.cc
+++ b/src/lib/tpk_archive_info.cc
@@ -116,19 +116,16 @@ bool TpkArchiveInfo::GetLabelInfo(const tpk::parse::TPKConfigParser& parser,
     return false;
   }
 
-  // search current locale first
   for (auto& label : pkg_info->labels()) {
     if (label.first.empty())
       continue;
-    // There is label info in PackageInfo using below format:
-    //   using LangTextPair = pair<lang, text>;
-    // Maybe this can be refactored.
+
     if (!strcmp(label.first.c_str(), locale)) {
       label_ = label.second;
       return true;
     }
   }
-  // search again if cannot find label with current locale
+
   for (auto& label : pkg_info->labels()) {
     if (label.first.empty()) {
       label_ = label.second;
@@ -140,12 +137,15 @@ bool TpkArchiveInfo::GetLabelInfo(const tpk::parse::TPKConfigParser& parser,
   if (GetAppLabel<tpk::parse::UIApplicationInfoList>(parser,
       tpk::application_keys::kUIApplicationKey, locale))
     return true;
+
   if (GetAppLabel<tpk::parse::ServiceApplicationInfoList>(parser,
       tpk::application_keys::kServiceApplicationKey, locale))
     return true;
+
   if (GetAppLabel<tpk::parse::WidgetApplicationInfoList>(parser,
       tpk::application_keys::kWidgetApplicationKey, locale))
     return true;
+
   if (GetAppLabel<tpk::parse::WatchApplicationInfoList>(parser,
       tpk::application_keys::kWatchApplicationKey, locale))
     return true;
@@ -161,18 +161,18 @@ bool TpkArchiveInfo::GetDescriptionInfo(
   if (!desc_info)
     return false;
 
-  // search current locale first
   if (locale) {
     for (auto& desc : desc_info->descriptions) {
       if (desc.xml_lang().empty())
         continue;
+
       if (!strcmp(desc.xml_lang().c_str(), locale)) {
         description_ = desc.description();
         return true;
       }
     }
   }
-  // search again if cannot find desc with current locale
+
   for (auto& desc : desc_info->descriptions) {
     if (desc.xml_lang().empty()) {
       description_ = desc.description();
@@ -185,16 +185,18 @@ bool TpkArchiveInfo::GetDescriptionInfo(
 
 bool TpkArchiveInfo::GetIconInfo(const tpk::parse::TPKConfigParser& parser,
     const char* locale) {
-  // get icon from ui application
   if (GetAppIcon<tpk::parse::UIApplicationInfoList>(parser,
       tpk::application_keys::kUIApplicationKey, locale))
     return true;
+
   if (GetAppIcon<tpk::parse::ServiceApplicationInfoList>(parser,
       tpk::application_keys::kServiceApplicationKey, locale))
     return true;
+
   if (GetAppIcon<tpk::parse::WidgetApplicationInfoList>(parser,
       tpk::application_keys::kWidgetApplicationKey, locale))
     return true;
+
   if (GetAppIcon<tpk::parse::WatchApplicationInfoList>(parser,
       tpk::application_keys::kWatchApplicationKey, locale))
     return true;
@@ -236,8 +238,10 @@ bool TpkArchiveInfo::ReadIcon(const bf::path& icon, const bf::path& tmp_dir) {
 
 bool TpkArchiveInfo::LoadArchiveInfo() {
   bf::path tmp_dir = ci::GenerateTmpDir("/tmp");
+
   if (!ci::CreateDir(tmp_dir))
     return false;
+
   LOG(DEBUG) << "Unpack at temporary dir: " << tmp_dir;
   if (!ExtractPackageArchive(path_, kManifestFileName, tmp_dir.string()))
     return false;
@@ -254,8 +258,10 @@ bool TpkArchiveInfo::LoadArchiveInfo() {
 
   if (!GetAuthorInfo(parser))
     LOG(WARNING) << "Failed to get author info";
+
   if (!GetPrivilegesInfo(parser))
     LOG(WARNING) << "Failed to get privilege info";
+
   if (!GetDependencyInfo(parser))
     LOG(WARNING) << "Failed to get dependency info";
 
@@ -269,10 +275,13 @@ bool TpkArchiveInfo::LoadArchiveInfo() {
 
   if (!GetLabelInfo(parser, locale))
     LOG(WARNING) << "Failed to get label info";
+
   if (!GetDescriptionInfo(parser, locale))
     LOG(WARNING) << "Failed to get description info";
+
   if (!GetIconInfo(parser, locale))
     LOG(WARNING) << "Failed to get icon path";
+
   free(locale);
   if (!icon_.empty()) {
     bf::path icon_path = bf::path(kSharedResDir) / icon_;
@@ -280,6 +289,7 @@ bool TpkArchiveInfo::LoadArchiveInfo() {
       RemoveTmpDir(tmp_dir.string());
       return false;
     }
+
     if (!ReadIcon(icon_path, tmp_dir)) {
       LOG(WARNING) << "Failed to get icon info";
       RemoveTmpDir(tmp_dir.string());

diff --git a/src/tpk/external_dirs.cc b/src/tpk/external_dirs.cc
index 9d9a798b167e932263c9d07705576870bd9b3191..eed0d3f803f1f8c6973da90d1e19e66131ca1b94 100644 (file)
--- a/src/tpk/external_dirs.cc
+++ b/src/tpk/external_dirs.cc
@@ -58,11 +58,6 @@ bool DeleteExternalAppdataDirectories(const std::string& pkgid,
       break;
     }
     case ci::RequestMode::USER: {
-      // if package is globally installed, leave directories
-      ci::PkgQueryInterface pkg_query(pkgid, GLOBAL_USER);
-      if (pkg_query.IsPackageInstalled(ci::RequestMode::GLOBAL))
-        return true;
-
       LOG(DEBUG) << "Removing external directories for user: " << uid;
       ci::PerformExternalDirectoryDeletionForUser(uid, pkgid);
       break;

diff --git a/src/tpk/step/configuration/step_adjust_install_location.cc b/src/tpk/step/configuration/step_adjust_install_location.cc
index fd4bee95f8f4fd5d0c649c002ddb3abd8d9eabe2..e0e15b5281846e0a5bc71e25cca7967a9f6b8df9 100644 (file)
--- a/src/tpk/step/configuration/step_adjust_install_location.cc
+++ b/src/tpk/step/configuration/step_adjust_install_location.cc
@@ -20,18 +20,21 @@ namespace configuration {
 
 ci::Step::Status StepAdjustInstallLocation::process() {
   ci::PrivilegeLevel level = context_->privilege_level.get();
-  // Unfortunately we don't know privilege level when parsing manifest,
-  // because checking signature is done after parsing.
-  if (level == ci::PrivilegeLevel::PUBLIC) {
-    manifest_x* manifest = context_->manifest_data.get();
-    // This may be allocated by step parse
-    free(const_cast<char*>(manifest->installlocation));
-    manifest->installlocation = strdup(kAutoLocation);
-    if (!manifest->installlocation) {
-      LOG(ERROR) << "Out of memory";
-      return Status::ERROR;
-    }
+
+  if (level != ci::PrivilegeLevel::PUBLIC)
+    return Status::OK;
+
+  manifest_x* manifest = context_->manifest_data.get();
+  if (!manifest)
+    return Status::ERROR;
+
+  free(const_cast<char*>(manifest->installlocation));
+  manifest->installlocation = strdup(kAutoLocation);
+  if (!manifest->installlocation) {
+    LOG(ERROR) << "Out of memory";
+    return Status::ERROR;
   }
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/configuration/step_check_reinstall_manifest.cc b/src/tpk/step/configuration/step_check_reinstall_manifest.cc
index 82bb073d42862d4745d1cf3d712ef3b23cb4dae4..7e6b22f47a790f397586bcd1b169c4f3cdfe9a7a 100644 (file)
--- a/src/tpk/step/configuration/step_check_reinstall_manifest.cc
+++ b/src/tpk/step/configuration/step_check_reinstall_manifest.cc
@@ -8,6 +8,8 @@
 #include <boost/filesystem/path.hpp>
 #include <boost/system/error_code.hpp>
 
+#include <common/utils/file_util.h>
+
 namespace bf = boost::filesystem;
 namespace bs = boost::system;
 
@@ -22,6 +24,7 @@ namespace configuration {
 
 common_installer::Step::Status StepCheckReinstallManifest::process() {
   bf::path target = context_->unpacked_dir_path.get() / kManifest;
+
   if (bf::exists(target))
     return Status::OK;
 
@@ -31,9 +34,8 @@ common_installer::Step::Status StepCheckReinstallManifest::process() {
     LOG(ERROR) << "Cannot find old manifest file";
     return Status::APP_DIR_ERROR;
   }
-  bs::error_code error;
-  bf::copy_file(source, target, error);
-  if (error) {
+
+  if (!common_installer::CopyFile(source, target)) {
     LOG(ERROR) << "Failed to copy old manifest file";
     return Status::APP_DIR_ERROR;
   }

diff --git a/src/tpk/step/filesystem/step_check_pkg_directory_path.cc b/src/tpk/step/filesystem/step_check_pkg_directory_path.cc
index 8223d397fcbc268e902034dfb4647e1e96e12762..a43a04f0ea9c569806043471e9179fe9b39758f7 100644 (file)
--- a/src/tpk/step/filesystem/step_check_pkg_directory_path.cc
+++ b/src/tpk/step/filesystem/step_check_pkg_directory_path.cc
@@ -7,6 +7,8 @@
 #include <boost/filesystem.hpp>
 #include <string>
 
+#include <common/utils/file_util.h>
+
 namespace tpk {
 namespace filesystem {
 
@@ -17,16 +19,12 @@ common_installer::Step::Status StepCheckPkgDirPath::process() {
   if (bf::exists(context_->GetPkgPath()))
     return Status::OK;
 
-  LOG(INFO) << "Create pkg_path("
-            << context_->GetPkgPath()
-            << ") for package("
-            << context_->pkgid.get() << ")";
-  bs::error_code error;
-  bf::create_directories(context_->GetPkgPath(), error);
-  if (error) {
+  LOG(INFO) << "Create pkg_path(" << context_->GetPkgPath()
+            << ") for package(" << context_->pkgid.get() << ")";
+
+  if (!common_installer::CreateDir(context_->GetPkgPath())) {
     LOG(ERROR) << "Cannot create directory: "
-               << context_->GetPkgPath().string()
-               << ", error: " << error.message();
+               << context_->GetPkgPath().string();
     return Step::Status::APP_DIR_ERROR;
   }
 

diff --git a/src/tpk/step/filesystem/step_create_external_storage_directories.cc b/src/tpk/step/filesystem/step_create_external_storage_directories.cc
index 2cdf090a2c2ddc05a42b5bc0400690336aafa28f..9f790120d98b354d58a54b2dfcc766cea113caea 100644 (file)
--- a/src/tpk/step/filesystem/step_create_external_storage_directories.cc
+++ b/src/tpk/step/filesystem/step_create_external_storage_directories.cc
@@ -30,6 +30,7 @@ ci::Step::Status StepCreateExternalStorageDirectories::process() {
     LOG(ERROR) << "Failed to create external appdata directories";
     return Status::APP_DIR_ERROR;
   }
+
   return Status::OK;
 }
 
@@ -38,6 +39,7 @@ ci::Step::Status StepCreateExternalStorageDirectories::precheck() {
     LOG(ERROR) << "Manifest data not set";
     return Status::INVALID_VALUE;
   }
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/filesystem/step_create_tpk_symbolic_link.cc b/src/tpk/step/filesystem/step_create_tpk_symbolic_link.cc
index 716a6e31202835c2fa2fc33679be137fa59b4a08..1b5b6b7da94c3e40b141aae6813b23325186bfe5 100644 (file)
--- a/src/tpk/step/filesystem/step_create_tpk_symbolic_link.cc
+++ b/src/tpk/step/filesystem/step_create_tpk_symbolic_link.cc
@@ -24,9 +24,25 @@ namespace ci = common_installer;
 using common_installer::InstallerContext;
 typedef common_installer::Step::Status Status;
 
+bool CreateUGClientSymlink(const bf::path& app_exec_path) {
+  bf::path ug_client_path(tzplatform_mkpath(TZ_SYS_BIN, "ug-client"));
+
+  LOG(INFO) << "Createing symlink " << app_exec_path << " pointing " <<
+    ug_client_path;
+  boost::system::error_code error;
+  bf::create_symlink(ug_client_path, app_exec_path, error);
+  if (error) {
+    LOG(ERROR) << "Symlink creation failure: " << app_exec_path
+               << ", error :" << error.message();
+    return false;
+  }
+
+  return true;
+}
+
 bool CreateSymLink(application_x* app, InstallerContext* context) {
-  bf::path bindir = context->GetPkgPath() /
-      bf::path("bin");
+  bf::path bindir = context->GetPkgPath() / bf::path("bin");
+
   LOG(DEBUG) << "Creating dir: " << bindir;
   if (!bf::is_symlink(symlink_status(bindir)) &&
       !common_installer::CreateDir(bindir)) {
@@ -41,28 +57,20 @@ bool CreateSymLink(application_x* app, InstallerContext* context) {
   if (bf::exists(app_exec_path))
     return true;
 
-  // Ug-client path
-  // Make a symlink with the name of appid, pointing /usr/bin/ug-client
-  bf::path ug_client_path(tzplatform_mkpath(TZ_SYS_BIN, "ug-client"));
-  LOG(INFO) << "Createing symlink " << app_exec_path << " pointing " <<
-    ug_client_path;
-  boost::system::error_code error;
-  bf::create_symlink(ug_client_path, app_exec_path, error);
-  if (error) {
-    LOG(ERROR) << "Symlink creation failure: " << app_exec_path
-               << ", error :" << error.message();
+  if (!CreateUGClientSymlink(app_exec_path))
     return false;
-  }
 
   return true;
 }
 
 Status StepCreateTpkSymbolicLink::precheck() {
-  manifest_x *m = context_->manifest_data.get();
+  manifest_x* m = context_->manifest_data.get();
+
   if (!m) {
     LOG(ERROR) << "manifest_data attribute is empty";
     return Step::Status::INVALID_VALUE;
   }
+
   if (!m->application) {
     LOG(ERROR) << "No application exists";
     return Step::Status::ERROR;
@@ -77,6 +85,7 @@ Status StepCreateTpkSymbolicLink::process() {
     // filter out non-tpk apps as this step is run for hybrid backend too
     if (strcmp("webapp", app->type) == 0)
       continue;
+
     if (!CreateSymLink(app, context_))
       return Status::ERROR;
   }

diff --git a/src/tpk/step/filesystem/step_remove_external_storage_directories.cc b/src/tpk/step/filesystem/step_remove_external_storage_directories.cc
index 9cfe748998a565e24faebd83e6794f772a8e56b0..6086a88bc755b6084d2409fe38fbd50427ac1a62 100644 (file)
--- a/src/tpk/step/filesystem/step_remove_external_storage_directories.cc
+++ b/src/tpk/step/filesystem/step_remove_external_storage_directories.cc
@@ -16,6 +16,7 @@ ci::Step::Status StepRemoveExternalStorageDirectories::process() {
                                         context_->request_mode.get(),
                                         context_->uid.get()))
     return Status::APP_DIR_ERROR;
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/filesystem/step_tpk_patch_icons.cc b/src/tpk/step/filesystem/step_tpk_patch_icons.cc
index ed1512433095e3ab1ef214de12e06f98af4a16c8..5fa5a32b95254d4c513838a1a1380fabef7d51e6 100644 (file)
--- a/src/tpk/step/filesystem/step_tpk_patch_icons.cc
+++ b/src/tpk/step/filesystem/step_tpk_patch_icons.cc
@@ -56,6 +56,7 @@ common_installer::Step::Status StepTpkPatchIcons::ProcessIconOutsidePackage(
     const bf::path& common_icon_location, const bf::path& icon_text,
     application_x* app, icon_x* icon) {
   bf::path destination = common_icon_location / app->appid;
+
   if (!icon_text.extension().empty())
     destination += icon_text.extension();
   else
@@ -66,8 +67,10 @@ common_installer::Step::Status StepTpkPatchIcons::ProcessIconOutsidePackage(
 
   free(const_cast<char*>(icon->text));
   icon->text = strdup(destination.c_str());
-  if (!icon->text)
+  if (!icon->text) {
+    LOG(ERROR) << "Out of memory";
     return Status::ICON_ERROR;
+  }
 
   return Status::OK;
 }
@@ -79,11 +82,13 @@ common_installer::Step::Status StepTpkPatchIcons::FixIconLocation(
                                context_->root_application_path.get(),
                                context_->uid.get(),
                                context_->is_readonly_package.get());
+
   if (!source.empty()) {
     LOG(DEBUG) << "Fix location of icon: " << source << " to: " << icon_text;
     if (!common_installer::CopyFile(source, icon_text))
       return Status::ICON_ERROR;
   }
+
   return Status::OK;
 }
 
@@ -102,6 +107,7 @@ common_installer::Step::Status StepTpkPatchIcons::process() {
       GListRange<application_x*>(context_->manifest_data.get()->application)) {
     if (!IsTpkApp(app))
       continue;
+
     for (auto& icon : GListRange<icon_x*>(app->icon)) {
       bf::path icon_text(icon->text);
       if (icon_text.parent_path() != common_icon_location) {

diff --git a/src/tpk/step/filesystem/step_tpk_prepare_package_directory.cc b/src/tpk/step/filesystem/step_tpk_prepare_package_directory.cc
index f9fc9a9e9d1379f420d3a78abff1a793177851c3..74ff98308e3f35ed7e244f3c908bb961095eeac2 100644 (file)
--- a/src/tpk/step/filesystem/step_tpk_prepare_package_directory.cc
+++ b/src/tpk/step/filesystem/step_tpk_prepare_package_directory.cc
@@ -23,6 +23,7 @@ ci::Step::Status StepTpkPreparePackageDirectory::BackupDirectory(
     const std::string& entry, const bf::path& backup_path) {
   if (!bf::exists(context_->GetPkgPath() / entry))
     return Status::OK;
+
   if (!ci::MoveDir(context_->GetPkgPath() / entry, backup_path / entry,
       ci::FSFlag::FS_MERGE_OVERWRITE |
       ci::FSFlag::FS_COMMIT_COPY_FILE |
@@ -54,26 +55,31 @@ ci::Step::Status StepTpkPreparePackageDirectory::PrepareLink(
     const std::string& entry, const bf::path& mount_point) {
   bs::error_code error;
   bf::path mount_point_entry = mount_point / entry;
-  if (bf::exists(mount_point_entry)) {
-    bf::path destination = context_->GetPkgPath() / entry;
-    if (bf::exists(destination)) {
-      if (!bf::is_symlink(symlink_status(destination))) {
-        LOG(ERROR) << "Cannot proceed. "
-                   << "Location of link is used by another file";
-        return Status::APP_DIR_ERROR;
-      }
-      bf::remove(destination, error);
-      if (error) {
-        LOG(ERROR) << "Failed to remove previous symlink";
-        return Status::APP_DIR_ERROR;
-      }
+
+  if (!bf::exists(mount_point_entry))
+    return Status::OK;
+
+  bf::path destination = context_->GetPkgPath() / entry;
+  if (bf::exists(destination)) {
+    if (!bf::is_symlink(symlink_status(destination))) {
+      LOG(ERROR) << "Cannot proceed. "
+                  << "Location of link is used by another file";
+      return Status::APP_DIR_ERROR;
     }
-    bf::create_symlink(mount_point_entry, destination, error);
+
+    bf::remove(destination, error);
     if (error) {
-      LOG(ERROR) << "Failed to create symlink for entry: " << mount_point_entry;
+      LOG(ERROR) << "Failed to remove previous symlink";
       return Status::APP_DIR_ERROR;
     }
   }
+
+  bf::create_symlink(mount_point_entry, destination, error);
+  if (error) {
+    LOG(ERROR) << "Failed to create symlink for entry: " << mount_point_entry;
+    return Status::APP_DIR_ERROR;
+  }
+
   return Status::OK;
 }
 
@@ -85,13 +91,16 @@ ci::Step::Status StepTpkPreparePackageDirectory::ExtractEntries() {
 
   for (auto& entry : tpk::GetExtractEntries()) {
     LOG(DEBUG) << "Extracting: " << entry;
-    if (context_->request_type.get() == ci::RequestType::MountUpdate)
+    if (context_->request_type.get() == ci::RequestType::MountUpdate) {
       if (BackupDirectory(entry, backup_path) != Status::OK)
         return Status::APP_DIR_ERROR;
+    }
+
     auto status = PrepareDirectory(entry);
     if (status != Status::OK)
       return status;
   }
+
   return Status::OK;
 }
 
@@ -100,17 +109,21 @@ ci::Step::Status StepTpkPreparePackageDirectory::PrepareLinks() {
   LOG(DEBUG) << "Creating symlinks to zip package...";
   for (auto& link_entry : tpk::GetSymlinkEntries()) {
     LOG(DEBUG) << "Symlink: " << link_entry;
+
     auto status = PrepareLink(link_entry, mount_point);
     if (status != Status::OK)
       return status;
   }
+
   return Status::OK;
 }
 
 ci::Step::Status StepTpkPreparePackageDirectory::process() {
   auto status = ExtractEntries();
+
   if (status != Status::OK)
     return status;
+
   return PrepareLinks();
 }
 
@@ -119,6 +132,7 @@ ci::Step::Status StepTpkPreparePackageDirectory::precheck() {
     LOG(ERROR) << "Package installation path is not set";
     return Status::INVALID_VALUE;
   }
+
   return Status::OK;
 }
 
@@ -137,12 +151,14 @@ ci::Step::Status StepTpkPreparePackageDirectory::undo() {
     ci::RemoveAll(context_->GetPkgPath() / entry);
     if (!bf::exists(backupPath_ / entry))
       continue;
+
     ci::MoveDir(backupPath_ / entry, context_->GetPkgPath() / entry,
         ci::FSFlag::FS_MERGE_OVERWRITE |
         ci::FSFlag::FS_COMMIT_COPY_FILE |
         ci::FSFlag::FS_PRESERVE_OWNERSHIP_AND_PERMISSIONS);
   }
   ci::RemoveAll(backupPath_);
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/filesystem/step_tpk_update_package_directory.cc b/src/tpk/step/filesystem/step_tpk_update_package_directory.cc
index 6993ccb16b61dfe6871cd2959864cbb9dd78fd4f..ee0579f6923b7eb4b795942b4dfaf792d2d299db 100644 (file)
--- a/src/tpk/step/filesystem/step_tpk_update_package_directory.cc
+++ b/src/tpk/step/filesystem/step_tpk_update_package_directory.cc
@@ -22,18 +22,15 @@ namespace {
 
 bool ReplacePaths(const bf::path& source, const bf::path& destination) {
   if (!bf::exists(destination.parent_path())) {
-    bs::error_code error;
-    bf::create_directories(destination.parent_path(), error);
-    if (error) {
-      LOG(ERROR) << "Failed to create destination directory for directory "
-                 << "backup";
+    if (!ci::CreateDir(destination.parent_path()))
       return false;
-    }
   }
+
   if (!ci::MoveDir(source, destination)) {
     LOG(ERROR) << "Failed to move " << source << " to " << destination;
     return false;
   }
+
   return true;
 }
 
@@ -45,32 +42,31 @@ namespace filesystem {
 ci::Step::Status StepTpkUpdatePackageDirectory::BackupDirectory(
     const std::string& entry, const boost::filesystem::path& backup_path) {
   bf::path source = context_->GetPkgPath() / entry;
+
   if (bf::exists(source)) {
     bf::path destination = backup_path / entry;
     if (!ReplacePaths(source, destination))
       return Status::APP_DIR_ERROR;
   }
+
   return Status::OK;
 }
 
 ci::Step::Status StepTpkUpdatePackageDirectory::RestoreDirectory(
     const std::string& entry, const boost::filesystem::path& backup_path) {
-  // restore backup if directory exists
-  if (bf::exists(backup_path / entry)) {
-    bf::path source = backup_path / entry;
-    bf::path destination = context_->GetPkgPath() / entry;
-    if (bf::exists(destination)) {
-      bs::error_code error;
-      bf::remove_all(destination, error);
-      if (error) {
-        LOG(ERROR) << "Failed to remove restore destination path: "
-            << destination;
-        return Status::APP_DIR_ERROR;
-      }
-    }
-    if (!ReplacePaths(source, destination))
+  if (!bf::exists(backup_path / entry))
+    return Status::OK;
+
+  bf::path source = backup_path / entry;
+  bf::path destination = context_->GetPkgPath() / entry;
+  if (bf::exists(destination)) {
+    if (!ci::RemoveAll(destination))
       return Status::APP_DIR_ERROR;
   }
+
+  if (!ReplacePaths(source, destination))
+    return Status::APP_DIR_ERROR;
+
   return Status::OK;
 }
 
@@ -78,17 +74,20 @@ ci::Step::Status StepTpkUpdatePackageDirectory::RemoveDirectory(
     const boost::filesystem::path& dir_path) {
   if (!ci::RemoveAll(dir_path))
     return Status::APP_DIR_ERROR;
+
   return Status::OK;
 }
 
 ci::Step::Status StepTpkUpdatePackageDirectory::BackupEntries() {
   bf::path backup_path =
       ci::GetBackupPathForPackagePath(context_->GetPkgPath());
+
   for (auto& entry : tpk::GetExtractEntries()) {
     auto status = BackupDirectory(entry, backup_path);
     if (status != Status::OK)
       return status;
   }
+
   return Status::OK;
 }
 
@@ -96,9 +95,11 @@ ci::Step::Status StepTpkUpdatePackageDirectory::process() {
   auto status = BackupEntries();
   if (status != Status::OK)
     return status;
+
   status = ExtractEntries();
   if (status != Status::OK)
     return status;
+
   return PrepareLinks();
 }
 
@@ -106,6 +107,7 @@ ci::Step::Status StepTpkUpdatePackageDirectory::clean() {
   bf::path backup_path =
       ci::GetBackupPathForPackagePath(context_->GetPkgPath());
   RemoveDirectory(backup_path);
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/filesystem/step_update_external_storage_directories.cc b/src/tpk/step/filesystem/step_update_external_storage_directories.cc
index bd0150c12df54c4a47cfc5a0ea31bf72849ed774..94948c30d86f2da8d32bb0d7d9aff4fdfca6ab8a 100644 (file)
--- a/src/tpk/step/filesystem/step_update_external_storage_directories.cc
+++ b/src/tpk/step/filesystem/step_update_external_storage_directories.cc
@@ -31,6 +31,7 @@ ci::Step::Status StepUpdateExternalStorageDirectories::precheck() {
     LOG(ERROR) << "Manifest data not set";
     return Status::INVALID_VALUE;
   }
+
   return Status::OK;
 }
 

diff --git a/src/tpk/step/pkgmgr/step_convert_xml.cc b/src/tpk/step/pkgmgr/step_convert_xml.cc
index 9a1d1a5978b6fd19927bb911921cbdb9224bd2ee..c85e719a3b7de923a624c2368b8e35b20ff75c06 100644 (file)
--- a/src/tpk/step/pkgmgr/step_convert_xml.cc
+++ b/src/tpk/step/pkgmgr/step_convert_xml.cc
@@ -142,15 +142,17 @@ common_installer::Step::Status StepConvertXml::clean() {
 }
 
 common_installer::Step::Status StepConvertXml::undo() {
-  if (ci::Remove(new_path_)) {
-    if (!backup_path_.empty()) {
-      bs::error_code error;
-      bf::rename(backup_path_, new_path_, error);
-      if (error)
-        LOG(WARNING) << "Failed to restore " << new_path_
-                     << " due to error " << error;
-    }
-  }
+  if (!ci::Remove(new_path_))
+    return Step::Status::OK;
+
+  if (backup_path_.empty())
+    return Step::Status::OK;
+
+  bs::error_code error;
+  bf::rename(backup_path_, new_path_, error);
+  if (error)
+    LOG(WARNING) << "Failed to restore " << new_path_
+                  << " due to error " << error;
 
   return Step::Status::OK;
 }

diff --git a/src/tpk/step/pkgmgr/step_manifest_adjustment.cc b/src/tpk/step/pkgmgr/step_manifest_adjustment.cc
index 091e745def3ce52bb91281fe125d6310e6346213..8587f8cd6212fa4fcc1f3faa4f383dbdb818e997 100644 (file)
--- a/src/tpk/step/pkgmgr/step_manifest_adjustment.cc
+++ b/src/tpk/step/pkgmgr/step_manifest_adjustment.cc
@@ -30,6 +30,31 @@ const xmlChar kOnBootAttributeKey[] = "on-boot";
 const xmlChar kAutoRestartAttributeKey[] = "auto-restart";
 const char kXmlXSvcAppExpr[] = "//*[local-name()='service-application']";
 
+bool SetProperty(xmlNodePtr node, const char* attribute, const char* value) {
+  auto attrib = xmlSetProp(node,
+      reinterpret_cast<libxml_char>(attribute),
+      reinterpret_cast<libxml_char>(value));
+
+  if (attrib == nullptr) {
+    LOG(ERROR) << "Failed to set attribute: " << attribute;
+    return false;
+  }
+
+  return true;
+}
+
+bool SetManifestElement(char** src, const char* value) {
+  free(*src);
+  *src = strdup(value);
+
+  if (*src == nullptr) {
+    LOG(ERROR) << "Out of memory";
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 namespace tpk {
@@ -58,111 +83,65 @@ common_installer::Step::Status StepManifestAdjustment::process() {
 
   xmlNodePtr node = xmlDocGetRootElement(doc);
 
-  std::string pkgtype_attrib = "type";
-  auto attrib = xmlSetProp(node,
-      reinterpret_cast<libxml_char>(pkgtype_attrib.c_str()),
-      reinterpret_cast<libxml_char>(context_->manifest_data.get()->type));
-
-  if (attrib == nullptr) {
-    LOG(ERROR) << "Failed to set attribute pkgtype";
+  if (!SetProperty(node, "type", context_->manifest_data.get()->type)) {
     xmlFreeDoc(doc);
     return Step::Status::ERROR;
   }
 
-  std::string readonly_attrib = "readonly";
-  attrib = xmlSetProp(node,
-      reinterpret_cast<libxml_char>(readonly_attrib.c_str()),
-      reinterpret_cast<libxml_char>(context_->manifest_data.get()->readonly));
-
-  if (attrib == nullptr) {
-    LOG(ERROR) << "Failed to set attribute readonly";
+  if (!SetProperty(node, "readonly",
+      context_->manifest_data.get()->readonly)) {
     xmlFreeDoc(doc);
     return Step::Status::ERROR;
   }
 
-  std::string preload_attrib = "preload";
-  attrib = xmlSetProp(node,
-      reinterpret_cast<libxml_char>(preload_attrib.c_str()),
-      reinterpret_cast<libxml_char>(context_->manifest_data.get()->preload));
-
-  if (attrib == nullptr) {
-    LOG(ERROR) << "Failed to set attribute preload";
+  if (!SetProperty(node, "preload",
+      context_->manifest_data.get()->preload)) {
     xmlFreeDoc(doc);
     return Step::Status::ERROR;
   }
 
-  std::string removable_attrib = "removable";
-  attrib = xmlSetProp(node,
-      reinterpret_cast<libxml_char>(removable_attrib.c_str()),
-      reinterpret_cast<libxml_char>(context_->manifest_data.get()->removable));
-
-  if (attrib == nullptr) {
-    LOG(ERROR) << "Failed to set attribute removable";
+  if (!SetProperty(node, "removable",
+      context_->manifest_data.get()->removable)) {
     xmlFreeDoc(doc);
     return Step::Status::ERROR;
   }
 
-  if ((context_->privilege_level.get() !=
-      common_installer::PrivilegeLevel::PARTNER) &&
-      (context_->privilege_level.get() !=
-      common_installer::PrivilegeLevel::PLATFORM)) {
-    utils::VersionNumber api_version =
-        utils::VersionNumber(context_->manifest_data.get()->api_version);
-    utils::VersionNumber platform_version =
-        utils::VersionNumber((common_installer::GetTizenProfile() ==
-        common_installer::TizenProfile::WEARABLE) ?
-        "2.3.1" : "2.4");
-
-    if (api_version >= platform_version) {
-      xmlXPathContextPtr xpath_ctx = xmlXPathNewContext(doc);
-      if (!xpath_ctx) {
-        LOG(ERROR) << "Failed to create XPath context";
-        xmlFreeDoc(doc);
-        return Step::Status::ERROR;
-      }
+  if (IsNonPrivilegedPackage() && IsAdjustmentNecessary()) {
+    xmlXPathContextPtr xpath_ctx = xmlXPathNewContext(doc);
+    if (!xpath_ctx) {
+      LOG(ERROR) << "Failed to create XPath context";
+      xmlFreeDoc(doc);
+      return Step::Status::ERROR;
+    }
 
-      for (auto& app : GListRange<application_x*>(
-          context_->manifest_data.get()->application)) {
-        std::string expr = std::string(kXmlXSvcAppExpr);
-        xmlXPathObjectPtr xpath_obj = xmlXPathEvalExpression(
-            (const xmlChar*)expr.c_str(), xpath_ctx);
-        if (!xpath_obj || xmlXPathNodeSetIsEmpty(xpath_obj->nodesetval)) {
-          if (xpath_obj)
-            xmlXPathFreeObject(xpath_obj);
-          continue;
-        }
-
-        for (int i = 0; i < xpath_obj->nodesetval->nodeNr; i++) {
-          xmlNodePtr node = xpath_obj->nodesetval->nodeTab[i];
-          xmlSetProp(node, kOnBootAttributeKey, (const xmlChar*)"false");
-          xmlSetProp(node, kAutoRestartAttributeKey, (const xmlChar*)"false");
-        }
-
-        if (app->autorestart)
-          free(app->autorestart);
-        app->autorestart = strdup("false");
-        if (!app->autorestart) {
-          LOG(ERROR) << "Out of memory";
-          xmlXPathFreeObject(xpath_obj);
-          xmlXPathFreeContext(xpath_ctx);
-          xmlFreeDoc(doc);
-          return Step::Status::ERROR;
-        }
-
-        if (app->onboot)
-          free(app->onboot);
-        app->onboot = strdup("false");
-        if (!app->onboot) {
-          LOG(ERROR) << "Out of memory";
+    for (auto& app : GListRange<application_x*>(
+        context_->manifest_data.get()->application)) {
+      std::string expr = std::string(kXmlXSvcAppExpr);
+      xmlXPathObjectPtr xpath_obj = xmlXPathEvalExpression(
+          (const xmlChar*)expr.c_str(), xpath_ctx);
+      if (!xpath_obj || xmlXPathNodeSetIsEmpty(xpath_obj->nodesetval)) {
+        if (xpath_obj)
           xmlXPathFreeObject(xpath_obj);
-          xmlXPathFreeContext(xpath_ctx);
-          xmlFreeDoc(doc);
-          return Step::Status::ERROR;
-        }
+        continue;
+      }
+
+      for (int i = 0; i < xpath_obj->nodesetval->nodeNr; i++) {
+        xmlNodePtr node = xpath_obj->nodesetval->nodeTab[i];
+        xmlSetProp(node, kOnBootAttributeKey, (const xmlChar*)"false");
+        xmlSetProp(node, kAutoRestartAttributeKey, (const xmlChar*)"false");
+      }
+
+      if (!SetManifestElement(&(app->autorestart), "false") ||
+          !SetManifestElement(&(app->onboot), "false")) {
         xmlXPathFreeObject(xpath_obj);
+        xmlXPathFreeContext(xpath_ctx);
+        xmlFreeDoc(doc);
+        return Step::Status::ERROR;
       }
-      xmlXPathFreeContext(xpath_ctx);
+
+      xmlXPathFreeObject(xpath_obj);
     }
+    xmlXPathFreeContext(xpath_ctx);
   }
 
   if (xmlSaveFile(xml_path_.c_str(), doc) == -1) {
@@ -176,5 +155,29 @@ common_installer::Step::Status StepManifestAdjustment::process() {
   return Step::Status::OK;
 }
 
+bool StepManifestAdjustment::IsNonPrivilegedPackage() {
+  if ((context_->privilege_level.get() !=
+        common_installer::PrivilegeLevel::PARTNER) &&
+        (context_->privilege_level.get() !=
+        common_installer::PrivilegeLevel::PLATFORM))
+    return true;
+
+  return false;
+}
+
+bool StepManifestAdjustment::IsAdjustmentNecessary() {
+  utils::VersionNumber api_version =
+      utils::VersionNumber(context_->manifest_data.get()->api_version);
+  utils::VersionNumber platform_version =
+      utils::VersionNumber((common_installer::GetTizenProfile() ==
+      common_installer::TizenProfile::WEARABLE) ?
+      "2.3.1" : "2.4");
+
+  if (api_version >= platform_version)
+    return true;
+
+  return false;
+}
+
 }  // namespace pkgmgr
 }  // namespace tpk

diff --git a/src/tpk/step/pkgmgr/step_manifest_adjustment.h b/src/tpk/step/pkgmgr/step_manifest_adjustment.h
index 9ec583fe0f1b53f443cc323e0dadfaaf7ff05f3a..be79afec92fa07c679f8292ea8acb5aad92273ef 100644 (file)
--- a/src/tpk/step/pkgmgr/step_manifest_adjustment.h
+++ b/src/tpk/step/pkgmgr/step_manifest_adjustment.h
@@ -26,6 +26,9 @@ class StepManifestAdjustment : public common_installer::Step {
  private:
   boost::filesystem::path xml_path_;
 
+  bool IsNonPrivilegedPackage();
+  bool IsAdjustmentNecessary();
+
   STEP_NAME(ManifestAdjustment)
 };
 

diff --git a/src/tpk/tpk_app_query_interface.cc b/src/tpk/tpk_app_query_interface.cc
index 5756363cdc0edcce8bc9848d0580ef947aae55d5..28dca0c27f03b4ce2f98fecca89d0d737b65ca83 100644 (file)
--- a/src/tpk/tpk_app_query_interface.cc
+++ b/src/tpk/tpk_app_query_interface.cc
@@ -44,6 +44,7 @@ std::string TpkAppQueryInterface::GetPkgIdFromPath(
   bf::path tmp_path = ExtractManifest(path);
   if (tmp_path.empty())
     return {};
+
   bf::path manifest_path = tmp_path / kManifestFileName;
   if (!bf::exists(manifest_path)) {
     ClearTemporaryFile(tmp_path);
@@ -55,12 +56,14 @@ std::string TpkAppQueryInterface::GetPkgIdFromPath(
     ClearTemporaryFile(tmp_path);
     return {};
   }
+
   auto package_info = std::static_pointer_cast<const tpk::parse::PackageInfo>(
       parser.GetManifestData(tpk::application_keys::kManifestKey));
   if (!package_info) {
     ClearTemporaryFile(tmp_path);
     return {};
   }
+
   std::string pkg_id = package_info->package();
   ClearTemporaryFile(tmp_path);
   return pkg_id;