-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fixing some issues around handling multiple firestore DBs #9770
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @joehan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Firebase Firestore emulator by introducing comprehensive support for multiple Firestore databases. It addresses long-standing limitations and bugs, allowing developers to configure, emulate, and manage security rules for several Firestore instances within a single project. This change improves the development workflow for applications utilizing multiple Firestore databases by making the emulator more robust and feature-complete. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request successfully addresses the issues related to handling multiple Firestore databases during emulation and deployment. The changes in src/emulator/controller.ts correctly adapt the rule processing logic to support multiple configurations, and the FirestoreEmulatorArgs and updateRules method in src/emulator/firestoreEmulator.ts have been updated to reflect this. The new test cases in src/firestore/fsConfig.spec.ts provide good coverage for the getFirestoreConfig function's behavior with firestore:rules and firestore:indexes filters. However, there are a couple of areas that need attention to ensure correctness and efficiency.
| for (const c of this.args.rules) { | ||
| const rulesPath = c.rules; | ||
| this.rulesWatcher = chokidar.watch(rulesPath, { persistent: true, ignoreInitial: true }); | ||
| this.rulesWatcher.on("change", async () => { | ||
| // There have been some race conditions reported (on Windows) where reading the | ||
| // file too quickly after the watcher fires results in an empty file being read. | ||
| // Adding a small delay prevents that at very little cost. | ||
| await new Promise((res) => setTimeout(res, 5)); | ||
|
|
||
| utils.logLabeledBullet( | ||
| "firestore", | ||
| `Change detected, updating rules for ${c.database}...`, | ||
| ); | ||
| const newContent = fs.readFileSync(rulesPath, "utf8").toString(); | ||
| const issues = await this.updateRules(c.database, newContent); | ||
| if (issues) { | ||
| for (const issue of issues) { | ||
| utils.logWarning(this.prettyPrintRulesIssue(rulesPath, issue)); | ||
| } | ||
| } | ||
| } | ||
| if (issues.some((issue) => issue.severity === Severity.ERROR)) { | ||
| utils.logWarning("Failed to update rules"); | ||
| } else { | ||
| utils.logLabeledSuccess("firestore", "Rules updated."); | ||
| } | ||
| }); | ||
| if (issues.some((issue) => issue.severity === Severity.ERROR)) { | ||
| utils.logWarning("Failed to update rules"); | ||
| } else { | ||
| utils.logLabeledSuccess("firestore", "Rules updated."); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The this.rulesWatcher is reassigned within the for...of loop. This means that only the chokidar.watch instance created for the last firestoreConfig in the this.args.rules array will be active. Consequently, changes to rule files for other databases will not be detected or trigger updates. To correctly watch all rule files, each chokidar.watch instance should be stored in a collection (e.g., an array of FSWatcher instances) and then iterated over when stopping the emulator.
const rulesWatchers: chokidar.FSWatcher[] = [];
for (const c of this.args.rules) {
const rulesPath = c.rules;
const watcher = chokidar.watch(rulesPath, { persistent: true, ignoreInitial: true });
watcher.on("change", async () => {
// There have been some race conditions reported (on Windows) where reading the
// file too quickly after the watcher fires results in an empty file being read.
// Adding a small delay prevents that at very little cost.
await new Promise((res) => setTimeout(res, 5));
utils.logLabeledBullet(
"firestore",
`Change detected, updating rules for ${c.database}...`
);
const newContent = fs.readFileSync(rulesPath, "utf8").toString();
const issues = await this.updateRules(c.database, newContent);
if (issues) {
for (const issue of issues) {
utils.logWarning(this.prettyPrintRulesIssue(rulesPath, issue));
}
}
if (issues.some((issue) => issue.severity === Severity.ERROR)) {
utils.logWarning("Failed to update rules");
} else {
utils.logLabeledSuccess("firestore", "Rules updated.");
}
});
rulesWatchers.push(watcher);
}
this.rulesWatcher = rulesWatchers; // Update the type of rulesWatcher to be FSWatcher[]
aalej
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't seem to run the Firestore emulators. Given this test case https://github.com/aalej/pulls-9770, running firebase emulators:start will raise
Jan 21, 2026 9:50:13 PM com.google.cloud.datastore.emulator.firestore.CloudFirestore main
SEVERE: Exiting due to unexpected exception.
java.io.FileNotFoundException: [object Object],[object Object] (No such file or directory)
at java.base/java.io.FileInputStream.open0(Native Method)
at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
at java.base/java.io.FileInputStream.<init>(FileInputStream.java:152)
at com.google.common.io.Files$FileByteSource.openStream(Files.java:140)
at com.google.common.io.Files$FileByteSource.read(Files.java:170)
at com.google.common.io.ByteSource$AsCharSource.read(ByteSource.java:534)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.init(CloudFirestore.java:186)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.startLocally(CloudFirestore.java:120)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.main(CloudFirestore.java:101)
Exception in thread "main" java.lang.NoClassDefFoundError: com/google/common/util/LegacySystemExit
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.main(CloudFirestore.java:106)
Caused by: java.lang.ClassNotFoundException: com.google.common.util.LegacySystemExit
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
... 1 more
| // TODO(VicVer09): b/269787702 | ||
| let rulesLocalPath; | ||
| let rulesFileFound; | ||
| const rules: { database: string; rules: string }[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the Firestore emulator binary accepts an array of objects for rules(I could be wrong though, I can't find any docs on how to use the binary).
Running the emulator with the code below(from what I can tell, this is what we do with the CLI):
minimal firestore emulator spawn code
const childProcess = require("node:child_process")
const BINARY_PATH = "/Users/alejandromarco/.cache/firebase/emulators/cloud-firestore-emulator-v1.20.2.jar"
let emulator = {}
emulator.instance = childProcess.spawn("java", [
'-Dgoogle.cloud_firestore.debug_log_level=FINE',
'-Duser.language=en',
'-jar',
BINARY_PATH,
'--host',
'127.0.0.1',
'--port',
8080,
'--websocket_port',
9150,
'--project_id',
'demo-project',
'--rules',
[
{
database: '(default)',
rules: '/Users/alejandromarco/Desktop/firebase-tools/issues/9770/firestore.rules'
},
{
database: 'non-default',
rules: '/Users/alejandromarco/Desktop/firebase-tools/issues/9770/firestore.non-default.rules'
}
],
'--single_project_mode',
true
])
emulator.instance.stderr?.on("data", (data) => {
console.log("DEBUG", data.toString());
});
emulator.instance.once("exit", async (code, signal) => {
if (signal) {
console.log(`Firestore emulator has exited upon receiving signal: ${signal}`);
} else if (code && code !== 0 && code !== /* SIGINT */ 130) {
console.log("Firestore emulator", `has exited with code: ${code}`);
}
});Will raise an error java.io.FileNotFoundException: [object Object],[object Object] (No such file or directory)
error output
$ node .
DEBUG Jan 21, 2026 10:19:04 PM com.google.cloud.datastore.emulator.firestore.CloudFirestore main
SEVERE: Exiting due to unexpected exception.
java.io.FileNotFoundException: [object Object],[object Object] (No such file or directory)
at java.base/java.io.FileInputStream.open0(Native Method)
at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
at java.base/java.io.FileInputStream.<init>(FileInputStream.java:152)
at com.google.common.io.Files$FileByteSource.openStream(Files.java:140)
at com.google.common.io.Files$FileByteSource.read(Files.java:170)
at com.google.common.io.ByteSource$AsCharSource.read(ByteSource.java:534)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.init(CloudFirestore.java:186)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.startLocally(CloudFirestore.java:120)
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.main(CloudFirestore.java:101)
DEBUG Exception in thread "main" java.lang.NoClassDefFoundError: com/google/common/util/LegacySystemExit
at com.google.cloud.datastore.emulator.firestore.CloudFirestore.main(CloudFirestore.java:106)
Caused by: java.lang.ClassNotFoundException: com.google.common.util.LegacySystemExit
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
... 1 more
Firestore emulator has exited with code: 1
Changing the code the use a string for rules makes the process run again
minimal firestore spawn code
const childProcess = require("node:child_process")
const BINARY_PATH = "/Users/alejandromarco/.cache/firebase/emulators/cloud-firestore-emulator-v1.20.2.jar"
let emulator = {}
emulator.instance = childProcess.spawn("java", [
'-Dgoogle.cloud_firestore.debug_log_level=FINE',
'-Duser.language=en',
'-jar',
BINARY_PATH,
'--host',
'127.0.0.1',
'--port',
8080,
'--websocket_port',
9150,
'--project_id',
'demo-project',
'--rules',
'/Users/alejandromarco/Desktop/firebase-tools/issues/9770/firestore.rules',
'--single_project_mode',
true
])
emulator.instance.stderr?.on("data", (data) => {
console.log("DEBUG", data.toString());
});
emulator.instance.once("exit", async (code, signal) => {
if (signal) {
console.log(`Firestore emulator has exited upon receiving signal: ${signal}`);
} else if (code && code !== 0 && code !== /* SIGINT */ 130) {
console.log("Firestore emulator", `has exited with code: ${code}`);
}
});outputs
log output
$ node working.js
DEBUG Jan 21, 2026 10:18:45 PM com.google.cloud.datastore.emulator.firestore.websocket.WebSocketServer start
INFO: Started WebSocket server on ws://127.0.0.1:9150
Description
Fixes some issues related to handling multiple Firestore DBs during emulation and deployment. Fixes #9742 and #8114