Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fixes #9252] Migration form Version 3.2 to 4 – map config is missing #959

Merged
merged 3 commits into from
May 19, 2022
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions geonode_mapstore_client/migrations/0002_migrate_map_blob.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Generated by Django 3.2.13 on 2022-04-28 07:32
import ast
import base64
from django.db import migrations, connections


drop_mapstore2_adapter_mapstoreattribute = "DROP TABLE IF EXISTS mapstore2_adapter_mapstoreattribute CASCADE;"
drop_mapstore2_adapter_mapstoredata = "DROP TABLE IF EXISTS mapstore2_adapter_mapstoredata CASCADE;"
drop_mapstore2_adapter_mapstoreresource = "DROP TABLE IF EXISTS mapstore2_adapter_mapstoreresource CASCADE;"
drop_mapstore2_adapter_mapstoreresource_attributes = "DROP TABLE IF EXISTS mapstore2_adapter_mapstoreresource_attributes CASCADE;"


def migrate_map_forward(apps, schema_editor):

exists = False
sql_exists = "SELECT EXISTS (SELECT FROM information_schema.tables WHERE table_name = 'mapstore2_adapter_mapstoredata');"
with connections['default'].cursor() as cursor:
cursor.execute(sql_exists)
result = cursor.fetchall()
if result:
exists = result[0][0]
if exists:
# We can't import the Map model directly as it may be a newer
# version than this migration expects. We use the historical version.
ResourceBase = apps.get_model('base', 'ResourceBase')
for _resource in ResourceBase.objects.filter(resource_type='map').iterator():
to_update = {}
# We can't use map.resourcebase_ptr, we need to explicitly retrieve the resourcebase
# mapstore2_adapter does not exist anymore as an app. So we need raw sql to get the blob from the old table.
try:
sql_string = f'SELECT blob from mapstore2_adapter_mapstoredata where resource_id={_resource.id};'
'''
Getting the Data Blob
'''
with connections['default'].cursor() as cursor:
cursor.execute(sql_string)
result = cursor.fetchall()
if result:
to_update['blob'] = result[0][0]

sql_string = f'SELECT name, value from mapstore2_adapter_mapstoreattribute where resource_id={_resource.id};'
'''
Getting the attributes
'''
with connections['default'].cursor() as cursor:
cursor.execute(sql_string)
result = cursor.fetchall()
if result:
for x in result:
try:
'''
If is a byte we have to decode it
'''
to_update[x[0]] = base64.b64decode(ast.literal_eval(x[1])).decode()
except:
to_update[x[0]] = x[1]

thumb = to_update.pop('thumbnail', None)
if thumb and 'data:image/' not in thumb:
to_update['thumbnail_url'] = to_update.pop('thumbnail')
'''
Updating the resource
'''
if to_update:
ResourceBase.objects.filter(id=_resource.id).update(**to_update)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest running the drop_mapstore2_adapter_* SQL here, before the Exception and only once the data migration has successfully completed.

Also, the exception should be focused to a specific DoesNotExist rather than a catch-anything exception that will prevent the user knowing anything went wrong with their data.

This should prevent the situation where an exception occurs between line 31 and 65, it is caught in line 67 and then continue allows the four tables to still be dropped, via lines 95 to 98, even though the data has not been successfully moved into the resourcebase yet. This situation could create a very unfortunate test of your ability to restore a database from backups (!).

Copy link
Author

@mattiagiupponi mattiagiupponi May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, the exception was permissive. I'm going to remove it so that the migration process will stop in case of an exception (any kind) and the table is not dropped.
Related to the table drop, I'll prefer to keep them into another migration operation to make it more transparent and gives a single operation his job.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, perfect. Yes I think that's safest. If something fails in a data migration I want it to throw an error, so I can go back & fix the data.

Thanks for your work on this!

except Exception as e:
# the table may not exists if is a new-branch master installation
continue



def migrate_map_reverse(apps, schema_editor):
# We can't import the Map model directly as it may be a newer
# version than this migration expects. We use the historical version.

Map = apps.get_model('maps', 'Map')
ResourceBase = apps.get_model('base', 'ResourceBase')
for _map in Map.objects.iterator():
# We can't use map.resourcebase_ptr, we need to explicitly retrieve the resourcebase
resourcebase = ResourceBase.objects.get(id=_map.resourcebase_ptr_id)
resourcebase.blob = dict()
resourcebase.save()


class Migration(migrations.Migration):

dependencies = [
('geonode_mapstore_client', '0001_clean_prev_version_geoapps'),
('maps', '0042_remove_maplayer_styles'),
]

operations = [
migrations.RunPython(migrate_map_forward, migrate_map_reverse),
migrations.RunSQL(drop_mapstore2_adapter_mapstoreattribute),
migrations.RunSQL(drop_mapstore2_adapter_mapstoredata),
migrations.RunSQL(drop_mapstore2_adapter_mapstoreresource),
migrations.RunSQL(drop_mapstore2_adapter_mapstoreresource_attributes)
]