Rename Command::GetStatus into Command::GetState This name matches server side naming. BUG:24267885 Change-Id: I32a63bf9997544c55000a23af511e5ff905f8987 Reviewed-on: https://weave-review.googlesource.com/1266 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/include/weave/command.h b/libweave/include/weave/command.h index c09da32..1df24b8 100644 --- a/libweave/include/weave/command.h +++ b/libweave/include/weave/command.h
@@ -12,32 +12,32 @@ namespace weave { -enum class CommandStatus { - kQueued, - kInProgress, - kPaused, - kError, - kDone, - kCancelled, - kAborted, - kExpired, -}; - -enum class CommandOrigin { kLocal, kCloud }; - class Command { public: + enum class State { + kQueued, + kInProgress, + kPaused, + kError, + kDone, + kCancelled, + kAborted, + kExpired, + }; + + enum class Origin { kLocal, kCloud }; + // Returns the full command ID. virtual const std::string& GetID() const = 0; // Returns the full name of the command. virtual const std::string& GetName() const = 0; - // Returns the command status. - virtual CommandStatus GetStatus() const = 0; + // Returns the command state. + virtual Command::State GetState() const = 0; // Returns the origin of the command. - virtual CommandOrigin GetOrigin() const = 0; + virtual Command::Origin GetOrigin() const = 0; // Returns the command parameters. virtual std::unique_ptr<base::DictionaryValue> GetParameters() const = 0;
diff --git a/libweave/include/weave/test/mock_command.h b/libweave/include/weave/test/mock_command.h index c4324df..93fe1c2 100644 --- a/libweave/include/weave/test/mock_command.h +++ b/libweave/include/weave/test/mock_command.h
@@ -23,8 +23,8 @@ MOCK_CONST_METHOD0(GetID, const std::string&()); MOCK_CONST_METHOD0(GetName, const std::string&()); MOCK_CONST_METHOD0(GetCategory, const std::string&()); - MOCK_CONST_METHOD0(GetStatus, CommandStatus()); - MOCK_CONST_METHOD0(GetOrigin, CommandOrigin()); + MOCK_CONST_METHOD0(GetState, Command::State()); + MOCK_CONST_METHOD0(GetOrigin, Command::Origin()); MOCK_CONST_METHOD0(MockGetParameters, const std::string&()); MOCK_CONST_METHOD0(MockGetProgress, const std::string&()); MOCK_CONST_METHOD0(MockGetResults, const std::string&());
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc index 273162a..5d69b58 100644 --- a/libweave/src/base_api_handler.cc +++ b/libweave/src/base_api_handler.cc
@@ -52,8 +52,8 @@ auto command = cmd.lock(); if (!command) return; - CHECK(command->GetStatus() == CommandStatus::kQueued) - << EnumToString(command->GetStatus()); + CHECK(command->GetState() == Command::State::kQueued) + << EnumToString(command->GetState()); command->SetProgress(base::DictionaryValue{}, nullptr); const auto& settings = device_info_->GetSettings(); @@ -98,8 +98,8 @@ auto command = cmd.lock(); if (!command) return; - CHECK(command->GetStatus() == CommandStatus::kQueued) - << EnumToString(command->GetStatus()); + CHECK(command->GetState() == Command::State::kQueued) + << EnumToString(command->GetState()); command->SetProgress(base::DictionaryValue{}, nullptr); const auto& settings = device_info_->GetSettings();
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc index 8de1169..147fec1 100644 --- a/libweave/src/base_api_handler_unittest.cc +++ b/libweave/src/base_api_handler_unittest.cc
@@ -64,15 +64,15 @@ void AddCommand(const std::string& command) { auto command_instance = CommandInstance::FromJson( test::CreateDictionaryValue(command.c_str()).get(), - CommandOrigin::kLocal, command_manager_->GetCommandDictionary(), + Command::Origin::kLocal, command_manager_->GetCommandDictionary(), nullptr, nullptr); EXPECT_TRUE(!!command_instance); std::string id{base::IntToString(++command_id_)}; command_instance->SetID(id); command_manager_->AddCommand(std::move(command_instance)); - EXPECT_EQ(CommandStatus::kDone, - command_manager_->FindCommand(id)->GetStatus()); + EXPECT_EQ(Command::State::kDone, + command_manager_->FindCommand(id)->GetState()); } provider::test::MockConfigStore config_store_;
diff --git a/libweave/src/commands/cloud_command_proxy.cc b/libweave/src/commands/cloud_command_proxy.cc index 9d7a9d1..3826378 100644 --- a/libweave/src/commands/cloud_command_proxy.cc +++ b/libweave/src/commands/cloud_command_proxy.cc
@@ -49,10 +49,10 @@ QueueCommandUpdate(std::move(patch)); } -void CloudCommandProxy::OnStatusChanged() { +void CloudCommandProxy::OnStateChanged() { std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue}; patch->SetString(commands::attributes::kCommand_State, - EnumToString(command_instance_->GetStatus())); + EnumToString(command_instance_->GetState())); QueueCommandUpdate(std::move(patch)); }
diff --git a/libweave/src/commands/cloud_command_proxy.h b/libweave/src/commands/cloud_command_proxy.h index 12432df..86dc5d3 100644 --- a/libweave/src/commands/cloud_command_proxy.h +++ b/libweave/src/commands/cloud_command_proxy.h
@@ -43,7 +43,7 @@ void OnErrorChanged() override; void OnProgressChanged() override; void OnResultsChanged() override; - void OnStatusChanged() override; + void OnStateChanged() override; private: using UpdateID = StateChangeQueueInterface::UpdateID;
diff --git a/libweave/src/commands/cloud_command_proxy_unittest.cc b/libweave/src/commands/cloud_command_proxy_unittest.cc index f91e26c..c5863b6 100644 --- a/libweave/src/commands/cloud_command_proxy_unittest.cc +++ b/libweave/src/commands/cloud_command_proxy_unittest.cc
@@ -113,7 +113,7 @@ CHECK(command_json.get()); command_instance_ = - CommandInstance::FromJson(command_json.get(), CommandOrigin::kCloud, + CommandInstance::FromJson(command_json.get(), Command::Origin::kCloud, command_dictionary_, nullptr, nullptr); CHECK(command_instance_.get());
diff --git a/libweave/src/commands/command_instance.cc b/libweave/src/commands/command_instance.cc index e15b1b7..ac56eac 100644 --- a/libweave/src/commands/command_instance.cc +++ b/libweave/src/commands/command_instance.cc
@@ -22,20 +22,20 @@ namespace { -const EnumToStringMap<CommandStatus>::Map kMapStatus[] = { - {CommandStatus::kQueued, "queued"}, - {CommandStatus::kInProgress, "inProgress"}, - {CommandStatus::kPaused, "paused"}, - {CommandStatus::kError, "error"}, - {CommandStatus::kDone, "done"}, - {CommandStatus::kCancelled, "cancelled"}, - {CommandStatus::kAborted, "aborted"}, - {CommandStatus::kExpired, "expired"}, +const EnumToStringMap<Command::State>::Map kMapStatus[] = { + {Command::State::kQueued, "queued"}, + {Command::State::kInProgress, "inProgress"}, + {Command::State::kPaused, "paused"}, + {Command::State::kError, "error"}, + {Command::State::kDone, "done"}, + {Command::State::kCancelled, "cancelled"}, + {Command::State::kAborted, "aborted"}, + {Command::State::kExpired, "expired"}, }; -const EnumToStringMap<CommandOrigin>::Map kMapOrigin[] = { - {CommandOrigin::kLocal, "local"}, - {CommandOrigin::kCloud, "cloud"}, +const EnumToStringMap<Command::Origin>::Map kMapOrigin[] = { + {Command::Origin::kLocal, "local"}, + {Command::Origin::kCloud, "cloud"}, }; bool ReportDestroyedError(ErrorPtr* error) { @@ -46,8 +46,8 @@ } bool ReportInvalidStateTransition(ErrorPtr* error, - CommandStatus from, - CommandStatus to) { + Command::State from, + Command::State to) { Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, errors::commands::kInvalidState, "State switch impossible: '%s' -> '%s'", @@ -58,15 +58,15 @@ } // namespace template <> -LIBWEAVE_EXPORT EnumToStringMap<CommandStatus>::EnumToStringMap() +LIBWEAVE_EXPORT EnumToStringMap<Command::State>::EnumToStringMap() : EnumToStringMap(kMapStatus) {} template <> -LIBWEAVE_EXPORT EnumToStringMap<CommandOrigin>::EnumToStringMap() +LIBWEAVE_EXPORT EnumToStringMap<Command::Origin>::EnumToStringMap() : EnumToStringMap(kMapOrigin) {} CommandInstance::CommandInstance(const std::string& name, - CommandOrigin origin, + Command::Origin origin, const CommandDefinition* command_definition, const ValueMap& parameters) : name_{name}, @@ -88,11 +88,11 @@ return name_; } -CommandStatus CommandInstance::GetStatus() const { - return status_; +Command::State CommandInstance::GetState() const { + return state_; } -CommandOrigin CommandInstance::GetOrigin() const { +Command::Origin CommandInstance::GetOrigin() const { return origin_; } @@ -124,7 +124,7 @@ return false; // Change status even if progress unchanged, e.g. 0% -> 0%. - if (!SetStatus(CommandStatus::kInProgress, error)) + if (!SetStatus(State::kInProgress, error)) return false; if (obj != progress_) { @@ -151,7 +151,7 @@ FOR_EACH_OBSERVER(Observer, observers_, OnResultsChanged()); } // Change status even if result is unchanged. - bool result = SetStatus(CommandStatus::kDone, error); + bool result = SetStatus(State::kDone, error); RemoveFromQueue(); // The command will be destroyed after that, so do not access any members. return result; @@ -160,7 +160,7 @@ bool CommandInstance::SetError(const Error* command_error, ErrorPtr* error) { error_ = command_error ? command_error->Clone() : nullptr; FOR_EACH_OBSERVER(Observer, observers_, OnErrorChanged()); - return SetStatus(CommandStatus::kError, error); + return SetStatus(State::kError, error); } namespace { @@ -207,7 +207,7 @@ std::unique_ptr<CommandInstance> CommandInstance::FromJson( const base::Value* value, - CommandOrigin origin, + Command::Origin origin, const CommandDictionary& dictionary, std::string* command_id, ErrorPtr* error) { @@ -274,7 +274,7 @@ TypedValueToJson(progress_).release()); json->Set(commands::attributes::kCommand_Results, TypedValueToJson(results_).release()); - json->SetString(commands::attributes::kCommand_State, EnumToString(status_)); + json->SetString(commands::attributes::kCommand_State, EnumToString(state_)); if (error_) { json->Set(commands::attributes::kCommand_Error, ErrorInfoToJson(*error_).release()); @@ -292,44 +292,44 @@ } bool CommandInstance::Pause(ErrorPtr* error) { - return SetStatus(CommandStatus::kPaused, error); + return SetStatus(State::kPaused, error); } bool CommandInstance::Abort(const Error* command_error, ErrorPtr* error) { error_ = command_error ? command_error->Clone() : nullptr; FOR_EACH_OBSERVER(Observer, observers_, OnErrorChanged()); - bool result = SetStatus(CommandStatus::kAborted, error); + bool result = SetStatus(State::kAborted, error); RemoveFromQueue(); // The command will be destroyed after that, so do not access any members. return result; } bool CommandInstance::Cancel(ErrorPtr* error) { - bool result = SetStatus(CommandStatus::kCancelled, error); + bool result = SetStatus(State::kCancelled, error); RemoveFromQueue(); // The command will be destroyed after that, so do not access any members. return result; } -bool CommandInstance::SetStatus(CommandStatus status, ErrorPtr* error) { - if (status == status_) +bool CommandInstance::SetStatus(Command::State status, ErrorPtr* error) { + if (status == state_) return true; - if (status == CommandStatus::kQueued) - return ReportInvalidStateTransition(error, status_, status); - switch (status_) { - case CommandStatus::kDone: - case CommandStatus::kCancelled: - case CommandStatus::kAborted: - case CommandStatus::kExpired: - return ReportInvalidStateTransition(error, status_, status); - case CommandStatus::kQueued: - case CommandStatus::kInProgress: - case CommandStatus::kPaused: - case CommandStatus::kError: + if (status == State::kQueued) + return ReportInvalidStateTransition(error, state_, status); + switch (state_) { + case State::kDone: + case State::kCancelled: + case State::kAborted: + case State::kExpired: + return ReportInvalidStateTransition(error, state_, status); + case State::kQueued: + case State::kInProgress: + case State::kPaused: + case State::kError: break; } - status_ = status; - FOR_EACH_OBSERVER(Observer, observers_, OnStatusChanged()); + state_ = status; + FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged()); return true; }
diff --git a/libweave/src/commands/command_instance.h b/libweave/src/commands/command_instance.h index 843087c..5c8e80e 100644 --- a/libweave/src/commands/command_instance.h +++ b/libweave/src/commands/command_instance.h
@@ -37,7 +37,7 @@ virtual void OnErrorChanged() = 0; virtual void OnProgressChanged() = 0; virtual void OnResultsChanged() = 0; - virtual void OnStatusChanged() = 0; + virtual void OnStateChanged() = 0; protected: virtual ~Observer() = default; @@ -47,7 +47,7 @@ // be in format "<package_name>.<command_name>", a command |category| and // a list of parameters and their values specified in |parameters|. CommandInstance(const std::string& name, - CommandOrigin origin, + Command::Origin origin, const CommandDefinition* command_definition, const ValueMap& parameters); ~CommandInstance() override; @@ -55,8 +55,8 @@ // Command overrides. const std::string& GetID() const override; const std::string& GetName() const override; - CommandStatus GetStatus() const override; - CommandOrigin GetOrigin() const override; + Command::State GetState() const override; + Command::Origin GetOrigin() const override; std::unique_ptr<base::DictionaryValue> GetParameters() const override; std::unique_ptr<base::DictionaryValue> GetProgress() const override; std::unique_ptr<base::DictionaryValue> GetResults() const override; @@ -85,7 +85,7 @@ // This is used to report parse failures back to the server. static std::unique_ptr<CommandInstance> FromJson( const base::Value* value, - CommandOrigin origin, + Command::Origin origin, const CommandDictionary& dictionary, std::string* command_id, ErrorPtr* error); @@ -110,7 +110,7 @@ private: // Helper function to update the command status. // Used by Abort(), Cancel(), Done() methods. - bool SetStatus(CommandStatus status, ErrorPtr* error); + bool SetStatus(Command::State status, ErrorPtr* error); // Helper method that removes this command from the command queue. // Note that since the command queue owns the lifetime of the command instance // object, removing a command from the queue will also destroy it. @@ -121,7 +121,7 @@ // Full command name as "<package_name>.<command_name>". std::string name_; // The origin of the command, either "local" or "cloud". - CommandOrigin origin_ = CommandOrigin::kLocal; + Command::Origin origin_ = Command::Origin::kLocal; // Command definition. const CommandDefinition* command_definition_{nullptr}; // Command parameters and their values. @@ -130,8 +130,8 @@ ValueMap progress_; // Command results. ValueMap results_; - // Current command status. - CommandStatus status_ = CommandStatus::kQueued; + // Current command state. + Command::State state_ = Command::State::kQueued; // Error encountered during execution of the command. ErrorPtr error_; // Command observers.
diff --git a/libweave/src/commands/command_instance_unittest.cc b/libweave/src/commands/command_instance_unittest.cc index 7c2536b..f65353e 100644 --- a/libweave/src/commands/command_instance_unittest.cc +++ b/libweave/src/commands/command_instance_unittest.cc
@@ -76,7 +76,7 @@ params["phrase"] = str_prop.CreateValue(base::StringValue{"iPityDaFool"}, nullptr); params["volume"] = int_prop.CreateValue(base::FundamentalValue{5}, nullptr); - CommandInstance instance{"robot.speak", CommandOrigin::kCloud, + CommandInstance instance{"robot.speak", Command::Origin::kCloud, dict_.FindCommand("robot.speak"), params}; EXPECT_TRUE( @@ -84,21 +84,21 @@ EXPECT_EQ("", instance.GetID()); EXPECT_EQ("robot.speak", instance.GetName()); - EXPECT_EQ(CommandOrigin::kCloud, instance.GetOrigin()); + EXPECT_EQ(Command::Origin::kCloud, instance.GetOrigin()); EXPECT_JSON_EQ("{'phrase': 'iPityDaFool', 'volume': 5}", *instance.GetParameters()); EXPECT_JSON_EQ("{'foo': 239}", *instance.GetResults()); CommandInstance instance2{"base.reboot", - CommandOrigin::kLocal, + Command::Origin::kLocal, dict_.FindCommand("base.reboot"), {}}; - EXPECT_EQ(CommandOrigin::kLocal, instance2.GetOrigin()); + EXPECT_EQ(Command::Origin::kLocal, instance2.GetOrigin()); } TEST_F(CommandInstanceTest, SetID) { CommandInstance instance{"base.reboot", - CommandOrigin::kLocal, + Command::Origin::kLocal, dict_.FindCommand("base.reboot"), {}}; instance.SetID("command_id"); @@ -116,7 +116,7 @@ 'results': {} })"); std::string id; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, &id, nullptr); EXPECT_EQ("abcd", id); EXPECT_EQ("abcd", instance->GetID()); @@ -127,7 +127,7 @@ TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) { auto json = CreateDictionaryValue("{'name': 'base.reboot'}"); - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, nullptr); EXPECT_EQ("base.reboot", instance->GetName()); EXPECT_JSON_EQ("{}", *instance->GetParameters()); @@ -136,7 +136,7 @@ TEST_F(CommandInstanceTest, FromJson_NotObject) { auto json = CreateValue("'string'"); ErrorPtr error; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, &error); EXPECT_EQ(nullptr, instance.get()); EXPECT_EQ("json_object_expected", error->GetCode()); @@ -145,7 +145,7 @@ TEST_F(CommandInstanceTest, FromJson_NameMissing) { auto json = CreateDictionaryValue("{'param': 'value'}"); ErrorPtr error; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, &error); EXPECT_EQ(nullptr, instance.get()); EXPECT_EQ("parameter_missing", error->GetCode()); @@ -154,7 +154,7 @@ TEST_F(CommandInstanceTest, FromJson_UnknownCommand) { auto json = CreateDictionaryValue("{'name': 'robot.scream'}"); ErrorPtr error; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, &error); EXPECT_EQ(nullptr, instance.get()); EXPECT_EQ("invalid_command_name", error->GetCode()); @@ -166,7 +166,7 @@ 'parameters': 'hello' })"); ErrorPtr error; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, &error); EXPECT_EQ(nullptr, instance.get()); auto inner = error->GetInnerError(); @@ -183,7 +183,7 @@ } })"); ErrorPtr error; - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, &error); EXPECT_EQ(nullptr, instance.get()); auto first = error->GetFirstError(); @@ -202,7 +202,7 @@ }, 'results': {} })"); - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, nullptr); EXPECT_TRUE(instance->SetProgress(*CreateDictionaryValue("{'progress': 15}"), nullptr)); @@ -230,7 +230,7 @@ 'name': 'base.reboot', 'parameters': {} })"); - auto instance = CommandInstance::FromJson(json.get(), CommandOrigin::kCloud, + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, dict_, nullptr, nullptr); instance->SetID("testId");
diff --git a/libweave/src/commands/command_manager.cc b/libweave/src/commands/command_manager.cc index 5933146..7d17707 100644 --- a/libweave/src/commands/command_manager.cc +++ b/libweave/src/commands/command_manager.cc
@@ -114,8 +114,9 @@ UserRole role, std::string* id, ErrorPtr* error) { - auto command_instance = CommandInstance::FromJson( - &command, CommandOrigin::kLocal, GetCommandDictionary(), nullptr, error); + auto command_instance = + CommandInstance::FromJson(&command, Command::Origin::kLocal, + GetCommandDictionary(), nullptr, error); if (!command_instance) return false;
diff --git a/libweave/src/commands/command_queue.cc b/libweave/src/commands/command_queue.cc index efdcfdc..23b8f05 100644 --- a/libweave/src/commands/command_queue.cc +++ b/libweave/src/commands/command_queue.cc
@@ -32,7 +32,7 @@ << "Commands specific handler are not allowed after default one"; for (const auto& command : map_) { - if (command.second->GetStatus() == CommandStatus::kQueued && + if (command.second->GetState() == Command::State::kQueued && command.second->GetName() == command_name) { callback.Run(command.second); } @@ -43,7 +43,7 @@ } else { for (const auto& command : map_) { - if (command.second->GetStatus() == CommandStatus::kQueued && + if (command.second->GetState() == Command::State::kQueued && command_callbacks_.find(command.second->GetName()) == command_callbacks_.end()) { callback.Run(command.second);
diff --git a/libweave/src/commands/command_queue_unittest.cc b/libweave/src/commands/command_queue_unittest.cc index d119f74..1efea4e 100644 --- a/libweave/src/commands/command_queue_unittest.cc +++ b/libweave/src/commands/command_queue_unittest.cc
@@ -24,7 +24,7 @@ const std::string& name, const std::string& id) { std::unique_ptr<CommandInstance> cmd{new CommandInstance{ - name, CommandOrigin::kLocal, &command_definition_, {}}}; + name, Command::Origin::kLocal, &command_definition_, {}}}; cmd->SetID(id); return cmd; }
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc index 0e3d64f..e712130 100644 --- a/libweave/src/device_registration_info.cc +++ b/libweave/src/device_registration_info.cc
@@ -860,7 +860,7 @@ ErrorPtr error) { base::DictionaryValue command_patch; command_patch.SetString(commands::attributes::kCommand_State, - EnumToString(CommandStatus::kAborted)); + EnumToString(Command::State::kAborted)); if (error) { command_patch.Set(commands::attributes::kCommand_Error, ErrorInfoToJson(*error).release()); @@ -1080,8 +1080,8 @@ std::string command_id; ErrorPtr error; auto command_instance = CommandInstance::FromJson( - &command, CommandOrigin::kCloud, command_manager_->GetCommandDictionary(), - &command_id, &error); + &command, Command::Origin::kCloud, + command_manager_->GetCommandDictionary(), &command_id, &error); if (!command_instance) { LOG(WARNING) << "Failed to parse a command instance: " << command; if (!command_id.empty())