diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 98fc1965b51dff90a9aaae1717e5dc7ca5032538..18f181e884aec2877c07f3b6dac533411d653e94 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -463,26 +463,6 @@ class TrackFile(models.Model): self.mimetype = utils.guess_mimetype(self.audio_file) return super().save(**kwargs) - @property - def serve_from_source_path(self): - if not self.source or not self.source.startswith('file://'): - raise ValueError('Cannot serve this file from source') - serve_path = settings.MUSIC_DIRECTORY_SERVE_PATH - prefix = settings.MUSIC_DIRECTORY_PATH - if not serve_path or not prefix: - raise ValueError( - 'You need to specify MUSIC_DIRECTORY_SERVE_PATH and ' - 'MUSIC_DIRECTORY_PATH to serve in-place imported files' - ) - file_path = self.source.replace('file://', '', 1) - parts = os.path.split(file_path.replace(prefix, '', 1)) - if parts[0] == '/': - parts = parts[1:] - return os.path.join( - serve_path, - *parts - ) - IMPORT_STATUS_CHOICES = ( ('pending', 'Pending'), diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index af063da4679e7df8c4e295dc19dbd03812a1c810..cab8eb73876e0d2827e7f5e1e30f3d4a4f185817 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -206,6 +206,8 @@ class TrackViewSet( def get_file_path(audio_file): + serve_path = settings.MUSIC_DIRECTORY_SERVE_PATH + prefix = settings.MUSIC_DIRECTORY_PATH t = settings.REVERSE_PROXY_TYPE if t == 'nginx': # we have to use the internal locations @@ -213,14 +215,24 @@ def get_file_path(audio_file): path = audio_file.url except AttributeError: # a path was given - path = '/music' + audio_file + if not serve_path or not prefix: + raise ValueError( + 'You need to specify MUSIC_DIRECTORY_SERVE_PATH and ' + 'MUSIC_DIRECTORY_PATH to serve in-place imported files' + ) + path = '/music' + audio_file.replace(prefix, '', 1) return settings.PROTECT_FILES_PATH + path if t == 'apache2': try: path = audio_file.path except AttributeError: # a path was given - path = audio_file + if not serve_path or not prefix: + raise ValueError( + 'You need to specify MUSIC_DIRECTORY_SERVE_PATH and ' + 'MUSIC_DIRECTORY_PATH to serve in-place imported files' + ) + path = audio_file.replace(prefix, serve_path, 1) return path @@ -267,7 +279,7 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet): elif audio_file: file_path = get_file_path(audio_file) elif f.source and f.source.startswith('file://'): - file_path = get_file_path(f.serve_from_source_path) + file_path = get_file_path(f.source.replace('file://', '', 1)) response = Response() filename = f.filename mapping = { diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 5d7589af08c16efe1b69b72d48b37e4c77500633..7d4117f80155db4c90a9773ff1c22c04e1a70404 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -76,29 +76,60 @@ def test_can_serve_track_file_as_remote_library_deny_not_following( assert response.status_code == 403 -def test_serve_file_apache(factories, api_client, settings): +@pytest.mark.parametrize('proxy,serve_path,expected', [ + ('apache2', '/host/music', '/host/music/hello/world.mp3'), + ('apache2', '/app/music', '/app/music/hello/world.mp3'), + ('nginx', '/host/music', '/_protected/music/hello/world.mp3'), + ('nginx', '/app/music', '/_protected/music/hello/world.mp3'), +]) +def test_serve_file_in_place( + proxy, serve_path, expected, factories, api_client, settings): + headers = { + 'apache2': 'X-Sendfile', + 'nginx': 'X-Accel-Redirect', + } settings.PROTECT_AUDIO_FILES = False - settings.REVERSE_PROXY_TYPE = 'apache2' - tf = factories['music.TrackFile']() + settings.PROTECT_FILE_PATH = '/_protected/music' + settings.REVERSE_PROXY_TYPE = proxy + settings.MUSIC_DIRECTORY_PATH = '/app/music' + settings.MUSIC_DIRECTORY_SERVE_PATH = serve_path + tf = factories['music.TrackFile']( + in_place=True, + source='file:///app/music/hello/world.mp3' + ) response = api_client.get(tf.path) assert response.status_code == 200 - assert response['X-Sendfile'] == tf.audio_file.path + assert response[headers[proxy]] == expected -def test_serve_file_apache_in_place(factories, api_client, settings): +@pytest.mark.parametrize('proxy,serve_path,expected', [ + ('apache2', '/host/music', '/host/media/tracks/hello/world.mp3'), + # apache with container not supported yet + # ('apache2', '/app/music', '/app/music/tracks/hello/world.mp3'), + ('nginx', '/host/music', '/_protected/media/tracks/hello/world.mp3'), + ('nginx', '/app/music', '/_protected/media/tracks/hello/world.mp3'), +]) +def test_serve_file_media( + proxy, serve_path, expected, factories, api_client, settings): + headers = { + 'apache2': 'X-Sendfile', + 'nginx': 'X-Accel-Redirect', + } settings.PROTECT_AUDIO_FILES = False - settings.REVERSE_PROXY_TYPE = 'apache2' - settings.MUSIC_DIRECTORY_PATH = '/music' - settings.MUSIC_DIRECTORY_SERVE_PATH = '/host/music' - track_file = factories['music.TrackFile']( - in_place=True, - source='file:///music/test.ogg') + settings.MEDIA_ROOT = '/host/media' + settings.PROTECT_FILE_PATH = '/_protected/music' + settings.REVERSE_PROXY_TYPE = proxy + settings.MUSIC_DIRECTORY_PATH = '/app/music' + settings.MUSIC_DIRECTORY_SERVE_PATH = serve_path - response = api_client.get(track_file.path) + tf = factories['music.TrackFile']() + tf.__class__.objects.filter(pk=tf.pk).update( + audio_file='tracks/hello/world.mp3') + response = api_client.get(tf.path) assert response.status_code == 200 - assert response['X-Sendfile'] == '/host/music/test.ogg' + assert response[headers[proxy]] == expected def test_can_proxy_remote_track( @@ -118,25 +149,6 @@ def test_can_proxy_remote_track( assert library_track.audio_file.read() == b'test' -def test_can_serve_in_place_imported_file( - factories, settings, api_client, r_mock): - settings.PROTECT_AUDIO_FILES = False - settings.MUSIC_DIRECTORY_SERVE_PATH = '/host/music' - settings.MUSIC_DIRECTORY_PATH = '/music' - settings.MUSIC_DIRECTORY_PATH = '/music' - track_file = factories['music.TrackFile']( - in_place=True, - source='file:///music/test.ogg') - - response = api_client.get(track_file.path) - - assert response.status_code == 200 - assert response['X-Accel-Redirect'] == '{}{}'.format( - settings.PROTECT_FILES_PATH, - '/music/host/music/test.ogg' - ) - - def test_can_create_import_from_federation_tracks( factories, superuser_api_client, mocker): lts = factories['federation.LibraryTrack'].create_batch(size=5) diff --git a/changes/changelog.d/182.bugfix b/changes/changelog.d/182.bugfix new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391