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<ThisRepo> 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<T>.Instance.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Claudio Schaad 2026-07-02 21:47:55 +02:00
parent 3677b5cd6e
commit 3a770af016
12 changed files with 37 additions and 19 deletions

View file

@ -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 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<FileService>` 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 H** — Phase 2 item 18 (first slice): add `ILogger<FileService>` 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` `<Reference>` from `Common.csproj` — Common is now vendor-free and matches its documented role as the "shared models, DTOs, interfaces" layer. - **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` `<Reference>` 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<T>` 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<T>.Instance`. Remaining item 18 work (MatchOpenBankTransactions match-count summary in `ViewService`) still open.

View file

@ -2,6 +2,7 @@
using System.Collections.Generic; using System.Collections.Generic;
using System.IO; using System.IO;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories
{ {
private readonly string ACCOUNTS = "Accounts.xml"; private readonly string ACCOUNTS = "Accounts.xml";
public AccountRepository(ISettingsService settingsService, RepositoryCache cache) public AccountRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<AccountRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
EnsureAccountsFile(); EnsureAccountsFile();
} }

View file

@ -1,5 +1,6 @@
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Datasets; using Schaad.Accounting.Datasets;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories
{ {
private readonly string BANK_TRANSACTIONS = "BankTransactions.xml"; private readonly string BANK_TRANSACTIONS = "BankTransactions.xml";
public BankTransactionRepository(ISettingsService settingsService, RepositoryCache cache) public BankTransactionRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<BankTransactionRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
} }

View file

@ -5,6 +5,7 @@ using System.IO;
using System.Text; using System.Text;
using System.Xml; using System.Xml;
using System.Xml.Serialization; using System.Xml.Serialization;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
namespace Schaad.Accounting.Repositories namespace Schaad.Accounting.Repositories
@ -18,11 +19,13 @@ namespace Schaad.Accounting.Repositories
protected readonly ISettingsService settingsService; protected readonly ISettingsService settingsService;
protected readonly RepositoryCache cache; 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.settingsService = settingsService;
this.cache = cache; this.cache = cache;
this.logger = logger;
} }
protected void EnsureFileExists(string fileName) protected void EnsureFileExists(string fileName)
@ -79,8 +82,9 @@ namespace Schaad.Accounting.Repositories
File.Move(tmpPath, filePath, overwrite: true); File.Move(tmpPath, filePath, overwrite: true);
} }
catch catch (Exception ex)
{ {
logger.LogError(ex, "Failed to save {FilePath}", filePath);
if (File.Exists(tmpPath)) if (File.Exists(tmpPath))
{ {
try { File.Delete(tmpPath); } catch { /* best effort */ } try { File.Delete(tmpPath); } catch { /* best effort */ }

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories
{ {
private readonly string BOOKING_RULES = "BookingRules.xml"; private readonly string BOOKING_RULES = "BookingRules.xml";
public BookingRuleRepository(ISettingsService settingsService, RepositoryCache cache) public BookingRuleRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<BookingRuleRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
EnsureFileExists(BOOKING_RULES); EnsureFileExists(BOOKING_RULES);
} }

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories
{ {
private readonly string BOOKING_TEXTS = "BookingTexts.xml"; private readonly string BOOKING_TEXTS = "BookingTexts.xml";
public BookingTextRepository(ISettingsService settingsService, RepositoryCache cache) public BookingTextRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<BookingTextRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
EnsureFileExists(BOOKING_TEXTS); EnsureFileExists(BOOKING_TEXTS);
} }

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -10,8 +11,8 @@ namespace Schaad.Accounting.Repositories
{ {
private readonly string SPLIT_PREDEFINITION = "SplitPredefinitions.xml"; private readonly string SPLIT_PREDEFINITION = "SplitPredefinitions.xml";
public SplitPredefinitionRepository(ISettingsService settingsService, RepositoryCache cache) public SplitPredefinitionRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<SplitPredefinitionRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
EnsureFileExists(SPLIT_PREDEFINITION); EnsureFileExists(SPLIT_PREDEFINITION);
} }

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories
private readonly Dictionary<int, string> classes = new Dictionary<int, string>(); private readonly Dictionary<int, string> classes = new Dictionary<int, string>();
private readonly string SUBCLASSES = "SubClasses.xml"; private readonly string SUBCLASSES = "SubClasses.xml";
public SubclassRepository(ISettingsService settingsService, RepositoryCache cache) public SubclassRepository(ISettingsService settingsService, RepositoryCache cache, ILogger<SubclassRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
EnsureFileExists(SUBCLASSES); EnsureFileExists(SUBCLASSES);

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using Microsoft.Extensions.Logging;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -11,8 +12,8 @@ namespace Schaad.Accounting.Repositories
private readonly IAccountRepository accountRepository; private readonly IAccountRepository accountRepository;
private readonly string TRANSACTIONS = "Transactions.xml"; private readonly string TRANSACTIONS = "Transactions.xml";
public TransactionRepository(ISettingsService settingsService, RepositoryCache cache, IAccountRepository accountRepository) public TransactionRepository(ISettingsService settingsService, RepositoryCache cache, IAccountRepository accountRepository, ILogger<TransactionRepository> logger)
: base(settingsService, cache) : base(settingsService, cache, logger)
{ {
this.accountRepository = accountRepository; this.accountRepository = accountRepository;
} }

View file

@ -7,4 +7,8 @@
<ItemGroup> <ItemGroup>
<ProjectReference Include="..\Schaad.Accounting.Common\Schaad.Accounting.Common.csproj" /> <ProjectReference Include="..\Schaad.Accounting.Common\Schaad.Accounting.Common.csproj" />
</ItemGroup> </ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.9" />
</ItemGroup>
</Project> </Project>

View file

@ -1,3 +1,4 @@
using Microsoft.Extensions.Logging.Abstractions;
using NSubstitute; using NSubstitute;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -21,7 +22,7 @@ public class AccountRepositoryTestShould : IDisposable
settingsService.GetDbPath().Returns(dbDir); settingsService.GetDbPath().Returns(dbDir);
settingsService.GetLastYearDbPath().Returns(Path.Combine(dbDir, "no-such-dir")); settingsService.GetLastYearDbPath().Returns(Path.Combine(dbDir, "no-such-dir"));
sut = new AccountRepository(settingsService, new RepositoryCache()); sut = new AccountRepository(settingsService, new RepositoryCache(), NullLogger<AccountRepository>.Instance);
} }
public void Dispose() public void Dispose()
@ -103,7 +104,7 @@ public class AccountRepositoryTestShould : IDisposable
sut.SaveAccount(new Account { Number = 1000, Name = "Cash", Currency = "CHF" }); sut.SaveAccount(new Account { Number = 1000, Name = "Cash", Currency = "CHF" });
// Re-construct with the same directory: existing file, no year rollover, no data loss. // 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<AccountRepository>.Instance);
var accounts = fresh.GetAccountList(); var accounts = fresh.GetAccountList();
accounts.Count.ShouldBe(1); accounts.Count.ShouldBe(1);

View file

@ -1,3 +1,4 @@
using Microsoft.Extensions.Logging.Abstractions;
using NSubstitute; using NSubstitute;
using Schaad.Accounting.Interfaces; using Schaad.Accounting.Interfaces;
using Schaad.Accounting.Models; using Schaad.Accounting.Models;
@ -25,7 +26,7 @@ public class TransactionRepositoryTestShould : IDisposable
accountRepo = Substitute.For<IAccountRepository>(); accountRepo = Substitute.For<IAccountRepository>();
cache = new RepositoryCache(); cache = new RepositoryCache();
sut = new TransactionRepository(settingsService, cache, accountRepo); sut = new TransactionRepository(settingsService, cache, accountRepo, NullLogger<TransactionRepository>.Instance);
} }
public void Dispose() public void Dispose()