feat(cex-market-data): add CoinPaprika API provider as a fallback option#215
feat(cex-market-data): add CoinPaprika API provider as a fallback option#215
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds CoinPaprika provider/repository/models and integrates it into bootstrap, selection, and sparkline flows. Introduces index_generator with private barrel files, centralizes error handling via ApiErrorParser, refactors priorities/selection, expands OHLC/model enums, updates public exports, and adjusts constructors/usages. Includes extensive tests and example app updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant Bootstrap as MarketDataBootstrap
participant DI as DI Container
participant P as Providers
participant R as Repositories
App->>Bootstrap: init(config: enableCoinPaprika?)
Bootstrap->>P: register Binance/CoinGecko/(CoinPaprika?)
Bootstrap->>R: build repositories list (priority order)
Bootstrap->>DI: register providers & repositories
DI-->>App: ready
sequenceDiagram
autonumber
participant Caller
participant Sparkline as SparklineRepository
participant Select as RepositorySelectionStrategy
participant Repo1 as BinanceRepo
participant Repo2 as CoinPaprikaRepo
participant Repo3 as CoinGeckoRepo
Caller->>Sparkline: fetchSparkline(assetId)
Sparkline->>Select: selectRepository(assetId, fiat, requestType)
Select->>Repo1: supports(...)
alt Repo1 supports
Select-->>Sparkline: Repo1
else Repo1 not supported
Select->>Repo2: supports(...)
alt Repo2 supports
Select-->>Sparkline: Repo2
else none supports
Select-->>Sparkline: null
end
end
alt Repo selected
Sparkline->>RepoX: getCoinOhlc(..., batching if needed)
RepoX->>RepoX: batch requests (plan limits), 100ms gaps
RepoX-->>Sparkline: aggregated data
Sparkline-->>Caller: sparkline
else no repo
Sparkline-->>Caller: null/empty
end
sequenceDiagram
autonumber
participant Provider as CoinGecko/CoinPaprika Provider
participant API as HTTP API
participant Parser as ApiErrorParser
Provider->>API: request
API-->>Provider: non-200 response
Provider->>Parser: parse(status, body)
Parser-->>Provider: ApiError + safe message
Provider-->>Caller: throw parsed error (safe message)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds CoinPaprika as a fallback option for CEX market data, implementing a comprehensive CoinPaprika API provider with proper error handling, plan validation, and seamless integration into the existing repository selection strategy.
- Implements CoinPaprika API provider with support for coin data, ticker information, market data, and historical OHLC data
- Adds comprehensive API plan validation system supporting Free, Starter, Business, Pro, Ultimate, and Enterprise tiers
- Updates repository priority system to include CoinPaprika as the third priority option (after Komodo and Binance, before CoinGecko)
Reviewed Changes
Copilot reviewed 78 out of 85 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/komodo_defi_sdk/example/lib/widgets/assets/asset_item.dart | Updates sparkline widget to use repository provider pattern and conditionally display sparklines based on testnet status |
| packages/komodo_defi_sdk/example/lib/main.dart | Integrates SparklineRepository into the example app's dependency injection setup |
| packages/komodo_defi_sdk/analysis_options.yaml | Updates very_good_analysis version from 6.0.0 to 7.0.0 |
| packages/komodo_cex_market_data/test/sparkline_repository_test.dart | Updates test to use new SparklineRepository constructor pattern |
| packages/komodo_cex_market_data/test/repository_selection_strategy_test.dart | Refactors imports and adds comprehensive tests for repository selection with quote currency mapping |
| packages/komodo_cex_market_data/test/repository_priority_manager_test.dart | Updates priority expectations to account for CoinPaprika's new position in the hierarchy |
| packages/komodo_cex_market_data/test/integration_test.dart | Updates SparklineRepository constructor usage in integration tests |
| packages/komodo_cex_market_data/test/common/api_error_parser_test.dart | Adds comprehensive test coverage for API error parsing with security validation |
| packages/komodo_cex_market_data/test/coinpaprika_provider_validation_test.dart | Implements extensive validation tests for CoinPaprika API plans and provider behavior |
| packages/komodo_cex_market_data/test/coinpaprika/fixtures/ | Adds complete test infrastructure with helpers, constants, fixtures, and verification utilities |
| packages/komodo_cex_market_data/test/coinpaprika/coinpaprika_repository_test.dart | Comprehensive repository tests covering all functionality including batching and validation |
| packages/komodo_cex_market_data/test/coinpaprika/coinpaprika_cex_provider_test.dart | Detailed provider tests validating URL formatting, parameter handling, and API interactions |
| packages/komodo_cex_market_data/test/coingecko/models/coingecko_api_plan_test.dart | Adds missing test coverage for CoinGecko API plan models |
| packages/komodo_cex_market_data/test/coingecko/coingecko_repository_test.dart | Refactors imports to use core index |
| packages/komodo_cex_market_data/test/coingecko/coingecko_cex_provider_test.dart | Code formatting improvements |
| packages/komodo_cex_market_data/test/binance/binance_repository_test.dart | Code formatting improvements |
| packages/komodo_cex_market_data/pubspec.yaml | Adds index_generator dependency for better import management |
| packages/komodo_cex_market_data/lib/src/sparkline_repository.dart | Refactors to use factory pattern and removes global variable dependencies |
| packages/komodo_cex_market_data/lib/src/repository_selection_strategy.dart | Updates selection strategy to use repository-specific supports() method instead of caching |
| packages/komodo_cex_market_data/lib/src/repository_priority_manager.dart | Updates priority mappings to include CoinPaprika at priority 3 |
| packages/komodo_cex_market_data/lib/src/models/quote_currency.dart | Adds CoinPaprika-specific quote currency extension and additional fiat currencies |
| packages/komodo_cex_market_data/lib/src/models/models.dart | Removes deprecated export file in favor of index-based imports |
| packages/komodo_cex_market_data/lib/src/models/coin_ohlc.g.dart | Adds generated code for CoinPaprika OHLC data serialization |
Comments suppressed due to low confidence (1)
packages/komodo_cex_market_data/test/coinpaprika/fixtures/test_constants.dart:1
- The FiatCurrency.uah appears twice in the extendedSupportedCurrencies list at lines 139 and 141. This duplication should be removed to avoid confusion and potential issues with currency handling.
/// Common test constants and data used across CoinPaprika tests
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/komodo_cex_market_data/lib/src/komodo/prices/komodo_price_provider.dart (1)
38-50: Add timeout and robust JSON error handling to external callNetwork calls should have timeouts and handle malformed JSON.
Apply:
- final res = await http.get(mainUri); + final res = await http.get(mainUri).timeout(const Duration(seconds: 15)); @@ - final json = jsonDecode(res.body) as Map<String, dynamic>?; - - if (json == null) { - throw Exception('Invalid response from Komodo API: empty JSON'); - } + late final Map<String, dynamic> json; + try { + final decoded = jsonDecode(res.body); + if (decoded is! Map<String, dynamic>) { + throw const FormatException('Root is not an object'); + } + json = decoded; + } on FormatException catch (e) { + throw Exception('Invalid JSON from Komodo API: ${e.message}'); + }packages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_cex_provider.dart (4)
167-178: Bug: precision query parameter is mis-keyed.precision is assigned to price_change_percentage, overwriting that param and dropping precision.
Apply:
- if (precision != null) 'price_change_percentage': precision, + if (precision != null) 'precision': precision,
499-507: Remove test-specific logic from production (test-coin shortcut).Skipping 'test-coin' in production skews results and hides issues.
Apply:
- if (coingeckoId == 'test-coin') { - return; - }
436-442: Use dd-mm-yyyy format for CoinGecko history endpoint
The/coins/{id}/historyendpoint expects the date asdd-mm-yyyy. Inpackages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_cex_provider.dart, update_formatDateto:String _formatDate(DateTime date) { final day = date.day.toString().padLeft(2, '0'); final month = date.month.toString().padLeft(2, '0'); final year = date.year.toString(); - return '$year-$month-$day'; + return '$day-$month-$year'; }
549-556: Remove test-specific logic from production provider
Eliminate theif (coingeckoId == 'test-coin') return;clause inpackages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_cex_provider.dart(around line 495). Test fixtures should be injected or mocked in tests—not hard-coded in production code.packages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_repository.dart (2)
120-144: Bug: Batched OHLC uses CoinGecko “days” endpoint, which is always relative to “now.”Splitting into batches and calling fetchCoinOhlc(tradingSymbol, vsCurrency, batchDays) repeatedly fetches overlapping “last N days” windows anchored at now, not the requested [currentStart, batchEndDate]. This returns wrong data for older ranges and can duplicate/omit segments.
Option A (preferred): use a ranged endpoint and convert to OHLC.
- Add provider support for market_chart/range and aggregate to OHLC per day (then de-dup).
- Replace the batched loop with range-based calls.
Diff in this file (expects a new provider method fetchCoinOhlcRange):
- final batchOhlc = await coinGeckoProvider.fetchCoinOhlc( - tradingSymbol, - quoteCurrency.coinGeckoId, - batchDays, - ); + final batchOhlc = await coinGeckoProvider.fetchCoinOhlcRange( + tradingSymbol, + quoteCurrency.coinGeckoId, + startAt: currentStart, + endAt: batchEndDate, + interval: interval, // pass through if supported + );If fetchCoinOhlcRange is not available yet, temporarily drop batching and fail fast with a clear message instead of returning incorrect data:
- // Split the request into multiple sequential requests to stay within free tier limits - final allOhlcData = <Ohlc>[]; - var currentStart = startAt; - while (currentStart.isBefore(endAt)) { ... } - return CoinOhlc(ohlc: allOhlcData); + throw UnsupportedError( + 'Requested range exceeds ${maxDaysAllowed} days. ' + 'Range-based OHLC is not supported by the current provider path.', + );I can provide a helper to aggregate market_chart/range -> OHLC if you want to proceed with Option A.
221-256: getCoinFiatPrices inherits the same range bug and mutates input.
- Relies on getCoinOhlc’s batched path, so it yields incorrect results for long ranges.
- Also sorts dates in place; consider copying to avoid side effects.
Apply this diff (copy list; delegate to fixed OHLC range):
- dates.sort(); + final sortedDates = [...dates]..sort(); - if (dates.isEmpty) { + if (sortedDates.isEmpty) { return {}; } - final startDate = dates.first.add(const Duration(days: -2)); - final endDate = dates.last.add(const Duration(days: 2)); + final startDate = sortedDates.first.add(const Duration(days: -2)); + final endDate = sortedDates.last.add(const Duration(days: 2));Once getCoinOhlc range is corrected, this method will produce accurate historical fiat price maps.
🧹 Nitpick comments (109)
packages/komodo_defi_sdk/analysis_options.yaml (1)
5-5: Lint override may conflict with “always_specify_types”.If 7.0.0 enables always_specify_types, keeping omit_local_variable_types ignored still enforces explicit types. If your intent is to allow
varin locals, also down-rank always_specify_types.Apply if desired:
analyzer: errors: use_if_null_to_convert_nulls_to_bools: ignore omit_local_variable_types: ignore + always_specify_types: ignoreConfirm the team’s style preference (explicit vs inferred locals) and adjust accordingly.
packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/code_additions_deletions4_weeks.dart (1)
16-18: Use concreteint?foradditions/deletions
Replace bothdynamicproperties (in the class,fromJsoncasts andcopyWith) withint?and cast viajson['additions'] as int?/json['deletions'] as int?. Only usenum?if the API might return non-integer values.packages/komodo_cex_market_data/pubspec.yaml (1)
30-30: Add CI check to keep generated indices in sync.Include a pipeline step that runs index generation and fails on diffs to prevent drift (e.g., run generator, then
git diff --exit-code).packages/komodo_cex_market_data/test/binance/binance_repository_test.dart (1)
263-266: AvoidDateTime.now()twice in the same test; capture once for consistency.This prevents edge flakiness across millisecond boundaries.
Apply within the selected lines:
- openTime: DateTime.now() - .subtract(const Duration(days: 1)) - .millisecondsSinceEpoch, + openTime: baseNow + .subtract(const Duration(days: 1)) + .millisecondsSinceEpoch,- openTime: DateTime.now() - .subtract(const Duration(hours: 24)) - .millisecondsSinceEpoch, + openTime: baseNow + .subtract(const Duration(hours: 24)) + .millisecondsSinceEpoch,Also add outside the selected ranges at the start of each test:
final baseNow = DateTime.now();Also applies to: 338-341
packages/komodo_cex_market_data/lib/src/binance/data/binance_repository.dart (1)
191-195: Nit: extract page size constant for clarity and reuse.
Makes the batching logic self-documenting.- for (var i = 0; i <= daysDiff; i += 500) { - final batchStartDate = startDate.add(Duration(days: i)); - final batchEndDate = i + 500 > daysDiff + const pageSize = 500; + for (var i = 0; i <= daysDiff; i += pageSize) { + final batchStartDate = startDate.add(Duration(days: i)); + final batchEndDate = i + pageSize > daysDiff ? endDate - : startDate.add(Duration(days: i + 500)); + : startDate.add(Duration(days: i + pageSize));packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/current_price.dart (2)
69-132: Keep JSON key → field mapping; consider normalizing btc to double for consistency.
All currencies except btc are parsed to double. Normalizing btc avoids mixed num types.- btc: json['btc'] as num?, + btc: (json['btc'] as num?)?.toDouble(),
116-117: Optional: rename tRY to a clearer identifier to avoid reserved-word aesthetics.
Consider tryCurrency (still mapped to 'try' in JSON) to improve readability; otherwise add a comment explaining tRY.Also applies to: 243-244
packages/komodo_cex_market_data/lib/src/coingecko/models/coin_market_chart.dart (1)
37-39: Nit: unnecessary identity mapping.
You could inline as a single expression (or even use prices directly) to match the other fields’ style.- 'prices': prices - .map((List<num> e) => e.map((num e) => e).toList()) - .toList(), + 'prices': prices.map((e) => e.toList()).toList(),packages/komodo_defi_sdk/example/lib/widgets/assets/asset_item.dart (3)
66-69: Guard against null protocolTicker to avoid requesting “…/null.png”.
If parentId is unexpectedly null, NetworkImage will fetch a bad URL. Provide a fallback or placeholder.- final protocolTicker = isChildAsset - ? asset.id.parentId?.id - : asset.id.subClass.iconTicker; + final protocolTicker = isChildAsset + ? asset.id.parentId?.id + : asset.id.subClass.iconTicker; @@ - CircleAvatar( - radius: 12, - foregroundImage: NetworkImage( - 'https://komodoplatform.github.io/coins/icons/${protocolTicker?.toLowerCase()}.png', - ), - backgroundColor: Colors.white70, - ), + Builder(builder: (_) { + final icon = protocolTicker?.toLowerCase(); + return CircleAvatar( + radius: 12, + foregroundImage: icon != null + ? NetworkImage( + 'https://komodoplatform.github.io/coins/icons/$icon.png', + ) + : null, + child: icon == null ? const Icon(Icons.image_not_supported, size: 12) : null, + backgroundColor: Colors.white70, + ); + }),Also applies to: 93-101
129-132: Optional: use context.read() for brevity; also safer to access in didChangeDependencies.
Current usage is fine since it doesn’t listen; this is just stylistic/idiomatic.- _sparklineFuture = RepositoryProvider.of<SparklineRepository>( - context, - ).fetchSparkline(widget.assetId); + _sparklineFuture = context.read<SparklineRepository>() + .fetchSparkline(widget.assetId);Add import adjustment if desired:
-import 'package:flutter_bloc/flutter_bloc.dart' show RepositoryProvider; +import 'package:flutter_bloc/flutter_bloc.dart'; // for RepositoryProvider + context.read
139-142: Same optional refactor as above for didUpdateWidget.
Keeps both call sites consistent.- _sparklineFuture = RepositoryProvider.of<SparklineRepository>( - context, - ).fetchSparkline(widget.assetId); + _sparklineFuture = context.read<SparklineRepository>() + .fetchSparkline(widget.assetId);packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/developer_data.dart (1)
19-33: Drop redundantas dynamiccasts (and consider stronger typing).The casts are unnecessary since the target fields are already
dynamic. Dropping them reduces noise. Optionally, considerint?/num?with(json['x'] as num?)?.toInt()for safer equality and props comparison.- forks: json['forks'] as dynamic, - stars: json['stars'] as dynamic, - subscribers: json['subscribers'] as dynamic, - totalIssues: json['total_issues'] as dynamic, - closedIssues: json['closed_issues'] as dynamic, - pullRequestsMerged: json['pull_requests_merged'] as dynamic, - pullRequestContributors: json['pull_request_contributors'] as dynamic, + forks: json['forks'], + stars: json['stars'], + subscribers: json['subscribers'], + totalIssues: json['total_issues'], + closedIssues: json['closed_issues'], + pullRequestsMerged: json['pull_requests_merged'], + pullRequestContributors: json['pull_request_contributors'], - commitCount4Weeks: json['commit_count_4_weeks'] as dynamic, + commitCount4Weeks: json['commit_count_4_weeks'],packages/komodo_cex_market_data/lib/src/models/quote_currency.dart (2)
377-392: New fiat currencies added — small naming nitDisplay names LGTM. Optional: “Icelandic Króna” (accent) is the formal spelling; current form is acceptable if you’re avoiding diacritics.
896-911: CoinPaprika ID mapping may need fiat/stablecoin normalizationYou currently lowercase all symbols (e.g., USDT → "usdt"). Some CoinPaprika endpoints expect quotes like "usd", "eur", "btc". To avoid 4xx, consider mapping stablecoins to their underlying fiat for quotes, or add a provider-level normalization. Example:
String get coinPaprikaId { - return map( - fiat: (fiat) => fiat.symbol.toLowerCase(), - stablecoin: (stable) => stable.symbol.toLowerCase(), - crypto: (crypto) => crypto.symbol.toLowerCase(), - commodity: (commodity) => commodity.symbol.toLowerCase(), - ); + return when( + fiat: (symbol, _) => symbol.toLowerCase(), + stablecoin: (symbol, _, underlying) => underlying.symbol.toLowerCase(), + crypto: (symbol, _) => symbol.toLowerCase(), + commodity: (symbol, _) => symbol.toLowerCase(), + ); }If the provider actually supports USDT/USDC directly as quotes, keep your current version; otherwise normalize as above.
packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/total_volume.dart (2)
69-132: Parsing looks fine; consider normalizingvndcast and confirmtRYmapping.
- Minor consistency nit: all other numeric fields use
?.toDouble();vnddoes not. If not intentional, normalize it.- Verify that using JSON key
'try'to populate fieldtRYis intentional across the codebase.Suggested normalization within this hunk:
- vnd: json['vnd'] as num?, + vnd: (json['vnd'] as num?)?.toDouble(),
196-259: toJson map alignment LGTM; add a short note for the reserved keyword workaround.The
'try': tRYmapping is correct. Add a brief doc comment near thetRYfield to avoid future confusion.Outside this hunk (field declaration area):
/// CoinGecko JSON key 'try' (Turkish Lira). Named tRY to avoid reserved word. final num? tRY;packages/komodo_cex_market_data/lib/src/komodo/prices/komodo_price_repository.dart (1)
98-108: Coin list cache mapping LGTM; dedupe fiat set definition.Implementation is correct. Optional: extract the supported fiat set to a single constant to avoid duplication with
_cachedFiatCurrencies.Example (outside this hunk):
static const Set<String> _supportedFiats = {'USD', 'USDT'};Then use
_supportedFiatsfor bothcurrencies:here and when assigning_cachedFiatCurrencies.packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/image.dart (1)
13-15: copyWith one-liner LGTM; consider naming to avoid FlutterImagecollisions.Optional: if this model is imported alongside Flutter’s
Image, consider renaming (e.g.,CgImage) or using import prefixes to prevent ambiguity.packages/komodo_cex_market_data/lib/src/coingecko/models/coin_historical_data/market_cap.dart (2)
69-132: Parsing consistent overall; consider uniform numeric casting.
btc,idr,mmk, andvndare not cast to double while most others are. If a uniform numeric type is desired, consider normalizing.Proposed within this hunk:
- btc: json['btc'] as num?, + btc: (json['btc'] as num?)?.toDouble(), - idr: json['idr'] as num?, + idr: (json['idr'] as num?)?.toDouble(), - mmk: json['mmk'] as num?, + mmk: (json['mmk'] as num?)?.toDouble(), - vnd: json['vnd'] as num?, + vnd: (json['vnd'] as num?)?.toDouble(),If the current mix is intentional (e.g., to preserve integer semantics for certain currencies), keep as-is.
196-259: toJson map LGTM; reserved keyword note mirrors TotalVolume.Mapping
'try': tRYis correct. Consider adding the same short doc comment ontRYas suggested in TotalVolume.packages/komodo_cex_market_data/test/integration_test.dart (1)
52-55: Constructor update matches new positional repositories parameter.Good adaptation to SparklineRepository([...], selectionStrategy: …).
To harden the dedup/fallback tests, consider asserting the negative path too:
// In 'request deduplication prevents concurrent calls' verifyNever(() => fallbackRepo.getCoinOhlc(any(), any(), any(), startAt: any(named: 'startAt'), endAt: any(named: 'endAt'))); // In 'cache works with request deduplication' verifyNever(() => fallbackRepo.getCoinOhlc(any(), any(), any(), startAt: any(named: 'startAt'), endAt: any(named: 'endAt')));packages/komodo_cex_market_data/test/repository_priority_manager_test.dart (4)
183-184: Updated CoinGecko default priority assertion is fine—add coverage for CoinPaprika.Since CoinPaprika is introduced, include a smoke assertion that defaultPriorities contains an entry for CoinPaprikaRepository to prevent regressions.
expect( RepositoryPriorityManager.defaultPriorities.containsKey(CoinPaprikaRepository), isTrue, );
200-203: Sparkline priority assertion updated—mirror for CoinPaprika.Add a similar presence check for sparklinePriorities.
expect( RepositoryPriorityManager.sparklinePriorities.containsKey(CoinPaprikaRepository), isTrue, );
347-349: Priority constants test adjusted—consider guarding against map drift.Instead of only numeric checks, also assert the map size or keys set to catch accidental removals when adding providers.
expect( RepositoryPriorityManager.defaultPriorities.keys.toSet(), containsAll({KomodoPriceRepository, BinanceRepository, CoinGeckoRepository, CoinPaprikaRepository}), );
358-360: Sparkline constants adjusted—add presence check for new provider.Same idea for sparkline priorities.
expect( RepositoryPriorityManager.sparklinePriorities.keys.toSet(), containsAll({BinanceRepository, CoinGeckoRepository, CoinPaprikaRepository}), );packages/komodo_cex_market_data/lib/src/models/coin_ohlc.g.dart (1)
85-96: Annotateopen,high,low, andclosewithDecimalConverter
The generated_$CoinPaprikaOhlcToJsonemits rawDecimalinstances for these fields, which aren’t JSON‐safe and will causejsonEncodeto fail. Add the same@JsonKey(toJson: DecimalConverter().toJson, fromJson: DecimalConverter().fromJson)annotation on each of those fields inpackages/komodo_cex_market_data/lib/src/models/coin_ohlc.dartto ensure they serialize to JSON primitives.packages/komodo_cex_market_data/lib/src/id_resolution_strategy.dart (1)
142-155: Trim IDs to avoid whitespace-only false positivesGuard against values like " btc-bitcoin " by trimming before checks.
- List<String> getIdPriority(AssetId assetId) { - final coinPaprikaId = assetId.symbol.coinPaprikaId; + List<String> getIdPriority(AssetId assetId) { + final coinPaprikaId = assetId.symbol.coinPaprikaId?.trim(); @@ - return [ - coinPaprikaId, - ].where((id) => id != null && id.isNotEmpty).cast<String>().toList(); + return [ + coinPaprikaId, + ].where((id) => id != null && id.isNotEmpty).cast<String>().toList(); }packages/komodo_cex_market_data/lib/src/repository_priority_manager.dart (2)
79-85: Non-stable sort: optional tie-breakerIf equal priorities ever occur, List.sort is not stable. Add a secondary comparator (e.g., type name) for determinism.
- ..sort( - (a, b) => getPriorityWithCustomMap( - a, - customPriorities, - ).compareTo(getPriorityWithCustomMap(b, customPriorities)), - ); + ..sort((a, b) { + final pa = getPriorityWithCustomMap(a, customPriorities); + final pb = getPriorityWithCustomMap(b, customPriorities); + if (pa != pb) return pa.compareTo(pb); + return a.runtimeType.toString().compareTo(b.runtimeType.toString()); + });
96-99: Same tie-breaker advice for sparkline sortMirror the deterministic fallback here as well.
- ..sort( - (a, b) => getSparklinePriority(a).compareTo(getSparklinePriority(b)), - ); + ..sort((a, b) { + final pa = getSparklinePriority(a); + final pb = getSparklinePriority(b); + if (pa != pb) return pa.compareTo(pb); + return a.runtimeType.toString().compareTo(b.runtimeType.toString()); + });packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_market.g.dart (1)
31-47: Ensure nested objects serialize to primitives (explicitToJson).ToJson returns CoinPaprikaQuote/Decimal instances directly; jsonEncode will call toJson, but returning a fully primitive Map is safer for intermediate usage (e.g., Hive, logs).
Enable explicitToJson on the parent model and rely on the Decimal converter above:
// coinpaprika_market.dart @JsonSerializable(explicitToJson: true) class CoinPaprikaMarket { /* ... */ }Also applies to: 55-59
packages/komodo_cex_market_data/lib/komodo_cex_market_data.dart (2)
1-31: Doc nit: clarify where retry/backoff lives to avoid confusion.Per prior architecture, retry/backoff/fallback orchestration is handled by the manager layer (RepositoryFallbackMixin/CexMarketDataManager), not individual repositories. Consider tightening language to reflect that.
Suggested tweak: “Robust error handling and retry mechanisms via the manager layer (RepositoryFallbackMixin) with repository-level fallbacks.”
33-35: Publicly exporting _internal_exports may widen API surface.Confirm intent to expose private indices publicly; this can impact semver and consumer-facing API size.
Option: create lib/internal.dart for monorepo-internal consumers and keep komodo_cex_market_data.dart limited:
- export 'src/_internal_exports.dart'; + // For internal consumers, import: package:komodo_cex_market_data/internal.dartAdditional file (optional):
// lib/internal.dart library komodo_cex_market_data_internal; export 'src/_internal_exports.dart';packages/komodo_cex_market_data/lib/src/_internal_exports.dart (1)
3-12: Avoid leaking internals: don’t re-export src/_internal_exports.dart from the public APIIf komodo_cex_market_data.dart exports this file, all “src/” internals become public and break encapsulation/semver. Keep this barrel strictly internal and import it only within the package.
packages/komodo_cex_market_data/test/coingecko/models/coingecko_api_plan_test.dart (2)
354-356: Make fixed “oldDate” UTC for consistencyPrevents environment-dependent failures.
-final oldDate = DateTime(2010, 1, 1); +final oldDate = DateTime.utc(2010, 1, 1);
59-80: Stabilize now()-based testsConsider injecting a Clock (package:clock) or passing a fixed “now” into the cutoff helpers to eliminate date-drift flakiness.
Also applies to: 82-96
packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_ticker.g.dart (1)
33-46: Nested Map values aren’t explicitly serializedTo avoid relying on jsonEncode’s toEncodable, enable explicitToJson or map values to toJson in the generator.
// In coinpaprika_ticker.dart annotation: @JsonSerializable(fieldRename: FieldRename.snake, explicitToJson: true)packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_ticker_quote.dart (1)
11-62: Harden number parsing to accept int/double/stringAPIs sometimes return numbers as strings. Add a tolerant converter for price/percent fields.
double _asDouble(Object? v) { if (v == null) return 0.0; if (v is num) return v.toDouble(); if (v is String) return double.tryParse(v) ?? 0.0; return 0.0; } // Example: @JsonKey(fromJson: _asDouble) required double price, // ...apply to other double fields as neededpackages/komodo_cex_market_data/lib/src/repository_selection_strategy.dart (1)
35-37: Update docs to reflect no-op cacheThe comment still mentions cache initialization; it’s now delegated to repo.supports().
- /// Ensures the cache is initialized for the given repositories + /// No-op: capability is checked per repository via supports()Also applies to: 52-55
packages/komodo_cex_market_data/lib/src/_core_index.dart (1)
3-13: Confirm intent: “Internal/private” index is publicly exportedHeader says internal/private, but this index is reportedly exported from the public library. That exposes all re-exports (repositories, strategies, mixins) to package consumers and becomes part of your semver surface.
If this is unintentional, drop the public export of
src/_core_index.dartfromkomodo_cex_market_data.dartor create a curated public index that re-exports only the intended symbols. I can propose a minimal public facade if you confirm the desired API surface.packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_coin.dart (1)
6-37: Model looks solid; consider stronger typing for “type”The Freezed + JSON wiring is correct and the snake_case config matches CoinPaprika’s payloads. One improvement: replace
String typewith a sealed enum (e.g.,enum CoinPaprikaAssetType { coin, token }) plus@JsonEnumfor compile-time safety.I can draft the enum and migration if you want it in this PR.
packages/komodo_defi_sdk/example/lib/main.dart (4)
50-59: Manage SparklineRepository lifecycle (dispose/close) and avoid value provider if possibleYou initialize
SparklineRepositoryeagerly and inject it via.value, which means its lifecycle isn’t owned by a widget. If the repo exposesdispose()/close(), consider letting a provider own and dispose it, or store it in aStateand calldispose()there.Example with provider-managed lifecycle (adjust if
RepositoryProvidersupportsdisposein your version; otherwise useProviderfromproviderpackage or a Stateful wrapper):- final sparklineRepository = SparklineRepository.defaultInstance(); - unawaited( - sparklineRepository.init().catchError((Object? error, StackTrace? stackTrace) { - dragon.log('Error during sparklineRepository initialization: $error'); - debugPrintStack(stackTrace: stackTrace); - }), - ); + // Defer creation to the provider to own its lifecycle. + // If your RepositoryProvider version supports `dispose`, wire it here. + // Otherwise, wrap this in a small Stateful widget that calls repo.dispose() in `dispose()`. ... - MultiRepositoryProvider( - providers: [ - RepositoryProvider<KomodoDefiSdk>.value(value: defaultSdk), - RepositoryProvider<SparklineRepository>.value( - value: sparklineRepository, - ), - ], + MultiRepositoryProvider( + providers: [ + RepositoryProvider<KomodoDefiSdk>.value(value: defaultSdk), + RepositoryProvider<SparklineRepository>( + create: (_) => SparklineRepository.defaultInstance()..init(), + // dispose: (_, repo) => repo.dispose(), // if available + ), + ],If lifecycle control isn’t needed in the example app, feel free to keep as-is.
Also applies to: 67-72
130-137: Initialize instances in parallel to reduce startup timeThese awaits run sequentially. Using
Future.waitcan noticeably reduce time if there are multiple instances.- // Initialize state for each instance - for (final instance in manager.instances.values) { - await _initializeInstance(instance); - } + // Initialize all instances concurrently + await Future.wait( + manager.instances.values.map(_initializeInstance), + );
180-188: Guard setState after async to avoid calling it when unmounted
_fetchKnownUsersawaits before callingsetState. If the widget is disposed meanwhile, this can throw.- state.knownUsers = users; - setState(() {}); + state.knownUsers = users; + if (!mounted) return; + setState(() {});
197-203: Minor search UX enhancementTrim the query to avoid accidental leading/trailing spaces affecting matches.
- final query = _searchController.text.toLowerCase(); + final query = _searchController.text.trim().toLowerCase();packages/komodo_cex_market_data/test/coinpaprika/coinpaprika_cex_provider_test.dart (4)
60-67: endDate parameter is ignored — clarify/test explicitly.fetchHistoricalOhlc accepts endDate but never uses it in the query params. If CoinPaprika rejects end, either remove the parameter from the public API or assert in tests that passing it has no effect (and document why).
233-239: Avoid brittle exact-length assertion for supportedQuoteCurrencies.Hard-coding 42 will break on legitimate additions/removals. Prefer verifying required members and immutability.
Apply:
- expect(supportedCurrencies.length, equals(42)); + expect( + supportedCurrencies, + containsAll(TestConstants.defaultSupportedCurrencies), + );
360-383: Overly strict URL string equality.Asserting the full absolute URL makes tests fragile to benign changes (baseUrl, version). Prefer verifying host, path, and normalized query.
You already have VerificationHelpers that validate components; use those here.
575-604: “400 regression” test doesn’t exercise a failure path.This uses a mocked 200 response, so it doesn’t validate avoidance of 400s. Rename to reflect it’s validating URL composition, or add an integration test/fixture asserting the absence of quote/limit/end is enforced.
packages/komodo_cex_market_data/test/common/api_error_parser_test.dart (2)
291-302: Gitleaks false-positive triggers — sanitize placeholder secrets.Static analysis flagged generic API keys in test JSON. Replace with clearly fake, non-matching tokens to avoid secret scanners while preserving test intent.
Apply:
- "api_key": "sk-1234567890abcdef", - "token": "bearer_token_xyz", + "api_key": "sk_test_1234567890abcdef", + "token": "bearer_token_redacted", @@ - "authorization": "Bearer secret_key", - "x-api-key": "private_key_123" + "authorization": "Bearer <redacted>", + "x-api-key": "<redacted>"
212-275: Keep createSafeErrorMessage mapping in sync with parser.The expectations for phrases like “Resource not found” and “Payment/upgrade required” implicitly define UX copy. If ApiErrorParser copy changes, these tests will fail. Consider centralizing the phrase map in the parser and referencing it here to reduce duplication.
packages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_cex_provider.dart (4)
465-471: Avoid mutating caller-provided coinGeckoIds.Removing empties in-place can surprise callers. Copy-filter instead.
Apply:
- coinGeckoIds.removeWhere((String id) => id.isEmpty); + final filteredIds = + coinGeckoIds.where((id) => id.isNotEmpty).toSet().toList(); @@ - final tickersUrl = Uri.https(baseUrl, '$apiVersion/simple/price', { - 'ids': coinGeckoIds.join(','), + final tickersUrl = Uri.https(baseUrl, '$apiVersion/simple/price', { + 'ids': filteredIds.join(','),
557-566: Unnecessary id query parameter.The id query param duplicates the path segment and isn’t used by CoinGecko. Drop it to avoid confusion.
Apply:
- final queryParameters = <String, String>{ - 'id': id, + final queryParameters = <String, String>{
98-110: Error handling consistency: surface structured errors or include context in Exception.You parse ApiError but throw Exception(apiError.message), discarding errorType/retryAfter. Either throw ApiError or include structured details in the message for callers to react to rate limits and retries.
Also applies to: 126-138, 195-207, 296-309, 419-432, 574-587
150-153: Minor doc nit.“Comma-sepa” is truncated.
Apply:
- /// [priceChangePercentage] Comma-sepa + /// [priceChangePercentage] Comma-separated list of percentage windows (e.g., "1h,24h,7d").packages/komodo_cex_market_data/lib/src/models/coin_ohlc.dart (1)
17-22: Guard against CoinPaprika arrays in CoinOhlc.fromJson for clearer errorThis factory is array-only; if
source == OhlcSource.coinpaprikais passed, the cast toList<dynamic>will throw a less-informative error. Add an early, explicit guard.factory CoinOhlc.fromJson(List<dynamic> json, {OhlcSource? source}) { - return CoinOhlc( + if (source == OhlcSource.coinpaprika) { + throw ArgumentError( + 'CoinPaprika OHLC must be parsed from JSON objects, not arrays.', + ); + } + return CoinOhlc( ohlc: json .map( (dynamic kline) => Ohlc.fromKlineArray(kline as List<dynamic>, source: source), ) .toList(), ); }packages/komodo_cex_market_data/lib/src/sparkline_repository.dart (2)
15-21: Constructor docs are stale and allow empty repo lists
- The doc says “defaults to Binance and CoinGecko” but the constructor now requires repositories; defaults are provided by
defaultInstance().- Also add an
assertto prevent empty repository lists at runtime.- /// If repositories are not provided, defaults to Binance and CoinGecko. - SparklineRepository( - this._repositories, { - RepositorySelectionStrategy? selectionStrategy, - }) : _selectionStrategy = - selectionStrategy ?? DefaultRepositorySelectionStrategy(); + /// Use [SparklineRepository.defaultInstance] for the default trio. + SparklineRepository( + this._repositories, { + RepositorySelectionStrategy? selectionStrategy, + }) : _selectionStrategy = + selectionStrategy ?? DefaultRepositorySelectionStrategy() { + assert(_repositories.isNotEmpty, 'SparklineRepository requires at least one repository'); + }
22-31: Default composition updated: consider const constructors where availableThe default repo order and inclusion of CoinPaprika look good. If
CoinPaprikaProvider/CoinGeckoCexProviderhave const constructors, mark them const to avoid unnecessary allocations.factory SparklineRepository.defaultInstance() { return SparklineRepository([ BinanceRepository(binanceProvider: const BinanceProvider()), - CoinPaprikaRepository(coinPaprikaProvider: CoinPaprikaProvider()), - CoinGeckoRepository(coinGeckoProvider: CoinGeckoCexProvider()), + CoinPaprikaRepository(coinPaprikaProvider: const CoinPaprikaProvider()), + CoinGeckoRepository(coinGeckoProvider: const CoinGeckoCexProvider()), ], selectionStrategy: DefaultRepositorySelectionStrategy()); }packages/komodo_cex_market_data/lib/src/bootstrap/market_data_bootstrap.dart (1)
24-41: Double-check default opt-in for CoinPaprika.Enabling CoinPaprika by default (enableCoinPaprika = true) changes behavior for existing integrators and may introduce unexpected outbound traffic or API key requirements. If the intent is “fallback only when explicitly enabled,” consider defaulting to false or making this opt-in via release notes and a migration entry. Also confirm repositoryPriority defaults are documented externally.
packages/komodo_cex_market_data/test/coinpaprika_provider_validation_test.dart (7)
162-197: Strengthen verification across multiple calls.Verify call count and ensure all captured headers are checked deterministically.
Apply this diff:
- final capturedHeaders = verify( - () => mockHttpClient.get(any(), headers: captureAny(named: 'headers')), - ).captured; + final verification = verify( + () => mockHttpClient.get(any(), headers: captureAny(named: 'headers')), + ); + verification.called(3); + final capturedHeaders = verification.captured;
199-230: Mirror the same multi-call verification in OHLC test.Add .called(1) (or expected count) to assert the request actually happened and avoid false positives.
Apply this diff:
- final capturedHeaders = - verify( + final verification = verify( () => mockHttpClient.get( any(), headers: captureAny(named: 'headers'), ), - ).captured.single - as Map<String, String>; + ); + verification.called(1); + final capturedHeaders = verification.captured.single as Map<String, String>;
241-273: Time-based tests: reduce flakiness.Using DateTime.now() without UTC can be flaky across DST/timezone. Consider normalizing to UTC.
Apply this diff:
- final now = DateTime.now(); + final now = DateTime.now().toUtc();Repeat similarly in this group.
309-341: Same UTC normalization suggestion for starter plan tests.Apply this diff:
- final now = DateTime.now(); + final now = DateTime.now().toUtc();
353-364: Same UTC normalization for ultimate plan tests.- final now = DateTime.now(); + final now = DateTime.now().toUtc();
366-381: Duplicate negative-interval test.Two tests assert the same “1h unsupported for free plan.” Keep one; convert the other to a different interval (e.g., '2h') or remove.
634-646: Expectation breadth is good; consider explicit size check.You already assert > 10; optionally assert a minimum set to guard regressions (e.g., usd, eur, btc, eth).
packages/komodo_cex_market_data/lib/src/coingecko/data/coingecko_repository.dart (3)
64-74: Eager per-coin currency set may be heavy.copyWith(currencies: supportedCurrencies.toSet()) for every coin can be memory-heavy for large lists. Consider storing fiat support once (e.g., on the repository) and resolving at use-time, or keep currencies empty and rely on supports().
176-194: Error messages: add context.Include tradingSymbol and mappedFiatId in exceptions to simplify debugging mismatches.
Apply this diff:
- throw Exception( - 'Market data or current price not found in historical data response', - ); + throw Exception( + 'Market data or current price not found in historical data response ' + '(symbol=$mappedFiatId).', + ); ... - throw Exception( - 'Price data for $mappedFiatId not found in historical data response', - ); + throw Exception( + 'Price data for $mappedFiatId not found in historical data response.', + );
312-329: Batch size heuristic is fine; document/demo-plan assumption.Returning min(daysSinceCutoff, 365) is reasonable. Add a short comment that CoinGecko/day endpoint handles up to 365 per call in practice, hence the cap.
packages/komodo_cex_market_data/test/coinpaprika/coinpaprika_repository_test.dart (5)
175-189: Rename test to match behavior (it doesn’t throw).Name says “throws ArgumentError” but the expectation is empty result and no throw.
- test( - 'throws ArgumentError for requests exceeding 24h without start/end dates', + test( + 'does not throw and returns empty OHLC when defaults are used (<= 24h)',
221-223: Verify batching actually made multiple calls.Passing 0 undermines the intent. Require at least 2 calls (200 days).
- VerificationHelpers.verifyMultipleProviderCalls(mockProvider, 0); + VerificationHelpers.verifyMultipleProviderCalls(mockProvider, 2);If the helper supports named args, prefer a minimum:
minCalls: 2.
144-148: Stabilize “now” and use UTC to avoid flakiness.Capture
final now = DateTime.now().toUtc();once per test and derive ranges from it.Example:
- final now = DateTime.now(); + final now = DateTime.now().toUtc();Also applies to: 200-205, 235-241, 281-285, 628-631, 858-861, 938-941
454-466: Be explicit about the stablecoin in the price request.The test title asserts USDT mapping; pass it explicitly to avoid relying on defaults.
- final result = await repository.getCoinFiatPrice(TestData.bitcoinAsset); + final result = await repository.getCoinFiatPrice( + TestData.bitcoinAsset, + fiatCurrency: Stablecoin.usdt, + );
717-731: Reduce timing fragility in UTC check.Comparing to “now” within 1 minute can flake under CI load. Consider widening to 2–3 minutes or injecting a clock.
packages/komodo_cex_market_data/lib/src/common/api_error_parser.dart (3)
186-196: Narrow CoinGecko 400 detection to avoid false positives.Matching on “days”/“365” is too broad. Prefer JSON field parsing (e.g., error/message codes) or a regex specific to their plan-limit message.
I can adjust parsing to inspect
{ "error": "...", "code": "plan_limit_exceeded" }and fall back to generic bad_request if unrecognized.Also applies to: 205-224
288-329: Include retry-after hint in safe message when available.Surface a non-sensitive backoff hint to aid ops.
- static String createSafeErrorMessage({ + static String createSafeErrorMessage({ required String operation, required String service, required int statusCode, String? coinId, + int? retryAfterSeconds, }) { @@ - return buffer.toString(); + if (statusCode == 429 && retryAfterSeconds != null) { + buffer.write(' (retry after ~${retryAfterSeconds}s)'); + } + return buffer.toString(); }Call sites can pass
apiError.retryAfter.
94-183: Consider accepting headers to parse Retry-After properly.Retry-After is typically a response header, not body. Optionally extend parsers to accept headers.
packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_api_plan.dart (4)
8-11: Style: preferclassoverabstract classwith Freezed.Aligns with current Freezed recommendations and generated code expectations.
-abstract class CoinPaprikaApiPlan with _$CoinPaprikaApiPlan { +class CoinPaprikaApiPlan with _$CoinPaprikaApiPlan {
18-22: Deduplicate interval lists to avoid drift.Define shared constants for daily/hourly/5m sets and reuse across variants.
Example:
static const _daily = ['24h','1d','7d','14d','30d','90d','365d']; static const _hourly = ['1h','2h','3h','6h','12h']; static const _fiveMin = ['5m','10m','15m','30m','45m'];Then compose with
[...]where needed.Also applies to: 31-53, 62-84, 93-115, 123-145, 153-175
200-203: Type-safety for intervals.If GraphInterval exists, consider exposing
Set<GraphInterval>and map to provider strings at the edge to prevent typos.
181-238: Expose plan-driven batching hints.Tests rely on 90-day daily batching. Add an explicit property (e.g.,
Duration get maxDailyBatch) to centralize this instead of scattering constants.packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_market.dart (3)
47-55: Consider nullability for API-optional fields.CoinPaprika may omit or null some fields in edge cases (e.g., adjusted_volume_24h_share, market_url, category, fee_type, last_updated). Parsing will fail if any is absent/null.
Would you confirm with a couple of real payloads from /coins/{id}/markets? If any of these are nullable, we should switch to nullable types and add defaults in fromJson.
81-83: Guard DateTime parse.lastUpdatedDateTime will throw on malformed/empty timestamps. Provide a safe nullable getter and keep the strict one.
Add alongside the current getter:
DateTime? get tryLastUpdatedDateTime { try { return DateTime.parse(lastUpdated); } catch (_) { return null; } }
85-87: Tiny allocation nit: avoid repeated uppercasing on hot paths.If getQuoteFor is used in tight loops, consider normalizing keys to uppercase once at deserialization to allow quotes[currencyKey] lookups directly.
packages/komodo_cex_market_data/lib/src/coinpaprika/data/coinpaprika_cex_provider.dart (5)
171-175: Respect endDate when provided.Including end (when supplied) limits payload size and aligns with caller expectations. Keep your interval normalization; just pass end when present.
final queryParams = <String, String>{ 'start': _formatDateForApi(startDate), 'interval': apiInterval, }; + if (endDate != null) { + queryParams['end'] = _formatDateForApi(endDate); + }
34-41: Docs vs default mismatch ("24h" vs "1d").The comment says default "24h" but the parameter default is "1d".
- /// [interval]: Data interval (default: "24h") + /// [interval]: Data interval (default: "1d")
151-158: Unused quote parameter in historical fetch.Method signature takes quote but it's ignored (by design per your notes). Either document that quote is ignored for historical or consider dropping it to avoid confusion.
Do you want me to update the interface/docs to make this explicit?
459-486: Handle 429 rate limits explicitly.Return a clearer error for rate limits and suggest retry based on Retry-After.
- // Check if this is a CoinPaprika API limitation error + // Rate limit + if (response.statusCode == 429) { + final retryAfter = response.headers['retry-after']; + _logger.warning( + 'CoinPaprika rate limited (429) for $coinId. ' + 'Retry-After: ${retryAfter ?? 'unspecified'}', + ); + throw Exception( + 'CoinPaprika rate limit reached. ' + '${retryAfter != null ? 'Retry after $retryAfter seconds.' : 'Please retry later.'}', + ); + } + + // Check if this is a CoinPaprika API limitation error if (response.statusCode == 400 && response.body.contains('is not allowed in this plan')) {
13-23: Config declared but not used.CoinPaprikaConfig.baseUrl/maxRetries aren’t used. Either wire them in, or remove to avoid drift.
If you plan to add retries, I can wire a simple jitter backoff using maxRetries.
packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_market.freezed.dart (3)
31-32: Prefer DateTime for lastUpdated instead of StringCoinPaprika returns ISO-8601 timestamps; using DateTime improves type-safety across the codebase and aligns with other models (e.g., ticker). Update the source model and re-generate.
- String get lastUpdated; + DateTime get lastUpdated; … - @override final String lastUpdated; + @override final DateTime lastUpdated;Also applies to: 275-277
27-28: Model category/feeType as enumsThese look like closed sets (“Spot”, “Derivative”, “Percentage”, etc.). Strong enums reduce stringly-typed bugs and help exhaustive handling.
Also applies to: 260-262
30-30: Confirm nullability of adjustedVolume24hShare: The field is defined as a non-nullable double (coinpaprika_market.dart:48; coinpaprika_market.freezed.dart:30, 265). If the API ever omits or returns adjusted_volume24h_share as null, deserialization will throw. Verify whether the API guarantees this value is always present/non-null; if not, change the type todouble?and handle a default or null case.packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_ticker.freezed.dart (1)
233-234: Empty-string defaults for id/name/symbol can mask bad dataConsider requiring these or validating upstream, rather than defaulting to ''.
packages/komodo_cex_market_data/lib/src/coingecko/models/coingecko_api_plan.dart (1)
173-213: Optional: avoid non-determinism in tests by injecting clock/now.Multiple getters use
DateTime.now().toUtc(), which can cause flaky tests. Consider injecting a clock or passingnowas an optional parameter for deterministic behaviors.packages/komodo_cex_market_data/test/coinpaprika/fixtures/mock_helpers.dart (2)
23-28: Stabilize fallback values for deterministic tests.Use fixed values (e.g., epoch) instead of
DateTime.now().- registerFallbackValue(DateTime.now()); + registerFallbackValue(DateTime.utc(1970, 1, 1));
299-327: Batching scenario returns identical data each call; confirm expectations.
thenAnswer((_) async => batchData)returns the same combined set per batch call. If the repository aggregates per-batch, you may double-count. Verify test assertions match intended behavior.packages/komodo_cex_market_data/lib/src/coinpaprika/data/coinpaprika_repository.dart (1)
330-348: Minor: when deriving a 1-hour OHLC window, include UTC explicitly.Safer to compose UTC instants to avoid TZ drift when
priceDateis local.- final endDate = priceDate.add(const Duration(hours: 1)); + final endDate = priceDate.toUtc().add(const Duration(hours: 1));packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_api_plan.freezed.dart (2)
57-59: Misleading inline comment (“5 years”) on base mixin fields.The comment suggests a fixed “5 years” limit while variants differ (e.g., free: 365d; starter: 1825d; others: null = unlimited). Move accurate docs to the source union per-variant and drop this comment to avoid confusion in generated code.
370-378: Stray “// 5 years” comments near unrelated fields.These markers appear next to _availableIntervals, not the duration. Clean in the source so the generated file doesn’t carry misplaced comments.
packages/komodo_cex_market_data/test/repository_selection_strategy_test.dart (3)
191-216: Test name doesn’t match behavior (selection is by supports, not “priority”).The strategy picks the first supporting repo; no explicit priority is set. Rename for clarity.
- test('selects repository based on priority', () async { + test('selects the first repository that supports the request', () async {
274-302: Nice invariants for quote-currency mapping; add one negative case for robustness.Consider asserting that Stablecoin.usdt.coinPaprikaId != 'usd' to guard against regressions that collapse mappings.
+ expect( + Stablecoin.usdt.coinPaprikaId == 'usd', + isFalse, + reason: 'USDT must not collapse to USD in CoinPaprika mapping', + );
303-346: Selection test can assert determinism when both repos support.Add an assertion that the chosen repo matches the strategy’s iteration order to document expected tie-breaking.
- expect(selectedRepo, isNotNull); + expect(selectedRepo, isNotNull); + // By default strategy should pick the first supporting repo in the list. + expect(selectedRepo, equals(geckoLikeRepo));packages/komodo_cex_market_data/lib/src/coinpaprika/models/coinpaprika_ticker_quote.freezed.dart (1)
18-35: Optional: tighten docs for percentage fields.Consider clarifying expected units (percentage points vs fraction) in the source model to avoid ambiguity across providers.
packages/komodo_cex_market_data/test/coinpaprika/fixtures/test_fixtures.dart (3)
144-146: Inconsistent number types in markets response JSON.
priceandvolume_24hare encoded as strings here, while other fixtures use numbers. Align types with the parser’s expectations to avoid brittle tests.- 'price': TestConstants.bitcoinPrice.toString(), - 'volume_24h': TestConstants.highVolume.toString(), + 'price': TestConstants.bitcoinPrice, + 'volume_24h': TestConstants.highVolume,
232-235: Potential SDK-compat risk withDuration * int.
base.add(step * index)relies onDurationmultiplication operator. Verify your minimum Dart SDK supports it; otherwise, compute via microseconds.- final timeOpen = base.add(step * index); + final timeOpen = base.add(Duration(microseconds: step.inMicroseconds * index));
8-11: Prefer public exports oversrc/imports (tests).To reduce coupling to internals, import
CoinPaprikaMarketvia the package’s public API if it’s exported.-import 'package:komodo_cex_market_data/src/_core_index.dart' - show CoinPaprikaMarket; +import 'package:komodo_cex_market_data/komodo_cex_market_data.dart' + show CoinPaprikaMarket;packages/komodo_cex_market_data/test/coinpaprika/fixtures/verification_helpers.dart (2)
4-4: Remove unused import.
httpisn’t used in this file.-import 'package:http/http.dart' as http;
434-441: Unused parameter inverifyQuoteCurrencyMapping.
inputQuotesis unused; either remove it or assert mapping from input→expected.- static void verifyQuoteCurrencyMapping( - MockCoinPaprikaProvider mockProvider, - List<QuoteCurrency> inputQuotes, - List<QuoteCurrency> expectedQuotes, - ) { + static void verifyQuoteCurrencyMapping( + MockCoinPaprikaProvider mockProvider, + List<QuoteCurrency> expectedQuotes, + ) {packages/komodo_cex_market_data/test/coinpaprika/fixtures/test_constants.dart (2)
77-78: Name/value mismatch for “single digits” date.Either rename or use an actual non-padded date.
- static const String dateFormatWithSingleDigits = '2024-03-05'; + static const String dateFormatWithSingleDigits = '2024-3-5';
135-145: Duplicate currency entry inextendedSupportedCurrencies.
FiatCurrency.uahappears twice. Remove the duplicate to avoid misleading counts.FiatCurrency.myr, FiatCurrency.uah, FiatCurrency.lkr, FiatCurrency.mmk, FiatCurrency.idr, FiatCurrency.vnd, FiatCurrency.bdt, - FiatCurrency.uah,
The changes below are unpolluted by formatting or refactoring changes:
sparkline_repository.dart: removed global variables and replaced with a default factory constructor.repository_selection_strategy.dart: replace the duplicatedsupportslogic with a simple call to the repository.supportsfunction.coingecko_cex_provider.dart: add API pricing model and attempt to parse api errors for clearer upstream error messages (no custom exceptions at this stage)coingecko_repository.dart: add api plan model and use it to adjust requests to remain within the current plans limits.coingecko_api_plan.dart: the model representing the currently applicable coingecko api limits per plan/pricing level as a freezed union class along with its generated files.api_error_parser: attempts to parse API error codes and messages into more useful and standardised exception messages.packages/komodo_cex_market_data/lib/src/coinpaprika/: provider, repository, and models for coinpaprika endpoints required for current price, change24h and OHLC endpoints.Formatting:
komodo_cex_market_data/lib/src/coingecko/models/*.dart: primarily formatting changestest/example/lib/main.dartSummary by CodeRabbit
New Features
Improvements
Breaking Changes
Documentation
Chores