From 37278df338ab4d2753ca004e556a197f847be0ce Mon Sep 17 00:00:00 2001
From: Kiran Pawar <kinpaa@gmail.com>
Date: Thu, 22 Jun 2023 12:36:31 +0000
Subject: [PATCH] Fix duplicate entries in share_server_backend_details

share_server_backend_details_set() add entries in db table without
checking existing entries with given combinaton of share_server_id
and key. This causes duplicate records. Fix it by validating presence
of share server id and key.

Closes-bug: #2024658
Change-Id: I58dcd9716cf95d0d696c13a4c831df787726bcda
---
 manila/db/sqlalchemy/api.py                   | 36 +++++++++++++++----
 manila/exception.py                           |  4 +++
 manila/tests/db/sqlalchemy/test_api.py        |  8 +++++
 ...rver-backend-details-adf45b417d45b437.yaml |  8 +++++
 4 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/bug-2024658-fix-duplicate-entries-of-share-server-backend-details-adf45b417d45b437.yaml

diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py
index e668941595..493c15984c 100644
--- a/manila/db/sqlalchemy/api.py
+++ b/manila/db/sqlalchemy/api.py
@@ -5181,18 +5181,42 @@ def share_server_get_all_unused_deletable(context, host, updated_before):
     return result
 
 
+def _share_server_backend_details_get_item(context,
+                                           share_server_id,
+                                           key, session=None):
+    result = (_share_server_backend_details_get_query(
+        context, share_server_id, session=session).filter_by(key=key).first())
+    if not result:
+        raise exception.ShareServerBackendDetailsNotFound()
+    return result
+
+
+def _share_server_backend_details_get_query(context,
+                                            share_server_id,
+                                            session=None):
+    return (model_query(
+        context, models.ShareServerBackendDetails, session=session,
+        read_deleted="no").
+        filter_by(share_server_id=share_server_id))
+
+
 @require_context
 @context_manager.writer
 def share_server_backend_details_set(context, share_server_id, server_details):
     _share_server_get(context, share_server_id)
 
     for meta_key, meta_value in server_details.items():
-        meta_ref = models.ShareServerBackendDetails()
-        meta_ref.update({
-            'key': meta_key,
-            'value': meta_value,
-            'share_server_id': share_server_id
-        })
+
+        # update the value whether it exists or not
+        item = {"value": meta_value}
+        try:
+            meta_ref = _share_server_backend_details_get_item(
+                context, share_server_id, meta_key, session=context.session)
+        except exception.ShareServerBackendDetailsNotFound:
+            meta_ref = models.ShareServerBackendDetails()
+            item.update({"key": meta_key, "share_server_id": share_server_id})
+
+        meta_ref.update(item)
         meta_ref.save(session=context.session)
     return server_details
 
diff --git a/manila/exception.py b/manila/exception.py
index f4c32f81c4..947b640f72 100644
--- a/manila/exception.py
+++ b/manila/exception.py
@@ -305,6 +305,10 @@ class ShareServerNotReady(ManilaException):
                 "within %(time)s seconds.")
 
 
+class ShareServerBackendDetailsNotFound(NotFound):
+    message = _("Share server backend details does not exist.")
+
+
 class ServiceNotFound(NotFound):
     message = _("Service %(service_id)s could not be found.")
 
diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py
index 256e846def..8b62ea19f7 100644
--- a/manila/tests/db/sqlalchemy/test_api.py
+++ b/manila/tests/db/sqlalchemy/test_api.py
@@ -3596,6 +3596,14 @@ class ShareServerDatabaseAPITestCase(test.TestCase):
             db_api.share_server_get(self.ctxt, server['id'])['backend_details']
         )
 
+        details.update({'value2': '4'})
+        db_api.share_server_backend_details_set(self.ctxt, server['id'],
+                                                details)
+        self.assertDictEqual(
+            details,
+            db_api.share_server_get(self.ctxt, server['id'])['backend_details']
+        )
+
     def test_backend_details_set_not_found(self):
         fake_id = 'FAKE_UUID'
         self.assertRaises(exception.ShareServerNotFound,
diff --git a/releasenotes/notes/bug-2024658-fix-duplicate-entries-of-share-server-backend-details-adf45b417d45b437.yaml b/releasenotes/notes/bug-2024658-fix-duplicate-entries-of-share-server-backend-details-adf45b417d45b437.yaml
new file mode 100644
index 0000000000..85068be61a
--- /dev/null
+++ b/releasenotes/notes/bug-2024658-fix-duplicate-entries-of-share-server-backend-details-adf45b417d45b437.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Share server backend details set function adds db records without
+    checking existing entries. This results in duplicate records for the
+    combination of given share server id and key. Fixed it by updating records
+    if already exist else creating new. See the `launchpad bug 2024658
+    <https://bugs.launchpad.net/manila/+bug/2024658>`_ for more details.