Browse Source

Revision diffing issue with revision rollback.

* Fix for diffing issue after rollback in
conjunction with created and deleted buckets.
* Changed rollback function to check against the full set of documents
for a revision instead of just the documents at that particular revision
* Created a document_delete function to encapsulate document deletion
* Added additional test case to check that a rollback to
something other than 0 deletes the created buckets in between

Co-Authored-By: Michael Beaver <michaelbeaver64@gmail.com>
Change-Id: I0d57e67d68def1f15255a8c89290e8c70deedc03
changes/21/614421/18
Smruti Soumitra Khuntia 8 months ago
parent
commit
8fc98631b9

+ 135
- 33
deckhand/db/sqlalchemy/api.py View File

@@ -188,28 +188,11 @@ def documents_create(bucket_name, documents, session=None):
188 188
             deleted_documents = []
189 189
 
190 190
             for d in documents_to_delete:
191
-                doc = models.Document()
192
-                # Store bare minimum information about the document.
193
-                doc['schema'] = d['schema']
194
-                doc['name'] = d['name']
195
-                doc['layer'] = d['layer']
196
-                doc['data'] = {}
197
-                doc['meta'] = d['metadata']
198
-                doc['data_hash'] = _make_hash({})
199
-                doc['metadata_hash'] = _make_hash({})
200
-                doc['bucket_id'] = bucket['id']
201
-                doc['revision_id'] = revision['id']
191
+                doc = document_delete(d, revision['id'], bucket,
192
+                                      session=session)
202 193
 
203
-                # Save and mark the document as `deleted` in the database.
204
-                try:
205
-                    doc.save(session=session)
206
-                except db_exception.DBDuplicateEntry:
207
-                    raise errors.DuplicateDocumentExists(
208
-                        schema=doc['schema'], layer=doc['layer'],
209
-                        name=doc['name'], bucket=bucket['name'])
210
-                doc.safe_delete(session=session)
211 194
                 deleted_documents.append(doc)
212
-                resp.append(doc.to_dict())
195
+                resp.append(doc)
213 196
 
214 197
         if documents_to_create:
