diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index d0e07cd85cdea5f62157455afe23729e87dc4f71..f873e197f9afe405ecb8c9dd46a70d84e7dce67f 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -769,7 +769,7 @@ class TrackSerializer(MusicEntitySerializer):
from_activity = self.context.get("activity")
if from_activity:
metadata["from_activity_id"] = from_activity.pk
- track = music_tasks.get_track_from_import_metadata(metadata)
+ track = music_tasks.get_track_from_import_metadata(metadata, update_cover=True)
return track
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index c8fed58410d9abacd4f5872451a48238b8988840..946d7a411f9de1e1eb6089fd148974718e20ea65 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -26,7 +26,9 @@ from . import serializers
logger = logging.getLogger(__name__)
-def update_album_cover(album, source=None, cover_data=None, replace=False):
+def update_album_cover(
+ album, source=None, cover_data=None, musicbrainz=True, replace=False
+):
if album.cover and not replace:
return
if cover_data:
@@ -39,7 +41,7 @@ def update_album_cover(album, source=None, cover_data=None, replace=False):
cover = get_cover_from_fs(path)
if cover:
return album.get_image(data=cover)
- if album.mbid:
+ if musicbrainz and album.mbid:
try:
logger.info(
"[Album %s] Fetching cover from musicbrainz release %s",
@@ -179,8 +181,8 @@ def process_upload(upload):
import_metadata = upload.import_metadata or {}
old_status = upload.import_status
audio_file = upload.get_audio_file()
+ additional_data = {}
try:
- additional_data = {}
if not audio_file:
# we can only rely on user proveded data
final_metadata = import_metadata
@@ -241,6 +243,15 @@ def process_upload(upload):
"bitrate",
]
)
+
+ # update album cover, if needed
+ if not track.album.cover:
+ update_album_cover(
+ track.album,
+ source=final_metadata.get("upload_source"),
+ cover_data=final_metadata.get("cover_data"),
+ )
+
broadcast = getter(
import_metadata, "funkwhale", "config", "broadcast", default=True
)
@@ -369,7 +380,18 @@ def sort_candidates(candidates, important_fields):
@transaction.atomic
-def get_track_from_import_metadata(data):
+def get_track_from_import_metadata(data, update_cover=False):
+ track = _get_track(data)
+ if update_cover and track and not track.album.cover:
+ update_album_cover(
+ track.album,
+ source=data.get("upload_source"),
+ cover_data=data.get("cover_data"),
+ )
+ return track
+
+
+def _get_track(data):
track_uuid = getter(data, "funkwhale", "track", "uuid")
if track_uuid:
@@ -380,12 +402,6 @@ def get_track_from_import_metadata(data):
except models.Track.DoesNotExist:
raise UploadImportError(code="track_uuid_not_found")
- if not track.album.cover:
- update_album_cover(
- track.album,
- source=data.get("upload_source"),
- cover_data=data.get("cover_data"),
- )
return track
from_activity_id = data.get("from_activity_id", None)
@@ -479,10 +495,6 @@ def get_track_from_import_metadata(data):
album = get_best_candidate_or_create(
models.Album, query, defaults=defaults, sort_fields=["mbid", "fid"]
)[0]
- if not album.cover:
- update_album_cover(
- album, source=data.get("upload_source"), cover_data=data.get("cover_data")
- )
# get / create track
track_title = data["title"]
diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py
index b7b04674f713af3888391567119c782f07051ece..fcb464f3686e44b05f3733830441e5d073a85a51 100644
--- a/api/tests/music/test_tasks.py
+++ b/api/tests/music/test_tasks.py
@@ -152,7 +152,7 @@ def test_can_create_track_from_file_metadata_federation(factories, mocker, r_moc
r_mock.get(metadata["cover_data"]["url"], body=io.BytesIO(b"coucou"))
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
- track = tasks.get_track_from_import_metadata(metadata)
+ track = tasks.get_track_from_import_metadata(metadata, update_cover=True)
assert track.title == metadata["title"]
assert track.fid == metadata["fid"]
@@ -182,7 +182,9 @@ def test_sort_candidates(factories):
def test_upload_import(now, factories, temp_signal, mocker):
outbox = mocker.patch("funkwhale_api.federation.routes.outbox.dispatch")
- track = factories["music.Track"]()
+ update_album_cover = mocker.patch("funkwhale_api.music.tasks.update_album_cover")
+ get_picture = mocker.patch("funkwhale_api.music.metadata.Metadata.get_picture")
+ track = factories["music.Track"](album__cover="")
upload = factories["music.Upload"](
track=None, import_metadata={"funkwhale": {"track": {"uuid": str(track.uuid)}}}
)
@@ -195,6 +197,10 @@ def test_upload_import(now, factories, temp_signal, mocker):
assert upload.track == track
assert upload.import_status == "finished"
assert upload.import_date == now
+ get_picture.assert_called_once_with("cover_front", "other")
+ update_album_cover.assert_called_once_with(
+ upload.track.album, cover_data=get_picture.return_value, source=upload.source
+ )
handler.assert_called_once_with(
upload=upload,
old_status="pending",
diff --git a/changes/changelog.d/757.bugfix b/changes/changelog.d/757.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..f9e9eae351a63297a17b790e957ad7dabba292bf
--- /dev/null
+++ b/changes/changelog.d/757.bugfix
@@ -0,0 +1 @@
+Ensure cover art from uploaded files is picked up properly on existing albums (#757)
diff --git a/dev.yml b/dev.yml
index 2b1e7c5fee3af2996f204dfe4c7a47206266f790..51aad05898f4f04be13c594cc86bc04d7128f4f6 100644
--- a/dev.yml
+++ b/dev.yml
@@ -52,7 +52,7 @@ services:
command: python /app/manage.py runserver 0.0.0.0:${FUNKWHALE_API_PORT-5000}
volumes:
- ./api:/app
- - "${MUSIC_DIRECTORY_PATH-./data/music}:/music:ro"
+ - "${MUSIC_DIRECTORY_SERVE_PATH-./data/music}:/music:ro"
environment:
- "FUNKWHALE_HOSTNAME=${FUNKWHALE_HOSTNAME-localhost}"
- "FUNKWHALE_HOSTNAME_SUFFIX=funkwhale.test"
@@ -86,7 +86,7 @@ services:
- "CACHE_URL=redis://redis:6379/0"
volumes:
- ./api:/app
- - "${MUSIC_DIRECTORY_PATH-./data/music}:/music:ro"
+ - "${MUSIC_DIRECTORY_SERVE_PATH-./data/music}:/music:ro"
networks:
- internal
nginx:
@@ -109,7 +109,7 @@ services:
volumes:
- ./docker/nginx/conf.dev:/etc/nginx/nginx.conf.template:ro
- ./docker/nginx/entrypoint.sh:/entrypoint.sh:ro
- - "${MUSIC_DIRECTORY_PATH-./data/music}:/music:ro"
+ - "${MUSIC_DIRECTORY_SERVE_PATH-./data/music}:/music:ro"
- ./deploy/funkwhale_proxy.conf:/etc/nginx/funkwhale_proxy.conf:ro
- "${MEDIA_ROOT-./api/funkwhale_api/media}:/protected/media:ro"
networks: