Addressing feedback

7 files changed, 63 insertions, 12 deletions

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