Revert "Merge remote-tracking branch 'weave/master' into 'weave/aosp-master'" This reverts commit 5734b2f40e6a4487efded994a82fb5a6fd3680a0. Change-Id: I8aefb2ffb039de67da7a7664c07f6e35b56adbc2 Reviewed-on: https://weave-review.googlesource.com/1636 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/Android.mk b/Android.mk new file mode 100644 index 0000000..9833c4b --- /dev/null +++ b/Android.mk
@@ -0,0 +1,230 @@ +# Copyright (C) 2015 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +LOCAL_PATH := $(call my-dir) + +# Common variables +# ======================================================== + +libweaveCommonCppExtension := .cc +libweaveCommonCFlags := -Wall -Werror \ + -Wno-char-subscripts -Wno-missing-field-initializers \ + -Wno-unused-function -Wno-unused-parameter + +libweaveCommonCppFlags := \ + -Wno-deprecated-register \ + -Wno-sign-compare \ + -Wno-sign-promo \ + -Wno-non-virtual-dtor \ + +libweaveCommonCIncludes := \ + $(LOCAL_PATH)/.. \ + $(LOCAL_PATH)/include \ + $(LOCAL_PATH)/third_party/modp_b64/modp_b64 \ + external/gtest/include \ + +libweaveSharedLibraries := \ + libchrome \ + libexpat \ + libcrypto \ + +# libweave-external +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libweave-external +LOCAL_CPP_EXTENSION := $(libweaveCommonCppExtension) +LOCAL_CFLAGS := $(libweaveCommonCFlags) +LOCAL_CPPFLAGS := $(libweaveCommonCppFlags) +LOCAL_C_INCLUDES := $(libweaveCommonCIncludes) +LOCAL_SHARED_LIBRARIES := $(libweaveSharedLibraries) +LOCAL_STATIC_LIBRARIES := +LOCAL_RTTI_FLAG := -frtti +LOCAL_CLANG := true +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/external + +LOCAL_SRC_FILES := \ + third_party/chromium/crypto/p224.cc \ + third_party/chromium/crypto/p224_spake.cc \ + third_party/chromium/crypto/sha2.cc \ + third_party/modp_b64/modp_b64.cc \ + +include $(BUILD_STATIC_LIBRARY) + +# libweave-common +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libweave-common +LOCAL_CPP_EXTENSION := $(libweaveCommonCppExtension) +LOCAL_CFLAGS := $(libweaveCommonCFlags) +LOCAL_CPPFLAGS := $(libweaveCommonCppFlags) +LOCAL_C_INCLUDES := $(libweaveCommonCIncludes) +LOCAL_SHARED_LIBRARIES := $(libweaveSharedLibraries) +LOCAL_STATIC_LIBRARIES := libweave-external +LOCAL_RTTI_FLAG := -frtti +LOCAL_CLANG := true +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH) + +LOCAL_SRC_FILES := \ + src/backoff_entry.cc \ + src/base_api_handler.cc \ + src/commands/cloud_command_proxy.cc \ + src/commands/command_definition.cc \ + src/commands/command_dictionary.cc \ + src/commands/command_instance.cc \ + src/commands/command_manager.cc \ + src/commands/command_queue.cc \ + src/commands/object_schema.cc \ + src/commands/prop_constraints.cc \ + src/commands/prop_types.cc \ + src/commands/prop_values.cc \ + src/commands/schema_constants.cc \ + src/commands/schema_utils.cc \ + src/config.cc \ + src/data_encoding.cc \ + src/device_manager.cc \ + src/device_registration_info.cc \ + src/error.cc \ + src/http_constants.cc \ + src/json_error_codes.cc \ + src/notification/notification_parser.cc \ + src/notification/pull_channel.cc \ + src/notification/xml_node.cc \ + src/notification/xmpp_channel.cc \ + src/notification/xmpp_iq_stanza_handler.cc \ + src/notification/xmpp_stream_parser.cc \ + src/privet/cloud_delegate.cc \ + src/privet/constants.cc \ + src/privet/device_delegate.cc \ + src/privet/device_ui_kind.cc \ + src/privet/openssl_utils.cc \ + src/privet/privet_handler.cc \ + src/privet/privet_manager.cc \ + src/privet/privet_types.cc \ + src/privet/publisher.cc \ + src/privet/security_manager.cc \ + src/privet/wifi_bootstrap_manager.cc \ + src/privet/wifi_ssid_generator.cc \ + src/registration_status.cc \ + src/states/error_codes.cc \ + src/states/state_change_queue.cc \ + src/states/state_manager.cc \ + src/states/state_package.cc \ + src/string_utils.cc \ + src/utils.cc \ + +include $(BUILD_STATIC_LIBRARY) + +# libweave-test +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libweave-test +LOCAL_CPP_EXTENSION := $(libweaveCommonCppExtension) +LOCAL_CFLAGS := $(libweaveCommonCFlags) +LOCAL_CPPFLAGS := $(libweaveCommonCppFlags) +LOCAL_C_INCLUDES := \ + $(libweaveCommonCIncludes) \ + external/gmock/include \ + +LOCAL_SHARED_LIBRARIES := $(libweaveSharedLibraries) +LOCAL_STATIC_LIBRARIES := libgtest libgmock +LOCAL_CLANG := true +LOCAL_RTTI_FLAG := -frtti +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include + +LOCAL_SRC_FILES := \ + src/test/fake_stream.cc \ + src/test/fake_task_runner.cc \ + src/test/mock_command.cc \ + src/test/unittest_utils.cc \ + +include $(BUILD_STATIC_LIBRARY) + +# libweave +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libweave +LOCAL_CPP_EXTENSION := $(libweaveCommonCppExtension) +LOCAL_CFLAGS := $(libweaveCommonCFlags) +LOCAL_CPPFLAGS := $(libweaveCommonCppFlags) +LOCAL_C_INCLUDES := $(libweaveCommonCIncludes) +LOCAL_SHARED_LIBRARIES := $(libweaveSharedLibraries) +LOCAL_WHOLE_STATIC_LIBRARIES := libweave-external libweave-common +LOCAL_CLANG := true +LOCAL_RTTI_FLAG := -frtti +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include + +LOCAL_SRC_FILES := \ + src/empty.cc + +include $(BUILD_SHARED_LIBRARY) + +# libweave_test +# ======================================================== +include $(CLEAR_VARS) +LOCAL_MODULE := libweave_test +LOCAL_MODULE_TAGS := debug +LOCAL_CPP_EXTENSION := $(libweaveCommonCppExtension) +LOCAL_CFLAGS := $(libweaveCommonCFlags) +LOCAL_CPPFLAGS := $(libweaveCommonCppFlags) +LOCAL_C_INCLUDES := \ + $(libweaveCommonCIncludes) \ + external/gmock/include \ + +LOCAL_SHARED_LIBRARIES := \ + $(libweaveSharedLibraries) \ + +LOCAL_STATIC_LIBRARIES := \ + libweave-external \ + libweave-common \ + libweave-test \ + libgtest libgmock \ + libchrome_test_helpers \ + +LOCAL_CLANG := true +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH) + +LOCAL_SRC_FILES := \ + src/backoff_entry_unittest.cc \ + src/base_api_handler_unittest.cc \ + src/commands/cloud_command_proxy_unittest.cc \ + src/commands/command_definition_unittest.cc \ + src/commands/command_dictionary_unittest.cc \ + src/commands/command_instance_unittest.cc \ + src/commands/command_manager_unittest.cc \ + src/commands/command_queue_unittest.cc \ + src/commands/object_schema_unittest.cc \ + src/commands/schema_utils_unittest.cc \ + src/config_unittest.cc \ + src/data_encoding_unittest.cc \ + src/device_registration_info_unittest.cc \ + src/error_unittest.cc \ + src/notification/notification_parser_unittest.cc \ + src/notification/xml_node_unittest.cc \ + src/notification/xmpp_channel_unittest.cc \ + src/notification/xmpp_iq_stanza_handler_unittest.cc \ + src/notification/xmpp_stream_parser_unittest.cc \ + src/privet/privet_handler_unittest.cc \ + src/privet/security_manager_unittest.cc \ + src/privet/wifi_ssid_generator_unittest.cc \ + src/states/state_change_queue_unittest.cc \ + src/states/state_manager_unittest.cc \ + src/states/state_package_unittest.cc \ + src/string_utils_unittest.cc \ + src/test/weave_testrunner.cc \ + src/weave_unittest.cc \ + third_party/chromium/crypto/p224_spake_unittest.cc \ + third_party/chromium/crypto/p224_unittest.cc \ + third_party/chromium/crypto/sha2_unittest.cc \ + +include $(BUILD_NATIVE_TEST)
diff --git a/examples/daemon/ledflasher/ledflasher.cc b/examples/daemon/ledflasher/ledflasher.cc index 0019d2c..38314f5 100644 --- a/examples/daemon/ledflasher/ledflasher.cc +++ b/examples/daemon/ledflasher/ledflasher.cc
@@ -25,7 +25,7 @@ device_ = device; device->AddStateDefinitionsFromJson(R"({ - "_ledflasher": {"_leds": {"type": "array", "items": {"type": "boolean"}}} + "_ledflasher": {"_leds": {"items": "boolean"}} })"); device->SetStatePropertiesFromJson(R"({ @@ -37,13 +37,13 @@ "_ledflasher": { "_set":{ "parameters": { - "_led": {"type": "integer", "minimum": 1, "maximum": 3}, - "_on": {"type": "boolean"} + "_led": {"minimum": 1, "maximum": 3}, + "_on": "boolean" } }, "_toggle":{ "parameters": { - "_led": {"type": "integer", "minimum": 1, "maximum": 3} + "_led": {"minimum": 1, "maximum": 3} } } }
diff --git a/examples/daemon/light/light.cc b/examples/daemon/light/light.cc index a7eb9b3..d54de93 100644 --- a/examples/daemon/light/light.cc +++ b/examples/daemon/light/light.cc
@@ -18,31 +18,31 @@ device_ = device; device->AddStateDefinitionsFromJson(R"({ - "onOff": {"state": {"type": "string", "enum": ["on", "standby"]}}, - "brightness": {"brightness": {"type": "integer"}}, + "onOff": {"state": ["on", "standby"]}, + "brightness": {"brightness": "integer"}, "colorXY": { "colorSetting": { "properties": { - "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0}, - "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0} + "colorX": {"minimum": 0.0, "maximum": 1.0}, + "colorY": {"minimum": 0.0, "maximum": 1.0} } }, "colorCapRed": { "properties": { - "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0}, - "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0} + "colorX": {"minimum": 0.0, "maximum": 1.0}, + "colorY": {"minimum": 0.0, "maximum": 1.0} } }, "colorCapGreen": { "properties": { - "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0}, - "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0} + "colorX": {"minimum": 0.0, "maximum": 1.0}, + "colorY": {"minimum": 0.0, "maximum": 1.0} } }, "colorCapBlue": { "properties": { - "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0}, - "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0} + "colorX": {"minimum": 0.0, "maximum": 1.0}, + "colorY": {"minimum": 0.0, "maximum": 1.0} } } } @@ -64,7 +64,7 @@ "onOff": { "setConfig":{ "parameters": { - "state": {"type": "string", "enum": ["on", "standby"]} + "state": ["on", "standby"] } } },
diff --git a/examples/daemon/lock/lock.cc b/examples/daemon/lock/lock.cc index 7d941c6..3014fb1 100644 --- a/examples/daemon/lock/lock.cc +++ b/examples/daemon/lock/lock.cc
@@ -35,11 +35,8 @@ device->AddStateDefinitionsFromJson(R"({ "lock": { - "lockedState": { - "type": "string", - "enum": ["locked", "unlocked", "partiallyLocked"], - } - "isLockingSupported": {"type": "boolean"}} + "lockedState": ["locked", "unlocked", "partiallyLocked"], + "isLockingSupported": "boolean"} })"); device->SetStatePropertiesFromJson(R"({ @@ -54,7 +51,7 @@ "lock": { "setConfig":{ "parameters": { - "lockedState": {"type": "string", "enum":["locked", "unlocked"]} + "lockedState": ["locked", "unlocked"] } } }
diff --git a/examples/daemon/sample/sample.cc b/examples/daemon/sample/sample.cc index e065b06..905a977 100644 --- a/examples/daemon/sample/sample.cc +++ b/examples/daemon/sample/sample.cc
@@ -27,9 +27,9 @@ "_hello": { "minimalRole": "user", "parameters": { - "_name": {"type": "string"} + "_name": "string" }, - "results": { "_reply": {"type": "string"} } + "results": { "_reply": "string" } }, "_ping": { "minimalRole": "user", @@ -38,16 +38,16 @@ "_countdown": { "minimalRole": "user", "parameters": { - "_seconds": {"type": "integer", "minimum": 1, "maximum": 25} + "_seconds": {"minimum": 1, "maximum": 25} }, - "progress": { "_seconds_left": {"type": "integer"}}, + "progress": { "_seconds_left": "integer"}, "results": {} } } })"); device->AddStateDefinitionsFromJson(R"({ - "_sample": {"_ping_count": {"type": "integer"}} + "_sample": {"_ping_count":"integer"} })"); device->SetStatePropertiesFromJson(R"({
diff --git a/examples/daemon/speaker/speaker.cc b/examples/daemon/speaker/speaker.cc index 178be14..32591f9 100644 --- a/examples/daemon/speaker/speaker.cc +++ b/examples/daemon/speaker/speaker.cc
@@ -18,10 +18,10 @@ device_ = device; device->AddStateDefinitionsFromJson(R"({ - "onOff": {"state": {"type": "string", "enum": ["on", "standby"]}}, + "onOff": {"state": ["on", "standby"]}, "volume": { - "volume": {"type": "integer"}, - "isMuted": {"type": "boolean"} + "volume": "integer", + "isMuted": "boolean" } })"); @@ -38,7 +38,7 @@ "onOff": { "setConfig":{ "parameters": { - "state": {"type": "string", "enum": ["on", "standby"]} + "state": ["on", "standby"] } } }, @@ -50,7 +50,7 @@ "minimum": 0, "maximum": 100 }, - "isMuted": {"type": "boolean"} + "isMuted": "boolean" } } }
diff --git a/libweave_common.gypi b/libweave_common.gypi index 73d02fd..49f7ff9 100644 --- a/libweave_common.gypi +++ b/libweave_common.gypi
@@ -31,7 +31,6 @@ ], 'cflags!': ['-fPIE'], 'cflags': [ - '-fno-exceptions', '-fPIC', '-fvisibility=hidden', '-std=c++11',
diff --git a/src/base_api_handler.cc b/src/base_api_handler.cc index 3d22a10..c3aa616 100644 --- a/src/base_api_handler.cc +++ b/src/base_api_handler.cc
@@ -42,31 +42,20 @@ "updateBaseConfiguration": { "minimalRole": "manager", "parameters": { - "localAnonymousAccessMaxRole": { - "enum": [ "none", "viewer", "user" ], - "type": "string" - }, - "localDiscoveryEnabled": { - "type": "boolean" - }, - "localPairingEnabled": { - "type": "boolean" - } - } + "localDiscoveryEnabled": "boolean", + "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ], + "localPairingEnabled": "boolean" + }, + "results": {} }, "updateDeviceInfo": { "minimalRole": "manager", "parameters": { - "description": { - "type": "string" - }, - "location": { - "type": "string" - }, - "name": { - "type": "string" - } - } + "description": "string", + "name": "string", + "location": "string" + }, + "results": {} } } })");
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc index df78319..a025e44 100644 --- a/src/base_api_handler_unittest.cc +++ b/src/base_api_handler_unittest.cc
@@ -31,7 +31,7 @@ class BaseApiHandlerTest : public ::testing::Test { protected: void SetUp() override { - EXPECT_CALL(mock_state_change_queue_, MockNotifyPropertiesUpdated(_, _)) + EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_, _)) .WillRepeatedly(Return(true)); command_manager_ = std::make_shared<CommandManager>(); @@ -107,38 +107,39 @@ TEST_F(BaseApiHandlerTest, Initialization) { auto command_defs = - command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr); + command_manager_->GetCommandDictionary().GetCommandsAsJson( + [](const CommandDefinition* def) { return true; }, true, nullptr); auto expected = R"({ "base": { "updateBaseConfiguration": { - "minimalRole": "manager", - "parameters": { - "localAnonymousAccessMaxRole": { - "enum": [ "none", "viewer", "user" ], - "type": "string" - }, - "localDiscoveryEnabled": { - "type": "boolean" - }, - "localPairingEnabled": { - "type": "boolean" - } - } + "minimalRole": "manager", + "parameters": { + "localAnonymousAccessMaxRole": { + "enum": [ "none", "viewer", "user" ], + "type": "string" + }, + "localDiscoveryEnabled": { + "type": "boolean" + }, + "localPairingEnabled": { + "type": "boolean" + } + } }, "updateDeviceInfo": { - "minimalRole": "manager", - "parameters": { - "description": { - "type": "string" - }, - "location": { - "type": "string" - }, - "name": { - "type": "string" - } - } + "minimalRole": "manager", + "parameters": { + "description": { + "type": "string" + }, + "location": { + "type": "string" + }, + "name": { + "type": "string" + } + } } } })";
diff --git a/src/commands/cloud_command_proxy_unittest.cc b/src/commands/cloud_command_proxy_unittest.cc index fdb22fc..0c04592 100644 --- a/src/commands/cloud_command_proxy_unittest.cc +++ b/src/commands/cloud_command_proxy_unittest.cc
@@ -94,7 +94,7 @@ } })"); CHECK(json.get()); - CHECK(command_dictionary_.LoadCommands(*json, nullptr)) + CHECK(command_dictionary_.LoadCommands(*json, nullptr, nullptr)) << "Failed to parse test command dictionary"; CreateCommandInstance();
diff --git a/src/commands/command_definition.cc b/src/commands/command_definition.cc index dbae630..d7ebc83 100644 --- a/src/commands/command_definition.cc +++ b/src/commands/command_definition.cc
@@ -28,31 +28,61 @@ LIBWEAVE_EXPORT EnumToStringMap<UserRole>::EnumToStringMap() : EnumToStringMap(kMap) {} -CommandDefinition::CommandDefinition(const base::DictionaryValue& definition, - UserRole minimal_role) - : minimal_role_{minimal_role} { - definition_.MergeDictionary(&definition); +bool CommandDefinition::Visibility::FromString(const std::string& str, + ErrorPtr* error) { + // This special case is useful for places where we want to make a command + // to ALL clients, even if new clients are added in the future. + if (str == commands::attributes::kCommand_Visibility_All) { + local = true; + cloud = true; + return true; + } + + // Clear any bits first. + local = false; + cloud = false; + if (str == commands::attributes::kCommand_Visibility_None) + return true; + + for (const std::string& value : Split(str, ",", true, true)) { + if (value == commands::attributes::kCommand_Visibility_Local) { + local = true; + } else if (value == commands::attributes::kCommand_Visibility_Cloud) { + cloud = true; + } else { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidPropValue, + "Invalid command visibility value '%s'", + value.c_str()); + return false; + } + } + return true; } -std::unique_ptr<CommandDefinition> CommandDefinition::FromJson( - const base::DictionaryValue& dict, ErrorPtr* error) { - std::unique_ptr<CommandDefinition> definition; - // Validate the 'minimalRole' value if present. That's the only thing we - // care about so far. - std::string value; - UserRole minimal_role; - if (dict.GetString(commands::attributes::kCommand_Role, &value)) { - if (!StringToEnum(value, &minimal_role)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, - errors::commands::kInvalidPropValue, - "Invalid role: '%s'", value.c_str()); - return definition; - } - } else { - minimal_role = UserRole::kUser; - } - definition.reset(new CommandDefinition{dict, minimal_role}); - return definition; +std::string CommandDefinition::Visibility::ToString() const { + if (local && cloud) + return commands::attributes::kCommand_Visibility_All; + if (!local && !cloud) + return commands::attributes::kCommand_Visibility_None; + if (local) + return commands::attributes::kCommand_Visibility_Local; + return commands::attributes::kCommand_Visibility_Cloud; +} + +CommandDefinition::CommandDefinition( + std::unique_ptr<const ObjectSchema> parameters, + std::unique_ptr<const ObjectSchema> progress, + std::unique_ptr<const ObjectSchema> results) + : parameters_{std::move(parameters)}, + progress_{std::move(progress)}, + results_{std::move(results)} { + // Set to be available to all clients by default. + visibility_ = Visibility::GetAll(); +} + +void CommandDefinition::SetVisibility(const Visibility& visibility) { + visibility_ = visibility; } } // namespace weave
diff --git a/src/commands/command_definition.h b/src/commands/command_definition.h index da02bf5..3bcc07f 100644 --- a/src/commands/command_definition.h +++ b/src/commands/command_definition.h
@@ -9,8 +9,8 @@ #include <string> #include <base/macros.h> -#include <base/values.h> -#include <weave/error.h> + +#include "src/commands/object_schema.h" namespace weave { @@ -25,19 +25,55 @@ // describing the command parameter types and constraints. class CommandDefinition final { public: - // Factory method to construct a command definition from a JSON dictionary. - static std::unique_ptr<CommandDefinition> FromJson( - const base::DictionaryValue& dict, ErrorPtr* error); - const base::DictionaryValue& ToJson() const { return definition_; } + struct Visibility { + Visibility() = default; + Visibility(bool is_local, bool is_cloud) + : local{is_local}, cloud{is_cloud} {} + + // Converts a comma-separated string of visibility identifiers into the + // Visibility bitset (|str| is a string like "local,cloud"). + // Special string value "all" is treated as a list of every possible + // visibility values and "none" to have all the bits cleared. + bool FromString(const std::string& str, ErrorPtr* error); + + // Converts the visibility bitset to a string. + std::string ToString() const; + + static Visibility GetAll() { return Visibility{true, true}; } + static Visibility GetLocal() { return Visibility{true, false}; } + static Visibility GetCloud() { return Visibility{false, true}; } + static Visibility GetNone() { return Visibility{false, false}; } + + bool local{false}; // Command is available to local clients. + bool cloud{false}; // Command is available to cloud clients. + }; + + CommandDefinition(std::unique_ptr<const ObjectSchema> parameters, + std::unique_ptr<const ObjectSchema> progress, + std::unique_ptr<const ObjectSchema> results); + + // Gets the object schema for command parameters. + const ObjectSchema* GetParameters() const { return parameters_.get(); } + // Gets the object schema for command progress. + const ObjectSchema* GetProgress() const { return progress_.get(); } + // Gets the object schema for command results. + const ObjectSchema* GetResults() const { return results_.get(); } + // Returns the command visibility. + const Visibility& GetVisibility() const { return visibility_; } + // Changes the command visibility. + void SetVisibility(const Visibility& visibility); // Returns the role required to execute command. UserRole GetMinimalRole() const { return minimal_role_; } + // Changes the role required to execute command. + void SetMinimalRole(UserRole minimal_role) { minimal_role_ = minimal_role; } private: - CommandDefinition(const base::DictionaryValue& definition, - UserRole minimal_role); - - base::DictionaryValue definition_; - UserRole minimal_role_; + std::unique_ptr<const ObjectSchema> parameters_; // Command parameters def. + std::unique_ptr<const ObjectSchema> progress_; // Command progress def. + std::unique_ptr<const ObjectSchema> results_; // Command results def. + Visibility visibility_; // Available to all by default. + // Minimal role required to execute command. + UserRole minimal_role_{UserRole::kUser}; DISALLOW_COPY_AND_ASSIGN(CommandDefinition); };
diff --git a/src/commands/command_definition_unittest.cc b/src/commands/command_definition_unittest.cc index ecd6e54..77b2754 100644 --- a/src/commands/command_definition_unittest.cc +++ b/src/commands/command_definition_unittest.cc
@@ -6,47 +6,87 @@ #include <gtest/gtest.h> -#include "src/commands/unittest_utils.h" - namespace weave { -using test::CreateDictionaryValue; - -TEST(CommandDefinition, DefaultRole) { - auto params = CreateDictionaryValue(R"({ - 'parameters': { - 'height': 'integer', - 'jumpType': ['_withAirFlip', '_withSpin', '_withKick'] - }, - 'progress': {'progress': 'integer'}, - 'results': {'testResult': 'integer'} - })"); - auto def = CommandDefinition::FromJson(*params, nullptr); - EXPECT_EQ(UserRole::kUser, def->GetMinimalRole()); +TEST(CommandVisibility, DefaultConstructor) { + CommandDefinition::Visibility visibility; + EXPECT_FALSE(visibility.local); + EXPECT_FALSE(visibility.cloud); } -TEST(CommandDefinition, SpecifiedRole) { - auto params = CreateDictionaryValue(R"({ - 'parameters': {}, - 'progress': {}, - 'results': {}, - 'minimalRole': 'owner' - })"); - auto def = CommandDefinition::FromJson(*params, nullptr); - EXPECT_EQ(UserRole::kOwner, def->GetMinimalRole()); +TEST(CommandVisibility, InitialState) { + auto visibility = CommandDefinition::Visibility::GetAll(); + EXPECT_TRUE(visibility.local); + EXPECT_TRUE(visibility.cloud); + + visibility = CommandDefinition::Visibility::GetLocal(); + EXPECT_TRUE(visibility.local); + EXPECT_FALSE(visibility.cloud); + + visibility = CommandDefinition::Visibility::GetCloud(); + EXPECT_FALSE(visibility.local); + EXPECT_TRUE(visibility.cloud); + + visibility = CommandDefinition::Visibility::GetNone(); + EXPECT_FALSE(visibility.local); + EXPECT_FALSE(visibility.cloud); } -TEST(CommandDefinition, IncorrectRole) { - auto params = CreateDictionaryValue(R"({ - 'parameters': {}, - 'progress': {}, - 'results': {}, - 'minimalRole': 'foo' - })"); +TEST(CommandVisibility, FromString) { + CommandDefinition::Visibility visibility; + + ASSERT_TRUE(visibility.FromString("local", nullptr)); + EXPECT_TRUE(visibility.local); + EXPECT_FALSE(visibility.cloud); + + ASSERT_TRUE(visibility.FromString("cloud", nullptr)); + EXPECT_FALSE(visibility.local); + EXPECT_TRUE(visibility.cloud); + + ASSERT_TRUE(visibility.FromString("cloud,local", nullptr)); + EXPECT_TRUE(visibility.local); + EXPECT_TRUE(visibility.cloud); + + ASSERT_TRUE(visibility.FromString("none", nullptr)); + EXPECT_FALSE(visibility.local); + EXPECT_FALSE(visibility.cloud); + + ASSERT_TRUE(visibility.FromString("all", nullptr)); + EXPECT_TRUE(visibility.local); + EXPECT_TRUE(visibility.cloud); + + ASSERT_TRUE(visibility.FromString("", nullptr)); + EXPECT_FALSE(visibility.local); + EXPECT_FALSE(visibility.cloud); + ErrorPtr error; - auto def = CommandDefinition::FromJson(*params, &error); - EXPECT_EQ(nullptr, def.get()); + ASSERT_FALSE(visibility.FromString("cloud,all", &error)); EXPECT_EQ("invalid_parameter_value", error->GetCode()); } +TEST(CommandVisibility, ToString) { + EXPECT_EQ("none", CommandDefinition::Visibility::GetNone().ToString()); + EXPECT_EQ("local", CommandDefinition::Visibility::GetLocal().ToString()); + EXPECT_EQ("cloud", CommandDefinition::Visibility::GetCloud().ToString()); + EXPECT_EQ("all", CommandDefinition::Visibility::GetAll().ToString()); +} + +TEST(CommandDefinition, Test) { + std::unique_ptr<const ObjectSchema> params{ObjectSchema::Create()}; + std::unique_ptr<const ObjectSchema> progress{ObjectSchema::Create()}; + std::unique_ptr<const ObjectSchema> results{ObjectSchema::Create()}; + const ObjectSchema* param_ptr = params.get(); + const ObjectSchema* progress_ptr = progress.get(); + const ObjectSchema* results_ptr = results.get(); + CommandDefinition def{std::move(params), std::move(progress), + std::move(results)}; + EXPECT_EQ(param_ptr, def.GetParameters()); + EXPECT_EQ(progress_ptr, def.GetProgress()); + EXPECT_EQ(results_ptr, def.GetResults()); + EXPECT_EQ("all", def.GetVisibility().ToString()); + + def.SetVisibility(CommandDefinition::Visibility::GetLocal()); + EXPECT_EQ("local", def.GetVisibility().ToString()); +} + } // namespace weave
diff --git a/src/commands/command_dictionary.cc b/src/commands/command_dictionary.cc index 8fb7f43..053e7aa 100644 --- a/src/commands/command_dictionary.cc +++ b/src/commands/command_dictionary.cc
@@ -14,6 +14,7 @@ namespace weave { bool CommandDictionary::LoadCommands(const base::DictionaryValue& json, + const CommandDictionary* base_commands, ErrorPtr* error) { CommandMap new_defs; @@ -53,16 +54,89 @@ // Construct the compound command name as "pkg_name.cmd_name". std::string full_command_name = Join(".", package_name, command_name); - auto command_def = CommandDefinition::FromJson(*command_def_json, error); - if (!command_def) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, - errors::commands::kInvalidMinimalRole, - "Error parsing command '%s'", - full_command_name.c_str()); - return false; + const ObjectSchema* base_parameters_def = nullptr; + const ObjectSchema* base_progress_def = nullptr; + const ObjectSchema* base_results_def = nullptr; + // By default make it available to all clients. + auto visibility = CommandDefinition::Visibility::GetAll(); + UserRole minimal_role{UserRole::kUser}; + if (base_commands) { + auto cmd = base_commands->FindCommand(full_command_name); + if (cmd) { + base_parameters_def = cmd->GetParameters(); + base_progress_def = cmd->GetProgress(); + base_results_def = cmd->GetResults(); + visibility = cmd->GetVisibility(); + minimal_role = cmd->GetMinimalRole(); + } + + // If the base command dictionary was provided but the command was not + // found in it, this must be a custom (vendor) command. GCD spec states + // that all custom command names must begin with "_". Let's enforce + // this rule here. + if (!cmd) { + if (command_name.front() != '_') { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidCommandName, + "The name of custom command '%s' in package '%s'" + " must start with '_'", + command_name.c_str(), package_name.c_str()); + return false; + } + } } + auto parameters_schema = BuildObjectSchema( + command_def_json, commands::attributes::kCommand_Parameters, + base_parameters_def, full_command_name, error); + if (!parameters_schema) + return false; + + auto progress_schema = BuildObjectSchema( + command_def_json, commands::attributes::kCommand_Progress, + base_progress_def, full_command_name, error); + if (!progress_schema) + return false; + + auto results_schema = BuildObjectSchema( + command_def_json, commands::attributes::kCommand_Results, + base_results_def, full_command_name, error); + if (!results_schema) + return false; + + std::string value; + if (command_def_json->GetString(commands::attributes::kCommand_Visibility, + &value)) { + if (!visibility.FromString(value, error)) { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidCommandVisibility, + "Error parsing command '%s'", + full_command_name.c_str()); + return false; + } + } + + if (command_def_json->GetString(commands::attributes::kCommand_Role, + &value)) { + if (!StringToEnum(value, &minimal_role)) { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidPropValue, + "Invalid role: '%s'", value.c_str()); + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidMinimalRole, + "Error parsing command '%s'", + full_command_name.c_str()); + return false; + } + } + + std::unique_ptr<CommandDefinition> command_def{new CommandDefinition{ + std::move(parameters_schema), std::move(progress_schema), + std::move(results_schema)}}; + command_def->SetVisibility(visibility); + command_def->SetMinimalRole(minimal_role); new_defs.emplace(full_command_name, std::move(command_def)); + command_iter.Advance(); } package_iter.Advance(); @@ -85,10 +159,49 @@ return true; } +std::unique_ptr<ObjectSchema> CommandDictionary::BuildObjectSchema( + const base::DictionaryValue* command_def_json, + const char* property_name, + const ObjectSchema* base_def, + const std::string& command_name, + ErrorPtr* error) { + auto object_schema = ObjectSchema::Create(); + + const base::DictionaryValue* schema_def = nullptr; + if (!command_def_json->GetDictionaryWithoutPathExpansion(property_name, + &schema_def)) { + if (base_def) + return base_def->Clone(); + return object_schema; + } + + if (!object_schema->FromJson(schema_def, base_def, error)) { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidObjectSchema, + "Invalid definition for command '%s'", + command_name.c_str()); + return {}; + } + + return object_schema; +} + std::unique_ptr<base::DictionaryValue> CommandDictionary::GetCommandsAsJson( + const std::function<bool(const CommandDefinition*)>& filter, + bool full_schema, ErrorPtr* error) const { std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue); for (const auto& pair : definitions_) { + // Check if the command definition has the desired visibility. + // If not, then skip it. + if (!filter(pair.second.get())) + continue; + + std::unique_ptr<base::DictionaryValue> parameters = + pair.second->GetParameters()->ToJson(full_schema, true); + CHECK(parameters); + // Progress and results are not part of public commandDefs. + auto parts = SplitAtFirst(pair.first, ".", true); const std::string& package_name = parts.first; const std::string& command_name = parts.second; @@ -100,8 +213,12 @@ package = new base::DictionaryValue; dict->SetWithoutPathExpansion(package_name, package); } - package->SetWithoutPathExpansion(command_name, - pair.second->ToJson().DeepCopy()); + base::DictionaryValue* command_def = new base::DictionaryValue; + command_def->Set(commands::attributes::kCommand_Parameters, + parameters.release()); + command_def->SetString(commands::attributes::kCommand_Role, + EnumToString(pair.second->GetMinimalRole())); + package->SetWithoutPathExpansion(command_name, command_def); } return dict; }
diff --git a/src/commands/command_dictionary.h b/src/commands/command_dictionary.h index 03e080a..8d3d45c 100644 --- a/src/commands/command_dictionary.h +++ b/src/commands/command_dictionary.h
@@ -23,6 +23,8 @@ namespace weave { +class ObjectSchema; + // CommandDictionary is a wrapper around a map of command name and the // corresponding command definition schema. The command name (the key in // the map) is a compound name in a form of "package_name.command_name", @@ -35,15 +37,25 @@ // Loads command definitions from a JSON object. This is done at the daemon // startup and whenever a device daemon decides to update its command list. // |json| is a JSON dictionary that describes the complete commands. Optional - // Returns false on failure and |error| provides additional error information - // when provided. + // |base_commands| parameter specifies the definition of standard GCD commands + // for parameter schema validation. Can be set to nullptr if no validation is + // needed. Returns false on failure and |error| provides additional error + // information when provided. bool LoadCommands(const base::DictionaryValue& json, + const CommandDictionary* base_commands, ErrorPtr* error); // Converts all the command definitions to a JSON object for CDD/Device // draft. + // |filter| is a predicate used to filter out the command definitions to + // be returned by this method. Only command definitions for which the + // predicate returns true will be included in the resulting JSON. + // |full_schema| specifies whether full command definitions must be generated + // (true) for CDD or only overrides from the base schema (false). // Returns empty unique_ptr in case of an error and fills in the additional // error details in |error|. std::unique_ptr<base::DictionaryValue> GetCommandsAsJson( + const std::function<bool(const CommandDefinition*)>& filter, + bool full_schema, ErrorPtr* error) const; // Returns the number of command definitions in the dictionary. size_t GetSize() const { return definitions_.size(); } @@ -58,6 +70,13 @@ private: using CommandMap = std::map<std::string, std::unique_ptr<CommandDefinition>>; + std::unique_ptr<ObjectSchema> BuildObjectSchema( + const base::DictionaryValue* command_def_json, + const char* property_name, + const ObjectSchema* base_def, + const std::string& command_name, + ErrorPtr* error); + CommandMap definitions_; // List of all available command definitions. DISALLOW_COPY_AND_ASSIGN(CommandDictionary); };
diff --git a/src/commands/command_dictionary_unittest.cc b/src/commands/command_dictionary_unittest.cc index 5cecd76..819718f 100644 --- a/src/commands/command_dictionary_unittest.cc +++ b/src/commands/command_dictionary_unittest.cc
@@ -35,7 +35,7 @@ } })"); CommandDictionary dict; - EXPECT_TRUE(dict.LoadCommands(*json, nullptr)); + EXPECT_TRUE(dict.LoadCommands(*json, nullptr, nullptr)); EXPECT_EQ(1, dict.GetSize()); EXPECT_NE(nullptr, dict.FindCommand("robot.jump")); json = CreateDictionaryValue(R"({ @@ -47,7 +47,7 @@ } } })"); - EXPECT_TRUE(dict.LoadCommands(*json, nullptr)); + EXPECT_TRUE(dict.LoadCommands(*json, nullptr, nullptr)); EXPECT_EQ(3, dict.GetSize()); EXPECT_NE(nullptr, dict.FindCommand("robot.jump")); EXPECT_NE(nullptr, dict.FindCommand("base.reboot")); @@ -55,41 +55,177 @@ EXPECT_EQ(nullptr, dict.FindCommand("foo.bar")); } +TEST(CommandDictionary, LoadWithInheritance) { + auto json = CreateDictionaryValue(R"({ + 'robot': { + 'jump': { + 'minimalRole': 'viewer', + 'visibility':'local', + 'parameters': { + 'height': 'integer' + }, + 'progress': { + 'progress': 'integer' + }, + 'results': { + 'success': 'boolean' + } + } + } + })"); + CommandDictionary base_dict; + EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr, nullptr)); + EXPECT_EQ(1, base_dict.GetSize()); + json = CreateDictionaryValue(R"({'robot': {'jump': {}}})"); + + CommandDictionary dict; + EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, nullptr)); + EXPECT_EQ(1, dict.GetSize()); + + auto cmd = dict.FindCommand("robot.jump"); + EXPECT_NE(nullptr, cmd); + + EXPECT_EQ("local", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole()); + + EXPECT_JSON_EQ("{'height': {'type': 'integer'}}", + *cmd->GetParameters()->ToJson(true, true)); + EXPECT_JSON_EQ("{'progress': {'type': 'integer'}}", + *cmd->GetProgress()->ToJson(true, false)); + EXPECT_JSON_EQ("{'success': {'type': 'boolean'}}", + *cmd->GetResults()->ToJson(true, false)); +} + TEST(CommandDictionary, LoadCommands_Failures) { CommandDictionary dict; ErrorPtr error; // Command definition is not an object. auto json = CreateDictionaryValue("{'robot':{'jump':0}}"); - EXPECT_FALSE(dict.LoadCommands(*json, &error)); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); EXPECT_EQ("type_mismatch", error->GetCode()); error.reset(); // Package definition is not an object. json = CreateDictionaryValue("{'robot':'blah'}"); - EXPECT_FALSE(dict.LoadCommands(*json, &error)); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); EXPECT_EQ("type_mismatch", error->GetCode()); error.reset(); + // Invalid command definition is not an object. + json = CreateDictionaryValue( + "{'robot':{'jump':{'parameters':{'flip':0},'results':{}}}}"); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); + EXPECT_EQ("invalid_object_schema", error->GetCode()); + EXPECT_NE(nullptr, error->GetInnerError()); // Must have additional info. + error.reset(); + // Empty command name. json = CreateDictionaryValue("{'robot':{'':{'parameters':{},'results':{}}}}"); - EXPECT_FALSE(dict.LoadCommands(*json, &error)); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); EXPECT_EQ("invalid_command_name", error->GetCode()); error.reset(); } -TEST(CommandDictionaryDeathTest, LoadCommands_Redefine) { - // Redefine commands. +TEST(CommandDictionaryDeathTest, LoadCommands_RedefineInDifferentCategory) { + // Redefine commands in different category. CommandDictionary dict; ErrorPtr error; auto json = CreateDictionaryValue("{'robot':{'jump':{}}}"); - dict.LoadCommands(*json, nullptr); - ASSERT_DEATH(dict.LoadCommands(*json, &error), + dict.LoadCommands(*json, nullptr, &error); + ASSERT_DEATH(dict.LoadCommands(*json, nullptr, &error), ".*Definition for command 'robot.jump' overrides an " "earlier definition"); } +TEST(CommandDictionary, LoadCommands_CustomCommandNaming) { + // Custom command must start with '_'. + CommandDictionary base_dict; + CommandDictionary dict; + ErrorPtr error; + auto json = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {'delay': 'integer'}, + 'results': {} + } + } + })"); + base_dict.LoadCommands(*json, nullptr, &error); + EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, &error)); + auto json2 = + CreateDictionaryValue("{'base':{'jump':{'parameters':{},'results':{}}}}"); + EXPECT_FALSE(dict.LoadCommands(*json2, &base_dict, &error)); + EXPECT_EQ("invalid_command_name", error->GetCode()); + error.reset(); + + // If the command starts with "_", then it's Ok. + json2 = CreateDictionaryValue( + "{'base':{'_jump':{'parameters':{},'results':{}}}}"); + EXPECT_TRUE(dict.LoadCommands(*json2, &base_dict, nullptr)); +} + +TEST(CommandDictionary, LoadCommands_RedefineStdCommand) { + // Redefine commands parameter type. + CommandDictionary base_dict; + CommandDictionary dict; + ErrorPtr error; + auto json = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {'delay': 'integer'}, + 'results': {'version': 'integer'} + } + } + })"); + base_dict.LoadCommands(*json, nullptr, &error); + + auto json2 = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {'delay': 'string'}, + 'results': {'version': 'integer'} + } + } + })"); + EXPECT_FALSE(dict.LoadCommands(*json2, &base_dict, &error)); + EXPECT_EQ("invalid_object_schema", error->GetCode()); + EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode()); + EXPECT_EQ("param_type_changed", error->GetFirstError()->GetCode()); + error.reset(); + + auto json3 = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {'delay': 'integer'}, + 'results': {'version': 'string'} + } + } + })"); + EXPECT_FALSE(dict.LoadCommands(*json3, &base_dict, &error)); + EXPECT_EQ("invalid_object_schema", error->GetCode()); + // TODO(antonm): remove parameter from error below and use some generic. + EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode()); + EXPECT_EQ("param_type_changed", error->GetFirstError()->GetCode()); + error.reset(); +} + TEST(CommandDictionary, GetCommandsAsJson) { + auto json_base = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {'delay': {'maximum': 100}}, + 'results': {} + }, + 'shutdown': { + 'parameters': {}, + 'results': {} + } + } + })"); + CommandDictionary base_dict; + base_dict.LoadCommands(*json_base, nullptr, nullptr); + auto json = CreateDictionaryValue(R"({ 'base': { 'reboot': { @@ -100,20 +236,21 @@ 'robot': { '_jump': { 'parameters': {'_height': 'integer'}, - 'minimalRole': 'user' + 'results': {} } } })"); CommandDictionary dict; - dict.LoadCommands(*json, nullptr); + dict.LoadCommands(*json, &base_dict, nullptr); - json = dict.GetCommandsAsJson(nullptr); + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { return true; }, false, nullptr); ASSERT_NE(nullptr, json.get()); auto expected = R"({ 'base': { 'reboot': { 'parameters': {'delay': {'minimum': 10}}, - 'results': {} + 'minimalRole': 'user' } }, 'robot': { @@ -124,6 +261,143 @@ } })"; EXPECT_JSON_EQ(expected, *json); + + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { return true; }, true, nullptr); + ASSERT_NE(nullptr, json.get()); + expected = R"({ + 'base': { + 'reboot': { + 'parameters': { + 'delay': { + 'maximum': 100, + 'minimum': 10, + 'type': 'integer' + } + }, + 'minimalRole': 'user' + } + }, + 'robot': { + '_jump': { + 'parameters': { + '_height': { + 'type': 'integer' + } + }, + 'minimalRole': 'user' + } + } + })"; + EXPECT_JSON_EQ(expected, *json); +} + +TEST(CommandDictionary, GetCommandsAsJsonWithVisibility) { + auto json = CreateDictionaryValue(R"({ + 'test': { + 'command1': { + 'parameters': {}, + 'results': {}, + 'visibility': 'none' + }, + 'command2': { + 'parameters': {}, + 'results': {}, + 'visibility': 'local' + }, + 'command3': { + 'parameters': {}, + 'results': {}, + 'visibility': 'cloud' + }, + 'command4': { + 'parameters': {}, + 'results': {}, + 'visibility': 'all' + }, + 'command5': { + 'parameters': {}, + 'results': {}, + 'visibility': 'none' + }, + 'command6': { + 'parameters': {}, + 'results': {}, + 'visibility': 'local' + }, + 'command7': { + 'parameters': {}, + 'results': {}, + 'visibility': 'cloud' + }, + 'command8': { + 'parameters': {}, + 'results': {}, + 'visibility': 'all' + } + } + })"); + CommandDictionary dict; + ASSERT_TRUE(dict.LoadCommands(*json, nullptr, nullptr)); + + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { return true; }, false, nullptr); + ASSERT_NE(nullptr, json.get()); + auto expected = R"({ + 'test': { + 'command1': {'parameters': {}, 'minimalRole': 'user'}, + 'command2': {'parameters': {}, 'minimalRole': 'user'}, + 'command3': {'parameters': {}, 'minimalRole': 'user'}, + 'command4': {'parameters': {}, 'minimalRole': 'user'}, + 'command5': {'parameters': {}, 'minimalRole': 'user'}, + 'command6': {'parameters': {}, 'minimalRole': 'user'}, + 'command7': {'parameters': {}, 'minimalRole': 'user'}, + 'command8': {'parameters': {}, 'minimalRole': 'user'} + } + })"; + EXPECT_JSON_EQ(expected, *json); + + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { return def->GetVisibility().local; }, + false, nullptr); + ASSERT_NE(nullptr, json.get()); + expected = R"({ + 'test': { + 'command2': {'parameters': {}, 'minimalRole': 'user'}, + 'command4': {'parameters': {}, 'minimalRole': 'user'}, + 'command6': {'parameters': {}, 'minimalRole': 'user'}, + 'command8': {'parameters': {}, 'minimalRole': 'user'} + } + })"; + EXPECT_JSON_EQ(expected, *json); + + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { return def->GetVisibility().cloud; }, + false, nullptr); + ASSERT_NE(nullptr, json.get()); + expected = R"({ + 'test': { + 'command3': {'parameters': {}, 'minimalRole': 'user'}, + 'command4': {'parameters': {}, 'minimalRole': 'user'}, + 'command7': {'parameters': {}, 'minimalRole': 'user'}, + 'command8': {'parameters': {}, 'minimalRole': 'user'} + } + })"; + EXPECT_JSON_EQ(expected, *json); + + json = dict.GetCommandsAsJson( + [](const CommandDefinition* def) { + return def->GetVisibility().local && def->GetVisibility().cloud; + }, + false, nullptr); + ASSERT_NE(nullptr, json.get()); + expected = R"({ + 'test': { + 'command4': {'parameters': {}, 'minimalRole': 'user'}, + 'command8': {'parameters': {}, 'minimalRole': 'user'} + } + })"; + EXPECT_JSON_EQ(expected, *json); } TEST(CommandDictionary, LoadWithPermissions) { @@ -132,51 +406,161 @@ 'base': { 'command1': { 'parameters': {}, - 'results': {} + 'results': {}, + 'visibility':'none' }, 'command2': { 'minimalRole': 'viewer', 'parameters': {}, - 'results': {} + 'results': {}, + 'visibility':'local' }, 'command3': { 'minimalRole': 'user', 'parameters': {}, - 'results': {} + 'results': {}, + 'visibility':'cloud' }, 'command4': { 'minimalRole': 'manager', 'parameters': {}, - 'results': {} + 'results': {}, + 'visibility':'all' }, 'command5': { 'minimalRole': 'owner', 'parameters': {}, + 'results': {}, + 'visibility':'local,cloud' + } + } + })"); + EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr, nullptr)); + + auto cmd = base_dict.FindCommand("base.command1"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("none", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole()); + + cmd = base_dict.FindCommand("base.command2"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("local", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole()); + + cmd = base_dict.FindCommand("base.command3"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("cloud", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole()); + + cmd = base_dict.FindCommand("base.command4"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("all", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kManager, cmd->GetMinimalRole()); + + cmd = base_dict.FindCommand("base.command5"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("all", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kOwner, cmd->GetMinimalRole()); + + CommandDictionary dict; + json = CreateDictionaryValue(R"({ + 'base': { + 'command1': { + 'parameters': {}, + 'results': {} + }, + 'command2': { + 'parameters': {}, + 'results': {} + }, + 'command3': { + 'parameters': {}, + 'results': {} + }, + 'command4': { + 'parameters': {}, + 'results': {} + }, + 'command5': { + 'parameters': {}, + 'results': {} + }, + '_command6': { + 'parameters': {}, 'results': {} } } })"); - EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr)); + EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, nullptr)); - auto cmd = base_dict.FindCommand("base.command1"); + cmd = dict.FindCommand("base.command1"); ASSERT_NE(nullptr, cmd); + EXPECT_EQ("none", cmd->GetVisibility().ToString()); EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole()); - cmd = base_dict.FindCommand("base.command2"); + cmd = dict.FindCommand("base.command2"); ASSERT_NE(nullptr, cmd); + EXPECT_EQ("local", cmd->GetVisibility().ToString()); EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole()); - cmd = base_dict.FindCommand("base.command3"); + cmd = dict.FindCommand("base.command3"); ASSERT_NE(nullptr, cmd); + EXPECT_EQ("cloud", cmd->GetVisibility().ToString()); EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole()); - cmd = base_dict.FindCommand("base.command4"); + cmd = dict.FindCommand("base.command4"); ASSERT_NE(nullptr, cmd); + EXPECT_EQ("all", cmd->GetVisibility().ToString()); EXPECT_EQ(UserRole::kManager, cmd->GetMinimalRole()); - cmd = base_dict.FindCommand("base.command5"); + cmd = dict.FindCommand("base.command5"); ASSERT_NE(nullptr, cmd); + EXPECT_EQ("all", cmd->GetVisibility().ToString()); EXPECT_EQ(UserRole::kOwner, cmd->GetMinimalRole()); + + cmd = dict.FindCommand("base._command6"); + ASSERT_NE(nullptr, cmd); + EXPECT_EQ("all", cmd->GetVisibility().ToString()); + EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole()); +} + +TEST(CommandDictionary, LoadWithPermissions_InvalidVisibility) { + CommandDictionary dict; + ErrorPtr error; + + auto json = CreateDictionaryValue(R"({ + 'base': { + 'jump': { + 'parameters': {}, + 'results': {}, + 'visibility':'foo' + } + } + })"); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); + EXPECT_EQ("invalid_command_visibility", error->GetCode()); + EXPECT_EQ("invalid_parameter_value", error->GetInnerError()->GetCode()); + error.reset(); +} + +TEST(CommandDictionary, LoadWithPermissions_InvalidRole) { + CommandDictionary dict; + ErrorPtr error; + + auto json = CreateDictionaryValue(R"({ + 'base': { + 'jump': { + 'parameters': {}, + 'results': {}, + 'visibility':'local,cloud', + 'minimalRole':'foo' + } + } + })"); + EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error)); + EXPECT_EQ("invalid_minimal_role", error->GetCode()); + EXPECT_EQ("invalid_parameter_value", error->GetInnerError()->GetCode()); + error.reset(); } } // namespace weave
diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc index aa71b0e..f152d82 100644 --- a/src/commands/command_instance.cc +++ b/src/commands/command_instance.cc
@@ -12,7 +12,9 @@ #include "src/commands/command_definition.h" #include "src/commands/command_dictionary.h" #include "src/commands/command_queue.h" +#include "src/commands/prop_types.h" #include "src/commands/schema_constants.h" +#include "src/commands/schema_utils.h" #include "src/json_error_codes.h" #include "src/utils.h" @@ -66,12 +68,12 @@ CommandInstance::CommandInstance(const std::string& name, Command::Origin origin, const CommandDefinition* command_definition, - const base::DictionaryValue& parameters) + const ValueMap& parameters) : name_{name}, origin_{origin}, - command_definition_{command_definition} { + command_definition_{command_definition}, + parameters_{parameters} { CHECK(command_definition_); - parameters_.MergeDictionary(¶meters); } CommandInstance::~CommandInstance() { @@ -95,15 +97,15 @@ } std::unique_ptr<base::DictionaryValue> CommandInstance::GetParameters() const { - return std::unique_ptr<base::DictionaryValue>(parameters_.DeepCopy()); + return TypedValueToJson(parameters_); } std::unique_ptr<base::DictionaryValue> CommandInstance::GetProgress() const { - return std::unique_ptr<base::DictionaryValue>(progress_.DeepCopy()); + return TypedValueToJson(progress_); } std::unique_ptr<base::DictionaryValue> CommandInstance::GetResults() const { - return std::unique_ptr<base::DictionaryValue>(results_.DeepCopy()); + return TypedValueToJson(results_); } const Error* CommandInstance::GetError() const { @@ -112,13 +114,21 @@ bool CommandInstance::SetProgress(const base::DictionaryValue& progress, ErrorPtr* error) { + if (!command_definition_) + return ReportDestroyedError(error); + ObjectPropType obj_prop_type; + obj_prop_type.SetObjectSchema(command_definition_->GetProgress()->Clone()); + + ValueMap obj; + if (!TypedValueFromJson(&progress, &obj_prop_type, &obj, error)) + return false; + // Change status even if progress unchanged, e.g. 0% -> 0%. if (!SetStatus(State::kInProgress, error)) return false; - if (!progress_.Equals(&progress)) { - progress_.Clear(); - progress_.MergeDictionary(&progress); + if (obj != progress_) { + progress_ = obj; FOR_EACH_OBSERVER(Observer, observers_, OnProgressChanged()); } @@ -127,9 +137,17 @@ bool CommandInstance::Complete(const base::DictionaryValue& results, ErrorPtr* error) { - if (!results_.Equals(&results)) { - results_.Clear(); - results_.MergeDictionary(&results); + if (!command_definition_) + return ReportDestroyedError(error); + ObjectPropType obj_prop_type; + obj_prop_type.SetObjectSchema(command_definition_->GetResults()->Clone()); + + ValueMap obj; + if (!TypedValueFromJson(&results, &obj_prop_type, &obj, error)) + return false; + + if (obj != results_) { + results_ = obj; FOR_EACH_OBSERVER(Observer, observers_, OnResultsChanged()); } // Change status even if result is unchanged. @@ -153,29 +171,36 @@ // On success, returns |true| and the validated parameters and values through // |parameters|. Otherwise returns |false| and additional error information in // |error|. -std::unique_ptr<base::DictionaryValue> GetCommandParameters( - const base::DictionaryValue* json, - const CommandDefinition* command_def, - ErrorPtr* error) { +bool GetCommandParameters(const base::DictionaryValue* json, + const CommandDefinition* command_def, + ValueMap* parameters, + ErrorPtr* error) { // Get the command parameters from 'parameters' property. - std::unique_ptr<base::DictionaryValue> params; + base::DictionaryValue no_params; // Placeholder when no params are specified. + const base::DictionaryValue* params = nullptr; const base::Value* params_value = nullptr; if (json->Get(commands::attributes::kCommand_Parameters, ¶ms_value)) { // Make sure the "parameters" property is actually an object. - const base::DictionaryValue* params_dict = nullptr; - if (!params_value->GetAsDictionary(¶ms_dict)) { + if (!params_value->GetAsDictionary(¶ms)) { Error::AddToPrintf(error, FROM_HERE, errors::json::kDomain, errors::json::kObjectExpected, "Property '%s' must be a JSON object", commands::attributes::kCommand_Parameters); - return params; + return false; } - params.reset(params_dict->DeepCopy()); } else { // "parameters" are not specified. Assume empty param list. - params.reset(new base::DictionaryValue); + params = &no_params; } - return params; + + // Now read in the parameters and validate their values against the command + // definition schema. + ObjectPropType obj_prop_type; + obj_prop_type.SetObjectSchema(command_def->GetParameters()->Clone()); + if (!TypedValueFromJson(params, &obj_prop_type, parameters, error)) { + return false; + } + return true; } } // anonymous namespace @@ -221,8 +246,8 @@ return instance; } - auto parameters = GetCommandParameters(json, command_def, error); - if (!parameters) { + ValueMap parameters; + if (!GetCommandParameters(json, command_def, ¶meters, error)) { Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, errors::commands::kCommandFailed, "Failed to validate command '%s'", command_name.c_str()); @@ -230,7 +255,7 @@ } instance.reset( - new CommandInstance{command_name, origin, command_def, *parameters}); + new CommandInstance{command_name, origin, command_def, parameters}); if (!command_id->empty()) instance->SetID(*command_id); @@ -243,9 +268,12 @@ json->SetString(commands::attributes::kCommand_Id, id_); json->SetString(commands::attributes::kCommand_Name, name_); - json->Set(commands::attributes::kCommand_Parameters, parameters_.DeepCopy()); - json->Set(commands::attributes::kCommand_Progress, progress_.DeepCopy()); - json->Set(commands::attributes::kCommand_Results, results_.DeepCopy()); + json->Set(commands::attributes::kCommand_Parameters, + TypedValueToJson(parameters_).release()); + json->Set(commands::attributes::kCommand_Progress, + TypedValueToJson(progress_).release()); + json->Set(commands::attributes::kCommand_Results, + TypedValueToJson(results_).release()); json->SetString(commands::attributes::kCommand_State, EnumToString(state_)); if (error_) { json->Set(commands::attributes::kCommand_Error,
diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h index 30ef907..5a2ebf7 100644 --- a/src/commands/command_instance.h +++ b/src/commands/command_instance.h
@@ -44,12 +44,12 @@ }; // Construct a command instance given the full command |name| which must - // be in format "<package_name>.<command_name>" and a list of parameters and - // their values specified in |parameters|. + // 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, Command::Origin origin, const CommandDefinition* command_definition, - const base::DictionaryValue& parameters); + const ValueMap& parameters); ~CommandInstance() override; // Command overrides. @@ -124,11 +124,11 @@ // Command definition. const CommandDefinition* command_definition_{nullptr}; // Command parameters and their values. - base::DictionaryValue parameters_; + ValueMap parameters_; // Current command execution progress. - base::DictionaryValue progress_; + ValueMap progress_; // Command results. - base::DictionaryValue results_; + ValueMap results_; // Current command state. Command::State state_ = Command::State::kQueued; // Error encountered during execution of the command.
diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc index cacb86c..4e208ed 100644 --- a/src/commands/command_instance_unittest.cc +++ b/src/commands/command_instance_unittest.cc
@@ -7,6 +7,8 @@ #include <gtest/gtest.h> #include "src/commands/command_dictionary.h" +#include "src/commands/prop_types.h" +#include "src/commands/schema_utils.h" #include "src/commands/unittest_utils.h" namespace weave { @@ -59,7 +61,7 @@ } } })"); - CHECK(dict_.LoadCommands(*json, nullptr)) + CHECK(dict_.LoadCommands(*json, nullptr, nullptr)) << "Failed to parse test command dictionary"; } CommandDictionary dict_; @@ -68,12 +70,14 @@ } // anonymous namespace TEST_F(CommandInstanceTest, Test) { - auto params = CreateDictionaryValue(R"({ - 'phrase': 'iPityDaFool', - 'volume': 5 - })"); + StringPropType str_prop; + IntPropType int_prop; + ValueMap params; + params["phrase"] = + str_prop.CreateValue(base::StringValue{"iPityDaFool"}, nullptr); + params["volume"] = int_prop.CreateValue(base::FundamentalValue{5}, nullptr); CommandInstance instance{"robot.speak", Command::Origin::kCloud, - dict_.FindCommand("robot.speak"), *params}; + dict_.FindCommand("robot.speak"), params}; EXPECT_TRUE( instance.Complete(*CreateDictionaryValue("{'foo': 239}"), nullptr)); @@ -170,6 +174,25 @@ EXPECT_EQ("command_failed", error->GetCode()); } +TEST_F(CommandInstanceTest, FromJson_ParamError) { + auto json = CreateDictionaryValue(R"({ + 'name': 'robot.speak', + 'parameters': { + 'phrase': 'iPityDaFool', + 'volume': 20 + } + })"); + ErrorPtr error; + auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud, + dict_, nullptr, &error); + EXPECT_EQ(nullptr, instance.get()); + auto first = error->GetFirstError(); + EXPECT_EQ("out_of_range", first->GetCode()); + auto inner = error->GetInnerError(); + EXPECT_EQ("invalid_parameter_value", inner->GetCode()); + EXPECT_EQ("command_failed", error->GetCode()); +} + TEST_F(CommandInstanceTest, ToJson) { auto json = CreateDictionaryValue(R"({ 'name': 'robot.jump',
diff --git a/src/commands/command_manager.cc b/src/commands/command_manager.cc index a64c8e5..75d8295 100644 --- a/src/commands/command_manager.cc +++ b/src/commands/command_manager.cc
@@ -28,7 +28,7 @@ bool CommandManager::LoadCommands(const base::DictionaryValue& dict, ErrorPtr* error) { - bool result = dictionary_.LoadCommands(dict, error); + bool result = dictionary_.LoadCommands(dict, nullptr, error); for (const auto& cb : on_command_changed_) cb.Run(); return result; @@ -83,6 +83,37 @@ return command_queue_.Find(id); } +bool CommandManager::SetCommandVisibility( + const std::vector<std::string>& command_names, + CommandDefinition::Visibility visibility, + ErrorPtr* error) { + if (command_names.empty()) + return true; + + std::vector<CommandDefinition*> definitions; + definitions.reserve(command_names.size()); + + // Find/validate command definitions first. + for (const std::string& name : command_names) { + CommandDefinition* def = dictionary_.FindCommand(name); + if (!def) { + Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidCommandName, + "Command '%s' is unknown", name.c_str()); + return false; + } + definitions.push_back(def); + } + + // Now that we know that all the command names were valid, + // update the respective commands' visibility. + for (CommandDefinition* def : definitions) + def->SetVisibility(visibility); + for (const auto& cb : on_command_changed_) + cb.Run(); + return true; +} + void CommandManager::AddCommandAddedCallback( const CommandQueue::CommandCallback& callback) { command_queue_.AddCommandAddedCallback(callback);
diff --git a/src/commands/command_manager.h b/src/commands/command_manager.h index 04f49ee..7cc8907 100644 --- a/src/commands/command_manager.h +++ b/src/commands/command_manager.h
@@ -60,6 +60,11 @@ // Adds a new command to the command queue. void AddCommand(std::unique_ptr<CommandInstance> command_instance); + // Changes the visibility of commands. + bool SetCommandVisibility(const std::vector<std::string>& command_names, + CommandDefinition::Visibility visibility, + ErrorPtr* error); + bool AddCommand(const base::DictionaryValue& command, UserRole role, std::string* id,
diff --git a/src/commands/command_manager_unittest.cc b/src/commands/command_manager_unittest.cc index eca5175..0c890a9 100644 --- a/src/commands/command_manager_unittest.cc +++ b/src/commands/command_manager_unittest.cc
@@ -91,4 +91,59 @@ EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("test._yo")); } +TEST(CommandManager, UpdateCommandVisibility) { + CommandManager manager; + int update_count = 0; + auto on_command_change = [&update_count]() { update_count++; }; + manager.AddCommandDefChanged(base::Bind(on_command_change)); + + auto json = CreateDictionaryValue(R"({ + 'foo': { + '_baz': { + 'parameters': {}, + 'results': {} + }, + '_bar': { + 'parameters': {}, + 'results': {} + } + }, + 'bar': { + '_quux': { + 'parameters': {}, + 'results': {}, + 'visibility': 'none' + } + } + })"); + ASSERT_TRUE(manager.LoadCommands(*json, nullptr)); + EXPECT_EQ(2, update_count); + const CommandDictionary& dict = manager.GetCommandDictionary(); + EXPECT_TRUE(manager.SetCommandVisibility( + {"foo._baz"}, CommandDefinition::Visibility::GetLocal(), nullptr)); + EXPECT_EQ(3, update_count); + EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString()); + + ErrorPtr error; + ASSERT_FALSE(manager.SetCommandVisibility( + {"foo._baz", "foo._bar", "test.cmd"}, + CommandDefinition::Visibility::GetLocal(), &error)); + EXPECT_EQ(errors::commands::kInvalidCommandName, error->GetCode()); + // The visibility state of commands shouldn't have changed. + EXPECT_EQ(3, update_count); + EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString()); + + EXPECT_TRUE(manager.SetCommandVisibility( + {"foo._baz", "bar._quux"}, CommandDefinition::Visibility::GetCloud(), + nullptr)); + EXPECT_EQ(4, update_count); + EXPECT_EQ("cloud", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("cloud", dict.FindCommand("bar._quux")->GetVisibility().ToString()); +} + } // namespace weave
diff --git a/src/commands/command_queue_unittest.cc b/src/commands/command_queue_unittest.cc index a9e953e..5060071 100644 --- a/src/commands/command_queue_unittest.cc +++ b/src/commands/command_queue_unittest.cc
@@ -20,15 +20,11 @@ class CommandQueueTest : public testing::Test { public: - CommandQueueTest() { - command_definition_ = CommandDefinition::FromJson({}, nullptr); - } - std::unique_ptr<CommandInstance> CreateDummyCommandInstance( const std::string& name, const std::string& id) { std::unique_ptr<CommandInstance> cmd{new CommandInstance{ - name, Command::Origin::kLocal, command_definition_.get(), {}}}; + name, Command::Origin::kLocal, &command_definition_, {}}}; cmd->SetID(id); return cmd; } @@ -43,7 +39,8 @@ CommandQueue queue_; private: - std::unique_ptr<CommandDefinition> command_definition_; + CommandDefinition command_definition_{ + ObjectSchema::Create(), ObjectSchema::Create(), ObjectSchema::Create()}; }; // Keeps track of commands being added to and removed from the queue_.
diff --git a/src/commands/schema_constants.cc b/src/commands/schema_constants.cc index 7f8431f..c99536b 100644 --- a/src/commands/schema_constants.cc +++ b/src/commands/schema_constants.cc
@@ -23,6 +23,7 @@ const char kDuplicateCommandDef[] = "duplicate_command_definition"; const char kInvalidCommandName[] = "invalid_command_name"; const char kCommandFailed[] = "command_failed"; +const char kInvalidCommandVisibility[] = "invalid_command_visibility"; const char kInvalidMinimalRole[] = "invalid_minimal_role"; const char kCommandDestroyed[] = "command_destroyed"; const char kInvalidState[] = "invalid_state"; @@ -65,6 +66,12 @@ const char kCommand_Role_User[] = "user"; const char kCommand_Role_Viewer[] = "viewer"; +const char kCommand_Visibility[] = "visibility"; +const char kCommand_Visibility_None[] = "none"; +const char kCommand_Visibility_Local[] = "local"; +const char kCommand_Visibility_Cloud[] = "cloud"; +const char kCommand_Visibility_All[] = "all"; + } // namespace attributes } // namespace commands
diff --git a/src/commands/schema_constants.h b/src/commands/schema_constants.h index ea59f17..742245f 100644 --- a/src/commands/schema_constants.h +++ b/src/commands/schema_constants.h
@@ -26,6 +26,7 @@ extern const char kDuplicateCommandDef[]; extern const char kInvalidCommandName[]; extern const char kCommandFailed[]; +extern const char kInvalidCommandVisibility[]; extern const char kInvalidMinimalRole[]; extern const char kCommandDestroyed[]; extern const char kInvalidState[]; @@ -68,6 +69,12 @@ extern const char kCommand_Role_User[]; extern const char kCommand_Role_Viewer[]; +extern const char kCommand_Visibility[]; +extern const char kCommand_Visibility_None[]; +extern const char kCommand_Visibility_Local[]; +extern const char kCommand_Visibility_Cloud[]; +extern const char kCommand_Visibility_All[]; + } // namespace attributes } // namespace commands
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 5c8625a..751e530 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc
@@ -480,8 +480,9 @@ std::unique_ptr<base::DictionaryValue> DeviceRegistrationInfo::BuildDeviceResource(ErrorPtr* error) { // Limit only to commands that are visible to the cloud. - auto commands = - command_manager_->GetCommandDictionary().GetCommandsAsJson(error); + auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson( + [](const CommandDefinition* def) { return def->GetVisibility().cloud; }, + true, error); if (!commands) return nullptr; @@ -1120,11 +1121,23 @@ return; std::unique_ptr<base::ListValue> patches{new base::ListValue}; - for (auto& state_change : state_changes) { + for (const auto& state_change : state_changes) { std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue}; patch->SetString("timeMs", std::to_string(state_change.timestamp.ToJavaTime())); - patch->Set("patch", state_change.changed_properties.release()); + + std::unique_ptr<base::DictionaryValue> changes{new base::DictionaryValue}; + for (const auto& pair : state_change.changed_properties) { + auto value = pair.second->ToJson(); + CHECK(value); + // The key in |pair.first| is the full property name in format + // "package.property_name", so must use DictionaryValue::Set() instead of + // DictionaryValue::SetWithoutPathExpansion to recreate the JSON + // property tree properly. + changes->Set(pair.first, value.release()); + } + patch->Set("patch", changes.release()); + patches->Append(patch.release()); }
diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc index 9b53872..df4a438 100644 --- a/src/device_registration_info_unittest.cc +++ b/src/device_registration_info_unittest.cc
@@ -355,14 +355,16 @@ auto json_cmds = CreateDictionaryValue(R"({ 'base': { 'reboot': { - 'parameters': {'delay': {'minimum': 10, 'type': 'integer'}}, - 'minimalRole': 'user' + 'parameters': {'delay': {'minimum': 10}}, + 'minimalRole': 'user', + 'results': {} } }, 'robot': { '_jump': { - 'parameters': {'_height': {'type': 'integer'}}, - 'minimalRole': 'user' + 'parameters': {'_height': 'integer'}, + 'minimalRole': 'user', + 'results': {} } } })");
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc index 32ebf48..d612668 100644 --- a/src/privet/cloud_delegate.cc +++ b/src/privet/cloud_delegate.cc
@@ -251,8 +251,9 @@ void OnCommandDefChanged() { command_defs_.Clear(); - auto commands = - command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr); + auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson( + [](const CommandDefinition* def) { return def->GetVisibility().local; }, + true, nullptr); CHECK(commands); command_defs_.MergeDictionary(commands.get()); NotifyOnCommandDefsChanged();
diff --git a/src/states/mock_state_change_queue_interface.h b/src/states/mock_state_change_queue_interface.h index 78c6c82..c3f50c0 100644 --- a/src/states/mock_state_change_queue_interface.h +++ b/src/states/mock_state_change_queue_interface.h
@@ -16,11 +16,9 @@ class MockStateChangeQueueInterface : public StateChangeQueueInterface { public: MOCK_CONST_METHOD0(IsEmpty, bool()); - MOCK_METHOD2(MockNotifyPropertiesUpdated, - bool(base::Time timestamp, - const base::DictionaryValue& changed_properties)); - MOCK_METHOD0(MockGetAndClearRecordedStateChanges, - std::vector<StateChange>&()); + MOCK_METHOD2(NotifyPropertiesUpdated, + bool(base::Time timestamp, ValueMap changed_properties)); + MOCK_METHOD0(GetAndClearRecordedStateChanges, std::vector<StateChange>()); MOCK_CONST_METHOD0(GetLastStateChangeId, UpdateID()); MOCK_METHOD1(MockAddOnStateUpdatedCallback, base::CallbackList<void(UpdateID)>::Subscription*( @@ -28,15 +26,6 @@ MOCK_METHOD1(NotifyStateUpdatedOnServer, void(UpdateID)); private: - bool NotifyPropertiesUpdated( - base::Time timestamp, - std::unique_ptr<base::DictionaryValue> changed_properties) override { - return MockNotifyPropertiesUpdated(timestamp, *changed_properties); - } - std::vector<StateChange> GetAndClearRecordedStateChanges() override { - return std::move(MockGetAndClearRecordedStateChanges()); - } - Token AddOnStateUpdatedCallback( const base::Callback<void(UpdateID)>& callback) override { return Token{MockAddOnStateUpdatedCallback(callback)};
diff --git a/src/states/state_change_queue.cc b/src/states/state_change_queue.cc index 87cc18d..288dadc 100644 --- a/src/states/state_change_queue.cc +++ b/src/states/state_change_queue.cc
@@ -13,15 +13,12 @@ CHECK_GT(max_queue_size_, 0U) << "Max queue size must not be zero"; } -bool StateChangeQueue::NotifyPropertiesUpdated( - base::Time timestamp, - std::unique_ptr<base::DictionaryValue> changed_properties) { +bool StateChangeQueue::NotifyPropertiesUpdated(base::Time timestamp, + ValueMap changed_properties) { auto& stored_changes = state_changes_[timestamp]; // Merge the old property set. - if (stored_changes) - stored_changes->MergeDictionary(changed_properties.get()); - else - stored_changes = std::move(changed_properties); + changed_properties.insert(stored_changes.begin(), stored_changes.end()); + stored_changes = std::move(changed_properties); while (state_changes_.size() > max_queue_size_) { // Queue is full. @@ -33,8 +30,8 @@ auto element_old = state_changes_.begin(); auto element_new = std::next(element_old); // This will skip elements that exist in both [old] and [new]. - element_old->second->MergeDictionary(element_new->second.get()); - std::swap(element_old->second, element_new->second); + element_new->second.insert(element_old->second.begin(), + element_old->second.end()); state_changes_.erase(element_old); } ++last_change_id_; @@ -44,8 +41,8 @@ std::vector<StateChange> StateChangeQueue::GetAndClearRecordedStateChanges() { std::vector<StateChange> changes; changes.reserve(state_changes_.size()); - for (auto& pair : state_changes_) { - changes.push_back(StateChange{pair.first, std::move(pair.second)}); + for (const auto& pair : state_changes_) { + changes.emplace_back(pair.first, std::move(pair.second)); } state_changes_.clear(); return changes;
diff --git a/src/states/state_change_queue.h b/src/states/state_change_queue.h index 314f746..00b827f 100644 --- a/src/states/state_change_queue.h +++ b/src/states/state_change_queue.h
@@ -21,9 +21,8 @@ // Overrides from StateChangeQueueInterface. bool IsEmpty() const override { return state_changes_.empty(); } - bool NotifyPropertiesUpdated( - base::Time timestamp, - std::unique_ptr<base::DictionaryValue> changed_properties) override; + bool NotifyPropertiesUpdated(base::Time timestamp, + ValueMap changed_properties) override; std::vector<StateChange> GetAndClearRecordedStateChanges() override; UpdateID GetLastStateChangeId() const override { return last_change_id_; } Token AddOnStateUpdatedCallback( @@ -36,7 +35,7 @@ const size_t max_queue_size_; // Accumulated list of device state change notifications. - std::map<base::Time, std::unique_ptr<base::DictionaryValue>> state_changes_; + std::map<base::Time, ValueMap> state_changes_; // An ID of last state change update. Each NotifyPropertiesUpdated() // invocation increments this value by 1.
diff --git a/src/states/state_change_queue_interface.h b/src/states/state_change_queue_interface.h index 81a2cfc..e3b3650 100644 --- a/src/states/state_change_queue_interface.h +++ b/src/states/state_change_queue_interface.h
@@ -19,11 +19,10 @@ // |changed_properties| contains a property set with the new property values // which were updated at the time the event was recorded. struct StateChange { - StateChange(base::Time time, - std::unique_ptr<base::DictionaryValue> properties) + StateChange(base::Time time, ValueMap properties) : timestamp{time}, changed_properties{std::move(properties)} {} base::Time timestamp; - std::unique_ptr<base::DictionaryValue> changed_properties; + ValueMap changed_properties; }; // An abstract interface to StateChangeQueue to record and retrieve state @@ -38,9 +37,8 @@ virtual bool IsEmpty() const = 0; // Called by StateManager when device state properties are updated. - virtual bool NotifyPropertiesUpdated( - base::Time timestamp, - std::unique_ptr<base::DictionaryValue> changed_properties) = 0; + virtual bool NotifyPropertiesUpdated(base::Time timestamp, + ValueMap changed_properties) = 0; // Returns the recorded state changes since last time this method was called. virtual std::vector<StateChange> GetAndClearRecordedStateChanges() = 0;
diff --git a/src/states/state_change_queue_unittest.cc b/src/states/state_change_queue_unittest.cc index 5fbc012..9f26071 100644 --- a/src/states/state_change_queue_unittest.cc +++ b/src/states/state_change_queue_unittest.cc
@@ -11,8 +11,6 @@ namespace weave { -using test::CreateDictionaryValue; - class StateChangeQueueTest : public ::testing::Test { public: void SetUp() override { queue_.reset(new StateChangeQueue(100)); } @@ -29,39 +27,43 @@ } TEST_F(StateChangeQueueTest, UpdateOne) { - auto timestamp = base::Time::Now(); - ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp, CreateDictionaryValue("{'prop': {'name': 23}}"))); + StateChange change{base::Time::Now(), + ValueMap{{"prop.name", test::make_int_prop_value(23)}}}; + ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change.timestamp, + change.changed_properties)); EXPECT_FALSE(queue_->IsEmpty()); EXPECT_EQ(1, queue_->GetLastStateChangeId()); auto changes = queue_->GetAndClearRecordedStateChanges(); EXPECT_EQ(1, queue_->GetLastStateChangeId()); ASSERT_EQ(1, changes.size()); - EXPECT_EQ(timestamp, changes.front().timestamp); - EXPECT_JSON_EQ("{'prop':{'name': 23}}", *changes.front().changed_properties); + EXPECT_EQ(change.timestamp, changes.front().timestamp); + EXPECT_EQ(change.changed_properties, changes.front().changed_properties); EXPECT_TRUE(queue_->IsEmpty()); EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty()); } -TEST_F(StateChangeQueueTest, UpdateMany) { - auto timestamp1 = base::Time::Now(); - const std::string state1 = "{'prop': {'name1': 23}}"; - auto timestamp2 = timestamp1 + base::TimeDelta::FromSeconds(1); - const std::string state2 = - "{'prop': {'name1': 17, 'name2': 1.0, 'name3': false}}"; - ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp1, CreateDictionaryValue(state1))); - ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp2, CreateDictionaryValue(state2))); - +// TODO(vitalybuka): Fix flakiness. +TEST_F(StateChangeQueueTest, DISABLED_UpdateMany) { + StateChange change1{base::Time::Now(), + ValueMap{{"prop.name1", test::make_int_prop_value(23)}}}; + ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change1.timestamp, + change1.changed_properties)); + StateChange change2{base::Time::Now(), + ValueMap{ + {"prop.name1", test::make_int_prop_value(17)}, + {"prop.name2", test::make_double_prop_value(1.0)}, + {"prop.name3", test::make_bool_prop_value(false)}, + }}; + ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change2.timestamp, + change2.changed_properties)); EXPECT_EQ(2, queue_->GetLastStateChangeId()); EXPECT_FALSE(queue_->IsEmpty()); auto changes = queue_->GetAndClearRecordedStateChanges(); ASSERT_EQ(2, changes.size()); - EXPECT_EQ(timestamp1, changes[0].timestamp); - EXPECT_JSON_EQ(state1, *changes[0].changed_properties); - EXPECT_EQ(timestamp2, changes[1].timestamp); - EXPECT_JSON_EQ(state2, *changes[1].changed_properties); + EXPECT_EQ(change1.timestamp, changes[0].timestamp); + EXPECT_EQ(change1.changed_properties, changes[0].changed_properties); + EXPECT_EQ(change2.timestamp, changes[1].timestamp); + EXPECT_EQ(change2.changed_properties, changes[1].changed_properties); EXPECT_TRUE(queue_->IsEmpty()); EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty()); } @@ -71,27 +73,33 @@ base::TimeDelta time_delta = base::TimeDelta::FromMinutes(1); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp, CreateDictionaryValue("{'prop': {'name1': 1}}"))); + timestamp, ValueMap{{"prop.name1", test::make_int_prop_value(1)}})); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp, CreateDictionaryValue("{'prop': {'name2': 2}}"))); + timestamp, ValueMap{{"prop.name2", test::make_int_prop_value(2)}})); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp, CreateDictionaryValue("{'prop': {'name1': 3}}"))); + timestamp, ValueMap{{"prop.name1", test::make_int_prop_value(3)}})); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - timestamp + time_delta, CreateDictionaryValue("{'prop': {'name1': 4}}"))); + timestamp + time_delta, + ValueMap{{"prop.name1", test::make_int_prop_value(4)}})); auto changes = queue_->GetAndClearRecordedStateChanges(); EXPECT_EQ(4, queue_->GetLastStateChangeId()); ASSERT_EQ(2, changes.size()); - const std::string expected1 = "{'prop': {'name1': 3, 'name2': 2}}"; - const std::string expected2 = "{'prop': {'name1': 4}}"; + ValueMap expected1{ + {"prop.name1", test::make_int_prop_value(3)}, + {"prop.name2", test::make_int_prop_value(2)}, + }; + ValueMap expected2{ + {"prop.name1", test::make_int_prop_value(4)}, + }; EXPECT_EQ(timestamp, changes[0].timestamp); - EXPECT_JSON_EQ(expected1, *changes[0].changed_properties); + EXPECT_EQ(expected1, changes[0].changed_properties); EXPECT_EQ(timestamp + time_delta, changes[1].timestamp); - EXPECT_JSON_EQ(expected2, *changes[1].changed_properties); + EXPECT_EQ(expected2, changes[1].changed_properties); } TEST_F(StateChangeQueueTest, MaxQueueSize) { @@ -101,29 +109,43 @@ base::TimeDelta time_delta2 = base::TimeDelta::FromMinutes(3); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - start_time, CreateDictionaryValue("{'prop': {'name1': 1, 'name2': 2}}"))); + start_time, ValueMap{ + {"prop.name1", test::make_int_prop_value(1)}, + {"prop.name2", test::make_int_prop_value(2)}, + })); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( start_time + time_delta1, - CreateDictionaryValue("{'prop': {'name1': 3, 'name3': 4}}"))); + ValueMap{ + {"prop.name1", test::make_int_prop_value(3)}, + {"prop.name3", test::make_int_prop_value(4)}, + })); ASSERT_TRUE(queue_->NotifyPropertiesUpdated( start_time + time_delta2, - CreateDictionaryValue("{'prop': {'name10': 10, 'name11': 11}}"))); + ValueMap{ + {"prop.name10", test::make_int_prop_value(10)}, + {"prop.name11", test::make_int_prop_value(11)}, + })); EXPECT_EQ(3, queue_->GetLastStateChangeId()); auto changes = queue_->GetAndClearRecordedStateChanges(); ASSERT_EQ(2, changes.size()); - const std::string expected1 = - "{'prop': {'name1': 3, 'name2': 2, 'name3': 4}}"; + ValueMap expected1{ + {"prop.name1", test::make_int_prop_value(3)}, + {"prop.name2", test::make_int_prop_value(2)}, + {"prop.name3", test::make_int_prop_value(4)}, + }; EXPECT_EQ(start_time + time_delta1, changes[0].timestamp); - EXPECT_JSON_EQ(expected1, *changes[0].changed_properties); + EXPECT_EQ(expected1, changes[0].changed_properties); - const std::string expected2 = - "{'prop': {'name10': 10, 'name11': 11}}"; + ValueMap expected2{ + {"prop.name10", test::make_int_prop_value(10)}, + {"prop.name11", test::make_int_prop_value(11)}, + }; EXPECT_EQ(start_time + time_delta2, changes[1].timestamp); - EXPECT_JSON_EQ(expected2, *changes[1].changed_properties); + EXPECT_EQ(expected2, changes[1].changed_properties); } TEST_F(StateChangeQueueTest, ImmediateStateChangeNotification) { @@ -139,8 +161,10 @@ TEST_F(StateChangeQueueTest, DelayedStateChangeNotification) { // When queue is not empty, registering a new callback will not trigger it. ASSERT_TRUE(queue_->NotifyPropertiesUpdated( - base::Time::Now(), - CreateDictionaryValue("{'prop': {'name1': 1, 'name3': 2}}"))); + base::Time::Now(), ValueMap{ + {"prop.name1", test::make_int_prop_value(1)}, + {"prop.name2", test::make_int_prop_value(2)}, + })); auto callback = [](StateChangeQueueInterface::UpdateID id) { FAIL() << "This should not be called";
diff --git a/src/states/state_manager.cc b/src/states/state_manager.cc index a424588..fe6e669 100644 --- a/src/states/state_manager.cc +++ b/src/states/state_manager.cc
@@ -93,9 +93,8 @@ if (!package->SetPropertyValue(property_name, value, error)) return false; - std::unique_ptr<base::DictionaryValue> prop_set{new base::DictionaryValue}; - prop_set->Set(full_property_name, value.DeepCopy()); - state_change_queue_->NotifyPropertiesUpdated(timestamp, std::move(prop_set)); + ValueMap prop_set{{full_property_name, package->GetProperty(property_name)}}; + state_change_queue_->NotifyPropertiesUpdated(timestamp, prop_set); return true; }
diff --git a/src/states/state_manager_unittest.cc b/src/states/state_manager_unittest.cc index 933e11b..3f854fe 100644 --- a/src/states/state_manager_unittest.cc +++ b/src/states/state_manager_unittest.cc
@@ -22,18 +22,17 @@ using testing::_; using testing::Return; -using testing::ReturnRef; using test::CreateDictionaryValue; namespace { const char kBaseDefinition[] = R"({ "base": { - "manufacturer":{"type":"string"}, - "serialNumber":{"type":"string"} + "manufacturer":"string", + "serialNumber":"string" }, "device": { - "state_property":{"type":"string"} + "state_property":"string" } })"; @@ -52,10 +51,6 @@ return CreateDictionaryValue(kBaseDefaults); } -MATCHER_P(IsState, str, "") { - return arg.Equals(CreateDictionaryValue(str).get()); -} - } // anonymous namespace class StateManagerTest : public ::testing::Test { @@ -63,9 +58,9 @@ void SetUp() override { // Initial expectations. EXPECT_CALL(mock_state_change_queue_, IsEmpty()).Times(0); - EXPECT_CALL(mock_state_change_queue_, MockNotifyPropertiesUpdated(_, _)) + EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_, _)) .WillRepeatedly(Return(true)); - EXPECT_CALL(mock_state_change_queue_, MockGetAndClearRecordedStateChanges()) + EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges()) .Times(0); mgr_.reset(new StateManager(&mock_state_change_queue_)); @@ -107,7 +102,9 @@ 'manufacturer': 'Test Factory', 'serialNumber': 'Test Model' }, - 'device': {} + 'device': { + 'state_property': '' + } })"; EXPECT_JSON_EQ(expected, *mgr_->GetState()); } @@ -125,8 +122,12 @@ 'manufacturer': 'Test Factory', 'serialNumber': 'Test Model' }, - 'power': {}, - 'device': {} + 'power': { + 'battery_level': 0 + }, + 'device': { + 'state_property': '' + } })"; EXPECT_JSON_EQ(expected, *mgr_->GetState()); } @@ -136,15 +137,12 @@ auto state_definition = R"({ "base": { - "firmwareVersion": {"type":"string"}, - "localDiscoveryEnabled": {"type":"boolean"}, - "localAnonymousAccessMaxRole": { - "type": "string", - "enum": ["none", "viewer", "user"] - }, - "localPairingEnabled": {"type":"boolean"} + "firmwareVersion": "string", + "localDiscoveryEnabled": "boolean", + "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ], + "localPairingEnabled": "boolean" }, - "power": {"battery_level":{"type":"integer"}} + "power": {"battery_level":"integer"} })"; ASSERT_TRUE(manager.LoadStateDefinitionFromJson(state_definition, nullptr)); @@ -174,9 +172,11 @@ } TEST_F(StateManagerTest, SetPropertyValue) { - const std::string state = "{'device': {'state_property': 'Test Value'}}"; + ValueMap expected_prop_set{ + {"device.state_property", test::make_string_prop_value("Test Value")}, + }; EXPECT_CALL(mock_state_change_queue_, - MockNotifyPropertiesUpdated(timestamp_, IsState(state))) + NotifyPropertiesUpdated(timestamp_, expected_prop_set)) .WillOnce(Return(true)); ASSERT_TRUE(SetPropertyValue("device.state_property", base::StringValue{"Test Value"}, nullptr)); @@ -216,36 +216,39 @@ } TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownProperty) { - ASSERT_TRUE( - SetPropertyValue("base.level", base::FundamentalValue{0}, nullptr)); + ErrorPtr error; + ASSERT_FALSE( + SetPropertyValue("base.level", base::FundamentalValue{0}, &error)); + EXPECT_EQ(errors::state::kDomain, error->GetDomain()); + EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode()); } TEST_F(StateManagerTest, GetAndClearRecordedStateChanges) { - EXPECT_CALL(mock_state_change_queue_, - MockNotifyPropertiesUpdated(timestamp_, _)) + EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp_, _)) .WillOnce(Return(true)); ASSERT_TRUE(SetPropertyValue("device.state_property", base::StringValue{"Test Value"}, nullptr)); - std::vector<StateChange> expected_state; - const std::string expected_val = - "{'device': {'state_property': 'Test Value'}}"; - expected_state.emplace_back( - timestamp_, - CreateDictionaryValue(expected_val)); - EXPECT_CALL(mock_state_change_queue_, MockGetAndClearRecordedStateChanges()) - .WillOnce(ReturnRef(expected_state)); + std::vector<StateChange> expected_val; + expected_val.emplace_back( + timestamp_, ValueMap{{"device.state_property", + test::make_string_prop_value("Test Value")}}); + EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges()) + .WillOnce(Return(expected_val)); EXPECT_CALL(mock_state_change_queue_, GetLastStateChangeId()) .WillOnce(Return(0)); auto changes = mgr_->GetAndClearRecordedStateChanges(); ASSERT_EQ(1, changes.second.size()); - EXPECT_EQ(timestamp_, changes.second.back().timestamp); - EXPECT_JSON_EQ(expected_val, *changes.second.back().changed_properties); + EXPECT_EQ(expected_val.back().timestamp, changes.second.back().timestamp); + EXPECT_EQ(expected_val.back().changed_properties, + changes.second.back().changed_properties); } TEST_F(StateManagerTest, SetProperties) { - const std::string state = "{'base': {'manufacturer': 'No Name'}}"; + ValueMap expected_prop_set{ + {"base.manufacturer", test::make_string_prop_value("No Name")}, + }; EXPECT_CALL(mock_state_change_queue_, - MockNotifyPropertiesUpdated(_, IsState(state))) + NotifyPropertiesUpdated(_, expected_prop_set)) .WillOnce(Return(true)); EXPECT_CALL(*this, OnStateChanged()).Times(1); @@ -257,14 +260,16 @@ 'manufacturer': 'No Name', 'serialNumber': 'Test Model' }, - 'device': {} + 'device': { + 'state_property': '' + } })"; EXPECT_JSON_EQ(expected, *mgr_->GetState()); } TEST_F(StateManagerTest, GetProperty) { EXPECT_JSON_EQ("'Test Model'", *mgr_->GetProperty("base.serialNumber")); - EXPECT_EQ(nullptr, mgr_->GetProperty("device.state_property")); + EXPECT_JSON_EQ("''", *mgr_->GetProperty("device.state_property")); EXPECT_EQ(nullptr, mgr_->GetProperty("device.unknown")); EXPECT_EQ(nullptr, mgr_->GetProperty("unknown.state_property")); }
diff --git a/src/states/state_package.cc b/src/states/state_package.cc index 55650b6..0b8e219 100644 --- a/src/states/state_package.cc +++ b/src/states/state_package.cc
@@ -7,6 +7,9 @@ #include <base/logging.h> #include <base/values.h> +#include "src/commands/prop_types.h" +#include "src/commands/prop_values.h" +#include "src/commands/schema_utils.h" #include "src/states/error_codes.h" namespace weave { @@ -15,18 +18,28 @@ bool StatePackage::AddSchemaFromJson(const base::DictionaryValue* json, ErrorPtr* error) { + ObjectSchema schema; + if (!schema.FromJson(json, nullptr, error)) + return false; + // Scan first to make sure we have no property redefinitions. - for (base::DictionaryValue::Iterator it(*json); !it.IsAtEnd(); it.Advance()) { - if (types_.HasKey(it.key())) { + for (const auto& pair : schema.GetProps()) { + if (types_.GetProp(pair.first)) { Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain, errors::state::kPropertyRedefinition, "State property '%s.%s' is already defined", - name_.c_str(), it.key().c_str()); + name_.c_str(), pair.first.c_str()); return false; } } - types_.MergeDictionary(json); + // Now move all the properties to |types_| object. + for (const auto& pair : schema.GetProps()) { + types_.AddProp(pair.first, pair.second->Clone()); + // Create default value for this state property. + values_.emplace(pair.first, pair.second->CreateDefaultValue()); + } + return true; } @@ -40,14 +53,20 @@ } std::unique_ptr<base::DictionaryValue> StatePackage::GetValuesAsJson() const { - return std::unique_ptr<base::DictionaryValue>(values_.DeepCopy()); + std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + for (const auto& pair : values_) { + auto value = pair.second->ToJson(); + CHECK(value); + dict->SetWithoutPathExpansion(pair.first, value.release()); + } + return dict; } std::unique_ptr<base::Value> StatePackage::GetPropertyValue( const std::string& property_name, ErrorPtr* error) const { - const base::Value* value = nullptr; - if (!values_.Get(property_name, &value)) { + auto it = values_.find(property_name); + if (it == values_.end()) { Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain, errors::state::kPropertyNotDefined, "State property '%s.%s' is not defined", name_.c_str(), @@ -55,13 +74,24 @@ return nullptr; } - return std::unique_ptr<base::Value>(value->DeepCopy()); + return it->second->ToJson(); } bool StatePackage::SetPropertyValue(const std::string& property_name, const base::Value& value, ErrorPtr* error) { - values_.Set(property_name, value.DeepCopy()); + auto it = values_.find(property_name); + if (it == values_.end()) { + Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain, + errors::state::kPropertyNotDefined, + "State property '%s.%s' is not defined", name_.c_str(), + property_name.c_str()); + return false; + } + auto new_value = it->second->GetPropType()->CreatePropValue(value, error); + if (!new_value) + return false; + it->second = std::move(new_value); return true; }
diff --git a/src/states/state_package.h b/src/states/state_package.h index 7632145..e8e3964 100644 --- a/src/states/state_package.h +++ b/src/states/state_package.h
@@ -10,9 +10,15 @@ #include <string> #include <base/macros.h> -#include <base/values.h> #include <weave/error.h> +#include "src/commands/object_schema.h" +#include "src/commands/prop_values.h" + +namespace base { +class DictionaryValue; +} // namespace base + namespace weave { // A package is a set of related state properties. GCD specification defines @@ -56,13 +62,20 @@ const base::Value& value, ErrorPtr* error); + std::shared_ptr<const PropValue> GetProperty( + const std::string& property_name) const { + auto it = values_.find(property_name); + return it != values_.end() ? it->second + : std::shared_ptr<const PropValue>{}; + } + // Returns the name of the this package. const std::string& GetName() const { return name_; } private: std::string name_; - base::DictionaryValue types_; - base::DictionaryValue values_; + ObjectSchema types_; + ValueMap values_; friend class StatePackageTestHelper; DISALLOW_COPY_AND_ASSIGN(StatePackage);
diff --git a/src/states/state_package_unittest.cc b/src/states/state_package_unittest.cc index 5e1b933..f958bf5 100644 --- a/src/states/state_package_unittest.cc +++ b/src/states/state_package_unittest.cc
@@ -21,11 +21,11 @@ class StatePackageTestHelper { public: // Returns the state property definitions (types/constraints/etc). - static const base::DictionaryValue& GetTypes(const StatePackage& package) { + static const ObjectSchema& GetTypes(const StatePackage& package) { return package.types_; } // Returns the all state property values in this package. - static const base::DictionaryValue& GetValues(const StatePackage& package) { + static const ValueMap& GetValues(const StatePackage& package) { return package.values_; } }; @@ -33,30 +33,15 @@ namespace { std::unique_ptr<base::DictionaryValue> GetTestSchema() { return CreateDictionaryValue(R"({ - 'color': { - 'type': 'string' + 'light': 'boolean', + 'color': 'string', + 'direction':{ + 'properties':{ + 'azimuth': {'type': 'number', 'isRequired': true}, + 'altitude': {'maximum': 90.0} + } }, - 'direction': { - 'additionalProperties': false, - 'properties': { - 'altitude': { - 'maximum': 90.0, - 'type': 'number' - }, - 'azimuth': { - 'type': 'number' - } - }, - 'type': 'object', - 'required': [ 'azimuth' ] - }, - 'iso': { - 'enum': [50, 100, 200, 400], - 'type': 'integer' - }, - 'light': { - 'type': 'boolean' - } + 'iso': [50, 100, 200, 400] })"); } @@ -69,11 +54,11 @@ })"); } -inline const base::DictionaryValue& GetTypes(const StatePackage& package) { +inline const ObjectSchema& GetTypes(const StatePackage& package) { return StatePackageTestHelper::GetTypes(package); } // Returns the all state property values in this package. -inline const base::DictionaryValue& GetValues(const StatePackage& package) { +inline const ValueMap& GetValues(const StatePackage& package) { return StatePackageTestHelper::GetValues(package); } @@ -93,15 +78,15 @@ TEST(StatePackage, Empty) { StatePackage package("test"); EXPECT_EQ("test", package.GetName()); - EXPECT_TRUE(GetTypes(package).empty()); + EXPECT_TRUE(GetTypes(package).GetProps().empty()); EXPECT_TRUE(GetValues(package).empty()); } TEST(StatePackage, AddSchemaFromJson_OnEmpty) { StatePackage package("test"); ASSERT_TRUE(package.AddSchemaFromJson(GetTestSchema().get(), nullptr)); - EXPECT_EQ(4, GetTypes(package).size()); - EXPECT_EQ(0, GetValues(package).size()); + EXPECT_EQ(4, GetTypes(package).GetProps().size()); + EXPECT_EQ(4, GetValues(package).size()); auto expected = R"({ 'color': { @@ -129,9 +114,15 @@ 'type': 'boolean' } })"; - EXPECT_JSON_EQ(expected, GetTypes(package)); + EXPECT_JSON_EQ(expected, *GetTypes(package).ToJson(true, false)); - EXPECT_JSON_EQ("{}", *package.GetValuesAsJson()); + expected = R"({ + 'color': '', + 'direction': {}, + 'iso': 0, + 'light': false + })"; + EXPECT_JSON_EQ(expected, *package.GetValuesAsJson()); } TEST(StatePackage, AddValuesFromJson_OnEmpty) { @@ -152,13 +143,10 @@ } TEST_F(StatePackageTest, AddSchemaFromJson_AddMore) { - auto dict = CreateDictionaryValue(R"({'brightness':{ - 'enum': ['low', 'medium', 'high'], - 'type': 'string' - }})"); + auto dict = CreateDictionaryValue("{'brightness':['low', 'medium', 'high']}"); ASSERT_TRUE(package_->AddSchemaFromJson(dict.get(), nullptr)); - EXPECT_EQ(5, GetTypes(*package_).size()); - EXPECT_EQ(4, GetValues(*package_).size()); + EXPECT_EQ(5, GetTypes(*package_).GetProps().size()); + EXPECT_EQ(5, GetValues(*package_).size()); auto expected = R"({ 'brightness': { 'enum': ['low', 'medium', 'high'], @@ -189,9 +177,10 @@ 'type': 'boolean' } })"; - EXPECT_JSON_EQ(expected, GetTypes(*package_)); + EXPECT_JSON_EQ(expected, *GetTypes(*package_).ToJson(true, false)); expected = R"({ + 'brightness': '', 'color': 'white', 'direction': { 'altitude': 89.9, @@ -204,10 +193,7 @@ } TEST_F(StatePackageTest, AddValuesFromJson_AddMore) { - auto dict = CreateDictionaryValue(R"({'brightness':{ - 'enum': ['low', 'medium', 'high'], - 'type': 'string' - }})"); + auto dict = CreateDictionaryValue("{'brightness':['low', 'medium', 'high']}"); ASSERT_TRUE(package_->AddSchemaFromJson(dict.get(), nullptr)); dict = CreateDictionaryValue("{'brightness':'medium'}"); ASSERT_TRUE(package_->AddValuesFromJson(dict.get(), nullptr)); @@ -226,8 +212,7 @@ } TEST_F(StatePackageTest, AddSchemaFromJson_Error_Redefined) { - auto dict = CreateDictionaryValue(R"({'color': - {'type':'string', 'enum':['white', 'blue', 'red']}})"); + auto dict = CreateDictionaryValue("{'color':['white', 'blue', 'red']}"); ErrorPtr error; EXPECT_FALSE(package_->AddSchemaFromJson(dict.get(), &error)); EXPECT_EQ(errors::state::kDomain, error->GetDomain()); @@ -236,7 +221,10 @@ TEST_F(StatePackageTest, AddValuesFromJson_Error_Undefined) { auto dict = CreateDictionaryValue("{'brightness':'medium'}"); - EXPECT_TRUE(package_->AddValuesFromJson(dict.get(), nullptr)); + ErrorPtr error; + EXPECT_FALSE(package_->AddValuesFromJson(dict.get(), &error)); + EXPECT_EQ(errors::state::kDomain, error->GetDomain()); + EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode()); } TEST_F(StatePackageTest, GetPropertyValue) { @@ -286,4 +274,85 @@ EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson()); } +TEST_F(StatePackageTest, SetPropertyValue_Error_TypeMismatch) { + ErrorPtr error; + ASSERT_FALSE( + package_->SetPropertyValue("color", base::FundamentalValue{12}, &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kTypeMismatch, error->GetCode()); + error.reset(); + + ASSERT_FALSE( + package_->SetPropertyValue("iso", base::FundamentalValue{false}, &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kTypeMismatch, error->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_OutOfRange) { + ErrorPtr error; + ASSERT_FALSE( + package_->SetPropertyValue("iso", base::FundamentalValue{150}, &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kOutOfRange, error->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_Object_TypeMismatch) { + ErrorPtr error; + ASSERT_FALSE(package_->SetPropertyValue( + "direction", + *CreateDictionaryValue("{'altitude': 45.0, 'azimuth': '15'}"), &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kInvalidPropValue, error->GetCode()); + const Error* inner = error->GetInnerError(); + EXPECT_EQ(errors::commands::kDomain, inner->GetDomain()); + EXPECT_EQ(errors::commands::kTypeMismatch, inner->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_Object_OutOfRange) { + ErrorPtr error; + ASSERT_FALSE(package_->SetPropertyValue( + "direction", + *CreateDictionaryValue("{'altitude': 100.0, 'azimuth': 290.0}"), &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kInvalidPropValue, error->GetCode()); + const Error* inner = error->GetInnerError(); + EXPECT_EQ(errors::commands::kDomain, inner->GetDomain()); + EXPECT_EQ(errors::commands::kOutOfRange, inner->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_Object_UnknownProperty) { + ErrorPtr error; + ASSERT_FALSE(package_->SetPropertyValue( + "direction", *CreateDictionaryValue( + "{'altitude': 10.0, 'azimuth': 20.0, 'spin': 30.0}"), + &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kUnknownProperty, error->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Object_OptionalProperty) { + EXPECT_JSON_EQ("{'altitude': 89.9, 'azimuth': 57.2957795}", + *package_->GetProperty("direction")->ToJson()); + ASSERT_TRUE(package_->SetPropertyValue( + "direction", *CreateDictionaryValue("{'azimuth': 10.0}"), nullptr)); + EXPECT_JSON_EQ("{'azimuth': 10.0}", + *package_->GetProperty("direction")->ToJson()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_Object_MissingProperty) { + ErrorPtr error; + ASSERT_FALSE(package_->SetPropertyValue( + "direction", *CreateDictionaryValue("{'altitude': 10.0}"), &error)); + EXPECT_EQ(errors::commands::kDomain, error->GetDomain()); + EXPECT_EQ(errors::commands::kPropertyMissing, error->GetCode()); +} + +TEST_F(StatePackageTest, SetPropertyValue_Error_Unknown) { + ErrorPtr error; + ASSERT_FALSE(package_->SetPropertyValue("volume", base::FundamentalValue{100}, + &error)); + EXPECT_EQ(errors::state::kDomain, error->GetDomain()); + EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode()); +} + } // namespace weave
diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc index 2e41852..c2f1e8f 100644 --- a/src/weave_unittest.cc +++ b/src/weave_unittest.cc
@@ -72,7 +72,7 @@ "base": { "reboot": { "minimalRole": "user", - "parameters": {"delay": {"type": "integer"}}, + "parameters": {"delay": "integer"}, "results": {} }, "shutdown": {