From cb39b915da846fb256dfaf1c5c27f659bf7d92c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Ja=C5=A1ek?= Date: Thu, 3 Oct 2024 08:56:11 +0200 Subject: [PATCH] fix delete of items missing in mongo (#2724) remove items which are stored only in elastic on delete. SDCP-831 --- superdesk/eve_backend.py | 16 +++++++++++++--- superdesk/services.py | 16 +++++++++------- tests/backend_test.py | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/superdesk/eve_backend.py b/superdesk/eve_backend.py index 83f96c21d8..fd5bf3ca41 100644 --- a/superdesk/eve_backend.py +++ b/superdesk/eve_backend.py @@ -365,10 +365,20 @@ def delete(self, endpoint_name, lookup): :param lookup: User mongo query syntax. example 1. ``{'_id':123}``, 2. ``{'item_id': {'$in': [123, 234]}}`` :returns: Returns list of ids which were removed. """ + search_backend = self._lookup_backend(endpoint_name) docs = list(self.get_from_mongo(endpoint_name, lookup=lookup, req=ParsedRequest()).sort("_id", 1)) - removed_ids = self.delete_docs(endpoint_name, docs) - if len(docs) and not len(removed_ids): - logger.warn("No documents for %s resource were deleted using lookup %s", endpoint_name, lookup) + removed_ids = [] + if docs: + removed_ids = self.delete_docs(endpoint_name, docs) + if not removed_ids: + logger.warning("No documents for %s resource were deleted using lookup %s", endpoint_name, lookup) + elif "_id" in lookup and search_backend: + logger.warning("Item missing in mongo, deleting from elastic resource=%s lookup=%s", endpoint_name, lookup) + try: + search_backend.remove(endpoint_name, lookup) + removed_ids = [lookup["_id"]] + except NotFoundError: + pass # not found in elastic and not in mongo cache.clean([endpoint_name]) return removed_ids diff --git a/superdesk/services.py b/superdesk/services.py index 90e4e8008a..e4960df978 100644 --- a/superdesk/services.py +++ b/superdesk/services.py @@ -105,7 +105,12 @@ def delete_from_mongo(self, lookup: Dict[str, Any]): self.backend.delete_from_mongo(self.datasource, lookup) def delete_docs(self, docs): - return self.backend.delete_docs(self.datasource, docs) + for doc in docs: + self.on_delete(doc) + res = self.backend.delete_docs(self.datasource, docs) + for doc in docs: + self.on_deleted(doc) + return res def find_one(self, req, **lookup): res = self.backend.find_one(self.datasource, req=req, **lookup) @@ -211,12 +216,9 @@ def delete_action(self, lookup=None): docs = [] else: docs = list(doc for doc in self.get_from_mongo(None, lookup).sort("_id", pymongo.ASCENDING)) - for doc in docs: - self.on_delete(doc) - res = self.delete(lookup) - for doc in docs: - self.on_deleted(doc) - return res + if not docs: + return self.delete(lookup) + return self.delete_docs(docs) def is_authorized(self, **kwargs): """Subclass should override if the resource handled by the service has intrinsic privileges. diff --git a/tests/backend_test.py b/tests/backend_test.py index 6dba57d9f5..e59874964e 100644 --- a/tests/backend_test.py +++ b/tests/backend_test.py @@ -8,6 +8,8 @@ # AUTHORS and LICENSE files distributed with this source code, or # at https://www.sourcefabric.org/superdesk/license +import bson + from datetime import timedelta from unittest.mock import patch, ANY from superdesk.tests import TestCase @@ -127,3 +129,27 @@ def test_update_doc_missing_in_elastic(self): backend.update("ingest", ids[0], updates, original) items = backend.search("ingest", {"query": {"match_all": {}}}) self.assertEqual(1, items.count()) + + def test_delete_item_missing_in_mongo(self): + backend = get_backend() + item = {"_id": bson.ObjectId(), "name": "bar"} + with self.app.app_context(): + backend.create_in_search("ingest", [item]) + items = backend.search("ingest", {"query": {"match_all": {}}}) + self.assertEqual(1, items.count()) + backend.delete("ingest", lookup={"_id": item["_id"]}) + + items = backend.search("ingest", {"query": {"match_all": {}}}) + self.assertEqual(0, items.count()) + + def test_delete_docs_missing_in_mongo(self): + backend = get_backend() + item = {"_id": bson.ObjectId(), "name": "bar"} + with self.app.app_context(): + backend.create_in_search("ingest", [item]) + items = backend.search("ingest", {"query": {"match_all": {}}}) + self.assertEqual(1, items.count()) + backend.delete_docs("ingest", [item]) + + items = backend.search("ingest", {"query": {"match_all": {}}}) + self.assertEqual(0, items.count())