Step::undo can also return ERROR.

Since AppInstaller is calling all "undo()" even if one of them returns
ERROR, Step::undo can return ERROR.

Commit adds returning ERROR from some of the Step::undo methods.

Change-Id: I989521c43ed6a603e24c06adb58c35fb1e0ed6c9

diff --git a/src/common/app_installer.cc b/src/common/app_installer.cc
index 10a55ac..7eba068 100644 (file)
--- a/src/common/app_installer.cc
+++ b/src/common/app_installer.cc
@@ -70,7 +70,7 @@ int AppInstaller::Run() {
     LOG(ERROR) << "Failure occurs";
     do {
       if ((*it)->undo() != Step::Status::OK) {
-        LOG(ERROR) << "Error during undo operation";
+        LOG(ERROR) << "Error during undo operation, but continuing...!";
         ret = -2;
       }
     } while (it-- != itStart);

diff --git a/src/common/step/step_backup_icons.cc b/src/common/step/step_backup_icons.cc
index f77f6ff..e02b1ea 100755 (executable)
--- a/src/common/step/step_backup_icons.cc
+++ b/src/common/step/step_backup_icons.cc
@@ -53,7 +53,7 @@ Step::Status StepBackupIcons::undo() {
   for (auto& pair : icons_) {
     if (!MoveFile(pair.second, pair.first)) {
       LOG(ERROR) << "Cannot revert icon from backup: " << pair.first;
-      // undo cannot fail, so no break
+      return Status::ERROR;
     }
   }
   LOG(DEBUG) << "Icons reverted from backup";

diff --git a/src/common/step/step_copy_storage_directories.cc b/src/common/step/step_copy_storage_directories.cc
index 43d8992..e230f3e 100644 (file)
--- a/src/common/step/step_copy_storage_directories.cc
+++ b/src/common/step/step_copy_storage_directories.cc
@@ -66,21 +66,22 @@ common_installer::Step::Status StepCopyStorageDirectories::process() {
 }
 
 common_installer::Step::Status StepCopyStorageDirectories::undo() {
+  common_installer::Step::Status ret = Status::OK;
   if (!MoveAppStorage(context_->pkg_path.get(),
                       backup_path_,
                       kDataLocation)) {
-    LOG(ERROR) << "Failed to restore private directory for widget in update";
-//    return Status::ERROR; // undo cannot fail...
+    LOG(ERROR) << "Failed to restore private directory for package in update";
+    ret = Status::ERROR;
   }
 
   if (!MoveAppStorage(context_->pkg_path.get(),
                       backup_path_,
                       kSharedLocation)) {
-    LOG(ERROR) << "Failed to restore shared directory for widget in update";
-//    return Status::ERROR; // undo cannot fail...
+    LOG(ERROR) << "Failed to restore shared directory for package in update";
+    ret = Status::ERROR;
   }
 
-  return Status::OK;
+  return ret;
 }
 
 }  // namespace filesystem

diff --git a/src/common/step/step_register_app.cc b/src/common/step/step_register_app.cc
index 5391e1b..0b050b6 100644 (file)
--- a/src/common/step/step_register_app.cc
+++ b/src/common/step/step_register_app.cc
@@ -46,8 +46,12 @@ Step::Status StepRegisterApplication::process() {
 }
 
 Step::Status StepRegisterApplication::undo() {
-  UnregisterAppInPkgmgr(context_->xml_path.get(), context_->pkgid.get(),
-                        context_->uid.get());
+  if (!UnregisterAppInPkgmgr(context_->xml_path.get(), context_->pkgid.get(),
+                           context_->uid.get())) {
+    LOG(ERROR) << "Application couldn't be unregistered";
+    return Status::ERROR;
+  }
+
   LOG(INFO) << "Successfuly clean database";
   return Status::OK;
 }

diff --git a/src/common/step/step_remove_files.cc b/src/common/step/step_remove_files.cc
index a10bdb5..b52fbed 100755 (executable)
--- a/src/common/step/step_remove_files.cc
+++ b/src/common/step/step_remove_files.cc
@@ -57,7 +57,7 @@ Step::Status StepRemoveFiles::undo() {
     LOG(DEBUG) << "Restoring directory: " << context_->pkg_path.get();
     if (!MoveDir(backup_path, context_->pkg_path.get())) {
       LOG(ERROR) << "Cannot restore widget files";
-      return Status::OK;
+      return Status::ERROR;
     }
   }
 

diff --git a/src/common/step/step_remove_icons.cc b/src/common/step/step_remove_icons.cc
index 6cb3d9e..327d6ee 100644 (file)
--- a/src/common/step/step_remove_icons.cc
+++ b/src/common/step/step_remove_icons.cc
@@ -63,16 +63,18 @@ Step::Status StepRemoveIcons::clean() {
 }
 
 Step::Status StepRemoveIcons::undo() {
+  Step::Status ret = Status::OK;
   if (!backups_.empty()) {
     LOG(DEBUG) << "Restoring icons files...";
     for (auto& pair : backups_) {
       if (!MoveFile(pair.first, pair.second)) {
         LOG(ERROR) << "Failed to restore: " << pair.second;
         // We need to try to restore all icons anyway...
+        ret = Status::ERROR;
       }
     }
   }
-  return Status::OK;
+  return ret;
 }
 }  // namespace filesystem
 }  // namespace common_installer

