diff --git a/packages/shorebird_cli/lib/src/code_push_client_wrapper.dart b/packages/shorebird_cli/lib/src/code_push_client_wrapper.dart index 52567f584..1cae5de17 100644 --- a/packages/shorebird_cli/lib/src/code_push_client_wrapper.dart +++ b/packages/shorebird_cli/lib/src/code_push_client_wrapper.dart @@ -94,6 +94,12 @@ class CodePushClientWrapper { /// The underlying code push client. final CodePushClient codePushClient; + /// Patch ids promoted by this wrapper instance — command-scoped, so a + /// fresh `shorebird patch` gets an empty set and cross-invocation reuse + /// still prompts. Suppresses the append-after-promotion warning for the + /// second platform of a single `--platforms ios,android` invocation. + final Set _patchesPromotedThisRun = {}; + /// Create an app with the given [organizationId] and [appName]. Future createApp({required int organizationId, String? appName}) async { late final String displayName; @@ -872,11 +878,16 @@ aar artifact already exists, continuing...'''); } /// Creates a patch for the given [appId], [releaseId], and [metadata]. + /// + /// When [clientPatchId] is supplied, the server treats the call as + /// idempotent: if a patch on this release already has that id, the + /// existing patch is returned instead of creating a new one. @visibleForTesting - Future createPatch({ + Future createPatch({ required String appId, required int releaseId, required Json metadata, + String? clientPatchId, }) async { final createPatchProgress = logger.progress('Creating patch'); try { @@ -884,6 +895,7 @@ aar artifact already exists, continuing...'''); appId: appId, releaseId: releaseId, metadata: metadata, + clientPatchId: clientPatchId, ); createPatchProgress.complete(); return patch; @@ -896,7 +908,7 @@ aar artifact already exists, continuing...'''); @visibleForTesting Future createPatchArtifacts({ required String appId, - required Patch patch, + required CreatePatchResponse patch, required ReleasePlatform platform, required Map patchArtifactBundles, }) async { @@ -943,21 +955,37 @@ aar artifact already exists, continuing...'''); /// Publishes a patch to the Shorebird server. This consists of creating a /// patch, uploading patch artifacts, and promoting the patch to a specific - /// channel based on the provided [track]. - Future publishPatch({ + /// channel based on the provided [track]. Returns the resulting + /// [CreatePatchResponse] so callers can compose a final success message + /// that spans every platform in the invocation. + Future publishPatch({ required String appId, required int releaseId, required Json metadata, required ReleasePlatform platform, required DeploymentTrack track, required Map patchArtifactBundles, + String? clientPatchId, }) async { final patch = await createPatch( appId: appId, releaseId: releaseId, metadata: metadata, + clientPatchId: clientPatchId, ); + // When the create call was idempotent (a clientPatchId hit on an existing + // patch row that's already on the stable channel), uploading this + // platform's artifacts will make them go live immediately to that + // platform's stable users. The server is permissive; the CLI is + // responsible for making sure the developer knows. + if (clientPatchId != null) { + await _confirmAppendToPromotedPatch( + patch: patch, + platform: platform, + ); + } + await createPatchArtifacts( appId: appId, patch: patch, @@ -970,8 +998,55 @@ aar artifact already exists, continuing...'''); await createChannel(appId: appId, name: track.channel); await promotePatch(appId: appId, patchId: patch.id, channel: channel); + _patchesPromotedThisRun.add(patch.id); + + return patch; + } - logger.success('\n✅ Published Patch ${patch.number}!'); + /// Detects the "idempotent hit on an already-promoted patch" case: the + /// caller passed a clientPatchId that matched an existing patch which is + /// already promoted to the stable channel. Uploading this platform's + /// artifacts will go live immediately to that platform's stable users. + /// + /// In an interactive terminal we surface that fact and prompt before + /// continuing. In CI we proceed silently — the iOS-promotes-then-Android- + /// completes flow is the canonical use case and prompting would deadlock. + /// + /// Skipped entirely when this run is the one that promoted the patch + /// (e.g. the second platform of a `--platforms ios,android` invocation), + /// since the user already opted in by passing both platforms together. + /// + /// The patch's current channel comes from [CreatePatchResponse.channel], + /// which the server populates on the idempotent return path. Older servers + /// that don't echo `channel` leave it null and the prompt simply doesn't + /// fire — append-after-promotion stays allowed per the design. + Future _confirmAppendToPromotedPatch({ + required CreatePatchResponse patch, + required ReleasePlatform platform, + }) async { + // If this run promoted the patch itself (e.g. the first platform of + // `--platforms ios,android`), the "already on stable" state is our own + // doing and the prompt would be spurious. + if (_patchesPromotedThisRun.contains(patch.id)) return; + + if (patch.channel != DeploymentTrack.stable.channel) return; + + final platformName = platform.displayName; + final message = + 'Patch ${patch.number} is already promoted to the stable track. ' + 'Uploading $platformName artifacts will go live to $platformName ' + 'stable users immediately.'; + + if (shorebirdEnv.canAcceptUserInput) { + logger.warn(message); + if (!logger.confirm('Continue?')) { + logger.info('Aborting.'); + throw ProcessExit(ExitCode.success.code); + } + } else { + // CI: log so the action is traceable, but proceed. + logger.info(message); + } } /// Returns a GCP download link for measuring download speed. diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart index 812b4778f..e8fe7c450 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart @@ -13,6 +13,7 @@ import 'package:shorebird_cli/src/commands/patch/patch.dart'; import 'package:shorebird_cli/src/common_arguments.dart'; import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/deployment_track.dart'; +import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/extensions/arg_results.dart'; import 'package:shorebird_cli/src/extensions/string.dart'; import 'package:shorebird_cli/src/formatters/formatters.dart'; @@ -30,6 +31,7 @@ import 'package:shorebird_cli/src/shorebird_validator.dart'; import 'package:shorebird_cli/src/third_party/flutter_tools/lib/flutter_tools.dart'; import 'package:shorebird_cli/src/version.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; +import 'package:uuid/uuid.dart'; /// Signature for a function that returns a [Patcher] for a given [ReleaseType]. typedef ResolvePatcher = Patcher Function(ReleaseType releaseType); @@ -98,6 +100,19 @@ To target the latest release (e.g. the release that was most recently updated) u help: 'The track to publish the patch to.', defaultsTo: DeploymentTrack.stable.channel, ) + ..addOption( + 'patch-id', + help: ''' +A stable correlation key (e.g. a git SHA) used to unify a logical patch +across platforms. When two invocations on the same release supply the same +--patch-id, the server returns the existing patch instead of allocating a +new number — the iOS and Android halves of the same change end up sharing +one patch number visible to end users. + +When omitted on a multi-platform invocation, the CLI generates a fresh +correlation key locally so the platforms in this single command share one +patch.''', + ) ..addFlag( 'staging', negatable: false, @@ -215,6 +230,80 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl /// The deployment track to publish the patch to. DeploymentTrack get track => DeploymentTrack(results['track'] as String); + /// The correlation key used to make patch creation idempotent across + /// platforms within this invocation. When the user passes `--patch-id`, + /// that value is used as-is. When the user omits it on a multi-platform + /// invocation, a fresh UUID is generated so the platforms in this single + /// command end up sharing one patch number on the server. + /// + /// Returns null only for single-platform invocations with no `--patch-id`, + /// preserving the legacy behavior where the server allocates a fresh patch + /// number per call. + /// + /// Resolved on first read and cached for the rest of the command so every + /// platform's `createPatch` sees the same value. Requires `argResults` to + /// be wired before first access — every code path through `run()` (and + /// every test going through `runWithOverrides`) satisfies that. + late final String? clientPatchId = _resolveClientPatchId(); + + String? _resolveClientPatchId() { + // `run()` already rejects an empty `--patch-id`, so non-null here implies + // non-empty. Empty-to-null coalescence lives at the client boundary + // (`CodePushClient.createPatch`) and stays the single normalizer. + final explicit = results['patch-id'] as String?; + if (explicit != null) return explicit; + if (results.releaseTypes.length > 1) return const Uuid().v4(); + return null; + } + + /// Patches collected from each platform's [createPatch] run, used to emit + /// a single aggregated success message after the fan-out completes. + @visibleForTesting + final platformPatches = {}; + + /// Warns when `--patch-id` is the current `HEAD` commit but the working + /// tree has uncommitted changes — the SHA wouldn't actually identify the + /// code being shipped. Detection is intentionally narrow (literal equality + /// with `git rev-parse HEAD`) so non-SHA ids like `hotfix-login` or CI run + /// IDs never trigger a false positive. Non-blocking: local debug patches + /// over uncommitted code are a legitimate flow. + @visibleForTesting + Future warnIfDirtyTreeMatchesPatchId() async { + final patchId = results['patch-id'] as String?; + if (patchId == null || patchId.isEmpty) return; + + final projectRoot = shorebirdEnv.getShorebirdProjectRoot(); + if (projectRoot == null) return; + + final String head; + try { + head = await git.revParse( + revision: 'HEAD', + directory: projectRoot.path, + ); + } on Exception { + return; + } + if (patchId != head) return; + + final String porcelain; + try { + porcelain = await git.status( + directory: projectRoot.path, + args: ['--porcelain'], + ); + } on Exception { + return; + } + if (porcelain.isEmpty) return; + + logger.warn( + '--patch-id is set to HEAD ($patchId), but the working tree has ' + 'uncommitted changes — this SHA does not identify the code being ' + 'shipped.', + ); + } + @override Future run() async { if (results.releaseTypes.isEmpty) { @@ -231,6 +320,20 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl return ExitCode.usage.code; } + // Reject `--patch-id=` (present but empty) outright. The typical cause is + // an unexpanded CI template variable; silently treating it as omitted + // would mean a multi-platform invocation generates a fresh UUID per bot, + // so iOS and Android never converge on the same patch number. + final patchIdRaw = results['patch-id'] as String?; + if (patchIdRaw != null && patchIdRaw.isEmpty) { + logger.err( + r'''--patch-id was provided but is empty. This usually means an unexpanded template variable in your CI config (e.g. --patch-id=${{ env.PATCH_SHA }} where PATCH_SHA is not set). Pass a non-empty value or omit the flag.''', + ); + return ExitCode.usage.code; + } + + await warnIfDirtyTreeMatchesPatchId(); + final patcherFutures = results.releaseTypes .map(_resolvePatcher) .map(createPatch); @@ -239,9 +342,37 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl await patcherFuture; } + logUnifiedSuccess(); + return ExitCode.success.code; } + /// Emits one aggregated success line per patch number that ended up with + /// at least one published platform — replaces the old per-platform log + /// that used to live inside the wrapper's `publishPatch`. + @visibleForTesting + void logUnifiedSuccess() { + if (platformPatches.isEmpty) return; + + // Group platforms by the patch number they ended up under. Under unified + // numbering this is almost always a single group, but pinning by number + // keeps the message correct in degenerate cases (different releases per + // platform, the user explicitly skipping --patch-id on a multi-platform + // call against a server that hasn't been updated, etc.). + final byPatchNumber = >{}; + for (final entry in platformPatches.entries) { + byPatchNumber.putIfAbsent(entry.value.number, () => []).add(entry.key); + } + + for (final number in byPatchNumber.keys.toList()..sort()) { + final platforms = + byPatchNumber[number]!.map((p) => p.displayName).toList()..sort(); + logger.success( + '\n✅ Published Patch $number (${platforms.join(', ')})!', + ); + } + } + /// Returns a [Patcher] for the given [ReleaseType]. @visibleForTesting Patcher getPatcher(ReleaseType releaseType) { @@ -574,13 +705,15 @@ Building patch with Flutter $flutterVersionString baseMetadata, ); - await patcher.uploadPatchArtifacts( + final publishedPatch = await patcher.uploadPatchArtifacts( appId: appId, releaseId: release.id, metadata: updateMetadata.toJson(), track: track, artifacts: patchArtifactBundles, + clientPatchId: clientPatchId, ); + platformPatches[patcher.releaseType.releasePlatform] = publishedPatch; }, values: {shorebirdEnvRef.overrideWith(() => releaseFlutterShorebirdEnv)}, ); diff --git a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart index 16b2922ad..bb70f4aac 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart @@ -134,21 +134,25 @@ More info: ${troubleshootingUrl.toLink()}. return metadata; } - /// Uploads the patch artifacts to the CodePush server. - Future uploadPatchArtifacts({ + /// Uploads the patch artifacts to the CodePush server. Returns the resulting + /// [CreatePatchResponse] so the orchestrating command can emit one + /// aggregated success message across every platform in the invocation. + Future uploadPatchArtifacts({ required String appId, required int releaseId, required Map metadata, required Map artifacts, required DeploymentTrack track, + String? clientPatchId, }) async { - await codePushClientWrapper.publishPatch( + return codePushClientWrapper.publishPatch( appId: appId, releaseId: releaseId, metadata: metadata, platform: releaseType.releasePlatform, track: track, patchArtifactBundles: artifacts, + clientPatchId: clientPatchId, ); } diff --git a/packages/shorebird_cli/test/src/code_push_client_wrapper_test.dart b/packages/shorebird_cli/test/src/code_push_client_wrapper_test.dart index 7c2ed7b78..ba7bf55c6 100644 --- a/packages/shorebird_cli/test/src/code_push_client_wrapper_test.dart +++ b/packages/shorebird_cli/test/src/code_push_client_wrapper_test.dart @@ -112,7 +112,7 @@ void main() { final channel = Channel(id: 0, appId: appId, name: track.channel); const patchId = 1; const patchNumber = 2; - const patch = Patch(id: patchId, number: patchNumber); + const patch = CreatePatchResponse(id: patchId, number: patchNumber); const releasePlatform = ReleasePlatform.ios; const releaseId = 123; const arch = Arch.arm64; @@ -2513,6 +2513,7 @@ You can manage this release in the ${link(uri: uri, message: 'Shorebird Console' appId: any(named: 'appId'), releaseId: any(named: 'releaseId'), metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), ), ).thenAnswer((_) async => patch); when( @@ -2528,6 +2529,12 @@ You can manage this release in the ${link(uri: uri, message: 'Shorebird Console' when( () => codePushClient.getChannels(appId: any(named: 'appId')), ).thenAnswer((_) async => [channel]); + when( + () => codePushClient.getPatches( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + ), + ).thenAnswer((_) async => []); when( () => codePushClient.promotePatch( appId: any(named: 'appId'), @@ -2638,8 +2645,8 @@ You can manage this release in the ${link(uri: uri, message: 'Shorebird Console' ).called(1); }); - test('prints patch number on success', () async { - await runWithOverrides( + test('returns the patch from the server', () async { + final result = await runWithOverrides( () => codePushClientWrapper.publishPatch( appId: appId, releaseId: releaseId, @@ -2650,10 +2657,313 @@ You can manage this release in the ${link(uri: uri, message: 'Shorebird Console' ), ); - verify( - () => logger.success(any(that: contains('Published Patch 2!'))), - ).called(1); + expect(result, equals(patch)); + // The aggregated success message is now emitted by patch_command; + // publishPatch itself no longer logs. + verifyNever( + () => logger.success(any(that: contains('Published Patch'))), + ); + }); + + test( + 'forwards clientPatchId to enable cross-platform idempotency', + () async { + const clientPatchId = 'sha-abcdef'; + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: clientPatchId, + ), + ); + + verify( + () => codePushClient.createPatch( + appId: appId, + releaseId: releaseId, + metadata: {'foo': 'bar'}, + clientPatchId: clientPatchId, + ), + ).called(1); + }, + ); + + // Append-after-promotion: idempotent createPatch returned an existing + // patch that's already on the stable channel, so uploading this + // platform's artifacts goes live to stable users immediately. The + // server reports the current channel inline on CreatePatchResponse, + // so the CLI doesn't need a second round-trip to detect this. + group('when appending to a patch already promoted to stable', () { + const promotedPatch = CreatePatchResponse( + id: patchId, + number: patchNumber, + channel: 'stable', + ); + + setUp(() { + when( + () => codePushClient.createPatch( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((_) async => promotedPatch); + }); + + test( + 'prompts and proceeds when the user confirms in a TTY', + () async { + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(true); + when(() => logger.confirm(any())).thenReturn(true); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + verify( + () => logger.warn( + any(that: contains('already promoted to the stable track')), + ), + ).called(1); + verify(() => logger.confirm(any())).called(1); + verify( + () => codePushClient.createPatchArtifact( + appId: appId, + artifactPath: any(named: 'artifactPath'), + patchId: patchId, + arch: any(named: 'arch'), + platform: any(named: 'platform'), + hash: any(named: 'hash'), + ), + ).called(1); + }, + ); + + test( + 'aborts before uploading artifacts when the user declines', + () async { + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(true); + when(() => logger.confirm(any())).thenReturn(false); + + await expectLater( + () async => runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ), + exitsWithCode(ExitCode.success), + ); + + verifyNever( + () => codePushClient.createPatchArtifact( + appId: any(named: 'appId'), + artifactPath: any(named: 'artifactPath'), + patchId: any(named: 'patchId'), + arch: any(named: 'arch'), + platform: any(named: 'platform'), + hash: any(named: 'hash'), + ), + ); + verifyNever( + () => codePushClient.promotePatch( + appId: any(named: 'appId'), + patchId: any(named: 'patchId'), + channelId: any(named: 'channelId'), + ), + ); + }, + ); + + test( + 'proceeds silently in CI without prompting', + () async { + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(false); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + verifyNever(() => logger.confirm(any())); + verify( + () => logger.info( + any(that: contains('already promoted to the stable track')), + ), + ).called(1); + verify( + () => codePushClient.createPatchArtifact( + appId: appId, + artifactPath: any(named: 'artifactPath'), + patchId: patchId, + arch: any(named: 'arch'), + platform: any(named: 'platform'), + hash: any(named: 'hash'), + ), + ).called(1); + }, + ); + + // Single-invocation `--platforms ios,android`: the first platform + // promotes the patch to stable; the second platform's call sees + // it as "already promoted" but we shouldn't prompt — the user + // opted into both platforms in one command. + test( + 'does not prompt on the second platform of the same run', + () async { + // First call: fresh patch, no channel populated yet. + when( + () => codePushClient.createPatch( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((_) async => patch); + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(true); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + // Second call: idempotent hit, server reports the patch is + // already on stable (because the first call promoted it). + when( + () => codePushClient.createPatch( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((_) async => promotedPatch); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + verifyNever(() => logger.confirm(any())); + verifyNever( + () => logger.warn( + any( + that: contains('already promoted to the stable track'), + ), + ), + ); + verifyNever( + () => logger.info( + any( + that: contains('already promoted to the stable track'), + ), + ), + ); + }, + ); }); + + test( + '''does not prompt when the existing patch is on a non-stable track''', + () async { + when( + () => codePushClient.createPatch( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer( + (_) async => const CreatePatchResponse( + id: patchId, + number: patchNumber, + channel: 'staging', + ), + ); + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(true); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + verifyNever(() => logger.confirm(any())); + }, + ); + + // Older servers that don't populate `channel` on the response leave + // the field null — append-after-promotion stays allowed per the + // design and the courtesy prompt simply doesn't fire. + test( + 'does not prompt when the server omits the channel field', + () async { + when(() => shorebirdEnv.canAcceptUserInput).thenReturn(true); + + await runWithOverrides( + () => codePushClientWrapper.publishPatch( + appId: appId, + releaseId: releaseId, + platform: releasePlatform, + track: track, + patchArtifactBundles: patchArtifactBundles, + metadata: {'foo': 'bar'}, + clientPatchId: 'sha-abcdef', + ), + ); + + verifyNever(() => logger.confirm(any())); + verifyNever( + () => logger.warn( + any(that: contains('already promoted to the stable track')), + ), + ); + }, + ); }); }); diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart index 3af45ea06..9e65ba121 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart @@ -44,6 +44,11 @@ class _FakeRelease extends Fake with EquatableMixin implements Release { List get props => [updatedAt]; } +/// RFC 4122 v4: 8-4-4-4-12 hex with the version nibble == 4. +final _uuidV4Pattern = RegExp( + r'^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$', +); + void main() { group(PatchCommand, () { const appId = 'test-app-id'; @@ -66,6 +71,7 @@ void main() { hasAssetChanges: false, hasNativeChanges: false, ); + const publishedPatch = CreatePatchResponse(id: 0, number: 1); final patchMetadata = CreatePatchMetadata.forTest(); final appMetadata = AppMetadata( @@ -125,6 +131,7 @@ void main() { late ArtifactManager artifactManager; late Cache cache; late CodePushClientWrapper codePushClientWrapper; + late Git git; late ShorebirdLogger logger; late Patcher patcher; late Progress progress; @@ -146,6 +153,7 @@ void main() { ), cacheRef.overrideWith(() => cache), codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), + gitRef.overrideWith(() => git), loggerRef.overrideWith(() => logger), shorebirdEnvRef.overrideWith(() => shorebirdEnv), shorebirdFlutterRef.overrideWith(() => shorebirdFlutter), @@ -180,6 +188,7 @@ void main() { artifactManager = MockArtifactManager(); cache = MockCache(); codePushClientWrapper = MockCodePushClientWrapper(); + git = MockGit(); logger = MockShorebirdLogger(); progress = MockProgress(); patcher = MockPatcher(); @@ -189,6 +198,7 @@ void main() { when(() => argResults['dry-run']).thenReturn(false); when(() => argResults['platforms']).thenReturn(['android']); + when(() => argResults['patch-id']).thenReturn(null); when(() => argResults['release-version']).thenReturn(releaseVersion); when( () => argResults[CommonArguments.minLinkPercentage.name], @@ -248,8 +258,9 @@ void main() { track: any(named: 'track'), artifacts: any(named: 'artifacts'), metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), ), - ).thenAnswer((_) async {}); + ).thenAnswer((_) async => publishedPatch); when( () => codePushClientWrapper.getReleaseArtifacts( @@ -382,6 +393,392 @@ void main() { }, ); }); + + group('when --patch-id is present but empty', () { + setUp(() { + when(() => argResults['patch-id']).thenReturn(''); + }); + + test('errors and exits with usage code', () async { + await expectLater( + runWithOverrides(command.run), + completion(equals(ExitCode.usage.code)), + ); + verify( + () => logger.err( + any( + that: allOf( + contains('--patch-id was provided but is empty'), + contains('unexpanded template variable'), + ), + ), + ), + ).called(1); + }); + }); + + group('warnIfDirtyTreeMatchesPatchId', () { + const headSha = '0123456789abcdef0123456789abcdef01234567'; + late Directory projectRoot; + + setUp(() { + projectRoot = Directory.systemTemp.createTempSync(); + when( + () => shorebirdEnv.getShorebirdProjectRoot(), + ).thenReturn(projectRoot); + }); + + test('does nothing when --patch-id is omitted', () async { + when(() => argResults['patch-id']).thenReturn(null); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever( + () => git.revParse( + revision: any(named: 'revision'), + directory: any(named: 'directory'), + ), + ); + verifyNever(() => logger.warn(any())); + }); + + test('does nothing when not running inside a project', () async { + when(() => argResults['patch-id']).thenReturn(headSha); + when(() => shorebirdEnv.getShorebirdProjectRoot()).thenReturn(null); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever( + () => git.revParse( + revision: any(named: 'revision'), + directory: any(named: 'directory'), + ), + ); + verifyNever(() => logger.warn(any())); + }); + + test('does nothing when not in a git repo', () async { + when(() => argResults['patch-id']).thenReturn(headSha); + when( + () => git.revParse( + revision: any(named: 'revision'), + directory: any(named: 'directory'), + ), + ).thenThrow(const ProcessException('git', ['rev-parse'])); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever(() => logger.warn(any())); + }); + + test( + '''does nothing when --patch-id does not match HEAD (avoids false positives on non-SHA ids)''', + () async { + when(() => argResults['patch-id']).thenReturn('hotfix-login'); + when( + () => git.revParse( + revision: 'HEAD', + directory: projectRoot.path, + ), + ).thenAnswer((_) async => headSha); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever( + () => git.status( + directory: any(named: 'directory'), + args: any(named: 'args'), + ), + ); + verifyNever(() => logger.warn(any())); + }, + ); + + test( + 'does nothing when --patch-id matches HEAD and tree is clean', + () async { + when(() => argResults['patch-id']).thenReturn(headSha); + when( + () => git.revParse( + revision: 'HEAD', + directory: projectRoot.path, + ), + ).thenAnswer((_) async => headSha); + when( + () => git.status( + directory: projectRoot.path, + args: ['--porcelain'], + ), + ).thenAnswer((_) async => ''); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever(() => logger.warn(any())); + }, + ); + + test('warns when --patch-id matches HEAD and tree is dirty', () async { + when(() => argResults['patch-id']).thenReturn(headSha); + when( + () => git.revParse( + revision: 'HEAD', + directory: projectRoot.path, + ), + ).thenAnswer((_) async => headSha); + when( + () => git.status( + directory: projectRoot.path, + args: ['--porcelain'], + ), + ).thenAnswer((_) async => ' M lib/main.dart'); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verify( + () => logger.warn( + any( + that: allOf( + contains('uncommitted changes'), + contains(headSha), + ), + ), + ), + ).called(1); + }); + + test( + 'silently no-ops if git status fails after rev-parse succeeded', + () async { + when(() => argResults['patch-id']).thenReturn(headSha); + when( + () => git.revParse( + revision: 'HEAD', + directory: projectRoot.path, + ), + ).thenAnswer((_) async => headSha); + when( + () => git.status( + directory: any(named: 'directory'), + args: any(named: 'args'), + ), + ).thenThrow(const ProcessException('git', ['status'])); + + await runWithOverrides(command.warnIfDirtyTreeMatchesPatchId); + + verifyNever(() => logger.warn(any())); + }, + ); + }); + + group('clientPatchId resolution', () { + // Captures the clientPatchId observed by uploadPatchArtifacts so we + // can assert on the resolved value across single-platform and + // multi-platform invocations. + String? capturedClientPatchId; + + setUp(() { + capturedClientPatchId = null; + when( + () => patcher.uploadPatchArtifacts( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + track: any(named: 'track'), + artifacts: any(named: 'artifacts'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((invocation) async { + capturedClientPatchId = + invocation.namedArguments[#clientPatchId] as String?; + return publishedPatch; + }); + }); + + test( + 'is null on a single-platform invocation with no --patch-id', + () async { + await runWithOverrides(() => command.createPatch(patcher)); + expect(capturedClientPatchId, isNull); + }, + ); + + test( + 'uses the explicit value when --patch-id is supplied', + () async { + when(() => argResults['patch-id']).thenReturn('sha-deadbeef'); + await runWithOverrides(() => command.createPatch(patcher)); + expect(capturedClientPatchId, equals('sha-deadbeef')); + }, + ); + + test( + '''generates a UUID for multi-platform invocations so platforms share a patch''', + () async { + when(() => argResults['platforms']).thenReturn(['ios', 'android']); + await runWithOverrides(() => command.createPatch(patcher)); + expect(capturedClientPatchId, isNotNull); + expect(capturedClientPatchId, matches(_uuidV4Pattern)); + }, + ); + + test( + '''uses the same generated id across every platform in one invocation''', + () async { + when(() => argResults['platforms']).thenReturn(['ios', 'android']); + + // Drive both platforms through the same command instance so we can + // confirm the lazily-resolved clientPatchId is shared. + final observed = []; + when( + () => patcher.uploadPatchArtifacts( + appId: any(named: 'appId'), + releaseId: any(named: 'releaseId'), + track: any(named: 'track'), + artifacts: any(named: 'artifacts'), + metadata: any(named: 'metadata'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((invocation) async { + observed.add( + invocation.namedArguments[#clientPatchId] as String?, + ); + return publishedPatch; + }); + + await runWithOverrides(() => command.createPatch(patcher)); + await runWithOverrides(() => command.createPatch(patcher)); + + expect(observed, hasLength(2)); + expect(observed.first, isNotNull); + expect(observed[0], equals(observed[1])); + }, + ); + + test( + 'prefers an explicit --patch-id over an auto-generated one', + () async { + when(() => argResults['platforms']).thenReturn(['ios', 'android']); + when(() => argResults['patch-id']).thenReturn('hotfix-login'); + await runWithOverrides(() => command.createPatch(patcher)); + expect(capturedClientPatchId, equals('hotfix-login')); + }, + ); + }); + + group('logUnifiedSuccess', () { + test('logs nothing when no platforms have published', () { + runWithOverrides(command.logUnifiedSuccess); + verifyNever(() => logger.success(any())); + }); + + test( + 'logs one line per patch with all platforms that landed on it', + () { + command.platformPatches + ..[ReleasePlatform.ios] = const CreatePatchResponse( + id: 7, + number: 3, + ) + ..[ReleasePlatform.android] = const CreatePatchResponse( + id: 7, + number: 3, + ); + + runWithOverrides(command.logUnifiedSuccess); + + verify( + () => logger.success( + any( + that: allOf( + contains('Published Patch 3'), + contains('Android'), + contains('iOS'), + ), + ), + ), + ).called(1); + }, + ); + + test( + 'still names the platform on a single-platform invocation', + () { + command.platformPatches[ReleasePlatform.android] = + const CreatePatchResponse(id: 7, number: 3); + + runWithOverrides(command.logUnifiedSuccess); + + verify( + () => logger.success( + any( + that: allOf( + contains('Published Patch 3'), + contains('Android'), + ), + ), + ), + ).called(1); + }, + ); + + test( + 'groups platforms separately when they end up on different numbers', + () { + // Degenerate but possible: server hadn't deployed PR 2, or + // releases differed across platforms. Each patch number gets + // its own line so the user sees what actually happened. + command.platformPatches + ..[ReleasePlatform.ios] = const CreatePatchResponse( + id: 7, + number: 3, + ) + ..[ReleasePlatform.android] = const CreatePatchResponse( + id: 8, + number: 4, + ); + + runWithOverrides(command.logUnifiedSuccess); + + verify( + () => logger.success( + any( + that: allOf( + contains('Published Patch 3'), + contains('iOS'), + ), + ), + ), + ).called(1); + verify( + () => logger.success( + any( + that: allOf( + contains('Published Patch 4'), + contains('Android'), + ), + ), + ), + ).called(1); + }, + ); + }); + + group('aggregated success message', () { + test( + 'no success message is logged when run() exits early', + () async { + when(() => argResults['platforms']).thenReturn(const []); + + await expectLater( + runWithOverrides(command.run), + completion(equals(ExitCode.usage.code)), + ); + verifyNever( + () => logger.success(any(that: contains('Published Patch'))), + ); + }, + ); + }); }); group('createPatch', () { diff --git a/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart index 6326fd21b..758651d15 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart @@ -206,6 +206,8 @@ void main() { const metadata = {}; const artifacts = {}; const track = DeploymentTrack.stable; + const clientPatchId = 'abc123'; + const publishedPatch = CreatePatchResponse(id: 7, number: 3); final codePushClientWrapper = MockCodePushClientWrapper(); when( () => codePushClientWrapper.publishPatch( @@ -215,18 +217,18 @@ void main() { platform: any(named: 'platform'), track: any(named: 'track'), patchArtifactBundles: any(named: 'patchArtifactBundles'), + clientPatchId: any(named: 'clientPatchId'), + ), + ).thenAnswer((_) async => publishedPatch); + final returnedPatch = await runScoped( + () => patcher.uploadPatchArtifacts( + appId: appId, + releaseId: releaseId, + metadata: metadata, + artifacts: artifacts, + track: track, + clientPatchId: clientPatchId, ), - ).thenAnswer((_) async {}); - await runScoped( - () async { - await patcher.uploadPatchArtifacts( - appId: appId, - releaseId: releaseId, - metadata: metadata, - artifacts: artifacts, - track: track, - ); - }, values: { codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), }, @@ -239,8 +241,10 @@ void main() { platform: ReleaseType.android.releasePlatform, track: track, patchArtifactBundles: artifacts, + clientPatchId: clientPatchId, ), ).called(1); + expect(returnedPatch, equals(publishedPatch)); }); }); diff --git a/packages/shorebird_code_push_client/lib/src/code_push_client.dart b/packages/shorebird_code_push_client/lib/src/code_push_client.dart index 58484feb1..0e91d83ad 100644 --- a/packages/shorebird_code_push_client/lib/src/code_push_client.dart +++ b/packages/shorebird_code_push_client/lib/src/code_push_client.dart @@ -273,14 +273,27 @@ class CodePushClient { } /// Create a new patch for the given [releaseId]. - Future createPatch({ + /// + /// When [clientPatchId] is supplied and a patch on this release already + /// has that id, the server returns the existing patch — letting two + /// invocations across platforms share one patch number. The returned + /// [CreatePatchResponse] echoes [clientPatchId] back so callers can + /// verify the correlation. + Future createPatch({ required String appId, required int releaseId, required Json metadata, + String? clientPatchId, }) async { + // Coerce empty to null. A caller that passes an unexpanded template + // variable or empty flag would otherwise land on the idempotent path + // keyed on `''` and inherit a stranger's patch. + final normalizedClientPatchId = + (clientPatchId == null || clientPatchId.isEmpty) ? null : clientPatchId; final request = CreatePatchRequest( releaseId: releaseId, metadata: metadata, + clientPatchId: normalizedClientPatchId, ); final response = await _httpClient.post( Uri.parse('$_v1/apps/$appId/patches'), @@ -292,7 +305,7 @@ class CodePushClient { } final body = json.decode(response.body) as Map; - return Patch.fromJson(body); + return CreatePatchResponse.fromJson(body); } /// Create a new release for the app with the provided [appId]. diff --git a/packages/shorebird_code_push_client/test/src/code_push_client_test.dart b/packages/shorebird_code_push_client/test/src/code_push_client_test.dart index fec38a67f..2d7c2792d 100644 --- a/packages/shorebird_code_push_client/test/src/code_push_client_test.dart +++ b/packages/shorebird_code_push_client/test/src/code_push_client_test.dart @@ -1068,7 +1068,9 @@ void main() { (_) async => http.StreamedResponse( Stream.value( utf8.encode( - json.encode(const Patch(id: patchId, number: patchNumber)), + json.encode( + const CreatePatchResponse(id: patchId, number: patchNumber), + ), ), ), HttpStatus.ok, @@ -1083,9 +1085,10 @@ void main() { ), completion( equals( - isA() + isA() .having((c) => c.id, 'id', patchId) - .having((c) => c.number, 'number', patchNumber), + .having((c) => c.number, 'number', patchNumber) + .having((c) => c.clientPatchId, 'clientPatchId', isNull), ), ), ); @@ -1099,6 +1102,71 @@ void main() { codePushClient.hostedUri.replace(path: '/api/v1/apps/$appId/patches'), ); }); + + test('sends client_patch_id in body when supplied', () async { + const sha = 'abc1234'; + when(() => httpClient.send(any())).thenAnswer( + (_) async => http.StreamedResponse( + Stream.value( + utf8.encode( + json.encode( + const CreatePatchResponse( + id: 0, + number: 1, + clientPatchId: sha, + ), + ), + ), + ), + HttpStatus.ok, + ), + ); + + final response = await codePushClient.createPatch( + appId: appId, + releaseId: releaseId, + metadata: const {'foo': 'bar'}, + clientPatchId: sha, + ); + expect(response.clientPatchId, equals(sha)); + + final request = + verify(() => httpClient.send(captureAny())).captured.single + as http.Request; + final body = json.decode(request.body) as Map; + expect(body['client_patch_id'], equals(sha)); + }); + + test( + 'normalizes empty clientPatchId to null before sending', + () async { + when(() => httpClient.send(any())).thenAnswer( + (_) async => http.StreamedResponse( + Stream.value( + utf8.encode( + json.encode( + const CreatePatchResponse(id: 0, number: 1), + ), + ), + ), + HttpStatus.ok, + ), + ); + + await codePushClient.createPatch( + appId: appId, + releaseId: releaseId, + metadata: const {'foo': 'bar'}, + clientPatchId: '', + ); + + final request = + verify(() => httpClient.send(captureAny())).captured.single + as http.Request; + final body = json.decode(request.body) as Map; + expect(body['client_patch_id'], isNull); + }, + ); }); group('createRelease', () { diff --git a/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_request.dart b/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_request.dart index c36a3f235..b8bbc7255 100644 --- a/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_request.dart +++ b/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_request.dart @@ -10,6 +10,7 @@ class CreatePatchRequest { const CreatePatchRequest({ required this.releaseId, required this.metadata, + this.clientPatchId, }); /// Converts a `Map` to a [CreatePatchRequest]. @@ -22,6 +23,7 @@ class CreatePatchRequest { metadata: (json['metadata'] as Map).map( MapEntry.new, ), + clientPatchId: json['client_patch_id'] as String?, ), ); } @@ -42,11 +44,20 @@ class CreatePatchRequest { /// create the patch and the environment it was run in. final Map metadata; + /// Optional client-supplied correlation key used to make patch + /// creation idempotent across invocations. When two requests on + /// the same release supply the same value, the server returns the + /// existing patch instead of creating a new one — letting + /// cross-platform builds share one patch number. Most commonly a + /// git SHA, but any stable token works. + final String? clientPatchId; + /// Converts a [CreatePatchRequest] to a `Map`. Map toJson() { return { 'release_id': releaseId, 'metadata': metadata, + 'client_patch_id': clientPatchId, }; } @@ -54,6 +65,7 @@ class CreatePatchRequest { int get hashCode => Object.hashAll([ releaseId, mapHash(metadata), + clientPatchId, ]); @override @@ -61,6 +73,7 @@ class CreatePatchRequest { if (identical(this, other)) return true; return other is CreatePatchRequest && releaseId == other.releaseId && - mapsEqual(metadata, other.metadata); + mapsEqual(metadata, other.metadata) && + clientPatchId == other.clientPatchId; } } diff --git a/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_response.dart b/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_response.dart index 71215e54e..820c19f92 100644 --- a/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_response.dart +++ b/packages/shorebird_code_push_protocol/lib/src/messages/create_patch/create_patch_response.dart @@ -20,6 +20,8 @@ class CreatePatchResponse { const CreatePatchResponse({ required this.id, required this.number, + this.clientPatchId, + this.channel, }); /// Converts a `Map` to a [CreatePatchResponse]. @@ -30,6 +32,8 @@ class CreatePatchResponse { () => CreatePatchResponse( id: json['id'] as int, number: json['number'] as int, + clientPatchId: json['client_patch_id'] as String?, + channel: json['channel'] as String?, ), ); } @@ -49,11 +53,27 @@ class CreatePatchResponse { /// The patch number. A larger number equates to a newer patch. final int number; + /// The client-supplied correlation key, if one was provided on the + /// request. Echoed back so callers can verify whether an + /// idempotent re-use occurred. See `CreatePatchRequest.client_patch_id`. + final String? clientPatchId; + + /// The channel this patch is currently promoted to, if any. Null on a + /// freshly-created patch (the common path) and on idempotent hits where + /// the existing patch has not been promoted. When non-null on an + /// idempotent hit it tells the client that uploading further artifacts + /// will go live to that channel's users immediately — letting the + /// client surface the append-after-promotion case without a second + /// round-trip to list patches. + final String? channel; + /// Converts a [CreatePatchResponse] to a `Map`. Map toJson() { return { 'id': id, 'number': number, + 'client_patch_id': clientPatchId, + 'channel': channel, }; } @@ -61,6 +81,8 @@ class CreatePatchResponse { int get hashCode => Object.hashAll([ id, number, + clientPatchId, + channel, ]); @override @@ -68,6 +90,8 @@ class CreatePatchResponse { if (identical(this, other)) return true; return other is CreatePatchResponse && id == other.id && - number == other.number; + number == other.number && + clientPatchId == other.clientPatchId && + channel == other.channel; } } diff --git a/packages/shorebird_code_push_protocol/lib/src/models/release_patch.dart b/packages/shorebird_code_push_protocol/lib/src/models/release_patch.dart index 982e7b3c7..7e54918bd 100644 --- a/packages/shorebird_code_push_protocol/lib/src/models/release_patch.dart +++ b/packages/shorebird_code_push_protocol/lib/src/models/release_patch.dart @@ -15,6 +15,7 @@ class ReleasePatch { required this.isRolledBack, this.channel, this.notes, + this.clientPatchId, }); /// Converts a `Map` to a [ReleasePatch]. @@ -33,6 +34,7 @@ class ReleasePatch { .toList(), isRolledBack: json['is_rolled_back'] as bool, notes: json['notes'] as String?, + clientPatchId: json['client_patch_id'] as String?, ), ); } @@ -64,6 +66,11 @@ class ReleasePatch { /// Freeform notes associated with the patch, if any. final String? notes; + /// The client-supplied correlation key, if one was provided when the + /// patch was created. See `CreatePatchRequest.client_patch_id`. Null + /// for patches predating unified patch numbering. + final String? clientPatchId; + /// Converts a [ReleasePatch] to a `Map`. Map toJson() { return { @@ -73,6 +80,7 @@ class ReleasePatch { 'artifacts': artifacts.map((e) => e.toJson()).toList(), 'is_rolled_back': isRolledBack, 'notes': notes, + 'client_patch_id': clientPatchId, }; } @@ -84,6 +92,7 @@ class ReleasePatch { listHash(artifacts), isRolledBack, notes, + clientPatchId, ]); @override @@ -95,6 +104,7 @@ class ReleasePatch { channel == other.channel && listsEqual(artifacts, other.artifacts) && isRolledBack == other.isRolledBack && - notes == other.notes; + notes == other.notes && + clientPatchId == other.clientPatchId; } } diff --git a/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_request_test.dart b/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_request_test.dart index cbb735b35..ccac5e6ac 100644 --- a/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_request_test.dart +++ b/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_request_test.dart @@ -14,15 +14,57 @@ void main() { ); }); - test('can be (de)serialized without metadata', () { + test('round-trips a clientPatchId', () { const request = CreatePatchRequest( releaseId: 1234, metadata: {'foo': 'bar'}, + clientPatchId: 'abc1234', ); + final json = request.toJson(); + expect(json['client_patch_id'], equals('abc1234')); expect( - CreatePatchRequest.fromJson(request.toJson()).toJson(), - equals(request.toJson()), + CreatePatchRequest.fromJson(json).clientPatchId, + equals('abc1234'), + ); + }); + + test('parses json without client_patch_id', () { + final request = CreatePatchRequest.fromJson(const { + 'release_id': 1234, + 'metadata': {'foo': 'bar'}, + }); + expect(request.clientPatchId, isNull); + }); + + test('toJson always includes client_patch_id (null when unset)', () { + const request = CreatePatchRequest( + releaseId: 1234, + metadata: {'foo': 'bar'}, ); + final json = request.toJson(); + expect(json.containsKey('client_patch_id'), isTrue); + expect(json['client_patch_id'], isNull); + }); + + test('clientPatchId participates in equality', () { + final a = CreatePatchRequest.fromJson(const { + 'release_id': 1234, + 'metadata': {'foo': 'bar'}, + 'client_patch_id': 'abc', + }); + final b = CreatePatchRequest.fromJson(const { + 'release_id': 1234, + 'metadata': {'foo': 'bar'}, + 'client_patch_id': 'abc', + }); + final c = CreatePatchRequest.fromJson(const { + 'release_id': 1234, + 'metadata': {'foo': 'bar'}, + 'client_patch_id': 'xyz', + }); + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + expect(a, isNot(equals(c))); }); }); } diff --git a/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_response_test.dart b/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_response_test.dart new file mode 100644 index 000000000..4d9227216 --- /dev/null +++ b/packages/shorebird_code_push_protocol/test/src/messages/create_patch/create_patch_response_test.dart @@ -0,0 +1,91 @@ +import 'package:shorebird_code_push_protocol/shorebird_code_push_protocol.dart'; +import 'package:test/test.dart'; + +void main() { + group(CreatePatchResponse, () { + test('round-trips a clientPatchId', () { + const response = CreatePatchResponse( + id: 1, + number: 2, + clientPatchId: 'abc1234', + ); + final json = response.toJson(); + expect(json['client_patch_id'], equals('abc1234')); + expect( + CreatePatchResponse.fromJson(json).clientPatchId, + equals('abc1234'), + ); + }); + + test('parses json without client_patch_id', () { + final response = CreatePatchResponse.fromJson(const { + 'id': 1, + 'number': 2, + }); + expect(response.clientPatchId, isNull); + }); + + test('toJson always includes client_patch_id (null when unset)', () { + const response = CreatePatchResponse(id: 1, number: 2); + final json = response.toJson(); + expect(json.containsKey('client_patch_id'), isTrue); + expect(json['client_patch_id'], isNull); + }); + + test('clientPatchId participates in equality', () { + final a = CreatePatchResponse.fromJson(const { + 'id': 1, + 'number': 2, + 'client_patch_id': 'abc', + }); + final b = CreatePatchResponse.fromJson(const { + 'id': 1, + 'number': 2, + 'client_patch_id': 'abc', + }); + final c = CreatePatchResponse.fromJson(const { + 'id': 1, + 'number': 2, + 'client_patch_id': 'xyz', + }); + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + expect(a, isNot(equals(c))); + }); + + test('round-trips channel', () { + const response = CreatePatchResponse( + id: 1, + number: 2, + channel: 'stable', + ); + final json = response.toJson(); + expect(json['channel'], equals('stable')); + expect(CreatePatchResponse.fromJson(json).channel, equals('stable')); + }); + + test('parses json without channel', () { + final response = CreatePatchResponse.fromJson(const { + 'id': 1, + 'number': 2, + }); + expect(response.channel, isNull); + }); + + test('toJson always includes channel (null when unset)', () { + const response = CreatePatchResponse(id: 1, number: 2); + final json = response.toJson(); + expect(json.containsKey('channel'), isTrue); + expect(json['channel'], isNull); + }); + + test('channel participates in equality', () { + const a = CreatePatchResponse(id: 1, number: 2, channel: 'stable'); + const b = CreatePatchResponse(id: 1, number: 2, channel: 'stable'); + const c = CreatePatchResponse(id: 1, number: 2, channel: 'staging'); + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + expect(a, isNot(equals(c))); + }); + }); +} diff --git a/packages/shorebird_code_push_protocol/test/src/models/release_patch_test.dart b/packages/shorebird_code_push_protocol/test/src/models/release_patch_test.dart index b04d3b1fd..b92ec6e61 100644 --- a/packages/shorebird_code_push_protocol/test/src/models/release_patch_test.dart +++ b/packages/shorebird_code_push_protocol/test/src/models/release_patch_test.dart @@ -25,5 +25,73 @@ void main() { ), ); }); + + test('round-trips a clientPatchId', () { + const patch = ReleasePatch( + id: 0, + number: 1, + channel: 'channel', + isRolledBack: false, + artifacts: [], + clientPatchId: 'abc1234', + ); + final json = patch.toJson(); + expect(json['client_patch_id'], equals('abc1234')); + expect(ReleasePatch.fromJson(json).clientPatchId, equals('abc1234')); + }); + + test('parses json without client_patch_id', () { + final patch = ReleasePatch.fromJson(const { + 'id': 0, + 'number': 1, + 'channel': 'channel', + 'is_rolled_back': false, + 'artifacts': >[], + }); + expect(patch.clientPatchId, isNull); + }); + + test('toJson always includes client_patch_id (null when unset)', () { + const patch = ReleasePatch( + id: 0, + number: 1, + channel: 'channel', + isRolledBack: false, + artifacts: [], + ); + final json = patch.toJson(); + expect(json.containsKey('client_patch_id'), isTrue); + expect(json['client_patch_id'], isNull); + }); + + test('clientPatchId participates in equality', () { + final a = ReleasePatch.fromJson(const { + 'id': 0, + 'number': 1, + 'channel': 'channel', + 'is_rolled_back': false, + 'artifacts': >[], + 'client_patch_id': 'abc', + }); + final b = ReleasePatch.fromJson(const { + 'id': 0, + 'number': 1, + 'channel': 'channel', + 'is_rolled_back': false, + 'artifacts': >[], + 'client_patch_id': 'abc', + }); + final c = ReleasePatch.fromJson(const { + 'id': 0, + 'number': 1, + 'channel': 'channel', + 'is_rolled_back': false, + 'artifacts': >[], + 'client_patch_id': 'xyz', + }); + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + expect(a, isNot(equals(c))); + }); }); }