PICARD-2565: Apply make_short_filename and make_save_path to saved image files

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)

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):