From 11e8ff11b8d85675a296ac661f7da96d3aa42438 Mon Sep 17 00:00:00 2001 From: Lars van Rhijn Date: Sat, 25 Nov 2023 07:38:28 +0100 Subject: [PATCH] Add logging for API endpoints --- .gitlab-ci.yml | 2 +- marietje/manage.py | 2 +- marietje/marietje/admin.py | 4 +- .../management/commands/importusers.py | 2 - .../migrations/0009_auto_20231124_2117.py | 28 +++++++ .../migrations/0010_remove_user_queue.py | 16 ++++ marietje/marietje/models.py | 8 +- marietje/marietje/settings/base.py | 2 +- marietje/marietje/utils.py | 82 ------------------- marietje/playerapi/services.py | 29 +++++++ marietje/playerapi/views.py | 2 +- marietje/queues/admin.py | 71 +++++++++++++++- marietje/queues/api/v1/views.py | 42 +++++++++- .../0012_userqueue_queuelogentry.py | 64 +++++++++++++++ .../migrations/0013_alter_userqueue_user.py | 22 +++++ marietje/queues/models.py | 51 ++++++++++++ marietje/queues/services.py | 41 ++++++++-- marietje/queues/signals.py | 18 ++++ marietje/songs/services.py | 21 ++++- pyproject.toml | 2 +- 20 files changed, 400 insertions(+), 109 deletions(-) create mode 100644 marietje/marietje/migrations/0009_auto_20231124_2117.py create mode 100644 marietje/marietje/migrations/0010_remove_user_queue.py delete mode 100644 marietje/marietje/utils.py create mode 100644 marietje/playerapi/services.py create mode 100644 marietje/queues/migrations/0012_userqueue_queuelogentry.py create mode 100644 marietje/queues/migrations/0013_alter_userqueue_user.py create mode 100644 marietje/queues/signals.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c57f10f..1e2e71d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -12,7 +12,7 @@ black: - python3 -m pip install --upgrade pip - curl -sSL https://install.python-poetry.org | python3 - - export PATH="/root/.local/bin:$PATH" - - poetry install --with dev + - poetry install --with dev --no-root script: - poetry run black --quiet --check marietje diff --git a/marietje/manage.py b/marietje/manage.py index 029aed6..0825dbd 100755 --- a/marietje/manage.py +++ b/marietje/manage.py @@ -17,7 +17,7 @@ if __name__ == "__main__": # issue is really that Django is missing to avoid masking other # exceptions on Python 2. try: - import django + import django # noqa except ImportError: raise ImportError( "Couldn't import Django. Are you sure it's installed and " diff --git a/marietje/marietje/admin.py b/marietje/marietje/admin.py index 5b5f6f4..b86c583 100644 --- a/marietje/marietje/admin.py +++ b/marietje/marietje/admin.py @@ -7,13 +7,13 @@ from .models import User @admin.register(User) class UserAdmin(BaseUserAdmin): fieldsets = ( - (None, {"fields": ("username", "password", "queue")}), + (None, {"fields": ("username", "password")}), (_("Personal info"), {"fields": ("name", "email")}), (_("Permissions"), {"fields": ("is_active", "is_staff", "is_superuser", "groups", "user_permissions")}), (_("Important dates"), {"fields": ("last_login", "date_joined")}), (_("Activation"), {"fields": ("activation_token", "reset_token")}), ) - list_display = ("username", "email", "name", "date_joined", "last_login", "queue", "is_staff") + list_display = ("username", "email", "name", "date_joined", "last_login", "is_staff") search_fields = ("username", "name", "email") def delete_model(self, request, user): diff --git a/marietje/marietje/management/commands/importusers.py b/marietje/marietje/management/commands/importusers.py index 9a7ac0b..aebf9ea 100644 --- a/marietje/marietje/management/commands/importusers.py +++ b/marietje/marietje/management/commands/importusers.py @@ -28,7 +28,6 @@ class Command(BaseCommand): user.name = import_user["n"].strip() user.email = user.username + "@science.ru.nl" user.password = "md5$$" + import_user["p"] - user.queue = get_first_queue() user.save() if options["tsv_file"]: @@ -45,7 +44,6 @@ class Command(BaseCommand): user.name = import_user[2].decode("utf-8", errors="ignore").strip() user.email = user.username + "@science.ru.nl" user.password = import_user[3].decode("utf-8", errors="strict") - user.queue = get_first_queue() user.study = import_user[5].decode("utf-8", errors="ignore").strip() user.save() diff --git a/marietje/marietje/migrations/0009_auto_20231124_2117.py b/marietje/marietje/migrations/0009_auto_20231124_2117.py new file mode 100644 index 0000000..1b0b084 --- /dev/null +++ b/marietje/marietje/migrations/0009_auto_20231124_2117.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.6 on 2023-11-24 20:17 + +from django.db import migrations + + +def create_new_queue_mappings(apps, schema_editor): + """Before removing the old reference to Queue from User, we should move this to the newly created model.""" + User = apps.get_model("marietje", "User") + UserQueue = apps.get_model("queues", "UserQueue") + for user in User.objects.all(): + if user.queue is not None: + UserQueue.objects.create(user=user, queue=user.queue) + else: + UserQueue.objects.create(user=user) + + +class Migration(migrations.Migration): + dependencies = [ + ("marietje", "0008_alter_user_id"), + ("queues", "0012_userqueue_queuelogentry"), + ] + + operations = [ + migrations.RunPython( + create_new_queue_mappings, + migrations.RunPython.noop + ), + ] diff --git a/marietje/marietje/migrations/0010_remove_user_queue.py b/marietje/marietje/migrations/0010_remove_user_queue.py new file mode 100644 index 0000000..0ba8b3b --- /dev/null +++ b/marietje/marietje/migrations/0010_remove_user_queue.py @@ -0,0 +1,16 @@ +# Generated by Django 4.2.6 on 2023-11-24 20:19 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("marietje", "0009_auto_20231124_2117"), + ] + + operations = [ + migrations.RemoveField( + model_name="user", + name="queue", + ), + ] diff --git a/marietje/marietje/models.py b/marietje/marietje/models.py index a6ff521..e4fbecc 100644 --- a/marietje/marietje/models.py +++ b/marietje/marietje/models.py @@ -7,9 +7,6 @@ from django.db import models from django.utils import timezone from django.utils.translation import gettext_lazy as _ -from marietje.utils import get_first_queue -from queues.models import Queue - class UserManager(BaseUserManager): use_in_migrations = True @@ -19,9 +16,8 @@ class UserManager(BaseUserManager): raise ValueError("The given username must be set") email = self.normalize_email(email) username = self.model.normalize_username(username) - queue = get_first_queue() - user = self.model(username=username, email=email, queue=queue, **extra_fields) + user = self.model(username=username, email=email, **extra_fields) user.set_password(password) user.save(using=self._db) return user @@ -80,8 +76,6 @@ class User(AbstractBaseUser, PermissionsMixin): objects = UserManager() - queue = models.ForeignKey(Queue, on_delete=models.SET_NULL, blank=True, null=True) - activation_token = models.TextField(_("activation token"), blank=True, null=True) reset_token = models.TextField(_("reset token"), blank=True, null=True) diff --git a/marietje/marietje/settings/base.py b/marietje/marietje/settings/base.py index 412c69e..aca5a02 100644 --- a/marietje/marietje/settings/base.py +++ b/marietje/marietje/settings/base.py @@ -4,6 +4,7 @@ from pathlib import Path BASE_DIR = Path(__file__).resolve().parent.parent.parent INSTALLED_APPS = [ + 'marietje', 'django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', @@ -15,7 +16,6 @@ INSTALLED_APPS = [ 'rest_framework', 'tinymce', 'announcements', - 'marietje', 'queues', 'songs', 'stats', diff --git a/marietje/marietje/utils.py b/marietje/marietje/utils.py deleted file mode 100644 index 363587d..0000000 --- a/marietje/marietje/utils.py +++ /dev/null @@ -1,82 +0,0 @@ -import binascii -import socket -import struct - -from django.conf import settings -from django.http import StreamingHttpResponse - -from queues.models import Queue, Playlist - - -def song_to_dict(song, include_hash=False, include_user=False, include_replaygain=False, **options): - data = { - "id": song.id, - "artist": song.artist, - "title": song.title, - "duration": song.duration, - } - - if include_hash: - data["hash"] = song.hash - - if include_user is not None and song.user is not None and song.user.name: - data["uploader_name"] = song.user.name - - if include_replaygain: - data["rg_gain"] = song.rg_gain - data["rg_peak"] = song.rg_peak - - return data - - -def playlist_song_to_dict(playlist_song, **options): - user = options.get("user") - return { - "id": playlist_song.id, - "requested_by": "Marietje" if playlist_song.user is None else playlist_song.user.name, - "song": song_to_dict(playlist_song.song, **options), - "can_move_down": playlist_song.user is not None and playlist_song.user == user, - } - - -# Send a file to bertha file storage. -def send_to_bertha(file): - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.connect(settings.BERTHA_HOST) - sock.sendall(struct.pack(" Optional[Queue]: + """Retrieve the Queue for a User.""" + return get_queue_for_user(obj) + + queue__queue.short_description = "queue" + + +admin.site.unregister(User) +admin.site.register(User, UserAdmin) diff --git a/marietje/queues/api/v1/views.py b/marietje/queues/api/v1/views.py index 446cfd2..f0de16b 100644 --- a/marietje/queues/api/v1/views.py +++ b/marietje/queues/api/v1/views.py @@ -1,3 +1,4 @@ +from django.db.models import Q from rest_framework.generics import ListAPIView, RetrieveAPIView, get_object_or_404, CreateAPIView, DestroyAPIView from rest_framework.views import APIView from rest_framework.response import Response @@ -8,7 +9,7 @@ from django.http import Http404 from queues.api.v1.serializers import PlaylistSerializer, QueueSerializer, PlaylistSongSerializer from queues.exceptions import RequestException -from queues.models import Playlist, PlaylistSong, QueueCommand +from queues.models import Playlist, PlaylistSong, QueueCommand, Queue from queues.services import get_user_or_default_queue from songs.counters import request_counter from songs.models import Song @@ -80,7 +81,7 @@ class QueueSkipAPIView(APIView): if queue is None: return Response(status=404) - playlist_song = request.user.queue.current_song() + playlist_song = queue.current_song() if ( request.user is not None and playlist_song.user != request.user @@ -90,6 +91,7 @@ class QueueSkipAPIView(APIView): playlist_song.state = 2 playlist_song.save() + queue.log_action(request.user, "next", "Skipped to next song.") return Response(status=200, data=QueueSerializer(queue).data) @@ -111,7 +113,18 @@ class PlaylistSongMoveDownAPIView(APIView): and not request.user.has_perm("queues.can_move") ): return Response(status=403) + playlist_song.move_down() + + for queue in Queue.objects.filter( + Q(playlist=playlist_song.playlist) | Q(random_playlist=playlist_song.playlist) + ): + queue.log_action( + request.user, + "down", + 'Moved song "{}" of playlist "{}" down.'.format(playlist_song.song, playlist_song.playlist), + ) + return Response(status=200, data=self.serializer_class(playlist_song).data) @@ -131,7 +144,18 @@ class PlaylistSongCancelAPIView(DestroyAPIView): and not request.user.has_perm("queues.can_cancel") ): return Response(status=403) + playlist_song.delete() + + for queue in Queue.objects.filter( + Q(playlist=playlist_song.playlist) | Q(random_playlist=playlist_song.playlist) + ): + queue.log_action( + request.user, + "cancel", + 'Cancelled song "{}" of playlist "{}".'.format(playlist_song.song, playlist_song.playlist), + ) + return Response(status=200, data=self.serializer_class(playlist_song).data) @@ -165,6 +189,8 @@ class QueueRequestAPIView(CreateAPIView): except RequestException as e: return Response(data={"success": False, "errorMessage": str(e)}) + queue.log_action(request.user, "request_song", "Requested song {}.".format(song)) + request_counter.labels(queue=queue.name).inc() return Response(status=200, data=self.serializer_class(playlist_song).data) @@ -196,7 +222,11 @@ class QueueVolumeDownAPIView(APIView): return Response(status=404) if request.user is not None and not request.user.has_perm("queues.can_control_volume"): return Response(status=403) + QueueCommand.objects.create(queue=queue, command="volume_down") + + queue.log_action(request.user, "volume_down", "Reduced the volume of {}.".format(queue)) + return Response(status=200, data=self.serializer_class(queue).data) @@ -227,7 +257,11 @@ class QueueVolumeUpAPIView(APIView): return Response(status=404) if request.user is not None and not request.user.has_perm("queues.can_control_volume"): return Response(status=403) + QueueCommand.objects.create(queue=queue, command="volume_up") + + queue.log_action(request.user, "volume_up", "Increased the volume of {}.".format(queue)) + return Response(status=200, data=self.serializer_class(queue).data) @@ -258,5 +292,9 @@ class QueueMuteAPIView(APIView): return Response(status=404) if request.user is not None and not request.user.has_perm("queues.can_control_volume"): return Response(status=403) + QueueCommand.objects.create(queue=queue, command="mute") + + queue.log_action(request.user, "mute", "Muted the volume of {}.".format(queue)) + return Response(status=200, data=self.serializer_class(queue).data) diff --git a/marietje/queues/migrations/0012_userqueue_queuelogentry.py b/marietje/queues/migrations/0012_userqueue_queuelogentry.py new file mode 100644 index 0000000..ef7e82c --- /dev/null +++ b/marietje/queues/migrations/0012_userqueue_queuelogentry.py @@ -0,0 +1,64 @@ +# Generated by Django 4.2.6 on 2023-11-24 20:17 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("queues", "0011_alter_playlistsong_playlist"), + ] + + operations = [ + migrations.CreateModel( + name="UserQueue", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ( + "queue", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="users", + to="queues.queue", + ), + ), + ( + "user", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + related_name="queue_new", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + migrations.CreateModel( + name="QueueLogEntry", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("action", models.CharField(max_length=255)), + ("timestamp", models.DateTimeField(auto_now_add=True)), + ("description", models.CharField(max_length=255)), + ( + "queue", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="logs", to="queues.queue" + ), + ), + ( + "user", + models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL + ), + ), + ], + options={ + "verbose_name": "player log entry", + "verbose_name_plural": "player log entries", + }, + ), + ] diff --git a/marietje/queues/migrations/0013_alter_userqueue_user.py b/marietje/queues/migrations/0013_alter_userqueue_user.py new file mode 100644 index 0000000..c518d01 --- /dev/null +++ b/marietje/queues/migrations/0013_alter_userqueue_user.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.6 on 2023-11-24 20:19 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("queues", "0012_userqueue_queuelogentry"), + ] + + operations = [ + migrations.AlterField( + model_name="userqueue", + name="user", + field=models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, related_name="queue", to=settings.AUTH_USER_MODEL + ), + ), + ] diff --git a/marietje/queues/models.py b/marietje/queues/models.py index 76d2388..3502b53 100644 --- a/marietje/queues/models.py +++ b/marietje/queues/models.py @@ -1,3 +1,4 @@ +from django.contrib.auth import get_user_model from django.db import models from django.db.models import Q from django.conf import settings @@ -6,6 +7,8 @@ from django.utils import timezone from queues.exceptions import RequestException from songs.models import Song +User = get_user_model() + class Playlist(models.Model): def __str__(self): @@ -143,10 +146,41 @@ class Queue(models.Model): playlist_song.save() song_count += 1 + def log_action(self, user: User, action: str, description: str) -> "QueueLogEntry": + """ + Log a queue action. + + :param user: The user performing the action. + :param action: An identifier of the action performed. + :param description: An optional description for the action. + :return: The created QueueLogEntry object. + """ + return QueueLogEntry.objects.create( + queue=self, + user=user, + action=action, + description=description, + ) + def __str__(self): return str(self.name) +class UserQueue(models.Model): + """ + UserQueue model. + + This model connects a user to its queue. + """ + + user = models.OneToOneField(User, on_delete=models.CASCADE, related_name="queue") + queue = models.ForeignKey(Queue, on_delete=models.SET_NULL, null=True, blank=True, related_name="users") + + def __str__(self): + """Convert this object to string.""" + return "Queue for user {}".format(self.user) + + class QueueCommand(models.Model): queue = models.ForeignKey( Queue, @@ -157,3 +191,20 @@ class QueueCommand(models.Model): def __str__(self): return str(self.command) + + +class QueueLogEntry(models.Model): + """Model for logging queue events.""" + + queue = models.ForeignKey(Queue, on_delete=models.CASCADE, related_name="logs") + user = models.ForeignKey(User, on_delete=models.CASCADE, null=True, blank=True) + action = models.CharField(max_length=255) + timestamp = models.DateTimeField(auto_now_add=True) + description = models.CharField(max_length=255) + + def __str__(self): + return f"{self.queue} {self.action} by {self.user} at {self.timestamp}" + + class Meta: + verbose_name = "player log entry" + verbose_name_plural = "player log entries" diff --git a/marietje/queues/services.py b/marietje/queues/services.py index 6916855..5072a28 100644 --- a/marietje/queues/services.py +++ b/marietje/queues/services.py @@ -1,15 +1,44 @@ -from queues.models import Queue +from typing import Optional + +from django.contrib.auth import get_user_model + +from queues.models import Queue, Playlist from django.conf import settings -def get_user_or_default_queue(request): - """Get the user or default queue.""" +User = get_user_model() + + +def get_user_or_default_queue(request) -> Queue: + """Get the user or default queue from a request.""" if request.user is None: return get_default_queue() else: - return request.user.queue + return get_queue_for_user(request.user) -def get_default_queue(): +def get_queue_for_user(user: User) -> Optional[Queue]: + """Get the queue for a User.""" + if user.queue is None: + return None + else: + return user.queue.queue + + +def get_default_queue() -> Queue: """Get the default queue.""" - return Queue.objects.get(pk=settings.DEFAULT_QUEUE) + try: + return Queue.objects.get(pk=settings.DEFAULT_QUEUE) + except Queue.DoesNotExist: + return get_first_queue() + + +def get_first_queue() -> Queue: + """Get the first Queue object or create one.""" + queue = Queue.objects.first() + if queue is not None: + return queue + + playlist = Playlist.objects.create() + random_playlist = Playlist.objects.create() + return Queue.objects.create(name="Queue", playlist=playlist, random_playlist=random_playlist) diff --git a/marietje/queues/signals.py b/marietje/queues/signals.py new file mode 100644 index 0000000..98b471e --- /dev/null +++ b/marietje/queues/signals.py @@ -0,0 +1,18 @@ +from django.contrib.auth import get_user_model +from django.db.models.signals import post_save +from django.dispatch import receiver + +from queues.models import UserQueue +from queues.services import get_default_queue + +User = get_user_model() + + +@receiver(post_save, sender=User) +def create_default_queue(sender, instance, created, **kwargs): + """Create a UserQueue object when a User gets created.""" + if created: + user_queue, user_queue_created = UserQueue.objects.get_or_create(user=instance) + if user_queue_created: + user_queue.queue = get_default_queue() + user_queue.save() diff --git a/marietje/songs/services.py b/marietje/songs/services.py index a44fa0f..0043487 100644 --- a/marietje/songs/services.py +++ b/marietje/songs/services.py @@ -1,4 +1,9 @@ -from marietje.utils import send_to_bertha +import binascii +import socket +import struct + +from django.conf import settings + from queues.models import PlaylistSong from songs.models import Song from django.db.models.functions import Coalesce @@ -11,6 +16,20 @@ class UploadException(Exception): pass +def send_to_bertha(file): + """Send a file to Berthad file storage.""" + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.connect(settings.BERTHA_HOST) + sock.sendall(struct.pack("", ] maintainers = [ - "Kees van Kempen ", "Lars van Rhijn ", ] readme = "README.md"