215 198
             LOG.debug(
@@ -240,6 +223,81 @@ def documents_create(bucket_name, documents, session=None):
240 223
     return resp
241 224
 
242 225
 
226
+def document_delete(document, revision_id, bucket, session=None):
227
+    """Delete a document
228
+
229
+    Creates a new document with the bare minimum information about the document
230
+    that is to be deleted, and then sets the appropriate deleted fields
231
+
232
+    :param document: document object/dict to be deleted
233
+    :param revision_id: id of the revision where the document is to be deleted
234
+    :param bucket: bucket object/dict where the document will be deleted from
235
+    :param session: Database session object.
236
+    :return: dict representation of deleted document
237
+    """
238
+    session = session or get_session()
239
+
240
+    doc = models.Document()
241
+    # Store bare minimum information about the document.
242
+    doc['schema'] = document['schema']
243
+    doc['name'] = document['name']
244
+    doc['layer'] = document['layer']
245
+    doc['data'] = {}
246
+    doc['meta'] = document['metadata']
247
+    doc['data_hash'] = _make_hash({})
248
+    doc['metadata_hash'] = _make_hash({})
249
+    doc['bucket_id'] = bucket['id']
250
+    doc['revision_id'] = revision_id
251
+
252
+    # Save and mark the document as `deleted` in the database.
253
+    try:
254
+        doc.save(session=session)
255
+    except db_exception.DBDuplicateEntry:
256
+        raise errors.DuplicateDocumentExists(
257
+            schema=doc['schema'], layer=doc['layer'],
258
+            name=doc['name'], bucket=bucket['name'])
259
+    doc.safe_delete(session=session)
260
+
261
+    return doc.to_dict()
262
+
263
+
264
+def documents_delete_from_buckets_list(bucket_names, session=None):
265
+    """Delete all documents in the provided list of buckets
266
+
267
+    :param bucket_names: list of bucket names for which the associated
268
+        buckets and their documents need to be deleted.
269
+    :param session: Database session object.
270
+    :returns: A new model.Revisions object after all the documents have been
271
+        deleted.
272
+    """
273
+    session = session or get_session()
274
+
275
+    with session.begin():
276
+        # Create a new revision
277
+        revision = models.Revision()
278
+        revision.save(session=session)
279
+
280
+        for bucket_name in bucket_names:
281
+
282
+            documents_to_delete = [
283
+                d for d in revision_documents_get(bucket_name=bucket_name,
284
+                                                  session=session)
285
+                if "deleted" not in d or not d['deleted']
286
+            ]
287
+
288
+            bucket = bucket_get_or_create(bucket_name, session=session)
289
+
290
+            if documents_to_delete:
291
+                LOG.debug('Deleting documents: %s.',
292
+                          [eng_utils.meta(d) for d in documents_to_delete])
293
+
294
+                for document in documents_to_delete:
295
+                    document_delete(document, revision['id'], bucket,
296
+                                    session=session)
297
+
298
+    return revision
299
+
300
+
243 301
 def _documents_create(bucket_name, documents, session=None):
244 302
     documents = copy.deepcopy(documents)
245 303
     session = session or get_session()
@@ -456,6 +514,24 @@ def bucket_get_or_create(bucket_name, session=None):
456 514
 
457 515
 ####################
458 516
 
517
+def bucket_get_all(session=None, **filters):
518
+    """Return list of all buckets.
519
+
520
+    :param session: Database session object.
521
+    :returns: List of dictionary representations of retrieved buckets.
522
+    """
523
+    session = session or get_session()
524
+
525
+    buckets = session.query(models.Bucket)\
526
+        .all()
527
+    result = []
528
+    for bucket in buckets:
529
+        revision_dict = bucket.to_dict()
530
+        if utils.deepfilter(revision_dict, **filters):
531
+            result.append(bucket)
532
+
533
+    return result
534
+
459 535
 
460 536
 def revision_create(session=None):
461 537
     """Create a revision.
@@ -777,30 +853,61 @@ def revision_rollback(revision_id, latest_revision, session=None):
777 853
     :returns: The newly created revision.
778 854
     """
779 855
     session = session or get_session()
856
+    latest_revision_docs = revision_documents_get(latest_revision['id'],
857
+                                                  session=session)
780 858
     latest_revision_hashes = [
781
-        (d['data_hash'], d['metadata_hash'])
782
-        for d in latest_revision['documents']]
859
+        (d['data_hash'], d['metadata_hash']) for d in latest_revision_docs
860
+    ]
783 861
 
784 862
     if latest_revision['id'] == revision_id:
785 863
         LOG.debug('The revision being rolled back to is the current revision.'
786 864
                   'Expect no meaningful changes.')
787 865
 
788 866
     if revision_id == 0:
789
-        # Placeholder revision as revision_id=0 doesn't exist.
790
-        orig_revision = {'documents': []}
867
+        # Delete all existing documents in all buckets
868
+        all_buckets = bucket_get_all(deleted=False)
869
+        bucket_names = [str(b['name']) for b in all_buckets]
870
+        revision = documents_delete_from_buckets_list(bucket_names,
871
+                                                      session=session)
872
+
873
+        return revision.to_dict()
791 874
     else:
792
-        orig_revision = revision_get(revision_id, session=session)
875
+        # Sorting the documents so the documents in the new revision are in
876
+        # the same order as the previous revision to support stable testing
877
+        orig_revision_docs = sorted(revision_documents_get(revision_id,
878
+                                                           session=session),
879
+                                    key=lambda d: d['id'])
793 880
 
794 881
     # A mechanism for determining whether a particular document has changed
795 882
     # between revisions. Keyed with the document_id, the value is True if
796 883
     # it has changed, else False.
797 884
     doc_diff = {}
798
-    for orig_doc in orig_revision['documents']:
885
+    # List of unique buckets that exist in this revision
886
+    unique_buckets = []
887
+    for orig_doc in orig_revision_docs:
799 888
         if ((orig_doc['data_hash'], orig_doc['metadata_hash'])
800 889
                 not in latest_revision_hashes):
