diff --git a/.cursor/rules/collaboration-and-docs.mdc b/.cursor/rules/collaboration-and-docs.mdc new file mode 100644 index 0000000000..c504d79cc9 --- /dev/null +++ b/.cursor/rules/collaboration-and-docs.mdc @@ -0,0 +1,21 @@ +--- +description: Collaboration style, planning docs, and engineering decision log for this project +alwaysApply: true +--- + +# Collaboration and documentation + +## How to work with the maintainer + +- **Step by step:** Discuss and implement **one step at a time** unless the user explicitly asks to batch or skip ahead. +- **No surprise decisions:** Do **not** choose libraries, frameworks, or structural changes (e.g. new classes, folders, or public APIs) on the maintainer’s behalf. Present **options and tradeoffs** and wait for an explicit choice when it matters. +- **No unsolicited code:** Do **not** write or edit application code unless the maintainer **explicitly asks** you to. It is fine to explain approaches, sketch snippets in chat when helpful, and review or suggest changes in prose—but avoid applying patches or creating/editing source files until asked. +- **`Program.cs` (current):** The maintainer is wiring [`jobs/Backend/Task/Program.cs`](jobs/Backend/Task/Program.cs) themselves (composition root, `IExchangeRateSource` + `ExchangeRateProvider`). Do **not** change that file unless they explicitly request it. + +## Where documentation lives + +- **Task / implementation plan:** The versioned plan for this backend task is [`jobs/Backend/docs/PLAN.md`](jobs/Backend/docs/PLAN.md). When the plan changes, **update that file** so it stays the single source of truth in the repo. +- **Engineering decisions:** Record substantive choices (libraries, patterns, whether to add a type or folder, sync vs async, etc.) in [`jobs/Backend/docs/DECISIONS.md`](jobs/Backend/docs/DECISIONS.md), using the same lightweight sections as existing entries (Context / Decision / Why / Consequences). +- **Test case matrix:** When adding, renaming, removing, or materially changing a test method, update [`jobs/Backend/docs/TEST_CASES.md`](jobs/Backend/docs/TEST_CASES.md) so the documented test matrices stay aligned with the test suite. + +Cursor may also store a generated plan under `.cursor/plans/`; treat **`PLAN.md` in the repo** as the authoritative plan for git and reviews unless the user says otherwise. diff --git a/jobs/Backend/SOLUTION.md b/jobs/Backend/SOLUTION.md new file mode 100644 index 0000000000..6355b8f04a --- /dev/null +++ b/jobs/Backend/SOLUTION.md @@ -0,0 +1,90 @@ +# Backend Solution Guide - Nicolas Darriulat + +Hi there! This guide is the starting point for reading the implemented .NET backend task. The original assignment files are still present, this file explains what was built, where the supporting documents live, and how to run the solution locally. + +Although the assignment is small, I intentionally used a production-oriented structure to demonstrate how I approach maintainable code: clear responsibilities, explicit dependencies, documented decisions, and focused tests. + +I also documented every decision, and created multiple documents to explain my thought process. I used Cursor while coding this project, so you may also find a .mdc file which tells which were the collaboration rules agreed with Cursor's AI Agents. + +**Note:** This PR is squashed into a few clean commits on top of the latest upstream `master`. The work was developed incrementally, but the final branch was rebuilt to keep the review focused on the backend solution instead of unrelated fork-history differences. + +## What This Implements + +The solution implements an `ExchangeRateProvider` backed by the Czech National Bank daily exchange-rate feed. The concrete CNB source downloads the published text document, parses it into normalized `ExchangeRate` objects, and the provider filters those rates for requested source currencies. + +CNB publishes rates as foreign currency against CZK. For example, requesting `USD` returns the source-provided `USD/CZK` rate; the caller does not need to request `CZK` separately. + +## Where To Start + +1. `[DotNet.md](DotNet.md)` - the original .NET assignment. +2. `[SOLUTION.md](SOLUTION.md)` - this guide and local setup instructions. +3. `[docs/PLAN.md](docs/PLAN.md)` - the implementation plan and high-level flow. +4. `[docs/DECISIONS.md](docs/DECISIONS.md)` - the engineering decisions and tradeoffs. +5. `[docs/TEST_CASES.md](docs/TEST_CASES.md)` - the documented test matrix. +6. `[.cursor/rules/collaboration-and-docs.mdc](../../.cursor/rules/collaboration-and-docs.mdc)` - the collaboration rules agreed with Cursor's AI agents. + +## Document Map + + +| Document | Purpose | +| -------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| `[Readme.md](Readme.md)` | Backend task index that links to the available backend exercises. | +| `[SOLUTION.md](SOLUTION.md)` | Reader-oriented guide for this implementation and local commands. | +| `[DotNet.md](DotNet.md)` | Original task description for the .NET implementation. | +| `[docs/PLAN.md](docs/PLAN.md)` | Current implementation plan, flow, checklist, and scope notes. | +| `[docs/DECISIONS.md](docs/DECISIONS.md)` | Decision log explaining the main design choices and consequences. | +| `[docs/TEST_CASES.md](docs/TEST_CASES.md)` | Test coverage matrix for provider, filtering, and CNB source behavior. | +| `[.cursor/rules/collaboration-and-docs.mdc](../../.cursor/rules/collaboration-and-docs.mdc)` | Collaboration rules agreed with Cursor's AI agents while building this project. | + + +## Code Structure + + +| Path | Role | +| ---------------------------------------------------------------- | --------------------------------------------------------------------------------- | +| `[Task/ExchangeRateProvider.cs](Task/ExchangeRateProvider.cs)` | Orchestrates source retrieval, filtering, and return of exchange rates. | +| `[Task/IExchangeRateSource.cs](Task/IExchangeRateSource.cs)` | Source abstraction returning parsed exchange-rate objects. | +| `[Task/CnbExchangeRateSource.cs](Task/CnbExchangeRateSource.cs)` | Fetches and parses the CNB daily exchange-rate text document. | +| `[Task/Currency.cs](Task/Currency.cs)` | Currency value object with case-insensitive code equality. | +| `[Task/ExchangeRate.cs](Task/ExchangeRate.cs)` | Exchange-rate model with source currency, target currency, and value. | +| `[Task/Program.cs](Task/Program.cs)` | Console composition root: configuration, DI, HTTP client, and provider execution. | +| `[Task.Tests](Task.Tests)` | xUnit tests for provider behavior, filtering, and CNB source parsing. | + + +## Run Locally + +Required tooling: + +- .NET SDK 10.0 or newer, because both projects target `net10.0`. +- Internet access for the first NuGet restore and for running the console app against the live CNB endpoint. + +From the repository root: + +```bash +dotnet restore "jobs/Backend/Task/ExchangeRateUpdater.csproj" +dotnet run --project "jobs/Backend/Task/ExchangeRateUpdater.csproj" +``` + +`dotnet run` also performs restore/build automatically, but running `dotnet restore` explicitly makes missing SDK or package issues easier to see. + +## Run Tests + +From the repository root: + +```bash +dotnet test "jobs/Backend/Task.Tests/Task.Tests.csproj" +``` + +The tests use xUnit. Provider tests use fake `IExchangeRateSource` implementations, while CNB source tests use stubbed HTTP responses so they do not depend on the live CNB website. + +## Behavior Notes + +- CNB rows are parsed as foreign currency to CZK, e.g. `USD/CZK`. +- Requested currencies are interpreted as requested source currencies. +- Unknown requested currencies are ignored. +- The provider does not synthesize inverse rates such as `CZK/USD`. +- CNB amounts greater than one, such as `100 JPY`, are normalized to a per-unit `ExchangeRate.Value`. + +## Production Notes + +The main production-oriented choices are documented in `[docs/DECISIONS.md](docs/DECISIONS.md)`: `HttpClient` creation through `IHttpClientFactory`, configuration through `appsettings.json` with startup validation, configurable CNB retries/timeouts through `Microsoft.Extensions.Http.Resilience`, source-specific parsing inside `CnbExchangeRateSource`, and a deliberate sync-over-async boundary to preserve the assignment-facing provider API. \ No newline at end of file diff --git a/jobs/Backend/Task.Tests/CnbExchangeRateSourceIntegrationTests.cs b/jobs/Backend/Task.Tests/CnbExchangeRateSourceIntegrationTests.cs new file mode 100644 index 0000000000..158941960a --- /dev/null +++ b/jobs/Backend/Task.Tests/CnbExchangeRateSourceIntegrationTests.cs @@ -0,0 +1,150 @@ +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using ExchangeRateUpdater; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Xunit; + +public class CnbExchangeRateSourceIntegrationTests +{ + private const string TestUrl = "https://example.test/denni_kurz.txt"; + + /// + /// Matrix of CNB daily document shapes (as returned by HTTP) and expected parsed rates. + /// Filtering by requested currencies happens in ; the source returns all parsed rows. + /// + public static IEnumerable CnbDocumentScenarios() + { + const string header = "07 May 2026 #87\nCountry|Currency|Amount|Code|Rate\n"; + + yield return new object[] + { + "single_row_amount_one_dot_decimal", + header + "USA|dollar|1|USD|20.648\n", + new (string Src, string Tgt, decimal Value)[] { ("USD", "CZK", 20.648m) }, + }; + + yield return new object[] + { + "amount_100_normalises_to_per_unit", + header + "Japan|yen|100|JPY|13.204\n", + new (string Src, string Tgt, decimal Value)[] { ("JPY", "CZK", 0.13204m) }, + }; + + yield return new object[] + { + "multiple_rows", + header + + "USA|dollar|1|USD|20.648\n" + + "EMU|euro|1|EUR|24.305\n", + new (string Src, string Tgt, decimal Value)[] + { + ("USD", "CZK", 20.648m), + ("EUR", "CZK", 24.305m), + }, + }; + + yield return new object[] + { + "comma_decimal_czech_style", + header + "USA|dollar|1|USD|20,648\n", + new (string Src, string Tgt, decimal Value)[] { ("USD", "CZK", 20.648m) }, + }; + + yield return new object[] + { + "malformed_pipe_row_skipped_valid_rows_kept", + header + + "not|enough|columns\n" + + "USA|dollar|1|USD|1\n" + + "||||\n", + new (string Src, string Tgt, decimal Value)[] { ("USD", "CZK", 1m) }, + }; + + yield return new object[] + { + "empty_body", + "", + System.Array.Empty<(string Src, string Tgt, decimal Value)>(), + }; + + yield return new object[] + { + "headers_only_no_data_rows", + header.TrimEnd(), + System.Array.Empty<(string Src, string Tgt, decimal Value)>(), + }; + + yield return new object[] + { + "whitespace_trimmed_in_fields", + header + " USA | dollar | 1 | USD | 10 \n", + new (string Src, string Tgt, decimal Value)[] { ("USD", "CZK", 10m) }, + }; + } + + [Theory] + [MemberData(nameof(CnbDocumentScenarios))] + public async Task GetExchangeRates_parses_http_body_as_cnb_daily_document( + string scenario, + string httpBody, + (string Src, string Tgt, decimal Value)[] expected) + { + _ = scenario; + using var handler = new StubHttpMessageHandler(HttpStatusCode.OK, httpBody); + using var httpClient = new HttpClient(handler); + var options = Options.Create(new CnbOptions { DailyKurzUrl = TestUrl }); + var source = new CnbExchangeRateSource(httpClient, options, NullLogger.Instance); + + var rates = (await source.GetExchangeRates()).ToList(); + + Assert.Equal(expected.Length, rates.Count); + foreach (var exp in expected) + { + Assert.Contains( + rates, + r => r.SourceCurrency.Code == exp.Src + && r.TargetCurrency.Code == exp.Tgt + && r.Value == exp.Value); + } + } + + [Fact] + public async Task GetExchangeRates_when_http_fails_throws() + { + using var handler = new StubHttpMessageHandler(HttpStatusCode.NotFound, body: ""); + using var httpClient = new HttpClient(handler); + var options = Options.Create(new CnbOptions { DailyKurzUrl = TestUrl }); + var source = new CnbExchangeRateSource(httpClient, options, NullLogger.Instance); + + await Assert.ThrowsAsync( + () => source.GetExchangeRates()); + } + + private sealed class StubHttpMessageHandler : HttpMessageHandler + { + private readonly HttpStatusCode _statusCode; + private readonly string _body; + + public StubHttpMessageHandler(HttpStatusCode statusCode, string body) + { + _statusCode = statusCode; + _body = body; + } + + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + var response = new HttpResponseMessage(_statusCode) + { + Content = new StringContent(_body), + }; + return Task.FromResult(response); + } + } +} diff --git a/jobs/Backend/Task.Tests/ExchangeRateProviderFilteringTests.cs b/jobs/Backend/Task.Tests/ExchangeRateProviderFilteringTests.cs new file mode 100644 index 0000000000..d6019643d1 --- /dev/null +++ b/jobs/Backend/Task.Tests/ExchangeRateProviderFilteringTests.cs @@ -0,0 +1,91 @@ +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using ExchangeRateUpdater; +using System.Threading.Tasks; +using Xunit; + +public class ExchangeRateProviderFilteringTests +{ + [Fact] + public void GetFilteredRates_returns_rates_for_requested_source_currency() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var eur = new Currency("EUR"); + var cnbRates = new List + { + new ExchangeRate(usd, czk, 25.0m), + new ExchangeRate(eur, czk, 24.0m), + }; + + var filteredRates = InvokeGetFilteredRates(cnbRates, new List { usd }).ToList(); + + Assert.Single(filteredRates); + Assert.Equal("USD", filteredRates[0].SourceCurrency.Code); + Assert.Equal("CZK", filteredRates[0].TargetCurrency.Code); + } + + [Fact] + public void GetFilteredRates_ignores_unknown_requested_currencies() + { + var usd = new Currency("USD"); + var cnbRates = new List + { + new ExchangeRate(usd, new Currency("CZK"), 25.0m), + }; + + var filteredRates = InvokeGetFilteredRates(cnbRates, new List { new Currency("XYZ") }); + + Assert.Empty(filteredRates); + } + + [Fact] + public void GetFilteredRates_does_not_create_inverse_pairs() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var cnbRates = new List + { + new ExchangeRate(usd, czk, 25.0m), + }; + + var filteredRates = InvokeGetFilteredRates(cnbRates, new List { usd }).ToList(); + + Assert.Single(filteredRates); + Assert.DoesNotContain(filteredRates, r => r.SourceCurrency.Code == "CZK" && r.TargetCurrency.Code == "USD"); + } + + private static IEnumerable InvokeGetFilteredRates( + IEnumerable cnbRates, + IEnumerable currencies) + { + var provider = new ExchangeRateProvider(new FakeExchangeRateSource()); + var method = typeof(ExchangeRateProvider).GetMethod("GetFilteredRates", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(method); + + var result = method!.Invoke(provider, new object[] { cnbRates, currencies }); + Assert.NotNull(result); + return (IEnumerable)result!; + } + + private sealed class FakeExchangeRateSource : IExchangeRateSource + { + private readonly IReadOnlyList _rates; + + public FakeExchangeRateSource() + : this(Enumerable.Empty()) + { + } + + public FakeExchangeRateSource(IEnumerable rates) + { + _rates = rates.ToList(); + } + + public Task> GetExchangeRates() + { + return Task.FromResult>(_rates); + } + } +} \ No newline at end of file diff --git a/jobs/Backend/Task.Tests/ExchangeRateProviderTests.cs b/jobs/Backend/Task.Tests/ExchangeRateProviderTests.cs new file mode 100644 index 0000000000..69ba8c2e12 --- /dev/null +++ b/jobs/Backend/Task.Tests/ExchangeRateProviderTests.cs @@ -0,0 +1,182 @@ +using System.Collections.Generic; +using System.Linq; +using ExchangeRateUpdater; +using System.Threading.Tasks; +using Xunit; + +public class ExchangeRateProviderTests +{ + [Fact] + public void GetExchangeRates_WhenSourceReturnsNoRates_ReturnsEmpty() + { + var provider = new ExchangeRateProvider(new FakeExchangeRateSource()); + var rates = provider.GetExchangeRates(new List { new Currency("USD") }); + + Assert.Empty(rates); + } + + [Fact] + public void GetExchangeRates_WhenOneExistingCurrencyRequested_ReturnsThatRate() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var sourceRates = new[] { new ExchangeRate(usd, czk, 25m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { usd }).ToList(); + + Assert.Single(rates); + Assert.Same(usd, rates[0].SourceCurrency); + Assert.Same(czk, rates[0].TargetCurrency); + Assert.Equal(25m, rates[0].Value); + } + + [Fact] + public void GetExchangeRates_WhenRequestedCurrencyHasSameCodeAsSourceCurrency_ReturnsThatRate() + { + var sourceRates = new[] + { + new ExchangeRate(new Currency("USD"), new Currency("CZK"), 25m), + }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { new Currency("USD") }).ToList(); + + Assert.Single(rates); + Assert.Equal("USD", rates[0].SourceCurrency.Code); + Assert.Equal("CZK", rates[0].TargetCurrency.Code); + Assert.Equal(25m, rates[0].Value); + } + + [Fact] + public void GetExchangeRates_WhenNoRateMatchesRequest_ReturnsEmpty() + { + var gbp = new Currency("GBP"); + var jpy = new Currency("JPY"); + var sourceRates = new[] { new ExchangeRate(gbp, jpy, 150m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { new Currency("USD") }); + + Assert.Empty(rates); + } + + [Fact] + public void GetExchangeRates_WhenOnlyUnknownRequestedCurrencies_ReturnsEmpty() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var sourceRates = new[] { new ExchangeRate(usd, czk, 25m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { new Currency("XYZ") }); + + Assert.Empty(rates); + } + + [Fact] + public void GetExchangeRates_WhenMultipleRequested_IncludesOnlyRatesWhoseSourceCurrencyWasRequested() + { + var usd = new Currency("USD"); + var eur = new Currency("EUR"); + var czk = new Currency("CZK"); + var sourceRates = new[] + { + new ExchangeRate(usd, czk, 25m), + new ExchangeRate(eur, czk, 24m), + }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { usd }).ToList(); + + Assert.Single(rates); + Assert.Same(usd, rates[0].SourceCurrency); + } + + [Fact] + public void GetExchangeRates_WhenMultipleSourceCurrenciesRequested_ReturnsRatesForThoseSourcesOnly() + { + var usd = new Currency("USD"); + var eur = new Currency("EUR"); + var jpy = new Currency("JPY"); + var czk = new Currency("CZK"); + + var sourceRates = new[] + { + new ExchangeRate(usd, czk, 25m), + new ExchangeRate(eur, czk, 24m), + new ExchangeRate(jpy, czk, 0.15m), + }; + + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new[] { usd, eur }).ToList(); + + Assert.Equal(2, rates.Count); + Assert.Contains(rates, r => r.SourceCurrency == usd && r.TargetCurrency == czk); + Assert.Contains(rates, r => r.SourceCurrency == eur && r.TargetCurrency == czk); + Assert.DoesNotContain(rates, r => r.SourceCurrency == jpy && r.TargetCurrency == czk); + } + + [Fact] + public void GetExchangeRates_WhenMixOfKnownAndUnknownRequested_ReturnsRatesForKnownOnly() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var sourceRates = new[] { new ExchangeRate(usd, czk, 25m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { usd, new Currency("XYZ") }).ToList(); + + Assert.Single(rates); + Assert.Same(usd, rates[0].SourceCurrency); + } + + [Fact] + public void GetExchangeRates_WhenSourcePublishesSingleDirection_DoesNotSynthesizeInversePair() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var sourceRates = new[] { new ExchangeRate(usd, czk, 25m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List { usd }).ToList(); + + Assert.Single(rates); + Assert.Contains(rates, r => r.SourceCurrency == usd && r.TargetCurrency == czk); + Assert.DoesNotContain(rates, r => r.SourceCurrency == czk && r.TargetCurrency == usd); + } + + [Fact] + public void GetExchangeRates_WhenRequestIsEmpty_ReturnsEmptyEvenIfSourceHasRates() + { + var usd = new Currency("USD"); + var czk = new Currency("CZK"); + var sourceRates = new[] { new ExchangeRate(usd, czk, 25m) }; + var provider = new ExchangeRateProvider(new FakeExchangeRateSource(sourceRates)); + + var rates = provider.GetExchangeRates(new List()); + + Assert.Empty(rates); + } + + private sealed class FakeExchangeRateSource : IExchangeRateSource + { + private readonly IReadOnlyList _rates; + + public FakeExchangeRateSource() + : this(Enumerable.Empty()) + { + } + + public FakeExchangeRateSource(IEnumerable rates) + { + _rates = rates.ToList(); + } + + public Task> GetExchangeRates() + { + return Task.FromResult>(_rates); + } + } +} diff --git a/jobs/Backend/Task.Tests/Task.Tests.csproj b/jobs/Backend/Task.Tests/Task.Tests.csproj new file mode 100644 index 0000000000..1f97f7f3c8 --- /dev/null +++ b/jobs/Backend/Task.Tests/Task.Tests.csproj @@ -0,0 +1,19 @@ + + + net10.0 + false + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + \ No newline at end of file diff --git a/jobs/Backend/Task/CnbExchangeRateSource.cs b/jobs/Backend/Task/CnbExchangeRateSource.cs new file mode 100644 index 0000000000..54d738f569 --- /dev/null +++ b/jobs/Backend/Task/CnbExchangeRateSource.cs @@ -0,0 +1,151 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace ExchangeRateUpdater +{ + public class CnbExchangeRateSource : IExchangeRateSource + { + private const int CnbHeaderLineCount = 2; + + private readonly HttpClient _httpClient; + private readonly IOptions _options; + private readonly ILogger _logger; + + public CnbExchangeRateSource( + HttpClient httpClient, + IOptions options, + ILogger logger) + { + _httpClient = httpClient; + _options = options; + _logger = logger; + } + + public async Task> GetExchangeRates() + { + var dailyKurzUrl = _options.Value.DailyKurzUrl; + _logger.LogDebug("Fetching CNB exchange rates from {Url}.", dailyKurzUrl); + + var response = await _httpClient.GetAsync(dailyKurzUrl).ConfigureAwait(false); + if (!response.IsSuccessStatusCode) + { + _logger.LogWarning( + "CNB exchange-rate request failed with status code {StatusCode}.", + response.StatusCode); + } + + response.EnsureSuccessStatusCode(); + var content = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + var rates = ParseCnbDailyKurz(content, out var skippedRowCount); + + if (skippedRowCount > 0) + { + _logger.LogWarning( + "Skipped {SkippedRowCount} malformed CNB exchange-rate rows.", + skippedRowCount); + } + + _logger.LogInformation("Parsed {RateCount} CNB exchange rates.", rates.Count); + return rates; + } + + private IReadOnlyList ParseCnbDailyKurz(string content, out int skippedRowCount) + { + skippedRowCount = 0; + var rates = new List(); + + if (string.IsNullOrWhiteSpace(content)) + { + return rates; + } + + var lines = content.Split(new[] { "\r\n", "\n", "\r" }, StringSplitOptions.None); + if (lines.Length <= CnbHeaderLineCount) + { + return rates; + } + + for (var i = CnbHeaderLineCount; i < lines.Length; i++) + { + var line = lines[i].Trim(); + if (line.Length == 0) + { + continue; + } + + var parts = line.Split('|'); + if (parts.Length < 5) + { + skippedRowCount++; + LogSkippedRow(i + 1, "expected at least 5 pipe-delimited columns", line); + continue; + } + + var amountText = parts[2].Trim(); + var code = parts[3].Trim(); + var rateText = parts[4].Trim(); + + if (string.IsNullOrEmpty(code)) + { + skippedRowCount++; + LogSkippedRow(i + 1, "missing currency code", line); + continue; + } + + if (!decimal.TryParse(amountText, NumberStyles.Integer, CultureInfo.InvariantCulture, out var amount) + || amount <= 0) + { + skippedRowCount++; + LogSkippedRow(i + 1, $"invalid amount '{amountText}'", line); + continue; + } + + if (!TryParseCnbDecimal(rateText, out var rate)) + { + skippedRowCount++; + LogSkippedRow(i + 1, $"invalid rate '{rateText}'", line); + continue; + } + + var value = rate / amount; + rates.Add(new ExchangeRate(new Currency(code), new Currency("CZK"), value)); + } + + return rates; + } + + private void LogSkippedRow(int lineNumber, string reason, string row) + { + _logger.LogDebug( + "Skipping malformed CNB exchange-rate row at line {LineNumber}: {Reason}. Row: {Row}", + lineNumber, + reason, + row); + } + + private static bool TryParseCnbDecimal(string text, out decimal value) + { + // Avoid AllowThousands: CNB English files use "." as decimal; a comma must not be read as a thousands separator. + const NumberStyles rateStyles = NumberStyles.Number & ~NumberStyles.AllowThousands; + + if (decimal.TryParse(text, rateStyles, CultureInfo.InvariantCulture, out value)) + { + return true; + } + + // Czech-published files often use "," as the decimal separator in this column. + var withDotDecimal = text.Replace(',', '.'); + if (decimal.TryParse(withDotDecimal, rateStyles, CultureInfo.InvariantCulture, out value)) + { + return true; + } + + return decimal.TryParse(text, NumberStyles.Number, CultureInfo.GetCultureInfo("cs-CZ"), out value); + } + } +} \ No newline at end of file diff --git a/jobs/Backend/Task/CnbOptions.cs b/jobs/Backend/Task/CnbOptions.cs new file mode 100644 index 0000000000..36c0e88fec --- /dev/null +++ b/jobs/Backend/Task/CnbOptions.cs @@ -0,0 +1,13 @@ +namespace ExchangeRateUpdater +{ + public class CnbOptions + { + public const string SectionName = "Cnb"; + + public string DailyKurzUrl { get; set; } + public int TotalTimeoutSeconds { get; set; } = 10; + public int AttemptTimeoutSeconds { get; set; } = 3; + public int RetryCount { get; set; } = 2; + public int RetryDelayMilliseconds { get; set; } = 250; + } +} \ No newline at end of file diff --git a/jobs/Backend/Task/Currency.cs b/jobs/Backend/Task/Currency.cs index f375776f25..aecde2ba51 100644 --- a/jobs/Backend/Task/Currency.cs +++ b/jobs/Backend/Task/Currency.cs @@ -1,4 +1,6 @@ -namespace ExchangeRateUpdater +using System; + +namespace ExchangeRateUpdater { public class Currency { @@ -16,5 +18,15 @@ public override string ToString() { return Code; } + + public override bool Equals(object obj) + { + return obj is Currency currency && currency.Code.Equals(Code, StringComparison.OrdinalIgnoreCase); + } + + public override int GetHashCode() + { + return StringComparer.OrdinalIgnoreCase.GetHashCode(Code); + } } } diff --git a/jobs/Backend/Task/ExchangeRateProvider.cs b/jobs/Backend/Task/ExchangeRateProvider.cs index 6f82a97fbe..514982ada2 100644 --- a/jobs/Backend/Task/ExchangeRateProvider.cs +++ b/jobs/Backend/Task/ExchangeRateProvider.cs @@ -1,10 +1,17 @@ using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; namespace ExchangeRateUpdater { public class ExchangeRateProvider { + private readonly IExchangeRateSource _exchangeRateSource; + + public ExchangeRateProvider(IExchangeRateSource exchangeRateSource) + { + _exchangeRateSource = exchangeRateSource; + } /// /// Should return exchange rates among the specified currencies that are defined by the source. But only those defined /// by the source, do not return calculated exchange rates. E.g. if the source contains "CZK/USD" but not "USD/CZK", @@ -13,7 +20,23 @@ public class ExchangeRateProvider /// public IEnumerable GetExchangeRates(IEnumerable currencies) { - return Enumerable.Empty(); + var cnbRates = GetSourceExchangeRates(); + var filteredRates = GetFilteredRates(cnbRates, currencies); + return filteredRates; + } + + private IEnumerable GetSourceExchangeRates() + { + // Public API stays synchronous per assignment; the source uses async HTTP. + var task = _exchangeRateSource.GetExchangeRates(); + return task.ConfigureAwait(false).GetAwaiter().GetResult(); + } + + private IEnumerable GetFilteredRates(IEnumerable cnbRates, IEnumerable currencies) + { + return cnbRates.Where(rate => currencies.Contains(rate.SourceCurrency)); } } } + + diff --git a/jobs/Backend/Task/ExchangeRateUpdater.csproj b/jobs/Backend/Task/ExchangeRateUpdater.csproj index 2fc654a12b..996e72bb70 100644 --- a/jobs/Backend/Task/ExchangeRateUpdater.csproj +++ b/jobs/Backend/Task/ExchangeRateUpdater.csproj @@ -2,7 +2,18 @@ Exe - net6.0 + net10.0 + PreserveNewest + + + + + + + + + + \ No newline at end of file diff --git a/jobs/Backend/Task/IExchangeRateSource.cs b/jobs/Backend/Task/IExchangeRateSource.cs new file mode 100644 index 0000000000..ccb7874174 --- /dev/null +++ b/jobs/Backend/Task/IExchangeRateSource.cs @@ -0,0 +1,11 @@ +using System.Collections.Generic; +using ExchangeRateUpdater; +using System.Threading.Tasks; + +namespace ExchangeRateUpdater +{ + public interface IExchangeRateSource + { + public Task> GetExchangeRates(); + } +} \ No newline at end of file diff --git a/jobs/Backend/Task/Program.cs b/jobs/Backend/Task/Program.cs index 379a69b1f8..f1c26f422c 100644 --- a/jobs/Backend/Task/Program.cs +++ b/jobs/Backend/Task/Program.cs @@ -1,6 +1,12 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Http.Resilience; +using Microsoft.Extensions.Hosting; +using Polly; namespace ExchangeRateUpdater { @@ -23,14 +29,45 @@ public static void Main(string[] args) { try { - var provider = new ExchangeRateProvider(); - var rates = provider.GetExchangeRates(currencies); + var builder = Host.CreateApplicationBuilder(new HostApplicationBuilderSettings + { + Args = args, + ContentRootPath = AppContext.BaseDirectory, + }); + + builder.Services.Configure( + builder.Configuration.GetSection(CnbOptions.SectionName)); + + var cnbOptions = builder.Configuration + .GetSection(CnbOptions.SectionName) + .Get() ?? new CnbOptions(); + ValidateCnbOptions(cnbOptions); + + builder.Services + .AddHttpClient(client => + { + client.Timeout = Timeout.InfiniteTimeSpan; + }) + .AddStandardResilienceHandler(options => + { + options.TotalRequestTimeout.Timeout = + TimeSpan.FromSeconds(cnbOptions.TotalTimeoutSeconds); + options.AttemptTimeout.Timeout = + TimeSpan.FromSeconds(cnbOptions.AttemptTimeoutSeconds); + options.Retry.MaxRetryAttempts = cnbOptions.RetryCount; + options.Retry.Delay = + TimeSpan.FromMilliseconds(cnbOptions.RetryDelayMilliseconds); + options.Retry.BackoffType = DelayBackoffType.Exponential; + options.Retry.UseJitter = true; + }); + + builder.Services.AddTransient(); + using var host = builder.Build(); + var exchangeRateProvider = host.Services.GetRequiredService(); + var rates = exchangeRateProvider.GetExchangeRates(currencies); Console.WriteLine($"Successfully retrieved {rates.Count()} exchange rates:"); - foreach (var rate in rates) - { - Console.WriteLine(rate.ToString()); - } + foreach (var rate in rates) Console.WriteLine(rate.ToString()); } catch (Exception e) { @@ -39,5 +76,39 @@ public static void Main(string[] args) Console.ReadLine(); } + + private static void ValidateCnbOptions(CnbOptions options) + { + if (string.IsNullOrWhiteSpace(options.DailyKurzUrl) + || !Uri.TryCreate(options.DailyKurzUrl, UriKind.Absolute, out _)) + { + throw new InvalidOperationException("Cnb:DailyKurzUrl must be configured as an absolute URL."); + } + + if (options.TotalTimeoutSeconds <= 0) + { + throw new InvalidOperationException("Cnb:TotalTimeoutSeconds must be greater than zero."); + } + + if (options.AttemptTimeoutSeconds <= 0) + { + throw new InvalidOperationException("Cnb:AttemptTimeoutSeconds must be greater than zero."); + } + + if (options.AttemptTimeoutSeconds > options.TotalTimeoutSeconds) + { + throw new InvalidOperationException("Cnb:AttemptTimeoutSeconds must not exceed Cnb:TotalTimeoutSeconds."); + } + + if (options.RetryCount < 0) + { + throw new InvalidOperationException("Cnb:RetryCount must not be negative."); + } + + if (options.RetryDelayMilliseconds < 0) + { + throw new InvalidOperationException("Cnb:RetryDelayMilliseconds must not be negative."); + } + } } } diff --git a/jobs/Backend/Task/appsettings.json b/jobs/Backend/Task/appsettings.json new file mode 100644 index 0000000000..ec857e07cf --- /dev/null +++ b/jobs/Backend/Task/appsettings.json @@ -0,0 +1,9 @@ +{ + "Cnb": { + "DailyKurzUrl": "https://www.cnb.cz/cs/financni_trhy/devizovy_trh/kurzy_devizoveho_trhu/denni_kurz.txt", + "TotalTimeoutSeconds": 10, + "AttemptTimeoutSeconds": 3, + "RetryCount": 2, + "RetryDelayMilliseconds": 250 + } + } \ No newline at end of file diff --git a/jobs/Backend/docs/DECISIONS.md b/jobs/Backend/docs/DECISIONS.md new file mode 100644 index 0000000000..47867591ca --- /dev/null +++ b/jobs/Backend/docs/DECISIONS.md @@ -0,0 +1,151 @@ +# Engineering Decision Log + +This file records lightweight technical decisions made during this task. + +## CNB daily rates URL in `appsettings.json` and HTTP via `IHttpClientFactory` + +### Context + +`CnbExchangeRateSource` needs the public CNB daily rates document URL. We also want to avoid creating a new `HttpClient` on every call (socket churn and related issues). + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Store the CNB daily kurz URL in [`Task/appsettings.json`](../Task/appsettings.json), read through `IConfiguration`. | Separates environment-specific or future URL changes from compiled logic and matches common .NET hosting patterns. | The console app composition root ([`Task/Program.cs`](../Task/Program.cs)) should build configuration and bind/pass settings into `CnbExchangeRateSource`. | +| Use `IHttpClientFactory` via `AddHttpClient` to obtain `HttpClient` instances. | This is the recommended default for production-style .NET apps and addresses lifetime concerns that a per-call `new HttpClient()` does not. | Additional NuGet packages are expected for configuration and HTTP client extensions, such as `Microsoft.Extensions.Configuration.Json`, `Microsoft.Extensions.Http`, and hosting/DI primitives as needed. | + +## Generic host for console bootstrap (`Host.CreateApplicationBuilder`) + +### Context + +The Task executable needs JSON configuration (e.g. CNB URL), options binding (`Configure`), and `AddHttpClient` registration. The main alternative is a **manual** stack: `ConfigurationBuilder` + `ServiceCollection` without a host. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Bootstrap the console app with `Host.CreateApplicationBuilder(args)`. | It gives one conventional pipeline for default `appsettings` loading, environment-specific files, configuration, and DI. | Add `Microsoft.Extensions.Hosting` and `Microsoft.Extensions.Http` to the Task project; implement [`Task/Program.cs`](../Task/Program.cs) around the host builder instead of only `new`ing types in `Main`. | +| Use `builder.Configuration` and `builder.Services` for options, HTTP clients, and DI registrations, then `Build()` and resolve services. | This uses less boilerplate than assembling `ConfigurationBuilder`, `Build()`, and `ServiceCollection` by hand for the same features. | `Configure(builder.Configuration.GetSection(...))` and related registrations live next to other `builder.Services` calls in the composition root. | +| Do not use only a manual `ServiceCollection` + `ConfigurationBuilder` stack for this task. | The manual stack is valid when minimizing dependencies or when another host already owns configuration/DI, but that is not needed here. | The project follows the generic-host style consistently. | + +## Use `Microsoft.Extensions.Options` (`IOptions`), not a custom `IOptions` type + +### Context + +It is tempting to add a small project-local interface (e.g. a non-generic `IOptions` with `DailyKurzUrl`) to abstract configuration for [`CnbExchangeRateSource`](../Task/CnbExchangeRateSource.cs). + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Do not introduce a custom `IOptions` or similarly named interface in the Task project. | `IOptions` and `IOptions` are standard .NET names; a local `IOptions` type would collide conceptually and confuse readers, docs, and `using` resolution. | [`CnbExchangeRateSource`](../Task/CnbExchangeRateSource.cs) should not take a hand-rolled options interface. | +| Inject `IOptions` from `Microsoft.Extensions.Options`, bind settings with `Configure(...)`, and read the URL via `options.Value`. | `Configure` is already the chosen wiring, and `IOptions` is the intended consumer API for that binding. | [`CnbExchangeRateSource`](../Task/CnbExchangeRateSource.cs) should take `IOptions` plus `HttpClient` from `IHttpClientFactory`. | +| If extra indirection is needed later, use a distinct custom name such as `ICnbSettings`. | A distinct name avoids overlap with the standard `IOptions` pattern. | Remove or avoid any obsolete `IOptions.cs` in [`Task`](../Task) that duplicates this role. | + +## Composition root in `Program.cs` (no default `ExchangeRateProvider` ctor) + +### Context + +`ExchangeRateProvider` depends on `IExchangeRateSource`. A common shortcut is a parameterless constructor that chains to `new CnbExchangeRateSource()` (or similar) so callers can write `new ExchangeRateProvider()` without wiring. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Do not add a parameterless `ExchangeRateProvider()` that internally `new`s a concrete source. | Dependencies stay visible at the application entry point, and `ExchangeRateProvider` avoids hard-coding a single concrete source implementation. | Every runnable entry point must create or receive an `IExchangeRateSource` before constructing `ExchangeRateProvider`. | +| Treat [`Program.cs`](../Task/Program.cs) as the composition root. | This aligns with explicit DI-style composition without pulling in a full container for this small task. | Unit tests continue to inject fakes via the same constructor; no convenience ctor is required for production. | +| Do not use a parameterless constructor that delegates to `new CnbExchangeRateSource()`. | That shortcut would reduce lines in `Program`, but it hides the dependency and couples the provider to one default implementation. | The provider remains source-agnostic and easier to test. | + +## CNB source owns CNB-specific parsing + +### Context + +`IExchangeRateSource` could either return the raw CNB daily rates text (for example `Task`) and leave parsing to `ExchangeRateProvider`, or return normalized `ExchangeRate` objects after handling the source-specific document format. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| `IExchangeRateSource` returns parsed `ExchangeRate` objects. | Avoiding `Task` in `ExchangeRateProvider` prevents the provider from becoming coupled to one source's transport format and keeps its responsibility focused on orchestration and filtering. | Provider tests should fake `IExchangeRateSource` by returning `ExchangeRate` instances, not raw text snippets. | +| `CnbExchangeRateSource` owns both fetching the CNB daily document and parsing the CNB-specific text format. | The CNB document shape, including header rows, pipe-delimited columns, comma/dot decimal handling, and amount/rate normalization, is specific to the CNB implementation. | CNB parsing tests belong with `CnbExchangeRateSource` and can use stubbed HTTP responses. | +| `ExchangeRateProvider` receives already-normalized rates from `IExchangeRateSource`. | Keeping parsing in the concrete source keeps `ExchangeRateProvider` generic: it can orchestrate source -> filter -> return without knowing about CNB text files. | If another bank/source is added later, it should get its own implementation that fetches and parses its own format into `ExchangeRate` objects. | +| Keep parser helpers such as `ParseCnbDailyKurz` non-public. | Public members should describe supported production behavior, not expose implementation details solely for test access. | Parsing coverage should exercise `CnbExchangeRateSource.GetExchangeRates(...)` with controlled HTTP responses instead of calling `ParseCnbDailyKurz` directly. | +| Do not introduce a parser interface for now. | A parser interface would add ceremony before there are multiple parsers or a need to test parsing independently from the source class. | Revisit this only if multiple parsers or a separate parsing lifecycle appear. | + +## Use xUnit for unit tests + +### Context + +We need a first unit-test setup for the .NET backend task and must choose a test framework. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Use `xUnit` as the default unit testing framework for `Task.Tests`. | It has strong and common ecosystem support in modern .NET projects, a simple `[Fact]` / `[Theory]` model, smooth `dotnet test` integration, and minimal setup overhead for a small codebase. | Test examples and conventions in this repository should assume xUnit attributes and assertions. | +| Prefer xUnit's constructor-based setup and data-driven tests for this task. | Constructor-based setup encourages explicit dependencies and cleaner tests; `[Theory]` plus data attributes gives strong parameterized test support. | If a team is historically NUnit/MSTest-heavy, migration has small friction because lifecycle patterns differ. | +| Do not choose NUnit for this task. | NUnit is mature, feature-rich, familiar to many teams, and has strong parameterized testing, but there is no project-specific reason to prefer it here. | Avoids another style divergence; extra NUnit features are not required for this small task. | +| Do not choose MSTest for this task. | MSTest is Microsoft-native and enterprise-friendly, but it is usually more verbose for simple behavior-driven tests and less commonly preferred in modern community samples. | If we later need richer fluent assertions or mocking, we can add packages without changing the test framework. | + +## Keep `ExchangeRateProvider.GetExchangeRates` synchronous + +### Context + +The assignment-facing API already exposes `ExchangeRateProvider.GetExchangeRates(...)` as a synchronous method returning `IEnumerable`. The concrete CNB source uses HTTP, where the natural .NET API is asynchronous. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Keep `ExchangeRateProvider.GetExchangeRates(...)` synchronous for this task. | It preserves the public provider API expected by the exercise and existing callers. | `ExchangeRateProvider` blocks while waiting for the source result. | +| Keep `IExchangeRateSource.GetExchangeRates(...)` asynchronous so the source can use async HTTP APIs. | It keeps HTTP-specific async behavior inside the source abstraction instead of pushing it through the whole console app for this small task. | This is acceptable for the current console assignment, but a server or high-concurrency app should prefer an async provider API such as `GetExchangeRatesAsync`. | +| Use a deliberate sync-over-async boundary inside `ExchangeRateProvider` when calling the source. | The boundary is explicit and localized, so it can be changed later if the public API moves to async. | If the task evolves to support a fully async public surface, update `Program.cs`, provider tests, and this decision. | + +## Keep requested-currency filtering in `ExchangeRateProvider` + +### Context + +`IExchangeRateSource.GetExchangeRates(...)` accepted the requested currencies even though the current source design returns all parsed CNB rows and the provider filters them afterward. + +CNB publishes rates as foreign currency against CZK, so `CZK` is implicit in each parsed rate such as `USD/CZK`. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Make `IExchangeRateSource.GetExchangeRates()` parameterless. | The source contract stays focused on fetching and parsing source-provided rates. | Source implementations cannot selectively fetch or filter by requested currencies through the interface. | +| Keep the requested `Currency` collection on `ExchangeRateProvider.GetExchangeRates(...)`, where filtering happens. | `ExchangeRateProvider` remains the single place that applies the assignment's requested-currency filtering rules. Avoids an unused parameter in `CnbExchangeRateSource` and fake source implementations. | If a future source needs request-aware fetching for performance, revisit the source contract deliberately. | +| Treat requested currencies as requested source currencies; `USD` is enough to return the source-provided `USD/CZK` rate. | This matches CNB's real data format and avoids requiring callers to pass `CZK` redundantly when every CNB rate is already against CZK. | This assumes the source's target currency is implicit. If a future source supports multiple target currencies, revisit the provider API, for example with `sourceCurrencies + targetCurrency` or explicit currency pairs. | + +## Configure CNB HTTP resilience + +### Context + +The CNB source calls an external HTTP endpoint. In production-style code, a stalled or transiently failing network call should be bounded and retried deliberately instead of relying on one fixed `HttpClient.Timeout` value. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Store CNB resilience settings in [`Task/appsettings.json`](../Task/appsettings.json): total timeout, per-attempt timeout, retry count, and retry delay. | These values are operational knobs, not parsing or domain logic. Keeping them in configuration lets environments tune CNB behavior without code changes. | [`Task/CnbOptions.cs`](../Task/CnbOptions.cs) now carries both the CNB URL and resilience settings. | +| Validate the CNB URL and resilience settings in [`Task/Program.cs`](../Task/Program.cs) before registering the HTTP client. | A missing URL or invalid timeout should fail with a clear configuration error instead of becoming a late HTTP or resilience-pipeline failure. | The console app reports configuration mistakes at startup through the existing top-level error handling. | +| Use `Microsoft.Extensions.Http.Resilience` with `AddStandardResilienceHandler` on the typed CNB `HttpClient`. | Retries and timeouts belong at the HTTP boundary. The standard handler provides a production-ready pipeline for retries, attempt timeout, total timeout, and jittered exponential backoff without a hand-written retry loop. | [`Task/Program.cs`](../Task/Program.cs) configures the resilience policy; [`Task/CnbExchangeRateSource.cs`](../Task/CnbExchangeRateSource.cs) remains focused on fetching through `HttpClient` and parsing CNB text. | +| Set `HttpClient.Timeout` to `Timeout.InfiniteTimeSpan` and let the resilience pipeline own request timing. | Stacking `HttpClient.Timeout` on top of resilience timeouts can produce competing cancellation behavior and harder-to-explain failures. | Timeout failures are controlled by the configured attempt and total request timeouts. | + +## Target .NET 10 LTS + +### Context + +The backend task should look like something we would maintain long term. The projects previously targeted .NET 9, which was a short-term support release. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Target `net10.0` for both the console app and test project. | .NET 10 is the current LTS line, so it is a better match for the task's production-maintenance framing than .NET 9 STS. | Local and reviewer machines need the .NET 10 SDK installed to restore, build, and test. | +| Align `Microsoft.Extensions.Hosting` and `Microsoft.Extensions.Http` package references to the latest stable 10.x packages selected by NuGet. | Keeping extension packages on the same major version as the target framework avoids unnecessary version skew. | Package restore now depends on the 10.x Microsoft.Extensions package line. | +| Add `Microsoft.Extensions.Http.Resilience` for the CNB HTTP policy. | This is the standard .NET HTTP resilience package and avoids maintaining custom retry code. | Restore also brings the related Microsoft resilience and diagnostics packages. | + +## Project docs and AI collaboration defaults + +### Context + +We want consistent planning and decision history, and a predictable way to collaborate with AI assistants (step-by-step discussion, no silent architectural choices). + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Keep the versioned implementation plan in [`PLAN.md`](PLAN.md) under `jobs/Backend/docs` and update it when the plan changes. | `PLAN.md` is reviewable in git and independent of IDE-generated plan files. | When scope or design shifts, edit `PLAN.md`. Optional Cursor plans under `.cursor/plans/` may still exist; treat `PLAN.md` as authoritative unless the team agrees otherwise. | +| Record substantive technical choices in [`DECISIONS.md`](DECISIONS.md). | `DECISIONS.md` stays the single place for why we chose a given approach. | Add a short entry here when a shift reflects a real decision, such as libraries, new types, or major tradeoffs. | +| Encode collaboration preferences in [`.cursor/rules/collaboration-and-docs.mdc`](../../../.cursor/rules/collaboration-and-docs.mdc). | Cursor rules give stable defaults for future sessions without repeating instructions. | Future AI sessions should work step by step and avoid silent architectural choices. | + +## No root `AGENTS.md` (for now) + +### Context + +We considered adding a root-level `AGENTS.md` for discoverability and cross-tool conventions, versus relying only on Cursor project rules and backend docs. + +| Decision | Why this choice | Consequences | +| --- | --- | --- | +| Do not add a repository root `AGENTS.md` at this stage. | Avoids duplication and drift between `AGENTS.md` and `.cursor/rules` if both repeated the same instructions. | Anyone looking for agent collaboration guidance should open `.cursor/rules/` and the backend `docs/PLAN.md` / `docs/DECISIONS.md` files. | +| Treat [`.cursor/rules/collaboration-and-docs.mdc`](../../../.cursor/rules/collaboration-and-docs.mdc), [`PLAN.md`](PLAN.md), and this [`DECISIONS.md`](DECISIONS.md) as authoritative. | Cursor's primary hook for persistent guidance is `.cursor/rules/`; a second root file does not add much for Cursor-only workflows on a small repo. | If we add `AGENTS.md` later, keep it as a short index of links unless we explicitly deprecate overlap with `.cursor/rules`. | diff --git a/jobs/Backend/docs/PLAN.md b/jobs/Backend/docs/PLAN.md new file mode 100644 index 0000000000..c3c0279aff --- /dev/null +++ b/jobs/Backend/docs/PLAN.md @@ -0,0 +1,75 @@ +# ExchangeRateProvider implementation plan + +Authoritative task plan for the `jobs/Backend` exercise. Update this file when the approach or scope changes. + +## Architecture and principles + +The solution keeps the exchange-rate workflow split into small responsibilities. `ExchangeRateProvider` orchestrates the use case: it asks a source for rates, filters them by requested source currency, and returns only source-provided rates. CNB-specific HTTP fetching and text parsing stay inside `CnbExchangeRateSource`. + +The main architectural pattern is dependency injection. `ExchangeRateProvider` depends on the `IExchangeRateSource` abstraction instead of constructing `CnbExchangeRateSource` directly. `CnbExchangeRateSource` also works as an adapter around the external CNB daily-rate feed: it translates the external pipe-delimited text format into the internal `ExchangeRate` model, so the rest of the application does not need to know about CNB headers, columns, decimal separators, or amount normalization. + +The design follows several SOLID principles: + +- **Single Responsibility Principle:** `ExchangeRateProvider` handles orchestration and filtering, while `CnbExchangeRateSource` handles CNB fetching and parsing. +- **Dependency Inversion Principle:** `ExchangeRateProvider` depends on `IExchangeRateSource`, not on the concrete CNB implementation. +- **Open/Closed Principle:** another source could be added by implementing `IExchangeRateSource` without changing the provider’s filtering logic. +- **Interface Segregation Principle:** `IExchangeRateSource` exposes only the operation the provider needs: retrieving exchange rates. + +The solution also uses standard .NET infrastructure patterns: the Options pattern for CNB configuration, `IHttpClientFactory` for HTTP client lifetime management, and a `Currency` value-object style equality implementation so separate `Currency` instances with the same ISO code compare correctly. + +## Design + +- **Source contract:** `IExchangeRateSource` exposes a parameterless method that returns parsed `ExchangeRate` objects. The concrete source owns any source-specific fetching and parsing needed to produce those objects; requested-currency filtering belongs in `ExchangeRateProvider`. +- **CNB parsing ownership:** CNB text parsing stays inside `CnbExchangeRateSource` for now, because the pipe-delimited CNB daily document format is specific to that source. A parser interface is unnecessary unless parsing grows enough to justify a separate type. +- **Abstraction name:** `IExchangeRateSource` (concrete implementation provided separately). +- **Provider role:** [`Task/ExchangeRateProvider.cs`](../Task/ExchangeRateProvider.cs) orchestrates: call source → filter → return. It should not know CNB document shape, header lines, `|` columns, decimal separators, or amount/rate normalisation rules. +- **Testability:** Provider tests use a fake `IExchangeRateSource` returning fixed `ExchangeRate` instances. CNB HTTP and parsing behaviour belong in dedicated `CnbExchangeRateSource` tests with stubbed HTTP. + +## Project layout (`IExchangeRateSource` placement) + +For this task’s size, a **separate folder is not required**. Keeping `IExchangeRateSource` and its CNB implementation as **one or two `.cs` files next to** [`Task/ExchangeRateProvider.cs`](../Task/ExchangeRateProvider.cs) under [`Task`](../Task) is clear and easy to navigate. + +Introduce a subfolder (e.g. `Sources/`, `Infrastructure/`, or `Cnb/`) only if we prefer that mental grouping or expect several implementations and parsers to accumulate. It is a readability preference, not a technical requirement here. + +## End-to-end flow + +```mermaid +flowchart LR + Program[Program] + Provider[ExchangeRateProvider] + Source[IExchangeRateSource] + CnbSource[CnbExchangeRateSource] + Fetch[Fetch_CNB_text] + Parse[Parse_CNB_text_to_rates] + Filter[GetFilteredRates] + Program --> Provider + Provider --> Source + Source --> CnbSource + CnbSource --> Fetch + Fetch --> Parse + Parse --> Provider + Provider --> Filter +``` + + + +## Implementation Steps + +1. **Fetch** — Implement `IExchangeRateSource` + concrete CNB type (CNB URL, `HttpClient`, options). Inject `IExchangeRateSource` into `ExchangeRateProvider` via constructor. The concrete source fetches the raw CNB daily text and returns parsed `ExchangeRate` objects without taking requested currencies. +2. **Parse** — Inside `CnbExchangeRateSource`, from the raw text: skip CNB header lines, split data lines by `|`, read country/code/amount/rate fields, **normalise** “rate per `Amount` units” into a single `decimal` suitable for `ExchangeRate.Value`, and build `ExchangeRate` instances with the **correct** `SourceCurrency` / `TargetCurrency` convention (CNB publishes foreign currency vs CZK — match what the task expects, typically one leg CZK). +3. **Filter** — Keep using `GetFilteredRates` logic for requested source currencies and **do not** synthesise inverse pairs ([`Task.Tests/ExchangeRateProviderFilteringTests.cs`](../Task.Tests/ExchangeRateProviderFilteringTests.cs) encodes that). CNB publishes rates as foreign currency against CZK, so `CZK` is implicit and does not need to be requested. [`Task/Currency.cs`](../Task/Currency.cs) now implements value equality by `Code`, so `currencies.Contains(rate.SourceCurrency)` works for separate `Currency` instances with the same ISO code. +4. **Return** — `GetExchangeRates` returns `IEnumerable` as today + +## Wiring and tests + +- **Composition:** [`Task/Program.cs`](../Task/Program.cs) is the composition root. It wires the real `IExchangeRateSource` implementation, binds and validates CNB options from configuration, applies the typed `HttpClient` resilience policy, and passes the source to `ExchangeRateProvider` via DI. +- **Tests:** + - [`Task.Tests/ExchangeRateProviderTests.cs`](../Task.Tests/ExchangeRateProviderTests.cs): pass a fake `IExchangeRateSource`; assert empty vs non-empty using canned `ExchangeRate` instances. + - [`Task.Tests/ExchangeRateProviderFilteringTests.cs`](../Task.Tests/ExchangeRateProviderFilteringTests.cs): invoke `GetFilteredRates` through reflection with an `ExchangeRateProvider` constructed from a dummy fake source. + - [`Task.Tests/CnbExchangeRateSourceIntegrationTests.cs`](../Task.Tests/CnbExchangeRateSourceIntegrationTests.cs): use stubbed HTTP responses to cover CNB document parsing and HTTP failures. + +## Production hardening + +- **Retries and timeouts:** CNB HTTP calls use the .NET HTTP resilience extensions with configurable total timeout, per-attempt timeout, retry count, retry delay, exponential backoff, and jitter. The source class stays focused on fetching and parsing; the HTTP policy lives in the composition root. +- **Freshness:** CNB daily-rate files include a publication date in the header. A future iteration could parse that date, log it with the parsed rates, expose it alongside the returned data if the public model evolves, and warn when the document appears unexpectedly stale. Any staleness threshold should account for weekends and bank holidays. + diff --git a/jobs/Backend/docs/TEST_CASES.md b/jobs/Backend/docs/TEST_CASES.md new file mode 100644 index 0000000000..36fb94266c --- /dev/null +++ b/jobs/Backend/docs/TEST_CASES.md @@ -0,0 +1,80 @@ +# Test Cases + +This file documents the current test coverage for the backend exchange-rate task. + +## Test Files + + +| File | Scope | Purpose | +| ------------------------------------------------------------------ | ---------------------- | ------------------------------------------------------------------------------------------------------------------ | +| `jobs/Backend/Task.Tests/ExchangeRateProviderTests.cs` | Unit | Public `ExchangeRateProvider.GetExchangeRates` behavior with a fake `IExchangeRateSource`. | +| `jobs/Backend/Task.Tests/ExchangeRateProviderFilteringTests.cs` | Unit | Private `ExchangeRateProvider.GetFilteredRates` behavior, invoked through reflection. | +| `jobs/Backend/Task.Tests/CnbExchangeRateSourceIntegrationTests.cs` | Integration-style unit | `CnbExchangeRateSource` HTTP response handling and CNB daily document parsing with a stubbed `HttpMessageHandler`. | + + +## Provider Unit Test Matrix + +These tests exercise the public provider method: + +`ExchangeRateProvider.GetExchangeRates(IEnumerable currencies)` + + +| Test case | Source rates | Requested currencies | Expected result | +| --------------------------------------------------- | ------------------------------- | -------------------- | -------------------------------------------------------------- | +| Source returns no rates | None | `USD` | Empty result. | +| One existing source currency requested | `USD/CZK = 25` | `USD` | Returns the `USD/CZK` rate. | +| Requested currency has same code as source currency | `new Currency("USD")/CZK = 25` | separate `USD` | Returns the rate because `Currency` equality is based on code. | +| No rate matches request | `GBP/JPY = 150` | `USD` | Empty result. | +| Only unknown requested currencies | `USD/CZK = 25` | `XYZ` | Empty result. | +| Multiple source rates, one requested source | `USD/CZK = 25`, `EUR/CZK = 24` | `USD` | Returns only `USD/CZK`. | +| Multiple source currencies requested | `USD/CZK`, `EUR/CZK`, `JPY/CZK` | `USD`, `EUR` | Returns `USD/CZK` and `EUR/CZK`; excludes `JPY/CZK`. | +| Mix of known and unknown requested currencies | `USD/CZK = 25` | `USD`, `XYZ` | Returns `USD/CZK`; ignores `XYZ`. | +| Source publishes single direction | `USD/CZK = 25` | `USD` | Returns only `USD/CZK`; does not synthesize `CZK/USD`. | +| Empty request | `USD/CZK = 25` | None | Empty result. | + + +## Filtering Unit Test Matrix + +These tests exercise the provider's private filtering method directly through reflection: + +`ExchangeRateProvider.GetFilteredRates(IEnumerable cnbRates, IEnumerable currencies)` + + +| Test case | Input rates | Requested currencies | Expected result | +| --------------------------------------------- | ------------------------------ | -------------------- | --------------------------------------------------------- | +| Returns rates for requested source currencies | `USD/CZK = 25`, `EUR/CZK = 24` | `USD` | Returns only `USD/CZK`. | +| Ignores unknown requested currencies | `USD/CZK = 25` | `XYZ` | Empty result. | +| Does not create inverse pairs | `USD/CZK = 25` | `USD` | Returns only the source-provided `USD/CZK`; no `CZK/USD`. | + + +## CNB Source Integration Test Matrix + +These tests exercise the CNB source method using controlled HTTP responses: + +`CnbExchangeRateSource.GetExchangeRates()` + +Filtering by requested currencies is intentionally not covered here; the source returns parsed rows and the provider filters them. + + +| Scenario | HTTP body shape | Expected parsed rates | +| -------------------------------------------- | ---------------------------------------------- | -------------------------------------- | +| `single_row_amount_one_dot_decimal` | CNB headers plus one USD row with dot decimal | Parses one `USD/CZK` rate. | +| `amount_100_normalises_to_per_unit` | CNB headers plus one JPY row with amount `100` | Normalizes the JPY rate per one unit. | +| `multiple_rows` | CNB headers plus USD and EUR rows | `USD/CZK = 20.648`, `EUR/CZK = 24.305` | +| `comma_decimal_czech_style` | CNB headers plus one USD row with comma decimal | Parses one `USD/CZK` rate. | +| `malformed_pipe_row_skipped_valid_rows_kept` | Malformed row, valid USD row, empty pipe row | Keeps only `USD/CZK = 1`. | +| `empty_body` | Empty HTTP body | Empty result. | +| `headers_only_no_data_rows` | CNB headers without data rows | Empty result. | +| `whitespace_trimmed_in_fields` | CNB headers plus fields padded with whitespace | `USD/CZK = 10` | + + +## CNB Source Error Handling Matrix + +Method under test: + +`CnbExchangeRateSource.GetExchangeRates()` + + +| Test case | HTTP status | Expected result | +| ------------------ | --------------- | ------------------------------ | +| HTTP request fails | `404 Not Found` | Throws `HttpRequestException`. |