Update tests and remove unused Storage methods

Change-Id: I00456f5a087014d9e176ab1dd5bee86aa92c374b

diff --git a/src/service/storage/Storage.cpp b/src/service/storage/Storage.cpp
index 1b1a29a..60de8ff 100644 (file)
--- a/src/service/storage/Storage.cpp
+++ b/src/service/storage/Storage.cpp
@@ -79,20 +79,6 @@ PolicyResult Storage::minimalPolicy(const PolicyBucket &bucket, const PolicyKey
     return minimal;
 }
 
-//todo to be removed, after tests get updated
-void Storage::insertPolicies(const std::vector<PolicyPolicyBucket> &policies) {
-    for (const auto &policyTuple : policies) {
-        PolicyBucketId bucketId;
-        PolicyPtr policyPtr;
-        std::tie(policyPtr, bucketId) = policyTuple;
-        auto existingPolicies = m_backend.searchBucket(bucketId, policyPtr->key());
-        for (auto existingPolicy : existingPolicies.policyCollection()) {
-            m_backend.deletePolicy(bucketId, existingPolicy->key());
-        }
-        m_backend.insertPolicy(bucketId, policyPtr);
-    }
-}
-
 void Storage::insertPolicies(const std::map<PolicyBucketId, std::vector<Policy>> &policies) {
     for (const auto &bucket : policies) {
         const PolicyBucketId &bucketId = bucket.first;
@@ -126,13 +112,6 @@ void Storage::deleteBucket(const PolicyBucketId &bucketId) {
     m_backend.deleteBucket(bucketId);
 }
 
-//todo to be removed, after tests get updated
-void Storage::deletePolicies(const std::vector<PolicyKeyBucket> &policies) {
-    for (const auto &policy : policies) {
-        m_backend.deletePolicy(std::get<1>(policy), std::get<0>(policy));
-    }
-}
-
 void Storage::deletePolicies(const std::map<PolicyBucketId, std::vector<PolicyKey>> &policies) {
     for (const auto &bucket : policies) {
         const PolicyBucketId &bucketId = bucket.first;

diff --git a/src/service/storage/Storage.h b/src/service/storage/Storage.h
index f7e9c78..a52d1cc 100644 (file)
--- a/src/service/storage/Storage.h
+++ b/src/service/storage/Storage.h
@@ -42,19 +42,10 @@ class PolicyBucket;
 class Storage
 {
 public:
-    // TODO: These tuples are ugly -- refactorize
-//todo to be removed, after tests get updated
-    typedef std::tuple<PolicyPtr, PolicyBucketId> PolicyPolicyBucket;
-    typedef std::tuple<PolicyKey, PolicyBucketId> PolicyKeyBucket;
-
     Storage(StorageBackend &backend) : m_backend(backend) {}
 
     PolicyResult checkPolicy(const PolicyKey &key);
 
-//todo below to functions should be removed, after tests get updated
-    void insertPolicies(const std::vector<PolicyPolicyBucket> &policies);
-    void deletePolicies(const std::vector<PolicyKeyBucket> &policies);
-
     void insertPolicies(const std::map<PolicyBucketId, std::vector<Policy>> &policies);
     void deletePolicies(const std::map<PolicyBucketId, std::vector<PolicyKey>> &policies);
 

diff --git a/test/storage/storage/policies.cpp b/test/storage/storage/policies.cpp
index 8fcf900..5c593b5 100644 (file)
--- a/test/storage/storage/policies.cpp
+++ b/test/storage/storage/policies.cpp
@@ -36,8 +36,10 @@
 #include "fakestoragebackend.h"
 #include "../../helpers.h"
 
+#include <map>
 #include <memory>
 #include <tuple>
+#include <vector>
 
 using namespace Cynara;
 
@@ -50,19 +52,19 @@ TEST(storage, deletePolicies) {
 
     auto pk1 = Helpers::generatePolicyKey("1");
     auto pk2 = Helpers::generatePolicyKey("2");
-    PolicyBucketId bucketId1 = "bucket";
+    PolicyBucketId bucketId1 = "bucket-1";
     PolicyBucketId bucketId2 = "bucket-2";
 
-
     EXPECT_CALL(backend, deletePolicy(bucketId1, pk1));
     EXPECT_CALL(backend, deletePolicy(bucketId2, pk1));
     EXPECT_CALL(backend, deletePolicy(bucketId1, pk2));
 
-    storage.deletePolicies({
-        std::make_tuple(pk1, bucketId1),
-        std::make_tuple(pk1, bucketId2),
-        std::make_tuple(pk2, bucketId1),
-    });
+    std::map<PolicyBucketId, std::vector<PolicyKey>> policies;
+    policies[bucketId1].push_back(pk1);
+    policies[bucketId2].push_back(pk1);
+    policies[bucketId1].push_back(pk2);
+
+    storage.deletePolicies(policies);
 }
 
 // TODO: isn't it the same test as storage.deleteBucket?
@@ -81,27 +83,27 @@ TEST(storage, deleteBucketWithLinkedPolicies) {
 
 TEST(storage, insertPolicies) {
     using ::testing::Return;
+    using ::testing::_;
     FakeStorageBackend backend;
     Cynara::Storage storage(backend);
 
     PolicyBucketId testBucket1 = "test-bucket-1";
     PolicyBucketId testBucket2 = "test-bucket-2";
 
-    std::vector<Storage::PolicyPolicyBucket> policiesToInsert = {
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("1"), PredefinedPolicyType::ALLOW), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("2"), PredefinedPolicyType::DENY), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("3"), PredefinedPolicyType::DENY), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("4"), PredefinedPolicyType::ALLOW), testBucket2),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("5"), PredefinedPolicyType::ALLOW), testBucket2),
-    };
-
-    for (const auto &policy : policiesToInsert) {
-        PolicyBucketId bucketId;
-        PolicyPtr policyPtr;
-        std::tie(policyPtr, bucketId) = policy;
-        EXPECT_CALL(backend, searchBucket(bucketId, policyPtr->key()))
-            .WillOnce(Return(PolicyBucket()));
-        EXPECT_CALL(backend, insertPolicy(bucketId, policyPtr));
+    std::map<PolicyBucketId, std::vector<Policy>> policiesToInsert;
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("1"), PredefinedPolicyType::ALLOW));
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("2"), PredefinedPolicyType::DENY));
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("3"), PredefinedPolicyType::DENY));
+    policiesToInsert[testBucket2].push_back(Policy(Helpers::generatePolicyKey("4"), PredefinedPolicyType::ALLOW));
+    policiesToInsert[testBucket2].push_back(Policy(Helpers::generatePolicyKey("5"), PredefinedPolicyType::ALLOW));
+
+    for (const auto &bucket : policiesToInsert) {
+        const PolicyBucketId &bucketId = bucket.first;
+        for (const auto &policy : bucket.second) {
+            EXPECT_CALL(backend, searchBucket(bucketId, policy.key()))
+                .WillOnce(Return(PolicyBucket()));
+            EXPECT_CALL(backend, insertPolicy(bucketId, _));
+        }
     }
 
     storage.insertPolicies(policiesToInsert);
