Skip to content

Fix _logId() collision in SyncRepository with per-instance counter #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Harsh4114 merged 2 commits into from
Feb 20, 2026
Merged

Fix _logId() collision in SyncRepository with per-instance counter #8

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21c8e88
Initial plan
Copilot Feb 20, 2026
e156a18
Fix _logId() collision: add per-instance counter to SyncRepository
Copilot Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/src/bloc_like/repository/sync_repository.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ class SyncRepository<T> {
final SyncLogStore logStore;
final EntityIdResolver<T> idResolver;

int _logCounter = 0;

Comment on lines +14 to +15

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_logId() now guarantees uniqueness only within a single SyncRepository instance. If multiple repositories share the same SyncLogStore (or if a store persists across app restarts and the counter resets), collisions can still happen because the ID lacks a repository/device/component discriminator. Consider including a configurable prefix (e.g., repository/device ID) or passing a logIdNamespace/instanceId into the repository so IDs are unique across all writers to the same store.

Copilot uses AI. Check for mistakes.
SyncRepository({
required this.local,
required this.cloud,
Expand Down Expand Up @@ -142,7 +144,8 @@ class SyncRepository<T> {
}

String _logId() {
_logCounter += 1;
final micros = DateTime.now().microsecondsSinceEpoch;
return 'sync_log_$micros';
return 'sync_log_${micros}_$_logCounter';
Comment on lines 146 to +149

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change addresses a timing-resolution collision, but there’s no deterministic test covering it (calling add() in a tight loop may not reliably reproduce the original bug on all platforms). To make this behavior testable and prevent regressions, consider injecting a time provider/clock into SyncRepository (used by _logId() and SyncLog.timestamp) so a test can force identical timestamps and assert IDs remain unique.

Copilot uses AI. Check for mistakes.
}
}