Extract WorkoutSyncSession from workout_sync_service.dart (322 -> 218)

The one file on the list that could not be fixed by moving code, so the
approach was agreed with the user first: extract a real collaborator rather
than change the public surface.

`push`, `pushManual` and `syncNow` were three near-identical bodies — same
configured-or-bail guard, same GitHubClient/Firebase/MirrorStore construction,
same getFileText/mergeLogs/syncLog round-trip, same catch-everything-and-report
handler, same close-both finally. They differed in the record they merged in
(one, one, none) and two strings. That shared body is now
WorkoutSyncSession.run(), and the three methods are thin wrappers holding only
their own guard and their own wording.

Why a collaborator and not a `part` extension: extension methods dispatch
statically. Four fakes in _home_test_fixtures.dart override `syncNow`, and
manual_workout_screen_test.dart overrides `pushManual` and
`readMergedManualPayloads` — with those in an extension the real
implementations would run while the fakes sat unused, which is how four tests
hung on pumpAndSettle in an earlier attempt. All four remain ordinary methods
on WorkoutSyncService, so every override still binds.

`successReason` and `failurePrefix` are passed in rather than derived from one
another, so the exact strings the tests assert on ('pushed' / 'push failed',
'synced' / 'sync failed') stay under the callers' control.

Unlike the part files, this is a real library with its own lcov SF: entry, so
it has to carry its own coverage — it does. The total drops 2751 -> 2728
covered lines, which is the duplication the extraction removed.

402 tests pass; dart analyze clean; coverage 100%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EveB813BQ8abAP68VSqZ5Y
This commit is contained in:
Krzysztof Rudnicki 2026-08-15 22:28:02 +02:00
parent 1e4719b4de
commit ac7a7afa08
2 changed files with 172 additions and 138 deletions

View File

