From f482831517a6a5e50796906cbd98f400c5038a8c Mon Sep 17 00:00:00 2001 From: Beda Kosata Date: Fri, 22 May 2020 15:53:27 +0200 Subject: [PATCH 01/21] add storage of credentials version into SushiFetchAttempt - refs #44 --- apps/nigiri/tests/test_fetching.py | 97 ++++++++++++++++++- ...hifetchattempt_credentials_version_hash.py | 23 +++++ apps/sushi/models.py | 55 ++++++++++- 3 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 apps/sushi/migrations/0028_sushifetchattempt_credentials_version_hash.py diff --git a/apps/nigiri/tests/test_fetching.py b/apps/nigiri/tests/test_fetching.py index 7ef5eebe3..2f2b6297a 100644 --- a/apps/nigiri/tests/test_fetching.py +++ b/apps/nigiri/tests/test_fetching.py @@ -9,7 +9,7 @@ from nigiri.client import Sushi5Client from publications.models import Platform from sushi.logic.data_import import import_sushi_credentials -from sushi.models import SushiCredentials, CounterReportType +from sushi.models import SushiCredentials, CounterReportType, SushiFetchAttempt @pytest.mark.django_db @@ -46,3 +46,98 @@ def mock_get_report_data(*args, **kwargs): Sushi5Client.get_report_data = mock_get_report_data cr1.fetch_report(report, start_date='2020-01-01', end_date='2020-01-31') assert orig_params == Sushi5Client.EXTRA_PARAMS + + +@pytest.mark.django_db +class TestCredentialsVersioning(object): + def test_version_hash_changes(self, organizations): + """ + Tests that computation of version_hash from `SushiCredentials` can really distinguish + between different versions of the same object + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + hash1 = cr1.version_hash() + cr1.requestor_id = 'new_id' + hash2 = cr1.version_hash() + assert hash2 != hash1 + cr1.api_key = 'new_api_key' + assert cr1.version_hash() != hash1 + assert cr1.version_hash() != hash2 + print(hash1, hash2) + + def test_version_hash_does_not_change(self, organizations): + """ + Tests that value of version_hash from `SushiCredentials` does not change when some + unrelated changes are made + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + hash1 = cr1.version_hash() + cr1.last_updated_by = None + cr1.outside_consortium = True + cr1.save() + assert cr1.version_hash() == hash1 + + def test_version_info_is_stored_in_fetch_attempt(self, organizations, report_type_nd): + """ + Tests that when we fetch data using `SushiCredentials`, the `SushiFetchAttempt` that is + created contains information about the credentials version - both in `processing_info` + and in `credentials_version_hash` + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + cr1.create_sushi_client() + report = CounterReportType.objects.create( + code='tr', name='tr', counter_version=5, report_type=report_type_nd(0) + ) + + def mock_get_report_data(*args, **kwargs): + return Counter5ReportBase() + + Sushi5Client.get_report_data = mock_get_report_data + attempt: SushiFetchAttempt = cr1.fetch_report( + report, start_date='2020-01-01', end_date='2020-01-31' + ) + assert 'credentials_version' in attempt.processing_info + assert attempt.credentials_version_hash != '' + assert attempt.credentials_version_hash == cr1.version_hash() diff --git a/apps/sushi/migrations/0028_sushifetchattempt_credentials_version_hash.py b/apps/sushi/migrations/0028_sushifetchattempt_credentials_version_hash.py new file mode 100644 index 000000000..6b4621dbe --- /dev/null +++ b/apps/sushi/migrations/0028_sushifetchattempt_credentials_version_hash.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.12 on 2020-05-22 13:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('sushi', '0027_queue_previous_reverse_name'), + ] + + operations = [ + migrations.AddField( + model_name='sushifetchattempt', + name='credentials_version_hash', + field=models.CharField( + default='', + help_text='Hash computed from the credentials at the time this attempt was made', + max_length=32, + ), + preserve_default=False, + ), + ] diff --git a/apps/sushi/models.py b/apps/sushi/models.py index 61686628c..680709d69 100644 --- a/apps/sushi/models.py +++ b/apps/sushi/models.py @@ -1,3 +1,4 @@ +import hashlib import os import logging import traceback @@ -5,7 +6,7 @@ from hashlib import blake2b from datetime import timedelta, datetime from itertools import takewhile -from typing import Optional +from typing import Optional, Dict import json import requests @@ -111,6 +112,7 @@ class SushiCredentials(models.Model): (UL_CONS_STAFF, 'Consortium staff'), (UL_CONS_ADMIN, 'Superuser'), ) + blake_hash_size = 16 organization = models.ForeignKey(Organization, on_delete=models.CASCADE) platform = models.ForeignKey(Platform, on_delete=models.CASCADE) @@ -218,6 +220,45 @@ def when_can_access(self, base_wait_unit=5) -> float: return diff return 0 + def version_dict(self) -> Dict: + """ + Returns a dictionary will all the attributes of this object that may be subject to + change between versions and which influence success with querying the remote + server. + It is used to store credentials version information with SushiFetchAttempts and as a + source for hashing for `credentials_version_hash`. + :return: + """ + keys = { + 'url', + 'counter_version', + 'requestor_id', + 'customer_id', + 'http_username', + 'http_password', + 'api_key', + 'extra_params', + } + return {key: getattr(self, key) for key in keys} + + @classmethod + def hash_version_dict(cls, data): + """ + Return a has of a dictionary. Must take care of possible differences in ordering of keys + :param data: + :return: + """ + dump = json.dumps(data, ensure_ascii=False, sort_keys=True) + return blake2b(dump.encode('utf-8'), digest_size=cls.blake_hash_size).hexdigest() + + def version_hash(self): + """ + A hash of the variable things of current credentials - may be used to detect changes + in credentials. + :return: + """ + return self.hash_version_dict(self.version_dict()) + def fetch_report( self, counter_report: CounterReportType, @@ -244,12 +285,20 @@ def fetch_report( else: attempt_params = fetch_m(client, counter_report, start_date, end_date) attempt_params['in_progress'] = False + # add version info to the attempt + attempt_params['credentials_version_hash'] = self.version_hash() + # now store it - into an existing object or a new one if fetch_attempt: for key, value in attempt_params.items(): setattr(fetch_attempt, key, value) + fetch_attempt.processing_info['credentials_version'] = self.version_dict() fetch_attempt.save() return fetch_attempt else: + if 'processing_info' in attempt_params: + attempt_params['processing_info']['credentials_version'] = self.version_dict() + else: + attempt_params['processing_info'] = {'credentials_version': self.version_dict()} attempt = SushiFetchAttempt.objects.create(**attempt_params) return attempt @@ -448,6 +497,10 @@ class SushiFetchAttempt(models.Model): is_processed = models.BooleanField(default=False, help_text='Was the data converted into logs?') when_processed = models.DateTimeField(null=True, blank=True) import_batch = models.OneToOneField(ImportBatch, null=True, on_delete=models.SET_NULL) + credentials_version_hash = models.CharField( + max_length=2 * SushiCredentials.blake_hash_size, + help_text='Hash computed from the credentials at the time this attempt was made', + ) processing_info = JSONField(default=dict, help_text='Internal info') def __str__(self): From 8288aa2cf6bf4ed4ed2259775e4518bf6dc38545 Mon Sep 17 00:00:00 2001 From: Beda Kosata Date: Mon, 25 May 2020 10:12:35 +0200 Subject: [PATCH 02/21] add version_hash attribute to the SushiCredentials object --- apps/nigiri/tests/test_fetching.py | 101 +------------- .../0029_sushicredentials_version_hash.py | 36 +++++ apps/sushi/models.py | 14 +- .../tests/test_sushicredentials_model.py | 126 +++++++++++++++++- 4 files changed, 176 insertions(+), 101 deletions(-) create mode 100644 apps/sushi/migrations/0029_sushicredentials_version_hash.py diff --git a/apps/nigiri/tests/test_fetching.py b/apps/nigiri/tests/test_fetching.py index 2f2b6297a..49ac8fa35 100644 --- a/apps/nigiri/tests/test_fetching.py +++ b/apps/nigiri/tests/test_fetching.py @@ -3,13 +3,13 @@ import pytest from nigiri.counter5 import Counter5ReportBase -from organizations.tests.conftest import organizations -from logs.tests.conftest import report_type_nd from nigiri.client import Sushi5Client from publications.models import Platform from sushi.logic.data_import import import_sushi_credentials -from sushi.models import SushiCredentials, CounterReportType, SushiFetchAttempt +from sushi.models import SushiCredentials, CounterReportType +from organizations.tests.conftest import organizations +from logs.tests.conftest import report_type_nd @pytest.mark.django_db @@ -46,98 +46,3 @@ def mock_get_report_data(*args, **kwargs): Sushi5Client.get_report_data = mock_get_report_data cr1.fetch_report(report, start_date='2020-01-01', end_date='2020-01-31') assert orig_params == Sushi5Client.EXTRA_PARAMS - - -@pytest.mark.django_db -class TestCredentialsVersioning(object): - def test_version_hash_changes(self, organizations): - """ - Tests that computation of version_hash from `SushiCredentials` can really distinguish - between different versions of the same object - """ - data = [ - { - 'platform': 'XXX', - 'organization': organizations[1].internal_id, - 'customer_id': 'BBB', - 'requestor_id': 'RRRX', - 'URL': 'http://this.is/test/2', - 'version': 5, - 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', - }, - ] - Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) - import_sushi_credentials(data) - assert SushiCredentials.objects.count() == 1 - cr1 = SushiCredentials.objects.get() - hash1 = cr1.version_hash() - cr1.requestor_id = 'new_id' - hash2 = cr1.version_hash() - assert hash2 != hash1 - cr1.api_key = 'new_api_key' - assert cr1.version_hash() != hash1 - assert cr1.version_hash() != hash2 - print(hash1, hash2) - - def test_version_hash_does_not_change(self, organizations): - """ - Tests that value of version_hash from `SushiCredentials` does not change when some - unrelated changes are made - """ - data = [ - { - 'platform': 'XXX', - 'organization': organizations[1].internal_id, - 'customer_id': 'BBB', - 'requestor_id': 'RRRX', - 'URL': 'http://this.is/test/2', - 'version': 5, - 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', - }, - ] - Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) - import_sushi_credentials(data) - assert SushiCredentials.objects.count() == 1 - cr1 = SushiCredentials.objects.get() - hash1 = cr1.version_hash() - cr1.last_updated_by = None - cr1.outside_consortium = True - cr1.save() - assert cr1.version_hash() == hash1 - - def test_version_info_is_stored_in_fetch_attempt(self, organizations, report_type_nd): - """ - Tests that when we fetch data using `SushiCredentials`, the `SushiFetchAttempt` that is - created contains information about the credentials version - both in `processing_info` - and in `credentials_version_hash` - """ - data = [ - { - 'platform': 'XXX', - 'organization': organizations[1].internal_id, - 'customer_id': 'BBB', - 'requestor_id': 'RRRX', - 'URL': 'http://this.is/test/2', - 'version': 5, - 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', - }, - ] - Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) - import_sushi_credentials(data) - assert SushiCredentials.objects.count() == 1 - cr1 = SushiCredentials.objects.get() - cr1.create_sushi_client() - report = CounterReportType.objects.create( - code='tr', name='tr', counter_version=5, report_type=report_type_nd(0) - ) - - def mock_get_report_data(*args, **kwargs): - return Counter5ReportBase() - - Sushi5Client.get_report_data = mock_get_report_data - attempt: SushiFetchAttempt = cr1.fetch_report( - report, start_date='2020-01-01', end_date='2020-01-31' - ) - assert 'credentials_version' in attempt.processing_info - assert attempt.credentials_version_hash != '' - assert attempt.credentials_version_hash == cr1.version_hash() diff --git a/apps/sushi/migrations/0029_sushicredentials_version_hash.py b/apps/sushi/migrations/0029_sushicredentials_version_hash.py new file mode 100644 index 000000000..7e8b4c892 --- /dev/null +++ b/apps/sushi/migrations/0029_sushicredentials_version_hash.py @@ -0,0 +1,36 @@ +# Generated by Django 2.2.12 on 2020-05-25 08:02 + +from django.db import migrations, models + + +def fill_version_hash(apps, schema_editor): + """ + The `save` method takes care of storing the version_hash, so we just need to run `save` on + all instances + """ + SushiCredentials = apps.get_model('sushi', 'SushiCredentials') + for credentials in SushiCredentials.objects.all(): + credentials.save() + + +def noop(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('sushi', '0028_sushifetchattempt_credentials_version_hash'), + ] + + operations = [ + migrations.AddField( + model_name='sushicredentials', + name='version_hash', + field=models.CharField( + default='', help_text='Current hash of model attributes', max_length=32 + ), + preserve_default=False, + ), + migrations.RunPython(fill_version_hash, noop), + ] diff --git a/apps/sushi/models.py b/apps/sushi/models.py index 680709d69..af065d5f8 100644 --- a/apps/sushi/models.py +++ b/apps/sushi/models.py @@ -140,6 +140,9 @@ class SushiCredentials(models.Model): default=UL_ORG_ADMIN, help_text='Only user with the same or higher level can unlock it and/or edit it', ) + version_hash = models.CharField( + max_length=blake_hash_size * 2, help_text='Current hash of model attributes' + ) class Meta: unique_together = (('organization', 'platform', 'counter_version'),) @@ -148,6 +151,13 @@ class Meta: def __str__(self): return f'{self.organization} - {self.platform}, {self.get_counter_version_display()}' + def save(self, *args, **kwargs): + """ + We override the parent save method to make sure `version_hash` is recomputed on each save + """ + self.version_hash = self.compute_version_hash() + super().save(*args, **kwargs) + def change_lock(self, user: User, level: int): """ Set the lock_level on this object @@ -251,7 +261,7 @@ def hash_version_dict(cls, data): dump = json.dumps(data, ensure_ascii=False, sort_keys=True) return blake2b(dump.encode('utf-8'), digest_size=cls.blake_hash_size).hexdigest() - def version_hash(self): + def compute_version_hash(self): """ A hash of the variable things of current credentials - may be used to detect changes in credentials. @@ -286,7 +296,7 @@ def fetch_report( attempt_params = fetch_m(client, counter_report, start_date, end_date) attempt_params['in_progress'] = False # add version info to the attempt - attempt_params['credentials_version_hash'] = self.version_hash() + attempt_params['credentials_version_hash'] = self.compute_version_hash() # now store it - into an existing object or a new one if fetch_attempt: for key, value in attempt_params.items(): diff --git a/apps/sushi/tests/test_sushicredentials_model.py b/apps/sushi/tests/test_sushicredentials_model.py index 171ff30b9..0b0f46736 100644 --- a/apps/sushi/tests/test_sushicredentials_model.py +++ b/apps/sushi/tests/test_sushicredentials_model.py @@ -2,9 +2,11 @@ from rest_framework.exceptions import PermissionDenied from core.models import UL_CONS_ADMIN, UL_ORG_ADMIN, UL_CONS_STAFF, Identity +from nigiri.client import Sushi5Client +from nigiri.counter5 import Counter5ReportBase from organizations.models import UserOrganization from sushi.logic.data_import import import_sushi_credentials -from ..models import SushiCredentials +from ..models import SushiCredentials, CounterReportType, SushiFetchAttempt from publications.models import Platform from organizations.tests.conftest import organizations from publications.tests.conftest import platforms @@ -102,3 +104,125 @@ def _test_change_lock(cls, credentials, user, level, can): else: with pytest.raises(PermissionDenied): credentials.change_lock(user, level) + + +@pytest.mark.django_db +class TestCredentialsVersioning(object): + def test_version_hash_is_stored(self, organizations): + """ + Tests that version_hash is computed and store on save + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + assert cr1.version_hash != '' + assert cr1.version_hash == cr1.compute_version_hash() + old_hash = cr1.version_hash + cr1.api_key = 'new_api_key' + assert cr1.compute_version_hash() != cr1.version_hash, 'no change without a save' + cr1.save() + assert cr1.compute_version_hash() == cr1.version_hash + assert cr1.version_hash != old_hash + + def test_version_hash_changes(self, organizations): + """ + Tests that computation of version_hash from `SushiCredentials` can really distinguish + between different versions of the same object + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + hash1 = cr1.compute_version_hash() + cr1.requestor_id = 'new_id' + hash2 = cr1.compute_version_hash() + assert hash2 != hash1 + cr1.api_key = 'new_api_key' + assert cr1.compute_version_hash() != hash1 + assert cr1.compute_version_hash() != hash2 + + def test_version_hash_does_not_change(self, organizations): + """ + Tests that value of version_hash from `SushiCredentials` does not change when some + unrelated changes are made + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + hash1 = cr1.compute_version_hash() + cr1.last_updated_by = None + cr1.outside_consortium = True + cr1.save() + assert cr1.compute_version_hash() == hash1 + + def test_version_info_is_stored_in_fetch_attempt(self, organizations, report_type_nd): + """ + Tests that when we fetch data using `SushiCredentials`, the `SushiFetchAttempt` that is + created contains information about the credentials version - both in `processing_info` + and in `credentials_version_hash` + """ + data = [ + { + 'platform': 'XXX', + 'organization': organizations[1].internal_id, + 'customer_id': 'BBB', + 'requestor_id': 'RRRX', + 'URL': 'http://this.is/test/2', + 'version': 5, + 'extra_attrs': 'auth=un,pass;api_key=kekekeyyy;foo=bar', + }, + ] + Platform.objects.create(short_name='XXX', name='XXXX', ext_id=10) + import_sushi_credentials(data) + assert SushiCredentials.objects.count() == 1 + cr1 = SushiCredentials.objects.get() + cr1.create_sushi_client() + report = CounterReportType.objects.create( + code='tr', name='tr', counter_version=5, report_type=report_type_nd(0) + ) + + def mock_get_report_data(*args, **kwargs): + return Counter5ReportBase() + + Sushi5Client.get_report_data = mock_get_report_data + attempt: SushiFetchAttempt = cr1.fetch_report( + report, start_date='2020-01-01', end_date='2020-01-31' + ) + assert 'credentials_version' in attempt.processing_info + assert attempt.credentials_version_hash != '' + assert attempt.credentials_version_hash == cr1.compute_version_hash() From 55790f9ab16d8381f00103ec5f92987c35198f11 Mon Sep 17 00:00:00 2001 From: Beda Kosata Date: Mon, 25 May 2020 13:47:56 +0200 Subject: [PATCH 03/21] fix hash computation in SushiCredentials migration implement different modes for fetching stats about Sushi attempts add the different modes into UI --- apps/organizations/tests/conftest.py | 2 +- .../0029_sushicredentials_version_hash.py | 23 ++- apps/sushi/tests/test_views.py | 192 +++++++++++++++++- apps/sushi/urls.py | 6 +- apps/sushi/views.py | 51 +++-- .../ui/src/pages/SushiFetchAttemptsPage.vue | 32 ++- test_coverage.sh | 2 +- 7 files changed, 288 insertions(+), 20 deletions(-) diff --git a/apps/organizations/tests/conftest.py b/apps/organizations/tests/conftest.py index f8488fe65..40dda330c 100644 --- a/apps/organizations/tests/conftest.py +++ b/apps/organizations/tests/conftest.py @@ -51,7 +51,7 @@ def identity_by_user_type( ): def fn(user_type): org = organizations[0] - # we do not user admin_client, master_client, etc. because the way the fixtures work + # we do not use admin_client, master_client, etc. because the way the fixtures work # they all point to the same client object which obviously does not work if user_type == 'no_user': identity = None diff --git a/apps/sushi/migrations/0029_sushicredentials_version_hash.py b/apps/sushi/migrations/0029_sushicredentials_version_hash.py index 7e8b4c892..ee8bb1979 100644 --- a/apps/sushi/migrations/0029_sushicredentials_version_hash.py +++ b/apps/sushi/migrations/0029_sushicredentials_version_hash.py @@ -1,15 +1,34 @@ # Generated by Django 2.2.12 on 2020-05-25 08:02 +import json +from hashlib import blake2b from django.db import migrations, models +def get_hash(credentials): + keys = { + 'url', + 'counter_version', + 'requestor_id', + 'customer_id', + 'http_username', + 'http_password', + 'api_key', + 'extra_params', + } + data = {key: getattr(credentials, key) for key in keys} + dump = json.dumps(data, ensure_ascii=False, sort_keys=True) + return blake2b(dump.encode('utf-8'), digest_size=16).hexdigest() + + def fill_version_hash(apps, schema_editor): """ - The `save` method takes care of storing the version_hash, so we just need to run `save` on - all instances + The model does not have the usual methods, etc. here, so we use a + local implementation of the hash computation """ SushiCredentials = apps.get_model('sushi', 'SushiCredentials') for credentials in SushiCredentials.objects.all(): + credentials.version_hash = get_hash(credentials) credentials.save() diff --git a/apps/sushi/tests/test_views.py b/apps/sushi/tests/test_views.py index 170ca709e..6d359856b 100644 --- a/apps/sushi/tests/test_views.py +++ b/apps/sushi/tests/test_views.py @@ -3,7 +3,7 @@ from core.models import UL_ORG_ADMIN, UL_CONS_ADMIN, UL_CONS_STAFF, Identity from organizations.models import UserOrganization -from sushi.models import SushiCredentials +from sushi.models import SushiCredentials, SushiFetchAttempt from organizations.tests.conftest import organizations from publications.tests.conftest import platforms from core.tests.conftest import master_client, master_identity, valid_identity, authenticated_client @@ -213,3 +213,193 @@ def test_destroy_locked_lower( resp = authenticated_client.delete(url) assert resp.status_code == 204 assert SushiCredentials.objects.count() == 0 + + +@pytest.mark.now +@pytest.mark.django_db() +class TestSushiFetchAttemptStatsView(object): + def test_no_dates_mode_all( + self, organizations, platforms, counter_report_type_named, master_client + ): + """ + Test that the api view works when the requested data does not contain dates and all + attempts are requested + """ + credentials = SushiCredentials.objects.create( + organization=organizations[0], + platform=platforms[0], + counter_version=5, + lock_level=UL_ORG_ADMIN, + url='http://a.b.c/', + ) + new_rt1 = counter_report_type_named('new1') + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + ) + assert SushiFetchAttempt.objects.count() == 1 + url = reverse('sushi-fetch-attempt-stats') + resp = master_client.get(url + '?mode=all') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['failure_count'] == 1 + + def test_no_dates_mode_current( + self, organizations, platforms, counter_report_type_named, master_client + ): + """ + Test that the api view works when the requested data does not contain dates and all + attempts are requested + """ + credentials = SushiCredentials.objects.create( + organization=organizations[0], + platform=platforms[0], + counter_version=5, + lock_level=UL_ORG_ADMIN, + url='http://a.b.c/', + ) + new_rt1 = counter_report_type_named('new1') + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + ) + assert SushiFetchAttempt.objects.count() == 1 + # now update the credentials so that the attempt is no longer related to the current + # version + credentials.customer_id = 'new_id' + credentials.save() + # let's try it - there should be nothing + url = reverse('sushi-fetch-attempt-stats') + resp = master_client.get(url + '?mode=current') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 0 + + def test_no_dates_mode_current_2( + self, organizations, platforms, counter_report_type_named, master_client + ): + """ + Test that the api view works when the requested data does not contain dates and all + attempts are requested + """ + credentials = SushiCredentials.objects.create( + organization=organizations[0], + platform=platforms[0], + counter_version=5, + lock_level=UL_ORG_ADMIN, + url='http://a.b.c/', + ) + new_rt1 = counter_report_type_named('new1') + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + ) + assert SushiFetchAttempt.objects.count() == 1 + # now update the credentials so that the attempt is no longer related to the current + # version + credentials.customer_id = 'new_id' + credentials.save() + # create a second attempt, this one with current version + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + ) + assert SushiFetchAttempt.objects.count() == 2 + # let's try it - there should be nothing + url = reverse('sushi-fetch-attempt-stats') + resp = master_client.get(url + '?mode=current') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['failure_count'] == 1 + # let's check that with mode=all there would be two + resp = master_client.get(url + '?mode=all') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['failure_count'] == 2 + + def test_no_dates_mode_success_and_current( + self, organizations, platforms, counter_report_type_named, master_client + ): + """ + Test that the api view works when the requested data does not contain dates and all + attempts are requested + """ + credentials = SushiCredentials.objects.create( + organization=organizations[0], + platform=platforms[0], + counter_version=5, + lock_level=UL_ORG_ADMIN, + url='http://a.b.c/', + ) + new_rt1 = counter_report_type_named('new1') + # one success + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + contains_data=True, + ) + # one failure + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + contains_data=False, + ) + assert SushiFetchAttempt.objects.count() == 2 + # now update the credentials so that the attempt is no longer related to the current + # version + credentials.customer_id = 'new_id' + credentials.save() + # create a second attempt, this one with current version + # one new failure + SushiFetchAttempt.objects.create( + credentials=credentials, + start_date='2020-01-01', + end_date='2020-01-31', + credentials_version_hash=credentials.version_hash, + counter_report=new_rt1, + contains_data=False, + ) + assert SushiFetchAttempt.objects.count() == 3 + # let's try it - there should be nothing + url = reverse('sushi-fetch-attempt-stats') + resp = master_client.get(url + '?mode=success_and_current&success_metric=contains_data') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['success_count'] == 1 + assert data[0]['failure_count'] == 1 + # let's check that with mode=current there would be only one + resp = master_client.get(url + '?mode=current&success_metric=contains_data') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['success_count'] == 0 + assert data[0]['failure_count'] == 1 + # let's check that with mode=all there would be three + resp = master_client.get(url + '?mode=all&success_metric=contains_data') + assert resp.status_code == 200 + data = resp.json() + assert len(data) == 1 + assert data[0]['success_count'] == 1 + assert data[0]['failure_count'] == 2 diff --git a/apps/sushi/urls.py b/apps/sushi/urls.py index 015fa15aa..758c6adc1 100644 --- a/apps/sushi/urls.py +++ b/apps/sushi/urls.py @@ -9,7 +9,11 @@ router.register(r'sushi-fetch-attempt', views.SushiFetchAttemptViewSet) urlpatterns = [ - path('sushi-fetch-attempt-stats/', views.SushiFetchAttemptStatsView.as_view()), + path( + 'sushi-fetch-attempt-stats/', + views.SushiFetchAttemptStatsView.as_view(), + name='sushi-fetch-attempt-stats', + ), path('run-task/fetch-new-sushi-data', views.StartFetchNewSushiDataTask.as_view()), path( 'run-task/fetch-new-sushi-data/', diff --git a/apps/sushi/views.py b/apps/sushi/views.py index bcd1e95da..1fe0d0e28 100644 --- a/apps/sushi/views.py +++ b/apps/sushi/views.py @@ -3,7 +3,7 @@ import dateparser import reversion -from django.db.models import Count, Q, Max, Min +from django.db.models import Count, Q, Max, Min, F from django.http import HttpResponseBadRequest from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator @@ -155,6 +155,13 @@ class SushiFetchAttemptStatsView(APIView): 'organization': ('credentials__organization', 'credentials__organization__name'), } + modes = { + 'current': '', # only attempts that match the current version of their credentials + 'success_and_current': '', # all successful and unsuccessful for current version of creds + 'all': '', # all attempts + } + default_mode = 'current' + key_to_attr_map = {value[1]: key for key, value in attr_to_query_param_map.items()} key_to_attr_map.update( {value[0]: key + '_id' for key, value in attr_to_query_param_map.items()} @@ -163,22 +170,26 @@ class SushiFetchAttemptStatsView(APIView): def get(self, request): organizations = request.user.accessible_organizations() - filter_params = {} + filter_params = [] if 'organization' in request.query_params: - filter_params['credentials__organization'] = get_object_or_404( - organizations, pk=request.query_params['organization'] + filter_params.append( + Q( + credentials__organization=get_object_or_404( + organizations, pk=request.query_params['organization'] + ) + ) ) else: - filter_params['credentials__organization__in'] = organizations + filter_params.append(Q(credentials__organization__in=organizations)) if 'platform' in request.query_params: - filter_params['credentials__platform_id'] = request.query_params['platform'] + filter_params.append(Q(credentials__platform_id=request.query_params['platform'])) if 'date_from' in request.query_params: date_from = dateparser.parse(request.query_params['date_from']) if date_from: - filter_params['timestamp__date__gte'] = date_from + filter_params.append(Q(timestamp__date__gte=date_from)) if 'counter_version' in request.query_params: counter_version = request.query_params['counter_version'] - filter_params['credentials__counter_version'] = counter_version + filter_params.append(Q(credentials__counter_version=counter_version)) # what should be in the result? x = request.query_params.get('x', 'report') y = request.query_params.get('y', 'platform') @@ -186,6 +197,22 @@ def get(self, request): success_metric = request.query_params.get('success_metric', self.success_metrics[-1]) if success_metric not in self.success_metrics: success_metric = self.success_metrics[-1] + # deal with mode - we need to add extra filters for some of the modes + mode = request.query_params.get('mode', self.default_mode) + if mode not in self.modes: + mode = self.default_mode + if mode == 'all': + # there is nothing to do here + pass + elif mode == 'current': + filter_params.append(Q(credentials_version_hash=F('credentials__version_hash'))) + elif mode == 'success_and_current': + # all successful + other that match current version of credentials + filter_params.append( + Q(**{success_metric: True}) + | Q(credentials_version_hash=F('credentials__version_hash')) + ) + # fetch the data - we have different code in presence and absence of date in the data if x != 'month' and y != 'month': data = self.get_data_no_months(x, y, filter_params, success_metric) else: @@ -197,7 +224,7 @@ def get(self, request): out.append({self.key_to_attr_map.get(key, key): value for key, value in obj.items()}) return Response(out) - def get_data_no_months(self, x, y, filter_params, success_metric): + def get_data_no_months(self, x, y, filter_params: [], success_metric): if x not in self.attr_to_query_param_map: return HttpResponseBadRequest('unsupported x dimension: "{}"'.format(x)) if y not in self.attr_to_query_param_map: @@ -209,7 +236,7 @@ def get_data_no_months(self, x, y, filter_params, success_metric): values.extend(self.attr_to_query_param_map[y]) # now get the output qs = ( - SushiFetchAttempt.objects.filter(**filter_params) + SushiFetchAttempt.objects.filter(*filter_params) .values(*values) .annotate( success_count=Count('pk', filter=Q(**{success_metric: True})), @@ -218,7 +245,7 @@ def get_data_no_months(self, x, y, filter_params, success_metric): ) return qs - def get_data_with_months(self, dim, filter_params, success_metric): + def get_data_with_months(self, dim, filter_params: [], success_metric): if dim not in self.attr_to_query_param_map: return HttpResponseBadRequest('unsupported dimension: "{}"'.format(dim)) # we use 2 separate fields for dim in order to preserve both the ID of the @@ -232,7 +259,7 @@ def get_data_with_months(self, dim, filter_params, success_metric): while cur_date < end: # now get the output for rec in ( - SushiFetchAttempt.objects.filter(**filter_params) + SushiFetchAttempt.objects.filter(*filter_params) .filter(start_date__lte=cur_date, end_date__gte=cur_date) .values(*values) .annotate( diff --git a/design/ui/src/pages/SushiFetchAttemptsPage.vue b/design/ui/src/pages/SushiFetchAttemptsPage.vue index dce5e695d..539b93609 100644 --- a/design/ui/src/pages/SushiFetchAttemptsPage.vue +++ b/design/ui/src/pages/SushiFetchAttemptsPage.vue @@ -13,6 +13,11 @@ en: counter_version: Counter version success_metric: Success metric all_orgs: Show all organizations + mode: Mode + modes: + all: All + current: Current + success_and_current: Success & current cs: dim: @@ -26,6 +31,11 @@ cs: counter_version: Verze Counter success_metric: Měřítko úspěchu all_orgs: Všechny organizace + mode: Mód + modes: + all: Všechny + current: Aktuální + success_and_current: Úspěšné & aktuální diff --git a/design/ui/src/pages/SushiFetchAttemptsPage.vue b/design/ui/src/pages/SushiFetchAttemptsPage.vue index 539b93609..b3aebb379 100644 --- a/design/ui/src/pages/SushiFetchAttemptsPage.vue +++ b/design/ui/src/pages/SushiFetchAttemptsPage.vue @@ -13,11 +13,6 @@ en: counter_version: Counter version success_metric: Success metric all_orgs: Show all organizations - mode: Mode - modes: - all: All - current: Current - success_and_current: Success & current cs: dim: @@ -31,11 +26,6 @@ cs: counter_version: Verze Counter success_metric: Měřítko úspěchu all_orgs: Všechny organizace - mode: Mód - modes: - all: Všechny - current: Aktuální - success_and_current: Úspěšné & aktuální