Move old validation scheme to compat, fix some tests

Author: hsubox76

packages/storage/exp-types/index.d.ts renamed to packages/storage-types/exp/index.d.ts

@@ -104,12 +104,4 @@ export class FirebaseStorage {
   app: FirebaseApp | null;
   maxOperationRetryTime: number;
   maxUploadRetryTime: number;
-  setMaxOperationRetryTime(time: number): void;
-  setMaxUploadRetryTime(time: number): void;
-}
-
-declare module '@firebase/component' {
-  interface NameServiceMapping {
-    'storage-exp': FirebaseStorage;
-  }
 }

packages/storage/.eslintrc.js

@@ -37,7 +37,11 @@ module.exports = {
     'import/no-extraneous-dependencies': [
       'error',
       {
-        'packageDir': [path.resolve(__dirname, '../../'), __dirname]
+        'packageDir': [
+          path.resolve(__dirname, '../../'),
+          __dirname,
+          path.resolve(__dirname, 'exp')
+        ]
       }
     ]
   }

packages/storage/compat/reference.ts

@@ -28,16 +28,26 @@ import {
   updateMetadata,
   deleteObject
 } from '../src/reference';
-import { validate, stringSpec } from '../src/implementation/args';
+import {
+  validate,
+  stringSpec,
+  listOptionSpec,
+  metadataSpec,
+  uploadDataSpec
+} from '../src/implementation/args';
 import { Metadata } from '../src/metadata';
 import { UploadTask } from '../src/task';
