From 6dde4b73cdf803b916033d8ac8cb1b94e6391dc8 Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Mon, 8 Jul 2019 15:26:14 +0200 Subject: [PATCH] Implement tag models --- api/config/settings/common.py | 2 +- .../management/commands/load_test_data.py | 267 ++++++++++++++++ .../common/migrations/0003_cit_extension.py | 22 ++ api/funkwhale_api/factories.py | 298 ++++++++++++++++++ api/funkwhale_api/music/factories.py | 25 +- api/funkwhale_api/music/filters.py | 7 + .../music/migrations/0004_track_tags.py | 19 +- .../migrations/0027_auto_20180515_1808.py | 12 - api/funkwhale_api/music/models.py | 24 +- api/funkwhale_api/music/serializers.py | 4 +- api/funkwhale_api/music/views.py | 21 +- api/funkwhale_api/radios/radios.py | 4 +- api/funkwhale_api/tags/__init__.py | 0 api/funkwhale_api/tags/admin.py | 18 ++ api/funkwhale_api/tags/factories.py | 20 ++ .../tags/migrations/0001_initial.py | 85 +++++ api/funkwhale_api/tags/migrations/__init__.py | 0 api/funkwhale_api/tags/models.py | 79 +++++ api/funkwhale_api/users/factories.py | 15 +- api/funkwhale_api/users/models.py | 11 +- api/requirements/base.txt | 2 - api/tests/common/test_commands.py | 104 ++++++ api/tests/conftest.py | 21 -- api/tests/music/test_filters.py | 30 ++ api/tests/music/test_music.py | 27 +- api/tests/radios/test_radios.py | 5 +- api/tests/tags/__init__.py | 0 api/tests/tags/test_models.py | 53 ++++ 28 files changed, 1034 insertions(+), 141 deletions(-) create mode 100644 api/funkwhale_api/common/management/commands/load_test_data.py create mode 100644 api/funkwhale_api/common/migrations/0003_cit_extension.py create mode 100644 api/funkwhale_api/tags/__init__.py create mode 100644 api/funkwhale_api/tags/admin.py create mode 100644 api/funkwhale_api/tags/factories.py create mode 100644 api/funkwhale_api/tags/migrations/0001_initial.py create mode 100644 api/funkwhale_api/tags/migrations/__init__.py create mode 100644 api/funkwhale_api/tags/models.py create mode 100644 api/tests/common/test_commands.py create mode 100644 api/tests/tags/__init__.py create mode 100644 api/tests/tags/test_models.py diff --git a/api/config/settings/common.py b/api/config/settings/common.py index ca7495165c..d0ff2e1883 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -161,7 +161,6 @@ THIRD_PARTY_APPS = ( "oauth2_provider", "rest_framework", "rest_framework.authtoken", - "taggit", "rest_auth", "rest_auth.registration", "dynamic_preferences", @@ -201,6 +200,7 @@ LOCAL_APPS = ( "funkwhale_api.history", "funkwhale_api.playlists", "funkwhale_api.subsonic", + "funkwhale_api.tags", ) # See: https://docs.djangoproject.com/en/dev/ref/settings/#installed-apps diff --git a/api/funkwhale_api/common/management/commands/load_test_data.py b/api/funkwhale_api/common/management/commands/load_test_data.py new file mode 100644 index 0000000000..2c7baeb7d5 --- /dev/null +++ b/api/funkwhale_api/common/management/commands/load_test_data.py @@ -0,0 +1,267 @@ +import math +import random + +from django.conf import settings +from django.core.management.base import BaseCommand +from django.db import transaction + + +from funkwhale_api.federation import keys +from funkwhale_api.federation import models as federation_models +from funkwhale_api.music import models as music_models +from funkwhale_api.tags import models as tags_models +from funkwhale_api.users import models as users_models + + +BATCH_SIZE = 500 + + +def create_local_accounts(factories, count, dependencies): + password = factories["users.User"].build().password + users = factories["users.User"].build_batch(size=count) + for user in users: + # we set the hashed password by hand, because computing one for each user + # is CPU intensive + user.password = password + users = users_models.User.objects.bulk_create(users, batch_size=BATCH_SIZE) + actors = [] + domain = federation_models.Domain.objects.get_or_create( + name=settings.FEDERATION_HOSTNAME + )[0] + users = [u for u in users if u.pk] + private, public = keys.get_key_pair() + for user in users: + if not user.pk: + continue + actor = federation_models.Actor( + private_key=private.decode("utf-8"), + public_key=public.decode("utf-8"), + **users_models.get_actor_data(user.username, domain=domain) + ) + actors.append(actor) + actors = federation_models.Actor.objects.bulk_create(actors, batch_size=BATCH_SIZE) + for user, actor in zip(users, actors): + user.actor = actor + users_models.User.objects.bulk_update(users, ["actor"]) + return actors + + +def create_tagged_tracks(factories, count, dependencies): + + objs = [] + for track in dependencies["tracks"]: + tag = random.choice(dependencies["tags"]) + objs.append(factories["tags.TaggedItem"](content_object=track, tag=tag)) + + return tags_models.TaggedItem.objects.bulk_create( + objs, batch_size=BATCH_SIZE, ignore_conflicts=True + ) + + +CONFIG = [ + { + "id": "tracks", + "model": music_models.Track, + "factory": "music.Track", + "factory_kwargs": {"artist": None, "album": None}, + "depends_on": [ + {"field": "album", "id": "albums", "default_factor": 0.1}, + {"field": "artist", "id": "artists", "default_factor": 0.05}, + ], + }, + { + "id": "albums", + "model": music_models.Album, + "factory": "music.Album", + "factory_kwargs": {"artist": None}, + "depends_on": [{"field": "artist", "id": "artists", "default_factor": 0.3}], + }, + {"id": "artists", "model": music_models.Artist, "factory": "music.Artist"}, + { + "id": "local_accounts", + "model": federation_models.Actor, + "handler": create_local_accounts, + }, + { + "id": "local_libraries", + "model": music_models.Library, + "factory": "music.Library", + "factory_kwargs": {"actor": None}, + "depends_on": [{"field": "actor", "id": "local_accounts", "default_factor": 1}], + }, + { + "id": "local_uploads", + "model": music_models.Upload, + "factory": "music.Upload", + "factory_kwargs": {"import_status": "finished", "library": None, "track": None}, + "depends_on": [ + { + "field": "library", + "id": "local_libraries", + "default_factor": 0.05, + "queryset": music_models.Library.objects.all().select_related( + "actor__user" + ), + }, + {"field": "track", "id": "tracks", "default_factor": 1}, + ], + }, + {"id": "tags", "model": tags_models.Tag, "factory": "tags.Tag"}, + { + "id": "track_tags", + "model": tags_models.TaggedItem, + "handler": create_tagged_tracks, + "depends_on": [ + { + "field": "tag", + "id": "tags", + "default_factor": 0.1, + "queryset": tags_models.Tag.objects.all(), + "set": False, + }, + { + "field": "content_object", + "id": "tracks", + "default_factor": 1, + "set": False, + }, + ], + }, +] + +CONFIG_BY_ID = {c["id"]: c for c in CONFIG} + + +class Rollback(Exception): + pass + + +def create_objects(row, factories, count, **factory_kwargs): + return factories[row["factory"]].build_batch(size=count, **factory_kwargs) + + +class Command(BaseCommand): + help = """ + Inject demo data into your database. Useful for load testing, or setting up a demo instance. + + Use with caution and only if you know what you are doing. + """ + + def add_arguments(self, parser): + parser.add_argument( + "--no-dry-run", + action="store_false", + dest="dry_run", + help="Commit the changes to the database", + ) + parser.add_argument( + "--create-dependencies", action="store_true", dest="create_dependencies" + ) + for row in CONFIG: + parser.add_argument( + "--{}".format(row["id"].replace("_", "-")), + dest=row["id"], + type=int, + help="Number of {} objects to create".format(row["id"]), + ) + dependencies = row.get("depends_on", []) + for dependency in dependencies: + parser.add_argument( + "--{}-{}-factor".format(row["id"], dependency["field"]), + dest="{}_{}_factor".format(row["id"], dependency["field"]), + type=float, + help="Number of {} objects to create per {} object".format( + dependency["id"], row["id"] + ), + ) + + def handle(self, *args, **options): + from django.apps import apps + from funkwhale_api import factories + + app_names = [app.name for app in apps.app_configs.values()] + factories.registry.autodiscover(app_names) + try: + return self.inner_handle(*args, **options) + except Rollback: + pass + + @transaction.atomic + def inner_handle(self, *args, **options): + results = {} + for row in CONFIG: + self.create_batch(row, results, options, count=options.get(row["id"])) + + self.stdout.write("\nFinal state of database:\n\n") + for row in CONFIG: + model = row["model"] + total = model.objects.all().count() + self.stdout.write("- {} {} objects".format(total, row["id"])) + + self.stdout.write("") + if options["dry_run"]: + + self.stdout.write( + "Run this command with --no-dry-run to commit the changes to the database" + ) + raise Rollback() + + self.stdout.write(self.style.SUCCESS("Done!")) + + def create_batch(self, row, results, options, count): + from funkwhale_api import factories + + if row["id"] in results: + # already generated + return results[row["id"]] + if not count: + return [] + dependencies = row.get("depends_on", []) + dependencies_results = {} + create_dependencies = options.get("create_dependencies") + for dependency in dependencies: + if not create_dependencies: + continue + dep_count = options.get(dependency["id"]) + if dep_count is None: + factor = options[ + "{}_{}_factor".format(row["id"], dependency["field"]) + ] or dependency.get("default_factor") + dep_count = math.ceil(factor * count) + + dependencies_results[dependency["id"]] = self.create_batch( + CONFIG_BY_ID[dependency["id"]], results, options, count=dep_count + ) + self.stdout.write("Creating {} {}…".format(count, row["id"])) + handler = row.get("handler") + if handler: + objects = handler( + factories.registry, count, dependencies=dependencies_results + ) + else: + objects = create_objects( + row, factories.registry, count, **row.get("factory_kwargs", {}) + ) + for dependency in dependencies: + if not dependency.get("set", True): + continue + if create_dependencies: + candidates = dependencies_results[dependency["id"]] + else: + # we use existing objects in the database + queryset = dependency.get( + "queryset", CONFIG_BY_ID[dependency["id"]]["model"].objects.all() + ) + candidates = list(queryset.values_list("pk", flat=True)) + picked_pks = [random.choice(candidates) for _ in objects] + picked_objects = {o.pk: o for o in queryset.filter(pk__in=picked_pks)} + for i, obj in enumerate(objects): + if create_dependencies: + value = random.choice(candidates) + else: + value = picked_objects[picked_pks[i]] + setattr(obj, dependency["field"], value) + if not handler: + objects = row["model"].objects.bulk_create(objects, batch_size=BATCH_SIZE) + results[row["id"]] = objects + return objects diff --git a/api/funkwhale_api/common/migrations/0003_cit_extension.py b/api/funkwhale_api/common/migrations/0003_cit_extension.py new file mode 100644 index 0000000000..94ca96c49a --- /dev/null +++ b/api/funkwhale_api/common/migrations/0003_cit_extension.py @@ -0,0 +1,22 @@ +# Generated by Django 2.0.2 on 2018-02-27 18:43 +from django.db import migrations +from django.contrib.postgres.operations import CITextExtension + + +class CustomCITExtension(CITextExtension): + def database_forwards(self, app_label, schema_editor, from_state, to_state): + check_sql = "SELECT 1 FROM pg_extension WHERE extname = 'citext'" + with schema_editor.connection.cursor() as cursor: + cursor.execute(check_sql) + result = cursor.fetchall() + + if result: + return + return super().database_forwards(app_label, schema_editor, from_state, to_state) + + +class Migration(migrations.Migration): + + dependencies = [("common", "0002_mutation")] + + operations = [CustomCITExtension()] diff --git a/api/funkwhale_api/factories.py b/api/funkwhale_api/factories.py index 3517ea0079..f75f327f44 100644 --- a/api/funkwhale_api/factories.py +++ b/api/funkwhale_api/factories.py @@ -1,5 +1,6 @@ import uuid import factory +import random import persisting_theory from django.conf import settings @@ -46,6 +47,268 @@ class NoUpdateOnCreate: return +TAGS_DATA = { + "type": [ + "acoustic", + "acid", + "ambient", + "alternative", + "brutalist", + "chill", + "club", + "cold", + "cool", + "contemporary", + "dark", + "doom", + "electro", + "folk", + "freestyle", + "fusion", + "garage", + "glitch", + "hard", + "healing", + "industrial", + "instrumental", + "hardcore", + "holiday", + "hot", + "liquid", + "modern", + "minimalist", + "new", + "parody", + "postmodern", + "progressive", + "smooth", + "symphonic", + "traditional", + "tribal", + "metal", + ], + "genre": [ + "blues", + "classical", + "chiptune", + "dance", + "disco", + "funk", + "jazz", + "house", + "hiphop", + "NewAge", + "pop", + "punk", + "rap", + "RnB", + "reggae", + "rock", + "soul", + "soundtrack", + "ska", + "swing", + "trance", + ], + "nationality": [ + "Afghan", + "Albanian", + "Algerian", + "American", + "Andorran", + "Angolan", + "Antiguans", + "Argentinean", + "Armenian", + "Australian", + "Austrian", + "Azerbaijani", + "Bahamian", + "Bahraini", + "Bangladeshi", + "Barbadian", + "Barbudans", + "Batswana", + "Belarusian", + "Belgian", + "Belizean", + "Beninese", + "Bhutanese", + "Bolivian", + "Bosnian", + "Brazilian", + "British", + "Bruneian", + "Bulgarian", + "Burkinabe", + "Burmese", + "Burundian", + "Cambodian", + "Cameroonian", + "Canadian", + "Cape Verdean", + "Central African", + "Chadian", + "Chilean", + "Chinese", + "Colombian", + "Comoran", + "Congolese", + "Costa Rican", + "Croatian", + "Cuban", + "Cypriot", + "Czech", + "Danish", + "Djibouti", + "Dominican", + "Dutch", + "East Timorese", + "Ecuadorean", + "Egyptian", + "Emirian", + "Equatorial Guinean", + "Eritrean", + "Estonian", + "Ethiopian", + "Fijian", + "Filipino", + "Finnish", + "French", + "Gabonese", + "Gambian", + "Georgian", + "German", + "Ghanaian", + "Greek", + "Grenadian", + "Guatemalan", + "Guinea-Bissauan", + "Guinean", + "Guyanese", + "Haitian", + "Herzegovinian", + "Honduran", + "Hungarian", + "I-Kiribati", + "Icelander", + "Indian", + "Indonesian", + "Iranian", + "Iraqi", + "Irish", + "Israeli", + "Italian", + "Ivorian", + "Jamaican", + "Japanese", + "Jordanian", + "Kazakhstani", + "Kenyan", + "Kittian and Nevisian", + "Kuwaiti", + "Kyrgyz", + "Laotian", + "Latvian", + "Lebanese", + "Liberian", + "Libyan", + "Liechtensteiner", + "Lithuanian", + "Luxembourger", + "Macedonian", + "Malagasy", + "Malawian", + "Malaysian", + "Maldivian", + "Malian", + "Maltese", + "Marshallese", + "Mauritanian", + "Mauritian", + "Mexican", + "Micronesian", + "Moldovan", + "Monacan", + "Mongolian", + "Moroccan", + "Mosotho", + "Motswana", + "Mozambican", + "Namibian", + "Nauruan", + "Nepalese", + "New Zealander", + "Ni-Vanuatu", + "Nicaraguan", + "Nigerian", + "Nigerien", + "North Korean", + "Northern Irish", + "Norwegian", + "Omani", + "Pakistani", + "Palauan", + "Panamanian", + "Papua New Guinean", + "Paraguayan", + "Peruvian", + "Polish", + "Portuguese", + "Qatari", + "Romanian", + "Russian", + "Rwandan", + "Saint Lucian", + "Salvadoran", + "Samoan", + "San Marinese", + "Sao Tomean", + "Saudi", + "Scottish", + "Senegalese", + "Serbian", + "Seychellois", + "Sierra Leonean", + "Singaporean", + "Slovakian", + "Slovenian", + "Solomon Islander", + "Somali", + "South African", + "South Korean", + "Spanish", + "Sri Lankan", + "Sudanese", + "Surinamer", + "Swazi", + "Swedish", + "Swiss", + "Syrian", + "Taiwanese", + "Tajik", + "Tanzanian", + "Thai", + "Togolese", + "Tongan", + "Trinidadian", + "Tunisian", + "Turkish", + "Tuvaluan", + "Ugandan", + "Ukrainian", + "Uruguayan", + "Uzbekistani", + "Venezuelan", + "Vietnamese", + "Welsh", + "Yemenite", + "Zambian", + "Zimbabwean", + ], +} + + class FunkwhaleProvider(internet_provider.Provider): """ Our own faker data generator, since built-in ones are sometimes @@ -61,5 +324,40 @@ class FunkwhaleProvider(internet_provider.Provider): path = path_generator() return "{}://{}/{}".format(protocol, domain, path) + def user_name(self): + u = super().user_name() + return "{}{}".format(u, random.randint(10, 999)) + + def music_genre(self): + return random.choice(TAGS_DATA["genre"]) + + def music_type(self): + return random.choice(TAGS_DATA["type"]) + + def music_nationality(self): + return random.choice(TAGS_DATA["nationality"]) + + def music_hashtag(self, prefix_length=4): + genre = self.music_genre() + prefixes = [genre] + nationality = False + while len(prefixes) < prefix_length: + if nationality: + t = "type" + else: + t = random.choice(["type", "nationality", "genre"]) + + if t == "nationality": + nationality = True + + choice = random.choice(TAGS_DATA[t]) + if choice in prefixes: + continue + prefixes.append(choice) + + return "".join( + [p.capitalize().strip().replace(" ", "") for p in reversed(prefixes)] + ) + factory.Faker.add_provider(FunkwhaleProvider) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 430c824398..8b49808b7f 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -2,10 +2,11 @@ import os import factory -from funkwhale_api.factories import ManyToManyFromList, registry, NoUpdateOnCreate +from funkwhale_api.factories import registry, NoUpdateOnCreate from funkwhale_api.federation import factories as federation_factories from funkwhale_api.music import licenses +from funkwhale_api.tags import models as tags_models from funkwhale_api.users import factories as users_factories SAMPLES_PATH = os.path.join( @@ -103,7 +104,6 @@ class TrackFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): album = factory.SubFactory(AlbumFactory) artist = factory.SelfAttribute("album.artist") position = 1 - tags = ManyToManyFromList("tags") playable = playable_factory("track") class Meta: @@ -127,6 +127,15 @@ class TrackFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): self.license = LicenseFactory(code=extracted) self.save() + @factory.post_generation + def set_tags(self, create, extracted, **kwargs): + if not create: + # Simple build, do nothing. + return + + if extracted: + tags_models.set_tags(self, *extracted) + @registry.register class UploadFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): @@ -164,18 +173,6 @@ class UploadVersionFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): model = "music.UploadVersion" -@registry.register -class TagFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): - name = factory.SelfAttribute("slug") - slug = factory.Faker("slug") - - class Meta: - model = "taggit.Tag" - - -# XXX To remove - - class ImportBatchFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): submitted_by = factory.SubFactory(users_factories.UserFactory) diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index fa5a10f6d4..d8e68d7314 100644 --- a/api/funkwhale_api/music/filters.py +++ b/api/funkwhale_api/music/filters.py @@ -29,6 +29,7 @@ class ArtistFilter(moderation_filters.HiddenContentFilterSet): class TrackFilter(moderation_filters.HiddenContentFilterSet): q = fields.SearchFilter(search_fields=["title", "album__title", "artist__name"]) playable = filters.BooleanFilter(field_name="_", method="filter_playable") + tag = common_filters.MultipleQueryFilter(method="filter_tags") id = common_filters.MultipleQueryFilter(coerce=int) class Meta: @@ -47,6 +48,12 @@ class TrackFilter(moderation_filters.HiddenContentFilterSet): actor = utils.get_actor_from_request(self.request) return queryset.playable_by(actor, value) + def filter_tags(self, queryset, name, value): + non_empty_tags = [v.lower() for v in value if v] + for tag in non_empty_tags: + queryset = queryset.filter(tagged_items__tag__name=tag).distinct() + return queryset + class UploadFilter(filters.FilterSet): library = filters.CharFilter("library__uuid") diff --git a/api/funkwhale_api/music/migrations/0004_track_tags.py b/api/funkwhale_api/music/migrations/0004_track_tags.py index b999a70313..6fc627b4af 100644 --- a/api/funkwhale_api/music/migrations/0004_track_tags.py +++ b/api/funkwhale_api/music/migrations/0004_track_tags.py @@ -2,25 +2,10 @@ from __future__ import unicode_literals from django.db import migrations -import taggit.managers class Migration(migrations.Migration): - dependencies = [ - ("taggit", "0002_auto_20150616_2121"), - ("music", "0003_auto_20151222_2233"), - ] + dependencies = [("music", "0003_auto_20151222_2233")] - operations = [ - migrations.AddField( - model_name="track", - name="tags", - field=taggit.managers.TaggableManager( - verbose_name="Tags", - help_text="A comma-separated list of tags.", - through="taggit.TaggedItem", - to="taggit.Tag", - ), - ) - ] + operations = [] diff --git a/api/funkwhale_api/music/migrations/0027_auto_20180515_1808.py b/api/funkwhale_api/music/migrations/0027_auto_20180515_1808.py index 1e3949da48..0f01509ad1 100644 --- a/api/funkwhale_api/music/migrations/0027_auto_20180515_1808.py +++ b/api/funkwhale_api/music/migrations/0027_auto_20180515_1808.py @@ -1,7 +1,6 @@ # Generated by Django 2.0.3 on 2018-05-15 18:08 from django.db import migrations, models -import taggit.managers class Migration(migrations.Migration): @@ -19,15 +18,4 @@ class Migration(migrations.Migration): name="size", field=models.IntegerField(blank=True, null=True), ), - migrations.AlterField( - model_name="track", - name="tags", - field=taggit.managers.TaggableManager( - blank=True, - help_text="A comma-separated list of tags.", - through="taggit.TaggedItem", - to="taggit.Tag", - verbose_name="Tags", - ), - ), ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index cf6a0b7ba3..91fbb1b175 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -9,6 +9,7 @@ import uuid import pendulum import pydub from django.conf import settings +from django.contrib.contenttypes.fields import GenericRelation from django.contrib.postgres.fields import JSONField from django.core.files.base import ContentFile from django.core.serializers.json import DjangoJSONEncoder @@ -17,7 +18,6 @@ from django.db.models.signals import post_save from django.dispatch import receiver from django.urls import reverse from django.utils import timezone -from taggit.managers import TaggableManager from versatileimagefield.fields import VersatileImageField from versatileimagefield.image_warmer import VersatileImageFieldWarmer @@ -29,6 +29,7 @@ from funkwhale_api.common import session from funkwhale_api.common import utils as common_utils from funkwhale_api.federation import models as federation_models from funkwhale_api.federation import utils as federation_utils +from funkwhale_api.tags import models as tags_models from . import importers, metadata, utils logger = logging.getLogger(__name__) @@ -206,14 +207,6 @@ class Artist(APIModelMixin): def __str__(self): return self.name - @property - def tags(self): - t = [] - for album in self.albums.all(): - for tag in album.tags: - t.append(tag) - return set(t) - @classmethod def get_or_create_from_name(cls, name, **kwargs): kwargs.update({"name": name}) @@ -356,14 +349,6 @@ class Album(APIModelMixin): # external storage return self.cover.name - @property - def tags(self): - t = [] - for track in self.tracks.all(): - for tag in track.tags.all(): - t.append(tag) - return set(t) - @classmethod def get_or_create_from_title(cls, title, **kwargs): kwargs.update({"title": title}) @@ -380,7 +365,8 @@ def import_tags(instance, cleaned_data, raw_data): except ValueError: continue tags_to_add.append(tag_data["name"]) - instance.tags.add(*tags_to_add) + + tags_models.add_tags(instance, *tags_to_add) def import_album(v): @@ -472,7 +458,7 @@ class Track(APIModelMixin): } import_hooks = [import_tags] objects = TrackQuerySet.as_manager() - tags = TaggableManager(blank=True) + tagged_items = GenericRelation(tags_models.TaggedItem) class Meta: ordering = ["album", "disc_number", "position"] diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 867d15d8dd..6fd95f3482 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -4,7 +4,6 @@ from django.db import transaction from django import urls from django.conf import settings from rest_framework import serializers -from taggit.models import Tag from versatileimagefield.serializers import VersatileImageFieldSerializer from funkwhale_api.activity import serializers as activity_serializers @@ -12,6 +11,7 @@ from funkwhale_api.common import serializers as common_serializers from funkwhale_api.common import utils as common_utils from funkwhale_api.federation import routes from funkwhale_api.federation import utils as federation_utils +from funkwhale_api.tags.models import Tag from . import filters, models, tasks @@ -361,7 +361,7 @@ class UploadActionSerializer(common_serializers.ActionSerializer): class TagSerializer(serializers.ModelSerializer): class Meta: model = Tag - fields = ("id", "name", "slug") + fields = ("id", "name", "creation_date") class SimpleAlbumSerializer(serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 4f8f35ced2..9aa7924809 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -12,7 +12,6 @@ from rest_framework import settings as rest_settings from rest_framework import views, viewsets from rest_framework.decorators import action from rest_framework.response import Response -from taggit.models import Tag from funkwhale_api.common import decorators as common_decorators from funkwhale_api.common import permissions as common_permissions @@ -24,6 +23,7 @@ from funkwhale_api.federation import actors from funkwhale_api.federation import api_serializers as federation_api_serializers from funkwhale_api.federation import decorators as federation_decorators from funkwhale_api.federation import routes +from funkwhale_api.tags.models import Tag from funkwhale_api.users.oauth import permissions as oauth_permissions from . import filters, licenses, models, serializers, tasks, utils @@ -53,15 +53,6 @@ def get_libraries(filter_uploads): return libraries -class TagViewSetMixin(object): - def get_queryset(self): - queryset = super().get_queryset() - tag = self.request.query_params.get("tag") - if tag: - queryset = queryset.filter(tags__pk=tag) - return queryset - - class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet): queryset = models.Artist.objects.all() serializer_class = serializers.ArtistWithAlbumsSerializer @@ -182,9 +173,7 @@ class LibraryViewSet( return Response(serializer.data) -class TrackViewSet( - common_views.SkipFilterForGetObject, TagViewSetMixin, viewsets.ReadOnlyModelViewSet -): +class TrackViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet): """ A simple ViewSet for viewing and editing accounts. """ @@ -521,14 +510,14 @@ class Search(views.APIView): return common_utils.order_for_search(qs, "name")[: self.max_results] def get_tags(self, query): - search_fields = ["slug", "name__unaccent"] + search_fields = ["name__unaccent"] query_obj = utils.get_query(query, search_fields) # We want the shortest tag first qs = ( Tag.objects.all() - .annotate(slug_length=Length("slug")) - .order_by("slug_length") + .annotate(name_length=Length("name")) + .order_by("name_length") ) return qs.filter(query_obj)[: self.max_results] diff --git a/api/funkwhale_api/radios/radios.py b/api/funkwhale_api/radios/radios.py index a0dc36b62a..8940cdc152 100644 --- a/api/funkwhale_api/radios/radios.py +++ b/api/funkwhale_api/radios/radios.py @@ -3,10 +3,10 @@ import random from django.core.exceptions import ValidationError from django.db import connection from rest_framework import serializers -from taggit.models import Tag from funkwhale_api.moderation import filters as moderation_filters from funkwhale_api.music.models import Artist, Track +from funkwhale_api.tags.models import Tag from funkwhale_api.users.models import User from . import filters, models @@ -165,7 +165,7 @@ class TagRadio(RelatedObjectRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) - return qs.filter(tags__in=[self.session.related_object]) + return qs.filter(tagged_items__tag=self.session.related_object) def weighted_choice(choices): diff --git a/api/funkwhale_api/tags/__init__.py b/api/funkwhale_api/tags/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/api/funkwhale_api/tags/admin.py b/api/funkwhale_api/tags/admin.py new file mode 100644 index 0000000000..bead393314 --- /dev/null +++ b/api/funkwhale_api/tags/admin.py @@ -0,0 +1,18 @@ +from funkwhale_api.common import admin + +from . import models + + +@admin.register(models.Tag) +class TagAdmin(admin.ModelAdmin): + list_display = ["name", "creation_date"] + search_fields = ["name"] + list_select_related = True + + +@admin.register(models.TaggedItem) +class TaggedItemAdmin(admin.ModelAdmin): + list_display = ["object_id", "content_type", "tag", "creation_date"] + search_fields = ["tag__name"] + list_filter = ["content_type"] + list_select_related = True diff --git a/api/funkwhale_api/tags/factories.py b/api/funkwhale_api/tags/factories.py new file mode 100644 index 0000000000..9a9d2f216f --- /dev/null +++ b/api/funkwhale_api/tags/factories.py @@ -0,0 +1,20 @@ +import factory + +from funkwhale_api.factories import registry, NoUpdateOnCreate + + +@registry.register +class TagFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): + name = factory.Faker("music_hashtag") + + class Meta: + model = "tags.Tag" + + +@registry.register +class TaggedItemFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): + tag = factory.SubFactory(TagFactory) + content_object = None + + class Meta: + model = "tags.TaggedItem" diff --git a/api/funkwhale_api/tags/migrations/0001_initial.py b/api/funkwhale_api/tags/migrations/0001_initial.py new file mode 100644 index 0000000000..93b8bfbd9a --- /dev/null +++ b/api/funkwhale_api/tags/migrations/0001_initial.py @@ -0,0 +1,85 @@ +# Generated by Django 2.2.3 on 2019-07-05 08:22 + +import django.contrib.postgres.fields.citext +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("contenttypes", "0002_remove_content_type_name"), + ("common", "0003_cit_extension"), + ] + + operations = [ + migrations.CreateModel( + name="Tag", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "name", + django.contrib.postgres.fields.citext.CICharField( + max_length=100, unique=True + ), + ), + ( + "creation_date", + models.DateTimeField(default=django.utils.timezone.now), + ), + ], + ), + migrations.CreateModel( + name="TaggedItem", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "creation_date", + models.DateTimeField(default=django.utils.timezone.now), + ), + ( + "object_id", + models.IntegerField(db_index=True, verbose_name="Object id"), + ), + ( + "content_type", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="tags_taggeditem_tagged_items", + to="contenttypes.ContentType", + verbose_name="Content type", + ), + ), + ( + "tag", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="tags_taggeditem_items", + to="tags.Tag", + ), + ), + ], + ), + migrations.AlterUniqueTogether( + name="taggeditem", unique_together={("tag", "content_type", "object_id")} + ), + ] diff --git a/api/funkwhale_api/tags/migrations/__init__.py b/api/funkwhale_api/tags/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/api/funkwhale_api/tags/models.py b/api/funkwhale_api/tags/models.py new file mode 100644 index 0000000000..ef3178b2b9 --- /dev/null +++ b/api/funkwhale_api/tags/models.py @@ -0,0 +1,79 @@ +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType +from django.contrib.postgres.fields import CICharField +from django.db import models +from django.db import transaction + +from django.utils import timezone +from django.utils.translation import gettext_lazy as _ + + +class Tag(models.Model): + name = CICharField(max_length=100, unique=True) + creation_date = models.DateTimeField(default=timezone.now) + + def __str__(self): + return self.name + + +class TaggedItemQuerySet(models.QuerySet): + def for_content_object(self, obj): + return self.filter( + object_id=obj.id, + content_type__app_label=obj._meta.app_label, + content_type__model=obj._meta.model_name, + ) + + +class TaggedItem(models.Model): + creation_date = models.DateTimeField(default=timezone.now) + tag = models.ForeignKey( + Tag, related_name="%(app_label)s_%(class)s_items", on_delete=models.CASCADE + ) + content_type = models.ForeignKey( + ContentType, + on_delete=models.CASCADE, + verbose_name=_("Content type"), + related_name="%(app_label)s_%(class)s_tagged_items", + ) + object_id = models.IntegerField(verbose_name=_("Object id"), db_index=True) + content_object = GenericForeignKey() + + objects = TaggedItemQuerySet.as_manager() + + class Meta: + unique_together = ("tag", "content_type", "object_id") + + +@transaction.atomic +def add_tags(obj, *tags): + if not tags: + return + tag_objs = [Tag(name=t) for t in tags] + Tag.objects.bulk_create(tag_objs, ignore_conflicts=True) + tag_ids = Tag.objects.filter(name__in=tags).values_list("id", flat=True) + + tagged_items = [TaggedItem(tag_id=tag_id, content_object=obj) for tag_id in tag_ids] + + TaggedItem.objects.bulk_create(tagged_items, ignore_conflicts=True) + + +@transaction.atomic +def set_tags(obj, *tags): + tags = set(tags) + existing = set( + TaggedItem.objects.for_content_object(obj).values_list("tag__name", flat=True) + ) + found = tags & existing + to_add = tags - found + to_remove = existing - (found | to_add) + + add_tags(obj, *to_add) + remove_tags(obj, *to_remove) + + +@transaction.atomic +def remove_tags(obj, *tags): + if not tags: + return + TaggedItem.objects.for_content_object(obj).filter(tag__name__in=tags).delete() diff --git a/api/funkwhale_api/users/factories.py b/api/funkwhale_api/users/factories.py index e7f046ef3f..0588fa07c9 100644 --- a/api/funkwhale_api/users/factories.py +++ b/api/funkwhale_api/users/factories.py @@ -42,11 +42,20 @@ class InvitationFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): expired = factory.Trait(expiration_date=factory.LazyFunction(timezone.now)) +class PasswordSetter(factory.PostGenerationMethodCall): + def call(self, instance, step, context): + if context.value_provided and context.value is None: + # disable setting the password, it's set by hand outside of the factory + return + + return super().call(instance, step, context) + + @registry.register class UserFactory(factory.django.DjangoModelFactory): - username = factory.Sequence(lambda n: "user-{0}".format(n)) - email = factory.Sequence(lambda n: "user-{0}@example.com".format(n)) - password = factory.PostGenerationMethodCall("set_password", "test") + username = factory.Faker("user_name") + email = factory.Faker("email") + password = password = PasswordSetter("set_password", "test") subsonic_api_token = None groups = ManyToManyFromList("groups") avatar = factory.django.ImageField() diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index 3748dd634b..1e95aac2a4 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -321,13 +321,16 @@ class RefreshToken(oauth2_models.AbstractRefreshToken): pass -def get_actor_data(username): +def get_actor_data(username, **kwargs): slugified_username = federation_utils.slugify_username(username) + domain = kwargs.get("domain") + if not domain: + domain = federation_models.Domain.objects.get_or_create( + name=settings.FEDERATION_HOSTNAME + )[0] return { "preferred_username": slugified_username, - "domain": federation_models.Domain.objects.get_or_create( - name=settings.FEDERATION_HOSTNAME - )[0], + "domain": domain, "type": "Person", "name": username, "manually_approves_followers": False, diff --git a/api/requirements/base.txt b/api/requirements/base.txt index 963f368c5c..be7dd88b87 100644 --- a/api/requirements/base.txt +++ b/api/requirements/base.txt @@ -39,8 +39,6 @@ django-rest-auth>=0.9,<0.10 ipython>=7,<8 mutagen>=1.42,<1.43 - -django-taggit>=0.24,<0.25 pymemoize==1.0.3 django-dynamic-preferences>=1.7,<1.8 diff --git a/api/tests/common/test_commands.py b/api/tests/common/test_commands.py new file mode 100644 index 0000000000..e2755256e8 --- /dev/null +++ b/api/tests/common/test_commands.py @@ -0,0 +1,104 @@ +import pytest + +from django.core.management import call_command + +from funkwhale_api.federation import models as federation_models +from funkwhale_api.music import models as music_models +from funkwhale_api.tags import models as tags_models +from funkwhale_api.users import models as users_models + + +def test_load_test_data_dry_run(factories, mocker): + call_command("load_test_data", artists=10) + + assert music_models.Artist.objects.count() == 0 + + +@pytest.mark.parametrize( + "kwargs, expected_counts", + [ + ( + {"create_dependencies": True, "artists": 10}, + [(music_models.Artist.objects.all(), 10)], + ), + ( + {"create_dependencies": True, "albums": 10, "artists": 1}, + [ + (music_models.Album.objects.all(), 10), + (music_models.Artist.objects.all(), 1), + ], + ), + ( + {"create_dependencies": True, "tracks": 20, "albums": 10, "artists": 1}, + [ + (music_models.Track.objects.all(), 20), + (music_models.Album.objects.all(), 10), + (music_models.Artist.objects.all(), 1), + ], + ), + ( + {"create_dependencies": True, "albums": 10, "albums_artist_factor": 0.5}, + [ + (music_models.Album.objects.all(), 10), + (music_models.Artist.objects.all(), 5), + ], + ), + ( + {"create_dependencies": True, "albums": 3}, + [ + (music_models.Album.objects.all(), 3), + (music_models.Artist.objects.all(), 1), + ], + ), + ( + {"create_dependencies": True, "local_accounts": 3}, + [ + (users_models.User.objects.all(), 3), + (federation_models.Actor.objects.all(), 3), + ], + ), + ( + {"create_dependencies": True, "local_libraries": 3}, + [ + (users_models.User.objects.all(), 3), + (federation_models.Actor.objects.all(), 3), + (music_models.Library.objects.all(), 3), + ], + ), + ( + {"create_dependencies": True, "local_uploads": 3}, + [ + (users_models.User.objects.all(), 1), + (federation_models.Actor.objects.all(), 1), + (music_models.Library.objects.all(), 1), + (music_models.Upload.objects.filter(import_status="finished"), 3), + (music_models.Track.objects.all(), 3), + ], + ), + ( + {"create_dependencies": True, "tags": 3}, + [(tags_models.Tag.objects.all(), 3)], + ), + ( + {"create_dependencies": True, "track_tags": 3}, + [ + (tags_models.Tag.objects.all(), 1), + (tags_models.TaggedItem.objects.all(), 3), + (music_models.Track.objects.all(), 3), + ], + ), + ], +) +def test_load_test_data_args(factories, kwargs, expected_counts, mocker): + call_command("load_test_data", dry_run=False, **kwargs) + + for qs, expected_count in expected_counts: + assert qs.count() == expected_count + + +def test_load_test_data_skip_dependencies(factories): + factories["music.Artist"].create_batch(size=5) + call_command("load_test_data", dry_run=False, albums=10, create_dependencies=False) + + assert music_models.Artist.objects.count() == 5 + assert music_models.Album.objects.count() == 10 diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 286c147945..d76b1665ed 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -6,9 +6,7 @@ import PIL import random import shutil import tempfile -import uuid -from faker.providers import internet as internet_provider import factory import pytest @@ -36,25 +34,6 @@ from funkwhale_api.music import licenses pytest_plugins = "aiohttp.pytest_plugin" -class FunkwhaleProvider(internet_provider.Provider): - """ - Our own faker data generator, since built-in ones are sometimes - not random enough - """ - - def federation_url(self, prefix=""): - def path_generator(): - return "{}/{}".format(prefix, uuid.uuid4()) - - domain = self.domain_name() - protocol = "https" - path = path_generator() - return "{}://{}/{}".format(protocol, domain, path) - - -factory.Faker.add_provider(FunkwhaleProvider) - - @pytest.fixture def queryset_equal_queries(): """ diff --git a/api/tests/music/test_filters.py b/api/tests/music/test_filters.py index f9abc4b215..0076f3c93e 100644 --- a/api/tests/music/test_filters.py +++ b/api/tests/music/test_filters.py @@ -52,3 +52,33 @@ def test_artist_filter_track_album_artist(factories, mocker, queryset_equal_list ) assert filterset.qs == [hidden_track] + + +def test_track_filter_tag_single( + factories, mocker, queryset_equal_list, anonymous_user +): + factories["music.Track"]() + # tag name partially match the query, so this shouldn't match + factories["music.Track"](set_tags=["TestTag1"]) + tagged = factories["music.Track"](set_tags=["TestTag"]) + qs = models.Track.objects.all() + filterset = filters.TrackFilter( + {"tag": "testTaG"}, request=mocker.Mock(user=anonymous_user), queryset=qs + ) + + assert filterset.qs == [tagged] + + +def test_track_filter_tag_multiple( + factories, mocker, queryset_equal_list, anonymous_user +): + factories["music.Track"](set_tags=["TestTag1"]) + tagged = factories["music.Track"](set_tags=["TestTag1", "TestTag2"]) + qs = models.Track.objects.all() + filterset = filters.TrackFilter( + {"tag": ["testTaG1", "TestTag2"]}, + request=mocker.Mock(user=anonymous_user), + queryset=qs, + ) + + assert filterset.qs == [tagged] diff --git a/api/tests/music/test_music.py b/api/tests/music/test_music.py index 10d281071a..f20a28f3ae 100644 --- a/api/tests/music/test_music.py +++ b/api/tests/music/test_music.py @@ -91,7 +91,7 @@ def test_can_create_track_from_api_with_corresponding_tags( ) track = models.Track.create_from_api(id="9968a9d6-8d92-4051-8f76-674e157b6eed") expected_tags = ["techno", "good-music"] - track_tags = [tag.slug for tag in track.tags.all()] + track_tags = track.tagged_items.values_list("tag__name", flat=True) for tag in expected_tags: assert tag in track_tags @@ -123,31 +123,6 @@ def test_can_get_or_create_track_from_api(artists, albums, tracks, mocker, db): assert track == track2 -def test_album_tags_deduced_from_tracks_tags(factories, django_assert_num_queries): - tag = factories["taggit.Tag"]() - album = factories["music.Album"]() - factories["music.Track"].create_batch(5, album=album, tags=[tag]) - - album = models.Album.objects.prefetch_related("tracks__tags").get(pk=album.pk) - - with django_assert_num_queries(0): - assert tag in album.tags - - -def test_artist_tags_deduced_from_album_tags(factories, django_assert_num_queries): - tag = factories["taggit.Tag"]() - album = factories["music.Album"]() - artist = album.artist - factories["music.Track"].create_batch(5, album=album, tags=[tag]) - - artist = models.Artist.objects.prefetch_related("albums__tracks__tags").get( - pk=artist.pk - ) - - with django_assert_num_queries(0): - assert tag in artist.tags - - def test_can_download_image_file_for_album(binary_cover, mocker, factories): mocker.patch( "funkwhale_api.musicbrainz.api.images.get_front", return_value=binary_cover diff --git a/api/tests/radios/test_radios.py b/api/tests/radios/test_radios.py index 640e712117..36f76e1c1f 100644 --- a/api/tests/radios/test_radios.py +++ b/api/tests/radios/test_radios.py @@ -197,14 +197,15 @@ def test_can_start_artist_radio(factories): def test_can_start_tag_radio(factories): user = factories["users.User"]() - tag = factories["taggit.Tag"]() factories["music.Upload"].create_batch(5) - good_files = factories["music.Upload"].create_batch(5, track__tags=[tag]) + tag = factories["tags.Tag"]() + good_files = factories["music.Upload"].create_batch(5, track__set_tags=[tag]) good_tracks = [f.track for f in good_files] radio = radios.TagRadio() session = radio.start_session(user, related_object=tag) assert session.radio_type == "tag" + for i in range(5): assert radio.pick(filter_playable=False) in good_tracks diff --git a/api/tests/tags/__init__.py b/api/tests/tags/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/api/tests/tags/test_models.py b/api/tests/tags/test_models.py new file mode 100644 index 0000000000..1a3a075fce --- /dev/null +++ b/api/tests/tags/test_models.py @@ -0,0 +1,53 @@ +import pytest + +from funkwhale_api.tags import models + + +@pytest.mark.parametrize( + "existing, given, expected", + [ + ([], ["tag1"], ["tag1"]), + (["tag1"], ["tag1"], ["tag1"]), + (["tag1"], ["tag2"], ["tag1", "tag2"]), + (["tag1"], ["tag2", "tag3"], ["tag1", "tag2", "tag3"]), + ], +) +def test_add_tags(factories, existing, given, expected): + obj = factories["music.Artist"]() + for tag in existing: + factories["tags.TaggedItem"](content_object=obj, tag__name=tag) + + models.add_tags(obj, *given) + + tagged_items = models.TaggedItem.objects.all() + + assert tagged_items.count() == len(expected) + for tag in expected: + match = tagged_items.get(tag__name=tag) + assert match.content_object == obj + + +@pytest.mark.parametrize( + "existing, given, expected", + [ + ([], ["tag1"], ["tag1"]), + (["tag1"], ["tag1"], ["tag1"]), + (["tag1"], [], []), + (["tag1"], ["tag2"], ["tag2"]), + (["tag1", "tag2"], ["tag2"], ["tag2"]), + (["tag1", "tag2"], ["tag3", "tag4"], ["tag3", "tag4"]), + ], +) +def test_set_tags(factories, existing, given, expected): + obj = factories["music.Artist"]() + for tag in existing: + factories["tags.TaggedItem"](content_object=obj, tag__name=tag) + + models.set_tags(obj, *given) + + tagged_items = models.TaggedItem.objects.all() + + assert tagged_items.count() == len(expected) + for tag in expected: + match = tagged_items.get(tag__name=tag) + assert match.content_object == obj -- GitLab