From fec8841968ddef19390822d643a552a3d0b93533 Mon Sep 17 00:00:00 2001 From: Vitali Yanushchyk Date: Thu, 26 Sep 2024 05:53:39 -0300 Subject: [PATCH] add ! callback at any status change --- .../apps/api/admin/deduplicationset.py | 6 ++-- src/hope_dedup_engine/apps/api/apps.py | 3 ++ .../apps/api/deduplication/process.py | 3 -- ...6_alter_deduplicationset_state_and_more.py | 27 +++++++++++++++ .../apps/api/models/deduplication.py | 25 +++++++++++++- src/hope_dedup_engine/apps/api/serializers.py | 4 +-- src/hope_dedup_engine/apps/api/signals.py | 10 ++++++ src/hope_dedup_engine/apps/api/utils.py | 9 ----- src/hope_dedup_engine/apps/core/checks.py | 2 +- .../apps/core/management/commands/demo.py | 4 ++- tests/api/conftest.py | 12 ++++--- tests/api/test_deduplication_set_process.py | 1 + tests/api/test_find_duplicates.py | 34 +++++++++++++++---- tests/api/test_ignored_keys_create.py | 11 ++++-- tests/api/test_image_create.py | 11 ++++-- tests/api/test_image_delete.py | 13 +++++-- tests/api/test_utils.py | 30 ---------------- 17 files changed, 138 insertions(+), 67 deletions(-) create mode 100644 src/hope_dedup_engine/apps/api/migrations/0006_alter_deduplicationset_state_and_more.py create mode 100644 src/hope_dedup_engine/apps/api/signals.py delete mode 100644 tests/api/test_utils.py diff --git a/src/hope_dedup_engine/apps/api/admin/deduplicationset.py b/src/hope_dedup_engine/apps/api/admin/deduplicationset.py index 3bb82b72..fd3761d1 100644 --- a/src/hope_dedup_engine/apps/api/admin/deduplicationset.py +++ b/src/hope_dedup_engine/apps/api/admin/deduplicationset.py @@ -13,14 +13,14 @@ class DeduplicationSetAdmin(AdminFiltersMixin, ModelAdmin): "id", "name", "reference_pk", - "state", + "state_value", "created_at", "updated_at", "deleted", ) readonly_fields = ( "id", - "state", + "state_value", "external_system", "created_at", "created_by", @@ -30,7 +30,7 @@ class DeduplicationSetAdmin(AdminFiltersMixin, ModelAdmin): ) search_fields = ("name",) list_filter = ( - ("state", ChoicesFieldComboFilter), + ("state_value", ChoicesFieldComboFilter), ("created_at", DateRangeFilter), ("updated_at", DateRangeFilter), DjangoLookupFilter, diff --git a/src/hope_dedup_engine/apps/api/apps.py b/src/hope_dedup_engine/apps/api/apps.py index 1856625e..ad367107 100644 --- a/src/hope_dedup_engine/apps/api/apps.py +++ b/src/hope_dedup_engine/apps/api/apps.py @@ -4,3 +4,6 @@ class ApiConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "hope_dedup_engine.apps.api" + + def ready(self) -> None: + import hope_dedup_engine.apps.api.signals # noqa: F401 diff --git a/src/hope_dedup_engine/apps/api/deduplication/process.py b/src/hope_dedup_engine/apps/api/deduplication/process.py index cefbd55d..88388c39 100644 --- a/src/hope_dedup_engine/apps/api/deduplication/process.py +++ b/src/hope_dedup_engine/apps/api/deduplication/process.py @@ -8,7 +8,6 @@ get_finders, ) from hope_dedup_engine.apps.api.models import DeduplicationSet, Duplicate -from hope_dedup_engine.apps.api.utils import send_notification def _sort_keys(pair: DuplicateKeyPair) -> DuplicateKeyPair: @@ -82,5 +81,3 @@ def find_duplicates(deduplication_set_id: str, serialized_lock: str) -> None: deduplication_set.state = DeduplicationSet.State.ERROR deduplication_set.save() raise - finally: - send_notification(deduplication_set) diff --git a/src/hope_dedup_engine/apps/api/migrations/0006_alter_deduplicationset_state_and_more.py b/src/hope_dedup_engine/apps/api/migrations/0006_alter_deduplicationset_state_and_more.py new file mode 100644 index 00000000..01d7f3d9 --- /dev/null +++ b/src/hope_dedup_engine/apps/api/migrations/0006_alter_deduplicationset_state_and_more.py @@ -0,0 +1,27 @@ +# Generated by Django 5.0.7 on 2024-09-25 06:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0005_config_deduplicationset_config"), + ] + + operations = [ + migrations.AlterField( + model_name="deduplicationset", + name="state", + field=models.IntegerField( + choices=[(0, "Clean"), (1, "Dirty"), (2, "Processing"), (3, "Error")], + db_column="state", + default=0, + ), + ), + migrations.RenameField( + model_name="deduplicationset", + old_name="state", + new_name="state_value", + ), + ] diff --git a/src/hope_dedup_engine/apps/api/models/deduplication.py b/src/hope_dedup_engine/apps/api/models/deduplication.py index 97903227..19abd75d 100644 --- a/src/hope_dedup_engine/apps/api/models/deduplication.py +++ b/src/hope_dedup_engine/apps/api/models/deduplication.py @@ -4,9 +4,13 @@ from django.conf import settings from django.db import models +import requests +import sentry_sdk + from hope_dedup_engine.apps.security.models import ExternalSystem REFERENCE_PK_LENGTH: Final[int] = 100 +REQUEST_TIMEOUT: Final[int] = 5 class Config(models.Model): @@ -33,9 +37,10 @@ class State(models.IntegerChoices): ) description = models.TextField(null=True, blank=True) reference_pk = models.CharField(max_length=REFERENCE_PK_LENGTH) # source_id - state = models.IntegerField( + state_value = models.IntegerField( choices=State.choices, default=State.CLEAN, + db_column="state", ) deleted = models.BooleanField(null=False, blank=False, default=False) external_system = models.ForeignKey(ExternalSystem, on_delete=models.CASCADE) @@ -58,6 +63,24 @@ class State(models.IntegerChoices): notification_url = models.CharField(max_length=255, null=True, blank=True) config = models.OneToOneField(Config, null=True, on_delete=models.SET_NULL) + @property + def state(self) -> State: + return self.State(self.state_value) + + @state.setter + def state(self, value: State) -> None: + if value != self.state_value or value == self.State.CLEAN: + self.state_value = value + if self.notification_url: + self.send_notification() + + def send_notification(self) -> None: + try: + with requests.get(self.notification_url, timeout=REQUEST_TIMEOUT) as r: + r.raise_for_status + except requests.RequestException as e: + sentry_sdk.capture_exception(e) + def __str__(self) -> str: return f"ID: {self.pk}" if not self.name else f"{self.name}" diff --git a/src/hope_dedup_engine/apps/api/serializers.py b/src/hope_dedup_engine/apps/api/serializers.py index b77cd9c5..0e763acd 100644 --- a/src/hope_dedup_engine/apps/api/serializers.py +++ b/src/hope_dedup_engine/apps/api/serializers.py @@ -20,12 +20,12 @@ class Meta: class DeduplicationSetSerializer(serializers.ModelSerializer): - state = serializers.CharField(source="get_state_display", read_only=True) + state = serializers.CharField(source="get_state_value_display", read_only=True) config = ConfigSerializer(required=False) class Meta: model = DeduplicationSet - exclude = ("deleted",) + exclude = ("deleted", "state_value") read_only_fields = ( "external_system", "created_at", diff --git a/src/hope_dedup_engine/apps/api/signals.py b/src/hope_dedup_engine/apps/api/signals.py new file mode 100644 index 00000000..33c035aa --- /dev/null +++ b/src/hope_dedup_engine/apps/api/signals.py @@ -0,0 +1,10 @@ +from django.db.models.signals import post_save +from django.dispatch import receiver + +from .models import DeduplicationSet + + +@receiver(post_save, sender=DeduplicationSet) +def call_state_setter(sender, instance, created, **kwargs): + if created: + instance.state = instance.State.CLEAN diff --git a/src/hope_dedup_engine/apps/api/utils.py b/src/hope_dedup_engine/apps/api/utils.py index 4366b4f2..fe721931 100644 --- a/src/hope_dedup_engine/apps/api/utils.py +++ b/src/hope_dedup_engine/apps/api/utils.py @@ -1,4 +1,3 @@ -import requests from constance import config from rest_framework import status from rest_framework.exceptions import APIException @@ -33,11 +32,3 @@ def start_processing(deduplication_set: DeduplicationSet) -> None: def delete_model_data(_: DeduplicationSet) -> None: # TODO pass - - -REQUEST_TIMEOUT = 5 - - -def send_notification(deduplication_set: DeduplicationSet) -> None: - if url := deduplication_set.notification_url: - requests.get(url, timeout=REQUEST_TIMEOUT) diff --git a/src/hope_dedup_engine/apps/core/checks.py b/src/hope_dedup_engine/apps/core/checks.py index 3d45284e..a73348f8 100644 --- a/src/hope_dedup_engine/apps/core/checks.py +++ b/src/hope_dedup_engine/apps/core/checks.py @@ -36,7 +36,7 @@ class StorageErrorCodes: # pragma: no cover @register() -def example_check(app_configs, **kwargs: Any): +def example_check(app_configs, **kwargs: Any): # pragma: no cover errors = [] for t in settings.TEMPLATES: for d in t["DIRS"]: diff --git a/src/hope_dedup_engine/apps/core/management/commands/demo.py b/src/hope_dedup_engine/apps/core/management/commands/demo.py index 73b9d1de..8fb260ca 100644 --- a/src/hope_dedup_engine/apps/core/management/commands/demo.py +++ b/src/hope_dedup_engine/apps/core/management/commands/demo.py @@ -23,6 +23,7 @@ MESSAGES: Final[dict[str, str]] = { "upload": "Starting upload of files...", "not_exist": "Directory '%s' does not exist.", + "container_success": "Container '%s' created successfully.", "storage_success": "Files uploaded to storage '%s' successfully.", "success": "Finished uploading files to storage.", "failed": "Failed to upload files to storage '%s': %s", @@ -31,7 +32,7 @@ } -class Command(BaseCommand): +class Command(BaseCommand): # pragma: no cover help = "Create demo app" def add_arguments(self, parser: ArgumentParser) -> None: @@ -79,6 +80,7 @@ def handle(self, *args: Any, **options: dict[str, Any]) -> None: try: for storage_name, images_src_path in storages.items(): am = AzuriteManager(storage_name) + self.stdout.write(MESSAGES["container_success"] % storage_name) if images_src_path is None: continue if images_src_path.exists(): diff --git a/tests/api/conftest.py b/tests/api/conftest.py index 94d58d65..e772d59c 100644 --- a/tests/api/conftest.py +++ b/tests/api/conftest.py @@ -75,11 +75,15 @@ def start_processing(mocker: MockerFixture) -> MagicMock: return mocker.patch("hope_dedup_engine.apps.api.views.start_processing") +@fixture +def requests_get_mock(mocker: MockerFixture) -> MagicMock: + return mocker.patch("hope_dedup_engine.apps.api.models.deduplication.requests.get") + + @fixture(autouse=True) -def send_notification(mocker: MockerFixture) -> MagicMock: - return mocker.patch( - "hope_dedup_engine.apps.api.deduplication.process.send_notification" - ) +def send_notification(deduplication_set): + deduplication_set.send_notification = MagicMock() + return deduplication_set.send_notification @fixture diff --git a/tests/api/test_deduplication_set_process.py b/tests/api/test_deduplication_set_process.py index d2ac743a..dff215f6 100644 --- a/tests/api/test_deduplication_set_process.py +++ b/tests/api/test_deduplication_set_process.py @@ -23,6 +23,7 @@ def test_can_trigger_deduplication_set_processing_in_any_state( api_client: APIClient, start_processing: MagicMock, deduplication_set: DeduplicationSet, + requests_get_mock: MagicMock, ) -> None: response = api_client.post( reverse(DEDUPLICATION_SET_PROCESS_VIEW, (deduplication_set.pk,)) diff --git a/tests/api/test_find_duplicates.py b/tests/api/test_find_duplicates.py index 85d1271e..a46c4068 100644 --- a/tests/api/test_find_duplicates.py +++ b/tests/api/test_find_duplicates.py @@ -13,6 +13,7 @@ def test_previous_results_are_removed_before_processing( deduplication_set: DeduplicationSet, duplicate: Duplicate, duplicate_finders: list[DuplicateFinder], + requests_get_mock: MagicMock, ) -> None: assert deduplication_set.duplicate_set.count() find_duplicates( @@ -27,6 +28,7 @@ def test_duplicates_are_stored( image: Image, second_image: Image, all_duplicates_finder: DuplicateFinder, + requests_get_mock: MagicMock, ) -> None: assert not deduplication_set.duplicate_set.count() find_duplicates( @@ -41,6 +43,7 @@ def test_ignored_key_pairs( image: Image, second_image: Image, all_duplicates_finder: DuplicateFinder, + requests_get_mock: MagicMock, ) -> None: assert not deduplication_set.duplicate_set.count() ignored_key_pair = deduplication_set.ignoredkeypair_set.create( @@ -61,6 +64,7 @@ def test_weight_is_taken_into_account( second_image: Image, all_duplicates_finder: DuplicateFinder, no_duplicate_finder: DuplicateFinder, + requests_get_mock: MagicMock, ) -> None: find_duplicates( str(deduplication_set.pk), @@ -70,33 +74,49 @@ def test_weight_is_taken_into_account( @mark.parametrize( - ("notification_url", "notification_send"), - ((None, False), ("", False), ("https://example.com", True)), + ( + "deduplication_set__notification_url", + "deduplication_set__state", + "notification_send", + "new_state", + ), + [ + (None, 0, False, 0), + ("", 0, False, 0), + ("https://example.com", 0, True, 1), + ("https://example.com", 1, True, 1), + ], ) def test_notification_sent_on_successful_run( - notification_url: str | None, notification_send: bool, + new_state: int, deduplication_set: DeduplicationSet, duplicate_finders: list[DuplicateFinder], send_notification: MagicMock, + requests_get_mock: MagicMock, ) -> None: - deduplication_set.notification_url = notification_url - deduplication_set.save() + deduplication_set.state = new_state find_duplicates( str(deduplication_set.pk), str(DeduplicationSetLock.for_deduplication_set(deduplication_set)), ) - send_notification.assert_called_once_with(deduplication_set) + + if notification_send: + send_notification.assert_called_once() + else: + send_notification.assert_not_called() def test_notification_sent_on_failure( deduplication_set: DeduplicationSet, failing_duplicate_finder: DuplicateFinder, send_notification: MagicMock, + requests_get_mock: MagicMock, ) -> None: with raises(Exception): find_duplicates( str(deduplication_set.pk), str(DeduplicationSetLock.for_deduplication_set(deduplication_set)), ) - send_notification.assert_called_once_with(deduplication_set) + assert deduplication_set.state == deduplication_set.State.ERROR + send_notification.assert_called_once() diff --git a/tests/api/test_ignored_keys_create.py b/tests/api/test_ignored_keys_create.py index 8eaa594d..1f45f42a 100644 --- a/tests/api/test_ignored_keys_create.py +++ b/tests/api/test_ignored_keys_create.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + from api_const import IGNORED_KEYS_LIST_VIEW, JSON from pytest import mark from rest_framework import status @@ -12,7 +14,9 @@ def test_can_create_ignored_key_pair( - api_client: APIClient, deduplication_set: DeduplicationSet + api_client: APIClient, + deduplication_set: DeduplicationSet, + requests_get_mock: MagicMock, ) -> None: previous_amount = IgnoredKeyPair.objects.filter( deduplication_set=deduplication_set @@ -87,7 +91,10 @@ def test_missing_pk_handling( def test_deduplication_set_is_updated( - api_client: APIClient, user: User, deduplication_set: DeduplicationSet + api_client: APIClient, + user: User, + deduplication_set: DeduplicationSet, + requests_get_mock: MagicMock, ) -> None: assert deduplication_set.updated_by is None diff --git a/tests/api/test_image_create.py b/tests/api/test_image_create.py index e5c4ee3a..fc135688 100644 --- a/tests/api/test_image_create.py +++ b/tests/api/test_image_create.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + from api_const import IMAGE_LIST_VIEW, JSON from pytest import mark from rest_framework import status @@ -12,7 +14,9 @@ def test_can_create_image( - api_client: APIClient, deduplication_set: DeduplicationSet + api_client: APIClient, + deduplication_set: DeduplicationSet, + requests_get_mock: MagicMock, ) -> None: previous_amount = Image.objects.filter(deduplication_set=deduplication_set).count() data = ImageSerializer(ImageFactory.build()).data @@ -83,7 +87,10 @@ def test_missing_filename_handling( def test_deduplication_set_is_updated( - api_client: APIClient, user: User, deduplication_set: DeduplicationSet + api_client: APIClient, + user: User, + deduplication_set: DeduplicationSet, + requests_get_mock: MagicMock, ) -> None: assert deduplication_set.updated_by is None diff --git a/tests/api/test_image_delete.py b/tests/api/test_image_delete.py index 8bc805dd..8c30bb24 100644 --- a/tests/api/test_image_delete.py +++ b/tests/api/test_image_delete.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + from api_const import IMAGE_DETAIL_VIEW from rest_framework import status from rest_framework.reverse import reverse @@ -8,7 +10,10 @@ def test_can_delete_image( - api_client: APIClient, deduplication_set: DeduplicationSet, image: Image + api_client: APIClient, + deduplication_set: DeduplicationSet, + image: Image, + requests_get_mock: MagicMock, ) -> None: image_count = Image.objects.filter(deduplication_set=deduplication_set).count() assert deduplication_set.state == DeduplicationSet.State.CLEAN @@ -41,7 +46,11 @@ def test_cannot_delete_image_between_systems( def test_deduplication_set_is_updated( - api_client: APIClient, user: User, deduplication_set: DeduplicationSet, image: Image + api_client: APIClient, + user: User, + deduplication_set: DeduplicationSet, + image: Image, + requests_get_mock: MagicMock, ) -> None: assert deduplication_set.updated_by is None response = api_client.delete( diff --git a/tests/api/test_utils.py b/tests/api/test_utils.py deleted file mode 100644 index 886f5e3d..00000000 --- a/tests/api/test_utils.py +++ /dev/null @@ -1,30 +0,0 @@ -from unittest.mock import MagicMock - -from pytest import fixture, mark -from pytest_mock import MockerFixture - -from hope_dedup_engine.apps.api.models import DeduplicationSet -from hope_dedup_engine.apps.api.utils import REQUEST_TIMEOUT, send_notification - - -@fixture -def requests_get_mock(mocker: MockerFixture) -> MagicMock: - return mocker.patch("hope_dedup_engine.apps.api.utils.requests.get") - - -@mark.parametrize("deduplication_set__notification_url", ("https://example.com",)) -def test_notification_is_sent_when_url_is_set( - requests_get_mock: MagicMock, deduplication_set: DeduplicationSet -) -> None: - send_notification(deduplication_set) - requests_get_mock.assert_called_once_with( - deduplication_set.notification_url, timeout=REQUEST_TIMEOUT - ) - - -@mark.parametrize("deduplication_set__notification_url", (None,)) -def test_notification_is_not_sent_when_url_is_not_set( - requests_get_mock: MagicMock, deduplication_set: DeduplicationSet -) -> None: - send_notification(deduplication_set) - requests_get_mock.assert_not_called()