From 1c49225643a6be1024fd980ba3aac1593cd37469 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 8 Nov 2023 19:15:52 -0500 Subject: [PATCH 01/13] added undici messaging integration test --- integration/messaging/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/messaging/package.json b/integration/messaging/package.json index 2d19ff133c0..36720151f53 100644 --- a/integration/messaging/package.json +++ b/integration/messaging/package.json @@ -15,7 +15,7 @@ "express": "4.18.2", "geckodriver": "2.0.4", "mocha": "9.2.2", - "node-fetch": "2.6.7", + "undici": "5.26.5", "selenium-assistant": "6.1.1" } } From b80eced1a3e567c8afda8ee67c48aa7d58d024f5 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 8 Nov 2023 20:28:00 -0500 Subject: [PATCH 02/13] messaging itest source uses undici --- integration/messaging/test/utils/sendMessage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/messaging/test/utils/sendMessage.js b/integration/messaging/test/utils/sendMessage.js index 9d3b93986f1..1d2e95054eb 100644 --- a/integration/messaging/test/utils/sendMessage.js +++ b/integration/messaging/test/utils/sendMessage.js @@ -15,7 +15,7 @@ * limitations under the License. */ -const fetch = require('node-fetch'); +const undici = require('undici'); const FCM_SEND_ENDPOINT = 'https://fcm.googleapis.com/fcm/send'; // Rotatable fcm server key. It's generally a bad idea to expose server keys. The reason is to // simplify testing process (no need to implement server side decryption of git secret). The @@ -28,7 +28,7 @@ module.exports = async payload => { 'Requesting to send an FCM message with payload: ' + JSON.stringify(payload) ); - const response = await fetch(FCM_SEND_ENDPOINT, { + const response = await undici.fetch(FCM_SEND_ENDPOINT, { method: 'POST', body: JSON.stringify(payload), headers: { From acd536e92c0598d10535e0b1b87cd7666959337a Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 8 Nov 2023 20:28:47 -0500 Subject: [PATCH 03/13] rules-unit-testing util uses undici --- packages/rules-unit-testing/package.json | 1 + packages/rules-unit-testing/src/util.ts | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/rules-unit-testing/package.json b/packages/rules-unit-testing/package.json index a3d84cd4f36..c5bf7a77811 100644 --- a/packages/rules-unit-testing/package.json +++ b/packages/rules-unit-testing/package.json @@ -54,6 +54,7 @@ "url": "https://github.com/firebase/firebase-js-sdk/issues" }, "dependencies": { + "undici": "5.26.5", "node-fetch": "2.6.7", "@types/node-fetch": "2.6.4" } diff --git a/packages/rules-unit-testing/src/util.ts b/packages/rules-unit-testing/src/util.ts index 3fff44a066b..65e5a9ed635 100644 --- a/packages/rules-unit-testing/src/util.ts +++ b/packages/rules-unit-testing/src/util.ts @@ -21,7 +21,7 @@ import { } from './impl/discovery'; import { fixHostname, makeUrl } from './impl/url'; import { HostAndPort } from './public_types'; -import fetch from 'node-fetch'; +import { fetch as undiciFetch } from 'undici'; /** * Run a setup function with background Cloud Functions triggers disabled. This can be used to @@ -79,7 +79,7 @@ export async function withFunctionTriggersDisabled( hub.host = fixHostname(hub.host); makeUrl(hub, '/functions/disableBackgroundTriggers'); // Disable background triggers - const disableRes = await fetch( + const disableRes = await undiciFetch( makeUrl(hub, '/functions/disableBackgroundTriggers'), { method: 'PUT' @@ -98,7 +98,7 @@ export async function withFunctionTriggersDisabled( result = await maybeFn(); } finally { // Re-enable background triggers - const enableRes = await fetch( + const enableRes = await undiciFetch( makeUrl(hub, '/functions/enableBackgroundTriggers'), { method: 'PUT' From 086aaabd8e2277b47326bad79bc1aae70af48579 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Wed, 8 Nov 2023 21:25:22 -0500 Subject: [PATCH 04/13] spy undici fetch instead of fetch default --- packages/rules-unit-testing/src/util.ts | 1 + packages/rules-unit-testing/test/util.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/rules-unit-testing/src/util.ts b/packages/rules-unit-testing/src/util.ts index 65e5a9ed635..568ed48db47 100644 --- a/packages/rules-unit-testing/src/util.ts +++ b/packages/rules-unit-testing/src/util.ts @@ -21,6 +21,7 @@ import { } from './impl/discovery'; import { fixHostname, makeUrl } from './impl/url'; import { HostAndPort } from './public_types'; +import fetch from 'node-fetch'; import { fetch as undiciFetch } from 'undici'; /** diff --git a/packages/rules-unit-testing/test/util.test.ts b/packages/rules-unit-testing/test/util.test.ts index 9dc778fb93e..b81c6fa5142 100644 --- a/packages/rules-unit-testing/test/util.test.ts +++ b/packages/rules-unit-testing/test/util.test.ts @@ -165,7 +165,7 @@ describe('assertFails()', () => { describe('withFunctionTriggersDisabled()', () => { it('disabling function triggers does not throw, returns value', async function () { - const fetchSpy = sinon.spy(require('node-fetch'), 'default'); + const fetchSpy = sinon.spy(require('undici'), 'fetch'); const res = await withFunctionTriggersDisabled(() => { return Promise.resolve(1234); @@ -176,7 +176,7 @@ describe('withFunctionTriggersDisabled()', () => { }); it('disabling function triggers always re-enables, event when the function throws', async function () { - const fetchSpy = sinon.spy(require('node-fetch'), 'default'); + const fetchSpy = sinon.spy(require('undici'), 'fetch'); const res = withFunctionTriggersDisabled(() => { throw new Error('I throw!'); From 841f07ee45259dd4b3bf7330c2118fc4abe077e1 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 09:40:50 -0500 Subject: [PATCH 05/13] rules and util undicfetchified --- packages/rules-unit-testing/src/impl/rules.ts | 8 ++++---- packages/rules-unit-testing/src/util.ts | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/rules-unit-testing/src/impl/rules.ts b/packages/rules-unit-testing/src/impl/rules.ts index 7d4d900367f..ef96f168012 100644 --- a/packages/rules-unit-testing/src/impl/rules.ts +++ b/packages/rules-unit-testing/src/impl/rules.ts @@ -17,7 +17,7 @@ import { HostAndPort } from '../public_types'; import { makeUrl } from './url'; -import fetch from 'node-fetch'; +import { fetch as undiciFetch } from 'undici'; /** * @private @@ -29,7 +29,7 @@ export async function loadDatabaseRules( ): Promise { const url = makeUrl(hostAndPort, '/.settings/rules.json'); url.searchParams.append('ns', databaseName); - const resp = await fetch(url, { + const resp = await undiciFetch(url, { method: 'PUT', headers: { Authorization: 'Bearer owner' }, body: rules @@ -48,7 +48,7 @@ export async function loadFirestoreRules( projectId: string, rules: string ): Promise { - const resp = await fetch( + const resp = await undiciFetch( makeUrl(hostAndPort, `/emulator/v1/projects/${projectId}:securityRules`), { method: 'PUT', @@ -72,7 +72,7 @@ export async function loadStorageRules( hostAndPort: HostAndPort, rules: string ): Promise { - const resp = await fetch(makeUrl(hostAndPort, '/internal/setRules'), { + const resp = await undiciFetch(makeUrl(hostAndPort, '/internal/setRules'), { method: 'PUT', headers: { 'Content-Type': 'application/json' diff --git a/packages/rules-unit-testing/src/util.ts b/packages/rules-unit-testing/src/util.ts index 568ed48db47..65e5a9ed635 100644 --- a/packages/rules-unit-testing/src/util.ts +++ b/packages/rules-unit-testing/src/util.ts @@ -21,7 +21,6 @@ import { } from './impl/discovery'; import { fixHostname, makeUrl } from './impl/url'; import { HostAndPort } from './public_types'; -import fetch from 'node-fetch'; import { fetch as undiciFetch } from 'undici'; /** From 9f6cdfb6b2ea0b307feab5da789a9bc84b27c206 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 10:22:35 -0500 Subject: [PATCH 06/13] remove fetch-specific check in storage type.ts --- packages/storage/src/implementation/type.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/storage/src/implementation/type.ts b/packages/storage/src/implementation/type.ts index 33fb7bb227d..0e24d10ad93 100644 --- a/packages/storage/src/implementation/type.ts +++ b/packages/storage/src/implementation/type.ts @@ -40,9 +40,7 @@ export function isNativeBlob(p: unknown): p is Blob { } export function isNativeBlobDefined(): boolean { - // Note: The `isNode()` check can be removed when `node-fetch` adds native Blob support - // PR: https://github.com/node-fetch/node-fetch/pull/1664 - return typeof Blob !== 'undefined' && !isNode(); + return typeof Blob !== 'undefined'; } export function validateNumber( From bdcfe09d54d49220ea9ce7041a7560babf8e50d7 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 11:26:18 -0500 Subject: [PATCH 07/13] storage lint fix --- packages/storage/src/implementation/type.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/storage/src/implementation/type.ts b/packages/storage/src/implementation/type.ts index 0e24d10ad93..7d5bc0e19e5 100644 --- a/packages/storage/src/implementation/type.ts +++ b/packages/storage/src/implementation/type.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { isNode } from '@firebase/util'; import { invalidArgument } from './error'; export function isJustDef(p: T | null | undefined): p is T | null { From 87ed6c4609c2826956780ed013726d8c554f1e43 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 11:39:52 -0500 Subject: [PATCH 08/13] update emulator scripts to use undici --- scripts/emulator-testing/emulators/emulator.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/emulator-testing/emulators/emulator.ts b/scripts/emulator-testing/emulators/emulator.ts index d56226842eb..5175bb32b73 100644 --- a/scripts/emulator-testing/emulators/emulator.ts +++ b/scripts/emulator-testing/emulators/emulator.ts @@ -21,7 +21,7 @@ import { ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import fetch from 'node-fetch'; +import { fetch as undiciFetch} from 'undici'; // @ts-ignore import * as tmp from 'tmp'; @@ -57,7 +57,7 @@ export abstract class Emulator { const writeStream: fs.WriteStream = fs.createWriteStream(filepath); console.log(`Downloading emulator from [${this.binaryUrl}] ...`); - fetch(this.binaryUrl).then(resp => { + undiciFetch(this.binaryUrl).then(resp => { resp.body .pipe(writeStream) .on('finish', () => { @@ -120,7 +120,7 @@ export abstract class Emulator { reject(`Emulator not ready after ${timeout}s. Exiting ...`); } else { console.log(`Ping emulator at [http://localhost:${this.port}] ...`); - fetch(`http://localhost:${this.port}`).then( + undiciFetch(`http://localhost:${this.port}`).then( () => { // Database and Firestore emulators will return 400 and 200 respectively. // As long as we get a response back, it means the emulator is ready. From 0c747a099f0b4db484cf2af650d6aa0410ce819b Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 17:30:50 -0500 Subject: [PATCH 09/13] add write stream for response object --- .../emulator-testing/emulators/emulator.ts | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/scripts/emulator-testing/emulators/emulator.ts b/scripts/emulator-testing/emulators/emulator.ts index 5175bb32b73..0a65160f19d 100644 --- a/scripts/emulator-testing/emulators/emulator.ts +++ b/scripts/emulator-testing/emulators/emulator.ts @@ -21,7 +21,7 @@ import { ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { fetch as undiciFetch} from 'undici'; +import { fetch as undiciFetch } from 'undici'; // @ts-ignore import * as tmp from 'tmp'; @@ -58,27 +58,26 @@ export abstract class Emulator { console.log(`Downloading emulator from [${this.binaryUrl}] ...`); undiciFetch(this.binaryUrl).then(resp => { - resp.body - .pipe(writeStream) - .on('finish', () => { - console.log(`Saved emulator binary file to [${filepath}].`); - // Change emulator binary file permission to 'rwxr-xr-x'. - // The execute permission is required for it to be able to start - // with 'java -jar'. - fs.chmod(filepath, 0o755, err => { - if (err) reject(err); - console.log( - `Changed emulator file permissions to 'rwxr-xr-x'.` - ); - this.binaryPath = filepath; - - if (this.copyToCache()) { - console.log(`Cached emulator at ${this.cacheBinaryPath}`); - } - resolve(); - }); - }) - .on('error', reject); + const body = resp.body(); + writeStream.write(body); + writeStream.end(); + + console.log(`Saved emulator binary file to [${filepath}].`); + // Change emulator binary file permission to 'rwxr-xr-x'. + // The execute permission is required for it to be able to start + // with 'java -jar'. + fs.chmod(filepath, 0o755, err => { + if (err) reject(err); + console.log( + `Changed emulator file permissions to 'rwxr-xr-x'.` + ); + this.binaryPath = filepath; + + if (this.copyToCache()) { + console.log(`Cached emulator at ${this.cacheBinaryPath}`); + } + resolve(); + }); }); }); }); From 1e0cbdf864e25650c485f54e5286fe957bc7e120 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 17:44:32 -0500 Subject: [PATCH 10/13] body member, not a method --- scripts/emulator-testing/emulators/emulator.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scripts/emulator-testing/emulators/emulator.ts b/scripts/emulator-testing/emulators/emulator.ts index 0a65160f19d..f66ee38dc6e 100644 --- a/scripts/emulator-testing/emulators/emulator.ts +++ b/scripts/emulator-testing/emulators/emulator.ts @@ -58,8 +58,7 @@ export abstract class Emulator { console.log(`Downloading emulator from [${this.binaryUrl}] ...`); undiciFetch(this.binaryUrl).then(resp => { - const body = resp.body(); - writeStream.write(body); + writeStream.write(resp.body); writeStream.end(); console.log(`Saved emulator binary file to [${filepath}].`); @@ -68,9 +67,7 @@ export abstract class Emulator { // with 'java -jar'. fs.chmod(filepath, 0o755, err => { if (err) reject(err); - console.log( - `Changed emulator file permissions to 'rwxr-xr-x'.` - ); + console.log(`Changed emulator file permissions to 'rwxr-xr-x'.`); this.binaryPath = filepath; if (this.copyToCache()) { From 1dcf6c01807d29402436580cce583a5f8bbbe858 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Thu, 9 Nov 2023 19:50:45 -0500 Subject: [PATCH 11/13] create writableStream --- .../emulator-testing/emulators/emulator.ts | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/scripts/emulator-testing/emulators/emulator.ts b/scripts/emulator-testing/emulators/emulator.ts index f66ee38dc6e..fd97a03e76a 100644 --- a/scripts/emulator-testing/emulators/emulator.ts +++ b/scripts/emulator-testing/emulators/emulator.ts @@ -55,26 +55,35 @@ export abstract class Emulator { console.log(`Created temporary directory at [${dir}].`); const filepath: string = path.resolve(dir, this.binaryName); const writeStream: fs.WriteStream = fs.createWriteStream(filepath); + const writableStream = new WritableStream({ + write(chunk) { + writeStream.write(chunk); + }, + }); console.log(`Downloading emulator from [${this.binaryUrl}] ...`); undiciFetch(this.binaryUrl).then(resp => { - writeStream.write(resp.body); - writeStream.end(); - - console.log(`Saved emulator binary file to [${filepath}].`); - // Change emulator binary file permission to 'rwxr-xr-x'. - // The execute permission is required for it to be able to start - // with 'java -jar'. - fs.chmod(filepath, 0o755, err => { - if (err) reject(err); - console.log(`Changed emulator file permissions to 'rwxr-xr-x'.`); - this.binaryPath = filepath; - - if (this.copyToCache()) { - console.log(`Cached emulator at ${this.cacheBinaryPath}`); - } - resolve(); - }); + resp.body + .pipeTo(writableStream) + .on('finish', () => { + console.log(`Saved emulator binary file to [${filepath}].`); + // Change emulator binary file permission to 'rwxr-xr-x'. + // The execute permission is required for it to be able to start + // with 'java -jar'. + fs.chmod(filepath, 0o755, err => { + if (err) reject(err); + console.log( + `Changed emulator file permissions to 'rwxr-xr-x'.` + ); + this.binaryPath = filepath; + + if (this.copyToCache()) { + console.log(`Cached emulator at ${this.cacheBinaryPath}`); + } + resolve(); + }); + }) + .on('error', reject); }); }); }); From b0f4c599d02cd87a6e063b8efcf7beb975fe8455 Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 10 Nov 2023 10:45:40 -0500 Subject: [PATCH 12/13] revert undici in emulator.ts --- scripts/emulator-testing/emulators/emulator.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/scripts/emulator-testing/emulators/emulator.ts b/scripts/emulator-testing/emulators/emulator.ts index fd97a03e76a..d56226842eb 100644 --- a/scripts/emulator-testing/emulators/emulator.ts +++ b/scripts/emulator-testing/emulators/emulator.ts @@ -21,7 +21,7 @@ import { ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { fetch as undiciFetch } from 'undici'; +import fetch from 'node-fetch'; // @ts-ignore import * as tmp from 'tmp'; @@ -55,16 +55,11 @@ export abstract class Emulator { console.log(`Created temporary directory at [${dir}].`); const filepath: string = path.resolve(dir, this.binaryName); const writeStream: fs.WriteStream = fs.createWriteStream(filepath); - const writableStream = new WritableStream({ - write(chunk) { - writeStream.write(chunk); - }, - }); console.log(`Downloading emulator from [${this.binaryUrl}] ...`); - undiciFetch(this.binaryUrl).then(resp => { + fetch(this.binaryUrl).then(resp => { resp.body - .pipeTo(writableStream) + .pipe(writeStream) .on('finish', () => { console.log(`Saved emulator binary file to [${filepath}].`); // Change emulator binary file permission to 'rwxr-xr-x'. @@ -125,7 +120,7 @@ export abstract class Emulator { reject(`Emulator not ready after ${timeout}s. Exiting ...`); } else { console.log(`Ping emulator at [http://localhost:${this.port}] ...`); - undiciFetch(`http://localhost:${this.port}`).then( + fetch(`http://localhost:${this.port}`).then( () => { // Database and Firestore emulators will return 400 and 200 respectively. // As long as we get a response back, it means the emulator is ready. From 8795dfd11a7b9fe78382d3347a4ce0deb5265cba Mon Sep 17 00:00:00 2001 From: DellaBitta Date: Fri, 10 Nov 2023 10:52:37 -0500 Subject: [PATCH 13/13] test_enviornment uses undici to clearFirestore --- packages/rules-unit-testing/src/impl/test_environment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rules-unit-testing/src/impl/test_environment.ts b/packages/rules-unit-testing/src/impl/test_environment.ts index 5b32e8776f4..ff4c5f29eed 100644 --- a/packages/rules-unit-testing/src/impl/test_environment.ts +++ b/packages/rules-unit-testing/src/impl/test_environment.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import fetch from 'node-fetch'; +import { fetch as undiciFetch } from 'undici'; import firebase from 'firebase/compat/app'; import 'firebase/compat/firestore'; import 'firebase/compat/database'; @@ -106,7 +106,7 @@ export class RulesTestEnvironmentImpl implements RulesTestEnvironment { this.checkNotDestroyed(); assertEmulatorRunning(this.emulators, 'firestore'); - const resp = await fetch( + const resp = await undiciFetch( makeUrl( this.emulators.firestore, `/emulator/v1/projects/${this.projectId}/databases/(default)/documents`