Skip to content

Commit

Permalink
Fix bug in object migration, and add test to cover it
Browse files Browse the repository at this point in the history
There was a bug that could result in some serialized data being lost
during a migration from an older object version.
  • Loading branch information
eriknyquist committed Dec 5, 2022
1 parent a9bbf43 commit b5d2bc2
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 8 deletions.
62 changes: 61 additions & 1 deletion tests/test_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ def migrate_none_to_100(attrs):
self.assertEqual("1.0.3", result.target_version)
self.assertEqual("1.0.0", result.version_reached)

def test_dict_multiple_migrations(self):
def test_dict_multiple_migrations_1(self):
"""
Tests that from_dict can successfully migrate an object that is 4 versions old,
to the latest version (migration decorator)
Expand Down Expand Up @@ -1252,6 +1252,66 @@ def migrate_102_to_103(attrs):
self.assertEqual("1.0.3", result.target_version)
self.assertEqual("1.0.3", result.version_reached)

def test_dict_multiple_migrations_2(self):
"""
Tests that from_dict can successfully migrate an object that is 3 versions old,
when there are 4 versions total, to the latest version (migration decorator)
"""
# Newest config, with version number added
class TestConfig(VersionedObject):
version = "1.0.3"
var1 = 1
var2 = 2
var3 = 3
var4 = 4
var5 = 5
var6 = 6

# oldest config, different format, and without version number
fake_config = {'version': '1.0.1', 'var1': 11, 'var2': 12, 'var3': 113, 'var4': 114}

# Add migration from unversioned to 1.0.0
@migration(TestConfig, None, "1.0.0")
def migrate_none_to_100(attrs):
attrs['var3'] = 13
return attrs

# Add migration from 1.0.0 to 1.0.1
@migration(TestConfig, "1.0.0", "1.0.1")
def migrate_100_to_101(attrs):
attrs['var4'] = 14
return attrs

# Add migration from 1.0.1 to 1.0.2
@migration(TestConfig, "1.0.1", "1.0.2")
def migrate_101_to_102(attrs):
attrs['var5'] = 15
return attrs

# Add migration from 1.0.2 to 1.0.3
@migration(TestConfig, "1.0.2", "1.0.3")
def migrate_102_to_103(attrs):
attrs['var6'] = 16
return attrs

# Load oldest config from dict
ser = Serializer()
cfg = TestConfig()
result = ser.from_dict(fake_config, cfg)

# Verify all migrations were performed
self.assertEqual(11, cfg.var1)
self.assertEqual(12, cfg.var2)
self.assertEqual(113, cfg.var3)
self.assertEqual(114, cfg.var4)
self.assertEqual(15, cfg.var5)
self.assertEqual(16, cfg.var6)

self.assertEqual(True, result.success)
self.assertEqual("1.0.1", result.old_version)
self.assertEqual("1.0.3", result.target_version)
self.assertEqual("1.0.3", result.version_reached)

def test_json_multiple_migrations(self):
"""
Tests that from_json can successfully migrate an object that is 4 versions old,
Expand Down
14 changes: 7 additions & 7 deletions versionedobj/object.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ def _vobj__populate_instance(self):
def _vobj__migrate(cls, version, attrs):
old_version = attrs.get('version', None)
version_before_migration = old_version
version_after_migration = old_version
current_version = old_version

result = None

Expand All @@ -204,17 +204,17 @@ def _vobj__migrate(cls, version, attrs):

# Attempt migrations
for fromversion, toversion, migrate in cls._vobj__migrations:
if fromversion == version_after_migration:
if fromversion == current_version:
attrs = migrate(attrs)

version_after_migration = toversion
if toversion == version:
break
current_version = toversion
if toversion == version:
break

if version_after_migration != version:
if current_version != version:
result.success = False

result.version_reached = version_after_migration
result.version_reached = current_version

return result, attrs

Expand Down

0 comments on commit b5d2bc2

Please sign in to comment.