From 0bbd46c33f9be1492d96460d87c1ff6dd2e67d21 Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 1 Apr 2022 17:29:11 +0200 Subject: [PATCH] refactor(api): fix pylint errors --- api/libretime_api/core/models/user.py | 22 +-- .../core/tests/models/test_user.py | 1 - api/libretime_api/permission_constants.py | 4 +- api/libretime_api/permissions.py | 4 +- api/libretime_api/schedule/models/show.py | 4 +- .../schedule/models/webstream.py | 3 +- .../schedule/tests/models/test_schedule.py | 36 ++-- .../schedule/tests/views/test_schedule.py | 175 +++++++++--------- api/libretime_api/schedule/views/schedule.py | 5 +- .../storage/tests/views/test_file.py | 6 +- api/libretime_api/storage/views/file.py | 23 +-- api/libretime_api/tests/runner.py | 18 +- api/libretime_api/tests/test_permissions.py | 24 +-- api/pyproject.toml | 3 + 14 files changed, 159 insertions(+), 169 deletions(-) diff --git a/api/libretime_api/core/models/user.py b/api/libretime_api/core/models/user.py index 30e1a2ef9..134547989 100644 --- a/api/libretime_api/core/models/user.py +++ b/api/libretime_api/core/models/user.py @@ -59,28 +59,26 @@ class User(AbstractBaseUser): def get_short_name(self): return self.first_name - def set_password(self, password): - if not password: + def set_password(self, raw_password): + if not raw_password: self.set_unusable_password() else: - self.password = hashlib.md5(password.encode()).hexdigest() + self.password = hashlib.md5(raw_password.encode()).hexdigest() def is_staff(self): return self.type == ADMIN - def check_password(self, password): + def check_password(self, raw_password): if self.has_usable_password(): - test_password = hashlib.md5(password.encode()).hexdigest() + test_password = hashlib.md5(raw_password.encode()).hexdigest() return test_password == self.password return False - """ - The following methods have to be re-implemented here, as PermissionsMixin - assumes that the User class has a 'group' attribute, which LibreTime does - not currently provide. Once Django starts managing the Database - (managed = True), then this can be replaced with - django.contrib.auth.models.PermissionMixin. - """ + # The following methods have to be re-implemented here, as PermissionsMixin + # assumes that the User class has a 'group' attribute, which LibreTime does + # not currently provide. Once Django starts managing the Database + # (managed = True), then this can be replaced with + # django.contrib.auth.models.PermissionMixin. def is_superuser(self): return self.type == ADMIN diff --git a/api/libretime_api/core/tests/models/test_user.py b/api/libretime_api/core/tests/models/test_user.py index db96ee02d..49b4c88e1 100644 --- a/api/libretime_api/core/tests/models/test_user.py +++ b/api/libretime_api/core/tests/models/test_user.py @@ -1,4 +1,3 @@ -from django.apps import apps from rest_framework.test import APITestCase from ....permission_constants import GROUPS diff --git a/api/libretime_api/permission_constants.py b/api/libretime_api/permission_constants.py index 8431a9e0d..fedc3ae60 100644 --- a/api/libretime_api/permission_constants.py +++ b/api/libretime_api/permission_constants.py @@ -1,4 +1,4 @@ -from .core.models import DJ, GUEST, PROGRAM_MANAGER, USER_TYPES +from .core.models import DJ, GUEST, PROGRAM_MANAGER GUEST_PERMISSIONS = [ "view_schedule", @@ -100,5 +100,5 @@ PROGRAM_MANAGER_PERMISSIONS = GUEST_PERMISSIONS + [ GROUPS = { GUEST: GUEST_PERMISSIONS, DJ: DJ_PERMISSIONS, - PROGRAM_MANAGER: PROGRAM_MANAGER, + PROGRAM_MANAGER: PROGRAM_MANAGER_PERMISSIONS, } diff --git a/api/libretime_api/permissions.py b/api/libretime_api/permissions.py index 539621fb5..27eca1368 100644 --- a/api/libretime_api/permissions.py +++ b/api/libretime_api/permissions.py @@ -41,9 +41,7 @@ def get_permission_for_view(request, view): return f"{permission_type}_apiroot" model = view.model_permission_name own_obj = get_own_obj(request, view) - return "{permission_type}_{own_obj}{model}".format( - permission_type=permission_type, own_obj=own_obj, model=model - ) + return f"{permission_type}_{own_obj}{model}" except AttributeError: return None diff --git a/api/libretime_api/schedule/models/show.py b/api/libretime_api/schedule/models/show.py index 0aa74a437..33674d24b 100644 --- a/api/libretime_api/schedule/models/show.py +++ b/api/libretime_api/schedule/models/show.py @@ -74,7 +74,7 @@ class ShowInstance(models.Model): autoplaylist_built = models.BooleanField() def get_owner(self): - return show.get_owner() + return self.show.get_owner() class Meta: managed = False @@ -87,7 +87,7 @@ class ShowRebroadcast(models.Model): show = models.ForeignKey("Show", models.DO_NOTHING) def get_owner(self): - return show.get_owner() + return self.show.get_owner() class Meta: managed = False diff --git a/api/libretime_api/schedule/models/webstream.py b/api/libretime_api/schedule/models/webstream.py index 82e9fa89c..f3f9a6487 100644 --- a/api/libretime_api/schedule/models/webstream.py +++ b/api/libretime_api/schedule/models/webstream.py @@ -14,8 +14,7 @@ class Webstream(models.Model): mime = models.CharField(max_length=1024, blank=True, null=True) def get_owner(self): - User = get_user_model() - return User.objects.get(pk=self.creator_id) + return get_user_model().objects.get(pk=self.creator_id) class Meta: managed = False diff --git a/api/libretime_api/schedule/tests/models/test_schedule.py b/api/libretime_api/schedule/tests/models/test_schedule.py index eaa2ff502..4430a2232 100644 --- a/api/libretime_api/schedule/tests/models/test_schedule.py +++ b/api/libretime_api/schedule/tests/models/test_schedule.py @@ -28,30 +28,30 @@ class TestSchedule(TestCase): def test_get_cueout(self): # No overlapping schedule datetimes, normal usecase: - s1_starts = datetime(year=2021, month=10, day=2, hour=1, minute=30) - s1 = self.create_schedule(s1_starts) - self.assertEqual(s1.get_cueout(), self.cue_out) - self.assertEqual(s1.get_ends(), s1_starts + self.length) + item1_starts = datetime(year=2021, month=10, day=2, hour=1, minute=30) + item1 = self.create_schedule(item1_starts) + self.assertEqual(item1.get_cueout(), self.cue_out) + self.assertEqual(item1.get_ends(), item1_starts + self.length) # Mixed overlapping schedule datetimes (only ends is overlapping): - s2_starts = datetime(year=2021, month=10, day=2, hour=1, minute=55) - s2 = self.create_schedule(s2_starts) - self.assertEqual(s2.get_cueout(), timedelta(minutes=5)) - self.assertEqual(s2.get_ends(), self.show_instance.ends) + item_2_starts = datetime(year=2021, month=10, day=2, hour=1, minute=55) + item_2 = self.create_schedule(item_2_starts) + self.assertEqual(item_2.get_cueout(), timedelta(minutes=5)) + self.assertEqual(item_2.get_ends(), self.show_instance.ends) # Fully overlapping schedule datetimes (starts and ends are overlapping): - s3_starts = datetime(year=2021, month=10, day=2, hour=2, minute=1) - s3 = self.create_schedule(s3_starts) - self.assertEqual(s3.get_cueout(), self.cue_out) - self.assertEqual(s3.get_ends(), self.show_instance.ends) + item3_starts = datetime(year=2021, month=10, day=2, hour=2, minute=1) + item3 = self.create_schedule(item3_starts) + self.assertEqual(item3.get_cueout(), self.cue_out) + self.assertEqual(item3.get_ends(), self.show_instance.ends) def test_is_valid(self): # Starts before the schedule ends - s1_starts = datetime(year=2021, month=10, day=2, hour=1, minute=30) - s1 = self.create_schedule(s1_starts) - self.assertTrue(s1.is_valid) + item1_starts = datetime(year=2021, month=10, day=2, hour=1, minute=30) + item1 = self.create_schedule(item1_starts) + self.assertTrue(item1.is_valid) # Starts after the schedule ends - s2_starts = datetime(year=2021, month=10, day=2, hour=3) - s2 = self.create_schedule(s2_starts) - self.assertFalse(s2.is_valid) + item_2_starts = datetime(year=2021, month=10, day=2, hour=3) + item_2 = self.create_schedule(item_2_starts) + self.assertFalse(item_2.is_valid) diff --git a/api/libretime_api/schedule/tests/views/test_schedule.py b/api/libretime_api/schedule/tests/views/test_schedule.py index 9ece9fcd3..c65abc5fa 100644 --- a/api/libretime_api/schedule/tests/views/test_schedule.py +++ b/api/libretime_api/schedule/tests/views/test_schedule.py @@ -1,4 +1,3 @@ -import os from datetime import datetime, timedelta, timezone from django.conf import settings @@ -20,78 +19,7 @@ class TestScheduleViewSet(APITestCase): "storage.MusicDir", directory=str(fixture_path), ) - f = baker.make( - "storage.File", - directory=music_dir, - mime="audio/mp3", - filepath=AUDIO_FILENAME, - length=timedelta(seconds=40.86), - cuein=timedelta(seconds=0), - cueout=timedelta(seconds=40.8131), - ) - show = baker.make( - "schedule.ShowInstance", - starts=datetime.now(tz=timezone.utc) - timedelta(minutes=5), - ends=datetime.now(tz=timezone.utc) + timedelta(minutes=5), - ) - scheduleItem = baker.make( - "schedule.Schedule", - starts=datetime.now(tz=timezone.utc), - ends=datetime.now(tz=timezone.utc) + f.length, - cue_out=f.cueout, - instance=show, - file=f, - ) - self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") - response = self.client.get(self.path) - self.assertEqual(response.status_code, 200) - result = response.json() - self.assertEqual(dateparse.parse_datetime(result[0]["ends"]), scheduleItem.ends) - self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), f.cueout) - - def test_schedule_item_trunc(self): - music_dir = baker.make( - "storage.MusicDir", - directory=str(fixture_path), - ) - f = baker.make( - "storage.File", - directory=music_dir, - mime="audio/mp3", - filepath=AUDIO_FILENAME, - length=timedelta(seconds=40.86), - cuein=timedelta(seconds=0), - cueout=timedelta(seconds=40.8131), - ) - show = baker.make( - "schedule.ShowInstance", - starts=datetime.now(tz=timezone.utc) - timedelta(minutes=5), - ends=datetime.now(tz=timezone.utc) + timedelta(seconds=20), - ) - scheduleItem = baker.make( - "schedule.Schedule", - starts=datetime.now(tz=timezone.utc), - ends=datetime.now(tz=timezone.utc) + f.length, - instance=show, - file=f, - ) - self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") - response = self.client.get(self.path) - self.assertEqual(response.status_code, 200) - result = response.json() - self.assertEqual(dateparse.parse_datetime(result[0]["ends"]), show.ends) - expected = show.ends - scheduleItem.starts - self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), expected) - self.assertNotEqual( - dateparse.parse_datetime(result[0]["ends"]), scheduleItem.ends - ) - - def test_schedule_item_invalid(self): - music_dir = baker.make( - "storage.MusicDir", - directory=str(fixture_path), - ) - f = baker.make( + file = baker.make( "storage.File", directory=music_dir, mime="audio/mp3", @@ -108,18 +36,91 @@ class TestScheduleViewSet(APITestCase): schedule_item = baker.make( "schedule.Schedule", starts=datetime.now(tz=timezone.utc), - ends=datetime.now(tz=timezone.utc) + f.length, - cue_out=f.cueout, + ends=datetime.now(tz=timezone.utc) + file.length, + cue_out=file.cueout, instance=show, - file=f, + file=file, + ) + self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + result = response.json() + self.assertEqual( + dateparse.parse_datetime(result[0]["ends"]), schedule_item.ends + ) + self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), file.cueout) + + def test_schedule_item_trunc(self): + music_dir = baker.make( + "storage.MusicDir", + directory=str(fixture_path), + ) + file = baker.make( + "storage.File", + directory=music_dir, + mime="audio/mp3", + filepath=AUDIO_FILENAME, + length=timedelta(seconds=40.86), + cuein=timedelta(seconds=0), + cueout=timedelta(seconds=40.8131), + ) + show = baker.make( + "schedule.ShowInstance", + starts=datetime.now(tz=timezone.utc) - timedelta(minutes=5), + ends=datetime.now(tz=timezone.utc) + timedelta(seconds=20), + ) + schedule_item = baker.make( + "schedule.Schedule", + starts=datetime.now(tz=timezone.utc), + ends=datetime.now(tz=timezone.utc) + file.length, + instance=show, + file=file, + ) + self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + result = response.json() + self.assertEqual(dateparse.parse_datetime(result[0]["ends"]), show.ends) + expected = show.ends - schedule_item.starts + self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), expected) + self.assertNotEqual( + dateparse.parse_datetime(result[0]["ends"]), schedule_item.ends + ) + + def test_schedule_item_invalid(self): + music_dir = baker.make( + "storage.MusicDir", + directory=str(fixture_path), + ) + file = baker.make( + "storage.File", + directory=music_dir, + mime="audio/mp3", + filepath=AUDIO_FILENAME, + length=timedelta(seconds=40.86), + cuein=timedelta(seconds=0), + cueout=timedelta(seconds=40.8131), + ) + show = baker.make( + "schedule.ShowInstance", + starts=datetime.now(tz=timezone.utc) - timedelta(minutes=5), + ends=datetime.now(tz=timezone.utc) + timedelta(minutes=5), + ) + schedule_item = baker.make( + "schedule.Schedule", + starts=datetime.now(tz=timezone.utc), + ends=datetime.now(tz=timezone.utc) + file.length, + cue_out=file.cueout, + instance=show, + file=file, ) invalid_schedule_item = baker.make( "schedule.Schedule", starts=show.ends + timedelta(minutes=1), - ends=show.ends + timedelta(minutes=1) + f.length, - cue_out=f.cueout, + ends=show.ends + timedelta(minutes=1) + file.length, + cue_out=file.cueout, instance=show, - file=f, + file=file, ) self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") response = self.client.get(self.path, {"is_valid": True}) @@ -130,14 +131,14 @@ class TestScheduleViewSet(APITestCase): self.assertEqual( dateparse.parse_datetime(result[0]["ends"]), schedule_item.ends ) - self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), f.cueout) + self.assertEqual(dateparse.parse_duration(result[0]["cue_out"]), file.cueout) def test_schedule_item_range(self): music_dir = baker.make( "storage.MusicDir", directory=str(fixture_path), ) - f = baker.make( + file = baker.make( "storage.File", directory=music_dir, mime="audio/mp3", @@ -156,18 +157,18 @@ class TestScheduleViewSet(APITestCase): schedule_item = baker.make( "schedule.Schedule", starts=filter_point, - ends=filter_point + f.length, - cue_out=f.cueout, + ends=filter_point + file.length, + cue_out=file.cueout, instance=show, - file=f, + file=file, ) previous_item = baker.make( "schedule.Schedule", starts=filter_point - timedelta(minutes=5), - ends=filter_point - timedelta(minutes=5) + f.length, - cue_out=f.cueout, + ends=filter_point - timedelta(minutes=5) + file.length, + cue_out=file.cueout, instance=show, - file=f, + file=file, ) self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") range_start = (filter_point - timedelta(minutes=1)).isoformat( diff --git a/api/libretime_api/schedule/views/schedule.py b/api/libretime_api/schedule/views/schedule.py index a01a6e474..ea9a87f30 100644 --- a/api/libretime_api/schedule/views/schedule.py +++ b/api/libretime_api/schedule/views/schedule.py @@ -34,8 +34,9 @@ class ScheduleViewSet(viewsets.ModelViewSet): filter_valid = self.request.query_params.get("is_valid") if filter_valid is None: return self.queryset.all() + filter_valid = filter_valid.strip().lower() in ("true", "yes", "1") if filter_valid: return self.queryset.filter(starts__lt=F("instance__ends")) - else: - return self.queryset.filter(starts__gte=F("instance__ends")) + + return self.queryset.filter(starts__gte=F("instance__ends")) diff --git a/api/libretime_api/storage/tests/views/test_file.py b/api/libretime_api/storage/tests/views/test_file.py index 533c07f03..05d12415c 100644 --- a/api/libretime_api/storage/tests/views/test_file.py +++ b/api/libretime_api/storage/tests/views/test_file.py @@ -1,5 +1,3 @@ -import os - from django.conf import settings from model_bakery import baker from rest_framework.test import APITestCase @@ -30,13 +28,13 @@ class TestFileViewSet(APITestCase): "storage.MusicDir", directory=str(fixture_path), ) - f = baker.make( + file = baker.make( "storage.File", directory=music_dir, mime="audio/mp3", filepath=AUDIO_FILENAME, ) - path = self.path.format(id=str(f.pk)) + path = self.path.format(id=str(file.pk)) self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}") response = self.client.get(path) self.assertEqual(response.status_code, 200) diff --git a/api/libretime_api/storage/views/file.py b/api/libretime_api/storage/views/file.py index 702deacc9..ebffc1389 100644 --- a/api/libretime_api/storage/views/file.py +++ b/api/libretime_api/storage/views/file.py @@ -2,9 +2,9 @@ import os from django.http import FileResponse from django.shortcuts import get_object_or_404 -from rest_framework import status, viewsets +from rest_framework import viewsets from rest_framework.decorators import action -from rest_framework.response import Response +from rest_framework.serializers import IntegerField from ..models import File from ..serializers import FileSerializer @@ -17,17 +17,10 @@ class FileViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["GET"]) def download(self, request, pk=None): - if pk is None: - return Response("No file requested", status=status.HTTP_400_BAD_REQUEST) - try: - pk = int(pk) - except ValueError: - return Response( - "File ID should be an integer", status=status.HTTP_400_BAD_REQUEST - ) + pk = IntegerField().to_internal_value(data=pk) - filename = get_object_or_404(File, pk=pk) - directory = filename.directory - path = os.path.join(directory.directory, filename.filepath) - response = FileResponse(open(path, "rb"), content_type=filename.mime) - return response + file = get_object_or_404(File, pk=pk) + storage = file.directory + path = os.path.join(storage.directory, file.filepath) + + return FileResponse(open(path, "rb"), content_type=file.mime) diff --git a/api/libretime_api/tests/runner.py b/api/libretime_api/tests/runner.py index fc8ffefba..1a886afb3 100644 --- a/api/libretime_api/tests/runner.py +++ b/api/libretime_api/tests/runner.py @@ -1,3 +1,6 @@ +from typing import List, Type + +from django.db.models import Model from django.test.runner import DiscoverRunner @@ -8,16 +11,21 @@ class ManagedModelTestRunner(DiscoverRunner): to execute the SQL manually to create them. """ + unmanaged_models: List[Type[Model]] = [] + def setup_test_environment(self, *args, **kwargs): from django.apps import apps - self.unmanaged_models = [m for m in apps.get_models() if not m._meta.managed] - for m in self.unmanaged_models: - m._meta.managed = True + for model in apps.get_models(): + if not model._meta.managed: + model._meta.managed = True + self.unmanaged_models.append(model) + super().setup_test_environment(*args, **kwargs) def teardown_test_environment(self, *args, **kwargs): super().teardown_test_environment(*args, **kwargs) + # reset unmanaged models - for m in self.unmanaged_models: - m._meta.managed = False + for model in self.unmanaged_models: + model._meta.managed = False diff --git a/api/libretime_api/tests/test_permissions.py b/api/libretime_api/tests/test_permissions.py index 2b51e1cfd..8a1ae58db 100644 --- a/api/libretime_api/tests/test_permissions.py +++ b/api/libretime_api/tests/test_permissions.py @@ -1,17 +1,10 @@ -import os - from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser from model_bakery import baker from rest_framework.test import APIRequestFactory, APITestCase -from ..core.models import ADMIN, DJ, GUEST, PROGRAM_MANAGER -from ..permission_constants import ( - DJ_PERMISSIONS, - GUEST_PERMISSIONS, - PROGRAM_MANAGER_PERMISSIONS, -) +from ..core.models import DJ, GUEST from ..permissions import IsSystemTokenOrUser @@ -60,9 +53,8 @@ class TestPermissions(APITestCase): def logged_in_test_model(self, model, name, user_type, fn): path = self.path.format(model) - user_created = get_user_model().objects.filter(username=name) - if not user_created: - user = get_user_model().objects.create_user( + if not get_user_model().objects.filter(username=name): + get_user_model().objects.create_user( name, email="test@example.com", password="test", @@ -109,15 +101,15 @@ class TestPermissions(APITestCase): first_name="test", last_name="user", ) - f = baker.make("storage.File", owner=user) - model = f"files/{f.id}" + file = baker.make("storage.File", owner=user) + model = f"files/{file.id}" path = self.path.format(model) self.client.login(username="test-dj", password="test") response = self.client.patch(path, {"name": "newFilename"}) self.assertEqual(response.status_code, 200) def test_dj_post_permissions_failure(self): - user = get_user_model().objects.create_user( + get_user_model().objects.create_user( "test-dj", email="test@example.com", password="test", @@ -125,8 +117,8 @@ class TestPermissions(APITestCase): first_name="test", last_name="user", ) - f = baker.make("storage.File") - model = f"files/{f.id}" + file = baker.make("storage.File") + model = f"files/{file.id}" path = self.path.format(model) self.client.login(username="test-dj", password="test") response = self.client.patch(path, {"name": "newFilename"}) diff --git a/api/pyproject.toml b/api/pyproject.toml index 5b66b95d8..7fbc2ef67 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -10,6 +10,9 @@ disable = [ "missing-module-docstring", ] +[tool.pylint.design] +max-parents = 15 + [build-system] requires = ["setuptools", "wheel"] build-backend = "setuptools.build_meta"