diff --git a/src/common/step/step_unregister_app.cc b/src/common/step/step_unregister_app.cc
index 1e9a031..9743aaa 100755 (executable)
--- a/src/common/step/step_unregister_app.cc
+++ b/src/common/step/step_unregister_app.cc
@@ -97,8 +97,7 @@ Step::Status StepUnregisterApplication::undo() {
                            context_->certificate_info.get(),
                            context_->uid.get())) {
     LOG(ERROR) << "Failed to restore the app registration in pkgmgr";
-    // Continue to revert...
-    return Step::Status::OK;
+    return Step::Status::ERROR;
   }
 
   LOG(INFO) << "Successfully restored the app registration in pkgmgr";

diff --git a/src/tpk/step/step_copy_manifest_xml.cc b/src/tpk/step/step_copy_manifest_xml.cc
index c11e0fc..bb71e26 100644 (file)
--- a/src/tpk/step/step_copy_manifest_xml.cc
+++ b/src/tpk/step/step_copy_manifest_xml.cc
@@ -178,7 +178,10 @@ Status StepCopyManifestXml::undo() {
   // Revert old xml file if exists
   bf::path old_dest_xml_path = _getOldDestXmlPath(*dest_xml_path_);
   if (bf::exists(old_dest_xml_path)) {
-    common_installer::MoveFile(old_dest_xml_path, *dest_xml_path_);
+    if (!common_installer::MoveFile(old_dest_xml_path, *dest_xml_path_)) {
+      LOG(ERROR) << "Cannot revert old xml file: " << old_dest_xml_path;
+      return Status::ERROR;
+    }
   }
 
   return Status::OK;

diff --git a/src/tpk/step/step_create_symbolic_link.cc b/src/tpk/step/step_create_symbolic_link.cc
index 72b7855..921ece1 100644 (file)
--- a/src/tpk/step/step_create_symbolic_link.cc
+++ b/src/tpk/step/step_create_symbolic_link.cc
@@ -109,10 +109,18 @@ Status StepCreateSymbolicLink::undo() {
   manifest_x* m = context_->manifest_data.get();
   uiapplication_x *uiapp = m->uiapplication;
   serviceapplication_x *svcapp = m->serviceapplication;
-  if (!RemoveSymLink(uiapp, context_)) return Status::ERROR;
-  if (!RemoveSymLink(svcapp, context_)) return Status::ERROR;
 
-  return Status::OK;
+  Step::Status ret = Status::OK;
+  if (!RemoveSymLink(uiapp, context_)) {
+    LOG(ERROR) << "Cannot remove Symboliclink for uiapp";
+    ret = Status::ERROR;
+  }
+  if (!RemoveSymLink(svcapp, context_)) {
+    LOG(ERROR) << "Cannot remove Symboliclink for svcapp";
+    ret = Status::ERROR;
+  }
+
+  return ret;
 }
 
 }  // namespace filesystem