Replace Get* methods returning unique_ptr with reference alternative
Existing code created temporarily objects and returned them to the
client. It was not efficient and error-prone as client code could
retrieve pointers to internal objects without keeping unique_ptr alive.
Change-Id: I9e17c8d9f66dfc9f52ce9ffc9a31992b16b00461
Reviewed-on: https://weave-review.googlesource.com/1672
Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/examples/daemon/ledflasher/ledflasher.cc b/examples/daemon/ledflasher/ledflasher.cc
index 4733f18..9e4a9e1 100644
--- a/examples/daemon/ledflasher/ledflasher.cc
+++ b/examples/daemon/ledflasher/ledflasher.cc
@@ -66,10 +66,10 @@
return;
LOG(INFO) << "received command: " << cmd->GetName();
int32_t led_index = 0;
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
bool cmd_value = false;
- if (params->GetInteger("_led", &led_index) &&
- params->GetBoolean("_on", &cmd_value)) {
+ if (params.GetInteger("_led", &led_index) &&
+ params.GetBoolean("_on", &cmd_value)) {
// Display this command in terminal
LOG(INFO) << cmd->GetName() << " _led: " << led_index
<< ", _on: " << (cmd_value ? "true" : "false");
@@ -96,9 +96,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
int32_t led_index = 0;
- if (params->GetInteger("_led", &led_index)) {
+ if (params.GetInteger("_led", &led_index)) {
LOG(INFO) << cmd->GetName() << " _led: " << led_index;
led_index--;
led_status_[led_index] = ~led_status_[led_index];
diff --git a/examples/daemon/light/light.cc b/examples/daemon/light/light.cc
index 31e3a24..90680d2 100644
--- a/examples/daemon/light/light.cc
+++ b/examples/daemon/light/light.cc
@@ -122,9 +122,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
int32_t brightness_value = 0;
- if (params->GetInteger("brightness", &brightness_value)) {
+ if (params.GetInteger("brightness", &brightness_value)) {
// Display this command in terminal.
LOG(INFO) << cmd->GetName() << " brightness: " << brightness_value;
@@ -146,9 +146,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
std::string requested_state;
- if (params->GetString("state", &requested_state)) {
+ if (params.GetString("state", &requested_state)) {
LOG(INFO) << cmd->GetName() << " state: " << requested_state;
bool new_light_status = requested_state == "on";
@@ -172,9 +172,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
- base::DictionaryValue* colorXY = nullptr;
- if (params->GetDictionary("_colorSetting", &colorXY)) {
+ const auto& params = cmd->GetParameters();
+ const base::DictionaryValue* colorXY = nullptr;
+ if (params.GetDictionary("_colorSetting", &colorXY)) {
bool updateState = false;
double X = 0.0;
double Y = 0.0;
diff --git a/examples/daemon/lock/lock.cc b/examples/daemon/lock/lock.cc
index 860020c..10bea00 100644
--- a/examples/daemon/lock/lock.cc
+++ b/examples/daemon/lock/lock.cc
@@ -72,9 +72,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
std::string requested_state;
- if (params->GetString("lockedState", &requested_state)) {
+ if (params.GetString("lockedState", &requested_state)) {
LOG(INFO) << cmd->GetName() << " state: " << requested_state;
weave::lockstate::LockState new_lock_status;
diff --git a/examples/daemon/sample/sample.cc b/examples/daemon/sample/sample.cc
index 2ad56e3..811e9fb 100644
--- a/examples/daemon/sample/sample.cc
+++ b/examples/daemon/sample/sample.cc
@@ -69,9 +69,9 @@
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
std::string name;
- if (!params->GetString("_name", &name)) {
+ if (!params.GetString("_name", &name)) {
weave::ErrorPtr error;
weave::Error::AddTo(&error, FROM_HERE, "example",
"invalid_parameter_value", "Name is missing");
@@ -94,7 +94,7 @@
base::DictionaryValue state;
state.SetInteger("_sample._ping_count", ++ping_count_);
device_->SetStateProperties(state, nullptr);
- LOG(INFO) << "New state: " << *device_->GetState();
+ LOG(INFO) << "New state: " << device_->GetState();
base::DictionaryValue result;
cmd->Complete(result, nullptr);
@@ -108,9 +108,9 @@
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
int seconds;
- if (!params->GetInteger("_seconds", &seconds))
+ if (!params.GetInteger("_seconds", &seconds))
seconds = 10;
LOG(INFO) << "starting countdown";
@@ -123,9 +123,9 @@
return;
if (seconds > 0) {
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
std::string todo;
- params->GetString("_todo", &todo);
+ params.GetString("_todo", &todo);
LOG(INFO) << "countdown tick: " << seconds << " seconds left";
base::DictionaryValue progress;
diff --git a/examples/daemon/speaker/speaker.cc b/examples/daemon/speaker/speaker.cc
index 89595b0..cd7d62f 100644
--- a/examples/daemon/speaker/speaker.cc
+++ b/examples/daemon/speaker/speaker.cc
@@ -72,10 +72,10 @@
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
// Handle volume parameter
int32_t volume_value = 0;
- if (params->GetInteger("volume", &volume_value)) {
+ if (params.GetInteger("volume", &volume_value)) {
// Display this command in terminal.
LOG(INFO) << cmd->GetName() << " volume: " << volume_value;
@@ -89,7 +89,7 @@
// Handle isMuted parameter
bool isMuted_status = false;
- if (params->GetBoolean("isMuted", &isMuted_status)) {
+ if (params.GetBoolean("isMuted", &isMuted_status)) {
// Display this command in terminal.
LOG(INFO) << cmd->GetName() << " is "
<< (isMuted_status ? "muted" : "not muted");
@@ -111,9 +111,9 @@
if (!cmd)
return;
LOG(INFO) << "received command: " << cmd->GetName();
- auto params = cmd->GetParameters();
+ const auto& params = cmd->GetParameters();
std::string requested_state;
- if (params->GetString("state", &requested_state)) {
+ if (params.GetString("state", &requested_state)) {
LOG(INFO) << cmd->GetName() << " state: " << requested_state;
bool new_speaker_status = requested_state == "on";
diff --git a/include/weave/command.h b/include/weave/command.h
index 59a9305..08ea782 100644
--- a/include/weave/command.h
+++ b/include/weave/command.h
@@ -40,13 +40,13 @@
virtual Command::Origin GetOrigin() const = 0;
// Returns the command parameters.
- virtual std::unique_ptr<base::DictionaryValue> GetParameters() const = 0;
+ virtual const base::DictionaryValue& GetParameters() const = 0;
// Returns the command progress.
- virtual std::unique_ptr<base::DictionaryValue> GetProgress() const = 0;
+ virtual const base::DictionaryValue& GetProgress() const = 0;
// Returns the command results.
- virtual std::unique_ptr<base::DictionaryValue> GetResults() const = 0;
+ virtual const base::DictionaryValue& GetResults() const = 0;
// Returns the command error.
virtual const Error* GetError() const = 0;
diff --git a/include/weave/device.h b/include/weave/device.h
index 19012b5..cbcc193 100644
--- a/include/weave/device.h
+++ b/include/weave/device.h
@@ -96,7 +96,7 @@
// Returns value of the single property.
// |name| is full property name, including package name. e.g. "base.network".
- virtual std::unique_ptr<base::Value> GetStateProperty(
+ virtual const base::Value* GetStateProperty(
const std::string& name) const = 0;
// Sets value of the single property.
@@ -106,7 +106,7 @@
ErrorPtr* error) = 0;
// Returns aggregated state properties across all registered packages.
- virtual std::unique_ptr<base::DictionaryValue> GetState() const = 0;
+ virtual const base::DictionaryValue& GetState() const = 0;
// Returns current state of GCD connection.
virtual GcdState GetGcdState() const = 0;
diff --git a/include/weave/test/mock_command.h b/include/weave/test/mock_command.h
index 2b1080e..fe1a02a 100644
--- a/include/weave/test/mock_command.h
+++ b/include/weave/test/mock_command.h
@@ -25,9 +25,9 @@
MOCK_CONST_METHOD0(GetCategory, const std::string&());
MOCK_CONST_METHOD0(GetState, Command::State());
MOCK_CONST_METHOD0(GetOrigin, Command::Origin());
- MOCK_CONST_METHOD0(MockGetParameters, const std::string&());
- MOCK_CONST_METHOD0(MockGetProgress, const std::string&());
- MOCK_CONST_METHOD0(MockGetResults, const std::string&());
+ MOCK_CONST_METHOD0(GetParameters, const base::DictionaryValue&());
+ MOCK_CONST_METHOD0(GetProgress, const base::DictionaryValue&());
+ MOCK_CONST_METHOD0(GetResults, const base::DictionaryValue&());
MOCK_CONST_METHOD0(GetError, const Error*());
MOCK_METHOD2(SetProgress, bool(const base::DictionaryValue&, ErrorPtr*));
MOCK_METHOD2(Complete, bool(const base::DictionaryValue&, ErrorPtr*));
@@ -35,10 +35,6 @@
MOCK_METHOD2(SetError, bool(const Error*, ErrorPtr*));
MOCK_METHOD2(Abort, bool(const Error*, ErrorPtr*));
MOCK_METHOD1(Cancel, bool(ErrorPtr*));
-
- std::unique_ptr<base::DictionaryValue> GetParameters() const override;
- std::unique_ptr<base::DictionaryValue> GetProgress() const override;
- std::unique_ptr<base::DictionaryValue> GetResults() const override;
};
} // namespace test
diff --git a/include/weave/test/mock_device.h b/include/weave/test/mock_device.h
index f751f97..e5063e7 100644
--- a/include/weave/test/mock_device.h
+++ b/include/weave/test/mock_device.h
@@ -34,13 +34,13 @@
MOCK_METHOD2(SetStatePropertiesFromJson, bool(const std::string&, ErrorPtr*));
MOCK_METHOD2(SetStateProperties,
bool(const base::DictionaryValue&, ErrorPtr*));
- MOCK_CONST_METHOD1(MockGetStateProperty,
- base::Value*(const std::string& name));
+ MOCK_CONST_METHOD1(GetStateProperty,
+ const base::Value*(const std::string& name));
MOCK_METHOD3(SetStateProperty,
bool(const std::string& name,
const base::Value& value,
ErrorPtr* error));
- MOCK_CONST_METHOD0(MockGetState, base::DictionaryValue*());
+ MOCK_CONST_METHOD0(GetState, const base::DictionaryValue&());
MOCK_CONST_METHOD0(GetGcdState, GcdState());
MOCK_METHOD1(AddGcdStateChangedCallback,
void(const GcdStateChangedCallback& callback));
@@ -50,15 +50,6 @@
MOCK_METHOD2(AddPairingChangedCallbacks,
void(const PairingBeginCallback& begin_callback,
const PairingEndCallback& end_callback));
-
- // Gmock 1.7.0 does not work with unuque_ptr as return value.
- std::unique_ptr<base::Value> GetStateProperty(
- const std::string& name) const override {
- return std::unique_ptr<base::Value>(MockGetStateProperty(name));
- }
- std::unique_ptr<base::DictionaryValue> GetState() const override {
- return std::unique_ptr<base::DictionaryValue>(MockGetState());
- }
};
} // namespace test
diff --git a/libweave.gypi b/libweave.gypi
index 24d6995..a02f082 100644
--- a/libweave.gypi
+++ b/libweave.gypi
@@ -54,7 +54,6 @@
'weave_test_sources': [
'src/test/fake_stream.cc',
'src/test/fake_task_runner.cc',
- 'src/test/mock_command.cc',
'src/test/unittest_utils.cc',
],
'weave_unittest_sources': [
diff --git a/src/base_api_handler.cc b/src/base_api_handler.cc
index 3d22a10..1423dd1 100644
--- a/src/base_api_handler.cc
+++ b/src/base_api_handler.cc
@@ -99,10 +99,10 @@
bool discovery_enabled{settings.local_discovery_enabled};
bool pairing_enabled{settings.local_pairing_enabled};
- auto parameters = command->GetParameters();
- parameters->GetString("localAnonymousAccessMaxRole", &anonymous_access_role);
- parameters->GetBoolean("localDiscoveryEnabled", &discovery_enabled);
- parameters->GetBoolean("localPairingEnabled", &pairing_enabled);
+ const auto& parameters = command->GetParameters();
+ parameters.GetString("localAnonymousAccessMaxRole", &anonymous_access_role);
+ parameters.GetBoolean("localDiscoveryEnabled", &discovery_enabled);
+ parameters.GetBoolean("localPairingEnabled", &pairing_enabled);
AuthScope auth_scope{AuthScope::kNone};
if (!StringToEnum(anonymous_access_role, &auth_scope)) {
@@ -144,10 +144,10 @@
std::string description{settings.description};
std::string location{settings.location};
- auto parameters = command->GetParameters();
- parameters->GetString("name", &name);
- parameters->GetString("description", &description);
- parameters->GetString("location", &location);
+ const auto& parameters = command->GetParameters();
+ parameters.GetString("name", &name);
+ parameters.GetString("description", &description);
+ parameters.GetString("location", &location);
device_info_->UpdateDeviceInfo(name, description, location);
command->Complete({}, nullptr);
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc
index 357385d..15a575a 100644
--- a/src/base_api_handler_unittest.cc
+++ b/src/base_api_handler_unittest.cc
@@ -84,7 +84,8 @@
}
std::unique_ptr<base::DictionaryValue> GetBaseState() {
- auto state = state_manager_->GetState();
+ std::unique_ptr<base::DictionaryValue> state{
+ state_manager_->GetState().DeepCopy()};
std::set<std::string> result;
for (base::DictionaryValue::Iterator it{*state}; !it.IsAtEnd();
it.Advance()) {
diff --git a/src/commands/cloud_command_proxy.cc b/src/commands/cloud_command_proxy.cc
index 91db18c..3d472c7 100644
--- a/src/commands/cloud_command_proxy.cc
+++ b/src/commands/cloud_command_proxy.cc
@@ -43,7 +43,7 @@
void CloudCommandProxy::OnResultsChanged() {
std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
patch->Set(commands::attributes::kCommand_Results,
- command_instance_->GetResults().release());
+ command_instance_->GetResults().CreateDeepCopy());
QueueCommandUpdate(std::move(patch));
}
@@ -57,7 +57,7 @@
void CloudCommandProxy::OnProgressChanged() {
std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
patch->Set(commands::attributes::kCommand_Progress,
- command_instance_->GetProgress().release());
+ command_instance_->GetProgress().CreateDeepCopy());
QueueCommandUpdate(std::move(patch));
}
diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc
index aa71b0e..8328299 100644
--- a/src/commands/command_instance.cc
+++ b/src/commands/command_instance.cc
@@ -94,16 +94,16 @@
return origin_;
}
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetParameters() const {
- return std::unique_ptr<base::DictionaryValue>(parameters_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetParameters() const {
+ return parameters_;
}
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetProgress() const {
- return std::unique_ptr<base::DictionaryValue>(progress_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetProgress() const {
+ return progress_;
}
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetResults() const {
- return std::unique_ptr<base::DictionaryValue>(results_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetResults() const {
+ return results_;
}
const Error* CommandInstance::GetError() const {
diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h
index e7a6eca..60620a1 100644
--- a/src/commands/command_instance.h
+++ b/src/commands/command_instance.h
@@ -54,9 +54,9 @@
const std::string& GetName() const override;
Command::State GetState() const override;
Command::Origin GetOrigin() const override;
- std::unique_ptr<base::DictionaryValue> GetParameters() const override;
- std::unique_ptr<base::DictionaryValue> GetProgress() const override;
- std::unique_ptr<base::DictionaryValue> GetResults() const override;
+ const base::DictionaryValue& GetParameters() const override;
+ const base::DictionaryValue& GetProgress() const override;
+ const base::DictionaryValue& GetResults() const override;
const Error* GetError() const override;
bool SetProgress(const base::DictionaryValue& progress,
ErrorPtr* error) override;
diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index 1d32cd9..fb8fe84 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc
@@ -82,8 +82,8 @@
EXPECT_EQ("robot.speak", instance.GetName());
EXPECT_EQ(Command::Origin::kCloud, instance.GetOrigin());
EXPECT_JSON_EQ("{'phrase': 'iPityDaFool', 'volume': 5}",
- *instance.GetParameters());
- EXPECT_JSON_EQ("{'foo': 239}", *instance.GetResults());
+ instance.GetParameters());
+ EXPECT_JSON_EQ("{'foo': 239}", instance.GetResults());
CommandInstance instance2{"base.reboot",
Command::Origin::kLocal,
@@ -118,7 +118,7 @@
EXPECT_EQ("abcd", instance->GetID());
EXPECT_EQ("robot.jump", instance->GetName());
EXPECT_JSON_EQ("{'height': 53, '_jumpType': '_withKick'}",
- *instance->GetParameters());
+ instance->GetParameters());
}
TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) {
@@ -126,7 +126,7 @@
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
dict_, nullptr, nullptr);
EXPECT_EQ("base.reboot", instance->GetName());
- EXPECT_JSON_EQ("{}", *instance->GetParameters());
+ EXPECT_JSON_EQ("{}", instance->GetParameters());
}
TEST_F(CommandInstanceTest, FromJson_NotObject) {
diff --git a/src/device_manager.cc b/src/device_manager.cc
index c0b8e9a..52b2882 100644
--- a/src/device_manager.cc
+++ b/src/device_manager.cc
@@ -134,7 +134,7 @@
return state_manager_->SetProperties(dict, error);
}
-std::unique_ptr<base::Value> DeviceManager::GetStateProperty(
+const base::Value* DeviceManager::GetStateProperty(
const std::string& name) const {
return state_manager_->GetProperty(name);
}
@@ -145,7 +145,7 @@
return state_manager_->SetProperty(name, value, error);
}
-std::unique_ptr<base::DictionaryValue> DeviceManager::GetState() const {
+const base::DictionaryValue& DeviceManager::GetState() const {
return state_manager_->GetState();
}
diff --git a/src/device_manager.h b/src/device_manager.h
index ccf8778..6c3df05 100644
--- a/src/device_manager.h
+++ b/src/device_manager.h
@@ -52,12 +52,11 @@
ErrorPtr* error) override;
bool SetStateProperties(const base::DictionaryValue& dict,
ErrorPtr* error) override;
- std::unique_ptr<base::Value> GetStateProperty(
- const std::string& name) const override;
+ const base::Value* GetStateProperty(const std::string& name) const override;
bool SetStateProperty(const std::string& name,
const base::Value& value,
ErrorPtr* error) override;
- std::unique_ptr<base::DictionaryValue> GetState() const override;
+ const base::DictionaryValue& GetState() const override;
void Register(const std::string& ticket_id,
const DoneCallback& callback) override;
GcdState GetGcdState() const override;
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc
index 5c8625a..0c914b7 100644
--- a/src/device_registration_info.cc
+++ b/src/device_registration_info.cc
@@ -485,8 +485,7 @@
if (!commands)
return nullptr;
- std::unique_ptr<base::DictionaryValue> state = state_manager_->GetState();
- CHECK(state);
+ const base::DictionaryValue& state = state_manager_->GetState();
std::unique_ptr<base::DictionaryValue> resource{new base::DictionaryValue};
if (!GetSettings().cloud_id.empty())
@@ -507,7 +506,7 @@
}
resource->Set("channel", channel.release());
resource->Set("commandDefs", commands.release());
- resource->Set("state", state.release());
+ resource->Set("state", state.DeepCopy());
return resource;
}
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc
index efb69bd..c771366 100644
--- a/src/privet/cloud_delegate.cc
+++ b/src/privet/cloud_delegate.cc
@@ -243,9 +243,8 @@
void OnStateChanged() {
state_.Clear();
- auto state = state_manager_->GetState();
- CHECK(state);
- state_.MergeDictionary(state.get());
+ const auto& state = state_manager_->GetState();
+ state_.MergeDictionary(&state);
NotifyOnStateChanged();
}
diff --git a/src/states/state_manager.cc b/src/states/state_manager.cc
index b1e70a3..b170b86 100644
--- a/src/states/state_manager.cc
+++ b/src/states/state_manager.cc
@@ -27,14 +27,17 @@
callback.Run(); // Force to read current state.
}
-std::unique_ptr<base::DictionaryValue> StateManager::GetState() const {
- std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue};
- for (const auto& pair : packages_) {
- auto pkg_value = pair.second->GetValuesAsJson();
- CHECK(pkg_value);
- dict->SetWithoutPathExpansion(pair.first, pkg_value.release());
+const base::DictionaryValue& StateManager::GetState() const {
+ if (!cached_dict_valid_) {
+ cached_dict_.Clear();
+ for (const auto& pair : packages_) {
+ cached_dict_.SetWithoutPathExpansion(
+ pair.first, pair.second->GetValuesAsJson().DeepCopy());
+ }
+ cached_dict_valid_ = true;
}
- return dict;
+
+ return cached_dict_;
}
bool StateManager::SetProperty(const std::string& name,
@@ -46,8 +49,7 @@
return result;
}
-std::unique_ptr<base::Value> StateManager::GetProperty(
- const std::string& name) const {
+const base::Value* StateManager::GetProperty(const std::string& name) const {
auto parts = SplitAtFirst(name, ".", true);
const std::string& package_name = parts.first;
const std::string& property_name = parts.second;
@@ -93,6 +95,8 @@
if (!package->SetPropertyValue(property_name, value, error))
return false;
+ cached_dict_valid_ = 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));
diff --git a/src/states/state_manager.h b/src/states/state_manager.h
index 55faf76..c48a57c 100644
--- a/src/states/state_manager.h
+++ b/src/states/state_manager.h
@@ -39,11 +39,11 @@
bool LoadStateDefinitionFromJson(const std::string& json, ErrorPtr* error);
bool SetProperties(const base::DictionaryValue& dict, ErrorPtr* error);
bool SetPropertiesFromJson(const std::string& json, ErrorPtr* error);
- std::unique_ptr<base::Value> GetProperty(const std::string& name) const;
+ const base::Value* GetProperty(const std::string& name) const;
bool SetProperty(const std::string& name,
const base::Value& value,
ErrorPtr* error);
- std::unique_ptr<base::DictionaryValue> GetState() const;
+ const base::DictionaryValue& GetState() const;
// Returns the recorded state changes since last time this method has been
// called.
@@ -80,6 +80,9 @@
std::vector<base::Closure> on_changed_;
+ mutable base::DictionaryValue cached_dict_;
+ mutable bool cached_dict_valid_ = false;
+
DISALLOW_COPY_AND_ASSIGN(StateManager);
};
diff --git a/src/states/state_manager_unittest.cc b/src/states/state_manager_unittest.cc
index f655e84..6899646 100644
--- a/src/states/state_manager_unittest.cc
+++ b/src/states/state_manager_unittest.cc
@@ -109,7 +109,7 @@
},
'device': {}
})";
- EXPECT_JSON_EQ(expected, *mgr_->GetState());
+ EXPECT_JSON_EQ(expected, mgr_->GetState());
}
TEST_F(StateManagerTest, LoadStateDefinition) {
@@ -128,7 +128,7 @@
'power': {},
'device': {}
})";
- EXPECT_JSON_EQ(expected, *mgr_->GetState());
+ EXPECT_JSON_EQ(expected, mgr_->GetState());
}
TEST_F(StateManagerTest, Startup) {
@@ -170,7 +170,7 @@
'battery_level': 44
}
})";
- EXPECT_JSON_EQ(expected, *manager.GetState());
+ EXPECT_JSON_EQ(expected, manager.GetState());
}
TEST_F(StateManagerTest, SetPropertyValue) {
@@ -189,7 +189,7 @@
'state_property': 'Test Value'
}
})";
- EXPECT_JSON_EQ(expected, *mgr_->GetState());
+ EXPECT_JSON_EQ(expected, mgr_->GetState());
}
TEST_F(StateManagerTest, SetPropertyValue_Error_NoName) {
@@ -227,7 +227,7 @@
ASSERT_TRUE(SetPropertyValue("device.state_property",
base::StringValue{"Test Value"}, nullptr));
std::vector<StateChange> expected_state;
- const std::string expected_val =
+ const std::string expected_val =
"{'device': {'state_property': 'Test Value'}}";
expected_state.emplace_back(
timestamp_,
@@ -259,7 +259,7 @@
},
'device': {}
})";
- EXPECT_JSON_EQ(expected, *mgr_->GetState());
+ EXPECT_JSON_EQ(expected, mgr_->GetState());
}
TEST_F(StateManagerTest, GetProperty) {
diff --git a/src/states/state_package.cc b/src/states/state_package.cc
index 55650b6..b0ea199 100644
--- a/src/states/state_package.cc
+++ b/src/states/state_package.cc
@@ -39,11 +39,11 @@
return true;
}
-std::unique_ptr<base::DictionaryValue> StatePackage::GetValuesAsJson() const {
- return std::unique_ptr<base::DictionaryValue>(values_.DeepCopy());
+const base::DictionaryValue& StatePackage::GetValuesAsJson() const {
+ return values_;
}
-std::unique_ptr<base::Value> StatePackage::GetPropertyValue(
+const base::Value* StatePackage::GetPropertyValue(
const std::string& property_name,
ErrorPtr* error) const {
const base::Value* value = nullptr;
@@ -55,7 +55,7 @@
return nullptr;
}
- return std::unique_ptr<base::Value>(value->DeepCopy());
+ return value;
}
bool StatePackage::SetPropertyValue(const std::string& property_name,
diff --git a/src/states/state_package.h b/src/states/state_package.h
index 7632145..4092948 100644
--- a/src/states/state_package.h
+++ b/src/states/state_package.h
@@ -43,13 +43,12 @@
// "message": "Printer low on cyan ink"
// }
// }
- std::unique_ptr<base::DictionaryValue> GetValuesAsJson() const;
+ const base::DictionaryValue& GetValuesAsJson() const;
// Gets the value for a specific state property. |property_name| must not
// include the package name as part of the property name.
- std::unique_ptr<base::Value> GetPropertyValue(
- const std::string& property_name,
- ErrorPtr* error) const;
+ const base::Value* GetPropertyValue(const std::string& property_name,
+ ErrorPtr* error) const;
// Sets the value for a specific state property. |property_name| must not
// include the package name as part of the property name.
bool SetPropertyValue(const std::string& property_name,
diff --git a/src/states/state_package_unittest.cc b/src/states/state_package_unittest.cc
index c116b37..d09625a 100644
--- a/src/states/state_package_unittest.cc
+++ b/src/states/state_package_unittest.cc
@@ -131,7 +131,7 @@
})";
EXPECT_JSON_EQ(expected, GetTypes(package));
- EXPECT_JSON_EQ("{}", *package.GetValuesAsJson());
+ EXPECT_JSON_EQ("{}", package.GetValuesAsJson());
}
TEST(StatePackage, AddValuesFromJson_OnEmpty) {
@@ -148,7 +148,7 @@
'iso': 200,
'light': true
})";
- EXPECT_JSON_EQ(expected, *package.GetValuesAsJson());
+ EXPECT_JSON_EQ(expected, package.GetValuesAsJson());
}
TEST_F(StatePackageTest, AddSchemaFromJson_AddMore) {
@@ -200,7 +200,7 @@
'iso': 200,
'light': true
})";
- EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+ EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
}
TEST_F(StatePackageTest, AddValuesFromJson_AddMore) {
@@ -222,7 +222,7 @@
'iso': 200,
'light': true
})";
- EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+ EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
}
TEST_F(StatePackageTest, AddSchemaFromJson_Error_Redefined) {
@@ -283,7 +283,7 @@
'iso': 200,
'light': true
})";
- EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+ EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
}
} // namespace weave
diff --git a/src/test/mock_command.cc b/src/test/mock_command.cc
deleted file mode 100644
index 65c2bab..0000000
--- a/src/test/mock_command.cc
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright 2015 The Weave Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <weave/test/mock_command.h>
-
-#include <memory>
-#include <string>
-
-#include <base/values.h>
-#include <weave/test/unittest_utils.h>
-
-namespace weave {
-namespace test {
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetParameters() const {
- return CreateDictionaryValue(MockGetParameters());
-}
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetProgress() const {
- return CreateDictionaryValue(MockGetProgress());
-}
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetResults() const {
- return CreateDictionaryValue(MockGetResults());
-}
-
-} // namespace test
-} // namespace weave