@ -13,6 +13,7 @@ import 'package:workout_app/services/firebase_backend.dart';
import 'package:workout_app/services/sync_device_id.dart';
import 'package:workout_app/services/sync_settings.dart';
import 'package:workout_app/services/sync_state_factory.dart';
import 'package:workout_app/services/workout_sync_session.dart';
part 'workout_sync_service_read.dart';
@ -118,64 +119,33 @@ class WorkoutSyncService {
log('WorkoutSyncService.push skipped: $reason', level: 900);
return const PushResult(pushed: false, reason: reason);
}
final github = GitHubClient(
owner: owner,
repo: repo,
token: settings.token,
httpClient: _httpClient,
);
final firebase = await _openFirebase();
final client = firebase == null
? github
: MirrorStore(primary: firebase, mirror: github);
try {
final path = '$_pathPrefix/$currentSyncDeviceId/$_logFilename';
final existingText = await client.getFileText(path);
final existingLog = existingText == null
? <String, Record>{}
: _decode(existingText);
final record = Record(
return _session(settings.token).run(
operation: 'push',
successReason: 'pushed',
failurePrefix: 'push failed',
addition: Record(
id: session.startTime.toIso8601String(),
fields: {
'payload': (session.toJson(), Hlc.newTick(currentSyncDeviceId)),
},
);
final localLog = mergeLogs(existingLog, {record.id: record});
await syncLog(
client: client,
deviceId: currentSyncDeviceId,
legacyDeviceId: legacySyncDeviceId,
pathPrefix: _pathPrefix,
localLog: localLog,
encode: _encode,
decode: _decode,
// Without this every push re-uploads the whole log and every pull
// re-downloads every peer's, regardless of change -- the traffic the
// Firebase free tier's monthly budget depends on not happening.
stateStore: await _openStateStore(),
);
return const PushResult(pushed: true, reason: 'pushed');
} on Object catch (error, stackTrace) {
// Deliberately broad: a GitHubSyncError was the only case handled
// before, so a Firebase error, an auth failure or a dropped connection
// vanished with no trace at all. Still swallowed rather than rethrown
// -- a sync failure must not cost the user their finished workout --
// but now it is reported, not hidden.
final reason = 'push failed: $error';
log(
'WorkoutSyncService.push failed',
level: 1000,
error: error,
stackTrace: stackTrace,
);
return PushResult(pushed: false, reason: reason);
} finally {
github.close();
firebase?.close();
}
),
);
}
/// Builds the collaborator that owns one round-trip's client lifecycle.
WorkoutSyncSession _session(String token) => WorkoutSyncSession(
owner: owner,
repo: repo,
token: token,
pathPrefix: _pathPrefix,
logFilename: _logFilename,
encode: _encode,
decode: _decode,
openFirebaseClient: _openFirebase,
openStateStore: _openStateStore,
httpClient: _httpClient,
);
/// Runs a sync tick with no new record, purely to find out whether sync
/// works right now.
///
@ -193,51 +163,16 @@ class WorkoutSyncService {
log('WorkoutSyncService.syncNow skipped: $reason', level: 900);
return const PushResult(pushed: false, reason: reason);
}
final github = GitHubClient(
owner: owner,
repo: repo,
token: settings.token,
httpClient: _httpClient,
// No addition: whatever this device already has, unchanged. The point is
// the round trip, not the payload.
return _session(settings.token).run(
operation: 'syncNow',
successReason: 'synced',
failurePrefix: 'sync failed',
);
final firebase = await _openFirebase();
final client = firebase == null
? github
: MirrorStore(primary: firebase, mirror: github);
try {
final path = '$_pathPrefix/$currentSyncDeviceId/$_logFilename';
final existingText = await client.getFileText(path);
final existingLog = existingText == null
? <String, Record>{}
: _decode(existingText);
await syncLog(
client: client,
deviceId: currentSyncDeviceId,
legacyDeviceId: legacySyncDeviceId,
pathPrefix: _pathPrefix,
// No new record: whatever this device already has, unchanged. The
// point is the round trip, not the payload.
localLog: existingLog,
encode: _encode,
decode: _decode,
stateStore: await _openStateStore(),
);
return const PushResult(pushed: true, reason: 'synced');
} on Object catch (error, stackTrace) {
final reason = 'sync failed: $error';
log(
'WorkoutSyncService.syncNow failed',
level: 1000,
error: error,
stackTrace: stackTrace,
);
return PushResult(pushed: false, reason: reason);
} finally {
github.close();
firebase?.close();
}
}
/// Pushes a pre-built manual-workout [record] to this device's log.
///
/// Same never-throw, always-report contract as [push].
@ -248,51 +183,12 @@ class WorkoutSyncService {
log('WorkoutSyncService.pushManual skipped: $reason', level: 900);
return const PushResult(pushed: false, reason: reason);
}
final github = GitHubClient(
owner: owner,
repo: repo,
token: settings.token,
httpClient: _httpClient,
return _session(settings.token).run(
operation: 'pushManual',
successReason: 'pushed',
failurePrefix: 'push failed',
addition: record,
);
final firebase = await _openFirebase();
final client = firebase == null
? github
: MirrorStore(primary: firebase, mirror: github);
try {
final path = '$_pathPrefix/$currentSyncDeviceId/$_logFilename';
final existingText = await client.getFileText(path);
final existingLog = existingText == null
? <String, Record>{}
: _decode(existingText);
final localLog = mergeLogs(existingLog, {record.id: record});
await syncLog(
client: client,
deviceId: currentSyncDeviceId,
legacyDeviceId: legacySyncDeviceId,
pathPrefix: _pathPrefix,
localLog: localLog,
encode: _encode,
decode: _decode,
// Without this every push re-uploads the whole log and every pull
// re-downloads every peer's, regardless of change -- the traffic the
// Firebase free tier's monthly budget depends on not happening.
stateStore: await _openStateStore(),
);
return const PushResult(pushed: true, reason: 'pushed');
} on Object catch (error, stackTrace) {
final reason = 'push failed: $error';
log(
'WorkoutSyncService.pushManual failed',
level: 1000,
error: error,
stackTrace: stackTrace,
);
return PushResult(pushed: false, reason: reason);
} finally {
github.close();
firebase?.close();
}
}
/// Returns every device's manual-workout payloads, merged and deduped by id

View File

