From 9ab0514249d9f7531281b6f8753d4ae96ec03424 Mon Sep 17 00:00:00 2001 From: CM Lubinski Date: Thu, 14 Jul 2016 19:23:50 +0000 Subject: [PATCH 1/3] Separate "delete" operation from "put" We'd like to be able to call delete methods of documents, layers, etc. without replacing them. As a first step, divide those two operations out --- regcore/db/django_models.py | 25 ++++++++++----- regcore/db/es.py | 17 ++++++++--- regcore/db/interface.py | 34 ++++++++++++++++----- regcore/tests/db_django_models_tests.py | 14 ++++++--- regcore/tests/db_es_tests.py | 4 +-- regcore_write/tests/views_preamble_tests.py | 5 ++- regcore_write/views/diff.py | 1 + regcore_write/views/document.py | 3 +- regcore_write/views/layer.py | 3 +- regcore_write/views/notice.py | 1 + 10 files changed, 78 insertions(+), 29 deletions(-) diff --git a/regcore/db/django_models.py b/regcore/db/django_models.py index ccb3cb2..7fce92c 100644 --- a/regcore/db/django_models.py +++ b/regcore/db/django_models.py @@ -93,14 +93,17 @@ def _transform(self, reg, doc_type, version=None): root=(len(reg['label']) == 1), ) - def bulk_put(self, regs, doc_type, root_label, version): - """Store all reg objects""" + def bulk_delete(self, doc_type, root_label, version): + """Delete all documents that match these params""" # This does not handle subparts. Ignoring that for now Document.objects.filter( version=version, doc_type=doc_type, label_string__startswith=root_label, ).delete() + + def bulk_put(self, regs, doc_type, version): + """Store all document objects""" treeify(regs[0], Document.objects._get_next_tree_id()) Document.objects.bulk_create( [self._transform(r, doc_type, version) for r in regs], @@ -130,12 +133,15 @@ def _transform(self, layer, layer_name, doc_type): return Layer(name=layer_name, layer=layer, doc_type=doc_type, doc_id=doc_id) - def bulk_put(self, layers, layer_name, doc_type, root_doc_id): - """Store all layer objects""" + def bulk_delete(self, layer_name, doc_type, root_doc_id): + """Delete all layer data matching the parameters""" # This does not handle subparts; Ignoring that for now # @todo - use regex to avoid deleting 222-11 when replacing 22 Layer.objects.filter(name=layer_name, doc_type=doc_type, doc_id__startswith=root_doc_id).delete() + + def bulk_put(self, layers, layer_name, doc_type): + """Store all layer objects""" Layer.objects.bulk_create( [self._transform(l, layer_name, doc_type) for l in layers], batch_size=settings.BATCH_SIZE) @@ -152,10 +158,11 @@ def get(self, name, doc_type, doc_id): class DMNotices(interface.Notices): """Implementation of Django-models as notice backend""" - def put(self, doc_number, notice): - """Store a single notice""" + def delete(self, doc_number): Notice.objects.filter(document_number=doc_number).delete() + def put(self, doc_number, notice): + """Store a single notice""" model = Notice(document_number=doc_number, fr_url=notice['fr_url'], publication_date=notice['publication_date'], @@ -194,11 +201,13 @@ class DMDiffs(interface.Diffs): """Implementation of Django-models as diff backend""" def put(self, label, old_version, new_version, diff): """Store a diff between two versions of a regulation node""" - Diff.objects.filter(label=label, old_version=old_version, - new_version=new_version).delete() Diff(label=label, old_version=old_version, new_version=new_version, diff=diff).save() + def delete(self, label, old_version, new_version): + Diff.objects.filter(label=label, old_version=old_version, + new_version=new_version).delete() + def get(self, label, old_version, new_version): """Find the associated diff""" try: diff --git a/regcore/db/es.py b/regcore/db/es.py index 6f00755..9e2cdf4 100644 --- a/regcore/db/es.py +++ b/regcore/db/es.py @@ -1,5 +1,6 @@ """Each of the data structures relevant to the API (regulations, notices, etc.), implemented using Elastic Search as a data store""" +import logging from django.conf import settings from pyelasticsearch import ElasticSearch @@ -8,6 +9,9 @@ from regcore.db import interface +logger = logging.getLogger(__name__) + + def sanitize_doc_id(doc_id): """Not strictly required, but remove slashes from Elastic Search ids""" return ':'.join(doc_id.split('/')) @@ -27,6 +31,12 @@ def safe_fetch(self, doc_type, id): except ElasticHttpNotFoundError: return None + def bulk_delete(self, *args, **kwarg): + logger.warning("Elastic Search backend doesn't handle deletes") + + def delete(self, *args, **kwarg): + logger.warning("Elastic Search backend doesn't handle deletes") + class ESDocuments(ESBase, interface.Documents): """Implementation of Elastic Search as regulations backend""" @@ -55,7 +65,7 @@ def _transform(self, reg, doc_type, version): ) return node - def bulk_put(self, regs, doc_type, root_label, version): + def bulk_put(self, regs, doc_type, version): """Store all reg objects""" self.es.bulk_index( settings.ELASTIC_SEARCH_INDEX, 'reg_tree', @@ -84,9 +94,8 @@ def _transform(self, layer, layer_name, doc_type): doc_id = sanitize_doc_id(layer.pop('doc_id')) return {'id': ':'.join([layer_name, doc_type, doc_id]), 'layer': layer} - def bulk_put(self, layers, layer_name, doc_type, root_doc_id): - """Store all layer objects. Note this does not delete existing docs; - it only replaces/inserts docs, which has loop holes""" + def bulk_put(self, layers, layer_name, doc_type): + """Store all layer objects.""" self.es.bulk_index( settings.ELASTIC_SEARCH_INDEX, 'layer', [self._transform(l, layer_name, doc_type) for l in layers]) diff --git a/regcore/db/interface.py b/regcore/db/interface.py index da86042..16f5a7d 100644 --- a/regcore/db/interface.py +++ b/regcore/db/interface.py @@ -12,9 +12,13 @@ def get(self, doc_type, label, version): raise NotImplementedError @abc.abstractmethod - def bulk_put(self, regs, doc_type, root_label, version): - """Add many entries, with a root of root_label. Each should have the - provided version""" + def bulk_delete(self, doc_type, root_label, version): + """Delete all documents that match these parameters""" + raise NotImplementedError + + @abc.abstractmethod + def bulk_put(self, regs, doc_type, version): + """Add many entries, each with the provided version""" raise NotImplementedError @abc.abstractmethod @@ -26,15 +30,21 @@ def listing(self, doc_type, label=None): @six.add_metaclass(abc.ABCMeta) class Layers(object): - def bulk_put(self, layers, layer_name, doc_type, root_doc_id): + def bulk_delete(self, layer_name, doc_type, root_doc_id): + """Deletes multiple entries with the same layer_name. + :param str layer_name: Identifier for this layer, e.g. "toc", + "internal-citations", etc. + :param str doc_type: layers are keyed by doc_type + :param str root_doc_id: the doc id of the "root" layer.""" + raise NotImplementedError + + def bulk_put(self, layers, layer_name, doc_type): """Add multiple entries with the same layer_name. :param list[dict] layers: Each dictionary represents a layer; each should have a distinct "doc_id", which will be used during insertion. :param str layer_name: Identifier for this layer, e.g. "toc", "internal-citations", etc. - :param str doc_type: layers are keyed by doc_type - :param str root_doc_id: the doc id of the "root" layer. This is used - to delete existing data before inserting""" + :param str doc_type: layers are keyed by doc_type""" raise NotImplementedError def get(self, name, doc_type, doc_id): @@ -44,6 +54,10 @@ def get(self, name, doc_type, doc_id): @six.add_metaclass(abc.ABCMeta) class Notices(object): + def delete(self, doc_number): + """:param str doc_number:""" + raise NotImplementedError + def put(self, doc_number, notice): """:param str doc_number: :param dict notice:""" @@ -60,6 +74,12 @@ def listing(self, part=None): @six.add_metaclass(abc.ABCMeta) class Diffs(object): + def delete(self, label, old_version, new_version): + """:param str label: + :param str old_version: + :param str new_version:""" + raise NotImplementedError + def put(self, label, old_version, new_version, diff): """:param str label: :param str old_version: diff --git a/regcore/tests/db_django_models_tests.py b/regcore/tests/db_django_models_tests.py index 2f26ada..1889a49 100644 --- a/regcore/tests/db_django_models_tests.py +++ b/regcore/tests/db_django_models_tests.py @@ -90,11 +90,12 @@ def test_bulk_put(self): n2['parent'] = root n3['parent'] = root nodes = [root, n2, n3] - self.dmr.bulk_put(nodes, 'cfr', '111', 'verver') + self.dmr.bulk_put(nodes, 'cfr', 'verver') self.assertEqual(DMDocuments().get('cfr', '111', 'verver'), original) root['title'] = original['title'] = 'New Title' - self.dmr.bulk_put(nodes, 'cfr', '111', 'verver') + self.dmr.bulk_delete('cfr', '111', 'verver') + self.dmr.bulk_put(nodes, 'cfr', 'verver') self.assertEqual(DMDocuments().get('cfr', '111', 'verver'), original) @@ -118,7 +119,7 @@ def test_bulk_put(self): modified""" layers = [{'111-22': [], '111-22-a': [], 'doc_id': 'verver/111-22'}, {'111-23': [], 'doc_id': 'verver/111-23'}] - self.dml.bulk_put(layers, 'name', 'cfr', 'verver/111') + self.dml.bulk_put(layers, 'name', 'cfr') self.assertEqual(Layer.objects.count(), 2) self.assertEqual(self.dml.get('name', 'cfr', 'verver/111-22'), @@ -127,7 +128,8 @@ def test_bulk_put(self): {'111-23': []}) layers[1] = {'111-23': [1], 'doc_id': 'verver/111-23'} - self.dml.bulk_put(layers, 'name', 'cfr', 'verver/111') + self.dml.bulk_delete('name', 'cfr', 'verver/111') + self.dml.bulk_put(layers, 'name', 'cfr') self.assertEqual(Layer.objects.count(), 2) self.assertEqual(self.dml.get('name', 'cfr', 'verver/111-23'), @@ -189,6 +191,7 @@ def test_put(self): [expected]) doc['fr_url'] = 'url2' + self.dmn.delete('docdoc') self.dmn.put('docdoc', doc) expected['fr_url'] = 'url2' @@ -210,7 +213,7 @@ def test_get_success(self): self.assertEqual({"some": 'body'}, self.dmd.get('lablab', 'oldold', 'newnew')) - def test_put(self): + def test_put_delete(self): """We can insert and replace a diff""" self.dmd.put('lablab', 'oldold', 'newnew', {"some": "structure"}) @@ -220,6 +223,7 @@ def test_put(self): six.assertCountEqual(self, Diff.objects.all().values(*fields), [expected]) + self.dmd.delete('lablab', 'oldold', 'newnew') self.dmd.put('lablab', 'oldold', 'newnew', {"other": "structure"}) expected['diff'] = {'other': 'structure'} six.assertCountEqual(self, Diff.objects.all().values(*fields), diff --git a/regcore/tests/db_es_tests.py b/regcore/tests/db_es_tests.py index 4fb3b22..f0fd4f1 100644 --- a/regcore/tests/db_es_tests.py +++ b/regcore/tests/db_es_tests.py @@ -74,7 +74,7 @@ def test_bulk_put(self): nodes = [root, n2, n3] with self.expect_bulk_put('reg_tree', 3) as bulk_put: - ESDocuments().bulk_put(nodes, 'cfr', '111', 'verver') + ESDocuments().bulk_put(nodes, 'cfr', 'verver') root.update({'version': 'verver', 'regulation': '111', 'label_string': '111', 'id': 'verver/111', 'root': True, @@ -123,7 +123,7 @@ def test_bulk_put(self): layers = [{'111-22': [], '111-22-a': [], 'doc_id': 'verver:111-22'}, {'111-23': [], 'doc_id': 'verver:111-23'}] with self.expect_bulk_put('layer', 2) as bulk_put: - ESLayers().bulk_put(layers, 'name', 'cfr', 'verver:111') + ESLayers().bulk_put(layers, 'name', 'cfr') del layers[0]['doc_id'] del layers[1]['doc_id'] diff --git a/regcore_write/tests/views_preamble_tests.py b/regcore_write/tests/views_preamble_tests.py index 99561d4..9065460 100644 --- a/regcore_write/tests/views_preamble_tests.py +++ b/regcore_write/tests/views_preamble_tests.py @@ -27,6 +27,9 @@ def test_stores(self, storage): ) bulk_data = dict(data) bulk_data['parent'] = None + storage.for_documents.bulk_delete.assert_called_with( + 'preamble', 'label', None, + ) storage.for_documents.bulk_put.assert_called_with( - [bulk_data], 'preamble', 'label', None, + [bulk_data], 'preamble', None, ) diff --git a/regcore_write/views/diff.py b/regcore_write/views/diff.py index 34ce4c9..4b55273 100644 --- a/regcore_write/views/diff.py +++ b/regcore_write/views/diff.py @@ -8,6 +8,7 @@ def add(request, label_id, old_version, new_version): """Add the diff to the db, indexed by the label and versions""" # @todo: write a schema that verifies the diff's structure + storage.for_diffs.delete(label_id, old_version, new_version) storage.for_diffs.put( label_id, old_version, new_version, request.json_body) return success() diff --git a/regcore_write/views/document.py b/regcore_write/views/document.py index ff2539a..eac4e56 100644 --- a/regcore_write/views/document.py +++ b/regcore_write/views/document.py @@ -65,4 +65,5 @@ def add_node(node, parent=None): add_node(child, parent=node) add_node(node) - storage.for_documents.bulk_put(to_save, doc_type, label_id, version) + storage.for_documents.bulk_delete(doc_type, label_id, version) + storage.for_documents.bulk_put(to_save, doc_type, version) diff --git a/regcore_write/views/layer.py b/regcore_write/views/layer.py index 7256faf..c6e33ec 100644 --- a/regcore_write/views/layer.py +++ b/regcore_write/views/layer.py @@ -43,8 +43,9 @@ def add(request, name, doc_type, doc_id): return user_error('label mismatch: {}, {}'.format( params.tree_id, key)) + storage.for_layers.bulk_delete(name, params.doc_type, params.doc_id) storage.for_layers.bulk_put(child_layers(params, layer), name, - params.doc_type, params.doc_id) + params.doc_type) return success() diff --git a/regcore_write/views/notice.py b/regcore_write/views/notice.py index b05ff52..ab10645 100644 --- a/regcore_write/views/notice.py +++ b/regcore_write/views/notice.py @@ -16,5 +16,6 @@ def add(request, docnum): del notice['cfr_part'] notice['cfr_parts'] = cfr_parts + storage.for_notices.delete(docnum) storage.for_notices.put(docnum, notice) return success() From 12ac9a14b3bc8a0dd58453969b39cf1b06bade89 Mon Sep 17 00:00:00 2001 From: CM Lubinski Date: Thu, 14 Jul 2016 19:47:32 +0000 Subject: [PATCH 2/3] s/put/insert As the "delete" half of the "put" functions has been split off, rename the remaining portion "insert" --- regcore/db/django_models.py | 8 ++-- regcore/db/es.py | 8 ++-- regcore/db/interface.py | 8 ++-- regcore/tests/db_django_models_tests.py | 24 ++++++------ regcore/tests/db_es_tests.py | 37 ++++++++++--------- regcore_write/tests/views_diff_tests.py | 2 +- regcore_write/tests/views_layer_tests.py | 6 +-- regcore_write/tests/views_notice_tests.py | 12 +++--- regcore_write/tests/views_preamble_tests.py | 2 +- regcore_write/tests/views_regulation_tests.py | 22 +++++------ regcore_write/views/diff.py | 2 +- regcore_write/views/document.py | 2 +- regcore_write/views/layer.py | 4 +- regcore_write/views/notice.py | 2 +- 14 files changed, 70 insertions(+), 69 deletions(-) diff --git a/regcore/db/django_models.py b/regcore/db/django_models.py index 7fce92c..aa302b1 100644 --- a/regcore/db/django_models.py +++ b/regcore/db/django_models.py @@ -102,7 +102,7 @@ def bulk_delete(self, doc_type, root_label, version): label_string__startswith=root_label, ).delete() - def bulk_put(self, regs, doc_type, version): + def bulk_insert(self, regs, doc_type, version): """Store all document objects""" treeify(regs[0], Document.objects._get_next_tree_id()) Document.objects.bulk_create( @@ -140,7 +140,7 @@ def bulk_delete(self, layer_name, doc_type, root_doc_id): Layer.objects.filter(name=layer_name, doc_type=doc_type, doc_id__startswith=root_doc_id).delete() - def bulk_put(self, layers, layer_name, doc_type): + def bulk_insert(self, layers, layer_name, doc_type): """Store all layer objects""" Layer.objects.bulk_create( [self._transform(l, layer_name, doc_type) for l in layers], @@ -161,7 +161,7 @@ class DMNotices(interface.Notices): def delete(self, doc_number): Notice.objects.filter(document_number=doc_number).delete() - def put(self, doc_number, notice): + def insert(self, doc_number, notice): """Store a single notice""" model = Notice(document_number=doc_number, fr_url=notice['fr_url'], @@ -199,7 +199,7 @@ def listing(self, part=None): class DMDiffs(interface.Diffs): """Implementation of Django-models as diff backend""" - def put(self, label, old_version, new_version, diff): + def insert(self, label, old_version, new_version, diff): """Store a diff between two versions of a regulation node""" Diff(label=label, old_version=old_version, new_version=new_version, diff=diff).save() diff --git a/regcore/db/es.py b/regcore/db/es.py index 9e2cdf4..9e2c33b 100644 --- a/regcore/db/es.py +++ b/regcore/db/es.py @@ -65,7 +65,7 @@ def _transform(self, reg, doc_type, version): ) return node - def bulk_put(self, regs, doc_type, version): + def bulk_insert(self, regs, doc_type, version): """Store all reg objects""" self.es.bulk_index( settings.ELASTIC_SEARCH_INDEX, 'reg_tree', @@ -94,7 +94,7 @@ def _transform(self, layer, layer_name, doc_type): doc_id = sanitize_doc_id(layer.pop('doc_id')) return {'id': ':'.join([layer_name, doc_type, doc_id]), 'layer': layer} - def bulk_put(self, layers, layer_name, doc_type): + def bulk_insert(self, layers, layer_name, doc_type): """Store all layer objects.""" self.es.bulk_index( settings.ELASTIC_SEARCH_INDEX, 'layer', @@ -110,7 +110,7 @@ def get(self, name, doc_type, doc_id): class ESNotices(ESBase, interface.Notices): """Implementation of Elastic Search as notice backend""" - def put(self, doc_number, notice): + def insert(self, doc_number, notice): """Store a single notice""" self.es.index(settings.ELASTIC_SEARCH_INDEX, 'notice', notice, id=doc_number) @@ -142,7 +142,7 @@ class ESDiffs(ESBase, interface.Diffs): def to_id(label, old, new): return "%s/%s/%s" % (label, old, new) - def put(self, label, old_version, new_version, diff): + def insert(self, label, old_version, new_version, diff): """Store a diff between two versions of a regulation node""" struct = { 'label': label, diff --git a/regcore/db/interface.py b/regcore/db/interface.py index 16f5a7d..818c2de 100644 --- a/regcore/db/interface.py +++ b/regcore/db/interface.py @@ -17,7 +17,7 @@ def bulk_delete(self, doc_type, root_label, version): raise NotImplementedError @abc.abstractmethod - def bulk_put(self, regs, doc_type, version): + def bulk_insert(self, regs, doc_type, version): """Add many entries, each with the provided version""" raise NotImplementedError @@ -38,7 +38,7 @@ def bulk_delete(self, layer_name, doc_type, root_doc_id): :param str root_doc_id: the doc id of the "root" layer.""" raise NotImplementedError - def bulk_put(self, layers, layer_name, doc_type): + def bulk_insert(self, layers, layer_name, doc_type): """Add multiple entries with the same layer_name. :param list[dict] layers: Each dictionary represents a layer; each should have a distinct "doc_id", which will be used during insertion. @@ -58,7 +58,7 @@ def delete(self, doc_number): """:param str doc_number:""" raise NotImplementedError - def put(self, doc_number, notice): + def insert(self, doc_number, notice): """:param str doc_number: :param dict notice:""" raise NotImplementedError @@ -80,7 +80,7 @@ def delete(self, label, old_version, new_version): :param str new_version:""" raise NotImplementedError - def put(self, label, old_version, new_version, diff): + def insert(self, label, old_version, new_version, diff): """:param str label: :param str old_version: :param str new_version: diff --git a/regcore/tests/db_django_models_tests.py b/regcore/tests/db_django_models_tests.py index 1889a49..87e6b73 100644 --- a/regcore/tests/db_django_models_tests.py +++ b/regcore/tests/db_django_models_tests.py @@ -70,7 +70,7 @@ def test_listing(self): results = self.dmr.listing('cfr') self.assertEqual([('ver1', '1111'), ('ver2', '1111')], results) - def test_bulk_put(self): + def test_bulk_insert(self): """Writing multiple documents should save correctly. They can be modified. The lft and rght ids assigned by the Modified Preorder Tree Traversal algorithm are shown below: @@ -90,12 +90,12 @@ def test_bulk_put(self): n2['parent'] = root n3['parent'] = root nodes = [root, n2, n3] - self.dmr.bulk_put(nodes, 'cfr', 'verver') + self.dmr.bulk_insert(nodes, 'cfr', 'verver') self.assertEqual(DMDocuments().get('cfr', '111', 'verver'), original) root['title'] = original['title'] = 'New Title' self.dmr.bulk_delete('cfr', '111', 'verver') - self.dmr.bulk_put(nodes, 'cfr', 'verver') + self.dmr.bulk_insert(nodes, 'cfr', 'verver') self.assertEqual(DMDocuments().get('cfr', '111', 'verver'), original) @@ -114,12 +114,12 @@ def test_get_success(self): self.assertEqual({"some": 'body'}, self.dml.get('namnam', 'cfr', 'verver/lablab')) - def test_bulk_put(self): + def test_bulk_insert(self): """Writing multiple documents should save correctly. They can be modified""" layers = [{'111-22': [], '111-22-a': [], 'doc_id': 'verver/111-22'}, {'111-23': [], 'doc_id': 'verver/111-23'}] - self.dml.bulk_put(layers, 'name', 'cfr') + self.dml.bulk_insert(layers, 'name', 'cfr') self.assertEqual(Layer.objects.count(), 2) self.assertEqual(self.dml.get('name', 'cfr', 'verver/111-22'), @@ -129,7 +129,7 @@ def test_bulk_put(self): layers[1] = {'111-23': [1], 'doc_id': 'verver/111-23'} self.dml.bulk_delete('name', 'cfr', 'verver/111') - self.dml.bulk_put(layers, 'name', 'cfr') + self.dml.bulk_insert(layers, 'name', 'cfr') self.assertEqual(Layer.objects.count(), 2) self.assertEqual(self.dml.get('name', 'cfr', 'verver/111-23'), @@ -171,14 +171,14 @@ def test_listing(self): self.assertEqual(self.dmn.listing(), self.dmn.listing('876')) self.assertEqual([], self.dmn.listing('888')) - def test_put(self): + def test_insert(self): """We can insert and replace a notice""" doc = {"some": "structure", 'effective_on': '2011-01-01', 'fr_url': 'http://example.com', 'publication_date': '2010-02-02', 'cfr_parts': ['222']} - self.dmn.put('docdoc', doc) + self.dmn.insert('docdoc', doc) expected = {"document_number": "docdoc", "effective_on": date(2011, 1, 1), @@ -192,7 +192,7 @@ def test_put(self): doc['fr_url'] = 'url2' self.dmn.delete('docdoc') - self.dmn.put('docdoc', doc) + self.dmn.insert('docdoc', doc) expected['fr_url'] = 'url2' six.assertCountEqual(self, Notice.objects.all().values(*fields), @@ -213,9 +213,9 @@ def test_get_success(self): self.assertEqual({"some": 'body'}, self.dmd.get('lablab', 'oldold', 'newnew')) - def test_put_delete(self): + def test_insert_delete(self): """We can insert and replace a diff""" - self.dmd.put('lablab', 'oldold', 'newnew', {"some": "structure"}) + self.dmd.insert('lablab', 'oldold', 'newnew', {"some": "structure"}) expected = {"label": "lablab", "old_version": "oldold", "new_version": "newnew", "diff": {"some": "structure"}} @@ -224,7 +224,7 @@ def test_put_delete(self): [expected]) self.dmd.delete('lablab', 'oldold', 'newnew') - self.dmd.put('lablab', 'oldold', 'newnew', {"other": "structure"}) + self.dmd.insert('lablab', 'oldold', 'newnew', {"other": "structure"}) expected['diff'] = {'other': 'structure'} six.assertCountEqual(self, Diff.objects.all().values(*fields), [expected]) diff --git a/regcore/tests/db_es_tests.py b/regcore/tests/db_es_tests.py index f0fd4f1..b88d089 100644 --- a/regcore/tests/db_es_tests.py +++ b/regcore/tests/db_es_tests.py @@ -24,7 +24,7 @@ def expect_get(self, doc_type, id, doc=None): es.return_value.get.call_args[0][1:]) @contextmanager - def expect_put(self, doc_type, id): + def expect_insert(self, doc_type, id): """Expect a document to be written.""" with patch('regcore.db.es.ElasticSearch') as es: yield es.return_value.index @@ -32,7 +32,7 @@ def expect_put(self, doc_type, id): self.assertEqual(id, es.return_value.index.call_args[1].get('id')) @contextmanager - def expect_bulk_put(self, doc_type, num_docs): + def expect_bulk_insert(self, doc_type, num_docs): """Expect multiple documents to be written..""" with patch('regcore.db.es.ElasticSearch') as es: yield es.return_value.bulk_index @@ -65,7 +65,7 @@ def test_get_success(self): self.assertEqual(ESDocuments().get('cfr', 'lablab', 'verver'), {"first": 0}) - def test_bulk_put(self): + def test_bulk_insert(self): n2 = {'text': 'some text', 'label': ['111', '2'], 'children': []} n3 = {'text': 'other', 'label': ['111', '3'], 'children': []} # Use a copy of the children @@ -73,8 +73,8 @@ def test_bulk_put(self): dict(n3)]} nodes = [root, n2, n3] - with self.expect_bulk_put('reg_tree', 3) as bulk_put: - ESDocuments().bulk_put(nodes, 'cfr', 'verver') + with self.expect_bulk_insert('reg_tree', 3) as bulk_insert: + ESDocuments().bulk_insert(nodes, 'cfr', 'verver') root.update({'version': 'verver', 'regulation': '111', 'label_string': '111', 'id': 'verver/111', 'root': True, @@ -88,7 +88,7 @@ def test_bulk_put(self): bulk_data = [dict(root), dict(n2), dict(n3)] for node in bulk_data: node['doc_type'] = 'cfr' - self.assertEqual(bulk_data, bulk_put.call_args[0][2]) + self.assertEqual(bulk_data, bulk_insert.call_args[0][2]) def test_listing(self): query = {'match': {'label_string': 'lll', 'doc_type': 'cfr'}} @@ -119,17 +119,17 @@ def test_get_success(self): self.assertEqual(ESLayers().get('namnam', 'cfr', 'verver:lablab'), {"some": "body"}) - def test_bulk_put(self): + def test_bulk_insert(self): layers = [{'111-22': [], '111-22-a': [], 'doc_id': 'verver:111-22'}, {'111-23': [], 'doc_id': 'verver:111-23'}] - with self.expect_bulk_put('layer', 2) as bulk_put: - ESLayers().bulk_put(layers, 'name', 'cfr') + with self.expect_bulk_insert('layer', 2) as bulk_insert: + ESLayers().bulk_insert(layers, 'name', 'cfr') del layers[0]['doc_id'] del layers[1]['doc_id'] transformed = [{'id': 'name:cfr:verver:111-22', 'layer': layers[0]}, {'id': 'name:cfr:verver:111-23', 'layer': layers[1]}] - self.assertEqual(transformed, bulk_put.call_args[0][2]) + self.assertEqual(transformed, bulk_insert.call_args[0][2]) class ESNoticesTest(TestCase, ESBase): @@ -142,10 +142,10 @@ def test_get_success(self): self.assertEqual(ESNotices().get('docdoc'), {"some": 'body'}) - def test_put(self): - with self.expect_put('notice', 'docdoc') as put: - ESNotices().put('docdoc', {"some": "structure"}) - self.assertEqual(put.call_args[0][2], {"some": "structure"}) + def test_insert(self): + with self.expect_insert('notice', 'docdoc') as insert: + ESNotices().insert('docdoc', {"some": "structure"}) + self.assertEqual(insert.call_args[0][2], {"some": "structure"}) def test_listing(self): query = {'match_all': {}} @@ -178,10 +178,11 @@ def test_get_success(self): self.assertEqual(ESDiffs().get('lablab', 'oldold', 'newnew'), {"some": 'body'}) - def test_put(self): - with self.expect_put('diff', 'lablab/oldold/newnew') as put: - ESDiffs().put('lablab', 'oldold', 'newnew', {"some": "structure"}) - self.assertEqual(put.call_args[0][2], + def test_insert(self): + with self.expect_insert('diff', 'lablab/oldold/newnew') as insert: + ESDiffs().insert('lablab', 'oldold', 'newnew', + {"some": "structure"}) + self.assertEqual(insert.call_args[0][2], {'label': 'lablab', 'old_version': 'oldold', 'new_version': 'newnew', diff --git a/regcore_write/tests/views_diff_tests.py b/regcore_write/tests/views_diff_tests.py index 5603d85..71cc5ea 100644 --- a/regcore_write/tests/views_diff_tests.py +++ b/regcore_write/tests/views_diff_tests.py @@ -20,6 +20,6 @@ def test_add_label_success(self, storage): Client().put(url, content_type='application/json', data=json.dumps({'some': 'struct'})) - args = storage.for_diffs.put.call_args[0] + args = storage.for_diffs.insert.call_args[0] self.assertEqual(('lablab', 'oldold', 'newnew', {'some': 'struct'}), args) diff --git a/regcore_write/tests/views_layer_tests.py b/regcore_write/tests/views_layer_tests.py index 79c6545..ffe852a 100644 --- a/regcore_write/tests/views_layer_tests.py +++ b/regcore_write/tests/views_layer_tests.py @@ -41,8 +41,8 @@ def put_with_mock_data(self, message, *labels, **kwargs): with patch('regcore_write.views.layer.storage') as storage: storage.for_documents.get.return_value = root[0] self.put(message, **kwargs) - self.assertTrue(storage.for_layers.bulk_put.called) - layers_saved = storage.for_layers.bulk_put.call_args[0][0] + self.assertTrue(storage.for_layers.bulk_insert.called) + layers_saved = storage.for_layers.bulk_insert.call_args[0][0] return list(reversed(layers_saved)) # switch to outside in def test_add_success(self): @@ -139,7 +139,7 @@ def test_add_preamble_layer(self, storage): '111_22-3-b': 'layer3'} self.client.put('/layer/aname/preamble/111_22', data=json.dumps(message)) - stored = storage.for_layers.bulk_put.call_args[0][0] + stored = storage.for_layers.bulk_insert.call_args[0][0] self.assertEqual(len(stored), 9) for label in ('111_22-1', '111_22-2', '111_22-2-a', '111_22-3-a', '111_22-3-a-i', '111_22-3-b-i'): diff --git a/regcore_write/tests/views_notice_tests.py b/regcore_write/tests/views_notice_tests.py index fe982e0..cad1146 100644 --- a/regcore_write/tests/views_notice_tests.py +++ b/regcore_write/tests/views_notice_tests.py @@ -20,23 +20,23 @@ def test_add_label_success(self, storage): Client().put(url, content_type='application/json', data=json.dumps({'some': 'struct'})) - self.assertTrue(storage.for_notices.put.called) - args = storage.for_notices.put.call_args[0] + self.assertTrue(storage.for_notices.insert.called) + args = storage.for_notices.insert.call_args[0] self.assertEqual('docdoc', args[0]) self.assertEqual({'some': 'struct', 'cfr_parts': []}, args[1]) Client().put(url, content_type='application/json', data=json.dumps({'some': 'struct', 'cfr_part': '1111'})) - self.assertTrue(storage.for_notices.put.called) - args = storage.for_notices.put.call_args[0] + self.assertTrue(storage.for_notices.insert.called) + args = storage.for_notices.insert.call_args[0] self.assertEqual('docdoc', args[0]) self.assertEqual({'some': 'struct', 'cfr_parts': ['1111']}, args[1]) Client().put( url, content_type='application/json', data=json.dumps({'some': 'struct', 'cfr_parts': ['111', '222']})) - self.assertTrue(storage.for_notices.put.called) - args = storage.for_notices.put.call_args[0] + self.assertTrue(storage.for_notices.insert.called) + args = storage.for_notices.insert.call_args[0] self.assertEqual('docdoc', args[0]) self.assertEqual({'some': 'struct', 'cfr_parts': ['111', '222']}, args[1]) diff --git a/regcore_write/tests/views_preamble_tests.py b/regcore_write/tests/views_preamble_tests.py index 9065460..268b73f 100644 --- a/regcore_write/tests/views_preamble_tests.py +++ b/regcore_write/tests/views_preamble_tests.py @@ -30,6 +30,6 @@ def test_stores(self, storage): storage.for_documents.bulk_delete.assert_called_with( 'preamble', 'label', None, ) - storage.for_documents.bulk_put.assert_called_with( + storage.for_documents.bulk_insert.assert_called_with( [bulk_data], 'preamble', None, ) diff --git a/regcore_write/tests/views_regulation_tests.py b/regcore_write/tests/views_regulation_tests.py index 349f6d3..b8df12f 100644 --- a/regcore_write/tests/views_regulation_tests.py +++ b/regcore_write/tests/views_regulation_tests.py @@ -63,11 +63,11 @@ def test_add_label_success(self, storage): Client().put(url, content_type='application/json', data=json.dumps(message)) - self.assertTrue(storage.for_documents.bulk_put.called) - bulk_put_args = storage.for_documents.bulk_put.call_args[0] - self.assertEqual(3, len(bulk_put_args[0])) + self.assertTrue(storage.for_documents.bulk_insert.called) + bulk_insert_args = storage.for_documents.bulk_insert.call_args[0] + self.assertEqual(3, len(bulk_insert_args[0])) found = [False, False, False] - for arg in bulk_put_args[0]: + for arg in bulk_insert_args[0]: if arg['label'] == ['p']: found[0] = True if arg['label'] == ['p', 'c1']: @@ -76,12 +76,12 @@ def test_add_label_success(self, storage): found[2] = True self.assertEqual(found, [True, True, True]) - storage.for_documents.bulk_put.reset_mock() + storage.for_documents.bulk_insert.reset_mock() Client().post(url, content_type='application/json', data=json.dumps(message)) - self.assertTrue(storage.for_documents.bulk_put.called) - bulk_put_args = storage.for_documents.bulk_put.call_args[0] - self.assertEqual(3, len(bulk_put_args[0])) + self.assertTrue(storage.for_documents.bulk_insert.called) + bulk_insert_args = storage.for_documents.bulk_insert.call_args[0] + self.assertEqual(3, len(bulk_insert_args[0])) @patch('regcore_write.views.document.storage') def test_add_empty_children(self, storage): @@ -94,6 +94,6 @@ def test_add_empty_children(self, storage): } Client().put(url, content_type='application/json', data=json.dumps(message)) - self.assertTrue(storage.for_documents.bulk_put.called) - bulk_put_args = storage.for_documents.bulk_put.call_args[0] - self.assertEqual(1, len(bulk_put_args[0])) + self.assertTrue(storage.for_documents.bulk_insert.called) + bulk_insert_args = storage.for_documents.bulk_insert.call_args[0] + self.assertEqual(1, len(bulk_insert_args[0])) diff --git a/regcore_write/views/diff.py b/regcore_write/views/diff.py index 4b55273..f3e935d 100644 --- a/regcore_write/views/diff.py +++ b/regcore_write/views/diff.py @@ -9,6 +9,6 @@ def add(request, label_id, old_version, new_version): """Add the diff to the db, indexed by the label and versions""" # @todo: write a schema that verifies the diff's structure storage.for_diffs.delete(label_id, old_version, new_version) - storage.for_diffs.put( + storage.for_diffs.insert( label_id, old_version, new_version, request.json_body) return success() diff --git a/regcore_write/views/document.py b/regcore_write/views/document.py index eac4e56..2ae42f3 100644 --- a/regcore_write/views/document.py +++ b/regcore_write/views/document.py @@ -66,4 +66,4 @@ def add_node(node, parent=None): add_node(node) storage.for_documents.bulk_delete(doc_type, label_id, version) - storage.for_documents.bulk_put(to_save, doc_type, version) + storage.for_documents.bulk_insert(to_save, doc_type, version) diff --git a/regcore_write/views/layer.py b/regcore_write/views/layer.py index c6e33ec..43cecc8 100644 --- a/regcore_write/views/layer.py +++ b/regcore_write/views/layer.py @@ -44,8 +44,8 @@ def add(request, name, doc_type, doc_id): params.tree_id, key)) storage.for_layers.bulk_delete(name, params.doc_type, params.doc_id) - storage.for_layers.bulk_put(child_layers(params, layer), name, - params.doc_type) + storage.for_layers.bulk_insert(child_layers(params, layer), name, + params.doc_type) return success() diff --git a/regcore_write/views/notice.py b/regcore_write/views/notice.py index ab10645..5bee180 100644 --- a/regcore_write/views/notice.py +++ b/regcore_write/views/notice.py @@ -17,5 +17,5 @@ def add(request, docnum): notice['cfr_parts'] = cfr_parts storage.for_notices.delete(docnum) - storage.for_notices.put(docnum, notice) + storage.for_notices.insert(docnum, notice) return success() From af55411e602b79afc48b98fa0cd590dbe5f726f8 Mon Sep 17 00:00:00 2001 From: CM Lubinski Date: Thu, 14 Jul 2016 20:52:51 +0000 Subject: [PATCH 3/3] Add DELETE endpoints --- regcore/urls.py | 5 +++++ regcore_write/views/diff.py | 7 +++++++ regcore_write/views/document.py | 9 ++++++++- regcore_write/views/layer.py | 11 +++++++++++ regcore_write/views/notice.py | 7 +++++++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/regcore/urls.py b/regcore/urls.py index 1af18ac..8c1dbd0 100644 --- a/regcore/urls.py +++ b/regcore/urls.py @@ -39,6 +39,11 @@ mapping['notice'][verb] = wnotice.add mapping['preamble'][verb] = wdocument.add mapping['regulation'][verb] = wdocument.add + mapping['diff']['DELETE'] = wdiff.delete + mapping['layer']['DELETE'] = wlayer.delete + mapping['notice']['DELETE'] = wnotice.delete + mapping['preamble']['DELETE'] = wdocument.delete + mapping['regulation']['DELETE'] = wdocument.delete # Re-usable URL patterns. diff --git a/regcore_write/views/diff.py b/regcore_write/views/diff.py index f3e935d..40ecb82 100644 --- a/regcore_write/views/diff.py +++ b/regcore_write/views/diff.py @@ -12,3 +12,10 @@ def add(request, label_id, old_version, new_version): storage.for_diffs.insert( label_id, old_version, new_version, request.json_body) return success() + + +@secure_write +def delete(request, label_id, old_version, new_version): + """Delete the diff from the db""" + storage.for_diffs.delete(label_id, old_version, new_version) + return success() diff --git a/regcore_write/views/document.py b/regcore_write/views/document.py index 2ae42f3..1205945 100644 --- a/regcore_write/views/document.py +++ b/regcore_write/views/document.py @@ -34,7 +34,7 @@ @secure_write @json_body def add(request, doc_type, label_id, version=None): - """Add this regulation node and all of its children to the db""" + """Add this document node and all of its children to the db""" try: node = request.json_body jsonschema.validate(node, REGULATION_SCHEMA) @@ -67,3 +67,10 @@ def add_node(node, parent=None): storage.for_documents.bulk_delete(doc_type, label_id, version) storage.for_documents.bulk_insert(to_save, doc_type, version) + + +@secure_write +def delete(request, doc_type, label_id, version=None): + """Delete this document node and all of its children from the db""" + storage.for_documents.bulk_delete(doc_type, label_id, version) + return success() diff --git a/regcore_write/views/layer.py b/regcore_write/views/layer.py index 43cecc8..96847bf 100644 --- a/regcore_write/views/layer.py +++ b/regcore_write/views/layer.py @@ -49,6 +49,17 @@ def add(request, name, doc_type, doc_id): return success() +@secure_write +def delete(request, name, doc_type, doc_id): + """Delete the layer node and all of its children from the db""" + params = standardize_params(doc_type, doc_id) + if params.doc_type not in ('preamble', 'cfr'): + return user_error('invalid doc type') + + storage.for_layers.bulk_delete(name, params.doc_type, params.doc_id) + return success() + + def child_layers(layer_params, layer_data): """We are generally given a layer corresponding to an entire regulation. We need to split that layer up and store it per node within the diff --git a/regcore_write/views/notice.py b/regcore_write/views/notice.py index 5bee180..0710084 100644 --- a/regcore_write/views/notice.py +++ b/regcore_write/views/notice.py @@ -19,3 +19,10 @@ def add(request, docnum): storage.for_notices.delete(docnum) storage.for_notices.insert(docnum, notice) return success() + + +@secure_write +def delete(request, docnum): + """Delete the notice from the db""" + storage.for_notices.delete(docnum) + return success()