From 090f3d0dee5ea321daf6522a80f00c4d7589b836 Mon Sep 17 00:00:00 2001
From: Kevin Carrogan <kevin@carrogan.com>
Date: Wed, 1 May 2024 10:12:28 +0100
Subject: [PATCH 1/2] Use bulk updates and creates for denials migrations
---
.../migrations/0023_set_denial_entity_type.py | 42 ++++-----
.../migrations/0024_denials_data_migration.py | 86 ++++++++++---------
2 files changed, 69 insertions(+), 59 deletions(-)
diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py
index 253ad6483..82550e794 100644
--- a/api/external_data/migrations/0023_set_denial_entity_type.py
+++ b/api/external_data/migrations/0023_set_denial_entity_type.py
@@ -5,38 +5,40 @@
def get_denial_entity_type(data):
+ if not isinstance(data, dict):
+ return
- if isinstance(data, dict):
- entity_type = ""
- normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()}
+ entity_type = ""
+ normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()}
- is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true"
- is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true"
- is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0
+ is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true"
+ is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true"
+ is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0
- if is_end_user_flag and is_consignee_flag:
- entity_type = DenialEntityType.END_USER
- elif not is_end_user_flag and is_consignee_flag:
- entity_type = DenialEntityType.CONSIGNEE
- elif is_end_user_flag and not is_consignee_flag:
- entity_type = DenialEntityType.END_USER
- elif not is_end_user_flag and not is_consignee_flag and is_other_role:
- entity_type = DenialEntityType.THIRD_PARTY
+ if is_end_user_flag and is_consignee_flag:
+ entity_type = DenialEntityType.END_USER
+ elif not is_end_user_flag and is_consignee_flag:
+ entity_type = DenialEntityType.CONSIGNEE
+ elif is_end_user_flag and not is_consignee_flag:
+ entity_type = DenialEntityType.END_USER
+ elif not is_end_user_flag and not is_consignee_flag and is_other_role:
+ entity_type = DenialEntityType.THIRD_PARTY
- return entity_type
+ return entity_type
def set_denial_entity_type(apps, schema_editor):
-
DenialEntity = apps.get_model("external_data", "DenialEntity")
- for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True):
+ denials_to_update = []
+ for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True):
denial_entity_type = get_denial_entity_type(denial_entity.data)
-
- if denial_entity_type in ["end_user", "consignee", "third_party"]:
+ if denial_entity_type is not None and denial_entity_type in ["end_user", "consignee", "third_party"]:
denial_entity.entity_type = denial_entity_type
- denial_entity.save()
+ denials_to_update.append(denial_entity)
+
+ DenialEntity.objects.bulk_update(denials_to_update, ["entity_type"])
class Migration(migrations.Migration):
diff --git a/api/external_data/migrations/0024_denials_data_migration.py b/api/external_data/migrations/0024_denials_data_migration.py
index c579082db..7a6dc081c 100644
--- a/api/external_data/migrations/0024_denials_data_migration.py
+++ b/api/external_data/migrations/0024_denials_data_migration.py
@@ -1,65 +1,73 @@
# Generated by Django 4.2.10 on 2024-04-02 14:59
import logging
-from django.db import migrations
-from django.forms.models import model_to_dict
+
+from django.db import IntegrityError, migrations, transaction
from django.db.models import Q
-from django.db import IntegrityError
-from django.db import transaction
-from django.conf import settings
+from django.forms.models import model_to_dict
+
+
log = logging.getLogger(__name__)
+
required_fields = [
- "reference",
- "regime_reg_ref",
- "notifying_government",
- "item_list_codes",
- "item_description",
- "end_use",
- "is_revoked",
- "is_revoked_comment",
- "reason_for_refusal",
+ "reference",
+ "regime_reg_ref",
+ "notifying_government",
+ "item_list_codes",
+ "item_description",
+ "end_use",
+ "is_revoked",
+ "is_revoked_comment",
+ "reason_for_refusal",
]
-def denials_data_migration(apps, schema_editor):
- settings.ELASTICSEARCH_DSL_AUTOSYNC = False
+def denials_data_migration(apps, schema_editor):
DenialEntity = apps.get_model("external_data", "DenialEntity")
Denial = apps.get_model("external_data", "Denial")
# There are a handfull (10) of regime_reg_refs that are null which was in the initial load
- # Assumption here is that they can be deleted since it's erroneous data as we now know
- # regime_reg_ref is considered a unique DN record.
+ # Assumption here is that they can be deleted since it's erroneous data as we now know
+ # regime_reg_ref is considered a unique DN record.
- total_null_regime_reg_ref = DenialEntity.objects.filter(Q(regime_reg_ref__isnull=True) | Q(regime_reg_ref=''))
+ total_null_regime_reg_ref = DenialEntity.objects.filter(Q(regime_reg_ref__isnull=True) | Q(regime_reg_ref=""))
log.info(
- "Delete null regime_reg_ref total -> %s",
- total_null_regime_reg_ref.count(),
+ "Delete null regime_reg_ref total -> %s",
+ total_null_regime_reg_ref.count(),
)
total_null_regime_reg_ref.delete()
- duplicate_denial_errors = []
+
with transaction.atomic():
sid = transaction.savepoint()
+ denials_to_create = []
+ denials_to_update = []
for denial_entity in DenialEntity.objects.all():
+ denial_entity_dict = {
+ key: value for (key, value) in model_to_dict(denial_entity).items() if key in required_fields
+ }
+
try:
- denial_entity_dict = {key:value for (key,value) in model_to_dict(denial_entity).items() if key in required_fields}
- denial , _ = Denial.objects.get_or_create(**denial_entity_dict)
- denial_entity.denial = denial
- denial_entity.save()
- except IntegrityError as e:
- duplicate_denial_errors.append(denial_entity.regime_reg_ref)
-
- if duplicate_denial_errors:
- log.info(
- "There are the following duplicate denials in the database rolling back this migration: -> %s",
- duplicate_denial_errors,
- )
+ denial = Denial.objects.get(**denial_entity_dict)
+ except Denial.DoesNotExist:
+ denial = Denial(**denial_entity_dict)
+ denials_to_create.append(denial)
+
+ denial_entity.denial = denial
+ denials_to_update.append(denial_entity)
+
+ try:
+ Denial.objects.bulk_create(denials_to_create)
+ except IntegrityError:
+ log.exception("There were errors creating denial objects")
transaction.savepoint_rollback(sid)
- else:
- transaction.savepoint_commit(sid)
-
- settings.ELASTICSEARCH_DSL_AUTOSYNC = True
-
+ return
+
+ DenialEntity.objects.bulk_update(denials_to_update, ["denial"])
+
+ transaction.savepoint_commit(sid)
+
+
class Migration(migrations.Migration):
dependencies = [
("external_data", "0023_set_denial_entity_type"),
From 0099de80a63e2799898230e04f3792983aa5d390 Mon Sep 17 00:00:00 2001
From: Kevin Carrogan <kevin@carrogan.com>
Date: Wed, 1 May 2024 10:56:50 +0100
Subject: [PATCH 2/2] fixup
---
api/conf/settings.py | 2 +-
.../migrations/0023_set_denial_entity_type.py | 42 ++++----
.../migrations/0024_denials_data_migration.py | 49 ++++------
.../tests/test_0024_denials_data_migration.py | 95 ++++++++++++-------
4 files changed, 102 insertions(+), 86 deletions(-)
diff --git a/api/conf/settings.py b/api/conf/settings.py
index bc0cdfa5e..45988f022 100644
--- a/api/conf/settings.py
+++ b/api/conf/settings.py
@@ -342,7 +342,7 @@ def _build_redis_url(base_url, db_number, **query_args):
}
ENABLE_SPIRE_SEARCH = env.bool("ENABLE_SPIRE_SEARCH", False)
- ELASTICSEARCH_DSL_AUTOSYNC = True
+
ELASTICSEARCH_PRODUCT_INDEXES = {"LITE": ELASTICSEARCH_PRODUCT_INDEX_ALIAS}
ELASTICSEARCH_APPLICATION_INDEXES = {"LITE": ELASTICSEARCH_APPLICATION_INDEX_ALIAS}
SPIRE_APPLICATION_INDEX_NAME = env.str("SPIRE_APPLICATION_INDEX_NAME", "spire-application-alias")
diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py
index 82550e794..253ad6483 100644
--- a/api/external_data/migrations/0023_set_denial_entity_type.py
+++ b/api/external_data/migrations/0023_set_denial_entity_type.py
@@ -5,40 +5,38 @@
def get_denial_entity_type(data):
- if not isinstance(data, dict):
- return
- entity_type = ""
- normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()}
+ if isinstance(data, dict):
+ entity_type = ""
+ normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()}
- is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true"
- is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true"
- is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0
+ is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true"
+ is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true"
+ is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0
- if is_end_user_flag and is_consignee_flag:
- entity_type = DenialEntityType.END_USER
- elif not is_end_user_flag and is_consignee_flag:
- entity_type = DenialEntityType.CONSIGNEE
- elif is_end_user_flag and not is_consignee_flag:
- entity_type = DenialEntityType.END_USER
- elif not is_end_user_flag and not is_consignee_flag and is_other_role:
- entity_type = DenialEntityType.THIRD_PARTY
+ if is_end_user_flag and is_consignee_flag:
+ entity_type = DenialEntityType.END_USER
+ elif not is_end_user_flag and is_consignee_flag:
+ entity_type = DenialEntityType.CONSIGNEE
+ elif is_end_user_flag and not is_consignee_flag:
+ entity_type = DenialEntityType.END_USER
+ elif not is_end_user_flag and not is_consignee_flag and is_other_role:
+ entity_type = DenialEntityType.THIRD_PARTY
- return entity_type
+ return entity_type
def set_denial_entity_type(apps, schema_editor):
- DenialEntity = apps.get_model("external_data", "DenialEntity")
- denials_to_update = []
+ DenialEntity = apps.get_model("external_data", "DenialEntity")
for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True):
+
denial_entity_type = get_denial_entity_type(denial_entity.data)
- if denial_entity_type is not None and denial_entity_type in ["end_user", "consignee", "third_party"]:
- denial_entity.entity_type = denial_entity_type
- denials_to_update.append(denial_entity)
- DenialEntity.objects.bulk_update(denials_to_update, ["entity_type"])
+ if denial_entity_type in ["end_user", "consignee", "third_party"]:
+ denial_entity.entity_type = denial_entity_type
+ denial_entity.save()
class Migration(migrations.Migration):
diff --git a/api/external_data/migrations/0024_denials_data_migration.py b/api/external_data/migrations/0024_denials_data_migration.py
index 7a6dc081c..5f18ea5f2 100644
--- a/api/external_data/migrations/0024_denials_data_migration.py
+++ b/api/external_data/migrations/0024_denials_data_migration.py
@@ -38,35 +38,26 @@ def denials_data_migration(apps, schema_editor):
)
total_null_regime_reg_ref.delete()
- with transaction.atomic():
- sid = transaction.savepoint()
- denials_to_create = []
- denials_to_update = []
- for denial_entity in DenialEntity.objects.all():
- denial_entity_dict = {
- key: value for (key, value) in model_to_dict(denial_entity).items() if key in required_fields
- }
-
- try:
- denial = Denial.objects.get(**denial_entity_dict)
- except Denial.DoesNotExist:
- denial = Denial(**denial_entity_dict)
- denials_to_create.append(denial)
-
- denial_entity.denial = denial
- denials_to_update.append(denial_entity)
-
- try:
- Denial.objects.bulk_create(denials_to_create)
- except IntegrityError:
- log.exception("There were errors creating denial objects")
- transaction.savepoint_rollback(sid)
- return
-
- DenialEntity.objects.bulk_update(denials_to_update, ["denial"])
-
- transaction.savepoint_commit(sid)
-
+ denials_to_create = []
+ denials_entitiy_to_update = []
+ denials_dict_map = {}
+ for denial_entity in DenialEntity.objects.all():
+ denial_entity_dict = {
+ key: value for (key, value) in model_to_dict(denial_entity).items() if key in required_fields
+ }
+ denials_dict_map[denial_entity_dict["regime_reg_ref"]] = denial_entity_dict
+
+ for denials_dict_map_item in denials_dict_map.values():
+ denial = Denial(**denials_dict_map_item)
+ denials_to_create.append(denial)
+
+ Denial.objects.bulk_create(denials_to_create)
+
+ for denial_entity in DenialEntity.objects.all():
+ denial_entity.denial = Denial.objects.get(regime_reg_ref=denial_entity.regime_reg_ref)
+ denials_entitiy_to_update.append(denial_entity)
+
+ DenialEntity.objects.bulk_update(denials_entitiy_to_update, ["denial"])
class Migration(migrations.Migration):
dependencies = [
diff --git a/api/external_data/migrations/tests/test_0024_denials_data_migration.py b/api/external_data/migrations/tests/test_0024_denials_data_migration.py
index a17be6567..e1bfe8e0b 100644
--- a/api/external_data/migrations/tests/test_0024_denials_data_migration.py
+++ b/api/external_data/migrations/tests/test_0024_denials_data_migration.py
@@ -4,11 +4,65 @@
test_data = [
-{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.123","name":"name 1","address":"address 1","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"},
-{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 2","address":"address 2","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"},
-{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 3","address":"address 3","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"},
-{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 4","address":"address 4","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"},
-{"reference":"DN2010\/0057","name":"bad record","address":"bad record","notifying_government":"UK","country":"bad","item_list_codes":"all","item_description":"bad","end_use":"bad","reason_for_refusal":"bad "}
+ {
+ "reference": "DN2010\/0057",
+ "regime_reg_ref": "reg.123.123",
+ "name": "name 1",
+ "address": "address 1",
+ "notifying_government": "UK",
+ "country": "UK",
+ "item_list_codes": "all",
+ "item_description": "desc a",
+ "end_use": "use 1",
+ "reason_for_refusal": "a",
+ },
+ {
+ "reference": "DN2010\/0057",
+ "regime_reg_ref": "reg.123.1234",
+ "name": "name 2",
+ "address": "address 2",
+ "notifying_government": "UK",
+ "country": "UK",
+ "item_list_codes": "all",
+ "item_description": "desc a",
+ "end_use": "use 1",
+ "reason_for_refusal": "a",
+ },
+ {
+ "reference": "DN2010\/0057",
+ "regime_reg_ref": "reg.123.1234",
+ "name": "name 3",
+ "address": "address 3",
+ "notifying_government": "UK",
+ "country": "UK",
+ "item_list_codes": "all",
+ "item_description": "desc a",
+ "end_use": "use 1",
+ "reason_for_refusal": "a",
+ },
+ {
+ "reference": "DN2010\/0057",
+ "regime_reg_ref": "reg.123.1234",
+ "name": "name 4",
+ "address": "address 4",
+ "notifying_government": "UK",
+ "country": "UK",
+ "item_list_codes": "all",
+ "item_description": "desc a",
+ "end_use": "use 1",
+ "reason_for_refusal": "a",
+ },
+ {
+ "reference": "DN2010\/0057",
+ "name": "bad record",
+ "address": "bad record",
+ "notifying_government": "UK",
+ "country": "bad",
+ "item_list_codes": "all",
+ "item_description": "bad",
+ "end_use": "bad",
+ "reason_for_refusal": "bad ",
+ },
]
@@ -18,14 +72,10 @@ class TestDenialDataMigration(MigratorTestCase):
migrate_from = ("external_data", "0023_set_denial_entity_type")
migrate_to = ("external_data", "0024_denials_data_migration")
-
def prepare(self):
DenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity")
for row in test_data:
- DenialEntity.objects.create(**row)
-
-
-
+ DenialEntity.objects.create(**row)
def test_0023_denials_data_migration(self):
DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity")
@@ -33,27 +83,4 @@ def test_0023_denials_data_migration(self):
self.assertEqual(DenialEntity.objects.all().count(), 4)
self.assertEqual(Denial.objects.all().count(), 2)
- self.assertEqual(Denial.objects.get(regime_reg_ref='reg.123.1234').denial_entity.count(), 3)
-
-
-@pytest.mark.django_db()
-class TestDenialDataDuplicatesMigration(MigratorTestCase):
-
- migrate_from = ("external_data", "0023_set_denial_entity_type")
- migrate_to = ("external_data", "0024_denials_data_migration")
-
-
- def prepare(self):
- DenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity")
- for row in test_data:
- DenialEntity.objects.create(**row)
- test_data[0]["end_use"] = "end_use b"
- DenialEntity.objects.create(**test_data[0])
-
-
-
- def test_0024_denials_data_migration_duplicates(self):
- DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity")
- Denial = self.new_state.apps.get_model("external_data", "Denial")
- self.assertEqual(DenialEntity.objects.all().count(), 5)
- self.assertEqual(Denial.objects.all().count(), 0)
+ self.assertEqual(Denial.objects.get(regime_reg_ref="reg.123.1234").denial_entity.count(), 3)