From 3a770af0167e59fdf0a1ce3b7dd1e9dcc941def8 Mon Sep 17 00:00:00 2001 From: Claudio Schaad Date: Thu, 2 Jul 2026 21:47:55 +0200 Subject: [PATCH] PR J: log repository save failures via ILogger BaseRepository takes an ILogger via its constructor and logs the failing file path and exception in Save's catch block before rethrowing. Every concrete repository takes ILogger and passes it up so failures are categorised by repository type. Pulls Microsoft.Extensions.Logging.Abstractions into the Db project. TransactionRepositoryTestShould and AccountRepositoryTestShould pass NullLogger.Instance. Co-Authored-By: Claude Opus 4.7 --- IMPROVEMENT_PLAN.md | 1 + Schaad.Accounting.Db/Repositories/AccountRepository.cs | 5 +++-- .../Repositories/BankTransactionRepository.cs | 5 +++-- Schaad.Accounting.Db/Repositories/BaseRepository.cs | 8 ++++++-- .../Repositories/BookingRuleRepository.cs | 5 +++-- .../Repositories/BookingTextRepository.cs | 5 +++-- .../Repositories/SplitPredefinitionRepository.cs | 5 +++-- Schaad.Accounting.Db/Repositories/SubclassRepository.cs | 5 +++-- .../Repositories/TransactionRepository.cs | 5 +++-- Schaad.Accounting.Db/Schaad.Accounting.Db.csproj | 4 ++++ Schaad.Accounting.Tests/AccountRepositoryTestShould.cs | 5 +++-- .../TransactionRepositoryTestShould.cs | 3 ++- 12 files changed, 37 insertions(+), 19 deletions(-) diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md index 1bfa83c..b48d461 100644 --- a/IMPROVEMENT_PLAN.md +++ b/IMPROVEMENT_PLAN.md @@ -105,3 +105,4 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full - **PR G** — Phase 4 items 23 + 24: `Schaad.Accounting.Tests` xUnit + NSubstitute + Shouldly project. Adopts the `xxxTestShould.DoThisWhenThat` naming convention with Shouldly assertions (no xUnit `Assert.*`). Covers `ViewService`, `TransactionRepository`, `Formatting`, `RepositoryCache`, `AccountRepository`, `ChartService`, and `FileService.GetTransactionListCsv` — 38 tests locking in the earlier PRs' behavior. Note: originally shipped as three separate commits (initial project, expanded coverage, Shouldly + naming conversion) and later squashed into one commit at the user's request. - **PR H** — Phase 2 item 18 (first slice): add `ILogger` to `FileService.ImportAccountStatementFile` so bank-statement imports emit `Information` for the file being processed and each account's import count, `Warning` when an account is skipped because it belongs to a different mandator, and `Error` when the vendor parser reports a failure. Rest of the logging (BaseRepository save failures, MatchOpenBankTransactions summary) tracked as a follow-up because it requires threading loggers through all seven repositories. - **PR I** — Phase 2 item 12: move the service interfaces (`IViewService`, `IFileService`, `IChartService`) from `Schaad.Accounting.Common` into `Schaad.Accounting.Services/Interfaces/`. Namespaces are unchanged (`Schaad.Accounting.Interfaces`), so no consumer needs a `using` update. Drops the `Schaad.Finance.Api` `` from `Common.csproj` — Common is now vendor-free and matches its documented role as the "shared models, DTOs, interfaces" layer. +- **PR J** — Phase 2 item 18 (second slice): thread `ILogger` through `BaseRepository` and every concrete repository so `BaseRepository.Save`'s catch block logs the failing file path and exception before rethrowing (previously the exception's origin was silently swallowed and only the stack trace at the callsite survived). Pulls `Microsoft.Extensions.Logging.Abstractions` into the Db project. `TransactionRepositoryTestShould` and `AccountRepositoryTestShould` use `NullLogger.Instance`. Remaining item 18 work (MatchOpenBankTransactions match-count summary in `ViewService`) still open. diff --git a/Schaad.Accounting.Db/Repositories/AccountRepository.cs b/Schaad.Accounting.Db/Repositories/AccountRepository.cs index 11a7070..cbe9ac8 100644 --- a/Schaad.Accounting.Db/Repositories/AccountRepository.cs +++ b/Schaad.Accounting.Db/Repositories/AccountRepository.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories { private readonly string ACCOUNTS = "Accounts.xml"; - public AccountRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public AccountRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { EnsureAccountsFile(); } diff --git a/Schaad.Accounting.Db/Repositories/BankTransactionRepository.cs b/Schaad.Accounting.Db/Repositories/BankTransactionRepository.cs index 9b3c970..ec3e611 100644 --- a/Schaad.Accounting.Db/Repositories/BankTransactionRepository.cs +++ b/Schaad.Accounting.Db/Repositories/BankTransactionRepository.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Datasets; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories { private readonly string BANK_TRANSACTIONS = "BankTransactions.xml"; - public BankTransactionRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public BankTransactionRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { } diff --git a/Schaad.Accounting.Db/Repositories/BaseRepository.cs b/Schaad.Accounting.Db/Repositories/BaseRepository.cs index 3813fa8..63690a0 100644 --- a/Schaad.Accounting.Db/Repositories/BaseRepository.cs +++ b/Schaad.Accounting.Db/Repositories/BaseRepository.cs @@ -5,6 +5,7 @@ using System.IO; using System.Text; using System.Xml; using System.Xml.Serialization; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; namespace Schaad.Accounting.Repositories @@ -18,11 +19,13 @@ namespace Schaad.Accounting.Repositories protected readonly ISettingsService settingsService; protected readonly RepositoryCache cache; + protected readonly ILogger logger; - protected BaseRepository(ISettingsService settingsService, RepositoryCache cache) + protected BaseRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) { this.settingsService = settingsService; this.cache = cache; + this.logger = logger; } protected void EnsureFileExists(string fileName) @@ -79,8 +82,9 @@ namespace Schaad.Accounting.Repositories File.Move(tmpPath, filePath, overwrite: true); } - catch + catch (Exception ex) { + logger.LogError(ex, "Failed to save {FilePath}", filePath); if (File.Exists(tmpPath)) { try { File.Delete(tmpPath); } catch { /* best effort */ } diff --git a/Schaad.Accounting.Db/Repositories/BookingRuleRepository.cs b/Schaad.Accounting.Db/Repositories/BookingRuleRepository.cs index 34c04b1..623c92d 100644 --- a/Schaad.Accounting.Db/Repositories/BookingRuleRepository.cs +++ b/Schaad.Accounting.Db/Repositories/BookingRuleRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories { private readonly string BOOKING_RULES = "BookingRules.xml"; - public BookingRuleRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public BookingRuleRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { EnsureFileExists(BOOKING_RULES); } diff --git a/Schaad.Accounting.Db/Repositories/BookingTextRepository.cs b/Schaad.Accounting.Db/Repositories/BookingTextRepository.cs index 06c66ec..e98fe45 100644 --- a/Schaad.Accounting.Db/Repositories/BookingTextRepository.cs +++ b/Schaad.Accounting.Db/Repositories/BookingTextRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories { private readonly string BOOKING_TEXTS = "BookingTexts.xml"; - public BookingTextRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public BookingTextRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { EnsureFileExists(BOOKING_TEXTS); } diff --git a/Schaad.Accounting.Db/Repositories/SplitPredefinitionRepository.cs b/Schaad.Accounting.Db/Repositories/SplitPredefinitionRepository.cs index a73dae8..1cc276d 100644 --- a/Schaad.Accounting.Db/Repositories/SplitPredefinitionRepository.cs +++ b/Schaad.Accounting.Db/Repositories/SplitPredefinitionRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories { private readonly string SPLIT_PREDEFINITION = "SplitPredefinitions.xml"; - public SplitPredefinitionRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public SplitPredefinitionRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { EnsureFileExists(SPLIT_PREDEFINITION); } diff --git a/Schaad.Accounting.Db/Repositories/SubclassRepository.cs b/Schaad.Accounting.Db/Repositories/SubclassRepository.cs index 1f4f54f..3117592 100644 --- a/Schaad.Accounting.Db/Repositories/SubclassRepository.cs +++ b/Schaad.Accounting.Db/Repositories/SubclassRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories private readonly Dictionary classes = new Dictionary(); private readonly string SUBCLASSES = "SubClasses.xml"; - public SubclassRepository(ISettingsService settingsService, RepositoryCache cache) - : base(settingsService, cache) + public SubclassRepository(ISettingsService settingsService, RepositoryCache cache, ILogger logger) + : base(settingsService, cache, logger) { EnsureFileExists(SUBCLASSES); diff --git a/Schaad.Accounting.Db/Repositories/TransactionRepository.cs b/Schaad.Accounting.Db/Repositories/TransactionRepository.cs index 0e55408..e1f5c91 100644 --- a/Schaad.Accounting.Db/Repositories/TransactionRepository.cs +++ b/Schaad.Accounting.Db/Repositories/TransactionRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories private readonly IAccountRepository accountRepository; private readonly string TRANSACTIONS = "Transactions.xml"; - public TransactionRepository(ISettingsService settingsService, RepositoryCache cache, IAccountRepository accountRepository) - : base(settingsService, cache) + public TransactionRepository(ISettingsService settingsService, RepositoryCache cache, IAccountRepository accountRepository, ILogger logger) + : base(settingsService, cache, logger) { this.accountRepository = accountRepository; } diff --git a/Schaad.Accounting.Db/Schaad.Accounting.Db.csproj b/Schaad.Accounting.Db/Schaad.Accounting.Db.csproj index 8f28362..96e4a67 100644 --- a/Schaad.Accounting.Db/Schaad.Accounting.Db.csproj +++ b/Schaad.Accounting.Db/Schaad.Accounting.Db.csproj @@ -7,4 +7,8 @@ + + + + diff --git a/Schaad.Accounting.Tests/AccountRepositoryTestShould.cs b/Schaad.Accounting.Tests/AccountRepositoryTestShould.cs index ad7491f..59a1a2c 100644 --- a/Schaad.Accounting.Tests/AccountRepositoryTestShould.cs +++ b/Schaad.Accounting.Tests/AccountRepositoryTestShould.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -21,7 +22,7 @@ public class AccountRepositoryTestShould : IDisposable settingsService.GetDbPath().Returns(dbDir); settingsService.GetLastYearDbPath().Returns(Path.Combine(dbDir, "no-such-dir")); - sut = new AccountRepository(settingsService, new RepositoryCache()); + sut = new AccountRepository(settingsService, new RepositoryCache(), NullLogger.Instance); } public void Dispose() @@ -103,7 +104,7 @@ public class AccountRepositoryTestShould : IDisposable sut.SaveAccount(new Account { Number = 1000, Name = "Cash", Currency = "CHF" }); // Re-construct with the same directory: existing file, no year rollover, no data loss. - var fresh = new AccountRepository(settingsService, new RepositoryCache()); + var fresh = new AccountRepository(settingsService, new RepositoryCache(), NullLogger.Instance); var accounts = fresh.GetAccountList(); accounts.Count.ShouldBe(1); diff --git a/Schaad.Accounting.Tests/TransactionRepositoryTestShould.cs b/Schaad.Accounting.Tests/TransactionRepositoryTestShould.cs index a980738..ba7efee 100644 --- a/Schaad.Accounting.Tests/TransactionRepositoryTestShould.cs +++ b/Schaad.Accounting.Tests/TransactionRepositoryTestShould.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -25,7 +26,7 @@ public class TransactionRepositoryTestShould : IDisposable accountRepo = Substitute.For(); cache = new RepositoryCache(); - sut = new TransactionRepository(settingsService, cache, accountRepo); + sut = new TransactionRepository(settingsService, cache, accountRepo, NullLogger.Instance); } public void Dispose()