From 95d2943b27b5ad673297ccf389607f65f0329344 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 4 Aug 2016 12:58:53 +0200
Subject: [PATCH] Prevent adding same OVO version twice to history

Some patches go through a lot of rebases and changes and may end up
mistakenly reusing the same history version as a prior entry.

This is usually caught on review, but just to be sure that this never
gets merged this patch adds a check to the CinderObjectVersionsHistory
add method.

Change-Id: Id5ac41ffcce86c3499e49ad60ac46038567e9fdb
---
 cinder/objects/base.py                 |  4 ++++
 cinder/tests/unit/objects/test_base.py | 26 ++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/cinder/objects/base.py b/cinder/objects/base.py
index 56d49b62da9..78764c391bf 100644
--- a/cinder/objects/base.py
+++ b/cinder/objects/base.py
@@ -79,6 +79,10 @@ class CinderObjectVersionsHistory(dict):
         return self[self.get_current()]
 
     def add(self, ver, updates):
+        if ver in self.versions:
+            msg = 'Version %s already exists in history.' % ver
+            raise exception.ProgrammingError(reason=msg)
+
         self[ver] = self[self.get_current()].copy()
         self.versions.append(ver)
         self[ver].update(updates)
diff --git a/cinder/tests/unit/objects/test_base.py b/cinder/tests/unit/objects/test_base.py
index 0a7781db433..0ebcf121a2f 100644
--- a/cinder/tests/unit/objects/test_base.py
+++ b/cinder/tests/unit/objects/test_base.py
@@ -31,6 +31,32 @@ from cinder.tests.unit import fake_objects
 from cinder.tests.unit import objects as test_objects
 
 
+class TestCinderObjectVersionHistory(test_objects.BaseObjectsTestCase):
+    def test_add(self):
+        history = test_objects.obj_base.CinderObjectVersionsHistory()
+        v10 = {'Backup': '2.0'}
+        v11 = {'Backup': '2.1'}
+        history.add('1.0', v10)
+        history.add('1.1', v11)
+        # We have 3 elements because we have the liberty version by default
+        self.assertEqual(2 + 1, len(history))
+
+        expected_v10 = history['liberty'].copy()
+        expected_v10.update(v10)
+        expected_v11 = history['liberty'].copy()
+        expected_v11.update(v11)
+
+        self.assertEqual('1.1', history.get_current())
+        self.assertEqual(expected_v11, history.get_current_versions())
+        self.assertEqual(expected_v10, history['1.0'])
+
+    def test_add_existing(self):
+        history = test_objects.obj_base.CinderObjectVersionsHistory()
+        history.add('1.0', {'Backup': '1.0'})
+        self.assertRaises(exception.ProgrammingError,
+                          history.add, '1.0', {'Backup': '1.0'})
+
+
 class TestCinderObject(test_objects.BaseObjectsTestCase):
     """Tests methods from CinderObject."""