buffet: Extract StorageInterface and implementations into separate files This will make it easy to reuse file storage and memory storage implementations in the forthcoming state aggregator. BUG=chromium:369322 TEST=Unittests Change-Id: Ie0dc0dbbfcc13e038352ecf77e2ad28bd41907d9 Reviewed-on: https://chromium-review.googlesource.com/198045 Tested-by: Christopher Wiley <wiley@chromium.org> Reviewed-by: Alex Vakulenko <avakulenko@chromium.org> Commit-Queue: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/buffet.gyp b/buffet/buffet.gyp index b94677c..ed036bd 100644 --- a/buffet/buffet.gyp +++ b/buffet/buffet.gyp
@@ -41,6 +41,7 @@ 'async_event_sequencer.cc', 'manager.cc', 'mime_utils.cc', + 'storage_impls.cc', 'string_utils.cc', 'url_utils.cc' ],
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc index c9aea9f..e1cadbf 100644 --- a/buffet/device_registration_info.cc +++ b/buffet/device_registration_info.cc
@@ -4,15 +4,14 @@ #include "buffet/device_registration_info.h" -#include <base/file_util.h> -#include <base/files/important_file_writer.h> -#include <base/json/json_reader.h> +#include <memory> + #include <base/json/json_writer.h> #include <base/values.h> -#include <memory> #include "buffet/data_encoding.h" #include "buffet/device_registration_storage_keys.h" +#include "buffet/storage_impls.h" #include "buffet/http_transport_curl.h" #include "buffet/http_utils.h" #include "buffet/mime_utils.h" @@ -94,41 +93,15 @@ return url::AppendQueryParams(result, params); } -class FileStorage : public buffet::DeviceRegistrationInfo::StorageInterface { - public: - virtual std::unique_ptr<base::Value> Load() override { - // TODO(avakulenko): Figure out security implications of storing - // this data unencrypted. - std::string json; - if (!base::ReadFileToString(GetFilePath(), &json)) - return std::unique_ptr<base::Value>(); - - return std::unique_ptr<base::Value>(base::JSONReader::Read(json)); - } - - virtual bool Save(const base::Value* config) { - // TODO(avakulenko): Figure out security implications of storing - // this data unencrypted. - std::string json; - base::JSONWriter::WriteWithOptions(config, - base::JSONWriter::OPTIONS_PRETTY_PRINT, - &json); - return base::ImportantFileWriter::WriteFileAtomically(GetFilePath(), json); - } - - private: - base::FilePath GetFilePath() const{ - return base::FilePath(kDeviceInfoFilePath); - } -}; - } // anonymous namespace namespace buffet { DeviceRegistrationInfo::DeviceRegistrationInfo() : transport_(new http::curl::Transport()), - storage_(new FileStorage()) { + // TODO(avakulenko): Figure out security implications of storing + // this data unencrypted. + storage_(new FileStorage(base::FilePath(kDeviceInfoFilePath))) { } DeviceRegistrationInfo::DeviceRegistrationInfo(
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h index c60cf69..f48bae9 100644 --- a/buffet/device_registration_info.h +++ b/buffet/device_registration_info.h
@@ -14,6 +14,7 @@ #include "buffet/data_encoding.h" #include "buffet/http_transport.h" +#include "buffet/storage_interface.h" namespace base { class Value; @@ -24,19 +25,6 @@ // The DeviceRegistrationInfo class represents device registration information. class DeviceRegistrationInfo { public: - // The device registration configuration storage interface. - struct StorageInterface { - virtual ~StorageInterface() = default; - // Load the device registration configuration from storage. - // If it fails (e.g. the storage container [file?] doesn't exist), then - // it returns empty unique_ptr (aka nullptr). - virtual std::unique_ptr<base::Value> Load() = 0; - // Save the device registration configuration to storage. - // If saved successfully, returns true. Could fail when writing to - // physical storage like file system for various reasons (out of disk space, - // access permissions, etc). - virtual bool Save(const base::Value* config) = 0; - }; // This is a helper class for unit testing. class TestHelper; // Default-constructed uses CURL HTTP transport.
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc index 2357904..93584b4 100644 --- a/buffet/device_registration_info_unittest.cc +++ b/buffet/device_registration_info_unittest.cc
@@ -12,30 +12,13 @@ #include "buffet/http_request.h" #include "buffet/http_transport_fake.h" #include "buffet/mime_utils.h" +#include "buffet/storage_impls.h" using namespace buffet; using namespace chromeos; using namespace chromeos::http; namespace { -// StorageInterface for testing. Just stores the values in memory. -class MemStorage : public DeviceRegistrationInfo::StorageInterface { - public: - virtual std::unique_ptr<base::Value> Load() override { - return std::unique_ptr<base::Value>(cache_->DeepCopy()); - } - - virtual bool Save(const base::Value* config) { - cache_.reset(config->DeepCopy()); - ++save_count_; - return true; - } - - int save_count_ = 0; - -private: - std::unique_ptr<base::Value> cache_; -}; namespace test_data { @@ -332,10 +315,10 @@ transport->AddHandler(dev_reg->GetOAuthURL("token"), request_type::kPost, base::Bind(OAuth2Handler)); - storage->save_count_ = 0; + storage->reset_save_count(); DeviceRegistrationInfo::TestHelper::SetTestTicketId(dev_reg.get()); EXPECT_TRUE(dev_reg->FinishRegistration("")); - EXPECT_EQ(1, storage->save_count_); // The device info must have been saved. + EXPECT_EQ(1, storage->save_count()); // The device info must have been saved. EXPECT_EQ(2, transport->GetRequestCount()); // Validate the device info saved to storage... @@ -401,9 +384,9 @@ transport->AddHandler(ticket_url, request_type::kPatch, base::Bind(email_patch_handler)); - storage->save_count_ = 0; + storage->reset_save_count(); DeviceRegistrationInfo::TestHelper::SetTestTicketId(dev_reg.get()); EXPECT_TRUE(dev_reg->FinishRegistration(test_data::kUserAccountAuthCode)); - EXPECT_EQ(1, storage->save_count_); // The device info must have been saved. + EXPECT_EQ(1, storage->save_count()); // The device info must have been saved. EXPECT_EQ(4, transport->GetRequestCount()); }
diff --git a/buffet/storage_impls.cc b/buffet/storage_impls.cc new file mode 100644 index 0000000..2d2531d --- /dev/null +++ b/buffet/storage_impls.cc
@@ -0,0 +1,42 @@ +// Copyright 2014 The Chromium OS 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 "buffet/storage_impls.h" + +#include <base/files/important_file_writer.h> +#include <base/json/json_reader.h> +#include <base/json/json_writer.h> + +namespace buffet { + +FileStorage::FileStorage(const base::FilePath& file_path) + : file_path_(file_path) { } + +std::unique_ptr<base::Value> FileStorage::Load() { + std::string json; + if (!base::ReadFileToString(file_path_, &json)) + return std::unique_ptr<base::Value>(); + + return std::unique_ptr<base::Value>(base::JSONReader::Read(json)); +} + +bool FileStorage::Save(const base::Value* config) { + std::string json; + base::JSONWriter::WriteWithOptions( + config, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json); + return base::ImportantFileWriter::WriteFileAtomically(file_path_, json); +} + + +std::unique_ptr<base::Value> MemStorage::Load() { + return std::unique_ptr<base::Value>(cache_->DeepCopy()); +} + +bool MemStorage::Save(const base::Value* config) { + cache_.reset(config->DeepCopy()); + ++save_count_; + return true; +} + +} // namespace buffet
diff --git a/buffet/storage_impls.h b/buffet/storage_impls.h new file mode 100644 index 0000000..04d5f0f --- /dev/null +++ b/buffet/storage_impls.h
@@ -0,0 +1,47 @@ +// Copyright 2014 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BUFFET_STORAGE_IMPLS_H_ +#define BUFFET_STORAGE_IMPLS_H_ + +#include <base/basictypes.h> +#include <base/file_util.h> +#include <base/values.h> + +#include "buffet/storage_interface.h" + +namespace buffet { + +// Persists the given Value to an atomically written file. +class FileStorage : public StorageInterface { + public: + FileStorage(const base::FilePath& file_path); + virtual ~FileStorage() = default; + virtual std::unique_ptr<base::Value> Load() override; + virtual bool Save(const base::Value* config) override; + + private: + base::FilePath file_path_; + DISALLOW_COPY_AND_ASSIGN(FileStorage); +}; + +// StorageInterface for testing. Just stores the values in memory. +class MemStorage : public StorageInterface { + public: + MemStorage() = default; + virtual ~MemStorage() = default; + virtual std::unique_ptr<base::Value> Load() override; + virtual bool Save(const base::Value* config) override; + int save_count() { return save_count_; } + void reset_save_count() { save_count_ = 0; } + + private: + int save_count_ = 0; + std::unique_ptr<base::Value> cache_; + DISALLOW_COPY_AND_ASSIGN(MemStorage); +}; + +} // namespace buffet + +#endif // BUFFET_STORAGE_IMPLS_H_
diff --git a/buffet/storage_interface.h b/buffet/storage_interface.h new file mode 100644 index 0000000..26d71c1 --- /dev/null +++ b/buffet/storage_interface.h
@@ -0,0 +1,32 @@ +// Copyright 2014 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BUFFET_STORAGE_INTERFACE_H_ +#define BUFFET_STORAGE_INTERFACE_H_ + +#include <memory> + +#include <base/values.h> + +namespace buffet { + +// We need to persist data in a couple places, and it is convenient to hide +// the details of this storage behind an interface for test purposes. +class StorageInterface { + public: + // Load the device registration configuration from storage. + // If it fails (e.g. the storage container [file?] doesn't exist), then + // it returns empty unique_ptr (aka nullptr). + virtual std::unique_ptr<base::Value> Load() = 0; + + // Save the device registration configuration to storage. + // If saved successfully, returns true. Could fail when writing to + // physical storage like file system for various reasons (out of disk space, + // access permissions, etc). + virtual bool Save(const base::Value* config) = 0; +}; + +} // namespace buffet + +#endif // BUFFET_STORAGE_INTERFACE_H_