From 1576e56ba7c5c0c7838dc57b066ffadd1b6e4b02 Mon Sep 17 00:00:00 2001 From: Tomasz Iwanek Date: Thu, 6 Aug 2015 10:57:59 +0200 Subject: [PATCH] Refactoring for implementing smoke tests Tests will requires running multiple installations in one process. Changes: - removing singleton on PkgmgrInterface as it is not needed, - adding step will accept extra parameters so that step instance can be customized, - wgt static library extracted for linking with tests. Change-Id: I4c97c9484972f09d49ddf99e8c87d9a4fa447df0 --- CMakeLists.txt | 1 + src/common/app_installer.cc | 18 +++---- src/common/app_installer.h | 23 +++++++-- src/common/context_installer.cc | 1 + src/common/pkgmgr_interface.cc | 24 +++------ src/common/pkgmgr_interface.h | 14 +----- src/common/recovery_file.cc | 6 ++- src/common/recovery_file.h | 1 - src/common/step/step_configure.cc | 26 +++++----- src/common/step/step_configure.h | 5 +- src/common/step/step_recover_application.h | 1 + src/common/step/step_remove_temporary_directory.h | 1 + src/tpk/task.cc | 59 +++++++++-------------- src/tpk/task.h | 12 +++-- src/wgt/CMakeLists.txt | 11 +++-- src/wgt/wgt_backend.cc | 14 +++--- src/wgt/wgt_installer.cc | 16 +++--- src/wgt/wgt_installer.h | 2 +- 18 files changed, 119 insertions(+), 116 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff1c88d..94bb3e0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,6 +23,7 @@ SET(CMAKE_CXX_FLAGS_CCOV "-O0 -std=c++11 -g --coverage") # Targets SET(TARGET_LIBNAME_COMMON "common-installer") +SET(TARGET_LIBNAME_WGT "wgt-installer") SET(TARGET_WGT_BACKEND "wgt-backend") SET(TARGET_TPK_BACKEND "tpk-backend") diff --git a/src/common/app_installer.cc b/src/common/app_installer.cc index 7eba068..0216d0b 100644 --- a/src/common/app_installer.cc +++ b/src/common/app_installer.cc @@ -19,9 +19,9 @@ const unsigned kProgressRange = 100; namespace common_installer { -AppInstaller::AppInstaller(const char* package_type) - : context_(new ContextInstaller()) { - PkgMgrPtr pkgmgr = PkgMgrInterface::Instance(); +AppInstaller::AppInstaller(const char* package_type, PkgMgrPtr pkgmgr) + : pkgmgr_(pkgmgr), + context_(new ContextInstaller()) { pi_.reset(new PkgmgrSignal(pkgmgr.get()->GetRawPi())); // TODO(p.sikorski) below property is only used in AppInstaller. @@ -32,13 +32,13 @@ AppInstaller::AppInstaller(const char* package_type) AppInstaller::~AppInstaller() { } -int AppInstaller::Run() { +AppInstaller::Result AppInstaller::Run() { std::list>::iterator it(steps_.begin()); std::list>::iterator itStart(steps_.begin()); std::list>::iterator itEnd(steps_.end()); Step::Status process_status = Step::Status::OK; - int ret = 0; + Result ret = Result::OK; unsigned total_steps = steps_.size(); unsigned current_step = 1; @@ -56,7 +56,7 @@ int AppInstaller::Run() { if (process_status != Step::Status::OK) { LOG(ERROR) << "Error during processing"; - ret = -1; + ret = Result::ERROR; break; } @@ -70,15 +70,15 @@ int AppInstaller::Run() { LOG(ERROR) << "Failure occurs"; do { if ((*it)->undo() != Step::Status::OK) { - LOG(ERROR) << "Error during undo operation, but continuing...!"; - ret = -2; + LOG(ERROR) << "Error during undo operation, but continuing..."; + ret = Result::UNDO_ERROR; } } while (it-- != itStart); } else { for (auto& step : steps_) { if (step->clean() != Step::Status::OK) { LOG(ERROR) << "Error during clean operation"; - ret = -3; + ret = Result::CLEANUP_ERROR; break; } } diff --git a/src/common/app_installer.h b/src/common/app_installer.h index c584971..7571560 100644 --- a/src/common/app_installer.h +++ b/src/common/app_installer.h @@ -9,6 +9,7 @@ #include #include +#include "common/pkgmgr_interface.h" #include "common/pkgmgr_signal.h" #include "common/step/step.h" #include "common/utils/logging.h" @@ -18,19 +19,31 @@ namespace common_installer { class AppInstaller { public: - explicit AppInstaller(const char* package_type); + enum class Result { + OK, + ERROR, + CLEANUP_ERROR, + UNDO_ERROR, + }; + + explicit AppInstaller(const char* package_type, PkgMgrPtr pkgmgr); virtual ~AppInstaller(); // Adds new step to installer by specified type // Type of template parameter is used to create requested step class instance. // Context of installer is passed to step in this method // and is not being exposed outside installer. - template - void AddStep() { - steps_.push_back(std::unique_ptr(new StepT(context_.get()))); + // Step arguments are deduced and forwarded to constructor + template + void AddStep(Args&&... args) { + steps_.push_back(std::unique_ptr( + new StepT(context_.get(), std::forward(args)...))); } - int Run(); + Result Run(); + + protected: + PkgMgrPtr pkgmgr_; private: std::list> steps_; diff --git a/src/common/context_installer.cc b/src/common/context_installer.cc index 67cc3b3..f3450e4 100644 --- a/src/common/context_installer.cc +++ b/src/common/context_installer.cc @@ -4,6 +4,7 @@ // found in the LICENSE file. #include "common/context_installer.h" + #include #include diff --git a/src/common/pkgmgr_interface.cc b/src/common/pkgmgr_interface.cc index 7e7edf6..7191276 100644 --- a/src/common/pkgmgr_interface.cc +++ b/src/common/pkgmgr_interface.cc @@ -10,22 +10,14 @@ namespace common_installer { -PkgMgrPtr PkgMgrInterface::instance_; - -PkgMgrPtr PkgMgrInterface::Instance() { - return instance_; -} - -int PkgMgrInterface::Init(int argc, char** argv, AppQueryInterface* interface) { - if (instance_) - return 0; - - PkgMgrPtr tmp(new PkgMgrInterface(interface)); - int result = tmp->InitInternal(argc, argv); - - instance_ = tmp; - - return result; +PkgMgrPtr PkgMgrInterface::Create(int argc, char** argv, + AppQueryInterface* interface) { + PkgMgrPtr instance(new PkgMgrInterface(interface)); + int result = instance->InitInternal(argc, argv); + if (result != 0) + return nullptr; + + return instance; } int PkgMgrInterface::InitInternal(int argc, char** argv) { diff --git a/src/common/pkgmgr_interface.h b/src/common/pkgmgr_interface.h index 4912d7a..98a4ae0 100644 --- a/src/common/pkgmgr_interface.h +++ b/src/common/pkgmgr_interface.h @@ -34,18 +34,9 @@ class PkgMgrInterface { */ const char *GetRequestInfo() const; - /** Returns instance of PkgrMgr (Singleton pattern). - * - * However, Init method has to be called first (otherwise, this Instance - * returns nullptr). - * - * @see PkgMgr::Init(int argc, char** argv) - */ - static PkgMgrPtr Instance(); - - /** Initialize PkgMgrInterface. + /** Creates PkgMgrInterface. */ - static int Init(int argc, char** argv, + static PkgMgrPtr Create(int argc, char** argv, AppQueryInterface* interface = nullptr); /** Get Raw pointer to pkgmgr_installer object @@ -68,7 +59,6 @@ class PkgMgrInterface { pkgmgr_installer* pi_; bool is_app_installed_; AppQueryInterface* query_interface_; - static PkgMgrPtr instance_; SCOPE_LOG_TAG(PkgMgrInterface) DISALLOW_COPY_AND_ASSIGN(PkgMgrInterface); diff --git a/src/common/recovery_file.cc b/src/common/recovery_file.cc index cc8a6cb..1f44be6 100644 --- a/src/common/recovery_file.cc +++ b/src/common/recovery_file.cc @@ -72,11 +72,13 @@ RecoveryFile::RecoveryFile(const bf::path& path, bool load) return; } } else { - type_ = PkgMgrInterface::Instance()->GetRequestType(); - if (!WriteAndCommitFileContent()) { + // create file + FILE* handle = fopen(path.c_str(), "w"); + if (!handle) { path_.clear(); return; } + fclose(handle); LOG(DEBUG) << "Recovery file " << path_ << " created"; } } diff --git a/src/common/recovery_file.h b/src/common/recovery_file.h index dcafc8a..4c775c7 100644 --- a/src/common/recovery_file.h +++ b/src/common/recovery_file.h @@ -10,7 +10,6 @@ #include #include -#include "common/pkgmgr_interface.h" #include "common/request_type.h" namespace common_installer { diff --git a/src/common/step/step_configure.cc b/src/common/step/step_configure.cc index 5c1553d..9bd7409 100644 --- a/src/common/step/step_configure.cc +++ b/src/common/step/step_configure.cc @@ -7,7 +7,6 @@ #include #include -#include "common/pkgmgr_interface.h" #include "common/request_type.h" #include "common/utils/file_util.h" @@ -16,32 +15,35 @@ namespace configuration { const char *kStrEmpty = ""; -Step::Status StepConfigure::process() { - PkgMgrPtr pkgmgr = PkgMgrInterface::Instance(); +StepConfigure::StepConfigure(ContextInstaller* context, PkgMgrPtr pkgmgr) + : Step(context), + pkgmgr_(pkgmgr) { +} +Step::Status StepConfigure::process() { if (!SetupRootAppDirectory()) return Status::ERROR; - switch (pkgmgr->GetRequestType()) { + switch (pkgmgr_->GetRequestType()) { case RequestType::Install: - context_->file_path.set(pkgmgr->GetRequestInfo()); + context_->file_path.set(pkgmgr_->GetRequestInfo()); context_->pkgid.set(kStrEmpty); break; case RequestType::Update: - context_->file_path.set(pkgmgr->GetRequestInfo()); + context_->file_path.set(pkgmgr_->GetRequestInfo()); context_->pkgid.set(kStrEmpty); break; case RequestType::Uninstall: - context_->pkgid.set(pkgmgr->GetRequestInfo()); + context_->pkgid.set(pkgmgr_->GetRequestInfo()); context_->file_path.set(kStrEmpty); break; case RequestType::Reinstall: - context_->unpacked_dir_path.set(pkgmgr->GetRequestInfo()); + context_->unpacked_dir_path.set(pkgmgr_->GetRequestInfo()); context_->pkgid.set(kStrEmpty); context_->file_path.set(kStrEmpty); break; case RequestType::Recovery: - context_->file_path.set(pkgmgr->GetRequestInfo()); + context_->file_path.set(pkgmgr_->GetRequestInfo()); context_->pkgid.set(kStrEmpty); break; default: @@ -53,8 +55,8 @@ Step::Status StepConfigure::process() { } // Record recovery file for update and installation modes - if (pkgmgr->GetRequestType() == RequestType::Install || - pkgmgr->GetRequestType() == RequestType::Update) { + if (pkgmgr_->GetRequestType() == RequestType::Install || + pkgmgr_->GetRequestType() == RequestType::Update) { std::unique_ptr recovery_file = recovery::RecoveryFile::CreateRecoveryFileForPath( GenerateTemporaryPath( @@ -63,7 +65,7 @@ Step::Status StepConfigure::process() { LOG(ERROR) << "Failed to create recovery file"; return Status::ERROR; } - recovery_file->set_type(pkgmgr->GetRequestType()); + recovery_file->set_type(pkgmgr_->GetRequestType()); if (!recovery_file->WriteAndCommitFileContent()) { LOG(ERROR) << "Failed to write recovery file"; return Status::ERROR; diff --git a/src/common/step/step_configure.h b/src/common/step/step_configure.h index f7e3fb9..d7344d0 100644 --- a/src/common/step/step_configure.h +++ b/src/common/step/step_configure.h @@ -7,6 +7,7 @@ #include "common/context_installer.h" +#include "common/pkgmgr_interface.h" #include "common/step/step.h" #include "common/utils/logging.h" @@ -21,7 +22,7 @@ namespace configuration { */ class StepConfigure : public Step { public: - using Step::Step; + StepConfigure(ContextInstaller* context, PkgMgrPtr pkgmgr); Status process() override; Status clean() override; @@ -30,6 +31,8 @@ class StepConfigure : public Step { private: bool SetupRootAppDirectory(); + PkgMgrPtr pkgmgr_; + SCOPE_LOG_TAG(Configure) }; diff --git a/src/common/step/step_recover_application.h b/src/common/step/step_recover_application.h index 785fbd4..211362f 100644 --- a/src/common/step/step_recover_application.h +++ b/src/common/step/step_recover_application.h @@ -7,6 +7,7 @@ #include "common/context_installer.h" #include "common/step/step_recovery.h" +#include "common/utils/logging.h" namespace common_installer { namespace pkgmgr { diff --git a/src/common/step/step_remove_temporary_directory.h b/src/common/step/step_remove_temporary_directory.h index af31960..cb6b6c3 100644 --- a/src/common/step/step_remove_temporary_directory.h +++ b/src/common/step/step_remove_temporary_directory.h @@ -6,6 +6,7 @@ #define COMMON_STEP_STEP_REMOVE_TEMPORARY_DIRECTORY_H_ #include "common/step/step_recovery.h" +#include "common/utils/logging.h" namespace common_installer { namespace filesystem { diff --git a/src/tpk/task.cc b/src/tpk/task.cc index 5ca95ad..90c4681 100644 --- a/src/tpk/task.cc +++ b/src/tpk/task.cc @@ -4,7 +4,6 @@ #include "test/mock_pkgmgr_installer.h" #else #include "common/app_installer.h" -#include "common/pkgmgr_interface.h" #include "common/step/step_configure.h" #include "common/step/step_backup_icons.h" #include "common/step/step_backup_manifest.h" @@ -56,9 +55,9 @@ Task::Task() { bool Task::Init(int argc, char** argv) { - int result = ci::PkgMgrInterface::Init(argc, argv); - if (result != 0) { - LOG(ERROR) << "Cannot connect to PkgMgrInstaller"; + pkgmgr_ = ci::PkgMgrInterface::Create(argc, argv); + if (!pkgmgr_) { + LOG(ERROR) << "Options of pkgmgr installer cannot be parsed"; return false; } return true; @@ -66,38 +65,28 @@ bool Task::Init(int argc, char** argv) { bool Task::Run() { - int ret = 0; - switch (ci::PkgMgrInterface::Instance()->GetRequestType()) { + switch (pkgmgr_->GetRequestType()) { case ci::RequestType::Install: - ret = Install(); - break; + return Install(); case ci::RequestType::Update: - ret = Update(); - break; + return Update(); case ci::RequestType::Uninstall: - ret = Uninstall(); - break; + return Uninstall(); case ci::RequestType::Reinstall: - ret = Reinstall(); - break; + return Reinstall(); case ci::RequestType::Recovery: // TODO(t.iwanek): recovery mode invocation... - ret = EINVAL; - break; + return false; default: break; } - if (ret != 0) { - LOG(ERROR) << "Got error from AppInstaler: error code " << ret; - return false; - } - return true; + return false; } -int Task::Install() { - ci::AppInstaller ai(kPkgType); +bool Task::Install() { + ci::AppInstaller ai(kPkgType, pkgmgr_); - ai.AddStep(); + ai.AddStep(pkgmgr_); ai.AddStep(); ai.AddStep(); ai.AddStep(); @@ -109,13 +98,13 @@ int Task::Install() { ai.AddStep(); ai.AddStep(); - return ai.Run(); + return ai.Run() == ci::AppInstaller::Result::OK; } -int Task::Update() { - ci::AppInstaller ai(kPkgType); +bool Task::Update() { + ci::AppInstaller ai(kPkgType, pkgmgr_); - ai.AddStep(); + ai.AddStep(pkgmgr_); ai.AddStep(); ai.AddStep(); ai.AddStep(); @@ -131,13 +120,13 @@ int Task::Update() { ai.AddStep(); ai.AddStep(); - return ai.Run(); + return ai.Run() == ci::AppInstaller::Result::OK; } -int Task::Uninstall() { - ci::AppInstaller ai(kPkgType); +bool Task::Uninstall() { + ci::AppInstaller ai(kPkgType, pkgmgr_); - ai.AddStep(); + ai.AddStep(pkgmgr_); ai.AddStep(); ai.AddStep(); ai.AddStep(); @@ -145,11 +134,11 @@ int Task::Uninstall() { ai.AddStep(); ai.AddStep(); - return ai.Run(); + return ai.Run() == ci::AppInstaller::Result::OK; } -int Task::Reinstall() { - return 0; +bool Task::Reinstall() { + return false; } } // namespace tpk diff --git a/src/tpk/task.h b/src/tpk/task.h index d637aba..e3cfc12 100644 --- a/src/tpk/task.h +++ b/src/tpk/task.h @@ -7,6 +7,8 @@ #endif #include "common/utils/logging.h" +#include "common/pkgmgr_interface.h" + namespace tpk { class Task { @@ -17,10 +19,12 @@ class Task { bool Run(); private: - int Install(); - int Update(); - int Uninstall(); - int Reinstall(); + bool Install(); + bool Update(); + bool Uninstall(); + bool Reinstall(); + + common_installer::PkgMgrPtr pkgmgr_; SCOPE_LOG_TAG(TpkTask) }; // class Task diff --git a/src/wgt/CMakeLists.txt b/src/wgt/CMakeLists.txt index d3c95a9..ec050d8 100644 --- a/src/wgt/CMakeLists.txt +++ b/src/wgt/CMakeLists.txt @@ -14,7 +14,6 @@ SET(SRCS step/step_wgt_copy_storage_directories.cc step/step_wgt_resource_directory.cc wgt_app_query_interface.cc - wgt_backend.cc wgt_installer.cc ) @@ -26,11 +25,13 @@ ELSE(WRT_LAUNCHER) ENDIF(WRT_LAUNCHER) # Target - definition -ADD_EXECUTABLE(${TARGET_WGT_BACKEND} ${SRCS}) +ADD_LIBRARY(${TARGET_LIBNAME_WGT} STATIC ${SRCS}) +ADD_EXECUTABLE(${TARGET_WGT_BACKEND} "wgt_backend.cc") # Target - includes +TARGET_INCLUDE_DIRECTORIES(${TARGET_LIBNAME_WGT} PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../") TARGET_INCLUDE_DIRECTORIES(${TARGET_WGT_BACKEND} PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../") # Target - deps -APPLY_PKG_CONFIG(${TARGET_WGT_BACKEND} PUBLIC +APPLY_PKG_CONFIG(${TARGET_LIBNAME_WGT} PUBLIC MANIFEST_HANDLERS_DEPS MANIFEST_PARSER_DEPS PKGMGR_INSTALLER_DEPS @@ -38,7 +39,9 @@ APPLY_PKG_CONFIG(${TARGET_WGT_BACKEND} PUBLIC ) # Target - in-package deps -TARGET_LINK_LIBRARIES(${TARGET_WGT_BACKEND} PUBLIC ${TARGET_LIBNAME_COMMON}) +TARGET_LINK_LIBRARIES(${TARGET_LIBNAME_WGT} PUBLIC ${TARGET_LIBNAME_COMMON}) +TARGET_LINK_LIBRARIES(${TARGET_WGT_BACKEND} PRIVATE ${TARGET_LIBNAME_WGT}) # Install +INSTALL(TARGETS ${TARGET_LIBNAME_WGT} DESTINATION ${LIB_INSTALL_DIR}) INSTALL(TARGETS ${TARGET_WGT_BACKEND} DESTINATION ${BINDIR}) diff --git a/src/wgt/wgt_backend.cc b/src/wgt/wgt_backend.cc index 651d20f..32fb062 100644 --- a/src/wgt/wgt_backend.cc +++ b/src/wgt/wgt_backend.cc @@ -3,6 +3,8 @@ // Use of this source code is governed by a apache 2.0 license that can be // found in the LICENSE file. +#include + #include "common/pkgmgr_interface.h" #include "wgt/wgt_app_query_interface.h" #include "wgt/wgt_installer.h" @@ -11,11 +13,11 @@ namespace ci = common_installer; int main(int argc, char** argv) { wgt::WgtAppQueryInterface query_interface; - int result = ci::PkgMgrInterface::Init(argc, argv, &query_interface); - if (result) { - LOG(ERROR) << "Options cannot be parsed by PkgMgrInstaller"; - return -result; + auto pkgmgr = ci::PkgMgrInterface::Create(argc, argv, &query_interface); + if (!pkgmgr) { + LOG(ERROR) << "Options of pkgmgr installer cannot be parsed"; + return EINVAL; } - wgt::WgtInstaller installer; - return installer.Run(); + wgt::WgtInstaller installer(pkgmgr); + return (installer.Run() == ci::AppInstaller::Result::OK) ? 0 : 1; } diff --git a/src/wgt/wgt_installer.cc b/src/wgt/wgt_installer.cc index e32d59a..8894f30 100644 --- a/src/wgt/wgt_installer.cc +++ b/src/wgt/wgt_installer.cc @@ -51,12 +51,12 @@ namespace ci = common_installer; namespace wgt { -WgtInstaller::WgtInstaller() : AppInstaller("wgt") { - ci::PkgMgrPtr pkgmgr = ci::PkgMgrInterface::Instance(); +WgtInstaller::WgtInstaller(ci::PkgMgrPtr pkgrmgr) + : AppInstaller("wgt", pkgrmgr) { /* treat the request */ - switch (pkgmgr->GetRequestType()) { + switch (pkgmgr_->GetRequestType()) { case ci::RequestType::Install : { - AddStep(); + AddStep(pkgmgr_); AddStep(); AddStep(); AddStep(); @@ -72,7 +72,7 @@ WgtInstaller::WgtInstaller() : AppInstaller("wgt") { break; } case ci::RequestType::Update: { - AddStep(); + AddStep(pkgmgr_); AddStep(); AddStep(); AddStep(); @@ -92,7 +92,7 @@ WgtInstaller::WgtInstaller() : AppInstaller("wgt") { break; } case ci::RequestType::Uninstall: { - AddStep(); + AddStep(pkgmgr_); AddStep(); AddStep(); AddStep(); @@ -103,7 +103,7 @@ WgtInstaller::WgtInstaller() : AppInstaller("wgt") { break; } case ci::RequestType::Reinstall: { - AddStep(); + AddStep(pkgmgr_); AddStep(); AddStep(); AddStep(); @@ -112,7 +112,7 @@ WgtInstaller::WgtInstaller() : AppInstaller("wgt") { break; } case ci::RequestType::Recovery: { - AddStep(); + AddStep(pkgmgr_); AddStep(); AddStep(); AddStep(); diff --git a/src/wgt/wgt_installer.h b/src/wgt/wgt_installer.h index 4d88edd..da3bec4 100644 --- a/src/wgt/wgt_installer.h +++ b/src/wgt/wgt_installer.h @@ -18,7 +18,7 @@ namespace wgt { */ class WgtInstaller : public common_installer::AppInstaller { public: - WgtInstaller(); + explicit WgtInstaller(common_installer::PkgMgrPtr pkgrmgr); }; } // namespace wgt -- 2.7.4