Fix StateManager.set_state() logic
This fixes the logic of the StateManager's set_state() method: if, by accident, the state of an identifier is set to the same timestamp as it currently is at, we won't attempt to create another object. The 'uq_cloudkitty_storage_states_identifier' constraint was preventing a bug from happening, but this fixes the code logic. Change-Id: I530e63dbd67c1357d4aac964fffcdb45ce495768
This commit is contained in:
@@ -97,7 +97,7 @@ class StateManager(object):
|
||||
|
||||
# In case the identifier exists with empty columns, update them
|
||||
if not r:
|
||||
# NOTE(peschk_l): We must use == instead of 'is' because sqlachmey
|
||||
# NOTE(peschk_l): We must use == instead of 'is' because sqlalchemy
|
||||
# overloads this operator
|
||||
r = q.filter(self.model.identifier == identifier). \
|
||||
filter(self.model.scope_key == None). \
|
||||
@@ -138,9 +138,10 @@ class StateManager(object):
|
||||
r = self._get_db_item(
|
||||
session, identifier, fetcher, collector, scope_key)
|
||||
|
||||
if r and r.state != state:
|
||||
r.state = state
|
||||
session.commit()
|
||||
if r:
|
||||
if r.state != state:
|
||||
r.state = state
|
||||
session.commit()
|
||||
else:
|
||||
state_object = self.model(
|
||||
identifier=identifier,
|
||||
|
||||
@@ -108,3 +108,29 @@ class StateManagerTest(tests.TestCase):
|
||||
with mock.patch('cloudkitty.db.get_session'):
|
||||
self._test_x_state_no_column_update(
|
||||
lambda x: self._state.set_state(x, datetime(2042, 1, 1)))
|
||||
|
||||
def test_set_state_does_not_duplicate_entries(self):
|
||||
state = datetime(2042, 1, 1)
|
||||
_, query_mock = self._get_query_mock(
|
||||
self._get_r_mock('a', 'b', 'c', state))
|
||||
with mock.patch(
|
||||
'oslo_db.sqlalchemy.utils.model_query',
|
||||
new=query_mock), mock.patch('cloudkitty.db.get_session') as sm:
|
||||
sm.return_value = session_mock = mock.MagicMock()
|
||||
self._state.set_state('fake_identifier', state)
|
||||
session_mock.commit.assert_not_called()
|
||||
session_mock.add.assert_not_called()
|
||||
|
||||
def test_set_state_does_update_state(self):
|
||||
r_mock = self._get_r_mock('a', 'b', 'c', datetime(2000, 1, 1))
|
||||
_, query_mock = self._get_query_mock(r_mock)
|
||||
new_state = datetime(2042, 1, 1)
|
||||
with mock.patch(
|
||||
'oslo_db.sqlalchemy.utils.model_query',
|
||||
new=query_mock), mock.patch('cloudkitty.db.get_session') as sm:
|
||||
sm.return_value = session_mock = mock.MagicMock()
|
||||
self.assertNotEqual(r_mock.state, new_state)
|
||||
self._state.set_state('fake_identifier', new_state)
|
||||
self.assertEqual(r_mock.state, new_state)
|
||||
session_mock.commit.assert_called_once()
|
||||
session_mock.add.assert_not_called()
|
||||
|
||||
Reference in New Issue
Block a user