From 93cd72ff09da9154f5cca5deab8a2d798197b521 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Mon, 9 Jul 2018 22:35:32 +0200
Subject: [PATCH] Fix #351: Ensure we do not import artists with empty names

---
 api/funkwhale_api/music/importers.py |  7 +++++++
 api/tests/music/test_import.py       | 11 +++++++++++
 api/tests/users/test_models.py       |  2 +-
 changes/changelog.d/351.bugfix       |  1 +
 4 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 changes/changelog.d/351.bugfix

diff --git a/api/funkwhale_api/music/importers.py b/api/funkwhale_api/music/importers.py
index ce7ded02..fc4a9824 100644
--- a/api/funkwhale_api/music/importers.py
+++ b/api/funkwhale_api/music/importers.py
@@ -3,12 +3,19 @@ def load(model, *args, **kwargs):
     return importer.load(*args, **kwargs)
 
 
+EXCLUDE_VALIDATION = {"Track": ["artist"]}
+
+
 class Importer(object):
     def __init__(self, model):
         self.model = model
 
     def load(self, cleaned_data, raw_data, import_hooks):
         mbid = cleaned_data.pop("mbid")
+        # let's validate data, just in case
+        instance = self.model(**cleaned_data)
+        exclude = EXCLUDE_VALIDATION.get(self.model.__name__, [])
+        instance.full_clean(exclude=["mbid", "uuid"] + exclude)
         m = self.model.objects.update_or_create(mbid=mbid, defaults=cleaned_data)[0]
         for hook in import_hooks:
             hook(m, cleaned_data, raw_data)
diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py
index 2be267cf..12185f88 100644
--- a/api/tests/music/test_import.py
+++ b/api/tests/music/test_import.py
@@ -1,10 +1,15 @@
 import json
 import os
+import pytest
+import uuid
 
+from django import forms
 from django.urls import reverse
 
 from funkwhale_api.federation import actors
 from funkwhale_api.federation import serializers as federation_serializers
+from funkwhale_api.music import importers
+from funkwhale_api.music import models
 from funkwhale_api.music import tasks
 
 DATA_DIR = os.path.dirname(os.path.abspath(__file__))
@@ -237,3 +242,9 @@ def test__do_import_in_place_mbid(factories, tmpfile):
     assert bool(tf.audio_file) is False
     assert tf.source == "file://{}".format(path)
     assert tf.mimetype == "audio/ogg"
+
+
+def test_importer_cleans():
+    importer = importers.Importer(models.Artist)
+    with pytest.raises(forms.ValidationError):
+        importer.load({"name": "", "mbid": uuid.uuid4()}, {}, [])
diff --git a/api/tests/users/test_models.py b/api/tests/users/test_models.py
index ea760cc6..f4a27b40 100644
--- a/api/tests/users/test_models.py
+++ b/api/tests/users/test_models.py
@@ -126,4 +126,4 @@ def test_can_filter_closed_invitations(factories):
     used = factories["users.User"](invited=True).invitation
 
     assert models.Invitation.objects.count() == 3
-    assert list(models.Invitation.objects.open(False)) == [expired, used]
+    assert list(models.Invitation.objects.order_by("id").open(False)) == [expired, used]
diff --git a/changes/changelog.d/351.bugfix b/changes/changelog.d/351.bugfix
new file mode 100644
index 00000000..49fa9af4
--- /dev/null
+++ b/changes/changelog.d/351.bugfix
@@ -0,0 +1 @@
+Ensure we do not import artists with empty names (#351)
-- 
GitLab