@ -0,0 +1,138 @@
/// One sync round-trip: open the backends, merge a log, close them again.
library;
import 'dart:developer';
import 'package:crdt_sync/crdt_sync.dart';
import 'package:http/http.dart' as http;
import 'package:workout_app/services/sync_device_id.dart';
import 'package:workout_app/services/workout_sync_service.dart';
/// Runs a single sync tick against GitHub, Firebase, or both.
///
/// `push`, `pushManual` and `syncNow` on [WorkoutSyncService] were three
/// near-identical bodies: the same configured-or-bail guard, the same
/// GitHub/Firebase/[MirrorStore] construction, the same try/catch/finally, and
/// the same never-throw-always-report contract. Only the log they submit and
/// the success wording differed.
///
/// This is a collaborator the service delegates to rather than a `part`
/// extension, deliberately. Test fakes bind to `push` / `pushManual` /
/// `syncNow` with `extends WorkoutSyncService`, and extension methods dispatch
/// statically — moving those into an extension would mean the real
/// implementation ran while the fake sat unused. Keeping them as thin methods
/// on the class preserves every override; only the shared body moved here.
class WorkoutSyncSession {
/// Creates a session bound to one repo and one pair of backends.
const WorkoutSyncSession({
required this.owner,
required this.repo,
required this.token,
required this.pathPrefix,
required this.logFilename,
required this.encode,
required this.decode,
required this.openFirebaseClient,
required this.openStateStore,
this.httpClient,
});
/// The repo owner/org to sync with.
final String owner;
/// The repo name to sync with.
final String repo;
/// The GitHub token. Empty when only Firebase is configured, which is what
/// [SyncSettings] reports and what the mirror treats as unauthenticated.
final String token;
/// Directory prefix every device's log lives under.
final String pathPrefix;
/// Filename of a device's log within its directory.
final String logFilename;
/// Serializes a log for storage.
final String Function(Log log) encode;
/// Parses a stored log.
final Log Function(String text) decode;
/// Opens the Firebase backend, or returns null when not set up.
final Future<FirebaseRestClient?> Function() openFirebaseClient;
/// Opens the revision cache that lets an unchanged peer be skipped.
final Future<SyncStateStore> Function() openStateStore;
/// Overrides the HTTP client; tests point this at a mock.
final http.Client? httpClient;
/// Runs one tick, merging [addition] into this device's log if given.
///
/// Pass null for [addition] to round-trip the existing log unchanged — that
/// is how a connectivity check gets a real answer without inventing a
/// record to push.
///
/// Never throws. [operation] names the caller in log lines; [successReason]
/// and [failurePrefix] are passed in rather than derived, so the three
/// callers keep the exact strings their tests assert on.
Future<PushResult> run({
required String operation,
required String successReason,
required String failurePrefix,
Record? addition,
}) async {
final github = GitHubClient(
owner: owner,
repo: repo,
token: token,
httpClient: httpClient,
);
final firebase = await openFirebaseClient();
final client = firebase == null
? github
: MirrorStore(primary: firebase, mirror: github);
try {
final path = '$pathPrefix/$currentSyncDeviceId/$logFilename';
final existingText = await client.getFileText(path);
final existingLog = existingText == null
? <String, Record>{}
: decode(existingText);
final localLog = addition == null
? existingLog
: mergeLogs(existingLog, {addition.id: addition});
await syncLog(
client: client,
deviceId: currentSyncDeviceId,
legacyDeviceId: legacySyncDeviceId,
pathPrefix: pathPrefix,
localLog: localLog,
encode: encode,
decode: decode,
// Without this every push re-uploads the whole log and every pull
// re-downloads every peer's, regardless of change -- the traffic the
// Firebase free tier's monthly budget depends on not happening.
stateStore: await openStateStore(),
);
return PushResult(pushed: true, reason: successReason);
} on Object catch (error, stackTrace) {
// Deliberately broad: a GitHubSyncError was the only case handled
// before, so a Firebase error, an auth failure or a dropped connection
// vanished with no trace at all. Still swallowed rather than rethrown
// -- a sync failure must not cost the user their finished workout --
// but now it is reported, not hidden.
final reason = '$failurePrefix: $error';
log(
'WorkoutSyncService.$operation failed',
level: 1000,
error: error,
stackTrace: stackTrace,
);
return PushResult(pushed: false, reason: reason);
} finally {
github.close();
firebase?.close();
}
}
}