From e86665b1ba2bbaf7ddd70873118badc812e0bc20 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 13 Apr 2018 12:17:36 -0700 Subject: Addressing feedback --- Firebase/Storage/CHANGELOG.md | 6 ++++- Firebase/Storage/FIRStorageErrors.m | 6 +++++ Firebase/Storage/FIRStorageGetDownloadURLTask.m | 20 ++++++++++----- Firebase/Storage/FIRStorageGetMetadataTask.m | 2 +- Firebase/Storage/Private/FIRStorageErrors.h | 8 ++++++ .../Storage/Private/FIRStorageGetDownloadURLTask.h | 3 --- .../Private/FIRStorageGetDownloadURLTask_Private.h | 30 ++++++++++++++++++++++ 7 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 Firebase/Storage/Private/FIRStorageGetDownloadURLTask_Private.h (limited to 'Firebase/Storage') diff --git a/Firebase/Storage/CHANGELOG.md b/Firebase/Storage/CHANGELOG.md index 5054cf2..a48c3c5 100644 --- a/Firebase/Storage/CHANGELOG.md +++ b/Firebase/Storage/CHANGELOG.md @@ -1,5 +1,9 @@ +# v3.0.0 +- [removed] Removed `downloadURLs` property on `StorageMetadata`. Use `StorageReference.downloadURL(completion:)` to obtain a current download URL. +- [changed] The `maxOperationRetryTime` timeout now applies to calls to `StorageReference.getMetadata(completion:)`. + # v2.2.0 -- [changed] Deprecated `downloadURLs` property on `StorageMetadata`. Use `StorageReference.downloadURLWithCompletion()` to obtain a current download URL. +- [changed] Deprecated `downloadURLs` property on `StorageMetadata`. Use `StorageReference.downloadURL(completion:)` to obtain a current download URL. # v2.1.3 - [changed] Addresses CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF warnings that surface in newer versions of Xcode and CocoaPods. diff --git a/Firebase/Storage/FIRStorageErrors.m b/Firebase/Storage/FIRStorageErrors.m index 7cc4beb..651bfd1 100644 --- a/Firebase/Storage/FIRStorageErrors.m +++ b/Firebase/Storage/FIRStorageErrors.m @@ -181,4 +181,10 @@ return [FIRStorageErrors errorWithCode:FIRStorageErrorCodeUnknown infoDictionary:dict]; } ++ (NSError *)errorWithCustomMessage:(NSString *)errorMessage { + return [NSError errorWithDomain:FIRStorageErrorDomain + code:FIRStorageErrorCodeUnknown + userInfo:@{NSLocalizedDescriptionKey : errorMessage}]; +} + @end diff --git a/Firebase/Storage/FIRStorageGetDownloadURLTask.m b/Firebase/Storage/FIRStorageGetDownloadURLTask.m index 7447888..c4e0035 100644 --- a/Firebase/Storage/FIRStorageGetDownloadURLTask.m +++ b/Firebase/Storage/FIRStorageGetDownloadURLTask.m @@ -14,11 +14,7 @@ #import "FIRStorageGetDownloadURLTask.h" -#import "FIRStorageConstants.h" #import "FIRStorageTask_Private.h" -#import "FIRStorageUtils.h" - -#import "FirebaseStorage.h" @implementation FIRStorageGetDownloadURLTask { @private @@ -56,7 +52,13 @@ components.scheme = kFIRStorageScheme; components.host = kFIRStorageHost; components.percentEncodedPath = fullPath; - components.query = [NSString stringWithFormat:@"alt=media&token=%@", downloadTokenArray[0]]; + + // The backend can return an arbitrary number of download tokens, but we only expose the first + // token via the download URL. + NSURLQueryItem *altItem = [[NSURLQueryItem alloc] initWithName:@"alt" value:@"media"]; + NSURLQueryItem *tokenItem = + [[NSURLQueryItem alloc] initWithName:@"token" value:downloadTokenArray[0]]; + components.queryItems = @[ altItem, tokenItem ]; return [components URL]; } @@ -67,14 +69,14 @@ - (void)enqueue { NSMutableURLRequest *request = [self.baseRequest mutableCopy]; request.HTTPMethod = @"GET"; - request.timeoutInterval = self.reference.storage.maxDownloadRetryTime; + request.timeoutInterval = self.reference.storage.maxOperationRetryTime; FIRStorageVoidURLError callback = _completion; _completion = nil; GTMSessionFetcher *fetcher = [self.fetcherService fetcherWithRequest:request]; _fetcher = fetcher; - fetcher.comment = @"DownloadURLTask"; + fetcher.comment = @"GetDownloadURLTask"; #pragma clang diagnostic push #pragma clang diagnostic ignored "-Warc-retain-cycles" @@ -89,6 +91,10 @@ if (responseDictionary != nil) { downloadURL = [FIRStorageGetDownloadURLTask downloadURLFromMetadataDictionary:responseDictionary]; + if (!downloadURL) { + self.error = + [FIRStorageErrors errorWithCustomMessage:@"Failed to retrieve a download URL"]; + } } else { self.error = [FIRStorageErrors errorWithInvalidRequest:data]; } diff --git a/Firebase/Storage/FIRStorageGetMetadataTask.m b/Firebase/Storage/FIRStorageGetMetadataTask.m index 077dcd7..752c410 100644 --- a/Firebase/Storage/FIRStorageGetMetadataTask.m +++ b/Firebase/Storage/FIRStorageGetMetadataTask.m @@ -46,7 +46,7 @@ - (void)enqueue { NSMutableURLRequest *request = [self.baseRequest mutableCopy]; request.HTTPMethod = @"GET"; - request.timeoutInterval = self.reference.storage.maxDownloadRetryTime; + request.timeoutInterval = self.reference.storage.maxOperationRetryTime; FIRStorageVoidMetadataError callback = _completion; _completion = nil; diff --git a/Firebase/Storage/Private/FIRStorageErrors.h b/Firebase/Storage/Private/FIRStorageErrors.h index 46e87d2..a76a6aa 100644 --- a/Firebase/Storage/Private/FIRStorageErrors.h +++ b/Firebase/Storage/Private/FIRStorageErrors.h @@ -57,6 +57,14 @@ NS_ASSUME_NONNULL_BEGIN */ + (NSError *)errorWithInvalidRequest:(NSData *)request; +/** + * Creates a Firebase Storage error with a custom error message. + * + * @param errorMessage A custom error message. + * @return Returns the corresponding Firebase Storage error. + */ ++ (NSError *)errorWithCustomMessage:(NSString *)errorMessage; + @end NS_ASSUME_NONNULL_END diff --git a/Firebase/Storage/Private/FIRStorageGetDownloadURLTask.h b/Firebase/Storage/Private/FIRStorageGetDownloadURLTask.h index c349f30..8cd9eb3 100644 --- a/Firebase/Storage/Private/FIRStorageGetDownloadURLTask.h +++ b/Firebase/Storage/Private/FIRStorageGetDownloadURLTask.h @@ -29,9 +29,6 @@ NS_ASSUME_NONNULL_BEGIN fetcherService:(GTMSessionFetcherService *)service completion:(FIRStorageVoidURLError)completion; -// Visible for testing. -+ (NSURL *)downloadURLFromMetadataDictionary:(NSDictionary *)dictionary; - @end NS_ASSUME_NONNULL_END diff --git a/Firebase/Storage/Private/FIRStorageGetDownloadURLTask_Private.h b/Firebase/Storage/Private/FIRStorageGetDownloadURLTask_Private.h new file mode 100644 index 0000000..ac5fd80 --- /dev/null +++ b/Firebase/Storage/Private/FIRStorageGetDownloadURLTask_Private.h @@ -0,0 +1,30 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import "FIRStorageGetDownloadURLTask.h" + +NS_ASSUME_NONNULL_BEGIN + +/** + * Task which provides the ability to get a download URL for an object in Firebase Storage. + */ +@interface FIRStorageGetDownloadURLTask () + ++ (NSURL *)downloadURLFromMetadataDictionary:(NSDictionary *)dictionary; + +@end + +NS_ASSUME_NONNULL_END -- cgit v1.2.3