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()