801 890
             doc_diff[orig_doc['id']] = True
802 891
         else:
803 892
             doc_diff[orig_doc['id']] = False
893
+        if orig_doc['bucket_id'] not in unique_buckets:
894
+            unique_buckets.append(orig_doc['bucket_id'])
895
+
896
+    # We need to find which buckets did not exist at this revision
897
+    buckets_to_delete = []
898
+    all_buckets = bucket_get_all(deleted=False)
899
+    for bucket in all_buckets:
900
+        if bucket['id'] not in unique_buckets:
901
+            buckets_to_delete.append(str(bucket['name']))
902
+
903
+    # Create the new revision,
904
+    if len(buckets_to_delete) > 0:
905
+        new_revision = documents_delete_from_buckets_list(buckets_to_delete,
906
+                                                          session=session)
907
+    else:
908
+        new_revision = models.Revision()
909
+        with session.begin():
910
+            new_revision.save(session=session)
804 911
 
805 912
     # No changes have been made between the target revision to rollback to
806 913
     # and the latest revision.
@@ -809,13 +916,8 @@ def revision_rollback(revision_id, latest_revision, session=None):
809 916
                   'as that of the current revision. Expect no meaningful '
810 917
                   'changes.')
811 918
 
812
-    # Create the new revision,
813
-    new_revision = models.Revision()
814
-    with session.begin():
815
-        new_revision.save(session=session)
816
-
817 919
     # Create the documents for the revision.
818
-    for orig_document in orig_revision['documents']:
920
+    for orig_document in orig_revision_docs:
819 921
         orig_document['revision_id'] = new_revision['id']
820 922
         orig_document['meta'] = orig_document.pop('metadata')
821 923
 
@@ -831,7 +933,7 @@ def revision_rollback(revision_id, latest_revision, session=None):
831 933
         if doc_diff[orig_document['id']]:
832 934
             new_document['orig_revision_id'] = new_revision['id']
833 935
         else:
834
-            new_document['orig_revision_id'] = orig_revision['id']
936
+            new_document['orig_revision_id'] = revision_id
835 937
 
836 938
         with session.begin():
837 939
             new_document.save(session=session)

+ 99
- 0
deckhand/tests/functional/gabbits/revision-diff/revision-diff-rollback-null-success.yaml View File

