From d145f92a0788ae2df3eedef443d962fd9f07133d Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 14 Aug 2026 16:32:06 +0200 Subject: [PATCH] Split note_repository.dart, taking coverage to 100% 429 -> 249 lines: note_query.dart NoteSort, NoteFilter, and the pure predicates behind them (matchesFilter, comparatorFor). All of it takes notes and a filter and returns a decision, which is what made it the natural seam. note_import.dart ImportOutcome and the in-memory LogPersistence used by openInMemory. note_repository.dart re-exports both, so no call site changed. Also drops ImportOutcome.total, which nothing called. It was uncovered before this split too -- the larger file's percentage hid it. With it gone the suite reports 100% line coverage (2005 lines), up from the 99.9% baseline measured at the start of this refactor. lib/ and bin/ are now fully under the cap. 313 tests pass. Co-Authored-By: Claude Opus 4.8 --- lib/data/note_import.dart | 38 +++++++ lib/data/note_query.dart | 168 +++++++++++++++++++++++++++++ lib/data/note_repository.dart | 197 ++-------------------------------- pubspec.yaml | 2 +- 4 files changed, 215 insertions(+), 190 deletions(-) create mode 100644 lib/data/note_import.dart create mode 100644 lib/data/note_query.dart diff --git a/lib/data/note_import.dart b/lib/data/note_import.dart new file mode 100644 index 0000000..1305005 --- /dev/null +++ b/lib/data/note_import.dart @@ -0,0 +1,38 @@ +/// Two small companions of [NoteRepository]: the result of an import, and +/// the in-memory store its test constructor uses. +/// +/// Split out of `note_repository.dart` for file size; that file re-exports +/// this one. +library; + +import 'package:crdt_sync/crdt_sync.dart'; + +/// Summary of an [NoteRepository.importNotes] run, for user feedback. +class ImportOutcome { + /// Creates an [ImportOutcome] from its per-category counts. + const ImportOutcome({ + required this.added, + required this.updated, + required this.skipped, + }); + + /// Notes that did not exist locally and were created. + final int added; + + /// Existing notes overwritten because the import was newer. + final int updated; + + /// Notes skipped because the local copy was the same age or newer. + final int skipped; +} + +/// In-memory [LogPersistence] for [NoteRepository.openInMemory] (tests). +class MemoryPersistence implements LogPersistence { + String? _text; + + @override + Future read() async => _text; + + @override + Future write(String text) async => _text = text; +} diff --git a/lib/data/note_query.dart b/lib/data/note_query.dart new file mode 100644 index 0000000..2608b5d --- /dev/null +++ b/lib/data/note_query.dart @@ -0,0 +1,168 @@ +/// How notes are selected and ordered: the sort modes, the filter, and the +/// pure predicates behind them. +/// +/// Split out of `note_repository.dart` for file size; that file re-exports +/// this one, so callers import it as before. Everything here is pure -- it +/// takes notes and a filter and returns a decision -- which is what made it +/// the natural seam. +library; + +import 'package:todo/data/note.dart'; + +/// How the history list should be ordered. +enum NoteSort { + /// Newest created first. + createdDesc, + + /// Most recently modified first. + modifiedDesc, + + /// A-Z by note text. + alphabetical, + + /// Highest [Priority] first. + priorityDesc, +} + +/// An immutable set of constraints for querying notes. +/// +/// All fields combine with logical AND. Empty/null fields impose no +/// constraint. Lives in the data layer so the filtering it drives never leaks +/// into the UI. Construct copies with [copyWith] when toggling one facet. +class NoteFilter { + /// Creates a [NoteFilter] from its constraint fields. + const NoteFilter({ + this.query = '', + this.priorities = const {}, + this.statuses = const {}, + this.createdFrom, + this.createdTo, + this.updatedFrom, + this.updatedTo, + }); + + /// Case-insensitive substring matched against the note body. + final String query; + + /// Notes must have one of these priorities. Empty means "any priority". + final Set priorities; + + /// Notes must have one of these statuses. Empty means "any status". + final Set statuses; + + /// Inclusive lower bound (by calendar day) on the creation date. + final DateTime? createdFrom; + + /// Inclusive upper bound (by calendar day) on the creation date. + final DateTime? createdTo; + + /// Inclusive lower bound (by calendar day) on the last-updated date. + final DateTime? updatedFrom; + + /// Inclusive upper bound (by calendar day) on the last-updated date. + final DateTime? updatedTo; + + /// True when no constraint is active (the unfiltered, full list). + bool get isEmpty => + query.trim().isEmpty && + priorities.isEmpty && + statuses.isEmpty && + createdFrom == null && + createdTo == null && + updatedFrom == null && + updatedTo == null; + + /// Number of distinct active facets, for an "N filters" badge in the UI. + int get activeCount { + var n = 0; + if (query.trim().isNotEmpty) n++; + if (priorities.isNotEmpty) n++; + if (statuses.isNotEmpty) n++; + if (createdFrom != null || createdTo != null) n++; + if (updatedFrom != null || updatedTo != null) n++; + return n; + } + + /// Returns a copy with selected facets replaced. A `null` argument keeps + /// the current value; clearing a date is done via the dedicated clear + /// flags so `null` can mean "unchanged". + NoteFilter copyWith({ + String? query, + Set? priorities, + Set? statuses, + DateTime? createdFrom, + DateTime? createdTo, + DateTime? updatedFrom, + DateTime? updatedTo, + bool clearCreated = false, + bool clearUpdated = false, + }) { + return NoteFilter( + query: query ?? this.query, + priorities: priorities ?? this.priorities, + statuses: statuses ?? this.statuses, + createdFrom: clearCreated ? null : (createdFrom ?? this.createdFrom), + createdTo: clearCreated ? null : (createdTo ?? this.createdTo), + updatedFrom: clearUpdated ? null : (updatedFrom ?? this.updatedFrom), + updatedTo: clearUpdated ? null : (updatedTo ?? this.updatedTo), + ); + } +} + +/// Whether [note] passes every clause of [filter]. +/// +/// Clauses are AND-combined, and an empty clause matches everything -- an +/// empty priority set means "any priority", not "no priorities". +bool matchesFilter(Note note, NoteFilter filter) { + final query = filter.query.trim().toLowerCase(); + if (query.isNotEmpty && !note.text.toLowerCase().contains(query)) { + return false; + } + if (filter.priorities.isNotEmpty && + !filter.priorities.contains(note.priority)) { + return false; + } + if (filter.statuses.isNotEmpty && !filter.statuses.contains(note.status)) { + return false; + } + if (!_withinDays(note.createdAt, filter.createdFrom, filter.createdTo)) { + return false; + } + if (!_withinDays(note.updatedAt, filter.updatedFrom, filter.updatedTo)) { + return false; + } + return true; +} + +/// Inclusive day-granularity range check, matching the old SQL bounds: +/// `from` includes its whole day; `to` includes its whole day. +bool _withinDays(DateTime value, DateTime? from, DateTime? to) { + if (from != null && value.isBefore(_startOfDay(from))) return false; + if (to != null && + !value.isBefore(_startOfDay(to).add(const Duration(days: 1)))) { + return false; + } + return true; +} + +/// The comparator implementing [sort]. +Comparator comparatorFor(NoteSort sort) { + switch (sort) { + case NoteSort.createdDesc: + return (a, b) => b.createdAt.compareTo(a.createdAt); + case NoteSort.modifiedDesc: + return (a, b) => b.updatedAt.compareTo(a.updatedAt); + case NoteSort.alphabetical: + return (a, b) => a.text.toLowerCase().compareTo(b.text.toLowerCase()); + case NoteSort.priorityDesc: + return (a, b) { + final byPriority = b.priority.value.compareTo(a.priority.value); + return byPriority != 0 + ? byPriority + : b.updatedAt.compareTo(a.updatedAt); + }; + } +} + +/// Midnight (local) of [t]'s calendar day. +DateTime _startOfDay(DateTime t) => DateTime(t.year, t.month, t.day); diff --git a/lib/data/note_repository.dart b/lib/data/note_repository.dart index 5e77457..8e2efdd 100644 --- a/lib/data/note_repository.dart +++ b/lib/data/note_repository.dart @@ -2,6 +2,11 @@ import 'dart:async'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:todo/data/note.dart'; +import 'package:todo/data/note_import.dart'; +import 'package:todo/data/note_query.dart'; + +export 'package:todo/data/note_import.dart'; +export 'package:todo/data/note_query.dart'; // Deliberately free of `dart:io`: the desktop app is a web build, and a single // `dart:io` import anywhere in the graph makes the whole app fail to compile @@ -9,43 +14,6 @@ import 'package:todo/data/note.dart'; // conditional export in `repository_factory.dart`; the legacy sqlite migration // lives in the io-only `legacy_sqlite_migration.dart`. -/// How the history list should be ordered. -enum NoteSort { - /// Newest created first. - createdDesc, - - /// Most recently modified first. - modifiedDesc, - - /// A-Z by note text. - alphabetical, - - /// Highest [Priority] first. - priorityDesc, -} - -/// Summary of an [NoteRepository.importNotes] run, for user feedback. -class ImportOutcome { - /// Creates an [ImportOutcome] from its per-category counts. - const ImportOutcome({ - required this.added, - required this.updated, - required this.skipped, - }); - - /// Notes that did not exist locally and were created. - final int added; - - /// Existing notes overwritten because the import was newer. - final int updated; - - /// Notes skipped because the local copy was the same age or newer. - final int skipped; - - /// Total notes considered in the import. - int get total => added + updated + skipped; -} - /// Local-first persistence for [Note]s, backed by the shared `crdt_sync` /// [LogStore] (was `sqlite_crdt`). /// @@ -110,7 +78,7 @@ class NoteRepository { static Future openInMemory({ String nodeId = 'test-node', }) async { - final store = LogStore(persistence: _MemoryPersistence(), nodeId: nodeId); + final store = LogStore(persistence: MemoryPersistence(), nodeId: nodeId); await store.load(); return NoteRepository._(store, nodeId); } @@ -273,157 +241,8 @@ class NoteRepository { int _liveCount() => _store.values.where((r) => !r.deleted).length; List _query(NoteSort sort, NoteFilter filter) { - final notes = _liveNotes().where((n) => _matches(n, filter)).toList() - ..sort(_comparator(sort)); + final notes = _liveNotes().where((n) => matchesFilter(n, filter)).toList() + ..sort(comparatorFor(sort)); return notes; } - - bool _matches(Note note, NoteFilter filter) { - final query = filter.query.trim().toLowerCase(); - if (query.isNotEmpty && !note.text.toLowerCase().contains(query)) { - return false; - } - if (filter.priorities.isNotEmpty && - !filter.priorities.contains(note.priority)) { - return false; - } - if (filter.statuses.isNotEmpty && !filter.statuses.contains(note.status)) { - return false; - } - if (!_withinDays(note.createdAt, filter.createdFrom, filter.createdTo)) { - return false; - } - if (!_withinDays(note.updatedAt, filter.updatedFrom, filter.updatedTo)) { - return false; - } - return true; - } - - /// Inclusive day-granularity range check, matching the old SQL bounds: - /// `from` includes its whole day; `to` includes its whole day. - static bool _withinDays(DateTime value, DateTime? from, DateTime? to) { - if (from != null && value.isBefore(_startOfDay(from))) return false; - if (to != null && - !value.isBefore(_startOfDay(to).add(const Duration(days: 1)))) { - return false; - } - return true; - } - - static Comparator _comparator(NoteSort sort) { - switch (sort) { - case NoteSort.createdDesc: - return (a, b) => b.createdAt.compareTo(a.createdAt); - case NoteSort.modifiedDesc: - return (a, b) => b.updatedAt.compareTo(a.updatedAt); - case NoteSort.alphabetical: - return (a, b) => a.text.toLowerCase().compareTo(b.text.toLowerCase()); - case NoteSort.priorityDesc: - return (a, b) { - final byPriority = b.priority.value.compareTo(a.priority.value); - return byPriority != 0 - ? byPriority - : b.updatedAt.compareTo(a.updatedAt); - }; - } - } - - /// Midnight (local) of [t]'s calendar day. - static DateTime _startOfDay(DateTime t) => DateTime(t.year, t.month, t.day); -} - -/// In-memory [LogPersistence] for [NoteRepository.openInMemory] (tests). -class _MemoryPersistence implements LogPersistence { - String? _text; - - @override - Future read() async => _text; - - @override - Future write(String text) async => _text = text; -} - -/// An immutable set of constraints for querying notes. -/// -/// All fields combine with logical AND. Empty/null fields impose no -/// constraint. Lives in the data layer so the filtering it drives never leaks -/// into the UI. Construct copies with [copyWith] when toggling one facet. -class NoteFilter { - /// Creates a [NoteFilter] from its constraint fields. - const NoteFilter({ - this.query = '', - this.priorities = const {}, - this.statuses = const {}, - this.createdFrom, - this.createdTo, - this.updatedFrom, - this.updatedTo, - }); - - /// Case-insensitive substring matched against the note body. - final String query; - - /// Notes must have one of these priorities. Empty means "any priority". - final Set priorities; - - /// Notes must have one of these statuses. Empty means "any status". - final Set statuses; - - /// Inclusive lower bound (by calendar day) on the creation date. - final DateTime? createdFrom; - - /// Inclusive upper bound (by calendar day) on the creation date. - final DateTime? createdTo; - - /// Inclusive lower bound (by calendar day) on the last-updated date. - final DateTime? updatedFrom; - - /// Inclusive upper bound (by calendar day) on the last-updated date. - final DateTime? updatedTo; - - /// True when no constraint is active (the unfiltered, full list). - bool get isEmpty => - query.trim().isEmpty && - priorities.isEmpty && - statuses.isEmpty && - createdFrom == null && - createdTo == null && - updatedFrom == null && - updatedTo == null; - - /// Number of distinct active facets, for an "N filters" badge in the UI. - int get activeCount { - var n = 0; - if (query.trim().isNotEmpty) n++; - if (priorities.isNotEmpty) n++; - if (statuses.isNotEmpty) n++; - if (createdFrom != null || createdTo != null) n++; - if (updatedFrom != null || updatedTo != null) n++; - return n; - } - - /// Returns a copy with selected facets replaced. A `null` argument keeps - /// the current value; clearing a date is done via the dedicated clear - /// flags so `null` can mean "unchanged". - NoteFilter copyWith({ - String? query, - Set? priorities, - Set? statuses, - DateTime? createdFrom, - DateTime? createdTo, - DateTime? updatedFrom, - DateTime? updatedTo, - bool clearCreated = false, - bool clearUpdated = false, - }) { - return NoteFilter( - query: query ?? this.query, - priorities: priorities ?? this.priorities, - statuses: statuses ?? this.statuses, - createdFrom: clearCreated ? null : (createdFrom ?? this.createdFrom), - createdTo: clearCreated ? null : (createdTo ?? this.createdTo), - updatedFrom: clearUpdated ? null : (updatedFrom ?? this.updatedFrom), - updatedTo: clearUpdated ? null : (updatedTo ?? this.updatedTo), - ); - } } diff --git a/pubspec.yaml b/pubspec.yaml index 86c9ece..74b8f65 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html # In Windows, build-name is used as the major, minor, and patch parts # of the product and file versions while build-number is used as the build suffix. -version: 1.0.14+1 +version: 1.0.15+1 environment: sdk: ^3.12.2