Skip to content
Open
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
11 changes: 2 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
},
"homepage": "https://github.com/firebase/firebase-functions-test#readme",
"dependencies": {
"@types/lodash": "^4.14.202",
"lodash": "^4.17.21",
"ts-deepmerge": "^8.0.0"
},
"devDependencies": {
Expand Down
18 changes: 8 additions & 10 deletions spec/main.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

import { expect } from 'chai';
import * as functions from 'firebase-functions/v1';
import { set } from 'lodash';

import { mockConfig, makeChange, wrap } from '../src/main';
import { _makeResourceName, _extractParams } from '../src/v1';
import { features } from '../src/features';
Expand All @@ -36,18 +34,18 @@ describe('main', () => {
describe('background functions', () => {
const constructBackgroundCF = (eventType?: string) => {
const cloudFunction = (input) => input;
set(cloudFunction, 'run', (data, context) => {
(cloudFunction as any).run = (data, context) => {
return { data, context };
});
set(cloudFunction, '__endpoint', {
};
(cloudFunction as any).__endpoint = {
eventTrigger: {
eventFilters: {
resource: 'ref/{wildcard}/nested/{anotherWildcard}',
},
eventType: eventType || 'event',
retry: false,
},
});
};
return cloudFunction as functions.CloudFunction<any>;
};

Expand Down Expand Up @@ -244,12 +242,12 @@ describe('main', () => {

before(() => {
const cloudFunction = (input) => input;
set(cloudFunction, 'run', (data, context) => {
(cloudFunction as any).run = (data, context) => {
return { data, context };
});
set(cloudFunction, '__endpoint', {
};
(cloudFunction as any).__endpoint = {
callableTrigger: {},
});
};
wrappedCF = wrap(cloudFunction as functions.CloudFunction<any>);
});

Expand Down
6 changes: 3 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
// SOFTWARE.

import { AppOptions } from 'firebase-admin';
import { merge } from 'lodash';

import { FirebaseFunctionsTest } from './lifecycle';
import { FeaturesList } from './features';
Expand All @@ -35,8 +34,9 @@ export = (
// Ensure other files get loaded after init function, since they load `firebase-functions`
// which will issue warning if process.env.FIREBASE_CONFIG is not yet set.
let features = require('./features').features;
features = merge({}, features, {
features = {
...features,
cleanup: () => test.cleanup(),
});
};
return features;
};
8 changes: 3 additions & 5 deletions src/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

import { isEmpty } from 'lodash';
import { AppOptions } from 'firebase-admin';
import { forEach } from 'lodash';

import { testApp } from './app';

Expand Down Expand Up @@ -53,7 +51,7 @@ export class FirebaseFunctionsTest {
CLOUD_RUNTIME_CONFIG: process.env.CLOUD_RUNTIME_CONFIG,
};

if (isEmpty(firebaseConfig)) {
if (!firebaseConfig || Object.keys(firebaseConfig).length === 0) {
process.env.FIREBASE_CONFIG = JSON.stringify({
databaseURL: 'https://not-a-project.firebaseio.com',
storageBucket: 'not-a-project.appspot.com',
Expand All @@ -72,13 +70,13 @@ export class FirebaseFunctionsTest {

/** Complete clean up tasks. */
cleanup() {
forEach(this._oldEnv, (val, varName) => {
for (const [varName, val] of Object.entries(this._oldEnv)) {
if (typeof val !== 'undefined') {
process.env[varName] = val;
} else {
delete process.env[varName];
}
});
}
testApp().deleteApp();
}
}
67 changes: 46 additions & 21 deletions src/providers/firestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,27 @@

import { Change } from 'firebase-functions/v1';
import { firestore, app } from 'firebase-admin';
import { has, get, isEmpty, isPlainObject, mapValues } from 'lodash';
import { inspect } from 'util';

function isPlainObject(value: any): boolean {
return (
typeof value === 'object' &&
value !== null &&
Object.getPrototypeOf(value) === Object.prototype
);
}
Comment on lines +26 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The custom isPlainObject implementation does not handle objects created with Object.create(null) (which have a null prototype). In Lodash, _.isPlainObject returns true for such objects.

To ensure full compatibility with Lodash's behavior and prevent unexpected serialization failures when users pass clean dictionary objects (created via Object.create(null)), update the helper to also check for a null prototype.

Suggested change
function isPlainObject(value: any): boolean {
return (
typeof value === 'object' &&
value !== null &&
Object.getPrototypeOf(value) === Object.prototype
);
}
function isPlainObject(value: any): boolean {
if (typeof value !== 'object' || value === null) {
return false;
}
const proto = Object.getPrototypeOf(value);
return proto === null || proto === Object.prototype;
}


function mapValues<T, U>(
obj: Record<string, T>,
fn: (val: T) => U
): Record<string, U> {
const res: Record<string, U> = {};
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
res[key] = fn(obj[key]);
}
}
return res;
}
Comment on lines +34 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since the goal of this PR is to leverage modern ES6+ features, we can simplify the mapValues helper. Using Object.entries() is cleaner, more idiomatic, and automatically avoids iterating over inherited prototype properties, eliminating the need for an explicit hasOwnProperty check.

function mapValues<T, U>(
  obj: Record<string, T>,
  fn: (val: T) => U
): Record<string, U> {
  const res: Record<string, U> = {};
  for (const [key, val] of Object.entries(obj)) {
    res[key] = fn(val);
  }
  return res;
}


import { testApp } from '../app';

Expand Down Expand Up @@ -76,30 +95,32 @@ export function makeDocumentSnapshot(
) {
let firestoreService;
let project;
if (has(options, 'app')) {
firestoreService = firestore(options.firebaseApp);
project = get(options, 'app.options.projectId');
const legacyApp = options?.firebaseApp || (options as any)?.app;
if (legacyApp) {
firestoreService = firestore(legacyApp);
project = legacyApp.options?.projectId;
} else {
firestoreService = firestore(testApp().getApp());
project = process.env.GCLOUD_PROJECT;
}

const resource = `projects/${project}/databases/(default)/documents/${refPath}`;
const proto = isEmpty(data)
? resource
: {
fields: objectToValueProto(data),
createTime: dateToTimestampProto(
get(options, 'createTime', new Date().toISOString())
),
updateTime: dateToTimestampProto(
get(options, 'updateTime', new Date().toISOString())
),
name: resource,
};
const proto =
!data || Object.keys(data).length === 0
? resource
: {
fields: objectToValueProto(data),
createTime: dateToTimestampProto(
options?.createTime ?? new Date().toISOString()
),
updateTime: dateToTimestampProto(
options?.updateTime ?? new Date().toISOString()
),
name: resource,
};

const readTimeProto = dateToTimestampProto(
get(options, 'readTime') || new Date().toISOString()
options?.readTime || new Date().toISOString()
);
return firestoreService.snapshot_(proto, readTimeProto, 'json');
}
Expand Down Expand Up @@ -206,8 +227,8 @@ export function objectToValueProto(data: object) {
};
}
if (val instanceof firestore.DocumentReference) {
const projectId: string = get(val, '_referencePath.projectId');
const database: string = get(val, '_referencePath.databaseId');
const projectId: string = (val as any)._referencePath?.projectId;
const database: string = (val as any)._referencePath?.databaseId;
const referenceValue: string = [
'projects',
projectId,
Expand Down Expand Up @@ -266,7 +287,11 @@ export function clearFirestoreData(options: { projectId: string } | string) {

if (typeof options === 'string') {
projectId = options;
} else if (typeof options === 'object' && has(options, 'projectId')) {
} else if (
typeof options === 'object' &&
options &&
'projectId' in options
) {
projectId = options.projectId;
} else {
throw new Error('projectId not specified');
Expand Down
26 changes: 17 additions & 9 deletions src/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

import { has, merge, random, get } from 'lodash';
import { merge } from 'ts-deepmerge';

function random(min: number, max: number): number {
return Math.floor(Math.random() * (max - min + 1)) + min;
}

import {
CloudFunction,
Expand Down Expand Up @@ -124,13 +128,16 @@ export function wrapV1<T>(
export function wrapV1<T>(
cloudFunction: CloudFunction<T>
): WrappedScheduledFunction | WrappedFunction<T, CloudFunction<T>> {
if (!has(cloudFunction, '__endpoint')) {
if (!cloudFunction || !('__endpoint' in cloudFunction)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If cloudFunction is a truthy primitive (e.g., a string or number passed in JavaScript), evaluating '__endpoint' in cloudFunction will throw a TypeError (e.g., Cannot use 'in' operator to search for '__endpoint' in ...) instead of throwing the intended descriptive error.

To make this check completely safe, ensure cloudFunction is an object or function before using the in operator.

Suggested change
if (!cloudFunction || !('__endpoint' in cloudFunction)) {
if (!cloudFunction || (typeof cloudFunction !== 'object' && typeof cloudFunction !== 'function') || !('__endpoint' in cloudFunction)) {

throw new Error(
'Wrap can only be called on functions written with the firebase-functions SDK.'
);
}

if (has(cloudFunction, '__endpoint.scheduleTrigger')) {
if (
cloudFunction?.__endpoint &&
'scheduleTrigger' in cloudFunction.__endpoint
) {
const scheduledWrapped: WrappedScheduledFunction = (
options: ContextOptions
) => {
Expand All @@ -147,19 +154,21 @@ export function wrapV1<T>(
return scheduledWrapped;
}

if (has(cloudFunction, '__endpoint.httpsTrigger')) {
if (cloudFunction?.__endpoint && 'httpsTrigger' in cloudFunction.__endpoint) {
throw new Error(
'Wrap function is only available for `onCall` HTTP functions, not `onRequest`.'
);
}

if (!has(cloudFunction, 'run')) {
if (!cloudFunction || !('run' in cloudFunction)) {
throw new Error(
'This library can only be used with functions written with firebase-functions v1.0.0 and above'
);
}

const isCallableFunction = has(cloudFunction, '__endpoint.callableTrigger');
const isCallableFunction =
!!cloudFunction?.__endpoint &&
'callableTrigger' in cloudFunction.__endpoint;

let wrapped: WrappedFunction<T, typeof cloudFunction> = (data, options) => {
// Although in Typescript we require `options` some of our JS samples do not pass it.
Expand All @@ -183,8 +192,7 @@ export function wrapV1<T>(
const defaultContext = _makeDefaultContext(cloudFunction, _options, data);

if (
has(defaultContext, 'eventType') &&
defaultContext.eventType !== undefined &&
defaultContext?.eventType &&
defaultContext.eventType.match(/firebase.database/)
) {
defaultContext.authType = 'UNAUTHENTICATED';
Expand All @@ -208,7 +216,7 @@ export function _makeResourceName(
const wildcardRegex = new RegExp('{[^/{}]*}', 'g');
let resourceName = resource.replace(wildcardRegex, (wildcard) => {
let wildcardNoBraces = wildcard.slice(1, -1); // .slice removes '{' and '}' from wildcard
let sub = get(params, wildcardNoBraces);
let sub = params?.[wildcardNoBraces];
return sub || wildcardNoBraces + random(1, 9);
});
return resourceName;
Expand Down
Loading