Refactorize Cynara::BucketDeserializer

BucketDeserializer is no longer stateless.
The class is now constructed with reference
on input stream. It's easier testable and enables
dependency injection in Cynara::StorageDeserializer.

TODO: Consider squashing with 2 prior commits.

Change-Id: If79727c7759178bd9ac82351b16f043971abd577

diff --git a/src/service/storage/BucketDeserializer.cpp b/src/service/storage/BucketDeserializer.cpp
index f067c69..c40cd8d 100644 (file)
--- a/src/service/storage/BucketDeserializer.cpp
+++ b/src/service/storage/BucketDeserializer.cpp
@@ -31,13 +31,13 @@
 
 namespace Cynara {
 
-PolicyCollection BucketDeserializer::loadPolicies(std::istream &is) {
+PolicyCollection BucketDeserializer::loadPolicies(void) {
     PolicyCollection policies;
 
     // TODO: Get someone smart to do error checking on stream
-    for(std::size_t lineNum = 1; !is.eof(); ++lineNum) {
+    for(std::size_t lineNum = 1; !m_inStream.eof(); ++lineNum) {
         std::string line;
-        std::getline(is, line, StorageSerializer::recordSeparator());
+        std::getline(m_inStream, line, StorageSerializer::recordSeparator());
 
         if (line.empty())
             break;

diff --git a/src/service/storage/BucketDeserializer.h b/src/service/storage/BucketDeserializer.h
index 2c50fab..5659c45 100644 (file)
--- a/src/service/storage/BucketDeserializer.h
+++ b/src/service/storage/BucketDeserializer.h
@@ -37,13 +37,17 @@ class BucketDeserializer {
 friend StorageDeserializer;
 
 public:
-    static PolicyCollection loadPolicies(std::istream &is);
+    BucketDeserializer(std::istream &inStream) : m_inStream(inStream) {}
+    PolicyCollection loadPolicies(void);
 
 protected:
     static PolicyKey parseKey(const std::string &line, std::size_t &beginToken);
     static PolicyType parsePolicyType(const std::string &line, std::size_t &beginToken);
     static PolicyResult::PolicyMetadata parseMetadata(const std::string &line,
                                                       std::size_t &beginToken);
+
+private:
+    std::istream &m_inStream;
 };
 
 } /* namespace Cynara */

diff --git a/src/service/storage/StorageDeserializer.cpp b/src/service/storage/StorageDeserializer.cpp
index af5e3c0..655f4a4 100644 (file)
--- a/src/service/storage/StorageDeserializer.cpp
+++ b/src/service/storage/StorageDeserializer.cpp
@@ -59,11 +59,9 @@ void StorageDeserializer::loadBuckets(InMemoryStorageBackend::Buckets &buckets)
         const auto &bucketId = bucketIter.first;
         auto &bucket = bucketIter.second;
 
-        auto inStream = m_bucketStreamOpener(bucketId);
-
-        if (inStream != nullptr && inStream->good()) {
-            auto policies = BucketDeserializer::loadPolicies(*inStream);
-            bucket.setPolicyCollection(policies);
+        auto bucketDeserializer = m_bucketStreamOpener(bucketId);
+        if (bucketDeserializer != nullptr) {
+            bucket.setPolicyCollection(bucketDeserializer->loadPolicies());
         } else {
             // TODO: Throw?
         }

diff --git a/src/service/storage/StorageDeserializer.h b/src/service/storage/StorageDeserializer.h
index e7e799b..32310ac 100644 (file)
--- a/src/service/storage/StorageDeserializer.h
+++ b/src/service/storage/StorageDeserializer.h
@@ -22,6 +22,7 @@
 #ifndef SRC_SERVICE_STORAGE_STORAGEDESERIALIZER_H_
 #define SRC_SERVICE_STORAGE_STORAGEDESERIALIZER_H_
 
+#include <service/storage/BucketDeserializer.h>
 #include <service/storage/InMemoryStorageBackend.h>
 
 #include <istream>
@@ -32,7 +33,7 @@ namespace Cynara {
 
 class StorageDeserializer {
 public:
-    typedef std::function<std::shared_ptr<std::istream>(const std::string &)> BucketStreamOpener;
+    typedef std::function<std::shared_ptr<BucketDeserializer>(const std::string &)> BucketStreamOpener;
     StorageDeserializer(std::istream &inStream, BucketStreamOpener m_bucketStreamOpener);
     void initBuckets(InMemoryStorageBackend::Buckets &buckets);
     void loadBuckets(InMemoryStorageBackend::Buckets &buckets);

diff --git a/test/storage/serializer/bucket_load.cpp b/test/storage/serializer/bucket_load.cpp
index 0c2dcf6..87d6fb0 100644 (file)
--- a/test/storage/serializer/bucket_load.cpp
+++ b/test/storage/serializer/bucket_load.cpp
@@ -52,12 +52,13 @@ public:
 
     void checkCorruptedData(const std::string &data, const std::string &corruptedLine,
             size_t corruptedLineNumber) {
-        std::istringstream ss(data);
-        EXPECT_THROW(BucketDeserializer::loadPolicies(ss), BucketRecordCorruptedException);
+        std::istringstream bucketStream(data);
+        BucketDeserializer deserializer(bucketStream);
+        EXPECT_THROW(deserializer.loadPolicies(), BucketRecordCorruptedException);
 
-        ss.seekg(0);
+        bucketStream.seekg(0);
         try {
-            BucketDeserializer::loadPolicies(ss);
+            deserializer.loadPolicies();
         } catch (const BucketRecordCorruptedException &ex) {
             ASSERT_EQ(corruptedLine, ex.line());
             ASSERT_EQ(corruptedLineNumber, ex.lineNumber());
@@ -68,18 +69,20 @@ public:
 TEST_F(BucketDeserializerFixture, load_empty) {
     using ::testing::IsEmpty;
 
-    std::istringstream ss;
+    std::istringstream bucketStream;
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     ASSERT_THAT(policies, IsEmpty());
 }
 
 TEST_F(BucketDeserializerFixture, load_1) {
     using ::testing::UnorderedElementsAre;
 
-    std::istringstream ss("c;u;p;0;meta");
+    std::istringstream bucketStream("c;u;p;0;meta");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     auto expectedPolicy = createPolicy({ "c", "u", "p" }, { PredefinedPolicyType::DENY, "meta" });
 
     ASSERT_THAT(policies, UnorderedElementsAre(PolicyPtrEq(expectedPolicy)));
@@ -88,9 +91,10 @@ TEST_F(BucketDeserializerFixture, load_1) {
 TEST_F(BucketDeserializerFixture, load_1_allow) {
     using ::testing::UnorderedElementsAre;
 
-    std::istringstream ss("c;u;p;0xFFFF;meta");
+    std::istringstream bucketStream("c;u;p;0xFFFF;meta");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     auto expectedPolicy = createPolicy({ "c", "u", "p" }, { PredefinedPolicyType::ALLOW, "meta" });
 
     ASSERT_THAT(policies, UnorderedElementsAre(PolicyPtrEq(expectedPolicy)));
@@ -99,9 +103,10 @@ TEST_F(BucketDeserializerFixture, load_1_allow) {
 TEST_F(BucketDeserializerFixture, load_1_no_meta_sep) {
     using ::testing::UnorderedElementsAre;
 
-    std::istringstream ss("c;u;p;0;");
+    std::istringstream bucketStream("c;u;p;0;");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     auto expectedPolicy = createPolicy({ "c", "u", "p" }, { PredefinedPolicyType::DENY, "" });
 
     ASSERT_THAT(policies, UnorderedElementsAre(PolicyPtrEq(expectedPolicy)));
@@ -110,9 +115,10 @@ TEST_F(BucketDeserializerFixture, load_1_no_meta_sep) {
 TEST_F(BucketDeserializerFixture, load_1_no_meta_no_sep) {
     using ::testing::UnorderedElementsAre;
 
-    std::istringstream ss("c;u;p;0");
+    std::istringstream bucketStream("c;u;p;0");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     auto expectedPolicy = createPolicy({ "c", "u", "p" }, { PredefinedPolicyType::DENY, "" });
 
     ASSERT_THAT(policies, UnorderedElementsAre(PolicyPtrEq(expectedPolicy)));
@@ -121,10 +127,11 @@ TEST_F(BucketDeserializerFixture, load_1_no_meta_no_sep) {
 TEST_F(BucketDeserializerFixture, load_2) {
     using ::testing::UnorderedElementsAre;
 
-    std::istringstream ss("c;u;p;0;meta\n"
-                          "c;u;p;0;meta\n");
+    std::istringstream bucketStream("c;u;p;0;meta\n"
+                                    "c;u;p;0;meta\n");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     auto expectedPolicy = createPolicy({ "c", "u", "p" }, { PredefinedPolicyType::DENY, "meta" });
 
     ASSERT_THAT(policies, UnorderedElementsAre(PolicyPtrEq(expectedPolicy),
@@ -135,12 +142,13 @@ TEST_F(BucketDeserializerFixture, load_mixed) {
     using ::testing::UnorderedElementsAre;
     using ::testing::UnorderedElementsAreArray;
 
-    std::istringstream ss("c1;u1;p1;0;meta\n"
-                          "c2;u2;p2;0xFFFF;meta2\n"
-                          "c2;u2;p2;0xFFFF;\n"
-                          "c1;u1;p3;0xFFFE;bucket\n");
+    std::istringstream bucketStream("c1;u1;p1;0;meta\n"
+                                    "c2;u2;p2;0xFFFF;meta2\n"
+                                    "c2;u2;p2;0xFFFF;\n"
+                                    "c1;u1;p3;0xFFFE;bucket\n");
+    BucketDeserializer deserializer(bucketStream);
 
-    auto policies = BucketDeserializer::loadPolicies(ss);
+    auto policies = deserializer.loadPolicies();
     PolicyCollection expectedPolices = {
         createPolicy({ "c1", "u1", "p1" }, { PredefinedPolicyType::DENY, "meta" }),
         createPolicy({ "c2", "u2", "p2" }, { PredefinedPolicyType::ALLOW, "meta2" }),

diff --git a/test/storage/serializer/deserialize.cpp b/test/storage/serializer/deserialize.cpp
index 5bcc773..0ab39f6 100644 (file)
--- a/test/storage/serializer/deserialize.cpp
+++ b/test/storage/serializer/deserialize.cpp
@@ -51,15 +51,27 @@ MATCHER_P(PolicyBucketIdPolicyEq, expected, "") {
 
 class FakeStreamForBucketId {
 public:
-    MOCK_METHOD1(streamForBucketId, std::shared_ptr<std::istream>(const std::string &));
+    MOCK_METHOD1(streamForBucketId, std::shared_ptr<Cynara::BucketDeserializer>(const std::string &));
+};
+
+class EmptyBucketDeserializer : public Cynara::BucketDeserializer {
+public:
+    EmptyBucketDeserializer() : Cynara::BucketDeserializer(m_emptyStream),
+        m_emptyStream("") {}
+private:
+    std::istringstream m_emptyStream;
 };
 
 class StorageDeserializerFixture : public ::testing::Test {
 public:
+    std::shared_ptr<Cynara::BucketDeserializer> emptyBucketStream() const {
+        return std::make_shared<EmptyBucketDeserializer>();
+    }
+
     Cynara::StorageDeserializer::BucketStreamOpener nullStreamOpener =
-            [] (const std::string &) -> std::shared_ptr<std::istream> {
-                return nullptr;
-            };
+        [this] (const std::string &) -> std::shared_ptr<Cynara::BucketDeserializer> {
+            return emptyBucketStream();
+        };
 };
 
 using namespace Cynara;
@@ -127,9 +139,10 @@ TEST_F(StorageDeserializerFixture, load_buckets_plus_policies) {
                                       std::placeholders::_1);
     StorageDeserializer deserializer(bucketsStream, streamOpenerFunc);
 
-    auto defaultBucketStream = std::make_shared<std::istringstream>("c;u;p;0;meta");
+    std::istringstream defaultBucketStream("c;u;p;0;meta");
+    auto bucketDeserializer = std::make_shared<BucketDeserializer>(defaultBucketStream);
     EXPECT_CALL(streamOpener, streamForBucketId(""))
-        .WillOnce(Return(defaultBucketStream));
+        .WillOnce(Return(bucketDeserializer));
 
     deserializer.loadBuckets(buckets);