From 6d27d7b2f3caab7be18342f1fec8ef56c3e1ebc9 Mon Sep 17 00:00:00 2001 From: jecaro Date: Mon, 22 Jun 2026 21:50:10 +0200 Subject: [PATCH] Fail when trying to override an existing file --- lib/Commands.hs | 12 ++++++++++-- tests/Tests/Commands.hs | 19 +++++++++++++++++++ tests/Tests/Common.hs | 23 +++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/lib/Commands.hs b/lib/Commands.hs index fdf891a..3ac2f0e 100644 --- a/lib/Commands.hs +++ b/lib/Commands.hs @@ -7,6 +7,7 @@ module Commands FixFilePathsOptions (..), fixFilePaths, fixFilePaths', + Error (..), errorToText, ) where @@ -27,13 +28,16 @@ import Sound.HTagLib qualified as HTagLib import Sound.HTagLib.Extra qualified as HTagLib import UnliftIO.Exception qualified as Exception -newtype Error = UnableToFormatFile (Path.Path Path.Abs Path.File) - deriving (Show) +data Error + = UnableToFormatFile (Path.Path Path.Abs Path.File) + | TargetFileAlreadyExists (Path.Path Path.Abs Path.File) + deriving (Show, Eq) instance Exception.Exception Error errorToText :: Error -> Text errorToText (UnableToFormatFile file) = "Unable to format file: " <> show file +errorToText (TargetFileAlreadyExists file) = "Target file already exists: " <> show file getTags :: (MonadIO m) => Path.Path Path.Abs Path.File -> m () getTags = putTextLn . AudioTrack.asText <=< AudioTrack.getTags @@ -132,6 +136,10 @@ fixFilePaths' FixFilePathsOptions {..} fromFile = do if toFileAbs == fromFile then pure Nothing else do + whenM (Path.doesFileExist toFileAbs) $ + Exception.throwIO $ + TargetFileAlreadyExists toFileAbs + unless fiDryRun $ do Path.ensureDir $ Path.parent toFileAbs diff --git a/tests/Tests/Commands.hs b/tests/Tests/Commands.hs index 56c59b3..9547e76 100644 --- a/tests/Tests/Commands.hs +++ b/tests/Tests/Commands.hs @@ -17,6 +17,7 @@ import Test.Hspec.Expectations (shouldBe) import Test.Tasty qualified as Tasty import Test.Tasty.HUnit qualified as Tasty import Tests.Common qualified as Common +import UnliftIO.Exception qualified as Exception test :: Tasty.TestTree test = Tasty.testGroup "Commands" [testFixFilePaths] @@ -98,6 +99,10 @@ testFixFilePaths = exists <- Path.doesFileExist cover exists `shouldBe` True, + Tasty.testCase "fails when target already exists" $ + testTargetAlreadyExists False, + Tasty.testCase "fails when target already exists (dry run)" $ + testTargetAlreadyExists True, Tasty.testCase "rename and move the cover image" $ Common.withTenTracksFilesInSubdir [reldir|./input|] $ \dir _ -> do let inputDir = dir [reldir|input|] @@ -124,6 +129,20 @@ testFixFilePaths = newCoverExists `shouldBe` True ] +testTargetAlreadyExists :: Bool -> Tasty.Assertion +testTargetAlreadyExists dryRun = + Common.withOneTrackFile $ \dir file -> do + targetFile <- + Unsafe.fromJust + <$> Commands.fixFilePaths' (fixFilePathsOptions True False dir) file + Path.ensureDir $ Path.parent targetFile + -- Create a dummy file where the file should be moved + System.writeFile (Path.toFilePath targetFile) "" + result <- + Exception.try $ + Commands.fixFilePaths' (fixFilePathsOptions dryRun False dir) file + result `shouldBe` Left (Commands.TargetFileAlreadyExists targetFile) + check :: (MonadIO m) => Path.Path Path.Abs Path.File -> m (Either Track.Error ()) check filename = do track <- AudioTrack.getTags filename diff --git a/tests/Tests/Common.hs b/tests/Tests/Common.hs index 1279815..93033a7 100644 --- a/tests/Tests/Common.hs +++ b/tests/Tests/Common.hs @@ -5,6 +5,7 @@ module Tests.Common tenTracksAlbum', withTenTracksFiles, withTenTracksFilesInSubdir, + withOneTrackFile, ) where @@ -46,6 +47,28 @@ withTenTracksFiles :: withTenTracksFiles withTempDirAndAlbum = withTenTracksFilesInSubdir [reldir|./|] withTempDirAndAlbum +withOneTrackFile :: + (Path.Path Path.Abs Path.Dir -> Path.Path Path.Abs Path.File -> IO ()) -> + Tasty.Assertion +withOneTrackFile action = + Path.withSystemTempDir "htagcli" $ \dir -> do + let file = dir [relfile|1-sample.mp3|] + track = + AudioTrack.AudioTrack + { AudioTrack.atFile = file, + AudioTrack.atTitle = HTagLib.mkTitle "Track 1", + AudioTrack.atArtist = HTagLib.mkArtist "Artist", + AudioTrack.atAlbumArtist = HTagLib.mkAlbumArtist "Album Artist", + AudioTrack.atAlbum = HTagLib.mkAlbum "Album", + AudioTrack.atGenre = HTagLib.mkGenre "Pop", + AudioTrack.atYear = HTagLib.mkYear 2025, + AudioTrack.atTrack = HTagLib.mkTrackNumber 1, + AudioTrack.atDisc = Nothing + } + Path.copyFile [relfile|./data/sample.mp3|] file + AudioTrack.setTags track + action dir file + tenTracksAlbum :: Album.Album tenTracksAlbum = tenTracksAlbum'