Skip to content

Commit

Permalink
Fixes #31: automatically unsubscribe listeners still active at Jest '…
Browse files Browse the repository at this point in the history
…afterAll'.
  • Loading branch information
akauppi committed Jul 7, 2021
1 parent 59269e2 commit 05b1eb9
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 77 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 7-Jul-21 (unpublished)

- **FIX**: Clean up remaining `doc` listeners, allowing Jest to return to OS prompt. Counteracts [Jest #11464](https://github.com/facebook/jest/issues/11464) Unfortunately no Firebase Admin SDK issue, though the root cause is likely there.
- **CHANGE**: Limiting API exposure of `DocumentReferenceLike` to only: `set`, `get`, `onSnapshot`.
- **CLEANUP**: Removed old (commented out) `eventually` code

## 30-Jun-21 (0.0.3-beta.4b)

- **BUG FIX**: Resolver fixed so `firestoreAdmin` works, from customer project.
Expand Down
2 changes: 1 addition & 1 deletion package/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"tag": "beta"
},
"peerDependencies": {
"jest": "^27.0.5"
"jest": "^27.0.6"
},
"dependencies": {
"firebase-admin": "^9.100.0-alpha.1",
Expand Down
55 changes: 2 additions & 53 deletions package/src/firestoreAdmin/dbAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { initializeApp } from 'firebase-admin/app' // "modular API" (in alpha

import {FIRESTORE_HOST, projectId} from '../config.js'

import { afterAll } from '@jest/globals'

/*
* All the exposed methods operate on this one Firestore Admin app. This hides emulator configuration from the rest.
*/
Expand All @@ -35,45 +37,6 @@ const dbAdmin = (_ => {
return db;
})();

/*** temporarily disabled
/_*
* Wait for a document to change so that predicate 'p' becomes true.
*
* If 'timeoutMs' is provided, times out after that time.
*
* Resolves with:
* object|null (the value of the object, or that it's not there) when the predicate holds
* false if timed out
*_/
function eventually(docPath, p, timeoutMs) { // (string, (object|null) => boolean, ms?) => Promise of object|null|undefined
// Undocumented: if 'p' omitted, just wait for any document
p = p || (o => !!o);
return new Promise( (resolve) => {
let unsub;
let timer;
// Note: JavaScript doesn't have cancellable Promises, but a promise can be timed out from within.
//
if (timeoutMs) {
timer = setTimeout( _ => {
resolve(undefined); // timed out // tbd. best API style for conveying timeout, still in flux
unsub();
}, timeoutMs);
}
unsub = dbAdmin.doc(docPath).onSnapshot( ss => {
const o = ss.exists ? ss.data() : null;
if (p(o)) {
if (timer) clearTimeout(timer);
resolve(o);
unsub();
}
});
})
}*/

/*
* Pre-heat the Admin SDK <-> Firebase Emulators listening connection.
*
Expand All @@ -92,19 +55,6 @@ function eventually(docPath, p, timeoutMs) { // (string, (object|null) => boo
* #contribute: test this, report to Firebase if you also think 300ms could be improved (say, to 100ms)
* ---
*/
/*** disabled
function listener_EXP(collPath) { // (string) => (string, ((object|null) => boolean)?)) => Promise of object|null
// Instantly unsubscribing doesn't matter - it still creates the connection ('firebase-admin' ; 'firebase-tools' 9.12.1)
//
const unsub = dbAdmin.doc(`${collPath}/...`).onSnapshot( ss => {} );
unsub();
return (docId, p, timeoutMs) => {
return eventually(`${collPath}/${docId}`, p, timeoutMs);
}
}***/

function preheat_EXP(collPath) { // (string) => (string, ((object|null) => boolean)?)) => ()

// Instantly unsubscribing doesn't matter - it still creates the connection ('firebase-admin' ; 'firebase-tools' 9.12.1)
Expand All @@ -115,7 +65,6 @@ function preheat_EXP(collPath) { // (string) => (string, ((object|null) => bo

export {
dbAdmin,
//eventually,

// EXPERIMENTAL
preheat_EXP
Expand Down
62 changes: 58 additions & 4 deletions package/src/firestoreAdmin/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
* Context:
* From tests
*/
import {dbAdmin, /*eventually,*/ preheat_EXP} from "./dbAdmin.js"
import { strict as assert } from 'assert'

import {dbAdmin, preheat_EXP} from "./dbAdmin.js"

import { afterAll } from '@jest/globals'

const autoUnsubs = new Set(); // Set of () => ()

/*
* Note: Just doing 'const { collection, doc } = dbAdmin' does not work.
Expand All @@ -13,14 +19,62 @@ function collection(path) {
return dbAdmin.collection(path);
}

function doc(path) {
return dbAdmin.doc(path);
/*
* Like Firebase Admin SDK's 'doc'.
*
* By restricting the API surface, we keep version updates or incompatibilities from leaking to test code.
* Also, this allows us to fix some known problems.
*/
function doc(docPath) { // (string) => DocumentReference like
const ref = dbAdmin.doc(docPath);

assert(ref.set && ref.get && ref.onSnapshot);

// Wrap us into the 'unsub' chain, so we can release resources if they are still listened to, once Jest times out.
//
function onSnapshot(handler) { // ((...) => ...) => () => Promise of () // returned function is 'unsub'
let unsub = ref.onSnapshot(handler);

autoUnsubs.add(unsub); // tbd. works?

return async () => { // wrapper around 'unsub'
if (!unsub) fail("'unsub' called twice")

assert(autoUnsubs.has(unsub));
autoUnsubs.delete(unsub);
await unsub();
unsub = null;
};
}

return {
get: () => ref.get(), // () => ...
set: (v) => ref.set(v), // (...) => Promise of ...
onSnapshot
}
}

/*
* Close any remaining listeners, when Jest is done.
*
* This counteracts what looks like a Firebase Admin SDK bug (incompatibility with Jest), where a remaining listener
* causes Jest not to be able to return to OS prompt.
*
* Reference:
* "Jest does not exit tests cleanly with Firebase Firestore, an older version does. [...]" (Jest issues)
* -> https://github.com/facebook/jest/issues/11464
*/
afterAll( async () => {

//console.log(`!!! ${ autoUnsubs.size } listeners still running (cleaning them up)`); // DEBUG

const proms = Array.from(autoUnsubs) .map( unsub => unsub() ); // Array of Promise of ()
await Promise.all(proms);
})

export {
collection,
doc,
//eventually,
//
preheat_EXP
}
33 changes: 14 additions & 19 deletions sample/test-fns/userInfo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ describe("userInfo shadowing", () => {
await collection("userInfo").doc("abc").set(william);

// Style 1:
// - 'waitForIt' only passes when the right kind of object is there.
// - 'docListener' only passes when the right kind of object is there.
//
//await expect( waitForIt("projects/1/userInfo/abc", shallowEqualsGen(william)) ).resolves.not.toThrow();
//await expect( docListener("projects/1/userInfo/abc", shallowEqualsGen(william)) ).resolves.toBeDefined();

// Style 2:
// - 'waitForIt' passes on first valid doc, checking is done outside of 'expect'.
// - 'eventually.doc' passes on first valid doc, checking is done outside of 'expect'.
//
await expect( waitForIt("projects/1/userInfo/abc") ).resolves.toContainObject(william);
await expect( docListener("projects/1/userInfo/abc") ).resolves.toContainObject(william);
});

test('Central user information is not distributed to a project where the user is not a member', async () => {
Expand All @@ -56,29 +56,24 @@ describe("userInfo shadowing", () => {
//await expect(prom).timesOut; // ..but with cancelling such a promise
});

/**
/*
* Generates a function that returns 'true' if the two objects have same values; shallow.
*_/
function shallowEqualsGen(o2) { // (obj) => (obj) => boolean
return o1 => Object.keys(o1).length === Object.keys(o2).length &&
Object.keys(o1).every(k => o1[k] === o2[k]);
}*/

/*
* Provide a Promise that resolves, if the right kind of change takes place in the watched doc.
*/
function waitForIt(docPath, predicate) { // (string, ((object) => any)? ) => Promise of {...Firestore document }
function shallowEqualsGen(o) { // (obj) => (obj) => boolean
return o1 => Object.keys(o1).length === Object.keys(o).length &&
Object.keys(o1).every(k => o1[k] === o[k]);
}

function docListener(docPath, filter) { // (string, ((object) => falsy|object)?) => Promise of object

return new Promise( (resolve) => {

const unsub = doc(docPath).onSnapshot( ss => {
const o = ss.exists ? ss.data() : null;
const o = ss.exists ? (filter || (o => o))( ss.data() ) : null;
if (!o) return;

if (!predicate || predicate(o)) {
resolve(o);
unsub();
}
resolve(o);
unsub();
});
});
}
Expand Down

0 comments on commit 05b1eb9

Please sign in to comment.