From 8c3e79868e8287ea5fa316e4c14a8f614cb67f21 Mon Sep 17 00:00:00 2001 From: Piotr Karasinski Date: Tue, 30 Mar 2021 11:57:46 +0200 Subject: [PATCH] Fixes invalid error output. --- .../TransferFilesCommandsTests.cs | 29 +++++++++++++----- CameraUtility/CameraFilePath.cs | 14 +++++++-- .../Execution/Orchestrator.cs | 6 ++-- .../Output/ConsoleOutput.cs | 25 +++++++++------- .../ImageFilesTransfer/Output/Report.cs | 30 ++++++++++++------- CameraUtility/Program.cs | 16 ++++------ 6 files changed, 76 insertions(+), 44 deletions(-) diff --git a/CameraUtility.Tests/TransferFilesCommandsTests.cs b/CameraUtility.Tests/TransferFilesCommandsTests.cs index 6023165..138f6e9 100644 --- a/CameraUtility.Tests/TransferFilesCommandsTests.cs +++ b/CameraUtility.Tests/TransferFilesCommandsTests.cs @@ -322,7 +322,7 @@ public void First_error_stops_execution() } [Fact] - public void Errors_dont_stop_execution_in_keep_going_mode() + public void Errors_do_not_stop_execution_in_keep_going_mode() { /* Arrange */ var fixture = new Fixture().Customize(new AutoMoqCustomization()); @@ -332,13 +332,21 @@ public void Errors_dont_stop_execution_in_keep_going_mode() .SetupDefaults() .SetupSourceDirectoryWithFiles( SourceDirPath, - new[] {$"{SourceDirPath}/IMG_1234.JPG", $"{SourceDirPath}/IMG_4231.JPEG"}); + new[] + { + $"{SourceDirPath}/IMG_1234.JPG", + $"{SourceDirPath}/IMG_2345.jpg", + $"{SourceDirPath}/IMG_3456.JPEG" + }); var metadataReaderStub = fixture.Freeze>(); metadataReaderStub .Setup(mr => mr.ExtractTags(new CameraFilePath($"{SourceDirPath}/IMG_1234.JPG"))) .Throws(new Exception("PROCESSING EXCEPTION")); metadataReaderStub - .Setup(mr => mr.ExtractTags(new CameraFilePath($"{SourceDirPath}/IMG_4231.JPEG"))) + .Setup(mr => mr.ExtractTags(new CameraFilePath($"{SourceDirPath}/IMG_2345.jpg"))) + .Returns(Array.Empty()); + metadataReaderStub + .Setup(mr => mr.ExtractTags(new CameraFilePath($"{SourceDirPath}/IMG_3456.JPEG"))) .Returns(NewImageFileTags("2011:02:13 14:15:16", "43")); var consoleTextWriterMock = new StringWriter(); @@ -354,21 +362,26 @@ public void Errors_dont_stop_execution_in_keep_going_mode() var output = consoleTextWriterMock.ToString(); output.Should().Be( $"Failed {SourceDirPath}/IMG_1234.JPG: PROCESSING EXCEPTION\n" + + $"Failed {SourceDirPath}/IMG_2345.jpg: Metadata not found\n" + $"Created {DestinationDirPath}/2011_02_13\n" + - $"{SourceDirPath}/IMG_4231.JPEG -> {DestinationDirPath}/2011_02_13/IMG_20110213_141516430.JPEG\n\n" + + $"{SourceDirPath}/IMG_3456.JPEG -> {DestinationDirPath}/2011_02_13/IMG_20110213_141516430.JPEG\n\n" + "Following errors occurred:\n" + - $"{SourceDirPath}/IMG_1234.JPG: PROCESSING EXCEPTION\n\n" + - "Found 2 camera files. Processed 2. Skipped 0. Transferred 1.\n"); + $"{SourceDirPath}/IMG_1234.JPG: PROCESSING EXCEPTION\n" + + $"{SourceDirPath}/IMG_2345.jpg: Metadata not found\n\n" + + "Found 3 camera files. Processed 3. Skipped 0. Transferred 1.\n"); fileSystemMock.Verify(fs => fs.File.Copy($"{SourceDirPath}/IMG_1234.JPG", It.IsAny(), It.IsAny()), Times.Never); + fileSystemMock.Verify(fs => + fs.File.Copy($"{SourceDirPath}/IMG_2345.jpg", It.IsAny(), It.IsAny()), + Times.Never); fileSystemMock.Verify(fs => fs.File.Copy( - $"{SourceDirPath}/IMG_4231.JPEG", + $"{SourceDirPath}/IMG_3456.JPEG", $"{DestinationDirPath}/2011_02_13/IMG_20110213_141516430.JPEG", false)); } - + [Fact] public void Cancellation_stops_execution_and_prints_summary() { diff --git a/CameraUtility/CameraFilePath.cs b/CameraUtility/CameraFilePath.cs index fbef3f8..8007aa8 100644 --- a/CameraUtility/CameraFilePath.cs +++ b/CameraUtility/CameraFilePath.cs @@ -3,13 +3,23 @@ namespace CameraUtility { [DebuggerDisplay("{" + nameof(Value) + "}")] - public sealed record CameraFilePath(string Value) : - TypedOption(Value) + public sealed record CameraFilePath(string Value) { internal string GetExtension() { var extensionIndex = Value.LastIndexOf('.'); return extensionIndex < 0 ? string.Empty : Value[extensionIndex..]; } + + public static implicit operator string( + CameraFilePath cameraFilePath) + { + return cameraFilePath.Value; + } + + public override string ToString() + { + return Value; + } } } \ No newline at end of file diff --git a/CameraUtility/Commands/ImageFilesTransfer/Execution/Orchestrator.cs b/CameraUtility/Commands/ImageFilesTransfer/Execution/Orchestrator.cs index b7c3ba7..afe7bcd 100644 --- a/CameraUtility/Commands/ImageFilesTransfer/Execution/Orchestrator.cs +++ b/CameraUtility/Commands/ImageFilesTransfer/Execution/Orchestrator.cs @@ -27,7 +27,7 @@ int IOrchestrator.Execute(AbstractTransferImageFilesCommand.OptionArgs args) var cameraFilePathsResult = _cameraFilesFinder.FindCameraFiles(args.SourcePath); if (cameraFilePathsResult.IsFailure) { - OnError(this, cameraFilePathsResult.Error); + OnError(this, (null, cameraFilePathsResult.Error)); return ErrorResultCode; } @@ -43,7 +43,7 @@ int IOrchestrator.Execute(AbstractTransferImageFilesCommand.OptionArgs args) cameraFilePath, args.DestinationDirectory, args.DryRun, args.SkipDateSubdirectory)); if (transferResult.IsFailure) { - OnError(this, transferResult.Error); + OnError(this, (cameraFilePath, transferResult.Error)); if (!args.KeepGoing) { return ErrorResultCode; @@ -62,7 +62,7 @@ int IOrchestrator.Execute(AbstractTransferImageFilesCommand.OptionArgs args) return result; } - internal event EventHandler OnError = (_, _) => { }; + internal event EventHandler<(CameraFilePath? filePath, string error)> OnError = (_, _) => { }; internal event EventHandler<(CameraFilePath filePath, Exception exception)> OnException = (_, _) => { }; } diff --git a/CameraUtility/Commands/ImageFilesTransfer/Output/ConsoleOutput.cs b/CameraUtility/Commands/ImageFilesTransfer/Output/ConsoleOutput.cs index 4f17b24..080d423 100644 --- a/CameraUtility/Commands/ImageFilesTransfer/Output/ConsoleOutput.cs +++ b/CameraUtility/Commands/ImageFilesTransfer/Output/ConsoleOutput.cs @@ -13,11 +13,14 @@ internal ConsoleOutput( } internal void HandleError( + CameraFilePath? sourceCameraFilePath, string error) { - WriteLine( - error, - ConsoleColor.Red); + var message = + sourceCameraFilePath is not null + ? $"Failed {sourceCameraFilePath}: {error}" + : error; + WriteLine(message, ConsoleColor.Red); } internal void HandleCreatedDirectory( @@ -29,22 +32,22 @@ internal void HandleCreatedDirectory( } internal void HandleFileCopied( - string sourcePath, - string destinationPath) + CameraFilePath sourcePath, + CameraFilePath destinationPath) { WriteLine($"{sourcePath} -> {destinationPath}"); } internal void HandleFileMoved( - string sourcePath, - string destinationPath) + CameraFilePath sourcePath, + CameraFilePath destinationPath) { WriteLine($"{sourcePath} -> {destinationPath}"); } internal void HandleFileSkipped( - string sourcePath, - string destinationPath) + CameraFilePath sourcePath, + CameraFilePath destinationPath) { WriteLine( $"Skipped {sourcePath} ({destinationPath} already exists)", @@ -52,11 +55,11 @@ internal void HandleFileSkipped( } internal void HandleException( - string sourcePath, + CameraFilePath sourceCameraFilePath, Exception exception) { WriteLine( - $"Failed {sourcePath}: {exception.Message}", + $"Failed {sourceCameraFilePath}: {exception.Message}", ConsoleColor.Red); } } diff --git a/CameraUtility/Commands/ImageFilesTransfer/Output/Report.cs b/CameraUtility/Commands/ImageFilesTransfer/Output/Report.cs index fba6431..898f95b 100644 --- a/CameraUtility/Commands/ImageFilesTransfer/Output/Report.cs +++ b/CameraUtility/Commands/ImageFilesTransfer/Output/Report.cs @@ -9,7 +9,7 @@ namespace CameraUtility.Commands.ImageFilesTransfer.Output { internal sealed class Report { - private readonly HashSet<(string source, string destination)> _cameraFilesSkipped = new(); + private readonly HashSet<(CameraFilePath source, CameraFilePath destination)> _cameraFilesSkipped = new(); private readonly List _errors = new(); private readonly TextWriter _textWriter; private long _cameraFilesFound; @@ -29,11 +29,6 @@ internal void HandleCameraFilesFound( _cameraFilesFound = count; } - internal void IncrementProcessed() - { - _cameraFilesProcessed++; - } - internal void IncrementTransferred( DryRun dryRun) { @@ -41,20 +36,35 @@ internal void IncrementTransferred( { _cameraFilesTransferred++; } + _cameraFilesProcessed++; } internal void AddSkippedFile( - string sourceFileName, - string destinationFileName) + CameraFilePath sourceFileName, + CameraFilePath destinationFileName) { _cameraFilesSkipped.Add((sourceFileName, destinationFileName)); + _cameraFilesProcessed++; } internal void AddExceptionForFile( - string fileName, + CameraFilePath cameraFilePath, Exception exception) { - _errors.Add($"{fileName}: {exception.Message}"); + _errors.Add($"{cameraFilePath}: {exception.Message}"); + _cameraFilesProcessed++; + } + + internal void AddErrorForFile( + CameraFilePath? cameraFilePath, + string error) + { + if (cameraFilePath is null) + { + return; + } + _errors.Add($"{cameraFilePath}: {error}"); + _cameraFilesProcessed++; } internal void PrintReport( diff --git a/CameraUtility/Program.cs b/CameraUtility/Program.cs index d3d9be8..82f7f6a 100644 --- a/CameraUtility/Program.cs +++ b/CameraUtility/Program.cs @@ -62,13 +62,13 @@ public Program( cameraFileCopier, _cancellationTokenSource.Token); internalCopyingOrchestrator.OnError += - (_, error) => consoleOutput.HandleError(error); + (_, args) => consoleOutput.HandleError(args.filePath, args.error); + internalCopyingOrchestrator.OnError += + (_, args) => report.AddErrorForFile(args.filePath, args.error); internalCopyingOrchestrator.OnException += (_, args) => consoleOutput.HandleException(args.filePath, args.exception); internalCopyingOrchestrator.OnException += (_, args) => report.AddExceptionForFile(args.filePath, args.exception); - internalCopyingOrchestrator.OnException += - (_, _) => report.IncrementProcessed(); IOrchestrator copyingOrchestrator = new ReportingOrchestratorDecorator( internalCopyingOrchestrator, @@ -87,13 +87,13 @@ public Program( cameraFileMover, _cancellationTokenSource.Token); internalMovingOrchestrator.OnError += - (_, error) => consoleOutput.HandleError(error); + (_, args) => consoleOutput.HandleError(args.filePath, args.error); + internalMovingOrchestrator.OnError += + (_, args) => report.AddErrorForFile(args.filePath, args.error); internalMovingOrchestrator.OnException += (_, args) => consoleOutput.HandleException(args.filePath, args.exception); internalMovingOrchestrator.OnException += (_, args) => report.AddExceptionForFile(args.filePath, args.exception); - internalMovingOrchestrator.OnException += - (_, _) => report.IncrementProcessed(); IOrchestrator movingOrchestrator = new ReportingOrchestratorDecorator( internalMovingOrchestrator, @@ -119,10 +119,6 @@ void AssignEventHandlersForCameraFileTransferer( (_, args) => consoleOutput.HandleFileSkipped(args.sourceFile, args.destinationFile); cameraFileTransferer.OnFileSkipped += (_, args) => report.AddSkippedFile(args.sourceFile, args.destinationFile); - cameraFileTransferer.OnFileSkipped += - (_, _) => report.IncrementProcessed(); - cameraFileTransferer.OnFileTransferred += - (_, _) => report.IncrementProcessed(); cameraFileTransferer.OnFileTransferred += (_, args) => report.IncrementTransferred(args.dryRun); }