Merge branch '222-update-import' into 'develop'
Resolve "Add flag during import to replace already present tracks with new version" Closes #222 See merge request funkwhale/funkwhale!264
This commit is contained in:
commit
2182227f50
|
@ -89,6 +89,7 @@ class ImportJobFactory(factory.django.DjangoModelFactory):
|
|||
batch = factory.SubFactory(ImportBatchFactory)
|
||||
source = factory.Faker("url")
|
||||
mbid = factory.Faker("uuid4")
|
||||
replace_if_duplicate = factory.Faker("boolean")
|
||||
|
||||
class Meta:
|
||||
model = "music.ImportJob"
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
# Generated by Django 2.0.6 on 2018-06-22 13:36
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('music', '0027_auto_20180515_1808'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='importjob',
|
||||
name='replace_if_duplicate',
|
||||
field=models.BooleanField(default=False),
|
||||
),
|
||||
]
|
|
@ -567,6 +567,7 @@ class ImportBatch(models.Model):
|
|||
|
||||
class ImportJob(models.Model):
|
||||
uuid = models.UUIDField(unique=True, db_index=True, default=uuid.uuid4)
|
||||
replace_if_duplicate = models.BooleanField(default=False)
|
||||
batch = models.ForeignKey(
|
||||
ImportBatch, related_name="jobs", on_delete=models.CASCADE
|
||||
)
|
||||
|
|
|
@ -80,10 +80,11 @@ def import_track_from_remote(library_track):
|
|||
)[0]
|
||||
|
||||
|
||||
def _do_import(import_job, replace=False, use_acoustid=False):
|
||||
def _do_import(import_job, use_acoustid=False):
|
||||
logger.info("[Import Job %s] starting job", import_job.pk)
|
||||
from_file = bool(import_job.audio_file)
|
||||
mbid = import_job.mbid
|
||||
replace = import_job.replace_if_duplicate
|
||||
acoustid_track_id = None
|
||||
duration = None
|
||||
track = None
|
||||
|
@ -135,8 +136,8 @@ def _do_import(import_job, replace=False, use_acoustid=False):
|
|||
|
||||
track_file = None
|
||||
if replace:
|
||||
logger.info("[Import Job %s] replacing existing audio file", import_job.pk)
|
||||
track_file = track.files.first()
|
||||
logger.info("[Import Job %s] deleting existing audio file", import_job.pk)
|
||||
track.files.all().delete()
|
||||
elif track.files.count() > 0:
|
||||
logger.info(
|
||||
"[Import Job %s] skipping, we already have a file for this track",
|
||||
|
@ -163,7 +164,7 @@ def _do_import(import_job, replace=False, use_acoustid=False):
|
|||
# no downloading, we hotlink
|
||||
pass
|
||||
elif not import_job.audio_file and not import_job.source.startswith("file://"):
|
||||
# not an implace import, and we have a source, so let's download it
|
||||
# not an inplace import, and we have a source, so let's download it
|
||||
logger.info("[Import Job %s] downloading audio file from remote", import_job.pk)
|
||||
track_file.download_file()
|
||||
elif not import_job.audio_file and import_job.source.startswith("file://"):
|
||||
|
@ -243,14 +244,14 @@ def get_cover_from_fs(dir_path):
|
|||
@celery.require_instance(
|
||||
models.ImportJob.objects.filter(status__in=["pending", "errored"]), "import_job"
|
||||
)
|
||||
def import_job_run(self, import_job, replace=False, use_acoustid=False):
|
||||
def import_job_run(self, import_job, use_acoustid=False):
|
||||
def mark_errored(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"])
|
||||
|
||||
try:
|
||||
tf = _do_import(import_job, replace, use_acoustid=use_acoustid)
|
||||
tf = _do_import(import_job, use_acoustid=use_acoustid)
|
||||
return tf.pk if tf else None
|
||||
except Exception as exc:
|
||||
if not settings.DEBUG:
|
||||
|
|
|
@ -55,6 +55,17 @@ class Command(BaseCommand):
|
|||
"import and not much disk space available."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--replace",
|
||||
action="store_true",
|
||||
dest="replace",
|
||||
default=False,
|
||||
help=(
|
||||
"Use this flag to replace duplicates (tracks with same "
|
||||
"musicbrainz mbid, or same artist, album and title) on import "
|
||||
"with their newest version."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--noinput",
|
||||
"--no-input",
|
||||
|
@ -112,16 +123,23 @@ class Command(BaseCommand):
|
|||
"No superuser available, please provide a --username"
|
||||
)
|
||||
|
||||
filtered = self.filter_matching(matching)
|
||||
if options["replace"]:
|
||||
filtered = {"initial": matching, "skipped": [], "new": matching}
|
||||
message = "- {} files to be replaced"
|
||||
import_paths = matching
|
||||
else:
|
||||
filtered = self.filter_matching(matching)
|
||||
message = "- {} files already found in database"
|
||||
import_paths = filtered["new"]
|
||||
|
||||
self.stdout.write("Import summary:")
|
||||
self.stdout.write(
|
||||
"- {} files found matching this pattern: {}".format(
|
||||
len(matching), options["path"]
|
||||
)
|
||||
)
|
||||
self.stdout.write(
|
||||
"- {} files already found in database".format(len(filtered["skipped"]))
|
||||
)
|
||||
self.stdout.write(message.format(len(filtered["skipped"])))
|
||||
|
||||
self.stdout.write("- {} new files".format(len(filtered["new"])))
|
||||
|
||||
self.stdout.write(
|
||||
|
@ -141,12 +159,12 @@ class Command(BaseCommand):
|
|||
if input("".join(message)) != "yes":
|
||||
raise CommandError("Import cancelled.")
|
||||
|
||||
batch, errors = self.do_import(filtered["new"], user=user, options=options)
|
||||
batch, errors = self.do_import(import_paths, user=user, options=options)
|
||||
message = "Successfully imported {} tracks"
|
||||
if options["async"]:
|
||||
message = "Successfully launched import for {} tracks"
|
||||
|
||||
self.stdout.write(message.format(len(filtered["new"])))
|
||||
self.stdout.write(message.format(len(import_paths)))
|
||||
if len(errors) > 0:
|
||||
self.stderr.write("{} tracks could not be imported:".format(len(errors)))
|
||||
|
||||
|
@ -196,7 +214,9 @@ class Command(BaseCommand):
|
|||
return batch, errors
|
||||
|
||||
def import_file(self, path, batch, import_handler, options):
|
||||
job = batch.jobs.create(source="file://" + path)
|
||||
job = batch.jobs.create(
|
||||
source="file://" + path, replace_if_duplicate=options["replace"]
|
||||
)
|
||||
if not options["in_place"]:
|
||||
name = os.path.basename(path)
|
||||
with open(path, "rb") as f:
|
||||
|
|
|
@ -118,7 +118,7 @@ def test_run_import_skipping_accoustid(factories, mocker):
|
|||
path = os.path.join(DATA_DIR, "test.ogg")
|
||||
job = factories["music.FileImportJob"](audio_file__path=path)
|
||||
tasks.import_job_run(import_job_id=job.pk, use_acoustid=False)
|
||||
m.assert_called_once_with(job, False, use_acoustid=False)
|
||||
m.assert_called_once_with(job, use_acoustid=False)
|
||||
|
||||
|
||||
def test__do_import_skipping_accoustid(factories, mocker):
|
||||
|
@ -130,7 +130,7 @@ def test__do_import_skipping_accoustid(factories, mocker):
|
|||
path = os.path.join(DATA_DIR, "test.ogg")
|
||||
job = factories["music.FileImportJob"](mbid=None, audio_file__path=path)
|
||||
p = job.audio_file.path
|
||||
tasks._do_import(job, replace=False, use_acoustid=False)
|
||||
tasks._do_import(job, use_acoustid=False)
|
||||
m.assert_called_once_with(p)
|
||||
|
||||
|
||||
|
@ -144,10 +144,27 @@ def test__do_import_skipping_accoustid_if_no_key(factories, mocker, preferences)
|
|||
path = os.path.join(DATA_DIR, "test.ogg")
|
||||
job = factories["music.FileImportJob"](mbid=None, audio_file__path=path)
|
||||
p = job.audio_file.path
|
||||
tasks._do_import(job, replace=False, use_acoustid=False)
|
||||
tasks._do_import(job, use_acoustid=False)
|
||||
m.assert_called_once_with(p)
|
||||
|
||||
|
||||
def test__do_import_replace_if_duplicate(factories, mocker):
|
||||
existing_file = factories["music.TrackFile"]()
|
||||
existing_track = existing_file.track
|
||||
path = os.path.join(DATA_DIR, "test.ogg")
|
||||
mocker.patch(
|
||||
"funkwhale_api.providers.audiofile.tasks.import_track_data_from_path",
|
||||
return_value=existing_track,
|
||||
)
|
||||
job = factories["music.FileImportJob"](
|
||||
replace_if_duplicate=True, audio_file__path=path
|
||||
)
|
||||
tasks._do_import(job)
|
||||
with pytest.raises(existing_file.__class__.DoesNotExist):
|
||||
existing_file.refresh_from_db()
|
||||
assert existing_file.creation_date != job.track_file.creation_date
|
||||
|
||||
|
||||
def test_import_job_skip_if_already_exists(artists, albums, tracks, factories, mocker):
|
||||
path = os.path.join(DATA_DIR, "test.ogg")
|
||||
mbid = "9968a9d6-8d92-4051-8f76-674e157b6eed"
|
||||
|
|
|
@ -6,6 +6,7 @@ from django.core.management import call_command
|
|||
from django.core.management.base import CommandError
|
||||
|
||||
from funkwhale_api.providers.audiofile import tasks
|
||||
from funkwhale_api.music.models import ImportJob
|
||||
|
||||
DATA_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "files")
|
||||
|
||||
|
@ -115,6 +116,19 @@ def test_import_with_multiple_argument(factories, mocker):
|
|||
mocked_filter.assert_called_once_with([path1, path2])
|
||||
|
||||
|
||||
def test_import_with_replace_flag(factories, mocker):
|
||||
factories["users.User"](username="me")
|
||||
path = os.path.join(DATA_DIR, "dummy_file.ogg")
|
||||
mocked_job_run = mocker.patch("funkwhale_api.music.tasks.import_job_run")
|
||||
call_command("import_files", path, username="me", replace=True, interactive=False)
|
||||
created_job = ImportJob.objects.latest("id")
|
||||
|
||||
assert created_job.replace_if_duplicate is True
|
||||
mocked_job_run.assert_called_once_with(
|
||||
import_job_id=created_job.id, use_acoustid=False
|
||||
)
|
||||
|
||||
|
||||
def test_import_files_creates_a_batch_and_job(factories, mocker):
|
||||
m = mocker.patch("funkwhale_api.music.tasks.import_job_run")
|
||||
user = factories["users.User"](username="me")
|
||||
|
|
|
@ -0,0 +1 @@
|
|||
Added replace flag during import to replace already present tracks with a new version of their track file (#222)
|
Loading…
Reference in New Issue