From 0f3182c4f34ca5096b6dd747edf6ade0d1ec1c9e Mon Sep 17 00:00:00 2001 From: Vinzent Date: Wed, 31 Jan 2024 05:28:46 +0100 Subject: [PATCH] fix: Use per client fetch instance (#818) * fix: use per client fetch instance * test: adapt tests to missing global fetch * test: remove mocktail usage --- packages/storage_client/lib/src/fetch.dart | 2 - .../lib/src/storage_bucket_api.dart | 3 + .../lib/src/storage_client.dart | 8 +- .../lib/src/storage_file_api.dart | 30 +-- packages/storage_client/pubspec.yaml | 2 +- packages/storage_client/test/basic_test.dart | 196 +++--------------- packages/storage_client/test/client_test.dart | 5 - .../test/custom_http_client.dart | 23 +- 8 files changed, 82 insertions(+), 187 deletions(-) diff --git a/packages/storage_client/lib/src/fetch.dart b/packages/storage_client/lib/src/fetch.dart index 2e95e702..a39c136e 100644 --- a/packages/storage_client/lib/src/fetch.dart +++ b/packages/storage_client/lib/src/fetch.dart @@ -11,8 +11,6 @@ import 'package:storage_client/src/types.dart'; import 'file_io.dart' if (dart.library.js) './file_stub.dart'; -Fetch storageFetch = Fetch(); - class Fetch { final Client? httpClient; diff --git a/packages/storage_client/lib/src/storage_bucket_api.dart b/packages/storage_client/lib/src/storage_bucket_api.dart index bea6eb41..9345a2d8 100644 --- a/packages/storage_client/lib/src/storage_bucket_api.dart +++ b/packages/storage_client/lib/src/storage_bucket_api.dart @@ -1,10 +1,13 @@ import 'package:http/http.dart'; +import 'package:meta/meta.dart'; import 'package:storage_client/src/fetch.dart'; import 'package:storage_client/src/types.dart'; class StorageBucketApi { final String url; final Map headers; + @internal + late Fetch storageFetch; StorageBucketApi(this.url, this.headers, {Client? httpClient}) { storageFetch = Fetch(httpClient); diff --git a/packages/storage_client/lib/src/storage_client.dart b/packages/storage_client/lib/src/storage_client.dart index fabf97e3..e9edf4bd 100644 --- a/packages/storage_client/lib/src/storage_client.dart +++ b/packages/storage_client/lib/src/storage_client.dart @@ -48,7 +48,13 @@ class SupabaseStorageClient extends StorageBucketApi { /// /// [id] The bucket id to operate on. StorageFileApi from(String id) { - return StorageFileApi(url, headers, id, _defaultRetryAttempts); + return StorageFileApi( + url, + headers, + id, + _defaultRetryAttempts, + storageFetch, + ); } void setAuth(String jwt) { diff --git a/packages/storage_client/lib/src/storage_file_api.dart b/packages/storage_client/lib/src/storage_file_api.dart index ffff74b5..39ae6433 100644 --- a/packages/storage_client/lib/src/storage_file_api.dart +++ b/packages/storage_client/lib/src/storage_file_api.dart @@ -10,12 +10,14 @@ class StorageFileApi { final Map headers; final String? bucketId; final int _retryAttempts; + final Fetch _storageFetch; const StorageFileApi( this.url, this.headers, this.bucketId, this._retryAttempts, + this._storageFetch, ); String _getFinalPath(String path) { @@ -51,7 +53,7 @@ class StorageFileApi { assert(retryAttempts == null || retryAttempts >= 0, 'retryAttempts has to be greater or equal to 0'); final finalPath = _getFinalPath(path); - final response = await storageFetch.postFile( + final response = await _storageFetch.postFile( '$url/object/$finalPath', file, fileOptions, @@ -86,7 +88,7 @@ class StorageFileApi { assert(retryAttempts == null || retryAttempts >= 0, 'retryAttempts has to be greater or equal to 0'); final finalPath = _getFinalPath(path); - final response = await storageFetch.postBinaryFile( + final response = await _storageFetch.postBinaryFile( '$url/object/$finalPath', data, fileOptions, @@ -121,7 +123,7 @@ class StorageFileApi { var url = Uri.parse('${this.url}/object/upload/sign/$finalPath'); url = url.replace(queryParameters: {'token': token}); - await storageFetch.putFile( + await _storageFetch.putFile( url.toString(), file, fileOptions, @@ -155,7 +157,7 @@ class StorageFileApi { var url = Uri.parse('${this.url}/object/upload/sign/$path0'); url = url.replace(queryParameters: {'token': token}); - await storageFetch.putBinaryFile( + await _storageFetch.putBinaryFile( url.toString(), data, fileOptions, @@ -175,7 +177,7 @@ class StorageFileApi { Future createSignedUploadUrl(String path) async { final finalPath = _getFinalPath(path); - final data = await storageFetch.post( + final data = await _storageFetch.post( '$url/object/upload/sign/$finalPath', {}, options: FetchOptions(headers: headers), @@ -220,7 +222,7 @@ class StorageFileApi { assert(retryAttempts == null || retryAttempts >= 0, 'retryAttempts has to be greater or equal to 0'); final finalPath = _getFinalPath(path); - final response = await storageFetch.putFile( + final response = await _storageFetch.putFile( '$url/object/$finalPath', file, fileOptions, @@ -256,7 +258,7 @@ class StorageFileApi { assert(retryAttempts == null || retryAttempts >= 0, 'retryAttempts has to be greater or equal to 0'); final finalPath = _getFinalPath(path); - final response = await storageFetch.putBinaryFile( + final response = await _storageFetch.putBinaryFile( '$url/object/$finalPath', data, fileOptions, @@ -276,7 +278,7 @@ class StorageFileApi { /// `folder/image-new.png`. Future move(String fromPath, String toPath) async { final options = FetchOptions(headers: headers); - final response = await storageFetch.post( + final response = await _storageFetch.post( '$url/object/move', { 'bucketId': bucketId, @@ -297,7 +299,7 @@ class StorageFileApi { /// `folder/image-copy.png`. Future copy(String fromPath, String toPath) async { final options = FetchOptions(headers: headers); - final response = await storageFetch.post( + final response = await _storageFetch.post( '$url/object/copy', { 'bucketId': bucketId, @@ -326,7 +328,7 @@ class StorageFileApi { }) async { final finalPath = _getFinalPath(path); final options = FetchOptions(headers: headers); - final response = await storageFetch.post( + final response = await _storageFetch.post( '$url/object/sign/$finalPath', { 'expiresIn': expiresIn, @@ -354,7 +356,7 @@ class StorageFileApi { int expiresIn, ) async { final options = FetchOptions(headers: headers); - final response = await storageFetch.post( + final response = await _storageFetch.post( '$url/object/sign/$bucketId', { 'expiresIn': expiresIn, @@ -391,7 +393,7 @@ class StorageFileApi { fetchUrl = fetchUrl.replace(queryParameters: queryParams); final response = - await storageFetch.get(fetchUrl.toString(), options: options); + await _storageFetch.get(fetchUrl.toString(), options: options); return response as Uint8List; } @@ -424,7 +426,7 @@ class StorageFileApi { /// name. For example: `remove(['folder/image.png'])`. Future> remove(List paths) async { final options = FetchOptions(headers: headers); - final response = await storageFetch.delete( + final response = await _storageFetch.delete( '$url/object/$bucketId', {'prefixes': paths}, options: options, @@ -451,7 +453,7 @@ class StorageFileApi { ...searchOptions.toMap(), }; final options = FetchOptions(headers: headers); - final response = await storageFetch.post( + final response = await _storageFetch.post( '$url/object/list/$bucketId', body, options: options, diff --git a/packages/storage_client/pubspec.yaml b/packages/storage_client/pubspec.yaml index d14e2f5c..aac86118 100644 --- a/packages/storage_client/pubspec.yaml +++ b/packages/storage_client/pubspec.yaml @@ -13,9 +13,9 @@ dependencies: http_parser: ^4.0.1 mime: ^1.0.2 retry: ^3.1.0 + meta: ^1.7.0 dev_dependencies: - mocktail: ^0.3.0 test: ^1.21.4 lints: ^2.1.1 path: ^1.8.2 diff --git a/packages/storage_client/test/basic_test.dart b/packages/storage_client/test/basic_test.dart index 7652afb3..931dbe7c 100644 --- a/packages/storage_client/test/basic_test.dart +++ b/packages/storage_client/test/basic_test.dart @@ -1,9 +1,6 @@ import 'dart:io'; import 'dart:typed_data'; -import 'package:mocktail/mocktail.dart'; -import 'package:storage_client/src/fetch.dart'; -import 'package:storage_client/src/types.dart'; import 'package:storage_client/storage_client.dart'; import 'package:test/test.dart'; @@ -12,12 +9,6 @@ import 'custom_http_client.dart'; const String supabaseUrl = 'SUPABASE_TEST_URL'; const String supabaseKey = 'SUPABASE_TEST_KEY'; -class MockFetch extends Mock implements Fetch {} - -FileOptions get mockFileOptions => any(); - -FetchOptions get mockFetchOptions => any(named: 'options'); - Map get testBucketJson => { 'id': 'test_bucket', 'name': 'test_bucket', @@ -39,25 +30,22 @@ Map get testFileObjectJson => { }; String get bucketUrl => '$supabaseUrl/storage/v1/bucket'; - String get objectUrl => '$supabaseUrl/storage/v1/object'; void main() { late SupabaseStorageClient client; + late CustomHttpClient customHttpClient = CustomHttpClient(); group('Client with default http client', () { setUp(() { // init SupabaseClient with test url & test key - client = SupabaseStorageClient('$supabaseUrl/storage/v1', { - 'Authorization': 'Bearer $supabaseKey', - }); - - // Use mocked version for `storageFetch`, to prevent actual http calls. - storageFetch = MockFetch(); - - // Register default mock values (used by mocktail) - registerFallbackValue(const FileOptions()); - registerFallbackValue(const FetchOptions()); + client = SupabaseStorageClient( + '$supabaseUrl/storage/v1', + { + 'Authorization': 'Bearer $supabaseKey', + }, + httpClient: customHttpClient, + ); }); tearDown(() { @@ -66,10 +54,7 @@ void main() { }); test('should list buckets', () async { - when(() => storageFetch.get(bucketUrl, options: mockFetchOptions)) - .thenAnswer( - (_) => Future.value([testBucketJson, testBucketJson]), - ); + customHttpClient.response = [testBucketJson, testBucketJson]; final response = await client.listBuckets(); expect(response, isA>()); @@ -77,24 +62,8 @@ void main() { test('should create bucket', () async { const testBucketId = 'test_bucket'; - const requestBody = { - 'id': testBucketId, - 'name': testBucketId, - 'public': false - }; - when( - () => storageFetch.post( - bucketUrl, - requestBody, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - { - 'name': 'test_bucket', - }, - ), - ); + + customHttpClient.response = {'name': 'test_bucket'}; final response = await client.createBucket(testBucketId); expect(response, isA()); @@ -103,16 +72,8 @@ void main() { test('should get bucket', () async { const testBucketId = 'test_bucket'; - when( - () => storageFetch.get( - '$bucketUrl/$testBucketId', - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - testBucketJson, - ), - ); + + customHttpClient.response = testBucketJson; final response = await client.getBucket(testBucketId); expect(response, isA()); @@ -122,19 +83,8 @@ void main() { test('should empty bucket', () async { const testBucketId = 'test_bucket'; - when( - () => storageFetch.post( - '$bucketUrl/$testBucketId/empty', - {}, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - { - 'message': 'Emptied', - }, - ), - ); + + customHttpClient.response = {'message': 'Emptied'}; final response = await client.emptyBucket(testBucketId); expect(response, 'Emptied'); @@ -142,15 +92,8 @@ void main() { test('should delete bucket', () async { const testBucketId = 'test_bucket'; - when( - () => storageFetch.delete( - '$bucketUrl/$testBucketId', - {}, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value({'message': 'Deleted'}), - ); + + customHttpClient.response = {'message': 'Deleted'}; final response = await client.deleteBucket(testBucketId); expect(response, 'Deleted'); @@ -160,18 +103,7 @@ void main() { final file = File('a.txt'); file.writeAsStringSync('File content'); - when( - () => storageFetch.postFile( - '$objectUrl/public/a.txt', - file, - mockFileOptions, - options: mockFetchOptions, - retryAttempts: 0, - retryController: null, - ), - ).thenAnswer( - (_) => Future.value({'Key': 'public/a.txt'}), - ); + customHttpClient.response = {'Key': 'public/a.txt'}; final response = await client.from('public').upload('a.txt', file); expect(response, isA()); @@ -182,18 +114,7 @@ void main() { final file = File('a.txt'); file.writeAsStringSync('Updated content'); - when( - () => storageFetch.putFile( - '$objectUrl/public/a.txt', - file, - mockFileOptions, - options: mockFetchOptions, - retryAttempts: 0, - retryController: null, - ), - ).thenAnswer( - (_) => Future.value({'Key': 'public/a.txt'}), - ); + customHttpClient.response = {'Key': 'public/a.txt'}; final response = await client.from('public').update('a.txt', file); expect(response, isA()); @@ -201,52 +122,21 @@ void main() { }); test('should move file', () async { - const requestBody = { - 'bucketId': 'public', - 'sourceKey': 'a.txt', - 'destinationKey': 'b.txt', - }; - when( - () => storageFetch.post( - '$objectUrl/move', - requestBody, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value({'message': 'Move'}), - ); + customHttpClient.response = {'message': 'Move'}; final response = await client.from('public').move('a.txt', 'b.txt'); expect(response, 'Move'); }); test('should createSignedUrl file', () async { - when( - () => storageFetch.post( - '$objectUrl/sign/public/b.txt', - {'expiresIn': 60}, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value({'signedURL': 'url'}), - ); + customHttpClient.response = {'signedURL': 'url'}; final response = await client.from('public').createSignedUrl('b.txt', 60); expect(response, isA()); }); test('should list files', () async { - when( - () => storageFetch.post( - '$objectUrl/list/public', - any(), - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - [testFileObjectJson, testFileObjectJson], - ), - ); + customHttpClient.response = [testFileObjectJson, testFileObjectJson]; final response = await client.from('public').list(); expect(response, isA>()); @@ -257,16 +147,7 @@ void main() { final file = File('a.txt'); file.writeAsStringSync('Updated content'); - when( - () => storageFetch.get( - '$objectUrl/public_bucket/b.txt', - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - file.readAsBytesSync(), - ), - ); + customHttpClient.response = file.readAsBytesSync(); final response = await client.from('public_bucket').download('b.txt'); expect(response, isA()); @@ -279,20 +160,7 @@ void main() { }); test('should remove file', () async { - final requestBody = { - 'prefixes': ['a.txt', 'b.txt'] - }; - when( - () => storageFetch.delete( - '$objectUrl/public', - requestBody, - options: mockFetchOptions, - ), - ).thenAnswer( - (_) => Future.value( - [testFileObjectJson, testFileObjectJson], - ), - ); + customHttpClient.response = [testFileObjectJson, testFileObjectJson]; final response = await client.from('public').remove(['a.txt', 'b.txt']); expect(response, isA()); @@ -307,10 +175,9 @@ void main() { '$supabaseUrl/storage/v1', {'Authorization': 'Bearer $supabaseKey'}, retryAttempts: 5, + // `RetryHttpClient` will throw `SocketException` for the first two tries + httpClient: RetryHttpClient(), ); - - // `RetryHttpClient` will throw `SocketException` for the first two tries - storageFetch = Fetch(RetryHttpClient()); }); test('Upload fails without retry', () async { @@ -383,10 +250,13 @@ void main() { group('Client with custom http client', () { setUp(() { // init SupabaseClient with test url & test key - client = SupabaseStorageClient('$supabaseUrl/storage/v1', { - 'Authorization': 'Bearer $supabaseKey', - }); - storageFetch = Fetch(CustomHttpClient()); + client = SupabaseStorageClient( + '$supabaseUrl/storage/v1', + { + 'Authorization': 'Bearer $supabaseKey', + }, + httpClient: FailingHttpClient(), + ); }); test('should list buckets', () async { try { diff --git a/packages/storage_client/test/client_test.dart b/packages/storage_client/test/client_test.dart index 6d2ab7c1..0560facc 100644 --- a/packages/storage_client/test/client_test.dart +++ b/packages/storage_client/test/client_test.dart @@ -1,9 +1,7 @@ import 'dart:io'; import 'package:mime/mime.dart'; -import 'package:mocktail/mocktail.dart'; import "package:path/path.dart" show join; -import 'package:storage_client/src/types.dart'; import 'package:storage_client/storage_client.dart'; import 'package:test/test.dart'; @@ -36,9 +34,6 @@ void main() { 'Authorization': 'Bearer $storageKey', }); - // Register default mock values (used by mocktail) - registerFallbackValue(const FileOptions()); - registerFallbackValue(const FetchOptions()); file = File(join( Directory.current.path, 'test', 'fixtures', 'upload', 'sadcat.jpg')); }); diff --git a/packages/storage_client/test/custom_http_client.dart b/packages/storage_client/test/custom_http_client.dart index f4599ddf..833576a9 100644 --- a/packages/storage_client/test/custom_http_client.dart +++ b/packages/storage_client/test/custom_http_client.dart @@ -1,8 +1,9 @@ import 'dart:convert'; +import 'dart:typed_data'; import 'package:http/http.dart'; -class CustomHttpClient extends BaseClient { +class FailingHttpClient extends BaseClient { @override Future send(BaseRequest request) async { //Return custom status code to check for usage of this client. @@ -31,3 +32,23 @@ class RetryHttpClient extends BaseClient { ); } } + +class CustomHttpClient extends BaseClient { + dynamic response; + + @override + Future send(BaseRequest request) async { + final dynamic body; + if (response is Uint8List) { + body = response; + } else { + body = utf8.encode(jsonEncode(response)); + } + + return StreamedResponse( + Stream.value(body), + 201, + request: request, + ); + } +}