From b7c4e12a27d324fab3c0bc1094dc58d7fd001c27 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 12 Jan 2026 13:44:13 -0600 Subject: [PATCH] pr feedback --- .../Connectivity/ConnectivityApiImpl.swift | 6 --- .../services/background_worker.service.dart | 10 +---- .../backup/drift_backup.provider.dart | 2 + .../lib/repositories/upload.repository.dart | 6 +-- mobile/lib/services/upload.service.dart | 40 +++++++++---------- 5 files changed, 26 insertions(+), 38 deletions(-) diff --git a/mobile/ios/Runner/Connectivity/ConnectivityApiImpl.swift b/mobile/ios/Runner/Connectivity/ConnectivityApiImpl.swift index 48927460f8..27f9653459 100644 --- a/mobile/ios/Runner/Connectivity/ConnectivityApiImpl.swift +++ b/mobile/ios/Runner/Connectivity/ConnectivityApiImpl.swift @@ -8,12 +8,6 @@ class ConnectivityApiImpl: ConnectivityApi { init() { monitor.pathUpdateHandler = { [weak self] path in self?.currentPath = path - print("[ConnectivityApi] Network status changed:") - print(" - Status: \(path.status)") - print(" - isExpensive: \(path.isExpensive)") - print(" - isConstrained: \(path.isConstrained)") - print(" - usesWifi: \(path.usesInterfaceType(.wifi))") - print(" - usesCellular: \(path.usesInterfaceType(.cellular))") } monitor.start(queue: queue) // Get initial state synchronously diff --git a/mobile/lib/domain/services/background_worker.service.dart b/mobile/lib/domain/services/background_worker.service.dart index 475ddeaed8..079ed17f21 100644 --- a/mobile/lib/domain/services/background_worker.service.dart +++ b/mobile/lib/domain/services/background_worker.service.dart @@ -249,15 +249,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { final networkCapabilities = await _ref?.read(connectivityApiProvider).getCapabilities() ?? []; return _ref ?.read(uploadServiceProvider) - .startUploadWithHttp( - currentUser.id, - networkCapabilities.isUnmetered, - _cancellationToken, - onProgress: (_, __, ___, ____) {}, - onSuccess: (_, __) {}, - onError: (_, __) {}, - onICloudProgress: (_, __) {}, - ); + .startUploadWithHttp(currentUser.id, networkCapabilities.isUnmetered, _cancellationToken); }, (error, stack) { dPrint(() => "Error in backup zone $error, $stack"); diff --git a/mobile/lib/providers/backup/drift_backup.provider.dart b/mobile/lib/providers/backup/drift_backup.provider.dart index 104807269b..3b2dd4f4d9 100644 --- a/mobile/lib/providers/backup/drift_backup.provider.dart +++ b/mobile/lib/providers/backup/drift_backup.provider.dart @@ -504,6 +504,8 @@ class DriftBackupNotifier extends StateNotifier { }, ); } + + _uploadSpeedManager.removeTask(localAssetId); } Future startBackupWithURLSession(String userId) async { diff --git a/mobile/lib/repositories/upload.repository.dart b/mobile/lib/repositories/upload.repository.dart index af716f4828..aff84683c3 100644 --- a/mobile/lib/repositories/upload.repository.dart +++ b/mobile/lib/repositories/upload.repository.dart @@ -110,7 +110,7 @@ class UploadRepository { final fileStream = file.openRead(); final assetRawUploadData = MultipartFile("assetData", fileStream, file.lengthSync(), filename: originalFileName); - final baseRequest = CustomMultipartRequest('POST', Uri.parse('$savedEndpoint/assets'), onProgress: onProgress); + final baseRequest = _CustomMultipartRequest('POST', Uri.parse('$savedEndpoint/assets'), onProgress: onProgress); baseRequest.headers.addAll(headers); baseRequest.fields.addAll(fields); @@ -183,8 +183,8 @@ class UploadResult { } } -class CustomMultipartRequest extends MultipartRequest { - CustomMultipartRequest(super.method, super.url, {required this.onProgress}); +class _CustomMultipartRequest extends MultipartRequest { + _CustomMultipartRequest(super.method, super.url, {required this.onProgress}); final void Function(int bytes, int totalBytes) onProgress; diff --git a/mobile/lib/services/upload.service.dart b/mobile/lib/services/upload.service.dart index 886d93c0df..5f1b8eb77b 100644 --- a/mobile/lib/services/upload.service.dart +++ b/mobile/lib/services/upload.service.dart @@ -150,10 +150,10 @@ class UploadService { String userId, bool hasWifi, CancellationToken cancelToken, { - required void Function(String localAssetId, String filename, int bytes, int totalBytes) onProgress, - required void Function(String localAssetId, String remoteAssetId) onSuccess, - required void Function(String localAssetId, String errorMessage) onError, - required void Function(String localAssetId, double progress) onICloudProgress, + void Function(String localAssetId, String filename, int bytes, int totalBytes)? onProgress, + void Function(String localAssetId, String remoteAssetId)? onSuccess, + void Function(String localAssetId, String errorMessage)? onError, + void Function(String localAssetId, double progress)? onICloudProgress, }) async { const concurrentUploads = 3; final httpClients = List.generate(concurrentUploads, (_) => Client()); @@ -220,10 +220,10 @@ class UploadService { LocalAsset asset, Client httpClient, CancellationToken cancelToken, { - required void Function(String id, String filename, int bytes, int totalBytes) onProgress, - required void Function(String localAssetId, String remoteAssetId) onSuccess, - required void Function(String localAssetId, String errorMessage) onError, - required void Function(String localAssetId, double progress) onICloudProgress, + required void Function(String id, String filename, int bytes, int totalBytes)? onProgress, + required void Function(String localAssetId, String remoteAssetId)? onSuccess, + required void Function(String localAssetId, String errorMessage)? onError, + required void Function(String localAssetId, double progress)? onICloudProgress, }) async { File? file; File? livePhotoFile; @@ -236,7 +236,7 @@ class UploadService { final isAvailableLocally = await _storageRepository.isAssetAvailableLocally(asset.id); - if (!isAvailableLocally && Platform.isIOS) { + if (!isAvailableLocally && CurrentPlatform.isIOS) { _logger.info("Loading iCloud asset ${asset.id} - ${asset.name}"); // Create progress handler for iCloud download @@ -245,7 +245,7 @@ class UploadService { progressHandler = PMProgressHandler(); progressSubscription = progressHandler.stream.listen((event) { - onICloudProgress(asset.localId!, event.progress); + onICloudProgress?.call(asset.localId!, event.progress); }); try { @@ -305,7 +305,7 @@ class UploadService { fields: fields, httpClient: httpClient, cancelToken: cancelToken, - onProgress: (bytes, totalBytes) => onProgress(asset.localId!, livePhotoTitle, bytes, totalBytes), + onProgress: (bytes, totalBytes) => onProgress?.call(asset.localId!, livePhotoTitle, bytes, totalBytes), logContext: 'livePhotoVideo[${asset.localId}]', ); @@ -325,37 +325,37 @@ class UploadService { fields: fields, httpClient: httpClient, cancelToken: cancelToken, - onProgress: (bytes, totalBytes) => onProgress(asset.localId!, originalFileName, bytes, totalBytes), + onProgress: (bytes, totalBytes) => onProgress?.call(asset.localId!, originalFileName, bytes, totalBytes), logContext: 'asset[${asset.localId}]', ); if (result.isSuccess && result.remoteAssetId != null) { - onSuccess(asset.localId!, result.remoteAssetId!); + onSuccess?.call(asset.localId!, result.remoteAssetId!); } else if (result.isCancelled) { - dPrint(() => "Backup was cancelled by the user"); + _logger.warning(() => "Backup was cancelled by the user"); shouldAbortQueuingTasks = true; } else if (result.errorMessage != null) { - dPrint( + _logger.severe( () => "Error(${result.statusCode}) uploading ${asset.localId} | $originalFileName | Created on ${asset.createdAt} | ${result.errorMessage}", ); - onError(asset.localId!, result.errorMessage!); + onError?.call(asset.localId!, result.errorMessage!); if (result.errorMessage == "Quota has been exceeded!") { shouldAbortQueuingTasks = true; } } } catch (error, stackTrace) { - dPrint(() => "Error backup asset: ${error.toString()}: $stackTrace"); - onError(asset.localId!, error.toString()); + _logger.severe(() => "Error backup asset: ${error.toString()}", stackTrace); + onError?.call(asset.localId!, error.toString()); } finally { if (Platform.isIOS) { try { await file?.delete(); await livePhotoFile?.delete(); - } catch (e) { - dPrint(() => "ERROR deleting file: ${e.toString()}"); + } catch (error, stackTrace) { + _logger.severe(() => "ERROR deleting file: ${error.toString()}", stackTrace); } } }