Merge branch 'release/0.14.1'

CHANGELOG

@@ -10,6 +10,94 @@ This changelog is viewable on the web at https://docs.funkwhale.audio/changelog.
 
 .. towncrier
 
+0.14.1 (2018-06-06)
+-------------------
+
+Upgrade instructions are available at https://docs.funkwhale.audio/upgrading.html
+
+Enhancements:
+
+- Display server version in the footer (#270)
+- fix_track_files will now update files with bad mimetype (and not only the one
+  with no mimetype) (#273)
+- Huge performance boost (~x5 to x7) during CLI import that queries MusicBrainz
+  (#288)
+- Removed alpha-state transcoding support (#271)
+
+Bugfixes:
+
+- Broken logging statement during import error (#274)
+- Broken search bar on library home (#278)
+- Do not crash when importing track with an artist that do not match the
+  release artist (#237)
+- Do not crash when tag contains multiple uuids with a / separator (#267)
+- Ensure we do not store bad mimetypes (such as application/x-empty) (#266)
+- Fix broken "play all" button that played only 25 tracks (#281)
+- Fixed broken track download modal (overflow and wrong URL) (#239)
+- Removed hardcoded size limit in file upload widget (#275)
+
+
+Documentation:
+
+- Added warning about _protected/music location in nginx configuration (#247)
+
+
+Removed alpha-state transcoding (#271)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+A few months ago, a basic transcoding feature was implemented. Due to the way
+this feature was designed, it was slow, CPU intensive on the server side,
+and very tightly coupled to the reverse-proxy configuration, preventing
+it to work Apache2, for instance. It was also not compatible with Subsonic clients.
+
+Based on that, we're currently removing support for transcoding
+**in its current state**. The work on a better designed transcoding feature
+can be tracked in https://code.eliotberriot.com/funkwhale/funkwhale/issues/272.
+
+You don't have to do anything on your side, but you may want to remove
+the now obsolete configuration from your reverse proxy file (nginx only)::
+
+    # Remove those blocks:
+
+    # transcode cache
+    proxy_cache_path /tmp/funkwhale-transcode levels=1:2 keys_zone=transcode:10m max_size=1g inactive=7d;
+
+    # Transcoding logic and caching
+    location = /transcode-auth {
+        include /etc/nginx/funkwhale_proxy.conf;
+        # needed so we can authenticate transcode requests, but still
+        # cache the result
+        internal;
+        set $query '';
+        # ensure we actually pass the jwt to the underlytin auth url
+        if ($request_uri ~* "[^\?]+\?(.*)$") {
+            set $query $1;
+        }
+        proxy_pass http://funkwhale-api/api/v1/trackfiles/viewable/?$query;
+        proxy_pass_request_body off;
+        proxy_set_header        Content-Length "";
+    }
+
+    location /api/v1/trackfiles/transcode/ {
+        include /etc/nginx/funkwhale_proxy.conf;
+        # this block deals with authenticating and caching transcoding
+        # requests. Caching is heavily recommended as transcoding
+        # is a CPU intensive process.
+        auth_request /transcode-auth;
+        if ($args ~ (.*)jwt=[^&]*(.*)) {
+            set $cleaned_args $1$2;
+        }
+        proxy_cache_key "$scheme$request_method$host$uri$is_args$cleaned_args";
+        proxy_cache transcode;
+        proxy_cache_valid 200 7d;
+        proxy_ignore_headers "Set-Cookie";
+        proxy_hide_header "Set-Cookie";
+        add_header X-Cache-Status $upstream_cache_status;
+        proxy_pass   http://funkwhale-api;
+    }
+    # end of transcoding logic
+
+
 0.14 (2018-06-02)
 -----------------
 

api/funkwhale_api/__init__.py

 # -*- coding: utf-8 -*-
-__version__ = '0.14'
+__version__ = '0.14.1'
 __version_info__ = tuple([int(num) if num.isdigit() else num for num in __version__.replace('-', '.', 1).split('.')])

api/funkwhale_api/music/forms.py

-from django import forms
-
-from . import models
-
-
-class TranscodeForm(forms.Form):
-    FORMAT_CHOICES = [
-        ('ogg', 'ogg'),
-        ('mp3', 'mp3'),
-    ]
-
-    to = forms.ChoiceField(choices=FORMAT_CHOICES)
-    BITRATE_CHOICES = [
-        (64, '64'),
-        (128, '128'),
-        (256, '256'),
-    ]
-    bitrate = forms.ChoiceField(
-        choices=BITRATE_CHOICES, required=False)
-
-    track_file = forms.ModelChoiceField(
-        queryset=models.TrackFile.objects.exclude(audio_file__isnull=True)
-    )

api/funkwhale_api/music/management/commands/fix_track_files.py

@@ -33,9 +33,9 @@ class Command(BaseCommand):
     def fix_mimetypes(self, dry_run, **kwargs):
         self.stdout.write('Fixing missing mimetypes...')
         matching = models.TrackFile.objects.filter(
-            source__startswith='file://', mimetype=None)
+            source__startswith='file://').exclude(mimetype__startswith='audio/')
         self.stdout.write(
-            '[mimetypes] {} entries found with no mimetype'.format(
+            '[mimetypes] {} entries found with bad or no mimetype'.format(
                 matching.count()))
         for extension, mimetype in utils.EXTENSION_TO_MIMETYPE.items():
             qs = matching.filter(source__endswith='.{}'.format(extension))

api/funkwhale_api/music/metadata.py

@@ -91,10 +91,23 @@ def convert_track_number(v):
         pass
 
 
+
+class FirstUUIDField(forms.UUIDField):
+    def to_python(self, value):
+        try:
+            # sometimes, Picard leaves to uuids in the field, separated
+            # by a slash
+            value = value.split('/')[0]
+        except (AttributeError, IndexError, TypeError):
+            pass
+
+        return super().to_python(value)
+
+
 VALIDATION = {
-    'musicbrainz_artistid': forms.UUIDField(),
-    'musicbrainz_albumid': forms.UUIDField(),
-    'musicbrainz_recordingid': forms.UUIDField(),
+    'musicbrainz_artistid': FirstUUIDField(),
+    'musicbrainz_albumid': FirstUUIDField(),
+    'musicbrainz_recordingid': FirstUUIDField(),
 }
 
 CONF = {

api/funkwhale_api/music/models.py

@@ -334,6 +334,11 @@ class TrackQuerySet(models.QuerySet):
                     .prefetch_related('files'))
 
 
+def get_artist(release_list):
+    return Artist.get_or_create_from_api(
+        mbid=release_list[0]['artist-credits'][0]['artists']['id'])[0]
+
+
 class Track(APIModelMixin):
     title = models.CharField(max_length=255)
     artist = models.ForeignKey(
@@ -363,8 +368,9 @@ class Track(APIModelMixin):
             'musicbrainz_field_name': 'title'
         },
         'artist': {
-            'musicbrainz_field_name': 'artist-credit',
-            'converter': lambda v: Artist.get_or_create_from_api(mbid=v[0]['artist']['id'])[0],
+            # we use the artist from the release to avoid #237
+            'musicbrainz_field_name': 'release-list',
+            'converter': get_artist,
         },
         'album': {
             'musicbrainz_field_name': 'release-list',
@@ -431,7 +437,40 @@ class Track(APIModelMixin):
             title__iexact=title,
             defaults=kwargs)
 
-
+    @classmethod
+    def get_or_create_from_release(cls, release_mbid, mbid):
+        release_mbid = str(release_mbid)
+        mbid = str(mbid)
+        try:
+            return cls.objects.get(mbid=mbid), False
+        except cls.DoesNotExist:
+            pass
+
+        album = Album.get_or_create_from_api(release_mbid)[0]
+        data = musicbrainz.client.api.releases.get(
+            str(album.mbid), includes=Album.api_includes)
+        tracks = [
+            t
+            for m in data['release']['medium-list']
+            for t in m['track-list']
+        ]
+        track_data = None
+        for track in tracks:
+            if track['recording']['id'] == mbid:
+                track_data = track
+                break
+        if not track_data:
+            raise ValueError('No track found matching this ID')
+
+        return cls.objects.update_or_create(
+            mbid=mbid,
+            defaults={
+                'position': int(track['position']),
+                'title': track['recording']['title'],
+                'album': album,
+                'artist': album.artist,
+            }
+        )
 class TrackFile(models.Model):
     uuid = models.UUIDField(
         unique=True, db_index=True, default=uuid.uuid4)

api/funkwhale_api/music/tasks.py

@@ -259,7 +259,9 @@ def get_cover_from_fs(dir_path):
     'import_job')
 def import_job_run(self, import_job, replace=False, use_acoustid=False):
     def mark_errored(exc):
-        logger.error('[Import Job %s] Error during import: %s', str(exc))
+        logger.error(
+            '[Import Job %s] Error during import: %s',
+            import_job.pk, str(exc))
         import_job.status = 'errored'
         import_job.save(update_fields=['status'])
 

api/funkwhale_api/music/utils.py

@@ -43,9 +43,9 @@ def get_query(query_string, search_fields):
 
 
 def guess_mimetype(f):
-    b = min(100000, f.size)
+    b = min(1000000, f.size)
     t = magic.from_buffer(f.read(b), mime=True)
-    if t == 'application/octet-stream':
+    if not t.startswith('audio/'):
         # failure, we try guessing by extension
         mt, _ = mimetypes.guess_type(f.path)
         if mt:

api/funkwhale_api/music/views.py

@@ -35,7 +35,6 @@ from funkwhale_api.musicbrainz import api
 from funkwhale_api.requests.models import ImportRequest
 
 from . import filters
-from . import forms
 from . import importers
 from . import models
 from . import permissions as music_permissions
@@ -324,42 +323,6 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
         except models.TrackFile.DoesNotExist:
             return Response(status=404)
 
-    @list_route(methods=['get'])
-    def viewable(self, request, *args, **kwargs):
-        return Response({}, status=200)
-
-    @list_route(methods=['get'])
-    def transcode(self, request, *args, **kwargs):
-        form = forms.TranscodeForm(request.GET)
-        if not form.is_valid():
-            return Response(form.errors, status=400)
-
-        f = form.cleaned_data['track_file']
-        if not f.audio_file:
-            return Response(status=400)
-        output_kwargs = {
-            'format': form.cleaned_data['to']
-        }
-        args = (ffmpeg
-            .input(f.audio_file.path)
-            .output('pipe:', **output_kwargs)
-            .get_args()
-        )
-        # we use a generator here so the view return immediatly and send
-        # file chunk to the browser, instead of blocking a few seconds
-        def _transcode():
-            p = subprocess.Popen(
-                ['ffmpeg'] + args,
-                stdout=subprocess.PIPE)
-            for line in p.stdout:
-                yield line
-
-        response = StreamingHttpResponse(
-            _transcode(), status=200,
-            content_type=form.cleaned_data['to'])
-
-        return response
-
 
 class TagViewSet(viewsets.ReadOnlyModelViewSet):
     queryset = Tag.objects.all().order_by('name')

api/funkwhale_api/providers/audiofile/tasks.py

@@ -12,25 +12,43 @@ from funkwhale_api.music import models, metadata
 @transaction.atomic
 def import_track_data_from_path(path):
     data = metadata.Metadata(path)
-    artist = models.Artist.objects.get_or_create(
-        name__iexact=data.get('artist'),
-        defaults={
-            'name': data.get('artist'),
-            'mbid': data.get('musicbrainz_artistid', None),
-        },
-    )[0]
+    album = None
+    track_mbid = data.get('musicbrainz_recordingid', None)
+    album_mbid = data.get('musicbrainz_albumid', None)
 
-    release_date = data.get('date', default=None)
-    album = models.Album.objects.get_or_create(
-        title__iexact=data.get('album'),
-        artist=artist,
-        defaults={
-            'title': data.get('album'),
-            'release_date': release_date,
-            'mbid': data.get('musicbrainz_albumid', None),
-        },
-    )[0]
+    if album_mbid and track_mbid:
+        # to gain performance and avoid additional mb lookups,
+        # we import from the release data, which is already cached
+        return models.Track.get_or_create_from_release(
+            album_mbid, track_mbid)[0]
+    elif track_mbid:
+        return models.Track.get_or_create_from_api(track_mbid)[0]
+    elif album_mbid:
+        album = models.Album.get_or_create_from_api(album_mbid)[0]
 
+    artist = album.artist if album else None
+    artist_mbid = data.get('musicbrainz_artistid', None)
+    if not artist:
+        if artist_mbid:
+            artist = models.Artist.get_or_create_from_api(artist_mbid)[0]
+        else:
+            artist = models.Artist.objects.get_or_create(
+                name__iexact=data.get('artist'),
+                defaults={
+                    'name': data.get('artist'),
+                },
+            )[0]
+
+    release_date = data.get('date', default=None)
+    if not album:
+        album = models.Album.objects.get_or_create(
+            title__iexact=data.get('album'),
+            artist=artist,
+            defaults={
+                'title': data.get('album'),
+                'release_date': release_date,
+            },
+        )[0]
     position = data.get('track_number', default=None)
     track = models.Track.objects.get_or_create(
         title__iexact=data.get('title'),
@@ -38,7 +56,6 @@ def import_track_data_from_path(path):
         defaults={
             'title': data.get('title'),
             'position': position,
-            'mbid': data.get('musicbrainz_recordingid', None),
         },
     )[0]
     return track

api/tests/music/test_commands.py

+import os
+
 from funkwhale_api.music.management.commands import fix_track_files
 
+DATA_DIR = os.path.dirname(os.path.abspath(__file__))
+
 
 def test_fix_track_files_bitrate_length(factories, mocker):
     tf1 = factories['music.TrackFile'](bitrate=1, duration=2)
@@ -43,3 +47,27 @@ def test_fix_track_files_size(factories, mocker):
 
     # updated
     assert tf2.size == 2
+
+
+def test_fix_track_files_mimetype(factories, mocker):
+    name = 'test.mp3'
+    mp3_path = os.path.join(DATA_DIR, 'test.mp3')
+    ogg_path = os.path.join(DATA_DIR, 'test.ogg')
+    tf1 = factories['music.TrackFile'](
+        audio_file__from_path=mp3_path,
+        source='file://{}'.format(mp3_path),
+        mimetype='application/x-empty')
+
+    # this one already has a mimetype set, to it should not be updated
+    tf2 = factories['music.TrackFile'](
+        audio_file__from_path=ogg_path,
+        source='file://{}'.format(ogg_path),
+        mimetype='audio/something')
+    c = fix_track_files.Command()
+    c.fix_mimetypes(dry_run=False)
+
+    tf1.refresh_from_db()
+    tf2.refresh_from_db()
+
+    assert tf1.mimetype == 'audio/mpeg'
+    assert tf2.mimetype == 'audio/something'

api/tests/music/test_metadata.py

@@ -95,3 +95,17 @@ def test_can_get_metadata_from_flac_file_not_crash_if_empty():
 
     with pytest.raises(metadata.TagNotFound):
         data.get('test')
+
+
+@pytest.mark.parametrize('field_name', [
+    'musicbrainz_artistid',
+    'musicbrainz_albumid',
+    'musicbrainz_recordingid',
+])
+def test_mbid_clean_keeps_only_first(field_name):
+    u1 = str(uuid.uuid4())
+    u2 = str(uuid.uuid4())
+    field = metadata.VALIDATION[field_name]
+    result = field.to_python('/'.join([u1, u2]))
+
+    assert str(result) == u1

api/tests/music/test_models.py

@@ -43,6 +43,53 @@ def test_import_album_stores_release_group(factories):
     assert album.artist == artist
 
 
+def test_import_track_from_release(factories, mocker):
+    album = factories['music.Album'](
+        mbid='430347cb-0879-3113-9fde-c75b658c298e')
+    album_data = {
+        'release': {
+            'id': album.mbid,
+            'title': 'Daydream Nation',
+            'status': 'Official',
+            'medium-count': 1,
+            'medium-list': [
+                {
+                    'position': '1',
+                    'format': 'CD',
+                    'track-list': [
+                        {
+                            'id': '03baca8b-855a-3c05-8f3d-d3235287d84d',
+                            'position': '4',
+                            'number': '4',
+                            'length': '417973',
+                            'recording': {
+                                'id': '2109e376-132b-40ad-b993-2bb6812e19d4',
+                                'title': 'Teen Age Riot',
+                                'length': '417973'},
+                            'track_or_recording_length': '417973'
+                        }
+                    ],
+                    'track-count': 1
+                }
+            ],
+        }
+    }
+    mocked_get = mocker.patch(
+        'funkwhale_api.musicbrainz.api.releases.get',
+        return_value=album_data)
+    track_data = album_data['release']['medium-list'][0]['track-list'][0]
+    track = models.Track.get_or_create_from_release(
+        '430347cb-0879-3113-9fde-c75b658c298e',
+        track_data['recording']['id'],
+    )[0]
+    mocked_get.assert_called_once_with(
+        album.mbid, includes=models.Album.api_includes)
+    assert track.title == track_data['recording']['title']
+    assert track.mbid == track_data['recording']['id']
+    assert track.album == album
+    assert track.artist == album.artist
+    assert track.position == int(track_data['position'])
+
 def test_import_job_is_bound_to_track_file(factories, mocker):
     track = factories['music.Track']()
     job = factories['music.ImportJob'](mbid=track.mbid)

api/tests/music/test_utils.py

@@ -15,9 +15,13 @@ def test_guess_mimetype_try_using_extension(factories, mocker):
     assert utils.guess_mimetype(f.audio_file) == 'audio/mpeg'
 
 
-def test_guess_mimetype_try_using_extension_if_fail(factories, mocker):
+@pytest.mark.parametrize('wrong', [
+    'application/octet-stream',
+    'application/x-empty',
+])
+def test_guess_mimetype_try_using_extension_if_fail(wrong, factories, mocker):
     mocker.patch(
-        'magic.from_buffer', return_value='application/octet-stream')
+        'magic.from_buffer', return_value=wrong)
     f = factories['music.TrackFile'].build(
         audio_file__filename='test.mp3')
 

api/tests/test_import_audio_file.py

@@ -15,16 +15,13 @@ DATA_DIR = os.path.join(
 )
 
 
-def test_can_create_track_from_file_metadata(db, mocker):
+def test_can_create_track_from_file_metadata_no_mbid(db, mocker):
     metadata = {
         'artist': ['Test artist'],
         'album': ['Test album'],
         'title': ['Test track'],
         'TRACKNUMBER': ['4'],
         'date': ['2012-08-15'],
-        'musicbrainz_albumid': ['a766da8b-8336-47aa-a3ee-371cc41ccc75'],
-        'musicbrainz_trackid': ['bd21ac48-46d8-4e78-925f-d9cc2a294656'],
-        'musicbrainz_artistid': ['013c8e5b-d72a-4cd3-8dee-6c64d6125823'],
     }
     m1 = mocker.patch('mutagen.File', return_value=metadata)
     m2 = mocker.patch(
@@ -35,13 +32,64 @@ def test_can_create_track_from_file_metadata(db, mocker):
         os.path.join(DATA_DIR, 'dummy_file.ogg'))
 
     assert track.title == metadata['title'][0]
-    assert track.mbid == uuid.UUID(metadata['musicbrainz_trackid'][0])
+    assert track.mbid is None
     assert track.position == 4
     assert track.album.title == metadata['album'][0]
-    assert track.album.mbid == uuid.UUID(metadata['musicbrainz_albumid'][0])
+    assert track.album.mbid is None
     assert track.album.release_date == datetime.date(2012, 8, 15)
     assert track.artist.name == metadata['artist'][0]
-    assert track.artist.mbid == uuid.UUID(metadata['musicbrainz_artistid'][0])
+    assert track.artist.mbid is None
+
+
+def test_can_create_track_from_file_metadata_mbid(factories, mocker):
+    album = factories['music.Album']()
+    mocker.patch(
+        'funkwhale_api.music.models.Album.get_or_create_from_api',
+        return_value=(album, True),
+    )
+
+    album_data = {
+        'release': {
+            'id': album.mbid,
+            'medium-list': [
+                {
+                    'track-list': [
+                        {
+                            'id': '03baca8b-855a-3c05-8f3d-d3235287d84d',
+                            'position': '4',
+                            'number': '4',
+                            'recording': {
+                                'id': '2109e376-132b-40ad-b993-2bb6812e19d4',
+                                'title': 'Teen Age Riot',
+                            },
+                        }
+                    ],
+                    'track-count': 1
+                }
+            ],
+        }
+    }
+    mocker.patch(
+        'funkwhale_api.musicbrainz.api.releases.get',
+        return_value=album_data)
+    track_data = album_data['release']['medium-list'][0]['track-list'][0]
+    metadata = {
+        'musicbrainz_albumid': [album.mbid],
+        'musicbrainz_trackid': [track_data['recording']['id']],
+    }
+    m1 = mocker.patch('mutagen.File', return_value=metadata)
+    m2 = mocker.patch(
+        'funkwhale_api.music.metadata.Metadata.get_file_type',
+        return_value='OggVorbis',
+    )
+    track = tasks.import_track_data_from_path(
+        os.path.join(DATA_DIR, 'dummy_file.ogg'))
+
+    assert track.title == track_data['recording']['title']
+    assert track.mbid == track_data['recording']['id']
+    assert track.position == 4
+    assert track.album == album
+    assert track.artist == album.artist
 
 
 def test_management_command_requires_a_valid_username(factories, mocker):

changes/template.rst

 
-Upgrade instructions are available at https://docs.funkwhale.audio/upgrading.html
+Upgrade instructions are available at
+https://docs.funkwhale.audio/upgrading.html
 
 {% for section, _ in sections.items() %}
 {% if sections[section] %}

deploy/nginx.conf

 # Ensure you update at least the server_name variables to match your own