From c74a782049db1991e136f5c646ddcc6dde5b1327 Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Mon, 22 Feb 2021 10:14:19 -0600 Subject: [PATCH 1/7] Add support for FIREBASE_STORAGE_EMULATOR_HOST env var --- src/storage/storage.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/storage/storage.ts b/src/storage/storage.ts index 4364658a8c..da0c643e7d 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -48,6 +48,10 @@ export class Storage implements StorageInterface { }); } + if (!process.env.STORAGE_EMULATOR_HOST && process.env.FIREBASE_STORAGE_EMULATOR_HOST) { + process.env.STORAGE_EMULATOR_HOST = process.env.FIREBASE_STORAGE_EMULATOR_HOST; + } + let storage: typeof StorageClient; try { storage = require('@google-cloud/storage').Storage; From 23f27de33a86ee5caad6d47258b7dc673f122cab Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Mon, 22 Feb 2021 11:11:44 -0600 Subject: [PATCH 2/7] Fixes lint error --- src/storage/storage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage.ts b/src/storage/storage.ts index da0c643e7d..aabf9dd30b 100644 --- a/src/storage/storage.ts +++ b/src/storage/storage.ts @@ -49,7 +49,7 @@ export class Storage implements StorageInterface { } if (!process.env.STORAGE_EMULATOR_HOST && process.env.FIREBASE_STORAGE_EMULATOR_HOST) { - process.env.STORAGE_EMULATOR_HOST = process.env.FIREBASE_STORAGE_EMULATOR_HOST; + process.env.STORAGE_EMULATOR_HOST = process.env.FIREBASE_STORAGE_EMULATOR_HOST; } let storage: typeof StorageClient; From 7df63d10fbade985fc851f4479640a444eb3feb1 Mon Sep 17 00:00:00 2001 From: Samuel Bushi Date: Thu, 11 Mar 2021 16:31:03 +0000 Subject: [PATCH 3/7] Add test for FIREBASE_STORAGE_EMULATOR_HOST support --- test/unit/storage/storage.spec.ts | 162 +++++++++++++++++------------- 1 file changed, 92 insertions(+), 70 deletions(-) diff --git a/test/unit/storage/storage.spec.ts b/test/unit/storage/storage.spec.ts index 997d44846e..c7f8dac1e3 100644 --- a/test/unit/storage/storage.spec.ts +++ b/test/unit/storage/storage.spec.ts @@ -29,88 +29,110 @@ describe('Storage', () => { let mockCredentialApp: FirebaseApp; let storage: Storage; - beforeEach(() => { - mockApp = mocks.app(); - mockCredentialApp = mocks.mockCredentialApp(); - storage = new Storage(mockApp); - }); - - afterEach(() => { - return mockApp.delete(); - }); - - describe('Constructor', () => { - const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; - invalidApps.forEach((invalidApp) => { - it(`should throw given invalid app: ${ JSON.stringify(invalidApp) }`, () => { + describe('Storage App', () => { + beforeEach(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + storage = new Storage(mockApp); + }); + + afterEach(() => { + return mockApp.delete(); + }); + + describe('Constructor', () => { + const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidApps.forEach((invalidApp) => { + it(`should throw given invalid app: ${ JSON.stringify(invalidApp) }`, () => { + expect(() => { + const storageAny: any = Storage; + return new storageAny(invalidApp); + }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); + }); + }); + + it('should throw given no app', () => { expect(() => { const storageAny: any = Storage; - return new storageAny(invalidApp); + return new storageAny(); }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); }); + + it('should throw given invalid credential', () => { + const expectedError = 'Failed to initialize Google Cloud Storage client with the available ' + + 'credential. Must initialize the SDK with a certificate credential or application default ' + + 'credentials to use Cloud Storage API.'; + expect(() => { + const storageAny: any = Storage; + return new storageAny(mockCredentialApp); + }).to.throw(expectedError); + }); + + it('should not throw given a valid app', () => { + expect(() => { + return new Storage(mockApp); + }).not.to.throw(); + }); }); - - it('should throw given no app', () => { - expect(() => { - const storageAny: any = Storage; - return new storageAny(); - }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); - }); - - it('should throw given invalid credential', () => { - const expectedError = 'Failed to initialize Google Cloud Storage client with the available ' + - 'credential. Must initialize the SDK with a certificate credential or application default ' + - 'credentials to use Cloud Storage API.'; - expect(() => { - const storageAny: any = Storage; - return new storageAny(mockCredentialApp); - }).to.throw(expectedError); - }); - - it('should not throw given a valid app', () => { - expect(() => { - return new Storage(mockApp); - }).not.to.throw(); + + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(storage.app).to.equal(mockApp); + }); + + it('is read-only', () => { + expect(() => { + (storage as any).app = mockApp; + }).to.throw('Cannot set property app of # which has only a getter'); + }); }); - }); - - describe('app', () => { - it('returns the app from the constructor', () => { - // We expect referential equality here - expect(storage.app).to.equal(mockApp); + + describe('bucket(invalid)', () => { + const expectedError = 'Bucket name not specified or invalid. Specify a valid bucket name via ' + + 'the storageBucket option when initializing the app, or specify the bucket name ' + + 'explicitly when calling the getBucket() method.'; + const invalidNames = [null, NaN, 0, 1, true, false, '', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidNames.forEach((invalidName) => { + it(`should throw given invalid bucket name: ${ JSON.stringify(invalidName) }`, () => { + expect(() => { + const bucketAny: any = storage.bucket; + bucketAny(invalidName); + }).to.throw(expectedError); + }); + }); }); - - it('is read-only', () => { - expect(() => { - (storage as any).app = mockApp; - }).to.throw('Cannot set property app of # which has only a getter'); + + describe('bucket()', () => { + it('should return a bucket object', () => { + expect(storage.bucket().name).to.equal('bucketName.appspot.com'); + }); }); - }); - - describe('bucket(invalid)', () => { - const expectedError = 'Bucket name not specified or invalid. Specify a valid bucket name via ' + - 'the storageBucket option when initializing the app, or specify the bucket name ' + - 'explicitly when calling the getBucket() method.'; - const invalidNames = [null, NaN, 0, 1, true, false, '', [], [1, 'a'], {}, { a: 1 }, _.noop]; - invalidNames.forEach((invalidName) => { - it(`should throw given invalid bucket name: ${ JSON.stringify(invalidName) }`, () => { - expect(() => { - const bucketAny: any = storage.bucket; - bucketAny(invalidName); - }).to.throw(expectedError); + + describe('bucket(valid)', () => { + it('should return a bucket object', () => { + expect(storage.bucket('foo').name).to.equal('foo'); }); }); }); - describe('bucket()', () => { - it('should return a bucket object', () => { - expect(storage.bucket().name).to.equal('bucketName.appspot.com'); - }); - }); + describe('Emulator Support', () => { + it('sets STORAGE_EMULATOR_HOST if FIREBASE_STORAGE_EMULATOR_HOST is set', () => { + const EMULATOR_HOST = "http://localhost:9199"; + delete process.env.STORAGE_EMULATOR_HOST; + process.env.FIREBASE_STORAGE_EMULATOR_HOST = EMULATOR_HOST; + + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + storage = new Storage(mockApp); - describe('bucket(valid)', () => { - it('should return a bucket object', () => { - expect(storage.bucket('foo').name).to.equal('foo'); + expect(process.env.STORAGE_EMULATOR_HOST).to.equal(EMULATOR_HOST); }); - }); + + afterEach(() => { + delete process.env.STORAGE_EMULATOR_HOST; + delete process.env.FIREBASE_STORAGE_EMULATOR_HOST; + return mockApp.delete(); + }); + }) }); From bdca9fa6e26f97891f9f77ae5d081c847013d5de Mon Sep 17 00:00:00 2001 From: Samuel Bushi Date: Thu, 11 Mar 2021 16:33:18 +0000 Subject: [PATCH 4/7] Lint fix --- test/unit/storage/storage.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/storage/storage.spec.ts b/test/unit/storage/storage.spec.ts index c7f8dac1e3..49e976d6f1 100644 --- a/test/unit/storage/storage.spec.ts +++ b/test/unit/storage/storage.spec.ts @@ -118,7 +118,7 @@ describe('Storage', () => { describe('Emulator Support', () => { it('sets STORAGE_EMULATOR_HOST if FIREBASE_STORAGE_EMULATOR_HOST is set', () => { - const EMULATOR_HOST = "http://localhost:9199"; + const EMULATOR_HOST = 'http://localhost:9199'; delete process.env.STORAGE_EMULATOR_HOST; process.env.FIREBASE_STORAGE_EMULATOR_HOST = EMULATOR_HOST; From 9ba17bc11fd097024b92c3dca8f9bc6e45d8ccea Mon Sep 17 00:00:00 2001 From: Samuel Bushi Date: Mon, 15 Mar 2021 17:23:19 +0000 Subject: [PATCH 5/7] Minor fixes to storage tests --- test/unit/storage/storage.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/storage/storage.spec.ts b/test/unit/storage/storage.spec.ts index 49e976d6f1..1552427a77 100644 --- a/test/unit/storage/storage.spec.ts +++ b/test/unit/storage/storage.spec.ts @@ -123,7 +123,6 @@ describe('Storage', () => { process.env.FIREBASE_STORAGE_EMULATOR_HOST = EMULATOR_HOST; mockApp = mocks.app(); - mockCredentialApp = mocks.mockCredentialApp(); storage = new Storage(mockApp); expect(process.env.STORAGE_EMULATOR_HOST).to.equal(EMULATOR_HOST); @@ -132,7 +131,6 @@ describe('Storage', () => { afterEach(() => { delete process.env.STORAGE_EMULATOR_HOST; delete process.env.FIREBASE_STORAGE_EMULATOR_HOST; - return mockApp.delete(); }); }) }); From 5c7956b3ab62d8e3eaa6d1ff97fb23d767349721 Mon Sep 17 00:00:00 2001 From: Samuel Bushi Date: Thu, 8 Apr 2021 13:22:15 +0000 Subject: [PATCH 6/7] Address review comments --- test/unit/storage/storage.spec.ts | 162 +++++++++++++++--------------- 1 file changed, 80 insertions(+), 82 deletions(-) diff --git a/test/unit/storage/storage.spec.ts b/test/unit/storage/storage.spec.ts index 1552427a77..99f55f6ab1 100644 --- a/test/unit/storage/storage.spec.ts +++ b/test/unit/storage/storage.spec.ts @@ -29,106 +29,104 @@ describe('Storage', () => { let mockCredentialApp: FirebaseApp; let storage: Storage; - describe('Storage App', () => { - beforeEach(() => { - mockApp = mocks.app(); - mockCredentialApp = mocks.mockCredentialApp(); - storage = new Storage(mockApp); - }); - - afterEach(() => { - return mockApp.delete(); - }); - - describe('Constructor', () => { - const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; - invalidApps.forEach((invalidApp) => { - it(`should throw given invalid app: ${ JSON.stringify(invalidApp) }`, () => { - expect(() => { - const storageAny: any = Storage; - return new storageAny(invalidApp); - }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); - }); - }); - - it('should throw given no app', () => { + beforeEach(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + storage = new Storage(mockApp); + }); + + afterEach(() => { + return mockApp.delete(); + }); + + describe('Constructor', () => { + const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidApps.forEach((invalidApp) => { + it(`should throw given invalid app: ${ JSON.stringify(invalidApp) }`, () => { expect(() => { const storageAny: any = Storage; - return new storageAny(); + return new storageAny(invalidApp); }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); }); - - it('should throw given invalid credential', () => { - const expectedError = 'Failed to initialize Google Cloud Storage client with the available ' + - 'credential. Must initialize the SDK with a certificate credential or application default ' + - 'credentials to use Cloud Storage API.'; - expect(() => { - const storageAny: any = Storage; - return new storageAny(mockCredentialApp); - }).to.throw(expectedError); - }); - - it('should not throw given a valid app', () => { - expect(() => { - return new Storage(mockApp); - }).not.to.throw(); - }); }); - - describe('app', () => { - it('returns the app from the constructor', () => { - // We expect referential equality here - expect(storage.app).to.equal(mockApp); - }); - - it('is read-only', () => { - expect(() => { - (storage as any).app = mockApp; - }).to.throw('Cannot set property app of # which has only a getter'); - }); + + it('should throw given no app', () => { + expect(() => { + const storageAny: any = Storage; + return new storageAny(); + }).to.throw('First argument passed to admin.storage() must be a valid Firebase app instance.'); }); - - describe('bucket(invalid)', () => { - const expectedError = 'Bucket name not specified or invalid. Specify a valid bucket name via ' + - 'the storageBucket option when initializing the app, or specify the bucket name ' + - 'explicitly when calling the getBucket() method.'; - const invalidNames = [null, NaN, 0, 1, true, false, '', [], [1, 'a'], {}, { a: 1 }, _.noop]; - invalidNames.forEach((invalidName) => { - it(`should throw given invalid bucket name: ${ JSON.stringify(invalidName) }`, () => { - expect(() => { - const bucketAny: any = storage.bucket; - bucketAny(invalidName); - }).to.throw(expectedError); - }); - }); + + it('should throw given invalid credential', () => { + const expectedError = 'Failed to initialize Google Cloud Storage client with the available ' + + 'credential. Must initialize the SDK with a certificate credential or application default ' + + 'credentials to use Cloud Storage API.'; + expect(() => { + const storageAny: any = Storage; + return new storageAny(mockCredentialApp); + }).to.throw(expectedError); }); - - describe('bucket()', () => { - it('should return a bucket object', () => { - expect(storage.bucket().name).to.equal('bucketName.appspot.com'); - }); + + it('should not throw given a valid app', () => { + expect(() => { + return new Storage(mockApp); + }).not.to.throw(); }); - - describe('bucket(valid)', () => { - it('should return a bucket object', () => { - expect(storage.bucket('foo').name).to.equal('foo'); + }); + + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(storage.app).to.equal(mockApp); + }); + + it('is read-only', () => { + expect(() => { + (storage as any).app = mockApp; + }).to.throw('Cannot set property app of # which has only a getter'); + }); + }); + + describe('bucket(invalid)', () => { + const expectedError = 'Bucket name not specified or invalid. Specify a valid bucket name via ' + + 'the storageBucket option when initializing the app, or specify the bucket name ' + + 'explicitly when calling the getBucket() method.'; + const invalidNames = [null, NaN, 0, 1, true, false, '', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidNames.forEach((invalidName) => { + it(`should throw given invalid bucket name: ${ JSON.stringify(invalidName) }`, () => { + expect(() => { + const bucketAny: any = storage.bucket; + bucketAny(invalidName); + }).to.throw(expectedError); }); }); }); - describe('Emulator Support', () => { - it('sets STORAGE_EMULATOR_HOST if FIREBASE_STORAGE_EMULATOR_HOST is set', () => { - const EMULATOR_HOST = 'http://localhost:9199'; + describe('bucket()', () => { + it('should return a bucket object', () => { + expect(storage.bucket().name).to.equal('bucketName.appspot.com'); + }); + }); + + describe('bucket(valid)', () => { + it('should return a bucket object', () => { + expect(storage.bucket('foo').name).to.equal('foo'); + }); + }); + + describe('Emulator mode', () => { + const EMULATOR_HOST = 'http://localhost:9199'; + + before(() => { delete process.env.STORAGE_EMULATOR_HOST; process.env.FIREBASE_STORAGE_EMULATOR_HOST = EMULATOR_HOST; + }); - mockApp = mocks.app(); - storage = new Storage(mockApp); - + it('sets STORAGE_EMULATOR_HOST if FIREBASE_STORAGE_EMULATOR_HOST is set', () => { expect(process.env.STORAGE_EMULATOR_HOST).to.equal(EMULATOR_HOST); }); - afterEach(() => { + after(() => { delete process.env.STORAGE_EMULATOR_HOST; delete process.env.FIREBASE_STORAGE_EMULATOR_HOST; }); From 6db3e7956b9cfbeded667b701b1d1b6fc96343a3 Mon Sep 17 00:00:00 2001 From: Samuel Bushi Date: Thu, 8 Apr 2021 18:23:09 +0000 Subject: [PATCH 7/7] Address review suggestion --- test/unit/storage/storage.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/storage/storage.spec.ts b/test/unit/storage/storage.spec.ts index 99f55f6ab1..69ea28b217 100644 --- a/test/unit/storage/storage.spec.ts +++ b/test/unit/storage/storage.spec.ts @@ -123,6 +123,8 @@ describe('Storage', () => { }); it('sets STORAGE_EMULATOR_HOST if FIREBASE_STORAGE_EMULATOR_HOST is set', () => { + new Storage(mockApp); + expect(process.env.STORAGE_EMULATOR_HOST).to.equal(EMULATOR_HOST); });