@@ -0,0 +1,99 @@
1
+# Test success path for revision diff with rollback to null revision.
2
+#
3
+#  1. Purges existing data to ensure test isolation
4
+#  2. Creates an initial document bucket
5
+#  3. Rollback to null (i.e revision 0)
6
+#  4. Verify diff between null (revision 0) and rollback revision to null
7
+#  5. Verify diff between rollback revision to null and null (revision 0)
8
+#  6. Create another document after rollback
9
+#  7. Verify diff between rollback revision and present revision
10
+#  8. Verify diff between present revision and rollback revision
11
+
12
+defaults:
13
+  request_headers:
14
+    content-type: application/x-yaml
15
+  response_headers:
16
+    content-type: application/x-yaml
17
+  verbose: true
18
+
19
+tests:
20
+  - name: purge
21
+    desc: Begin testing from known state.
22
+    DELETE: /api/v1.0/revisions
23
+    status: 204
24
+    response_headers: null
25
+
26
+  - name: create_a
27
+    desc: Create documents in bucket a
28
+    PUT: /api/v1.0/buckets/bucket_a/documents
29
+    status: 200
30
+    data: |-
31
+      ---
32
+      schema: example/Kind/v1
33
+      metadata:
34
+        schema: metadata/Document/v1
35
+        name: doc-a
36
+        storagePolicy: cleartext
37
+        layeringDefinition:
38
+          abstract: false
39
+          layer: site
40
+      data:
41
+        value: 1
42
+      ...
43
+
44
+  - name: rollback_to_null
45
+    desc: Rollback to revision 0
46
+    POST: /api/v1.0/rollback/0
47
+    status: 201
48
+
49
+  - name: verify_null_with_rollback_to_null
50
+    desc: Validates response for null diff rollback to null revision
51
+    GET: /api/v1.0/revisions/0/diff/$HISTORY['rollback_to_null'].$RESPONSE['$.[0].id']
52
+    status: 200
53
+    response_multidoc_jsonpaths:
54
+      $.`len`: 1
55
+      $.[0]: {}
56
+
57
+  - name: verify_rollback_to_null_with_null
58
+    desc: Validates response for rollback to null revision with null revision
59
+    GET: /api/v1.0/revisions/$HISTORY['rollback_to_null'].$RESPONSE['$.[0].id']/diff/0
60
+    status: 200
61
+    response_multidoc_jsonpaths:
62
+      $.`len`: 1
63
+      $.[0]: {}
64
+
65
+  - name: create_b
66
+    desc: Create documents in bucket b
67
+    PUT: /api/v1.0/buckets/bucket_b/documents
68
+    status: 200
69
+    data: |-
70
+      ---
71
+      schema: example/Kind/v1
72
+      metadata:
73
+        schema: metadata/Document/v1
74
+        name: doc-b
75
+        storagePolicy: cleartext
76
+        layeringDefinition:
77
+          abstract: false
78
+          layer: site
79
+      data:
80
+        value: 2
81
+      ...
82
+
83
+  - name: verify_rollback_with_present
84
+    desc: Validates response for diff with rollack to null and create bucket b
85
+    GET: /api/v1.0/revisions/$HISTORY['rollback_to_null'].$RESPONSE['$.[0].id']/diff/$HISTORY['create_b'].$RESPONSE['$.[0].status.revision']
86
+    status: 200
87
+    response_multidoc_jsonpaths:
88
+      $.`len`: 1
89
+      $.[0]:
90
+        bucket_b: created
91
+
92
+  - name: verify_present_with_rollback
93
+    desc: Validates response for diff with rollack to null and create bucket b
94
+    GET: /api/v1.0/revisions/$HISTORY['create_b'].$RESPONSE['$.[0].status.revision']/diff/$HISTORY['rollback_to_null'].$RESPONSE['$.[0].id']
95
+    status: 200
96
+    response_multidoc_jsonpaths:
97
+      $.`len`: 1
98
+      $.[0]:
99
+        bucket_b: created

+ 82
- 1
deckhand/tests/unit/db/test_revision_rollback.py View File

@@ -109,7 +109,7 @@ class TestRevisionRollback(base.DeckhandWithDBTestCase):
109 109
 
110 110
         rollback_revision = self.rollback_revision(0)
111 111
         rollback_documents = self.list_revision_documents(
112
-            rollback_revision['id'], include_history=False)
112
+            rollback_revision['id'], include_history=False, deleted=False)
113 113
         self.assertEqual(orig_revision_id + 1, rollback_revision['id'])
114 114
         self.assertEmpty(rollback_documents)
115 115
 
@@ -123,6 +123,87 @@ class TestRevisionRollback(base.DeckhandWithDBTestCase):
123 123
         self.assertEqual(1, rollback_revision['id'])
124 124
         self.assertEmpty(rollback_documents)
125 125
 
