-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Add timeout properties to FirebaseStorage #532
Conversation
720cdb1
to
5d630e6
Compare
5d630e6
to
1ae8689
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
- (void)setMaxDownloadRetryTime:(FlutterMethodCall *)call result:(FlutterResult)result { | ||
NSNumber *time = call.arguments[@"time"]; | ||
storage.maxDownloadRetryTime = [time longLongValue] / 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If time is 500ms, you'd end up with 0 because of integer division. You should divide by 1000.0.
@@ -44,6 +48,60 @@ class FirebaseStorage { | |||
/// Creates a new [StorageReference] initialized at the root | |||
/// Firebase Storage location. | |||
StorageReference ref() => new StorageReference._(const <String>[], this); | |||
|
|||
Future<int> getMaxDownloadRetryTimeMillis() async { | |||
final num n = await channel.invokeMethod( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should always be an int
.
if ([@"StorageReference#putFile" isEqualToString:call.method]) { | ||
storage = [self getStorage:call]; | ||
if ([@"FirebaseStorage#getMaxDownloadRetryTime" isEqualToString:call.method]) { | ||
result(@((long)storage.maxDownloadRetryTime * 1000.0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(int64_t)(storage.xxx * 1000)
break; | ||
case "FirebaseStorage#setMaxOperationRetryTime": | ||
setMaxOperationTimeMillis(call, result); | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, reduce to set/getRetryTime(maxDownload, maxUpload, maxOperation) to avoid a platform channel invocation per value.
'app': app?.name, | ||
'bucket': storageBucket, | ||
}); | ||
return n.toInt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and then you can just return n
.
|
||
NSMutableDictionary *bucketMap = _storageMap[app.name]; | ||
if (!bucketMap) { | ||
bucketMap = [[NSMutableDictionary alloc] init]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe [NSMutableDictionary dictionaryWithCapacity:1]
.
NSString *bucketName = [url host]; | ||
FIRStorage *storage = bucketMap[bucketName]; | ||
if (!storage) { | ||
// Raises an exception if bucketUrl is invalid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That exception never reaches Dart. Is there any way to get an NSError
from a FIRStorage
constructor?
@@ -80,6 +84,68 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result | |||
} | |||
} | |||
|
|||
- (FIRStorage *)getStorage:(FlutterMethodCall *)call { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment on why this method doesn't just return [FIRStorage x y]
.
No description provided.