From: Pawel Sikorski <p.sikorski@samsung.com>
Date: Wed, 8 Apr 2015 15:00:07 +0000 (+0200)
Subject: Introducing "precheck" method to tpk steps (parse, create_symbolic_link)
X-Git-Tag: accepted/tizen/mobile/20150811.091449~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F85%2F37985%2F4;p=platform%2Fcore%2Fappfw%2Fapp-installers.git

Introducing "precheck" method to tpk steps (parse, create_symbolic_link)

This refactoring unifies input data checking into a separate method.

Change-Id: I100a262fbbcd6b2f128579283f9e67c673afae5b
---

diff --git a/src/tpk/step/step_create_symbolic_link.cc b/src/tpk/step/step_create_symbolic_link.cc
index 5fae12d..79ba465 100644
--- a/src/tpk/step/step_create_symbolic_link.cc
+++ b/src/tpk/step/step_create_symbolic_link.cc
@@ -106,22 +106,27 @@ bool RemoveSymLink(T *app, ContextInstaller* context) {
 
 }  // namespace
 
+Status StepCreateSymbolicLink::precheck() {
+  manifest_x *m = context_->manifest_data.get();
+  if (!m) {
+    LOG(ERROR) << "manifest_data attribute is empty";
+    return Step::Status::INVALID_VALUE;
+  }
+  if (!(m->uiapplication || m->serviceapplication)) {
+    LOG(ERROR) << "Neither ui-application nor service-application exists";
+    return Step::Status::ERROR;
+  }
+
+  return Step::Status::OK;
+}
 
 Status StepCreateSymbolicLink::process() {
   // Get manifest_x
   manifest_x *m = context_->manifest_data.get();
-  if (!m) {
-    LOG(ERROR) << "manifest_x is null";
-    return Status::ERROR;
-  }
 
   // get ui-app and service-app
   uiapplication_x *uiapp = m->uiapplication;
   serviceapplication_x *svcapp = m->serviceapplication;
-  if (!(uiapp || svcapp)) {
-    LOG(ERROR) << "Neither ui-application nor service-application exists";
-    return Status::ERROR;
-  }
 
   if (!CreateSymLink(uiapp, context_)) return Status::ERROR;
   if (!CreateSymLink(svcapp, context_)) return Status::ERROR;
diff --git a/src/tpk/step/step_create_symbolic_link.h b/src/tpk/step/step_create_symbolic_link.h
index f955447..1d50a5e 100644
--- a/src/tpk/step/step_create_symbolic_link.h
+++ b/src/tpk/step/step_create_symbolic_link.h
@@ -13,7 +13,7 @@ class StepCreateSymbolicLink : public common_installer::Step {
   Status process() override;
   Status clean() override { return Status::OK; }
   Status undo() override;
-  Status precheck() override { return Status::OK; }
+  Status precheck() override;
 };
 
 }  // namespace filesystem
diff --git a/src/tpk/step/step_parse.cc b/src/tpk/step/step_parse.cc
index 496f5b1..961b021 100644
--- a/src/tpk/step/step_parse.cc
+++ b/src/tpk/step/step_parse.cc
@@ -70,20 +70,41 @@ SCOPE_LOG_TAG(StepParse)
 typedef common_installer::Step::Status Status;
 using boost::filesystem::path;
 
+Status StepParse::precheck() {
+  if (context_->unpacked_dir_path.get().empty()) {
+      LOG(ERROR) << "unpacked_dir_path attribute is empty";
+      return Step::Status::INVALID_VALUE;
+  }
+  if (!boost::filesystem::exists(context_->unpacked_dir_path.get())) {
+    LOG(ERROR) << "unpacked_dir_path ("
+               << context_->unpacked_dir_path.get()
+               << ") path does not exist";
+    return Step::Status::INVALID_VALUE;
+  }
+
+  boost::filesystem::path tmp(context_->unpacked_dir_path.get());
+  tmp /= kManifestFileName;
+
+  if (!boost::filesystem::exists(tmp)) {
+    LOG(ERROR) << kManifestFileName << " not found from the package";
+    return Step::Status::INVALID_VALUE;
+  }
+
+  return Step::Status::OK;
+}
+
 /* process()
  * Parse tizen-manifest.xml and get the data from it
  * Store the data into the context_
  */
 Status StepParse::process() {
-  std::unique_ptr<boost::filesystem::path> mPath(
-      GetManifestFilePath(context_->unpacked_dir_path.get()));
-  if (!mPath) {
-    return Status::ERROR;
-  }
-  LOG(INFO) << "Parse " << mPath->c_str();
+  boost::filesystem::path mPath(context_->unpacked_dir_path.get());
+  mPath /= kManifestFileName;
+
+  LOG(INFO) << "Parse " << mPath.c_str();
 
   XmlParser parser;
-  std::unique_ptr<XmlTree> tree(parser.ParseAndGetNewTree(mPath->c_str()));
+  std::unique_ptr<XmlTree> tree(parser.ParseAndGetNewTree(mPath.c_str()));
   if (tree == nullptr) {
     LOG(ERROR) << "Failure on parsing xml";
     return Status::ERROR;
@@ -94,23 +115,6 @@ Status StepParse::process() {
   return Status::OK;
 }
 
-
-/* in parse() : Get manifest file path from the package unzipped directory
- */
-boost::filesystem::path* StepParse::GetManifestFilePath(
-    const boost::filesystem::path& dir) {
-  path* mPath = new path(dir);
-  *mPath /= kManifestFileName;
-
-  LOG(INFO) << "manifest file path: " << mPath->string();
-  if (!boost::filesystem::exists(*mPath)) {
-    LOG(ERROR) << kManifestFileName << " not found from the package";
-    return nullptr;
-  }
-  return mPath;  // object copy
-}
-
-
 /* Read manifest xml, and set up context_ object
  */
 bool StepParse::SetContextByManifestParser(XmlTree* tree) {
diff --git a/src/tpk/step/step_parse.h b/src/tpk/step/step_parse.h
index b808252..62ccfcb 100644
--- a/src/tpk/step/step_parse.h
+++ b/src/tpk/step/step_parse.h
@@ -2,7 +2,6 @@
 #ifndef TPK_STEP_STEP_PARSE_H_
 #define TPK_STEP_STEP_PARSE_H_
 
-#include <boost/filesystem.hpp>
 #include "common/step/step.h"
 #include "tpk/xml_parser/xml_parser.h"
 
@@ -14,13 +13,11 @@ class StepParse : public common_installer::Step {
   using Step::Step;
 
   Status process()  override;
-  Status clean()    override { return Status::OK; }
-  Status undo()     override { return Status::OK; }
-  Status precheck() override { return Status::OK; }
+  Status clean()    override { return Status::OK; };
+  Status undo()     override { return Status::OK; };
+  Status precheck() override;
 
  private:
-  boost::filesystem::path* GetManifestFilePath(
-      const boost::filesystem::path& dir);
   bool SetContextByManifestParser(xml_parser::XmlTree* tree);
   bool SetPkgInfoManifest(manifest_x* m,
       xml_parser::XmlTree* tree, xml_parser::XmlElement* manifest);