126
+    def test_rollback_to_revision_n_removes_buckets(self):
127
+        """Rolling back to revision 1 should create a revision without the
128
+        buckets in between.
129
+        """
130
+        payload_a = base.DocumentFixture.get_minimal_multi_fixture(count=2)
131
+        bucket_name_a = test_utils.rand_name('bucket')
132
+        created_documents_a = self.create_documents(bucket_name_a, payload_a)
133
+
134
+        payload_b = base.DocumentFixture.get_minimal_multi_fixture(count=3)
135
+        bucket_name_b = test_utils.rand_name('bucket')
136
+        self.create_documents(bucket_name_b, payload_b)
137
+
138
+        payload_c = base.DocumentFixture.get_minimal_multi_fixture(count=3)
139
+        bucket_name_c = test_utils.rand_name('bucket')
140
+        created_documents_c = self.create_documents(bucket_name_c, payload_c)
141
+        orig_revision_id_c = created_documents_c[0]['revision_id']
142
+
143
+        rollback_revision = self.rollback_revision(1)
144
+        rollback_documents = self.list_revision_documents(
145
+            rollback_revision['id'], include_history=False, deleted=False)
146
+        self.assertEqual(orig_revision_id_c + 1, rollback_revision['id'])
147
+        sorted_roll = sorted(rollback_documents, key=lambda k: k['id'])
148
+        sorted_a = sorted(created_documents_a, key=lambda k: k['id'])
149
+        self.assertEqual(len(created_documents_a), len(rollback_documents))
150
+        ignored_fields = ['created_at',
151
+                          'updated_at',
152
+                          'orig_revision_id',
153
+                          'revision_id',
154
+                          'id']
155
+        self.assertDictItemsAlmostEqual(sorted_a, sorted_roll, ignored_fields)
156
+
157
+    def test_rollback_with_deleting_buckets(self):
158
+        """Even if deleting entire buckets before a rollback, rolling back to
159
+        a revision should have all the same documents
160
+        """
161
+        # Revision 1: create bucket a
162
+        payload_a = base.DocumentFixture.get_minimal_multi_fixture(count=2)
163
+        bucket_name_a = test_utils.rand_name('bucket')
164
+        self.create_documents(bucket_name_a, payload_a)
165
+
166
+        # Revision 2: create bucket b
167
+        payload_b = base.DocumentFixture.get_minimal_multi_fixture(count=3)
168
+        bucket_name_b = test_utils.rand_name('bucket')
169
+        created_documents_b = self.create_documents(bucket_name_b, payload_b)
170
+        orig_revision_id_b = created_documents_b[0]['revision_id']
171
+        revision_2_docs = self.list_revision_documents(orig_revision_id_b)
172
+
173
+        # Revision 3: explicitly delete bucket b
174
+        self.create_documents(bucket_name_b, [])
175
+
176
+        # Revision 4: rollback to 2, bucket a and b should exist
177
+        rollback_revision = self.rollback_revision(orig_revision_id_b)
178
+        rollback_docs = self.list_revision_documents(
179
+            rollback_revision['id'], include_history=False, deleted=False)
180
+
181
+        self.assertEqual(4, rollback_revision['id'])
182
+        self.assertEqual(len(revision_2_docs), len(rollback_docs))
183
+        sorted_roll = sorted(rollback_docs, key=lambda k: k['id'])
184
+        sorted_b = sorted(revision_2_docs, key=lambda k: k['id'])
185
+        ignored_fields = ['created_at',
186
+                          'updated_at',
187
+                          'orig_revision_id',
188
+                          'revision_id',
189
+                          'id']
190
+        self.assertDictItemsAlmostEqual(sorted_b, sorted_roll, ignored_fields)
191
+
192
+        # Revision 5: rollback to 0, should delete everything
193
+        self.rollback_revision(0)
194
+
195
+        # Revision 6: rollback to 2, bucket a and b should exist
196
+        rollback_revision = self.rollback_revision(orig_revision_id_b)
197
+        rollback_docs = self.list_revision_documents(
198
+            rollback_revision['id'], include_history=False, deleted=False)
199
+        revision_2_docs = self.list_revision_documents(orig_revision_id_b)
200
+
201
+        self.assertEqual(6, rollback_revision['id'])
202
+        self.assertEqual(len(revision_2_docs), len(rollback_docs))
203
+        sorted_roll = sorted(rollback_docs, key=lambda k: k['id'])
204
+        sorted_b = sorted(revision_2_docs, key=lambda k: k['id'])
205
+        self.assertDictItemsAlmostEqual(sorted_b, sorted_roll, ignored_fields)
206
+
126 207
 
127 208
 class TestRevisionRollbackNegative(base.DeckhandWithDBTestCase):
128 209
 

Loading…
Cancel
Save