From 4e126bd2a3099228ed771ac222155012e9c8dc84 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Thu, 3 Nov 2022 08:29:35 +0100 Subject: [PATCH] PICARD-2565: Apply make_short_filename and make_save_path to saved image files --- picard/coverart/image.py | 25 +++++++++++++++++++------ test/test_coverart_image.py | 32 +++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/picard/coverart/image.py b/picard/coverart/image.py index 10e7246e8..2471ed158 100644 --- a/picard/coverart/image.py +++ b/picard/coverart/image.py @@ -43,7 +43,10 @@ from PyQt5.QtCore import ( from picard import log from picard.config import get_config from picard.const import DEFAULT_COVER_IMAGE_FILENAME -from picard.const.sys import IS_WIN +from picard.const.sys import ( + IS_MACOS, + IS_WIN, +) from picard.coverart.utils import ( Id3ImageType, image_type_as_id3_num, @@ -58,6 +61,10 @@ from picard.util import ( periodictouch, sanitize_filename, ) +from picard.util.filenaming import ( + make_save_path, + make_short_filename, +) from picard.util.scripttofilename import script_to_filename @@ -305,7 +312,7 @@ class CoverArtImage: type = Id3ImageType(type) self._id3_type = type - def _make_image_filename(self, filename, dirname, _metadata): + def _make_image_filename(self, filename, dirname, _metadata, win_compat, win_shorten_path): metadata = Metadata() metadata.copy(_metadata) metadata["coverart_maintype"] = self.maintype @@ -317,8 +324,12 @@ class CoverArtImage: filename = script_to_filename(filename, metadata) if not filename: filename = DEFAULT_COVER_IMAGE_FILENAME - if not is_absolute_path(filename): - filename = os.path.join(dirname, filename) + if is_absolute_path(filename): + dirname = os.path.dirname(filename) + filename = make_short_filename( + dirname, os.path.basename(filename), win_shorten_path=win_shorten_path) + filename = os.path.join(dirname, filename) + filename = make_save_path(filename, win_compat=win_compat, mac_compat=IS_MACOS) return encode_filename(filename) def save(self, dirname, metadata, counters): @@ -332,15 +343,17 @@ class CoverArtImage: if not self.can_be_saved_to_disk: return config = get_config() + win_compat = IS_WIN or config.setting["windows_compatibility"] + win_shorten_path = win_compat and not config.setting['windows_long_paths'] if config.setting["image_type_as_filename"] and not self.is_front_image(): - win_compat = IS_WIN or config.setting["windows_compatibility"] filename = sanitize_filename(self.maintype, win_compat=win_compat) log.debug("Make cover filename from types: %r -> %r", self.types, filename) else: filename = config.setting["cover_image_filename"] log.debug("Using default cover image filename %r", filename) - filename = self._make_image_filename(filename, dirname, metadata) + filename = self._make_image_filename( + filename, dirname, metadata, win_compat, win_shorten_path) overwrite = config.setting["save_images_overwrite"] ext = encode_filename(self.extension) diff --git a/test/test_coverart_image.py b/test/test_coverart_image.py index eeebfad64..b7fcb8d19 100644 --- a/test/test_coverart_image.py +++ b/test/test_coverart_image.py @@ -2,7 +2,7 @@ # # Picard, the next-generation MusicBrainz tagger # -# Copyright (C) 2019, 2021 Philipp Wolfer +# Copyright (C) 2019, 2021-2022 Philipp Wolfer # Copyright (C) 2019-2021 Laurent Monin # # This program is free software; you can redistribute it and/or @@ -20,7 +20,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +from collections import Counter import os.path +from tempfile import TemporaryDirectory import unittest from test.picardtestcase import ( @@ -34,6 +36,8 @@ from picard.coverart.image import ( LocalFileCoverArtImage, ) from picard.coverart.utils import Id3ImageType +from picard.metadata import Metadata +from picard.util import encode_filename def create_image(extra_data, types=None, support_types=False, @@ -155,6 +159,32 @@ class CoverArtImageTest(PicardTestCase): self.assertEqual(filesize, len(imgdata)) self.assertEqual(coverartimage.data, imgdata) + def test_save(self): + self.set_config_values({ + 'image_type_as_filename': True, + 'windows_compatibility': True, + 'windows_long_paths': False, + 'enabled_plugins': [], + 'ascii_filenames': False, + 'save_images_overwrite': False, + }) + metadata = Metadata() + counters = Counter() + with TemporaryDirectory() as d: + image1 = create_image(b'a', types=['back'], support_types=True) + expected_filename = os.path.join(d, 'back.png') + counter_filename = encode_filename(os.path.join(d, 'back')) + image1.save(d, metadata, counters) + self.assertTrue(os.path.exists(expected_filename)) + self.assertEqual(len(image1.data), os.path.getsize(expected_filename)) + self.assertEqual(1, counters[counter_filename]) + image2 = create_image(b'bb', types=['back'], support_types=True) + image2.save(d, metadata, counters) + expected_filename_2 = os.path.join(d, 'back (1).png') + self.assertTrue(os.path.exists(expected_filename_2)) + self.assertEqual(len(image2.data), os.path.getsize(expected_filename_2)) + self.assertEqual(2, counters[counter_filename]) + class LocalFileCoverArtImageTest(PicardTestCase): def test_set_file_url(self):