Skip to content

Migrate storage to component framework #2326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions packages/storage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,36 @@
*/

import firebase from '@firebase/app';
import { FirebaseApp } from '@firebase/app-types';
import {
FirebaseServiceFactory,
_FirebaseNamespace
} from '@firebase/app-types/private';
import { _FirebaseNamespace } from '@firebase/app-types/private';
import { StringFormat } from './src/implementation/string';
import { TaskEvent, TaskState } from './src/implementation/taskenums';

import { XhrIoPool } from './src/implementation/xhriopool';
import { Reference } from './src/reference';
import { Service } from './src/service';
import * as types from '@firebase/storage-types';
import {
Component,
ComponentType,
ComponentContainer
} from '@firebase/component';

/**
* Type constant for Firebase Storage.
*/
const STORAGE_TYPE = 'storage';

function factory(
app: FirebaseApp,
unused: unknown,
container: ComponentContainer,
url?: string
): types.FirebaseStorage {
// Dependencies
const app = container.getProvider('app').getImmediate();
const authProvider = container.getProvider('auth-internal');

return (new Service(
app,
authProvider,
new XhrIoPool(),
url
) as unknown) as types.FirebaseStorage;
Expand All @@ -55,13 +60,10 @@ export function registerStorage(instance: _FirebaseNamespace): void {
Storage: Service,
Reference
};
instance.INTERNAL.registerService(
STORAGE_TYPE,
factory as FirebaseServiceFactory,
namespaceExports,
undefined,
// Allow multiple storage instances per app.
true
instance.INTERNAL.registerComponent(
new Component(STORAGE_TYPE, factory, ComponentType.PUBLIC)
.setServiceProps(namespaceExports)
.setMultipleInstances(true)
);
}

Expand Down
1 change: 1 addition & 0 deletions packages/storage/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"dependencies": {
"@firebase/storage-types": "0.3.5",
"@firebase/util": "0.2.31",
"@firebase/component": "0.1.0",
"tslib": "1.10.0"
},
"peerDependencies": {
Expand Down
16 changes: 8 additions & 8 deletions packages/storage/src/implementation/authwrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
_FirebaseApp,
FirebaseAuthTokenData
} from '@firebase/app-types/private';
import { Provider } from '@firebase/component';
import { FirebaseAuthInternal } from '@firebase/auth-interop-types';

/**
* @param app If null, getAuthToken always resolves with null.
Expand All @@ -40,6 +42,7 @@ import {
*/
export class AuthWrapper {
private app_: FirebaseApp | null;
private authProvider_: Provider<FirebaseAuthInternal>;
private bucket_: string | null = null;

private storageRefMaker_: (p1: AuthWrapper, p2: Location) => Reference;
Expand All @@ -53,6 +56,7 @@ export class AuthWrapper {

constructor(
app: FirebaseApp | null,
authProvider: Provider<FirebaseAuthInternal>,
maker: (p1: AuthWrapper, p2: Location) => Reference,
requestMaker: requestMaker,
service: Service,
Expand All @@ -65,6 +69,7 @@ export class AuthWrapper {
this.bucket_ = AuthWrapper.extractBucket_(options);
}
}
this.authProvider_ = authProvider;
this.storageRefMaker_ = maker;
this.requestMaker_ = requestMaker;
this.pool_ = pool;
Expand All @@ -84,14 +89,9 @@ export class AuthWrapper {
}

getAuthToken(): Promise<string | null> {
// TODO(andysoto): remove ifDef checks after firebase-app implements stubs
// (b/28673818).
if (
this.app_ !== null &&
type.isDef((this.app_ as _FirebaseApp).INTERNAL) &&
type.isDef((this.app_ as _FirebaseApp).INTERNAL.getToken)
) {
return (this.app_ as _FirebaseApp).INTERNAL.getToken().then(
const auth = this.authProvider_.getImmediate(undefined, { optional: true });
if (auth) {
return auth.getToken().then(
(response: FirebaseAuthTokenData | null): string | null => {
if (response !== null) {
return response.accessToken;
Expand Down
10 changes: 9 additions & 1 deletion packages/storage/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { Location } from './implementation/location';
import * as RequestExports from './implementation/request';
import { XhrIoPool } from './implementation/xhriopool';
import { Reference } from './reference';
import { Provider } from '@firebase/component';
import { FirebaseAuthInternal } from '@firebase/auth-interop-types';

/**
* A service that provides firebaseStorage.Reference instances.
Expand All @@ -35,12 +37,18 @@ export class Service {
private bucket_: Location | null = null;
private internals_: ServiceInternals;

constructor(app: FirebaseApp, pool: XhrIoPool, url?: string) {
constructor(
app: FirebaseApp,
authProvider: Provider<FirebaseAuthInternal>,
pool: XhrIoPool,
url?: string
) {
function maker(authWrapper: AuthWrapper, loc: Location): Reference {
return new Reference(authWrapper, loc);
}
this.authWrapper_ = new AuthWrapper(
app,
authProvider,
maker,
RequestExports.makeRequest,
this,
Expand Down
24 changes: 20 additions & 4 deletions packages/storage/test/reference.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,16 @@ import { Service } from '../src/service';
import * as testShared from './testshared';
import { SendHook, TestingXhrIo } from './xhrio';
import { DEFAULT_HOST } from '../src/implementation/constants';
import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
import { Provider } from '@firebase/component';

/* eslint-disable @typescript-eslint/no-floating-promises */
function makeFakeService(app: FirebaseApp, sendHook: SendHook): Service {
return new Service(app, testShared.makePool(sendHook));
function makeFakeService(
app: FirebaseApp,
authProvider: Provider<FirebaseAuthInternal>,
sendHook: SendHook
): Service {
return new Service(app, authProvider, testShared.makePool(sendHook));
}

function makeStorage(url: string): Reference {
Expand All @@ -38,6 +45,7 @@ function makeStorage(url: string): Reference {

const authWrapper = new AuthWrapper(
null,
testShared.emptyAuthProvider,
maker,
makeRequest,
({} as any) as Service,
Expand Down Expand Up @@ -190,7 +198,11 @@ describe('Firebase Storage > Reference', () => {
done();
}

const service = makeFakeService(testShared.fakeAppNoAuth, newSend);
const service = makeFakeService(
testShared.fakeApp,
testShared.emptyAuthProvider,
newSend
);
const ref = service.refFromURL('gs://test-bucket');
ref.child('foo').getMetadata();
});
Expand All @@ -212,7 +224,11 @@ describe('Firebase Storage > Reference', () => {
done();
}

const service = makeFakeService(testShared.fakeApp, newSend);
const service = makeFakeService(
testShared.fakeApp,
testShared.fakeAuthProvider,
newSend
);
const ref = service.refFromURL('gs://test-bucket');
ref.child('foo').getMetadata();
});
Expand Down
7 changes: 6 additions & 1 deletion packages/storage/test/requests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ import { XhrIoPool } from '../src/implementation/xhriopool';
import { Metadata } from '../src/metadata';
import { Reference } from '../src/reference';
import { Service } from '../src/service';
import { assertObjectIncludes, fakeXhrIo } from './testshared';
import {
assertObjectIncludes,
fakeXhrIo,
fakeAuthProvider
} from './testshared';
import {
DEFAULT_HOST,
CONFIG_STORAGE_BUCKET_KEY
Expand Down Expand Up @@ -61,6 +65,7 @@ describe('Firebase Storage > Requests', () => {

const authWrapper = new AuthWrapper(
mockApp,
fakeAuthProvider,
(authWrapper, loc) => {
return new Reference(authWrapper, loc);
},
Expand Down
56 changes: 45 additions & 11 deletions packages/storage/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import * as testShared from './testshared';
import { DEFAULT_HOST } from '../src/implementation/constants';
import { FirebaseStorageError } from '../src/implementation/error';

const fakeAppGs = testShared.makeFakeApp(null, 'gs://mybucket');
const fakeAppGsEndingSlash = testShared.makeFakeApp(null, 'gs://mybucket/');
const fakeAppInvalidGs = testShared.makeFakeApp(null, 'gs://mybucket/hello');
const fakeAppGs = testShared.makeFakeApp('gs://mybucket');
const fakeAppGsEndingSlash = testShared.makeFakeApp('gs://mybucket/');
const fakeAppInvalidGs = testShared.makeFakeApp('gs://mybucket/hello');
const xhrIoPool = new XhrIoPool();

function makeGsUrl(child: string = ''): string {
Expand All @@ -33,7 +33,11 @@ function makeGsUrl(child: string = ''): string {

describe('Firebase Storage > Service', () => {
describe('simple constructor', () => {
const service = new Service(testShared.fakeApp, xhrIoPool);
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool
);
it('Root refs point to the right place', () => {
const ref = service.ref();
assert.equal(ref.toString(), makeGsUrl());
Expand Down Expand Up @@ -65,6 +69,7 @@ describe('Firebase Storage > Service', () => {
it('gs:// custom bucket constructor refs point to the right place', () => {
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
'gs://foo-bar.appspot.com'
);
Expand All @@ -74,6 +79,7 @@ describe('Firebase Storage > Service', () => {
it('http:// custom bucket constructor refs point to the right place', () => {
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
`http://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o`
);
Expand All @@ -83,6 +89,7 @@ describe('Firebase Storage > Service', () => {
it('https:// custom bucket constructor refs point to the right place', () => {
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
`https://${DEFAULT_HOST}/v1/b/foo-bar.appspot.com/o`
);
Expand All @@ -93,6 +100,7 @@ describe('Firebase Storage > Service', () => {
it('Bare bucket name constructor refs point to the right place', () => {
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
'foo-bar.appspot.com'
);
Expand All @@ -102,6 +110,7 @@ describe('Firebase Storage > Service', () => {
it('Child refs point to the right place', () => {
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
'foo-bar.appspot.com'
);
Expand All @@ -110,28 +119,45 @@ describe('Firebase Storage > Service', () => {
});
it('Throws trying to construct with a gs:// URL containing an object path', () => {
const error = testShared.assertThrows(() => {
new Service(testShared.fakeApp, xhrIoPool, 'gs://bucket/object/');
new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool,
'gs://bucket/object/'
);
}, 'storage/invalid-default-bucket');
assert.match(error.message, /Invalid default bucket/);
});
});
describe('default bucket config', () => {
it('gs:// works without ending slash', () => {
const service = new Service(fakeAppGs, xhrIoPool);
const service = new Service(
fakeAppGs,
testShared.fakeAuthProvider,
xhrIoPool
);
assert.equal(service.ref().toString(), 'gs://mybucket/');
});
it('gs:// works with ending slash', () => {
const service = new Service(fakeAppGsEndingSlash, xhrIoPool);
const service = new Service(
fakeAppGsEndingSlash,
testShared.fakeAuthProvider,
xhrIoPool
);
assert.equal(service.ref().toString(), 'gs://mybucket/');
});
it('Throws when config bucket is gs:// with an object path', () => {
testShared.assertThrows(() => {
new Service(fakeAppInvalidGs, xhrIoPool);
new Service(fakeAppInvalidGs, testShared.fakeAuthProvider, xhrIoPool);
}, 'storage/invalid-default-bucket');
});
});
describe('refFromURL', () => {
const service = new Service(testShared.fakeApp, xhrIoPool);
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool
);
it('Throws on non-URL arg', () => {
const error = testShared.assertThrows(() => {
service.refFromURL('path/to/child');
Expand All @@ -158,7 +184,11 @@ describe('Firebase Storage > Service', () => {
});
});
describe('Argument verification', () => {
const service = new Service(testShared.fakeApp, xhrIoPool);
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool
);
describe('ref', () => {
it('Throws with two args', () => {
testShared.assertThrows(
Expand Down Expand Up @@ -272,7 +302,11 @@ describe('Firebase Storage > Service', () => {
});

describe('Deletion', () => {
const service = new Service(testShared.fakeApp, xhrIoPool);
const service = new Service(
testShared.fakeApp,
testShared.fakeAuthProvider,
xhrIoPool
);
it('In-flight requests are canceled when the service is deleted', () => {
const ref = service.refFromURL('gs://mybucket/image.jpg');
const toReturn = ref.getMetadata().then(
Expand Down
8 changes: 7 additions & 1 deletion packages/storage/test/task.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ import { Headers } from '../src/implementation/xhrio';
import { Reference } from '../src/reference';
import { Service } from '../src/service';
import { UploadTask } from '../src/task';
import { assertThrows, bind as fbsBind, makePool } from './testshared';
import {
assertThrows,
bind as fbsBind,
makePool,
emptyAuthProvider
} from './testshared';
import { StringHeaders, TestingXhrIo } from './xhrio';

const testLocation = new Location('bucket', 'object');
Expand Down Expand Up @@ -63,6 +68,7 @@ function authWrapperWithHandler(handler: RequestHandler): AuthWrapper {

return new AuthWrapper(
null,
emptyAuthProvider,
() => {
return {} as Reference;
},
Expand Down
Loading