From 18d857c4e4d13b1c2be8f5528a78dfb2bba4f5f4 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Fri, 16 Oct 2026 15:51:43 +0100 Subject: [PATCH 1/3] Add file-level validation and duplicate row handling --- .../FileFormatValidationTests.cs | 64 +++++++++++ .../FileProcessorDomainServiceTests.cs | 33 ++++++ .../EventHandling/FileDomainEventHandler.cs | 8 ++ .../FileValidationErrors.cs | 106 ++++++++++++++++++ .../FileFormatHandlers/IFileFormatHandler.cs | 5 +- .../SafaricomFileFormatHandler.cs | 24 +++- .../VoucherFileFormatHandler.cs | 24 +++- .../Services/FileProcessorDomainService.cs | 47 +++++++- .../FileAggregateDomainEvents.cs.cs | 2 + .../FileAggregateTests.cs | 7 +- FileProcessor.FileAggregate/FileAggregate.cs | 28 ++++- 11 files changed, 335 insertions(+), 13 deletions(-) create mode 100644 FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs create mode 100644 FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs diff --git a/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs b/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs new file mode 100644 index 0000000..46e975d --- /dev/null +++ b/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs @@ -0,0 +1,64 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using FileProcessor.BusinessLogic.FileFormatHandlers; +using Shouldly; +using Xunit; + +namespace FileProcessor.BusinessLogic.Tests; + +public class FileFormatValidationTests +{ + [Fact] + public void Safaricom_ValidFile_IsAccepted() + { + SafaricomFileFormatHandler handler = new(); + + FileValidationResult result = handler.ValidateFile(new StringReader("H,20210508,\nD,07777770001,10\nT,1,"), "\n"); + + result.IsValid.ShouldBeTrue(); + result.Errors.ShouldBeEmpty(); + } + + [Theory] + [InlineData("EmptyFile", new string[0], "EmptyFile")] + [InlineData("MissingHeader", new[] { "D,07777770001,10", "T,1," }, "MissingHeader")] + [InlineData("MissingTrailer", new[] { "H,20210508,", "D,07777770001,10" }, "MissingTrailer")] + [InlineData("HeaderNotFirst", new[] { "D,07777770001,10", "H,20210508,", "T,1," }, "HeaderNotFirst")] + [InlineData("TrailerNotLast", new[] { "H,20210508,", "T,1,", "D,07777770001,10" }, "TrailerNotLast")] + [InlineData("TrailerCountMismatch", new[] { "H,20210508,", "D,07777770001,10", "T,2," }, "TrailerDetailCountMismatch")] + public void Safaricom_InvalidFile_ReturnsSpecificError(string scenario, + IReadOnlyList lines, + string expectedCode) + { + _ = scenario; + SafaricomFileFormatHandler handler = new(); + + FileValidationResult result = handler.ValidateFile(new StringReader(string.Join("\n", lines)), "\n"); + + result.IsValid.ShouldBeFalse(); + result.Errors.ShouldContain(error => error.Code == expectedCode); + } + + [Fact] + public void Voucher_MalformedDetail_ReturnsRowValidationError() + { + VoucherFileFormatHandler handler = new(); + + FileValidationResult result = handler.ValidateFile(new StringReader("H,20210508,\nD,Issuer,recipient\nT,1,"), "\n"); + + result.IsValid.ShouldBeFalse(); + result.Errors.OfType().ShouldContain(error => error.LineNumber == 2 && + error.Code == "InvalidFieldCount"); + } + + [Fact] + public void Voucher_TrailerValue_DoesNotRepresentDetailRowCount() + { + VoucherFileFormatHandler handler = new(); + + FileValidationResult result = handler.ValidateFile(new StringReader("H,20210508,\nD,Issuer,07777777775,10\nD,Issuer,testrecipient1@recipient.com,20\nT,1,"), "\n"); + + result.IsValid.ShouldBeTrue(); + } +} diff --git a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs index 3e8ac6c..47e18fb 100644 --- a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs +++ b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs @@ -67,6 +67,8 @@ public FileProcessorDomainServiceTests() this.TransactionProcessorClient = new ITransactionProcessorClientImposter(); this.SecurityServiceClient = new ISecurityServiceClientImposter(); this.FileFormatHandler = new IFileFormatHandlerImposter(); + this.FileFormatHandler.ValidateFile(Arg.Any(), Arg.Any()).Returns( + new FileValidationResult(Guid.Empty, true, Array.Empty())); this.FileSystem = new MockFileSystem(); this.DiagnosticLogger = new CapturingLogger(); @@ -282,6 +284,37 @@ public async Task ProcessUploadedFile_WhenFileExceedsMaximumDetailRecords_Reject this.FileSystem.File.Exists($"{TestData.FileProfileSafaricom.FailedDirectory}/{Path.GetFileName(TestData.FilePathWithName)}").ShouldBeTrue(); } + [Fact] + public async Task ProcessUploadedFile_WhenFileValidationFails_RejectsBeforeAddingLines() + { + this.FileProcessorManager.GetFileProfile(Arg.Any(), Arg.Any()).ReturnsAsync(TestData.FileProfileSafaricom); + this.FileFormatHandler.FileLineCanBeIgnored(Arg.Any()).Returns((String line) => line.StartsWith("H") || line.StartsWith("T")); + this.FileFormatHandler.ValidateFile(Arg.Any(), Arg.Any()).Returns( + new FileValidationResult(Guid.Empty, + false, + new[] { new FileLevelValidationError("InvalidFile", "The file is invalid") })); + + FileAggregate createdAggregate = TestData.GetCreatedFileAggregate(); + this.FileAggregateRepository.GetLatestVersion(Arg.Any(), Arg.Any()) + .ReturnsAsync(Result.Success(TestData.GetEmptyFileAggregate())) + .Then().ReturnsAsync(Result.Success(createdAggregate)); + this.FileAggregateRepository.SaveChanges(Arg.Any(), Arg.Any()).ReturnsAsync(Result.Success()); + this.SecurityServiceClient.GetToken(Arg.Any(), Arg.Any(), Arg.Any()).ReturnsAsync(Result.Success(TestData.TokenResponse())); + this.TransactionProcessorClient.GetOperators(Arg.Any(), Arg.Any(), Arg.Any()).ReturnsAsync(TestData.OperatorList); + + this.FileSystem.AddFile(TestData.FilePathWithName, new MockFileData("H,20210508,\nD,invalid\nT,1,")); + this.FileSystem.AddDirectory(TestData.FileProfileSafaricom.ProcessedDirectory); + this.FileSystem.AddDirectory(TestData.FileProfileSafaricom.FailedDirectory); + + Result result = await this.FileProcessorDomainService.ProcessUploadedFile(TestData.ProcessUploadedFileCommand, CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + createdAggregate.GetFile().IsRejected.ShouldBeTrue(); + createdAggregate.GetFile().RejectionCode.ShouldBe("InvalidFile"); + createdAggregate.GetFile().FileLines.ShouldBeEmpty(); + this.FileSystem.File.Exists($"{TestData.FileProfileSafaricom.FailedDirectory}/{Path.GetFileName(TestData.FilePathWithName)}").ShouldBeTrue(); + } + [Theory] [InlineData("0")] [InlineData("-1")] diff --git a/FileProcessor.BusinessLogic/EventHandling/FileDomainEventHandler.cs b/FileProcessor.BusinessLogic/EventHandling/FileDomainEventHandler.cs index f1a5903..170b19e 100644 --- a/FileProcessor.BusinessLogic/EventHandling/FileDomainEventHandler.cs +++ b/FileProcessor.BusinessLogic/EventHandling/FileDomainEventHandler.cs @@ -93,6 +93,14 @@ private Task HandleSpecificDomainEvent(FileLineTransactionDispatchFailed return Task.FromResult(Result.Success()); } + private Task HandleSpecificDomainEvent(DuplicateFileLineAddedEvent domainEvent, + CancellationToken cancellationToken) + { + _ = domainEvent; + _ = cancellationToken; + return Task.FromResult(Result.Success()); + } + /// /// Handles the specific domain event. /// diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs new file mode 100644 index 0000000..3c65bf6 --- /dev/null +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs @@ -0,0 +1,106 @@ +namespace FileProcessor.BusinessLogic.FileFormatHandlers; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; + +public abstract record FileValidationError(String Code, String Reason); + +public record FileLevelValidationError(String Code, String Reason) + : FileValidationError(Code, Reason); + +public record RowValidationError(Int32 LineNumber, + String Field, + String Code, + String Reason) + : FileValidationError(Code, Reason); + +public record FileValidationResult(Guid FileId, + Boolean IsValid, + IReadOnlyList Errors); + +internal static class FileFormatValidation +{ + public static IReadOnlyList ReadLines(TextReader fileReader, String lineTerminator) + { + return fileReader.ReadToEnd().Split(lineTerminator); + } + + public static FileValidationResult Validate(IReadOnlyList fileLines, + Func isIgnored, + Func expectedFieldCount, + Func parseDetailError, + Boolean validateTrailerDetailCount) + { + List errors = new(); + List<(Int32 Number, String Value)> lines = fileLines + .Select((line, index) => (Number: index + 1, Value: line?.Trim() ?? String.Empty)) + .Where(line => String.IsNullOrWhiteSpace(line.Value) == false) + .ToList(); + + if (lines.Count == 0) + { + errors.Add(new FileLevelValidationError("EmptyFile", "The file contains no non-empty lines")); + return new FileValidationResult(Guid.Empty, false, errors); + } + + if (lines[0].Value.StartsWith("H", StringComparison.Ordinal) == false) + errors.Add(new FileLevelValidationError("MissingHeader", "The first non-empty line must be a header")); + + if (lines.Any(line => line.Value.StartsWith("H", StringComparison.Ordinal)) && + lines[0].Value.StartsWith("H", StringComparison.Ordinal) == false) + errors.Add(new FileLevelValidationError("HeaderNotFirst", "The header must be the first non-empty line")); + + if (lines[^1].Value.StartsWith("T", StringComparison.Ordinal) == false) + errors.Add(new FileLevelValidationError("MissingTrailer", "The last non-empty line must be a trailer")); + + if (lines.Any(line => line.Value.StartsWith("T", StringComparison.Ordinal)) && + lines[^1].Value.StartsWith("T", StringComparison.Ordinal) == false) + errors.Add(new FileLevelValidationError("TrailerNotLast", "The trailer must be the last non-empty line")); + + if (lines.Count(line => line.Value.StartsWith("H", StringComparison.Ordinal)) != 1) + errors.Add(new FileLevelValidationError("InvalidHeaderCount", "The file must contain exactly one header")); + + if (lines.Count(line => line.Value.StartsWith("T", StringComparison.Ordinal)) != 1) + errors.Add(new FileLevelValidationError("InvalidTrailerCount", "The file must contain exactly one trailer")); + + List<(Int32 Number, String Value)> detailLines = lines.Where(line => isIgnored(line.Value) == false).ToList(); + if (detailLines.Count == 0) + errors.Add(new FileLevelValidationError("NoDetailRows", "The file contains no detail rows")); + + foreach ((Int32 number, String value) detailLine in detailLines) + { + String[] fields = detailLine.value.Split(','); + if (fields.Length != expectedFieldCount(detailLine.value)) + { + errors.Add(new RowValidationError(detailLine.number, + "Record", + "InvalidFieldCount", + $"Expected {expectedFieldCount(detailLine.value)} fields but found {fields.Length}")); + continue; + } + + String parseError = parseDetailError(detailLine.value); + if (String.IsNullOrWhiteSpace(parseError) == false) + errors.Add(new RowValidationError(detailLine.number, "Record", "InvalidDetail", parseError)); + } + + if (validateTrailerDetailCount && lines[^1].Value.StartsWith("T,", StringComparison.Ordinal)) + { + String[] trailerFields = lines[^1].Value.Split(','); + if (trailerFields.Length > 1 && Int32.TryParse(trailerFields[1].Trim(), out Int32 expectedCount)) + { + if (expectedCount != detailLines.Count) + errors.Add(new FileLevelValidationError("TrailerDetailCountMismatch", + $"Trailer specifies {expectedCount} detail rows but found {detailLines.Count}")); + } + else + { + errors.Add(new RowValidationError(lines[^1].Number, "DetailCount", "InvalidTrailer", "Trailer detail count is not a valid integer")); + } + } + + return new FileValidationResult(Guid.Empty, errors.Count == 0, errors); + } +} diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/IFileFormatHandler.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/IFileFormatHandler.cs index ef398ac..072c072 100644 --- a/FileProcessor.BusinessLogic/FileFormatHandlers/IFileFormatHandler.cs +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/IFileFormatHandler.cs @@ -2,6 +2,7 @@ { using System; using System.Collections.Generic; + using System.IO; /// /// @@ -21,5 +22,7 @@ public interface IFileFormatHandler /// The file line. /// Boolean FileLineCanBeIgnored(String fileLine); + + FileValidationResult ValidateFile(TextReader fileReader, String lineTerminator); } -} \ No newline at end of file +} diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/SafaricomFileFormatHandler.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/SafaricomFileFormatHandler.cs index 842ba0e..d2fd8ac 100644 --- a/FileProcessor.BusinessLogic/FileFormatHandlers/SafaricomFileFormatHandler.cs +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/SafaricomFileFormatHandler.cs @@ -10,6 +10,28 @@ /// public class SafaricomFileFormatHandler : IFileFormatHandler { + public FileValidationResult ValidateFile(TextReader fileReader, String lineTerminator) + { + return FileFormatValidation.Validate(FileFormatValidation.ReadLines(fileReader, lineTerminator), + this.FileLineCanBeIgnored, + _ => 3, + line => TryParseDetail(line), + true); + } + + private String TryParseDetail(String line) + { + try + { + this.ParseFileLine(line); + return null; + } + catch (InvalidDataException exception) + { + return exception.Message; + } + } + /// /// Parses the file line. /// @@ -61,4 +83,4 @@ public Boolean FileLineCanBeIgnored(String fileLine) return (fileLine.StartsWith("H") || fileLine.StartsWith("T")); } } -} \ No newline at end of file +} diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/VoucherFileFormatHandler.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/VoucherFileFormatHandler.cs index 508660f..1962bde 100644 --- a/FileProcessor.BusinessLogic/FileFormatHandlers/VoucherFileFormatHandler.cs +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/VoucherFileFormatHandler.cs @@ -6,6 +6,28 @@ public class VoucherFileFormatHandler : IFileFormatHandler { + public FileValidationResult ValidateFile(TextReader fileReader, String lineTerminator) + { + return FileFormatValidation.Validate(FileFormatValidation.ReadLines(fileReader, lineTerminator), + this.FileLineCanBeIgnored, + _ => 4, + line => TryParseDetail(line), + false); + } + + private String TryParseDetail(String line) + { + try + { + this.ParseFileLine(line); + return null; + } + catch (InvalidDataException exception) + { + return exception.Message; + } + } + public Dictionary ParseFileLine(String fileLine) { // Format is @@ -58,4 +80,4 @@ public Boolean FileLineCanBeIgnored(String fileLine) return (fileLine.StartsWith("H") || fileLine.StartsWith("T")); } } -} \ No newline at end of file +} diff --git a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs index 956a7f1..2142586 100644 --- a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs +++ b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs @@ -791,13 +791,14 @@ private async Task ProcessFile(Guid fileId, } } - if (String.IsNullOrEmpty(fileContent) == false) { + { String[] fileLines = fileContent.Split(fileProfile.LineTerminator); + IFileFormatHandler fileFormatHandler = this.FileFormatHandlerResolver(fileProfile.FileFormatHandler); Int32 detailRecordCount = fileLines.Count(fileLine => { String trimmedLine = fileLine.Trim(); - return String.IsNullOrEmpty(trimmedLine) == false && this.FileLineCanBeIgnored(trimmedLine, fileProfile.FileFormatHandler) == false; + return String.IsNullOrEmpty(trimmedLine) == false && fileFormatHandler.FileLineCanBeIgnored(trimmedLine) == false; }); if (detailRecordCount > this.MaximumDetailRecordCount) @@ -814,6 +815,39 @@ private async Task ProcessFile(Guid fileId, return Result.Success(); } + FileValidationResult validationResult; + using (StringReader validationReader = new(fileContent)) + validationResult = fileFormatHandler.ValidateFile(validationReader, fileProfile.LineTerminator); + + if (validationResult == null) + { + Result unavailableResult = fileAggregate.RejectFile(detailRecordCount, + this.MaximumDetailRecordCount, + "ValidationUnavailable", + "File validation did not return a result", + DateTime.UtcNow); + if (unavailableResult.IsFailed) + return unavailableResult; + + fileWasRejected = true; + return Result.Success(); + } + + if (validationResult.IsValid == false) + { + String rejectionReason = String.Join("; ", validationResult.Errors.Select(FormatValidationError)); + Result rejectionResult = fileAggregate.RejectFile(detailRecordCount, + this.MaximumDetailRecordCount, + validationResult.Errors.First().Code, + rejectionReason, + DateTime.UtcNow); + if (rejectionResult.IsFailed) + return rejectionResult; + + fileWasRejected = true; + return Result.Success(); + } + foreach (String fileLine in fileLines) { Result stateResult = fileAggregate.AddFileLine(fileLine.Trim()); if (stateResult.IsFailed) @@ -859,6 +893,15 @@ private Boolean FileLineCanBeIgnored(String domainEventFileLine, return fileFormatHandler.FileLineCanBeIgnored(domainEventFileLine); } + private static String FormatValidationError(FileValidationError error) + { + return error switch + { + RowValidationError rowError => $"Line {rowError.LineNumber} Field {rowError.Field}: {rowError.Code} - {rowError.Reason}", + _ => $"{error.Code} - {error.Reason}" + }; + } + private Dictionary ParseFileLine(String domainEventFileLine, String fileProfileFileFormatHandler) { diff --git a/FileProcessor.File.DomainEvents/FileAggregateDomainEvents.cs.cs b/FileProcessor.File.DomainEvents/FileAggregateDomainEvents.cs.cs index b2900a6..ea89a7a 100644 --- a/FileProcessor.File.DomainEvents/FileAggregateDomainEvents.cs.cs +++ b/FileProcessor.File.DomainEvents/FileAggregateDomainEvents.cs.cs @@ -4,6 +4,8 @@ using Shared.DomainDrivenDesign.EventSourcing; public record FileLineAddedEvent(Guid FileId, Guid EstateId, Guid MerchantId, Int32 LineNumber, String FileLine) : DomainEvent(FileId, Guid.NewGuid()); + + public record DuplicateFileLineAddedEvent(Guid FileId, Guid EstateId, Guid MerchantId, Int32 LineNumber, String FileLine) : DomainEvent(FileId, Guid.NewGuid()); public record FileProcessingCompletedEvent(Guid FileId, Guid EstateId, Guid MerchantId, DateTime ProcessingCompletedDateTime) : DomainEvent(FileId, Guid.NewGuid()); diff --git a/FileProcessor.FileAggregate.Tests/FileAggregateTests.cs b/FileProcessor.FileAggregate.Tests/FileAggregateTests.cs index d27f96e..0f4d06e 100644 --- a/FileProcessor.FileAggregate.Tests/FileAggregateTests.cs +++ b/FileProcessor.FileAggregate.Tests/FileAggregateTests.cs @@ -152,7 +152,7 @@ public void FileAggregate_PlayFileLineAddedEvent_PreservesEventIdOnFileLine() } [Fact] - public void FileAggregate_AddFileLine_AddDuplicateLine_FileLineIsNotAddedAdded() + public void FileAggregate_AddFileLine_AddDuplicateLine_PreservesBothLines() { FileAggregate fileAggregate = FileAggregate.Create(TestData.FileId); FileAggregateExtensions.CreateFileRequest createFileRequest = new(TestData.FileImportLogId, TestData.EstateId, TestData.MerchantId, TestData.UserId, TestData.FileProfileId, TestData.FileLocation, TestData.FileUploadedDateTime, TestData.SafaricomOperatorId); @@ -164,8 +164,9 @@ public void FileAggregate_AddFileLine_AddDuplicateLine_FileLineIsNotAddedAdded() FileDetails fileDetails = fileAggregate.GetFile(); fileDetails.FileLines.ShouldNotBeNull(); fileDetails.FileLines.ShouldNotBeEmpty(); - fileDetails.FileLines.ShouldHaveSingleItem(); - fileDetails.FileLines.Count.ShouldBe(1); + fileDetails.FileLines.Count.ShouldBe(2); + fileDetails.FileLines[0].LineNumber.ShouldBe(1); + fileDetails.FileLines[1].LineNumber.ShouldBe(2); } [Fact] diff --git a/FileProcessor.FileAggregate/FileAggregate.cs b/FileProcessor.FileAggregate/FileAggregate.cs index 5c7b96d..b0b7097 100644 --- a/FileProcessor.FileAggregate/FileAggregate.cs +++ b/FileProcessor.FileAggregate/FileAggregate.cs @@ -70,6 +70,19 @@ public static void PlayEvent(this FileAggregate aggregate, FileLineAddedEvent do }); } + public static void PlayEvent(this FileAggregate aggregate, DuplicateFileLineAddedEvent domainEvent) + { + aggregate.FileLines.Add(new FileLine + { + LineData = domainEvent.FileLine, + LineNumber = domainEvent.LineNumber, + ProcessingResult = ProcessingResult.Ignored, + RejectedReason = "Duplicate detail row", + FailedDispatchAttempts = new List(), + EventId = domainEvent.EventId + }); + } + public static void PlayEvent(this FileAggregate aggregate, FileLineProcessingIgnoredEvent domainEvent) { // find the line @@ -180,13 +193,18 @@ public static Result AddFileLine(this FileAggregate aggregate, String fileLine) if (aggregate.IsRejected) return Result.Invalid($"File Id {aggregate.AggregateId} has been rejected"); - Boolean lineAlreadyExists = aggregate.FileLines.Any(f => f.LineData == fileLine); + Int32 lineNumber = aggregate.FileLines.Count + 1; - // We already have this line so just return - if (lineAlreadyExists) + if (aggregate.FileLines.Any(f => f.LineData == fileLine)) + { + DuplicateFileLineAddedEvent duplicateFileLineAddedEvent = new(aggregate.AggregateId, + aggregate.EstateId, + aggregate.MerchantId, + lineNumber, + fileLine); + aggregate.ApplyAndAppend(duplicateFileLineAddedEvent); return Result.Success(); - - Int32 lineNumber = aggregate.FileLines.Count + 1; + } FileLineAddedEvent fileLineAddedEvent = new(aggregate.AggregateId, aggregate.EstateId, aggregate.MerchantId, lineNumber, fileLine); aggregate.ApplyAndAppend(fileLineAddedEvent); From c11db4b7c2b42e3934f2fef24b922aad19c2dbbb Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Fri, 16 Oct 2026 16:08:21 +0100 Subject: [PATCH 2/3] Address PR review feedback --- .../FileFormatValidationTests.cs | 14 +++++++++++ .../FileProcessorDomainServiceTests.cs | 24 +++++++++++++++++++ .../FileValidationErrors.cs | 2 +- .../Services/FileProcessorDomainService.cs | 10 +++++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs b/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs index 46e975d..22f09f3 100644 --- a/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs +++ b/FileProcessor.BusinessLogic.Tests/FileFormatValidationTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -61,4 +62,17 @@ public void Voucher_TrailerValue_DoesNotRepresentDetailRowCount() result.IsValid.ShouldBeTrue(); } + + [Theory] + [InlineData("T")] + [InlineData("Trailer")] + public void Safaricom_TrailerWithoutDetailCount_IsRejected(String trailer) + { + SafaricomFileFormatHandler handler = new(); + + FileValidationResult result = handler.ValidateFile(new StringReader($"H,20210508,\nD,07777770001,10\n{trailer}"), "\n"); + + result.IsValid.ShouldBeFalse(); + result.Errors.ShouldContain(error => error.Code == "InvalidTrailer"); + } } diff --git a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs index 47e18fb..406e8d1 100644 --- a/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs +++ b/FileProcessor.BusinessLogic.Tests/FileProcessorDomainServiceTests.cs @@ -315,6 +315,30 @@ public async Task ProcessUploadedFile_WhenFileValidationFails_RejectsBeforeAddin this.FileSystem.File.Exists($"{TestData.FileProfileSafaricom.FailedDirectory}/{Path.GetFileName(TestData.FilePathWithName)}").ShouldBeTrue(); } + [Fact] + public async Task ProcessUploadedFile_WhenReplayed_DoesNotAppendPersistedLines() + { + this.FileProcessorManager.GetFileProfile(Arg.Any(), Arg.Any()).ReturnsAsync(TestData.FileProfileSafaricom); + this.FileFormatHandler.FileLineCanBeIgnored(Arg.Any()).Returns(false); + + FileAggregate createdAggregate = TestData.GetCreatedFileAggregate(); + createdAggregate.AddFileLine("D,1,1,1"); + this.FileAggregateRepository.GetLatestVersion(Arg.Any(), Arg.Any()) + .ReturnsAsync(Result.Success(createdAggregate)); + this.FileAggregateRepository.SaveChanges(Arg.Any(), Arg.Any()).ReturnsAsync(Result.Success()); + this.SecurityServiceClient.GetToken(Arg.Any(), Arg.Any(), Arg.Any()).ReturnsAsync(Result.Success(TestData.TokenResponse())); + this.TransactionProcessorClient.GetOperators(Arg.Any(), Arg.Any(), Arg.Any()).ReturnsAsync(TestData.OperatorList); + + this.FileSystem.AddFile(TestData.FilePathWithName, new MockFileData("D,1,1,1")); + this.FileSystem.AddDirectory(TestData.FileProfileSafaricom.ProcessedDirectory); + this.FileSystem.AddDirectory(TestData.FileProfileSafaricom.FailedDirectory); + + Result result = await this.FileProcessorDomainService.ProcessUploadedFile(TestData.ProcessUploadedFileCommand, CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + createdAggregate.GetFile().FileLines.Count.ShouldBe(1); + } + [Theory] [InlineData("0")] [InlineData("-1")] diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs index 3c65bf6..84665f0 100644 --- a/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs @@ -86,7 +86,7 @@ public static FileValidationResult Validate(IReadOnlyList fileLines, errors.Add(new RowValidationError(detailLine.number, "Record", "InvalidDetail", parseError)); } - if (validateTrailerDetailCount && lines[^1].Value.StartsWith("T,", StringComparison.Ordinal)) + if (validateTrailerDetailCount && lines[^1].Value.StartsWith("T", StringComparison.Ordinal)) { String[] trailerFields = lines[^1].Value.Split(','); if (trailerFields.Length > 1 && Int32.TryParse(trailerFields[1].Trim(), out Int32 expectedCount)) diff --git a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs index 2142586..cf29e66 100644 --- a/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs +++ b/FileProcessor.BusinessLogic/Services/FileProcessorDomainService.cs @@ -848,8 +848,16 @@ private async Task ProcessFile(Guid fileId, return Result.Success(); } + HashSet previouslyPersistedLines = fileAggregate.GetFile().FileLines + .Select(fileLine => fileLine.LineData) + .ToHashSet(StringComparer.Ordinal); + foreach (String fileLine in fileLines) { - Result stateResult = fileAggregate.AddFileLine(fileLine.Trim()); + String trimmedFileLine = fileLine.Trim(); + if (previouslyPersistedLines.Contains(trimmedFileLine)) + continue; + + Result stateResult = fileAggregate.AddFileLine(trimmedFileLine); if (stateResult.IsFailed) return stateResult; } From a38520dd04b636921e9fecbf37ff083bfd102a0d Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Fri, 16 Oct 2026 16:11:21 +0100 Subject: [PATCH 3/3] Reduce validation complexity --- .../FileValidationErrors.cs | 83 +++++++++++++------ 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs index 84665f0..1b3d014 100644 --- a/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs +++ b/FileProcessor.BusinessLogic/FileFormatHandlers/FileValidationErrors.cs @@ -33,30 +33,40 @@ public static FileValidationResult Validate(IReadOnlyList fileLines, Func parseDetailError, Boolean validateTrailerDetailCount) { - List errors = new(); List<(Int32 Number, String Value)> lines = fileLines .Select((line, index) => (Number: index + 1, Value: line?.Trim() ?? String.Empty)) .Where(line => String.IsNullOrWhiteSpace(line.Value) == false) .ToList(); if (lines.Count == 0) - { - errors.Add(new FileLevelValidationError("EmptyFile", "The file contains no non-empty lines")); - return new FileValidationResult(Guid.Empty, false, errors); - } + return new FileValidationResult(Guid.Empty, + false, + new[] { new FileLevelValidationError("EmptyFile", "The file contains no non-empty lines") }); + + List<(Int32 Number, String Value)> detailLines = lines.Where(line => isIgnored(line.Value) == false).ToList(); + List errors = ValidateStructure(lines); + errors.AddRange(ValidateDetailLines(detailLines, expectedFieldCount, parseDetailError)); + errors.AddRange(ValidateTrailerDetailCount(lines, detailLines.Count, validateTrailerDetailCount)); + + return new FileValidationResult(Guid.Empty, errors.Count == 0, errors); + } + + private static List ValidateStructure(IReadOnlyList<(Int32 Number, String Value)> lines) + { + List errors = new(); + Boolean firstLineIsHeader = lines[0].Value.StartsWith("H", StringComparison.Ordinal); + Boolean lastLineIsTrailer = lines[^1].Value.StartsWith("T", StringComparison.Ordinal); - if (lines[0].Value.StartsWith("H", StringComparison.Ordinal) == false) + if (firstLineIsHeader == false) errors.Add(new FileLevelValidationError("MissingHeader", "The first non-empty line must be a header")); - if (lines.Any(line => line.Value.StartsWith("H", StringComparison.Ordinal)) && - lines[0].Value.StartsWith("H", StringComparison.Ordinal) == false) + if (lines.Any(line => line.Value.StartsWith("H", StringComparison.Ordinal)) && firstLineIsHeader == false) errors.Add(new FileLevelValidationError("HeaderNotFirst", "The header must be the first non-empty line")); - if (lines[^1].Value.StartsWith("T", StringComparison.Ordinal) == false) + if (lastLineIsTrailer == false) errors.Add(new FileLevelValidationError("MissingTrailer", "The last non-empty line must be a trailer")); - if (lines.Any(line => line.Value.StartsWith("T", StringComparison.Ordinal)) && - lines[^1].Value.StartsWith("T", StringComparison.Ordinal) == false) + if (lines.Any(line => line.Value.StartsWith("T", StringComparison.Ordinal)) && lastLineIsTrailer == false) errors.Add(new FileLevelValidationError("TrailerNotLast", "The trailer must be the last non-empty line")); if (lines.Count(line => line.Value.StartsWith("H", StringComparison.Ordinal)) != 1) @@ -65,19 +75,27 @@ public static FileValidationResult Validate(IReadOnlyList fileLines, if (lines.Count(line => line.Value.StartsWith("T", StringComparison.Ordinal)) != 1) errors.Add(new FileLevelValidationError("InvalidTrailerCount", "The file must contain exactly one trailer")); - List<(Int32 Number, String Value)> detailLines = lines.Where(line => isIgnored(line.Value) == false).ToList(); + return errors; + } + + private static List ValidateDetailLines(IReadOnlyList<(Int32 Number, String Value)> detailLines, + Func expectedFieldCount, + Func parseDetailError) + { + List errors = new(); if (detailLines.Count == 0) errors.Add(new FileLevelValidationError("NoDetailRows", "The file contains no detail rows")); foreach ((Int32 number, String value) detailLine in detailLines) { String[] fields = detailLine.value.Split(','); - if (fields.Length != expectedFieldCount(detailLine.value)) + Int32 expectedCount = expectedFieldCount(detailLine.value); + if (fields.Length != expectedCount) { errors.Add(new RowValidationError(detailLine.number, "Record", "InvalidFieldCount", - $"Expected {expectedFieldCount(detailLine.value)} fields but found {fields.Length}")); + $"Expected {expectedCount} fields but found {fields.Length}")); continue; } @@ -86,21 +104,32 @@ public static FileValidationResult Validate(IReadOnlyList fileLines, errors.Add(new RowValidationError(detailLine.number, "Record", "InvalidDetail", parseError)); } - if (validateTrailerDetailCount && lines[^1].Value.StartsWith("T", StringComparison.Ordinal)) + return errors; + } + + private static List ValidateTrailerDetailCount(IReadOnlyList<(Int32 Number, String Value)> lines, + Int32 detailLineCount, + Boolean validateTrailerDetailCount) + { + List errors = new(); + if (validateTrailerDetailCount == false || lines[^1].Value.StartsWith("T", StringComparison.Ordinal) == false) + return errors; + + String[] trailerFields = lines[^1].Value.Split(','); + if (trailerFields.Length > 1 && Int32.TryParse(trailerFields[1].Trim(), out Int32 expectedCount)) { - String[] trailerFields = lines[^1].Value.Split(','); - if (trailerFields.Length > 1 && Int32.TryParse(trailerFields[1].Trim(), out Int32 expectedCount)) - { - if (expectedCount != detailLines.Count) - errors.Add(new FileLevelValidationError("TrailerDetailCountMismatch", - $"Trailer specifies {expectedCount} detail rows but found {detailLines.Count}")); - } - else - { - errors.Add(new RowValidationError(lines[^1].Number, "DetailCount", "InvalidTrailer", "Trailer detail count is not a valid integer")); - } + if (expectedCount != detailLineCount) + errors.Add(new FileLevelValidationError("TrailerDetailCountMismatch", + $"Trailer specifies {expectedCount} detail rows but found {detailLineCount}")); + } + else + { + errors.Add(new RowValidationError(lines[^1].Number, + "DetailCount", + "InvalidTrailer", + "Trailer detail count is not a valid integer")); } - return new FileValidationResult(Guid.Empty, errors.Count == 0, errors); + return errors; } }