@@ -109,31 +111,29 @@ TEST(storage, insertPolicies) {
 
 TEST(storage, updatePolicies) {
     using ::testing::Return;
+    using ::testing::_;
     FakeStorageBackend backend;
     Cynara::Storage storage(backend);
 
     PolicyBucketId testBucket1 = "test-bucket-1";
     PolicyBucketId testBucket2 = "test-bucket-2";
 
-    std::vector<Storage::PolicyPolicyBucket> policiesToInsert = {
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("1"), PredefinedPolicyType::ALLOW), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("2"), PredefinedPolicyType::DENY), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("3"), PredefinedPolicyType::DENY), testBucket1),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("4"), PredefinedPolicyType::ALLOW), testBucket2),
-        std::make_tuple(Policy::simpleWithKey(Helpers::generatePolicyKey("5"), PredefinedPolicyType::ALLOW), testBucket2),
-    };
-
-
-    PolicyCollection pc({std::get<0>(policiesToInsert.at(0))});
-
-    for (const auto &policy : policiesToInsert) {
-        PolicyBucketId bucketId;
-        PolicyPtr policyPtr;
-        std::tie(policyPtr, bucketId) = policy;
-        EXPECT_CALL(backend, searchBucket(bucketId, policyPtr->key()))
-            .WillOnce(Return(PolicyBucket(PolicyCollection({std::get<0>(policy)}))));
-        EXPECT_CALL(backend, deletePolicy(bucketId, policyPtr->key()));
-        EXPECT_CALL(backend, insertPolicy(bucketId, policyPtr));
+    std::map<PolicyBucketId, std::vector<Policy>> policiesToInsert;
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("1"), PredefinedPolicyType::ALLOW));
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("2"), PredefinedPolicyType::DENY));
+    policiesToInsert[testBucket1].push_back(Policy(Helpers::generatePolicyKey("3"), PredefinedPolicyType::DENY));
+    policiesToInsert[testBucket2].push_back(Policy(Helpers::generatePolicyKey("4"), PredefinedPolicyType::ALLOW));
+    policiesToInsert[testBucket2].push_back(Policy(Helpers::generatePolicyKey("5"), PredefinedPolicyType::ALLOW));
+
+    storage.insertPolicies(policiesToInsert);
+
+    for (const auto &bucket : policiesToInsert) {
+        const PolicyBucketId &bucketId = bucket.first;
+        for (const auto &policy : bucket.second) {
+            EXPECT_CALL(backend, searchBucket(bucketId, policy.key()));
+            EXPECT_CALL(backend, deletePolicy(bucketId, policy.key()));
+            EXPECT_CALL(backend, insertPolicy(bucketId, _));
+        }
     }
 
     storage.insertPolicies(policiesToInsert);