Disallow adding valid and invalid policies at once

Storage::insertPolicies() now cares, if all buckets exist before it
makes any change in database (in memory as well as in storage).

No changes are made if any part of request contains invalid parameters.

Change-Id: Ia8d180c7af88bd945dca22f2a4a41b049fdb4c33

diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp
index 39793e2..aa9bc93 100644 (file)
--- a/src/storage/Storage.cpp
+++ b/src/storage/Storage.cpp
@@ -101,7 +101,13 @@ void Storage::insertPolicies(const std::map<PolicyBucketId, std::vector<Policy>>
     // TODO: Rewrite, when transactions are supported
     // Check if all of buckets exist
     for (const auto &group : policiesByBucketId) {
+        const auto &bucketId = group.first;
         const auto &policies = group.second;
+
+        if (m_backend.hasBucket(bucketId) == false) {
+            throw BucketNotExistsException(bucketId);
+        }
+
         std::for_each(policies.cbegin(), policies.cend(), pointedBucketExists);
     }
 

diff --git a/test/storage/storage/policies.cpp b/test/storage/storage/policies.cpp
index 0166ff5..8d72dde 100644 (file)
--- a/test/storage/storage/policies.cpp
+++ b/test/storage/storage/policies.cpp
@@ -86,8 +86,12 @@ TEST(storage, insertPolicies) {
     FakeStorageBackend backend;
     Storage storage(backend);
 
-    PolicyBucketId testBucket1 = "test-bucket-1";
-    PolicyBucketId testBucket2 = "test-bucket-2";
+    std::vector<PolicyBucketId> testBuckets = {
+        PolicyBucketId("test-bucket-1"),
+        PolicyBucketId("test-bucket-2"),
+    };
+
+    PolicyResult defaultPolicy(PredefinedPolicyType::DENY);
 
     typedef std::pair<PolicyBucketId, std::vector<Policy>> BucketPolicyPair;
 
@@ -96,18 +100,26 @@ TEST(storage, insertPolicies) {
     };
 
     std::map<PolicyBucketId, std::vector<Policy>> policiesToInsert = {
-        BucketPolicyPair(testBucket1, {
+        BucketPolicyPair(testBuckets[0], {
             createPolicy("1", ALLOW),
             createPolicy("2", DENY),
             createPolicy("3", DENY)
         }),
-        BucketPolicyPair(testBucket2, {
-            createPolicy("4", { BUCKET, testBucket1 }),
+        BucketPolicyPair(testBuckets[1], {
+            createPolicy("4", { BUCKET, testBuckets[0] }),
             createPolicy("5", PredefinedPolicyType::ALLOW)
         })
     };
 
-    EXPECT_CALL(backend, hasBucket(testBucket1)).WillOnce(Return(true));
+    for (const auto &bucket: testBuckets) {
+        EXPECT_CALL(backend, hasBucket(bucket)).WillOnce(Return(false));
+        EXPECT_CALL(backend, createBucket(bucket, defaultPolicy)).WillOnce(Return());
+
+        storage.addOrUpdateBucket(bucket, defaultPolicy);
+    }
+
+    EXPECT_CALL(backend, hasBucket(testBuckets[0])).WillRepeatedly(Return(true));
+    EXPECT_CALL(backend, hasBucket(testBuckets[1])).WillOnce(Return(true));
 
     for (const auto &group : policiesToInsert) {
         const auto &bucketId = group.first;
@@ -130,6 +142,8 @@ TEST(storage, insertPointingToNonexistentBucket) {
     PolicyBucketId testBucketId = "test-bucket-1";
     PolicyBucketId nonexistentBucketId = "nonexistent";
 
+    PolicyResult defaultPolicy(PredefinedPolicyType::DENY);
+
     typedef std::pair<PolicyBucketId, std::vector<Policy>> BucketPolicyPair;
 
     auto createPolicy = [] (const std::string &keySuffix, const PolicyResult &result) -> Policy {
@@ -143,6 +157,12 @@ TEST(storage, insertPointingToNonexistentBucket) {
         }),
     };
 
+    EXPECT_CALL(backend, hasBucket(testBucketId)).WillOnce(Return(false));
+    EXPECT_CALL(backend, createBucket(testBucketId, defaultPolicy)).WillOnce(Return());
+
+    storage.addOrUpdateBucket(testBucketId, defaultPolicy);
+
+    EXPECT_CALL(backend, hasBucket(testBucketId)).WillOnce(Return(true));
     EXPECT_CALL(backend, hasBucket(nonexistentBucketId)).WillOnce(Return(false));
 
     ASSERT_THROW(storage.insertPolicies(policiesToInsert), BucketNotExistsException);