Skip to content

Commit

Permalink
feat: show download progress when downloading patch artifacts (#2563)
Browse files Browse the repository at this point in the history
  • Loading branch information
bryanoltman authored Oct 21, 2024
1 parent 41a624f commit 4cb17a5
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 12 deletions.
18 changes: 16 additions & 2 deletions packages/shorebird_cli/lib/src/artifact_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ final artifactManagerRef = create(ArtifactManager.new);
/// The [ArtifactManager] instance available in the current zone.
ArtifactManager get artifactManager => read(artifactManagerRef);

/// A callback that reports progress as a double between 0 and 1.
typedef ProgressCallback = void Function(double progress);

/// Manages artifacts for the Shorebird CLI.
class ArtifactManager {
/// Generates a binary diff between two files and returns the path to the
Expand Down Expand Up @@ -53,10 +56,13 @@ class ArtifactManager {
}

/// Downloads the file at the given [uri] to a temporary directory and returns
/// the downloaded [File].
/// the downloaded [File]. If [onProgress] is provided, it will be called with
/// the download progress as a double between 0 and 1 as the response is
/// streamed in.
Future<File> downloadFile(
Uri uri, {
String? outputPath,
ProgressCallback? onProgress,
}) async {
final request = http.Request('GET', uri);
final response = await httpClient.send(request);
Expand All @@ -81,7 +87,15 @@ class ArtifactManager {

final ioSink = outFile.openWrite();
try {
await ioSink.addStream(response.stream);
var downloadedBytes = 0;
final totalBytes = response.contentLength;
await for (final chunk in response.stream) {
ioSink.add(chunk);
downloadedBytes += chunk.length;
if (onProgress != null && totalBytes != null) {
onProgress(downloadedBytes / totalBytes);
}
}
} finally {
await ioSink.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,19 @@ Looked in:
final downloadReleaseArtifactProgress = logger.progress(
'Downloading release artifacts',
);
final numArtifacts = releaseArtifacts.length;

for (final releaseArtifact in releaseArtifacts.entries) {
for (final (i, releaseArtifact) in releaseArtifacts.entries.indexed) {
final baseMessage = 'Downloading release artifact ${i + 1}/$numArtifacts';
try {
downloadReleaseArtifactProgress.update(baseMessage);
final releaseArtifactFile = await artifactManager.downloadFile(
Uri.parse(releaseArtifact.value.url),
onProgress: (progress) {
downloadReleaseArtifactProgress.update(
'$baseMessage (${(progress * 100).toStringAsFixed(0)}%)',
);
},
);
releaseArtifactPaths[releaseArtifact.key] = releaseArtifactFile.path;
} catch (error) {
Expand Down
14 changes: 10 additions & 4 deletions packages/shorebird_cli/lib/src/commands/patch/patch_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,18 @@ ${summary.join('\n')}
required ReleaseArtifact releaseArtifact,
required Patcher patcher,
}) async {
final downloadProgress =
logger.progress('Downloading ${patcher.primaryReleaseArtifactArch}');
final downloadMessage = 'Downloading ${patcher.primaryReleaseArtifactArch}';
final downloadProgress = logger.progress(downloadMessage);
final File artifactFile;
try {
artifactFile =
await artifactManager.downloadFile(Uri.parse(releaseArtifact.url));
artifactFile = await artifactManager.downloadFile(
Uri.parse(releaseArtifact.url),
onProgress: (progress) {
downloadProgress.update(
'$downloadMessage (${(progress * 100).toStringAsFixed(0)}%)',
);
},
);
} catch (e) {
downloadProgress.fail(e.toString());
throw ProcessExit(ExitCode.software.code);
Expand Down
58 changes: 58 additions & 0 deletions packages/shorebird_cli/test/src/artifact_manager_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,64 @@ void main() {

expect(result.path, equals(outFile.path));
});

group('with progress update', () {
group('when response contentLength is null', () {
setUp(() {
when(() => httpClient.send(any())).thenAnswer(
(_) async => http.StreamedResponse(
Stream.fromIterable([
[1],
[2],
[3],
]),
HttpStatus.ok,
),
);
});

test('does not call onProgress callback', () async {
await runWithOverrides(
() => artifactManager.downloadFile(
Uri.parse('https://example.com'),
onProgress: (progress) {
fail(
'''onProgress should not be called when the response does not have a contentLength''',
);
},
),
);
});
});

group('when response contentLength is not null', () {
setUp(() {
when(() => httpClient.send(any())).thenAnswer(
(_) async => http.StreamedResponse(
Stream.fromIterable([
[1],
[2],
[3],
]),
HttpStatus.ok,
contentLength: 3,
),
);
});

test('calls onProgress with correct percentage', () async {
final progressUpdates = <double>[];
await runWithOverrides(
() => artifactManager.downloadFile(
Uri.parse('https://example.com'),
onProgress: progressUpdates.add,
),
);

expect(progressUpdates, equals([1 / 3, 2 / 3, 3 / 3]));
});
});
});
});

group('extractZip', () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,14 +439,21 @@ Looked in:
Arch.x86_64: releaseArtifact,
},
);
when(() => artifactManager.downloadFile(any()))
.thenAnswer((_) async => File(''));
when(
() => artifactManager.downloadFile(
any(),
onProgress: any(named: 'onProgress'),
),
).thenAnswer((_) async => File(''));
});

group('when release artifact fails to download', () {
setUp(() {
when(
() => artifactManager.downloadFile(any()),
() => artifactManager.downloadFile(
any(),
onProgress: any(named: 'onProgress'),
),
).thenThrow(Exception('error'));
});

Expand Down Expand Up @@ -544,6 +551,31 @@ Looked in:
}
});

test('updates progress with download percentage', () async {
await runWithOverrides(
() => patcher.createPatchArtifacts(
appId: 'appId',
releaseId: 0,
releaseArtifact: File('release.aab'),
),
);

final capturedOnProgress = verify(
() => artifactManager.downloadFile(
any(),
onProgress: captureAny(named: 'onProgress'),
),
).captured.first as ProgressCallback;

capturedOnProgress(0.5);

verify(
() => progress.update(
'Downloading release artifact 1/3 (50%)',
),
).called(1);
});

group('when a private key is provided', () {
setUp(() {
final privateKey = File(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ void main() {
when(aotTools.isLinkDebugInfoSupported).thenAnswer((_) async => true);

when(
() => artifactManager.downloadFile(any()),
() => artifactManager.downloadFile(
any(),
onProgress: any(named: 'onProgress'),
),
).thenAnswer((_) async => File(''));

when(() => cache.updateAll()).thenAnswer((_) async => {});
Expand Down Expand Up @@ -888,7 +891,12 @@ Please re-run the release command for this version or create a new release.''',
final error = Exception('Failed to download primary release artifact.');

setUp(() {
when(() => artifactManager.downloadFile(any())).thenThrow(error);
when(
() => artifactManager.downloadFile(
any(),
onProgress: any(named: 'onProgress'),
),
).thenThrow(error);
});

test('logs error and exits with code 70', () async {
Expand Down Expand Up @@ -967,5 +975,32 @@ Please re-run the release command for this version or create a new release.''',
).called(1);
});
});

group('downloadPrimaryReleaseArtifact', () {
setUp(() {
final releaseArtifact = MockReleaseArtifact();
when(() => releaseArtifact.url).thenReturn('http://example.com');
});

test('updates progress with download percentage', () async {
await runWithOverrides(
() => command.downloadPrimaryReleaseArtifact(
releaseArtifact: releaseArtifact,
patcher: patcher,
),
);

final capturedOnProgress = verify(
() => artifactManager.downloadFile(
Uri.parse(releaseArtifact.url),
onProgress: captureAny(named: 'onProgress'),
),
).captured.single as ProgressCallback;

capturedOnProgress(0.5);

verify(() => progress.update('Downloading aab (50%)')).called(1);
});
});
});
}

0 comments on commit 4cb17a5

Please sign in to comment.