From 91a649ab62c9cf60ae9d02dd38c23fc23f98a1f3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 21 May 2018 16:05:56 -0700 Subject: [PATCH] updater: Add ABORT command. This will be used for testing purpose only, replacing the previously used "fail", to intentionally abort an update. As we're separating the logic between commands parsing and execution, "abort" needs to be considered as a valid command during the parsing. Test: recovery_unit_test and recovery_component_test on marlin. Change-Id: I47c41c423e62c41cc8515fd92f3c5959be08da02 --- tests/component/updater_test.cpp | 13 ++++++++++--- tests/unit/commands_test.cpp | 24 ++++++++++++++++++++++++ updater/blockimg.cpp | 7 +++++++ updater/commands.cpp | 17 ++++++++++++++++- updater/include/private/commands.h | 12 ++++++++++++ 5 files changed, 69 insertions(+), 4 deletions(-) diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index fe4f45e1..e8abb535 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -46,6 +46,7 @@ #include "otautil/paths.h" #include "otautil/print_sha1.h" #include "otautil/sysutil.h" +#include "private/commands.h" #include "updater/blockimg.h" #include "updater/install.h" #include "updater/updater.h" @@ -150,6 +151,9 @@ class UpdaterTest : public ::testing::Test { Paths::Get().set_last_command_file(temp_last_command_.path); Paths::Get().set_stash_directory_base(temp_stash_base_.path); + // Enable a special command "abort" to simulate interruption. + Command::abort_allowed_ = true; + last_command_file_ = temp_last_command_.path; image_file_ = image_temp_file_.path; } @@ -580,7 +584,7 @@ TEST_F(UpdaterTest, block_image_update_fail) { "2", "stash " + src_hash + " 2,0,2", "free " + src_hash, - "fail", + "abort", // clang-format on }; @@ -714,7 +718,7 @@ TEST_F(UpdaterTest, last_command_update) { "stash " + block1_hash + " 2,0,1", "move " + block1_hash + " 2,1,2 1 2,0,1", "stash " + block3_hash + " 2,2,3", - "fail", + "abort", // clang-format on }; @@ -859,6 +863,9 @@ class ResumableUpdaterTest : public testing::TestWithParam { Paths::Get().set_last_command_file(temp_last_command_.path); Paths::Get().set_stash_directory_base(temp_stash_base_.path); + // Enable a special command "abort" to simulate interruption. + Command::abort_allowed_ = true; + index_ = GetParam(); image_file_ = image_temp_file_.path; last_command_file_ = temp_last_command_.path; @@ -1030,7 +1037,7 @@ TEST_P(ResumableUpdaterTest, InterruptVerifyResume) { << g_transfer_list[kTransferListHeaderLines + index_] << ")"; std::vector transfer_list_copy{ g_transfer_list }; - transfer_list_copy[kTransferListHeaderLines + index_] = "fail"; + transfer_list_copy[kTransferListHeaderLines + index_] = "abort"; g_entries["transfer_list"] = android::base::Join(transfer_list_copy, '\n'); diff --git a/tests/unit/commands_test.cpp b/tests/unit/commands_test.cpp index cb2be917..3daa58f3 100644 --- a/tests/unit/commands_test.cpp +++ b/tests/unit/commands_test.cpp @@ -159,6 +159,27 @@ TEST(CommandsTest, Parse_EmptyInput) { ASSERT_EQ("invalid type", err); } +TEST(CommandsTest, Parse_ABORT_Allowed) { + Command::abort_allowed_ = true; + + const std::string input{ "abort" }; + std::string err; + Command command = Command::Parse(input, 0, &err); + ASSERT_TRUE(command); + + ASSERT_EQ(TargetInfo(), command.target()); + ASSERT_EQ(SourceInfo(), command.source()); + ASSERT_EQ(StashInfo(), command.stash()); + ASSERT_EQ(PatchInfo(), command.patch()); +} + +TEST(CommandsTest, Parse_ABORT_NotAllowed) { + const std::string input{ "abort" }; + std::string err; + Command command = Command::Parse(input, 0, &err); + ASSERT_FALSE(command); +} + TEST(CommandsTest, Parse_BSDIFF) { const std::string input{ "bsdiff 0 148 " @@ -312,9 +333,12 @@ TEST(CommandsTest, Parse_ZERO) { } TEST(CommandsTest, Parse_InvalidNumberOfArgs) { + Command::abort_allowed_ = true; + // Note that the case of having excess args in BSDIFF, IMGDIFF and MOVE is covered by // ParseTargetInfoAndSourceInfo_InvalidInput. std::vector inputs{ + "abort foo", "bsdiff", "erase", "erase 4,3,5,10,12 hash1", diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 937c5e14..fc713859 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1489,6 +1489,11 @@ static int PerformCommandErase(CommandParameters& params) { return 0; } +static int PerformCommandAbort(CommandParameters&) { + LOG(INFO) << "Aborting as instructed"; + return -1; +} + using CommandFunction = std::function; using CommandMap = std::unordered_map; @@ -1888,6 +1893,7 @@ Value* BlockImageVerifyFn(const char* name, State* state, // Commands which are not allowed are set to nullptr to skip them completely. const CommandMap command_map{ // clang-format off + { Command::Type::ABORT, PerformCommandAbort }, { Command::Type::BSDIFF, PerformCommandDiff }, { Command::Type::ERASE, nullptr }, { Command::Type::FREE, PerformCommandFree }, @@ -1908,6 +1914,7 @@ Value* BlockImageUpdateFn(const char* name, State* state, const std::vector>& argv) { const CommandMap command_map{ // clang-format off + { Command::Type::ABORT, PerformCommandAbort }, { Command::Type::BSDIFF, PerformCommandDiff }, { Command::Type::ERASE, PerformCommandErase }, { Command::Type::FREE, PerformCommandFree }, diff --git a/updater/commands.cpp b/updater/commands.cpp index fb19ebc9..e8814967 100644 --- a/updater/commands.cpp +++ b/updater/commands.cpp @@ -29,8 +29,16 @@ using namespace std::string_literals; +bool Command::abort_allowed_ = false; + Command::Type Command::ParseType(const std::string& type_str) { - if (type_str == "bsdiff") { + if (type_str == "abort") { + if (!abort_allowed_) { + LOG(ERROR) << "ABORT disallowed"; + return Type::LAST; + } + return Type::ABORT; + } else if (type_str == "bsdiff") { return Type::BSDIFF; } else if (type_str == "erase") { return Type::ERASE; @@ -237,6 +245,13 @@ Command Command::Parse(const std::string& line, size_t index, std::string* err) src_hash, &source_info, err)) { return {}; } + } else if (op == Type::ABORT) { + // No-op, other than sanity checking the input args. + if (pos != tokens.size()) { + *err = android::base::StringPrintf("invalid number of args: %zu (expected 0)", + tokens.size() - pos); + return {}; + } } else { *err = "invalid op"; return {}; diff --git a/updater/include/private/commands.h b/updater/include/private/commands.h index 784892fb..087d7cfb 100644 --- a/updater/include/private/commands.h +++ b/updater/include/private/commands.h @@ -213,9 +213,13 @@ class PatchInfo { // - Free the given stash data. // - Meaningful args: StashInfo // +// abort +// - Abort the current update. Allowed for testing code only. +// class Command { public: enum class Type { + ABORT, BSDIFF, ERASE, FREE, @@ -280,6 +284,11 @@ class Command { } private: + friend class ResumableUpdaterTest; + friend class UpdaterTest; + + FRIEND_TEST(CommandsTest, Parse_ABORT_Allowed); + FRIEND_TEST(CommandsTest, Parse_InvalidNumberOfArgs); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_InvalidInput); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_StashesOnly); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_SourceBlocksAndStashes); @@ -293,6 +302,9 @@ class Command { const std::string& src_hash, SourceInfo* source, std::string* err); + // Allows parsing ABORT command, which should be used for testing purpose only. + static bool abort_allowed_; + // The type of the command. Type type_{ Type::LAST }; // The index of the Command object, which is specified by the caller.