-import { StringFormat } from '../src/implementation/string';
+import { StringFormat, formatValidator } from '../src/implementation/string';
 import { ListResult, ListOptions } from '../src/list';
 
 export class ReferenceCompat extends Reference {
   static fromReference(ref: Reference): ReferenceCompat {
     return new ReferenceCompat(ref.service, ref.location);
   }
+  toString(): string {
+    validate('toString', [], arguments);
+    return super.toString();
+  }
   /**
    * @return A reference to the object obtained by
    *     appending childPath, removing any duplicate, beginning, or trailing
@@ -54,6 +64,7 @@ export class ReferenceCompat extends Reference {
    *     current object, or null if the current object is the root.
    */
   get parent(): ReferenceCompat | null {
+    validate('parent', [], arguments);
     const reference = getParent(this);
     if (reference == null) {
       return null;
@@ -71,6 +82,8 @@ export class ReferenceCompat extends Reference {
     data: Blob | Uint8Array | ArrayBuffer,
     metadata: Metadata | null = null
   ): UploadTask {
+    validate('put', [uploadDataSpec(), metadataSpec(true)], arguments);
+    this.throwIfRoot_('put');
     return uploadBytes(this, data, metadata);
   }
   /**
@@ -85,6 +98,12 @@ export class ReferenceCompat extends Reference {
     format: StringFormat = StringFormat.RAW,
     metadata?: Metadata
   ): UploadTask {
+    validate(
+      'putString',
+      [stringSpec(), stringSpec(formatValidator, true), metadataSpec(true)],
+      arguments
+    );
+    this.throwIfRoot_('putString');
     return uploadString(this, value, format, metadata);
   }
   /**
@@ -105,6 +124,7 @@ export class ReferenceCompat extends Reference {
    *      folder. `nextPageToken` is never returned.
    */
   listAll(): Promise<ListResult> {
+    validate('listAll', [], arguments);
     return listAll(this);
   }
   /**
@@ -128,6 +148,7 @@ export class ReferenceCompat extends Reference {
    *      can be used to get the rest of the results.
    */
   list(options?: ListOptions | null): Promise<ListResult> {
+    validate('list', [listOptionSpec(true)], arguments);
     return list(this, options);
   }
 
@@ -137,6 +158,7 @@ export class ReferenceCompat extends Reference {
    *     rejected.
    */
   getMetadata(): Promise<Metadata> {
+    validate('getMetadata', [], arguments);
     return getMetadata(this);
   }
 
@@ -150,6 +172,7 @@ export class ReferenceCompat extends Reference {
    *     @see firebaseStorage.Reference.prototype.getMetadata
    */
   updateMetadata(metadata: Metadata): Promise<Metadata> {
+    validate('updateMetadata', [metadataSpec()], arguments);
     return updateMetadata(this, metadata);
   }
 
@@ -158,6 +181,7 @@ export class ReferenceCompat extends Reference {
    *     URL for this object.
    */
   getDownloadURL(): Promise<string> {
+    validate('getDownloadURL', [], arguments);
     return getDownloadURL(this);
   }
 
@@ -166,6 +190,8 @@ export class ReferenceCompat extends Reference {
    * @return A promise that resolves if the deletion succeeds.
    */
   delete(): Promise<void> {
+    validate('delete', [], arguments);
+    this.throwIfRoot_('delete');
     return deleteObject(this);
   }
 }

packages/storage/compat/service.ts

@@ -15,15 +15,38 @@
  * limitations under the License.
  */
 
-import { StorageService, pathValidator, urlValidator } from '../src/service';
+import { StorageService, isUrl } from '../src/service';
+import { Location } from '../src/implementation/location';
 import { FirebaseApp } from '@firebase/app-types';
 import { Provider } from '@firebase/component';
 import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
 import { XhrIoPool } from '../src/implementation/xhriopool';
-import { Reference } from '../src/reference';
 import * as args from '../src/implementation/args';
 import { ReferenceCompat } from './reference';
 
+export function urlValidator(maybeUrl: unknown): void {
+  if (typeof maybeUrl !== 'string') {
+    throw 'Path is not a string.';
+  }
+  if (!isUrl) {
+    throw 'Expected full URL but got a child path, use ref instead.';
+  }
+  try {
+    Location.makeFromUrl(maybeUrl as string);
+  } catch (e) {
+    throw 'Expected valid full URL but got an invalid one.';
+  }
+}
+
+export function pathValidator(path: unknown): void {
+  if (typeof path !== 'string') {
+    throw 'Path is not a string.';
+  }
+  if (isUrl(path)) {
+    throw 'Expected child path but got a URL, use refFromURL instead.';
+  }
+}
+
 /**
  * A service that provides firebaseStorage.Reference instances.
  * @param opt_url gs:// url to a custom Storage Bucket
@@ -62,13 +85,33 @@ export class StorageServiceCompat extends StorageService {
    * Returns a firebaseStorage.Reference object for the given absolute URL,
    * which must be a gs:// or http[s]:// URL.
    */
-  refFromURL(url: string): Reference {
+  refFromURL(url: string): ReferenceCompat {
     args.validate(
       'refFromURL',
       [args.stringSpec(urlValidator, false)],
       arguments
     );
-    return new Reference(this, url);
+    return new ReferenceCompat(this, url);
+  }
+
+  setMaxUploadRetryTime(time: number): void {
+    args.validate(
+      'setMaxUploadRetryTime',
+      [args.nonNegativeNumberSpec()],
+      arguments
+    );
+    // Can't call get/set on super class.
+    this.maxUploadRetryTime_ = time;
+  }
+
+  setMaxOperationRetryTime(time: number): void {
+    args.validate(
+      'setMaxOperationRetryTime',
+      [args.nonNegativeNumberSpec()],
+      arguments
+    );
+    // Can't call get/set on super class.
+    this.maxOperationRetryTime_ = time;
   }
 
   get app(): FirebaseApp | null {

packages/storage/exp/api.ts

@@ -29,6 +29,7 @@ import {
 } from '@firebase/component';
 
 import { name, version } from '../package.json';
+import { FirebaseStorage } from '@firebase/storage-types/exp';
 
 export { ref } from '../src/service';
 export {
@@ -79,3 +80,9 @@ export function registerStorage(): void {
 }
 
 registerStorage();
+
+declare module '@firebase/component' {
+  interface NameServiceMapping {
+    'storage-exp': FirebaseStorage;
+  }
+}

packages/storage/exp/index.ts

@@ -4,6 +4,9 @@
   "main": "../dist/exp/index.cjs.js",
   "module": "../dist/exp/index.esm2017.js",
   "browser": "../dist/exp/index.esm2017.js",
-  "typings": "../exp-types/index.d.ts",
-  "private": true
+  "typings": "./dist/exp/index.d.ts",
+  "private": true,
+  "dependencies": {
+    "@firebase/app-exp": "0.0.800"
+  }
 }