diff --git a/src/deploy/firestore/prepare.spec.ts b/src/deploy/firestore/prepare.spec.ts index 11e20a4ab10..9559ba047a2 100644 --- a/src/deploy/firestore/prepare.spec.ts +++ b/src/deploy/firestore/prepare.spec.ts @@ -14,6 +14,7 @@ describe("firestore prepare", () => { let sandbox: sinon.SinonSandbox; let getDatabaseStub: sinon.SinonStub; let createDatabaseStub: sinon.SinonStub; + let getFirestoreConfigStub: sinon.SinonStub; beforeEach(() => { sandbox = sinon.createSandbox(); @@ -23,7 +24,7 @@ describe("firestore prepare", () => { sandbox.stub(loadCJSON, "loadCJSON").returns({}); sandbox.stub(RulesDeploy.prototype, "addFile").returns(); sandbox.stub(RulesDeploy.prototype, "compile").resolves(); - sandbox.stub(fsConfig, "getFirestoreConfig").returns([ + getFirestoreConfigStub = sandbox.stub(fsConfig, "getFirestoreConfig").returns([ { database: "test-db", rules: "firestore.rules", @@ -36,6 +37,47 @@ describe("firestore prepare", () => { sandbox.restore(); }); + describe("database-scoped resource selection", () => { + const options = { + projectId: "test-project", + only: "firestore:test-db:rules", + config: { + path: (p: string) => p, + data: { + firestore: { + database: "test-db", + }, + }, + }, + } as unknown as Options; + + it("should prepare only rules for a database-scoped rules target", async () => { + getFirestoreConfigStub.returns([{ database: "test-db", rules: "firestore.rules" }]); + const context: Record = { projectId: "test-project" }; + + await prepare(context, options); + + expect(context["firestoreRules"]).to.be.true; + expect(context["firestoreIndexes"]).to.be.false; + const firestore = context["firestore"] as { rules: unknown[]; indexes: unknown[] }; + expect(firestore.rules).to.have.length(1); + expect(firestore.indexes).to.have.length(0); + }); + + it("should prepare only indexes for a database-scoped indexes target", async () => { + getFirestoreConfigStub.returns([{ database: "test-db", indexes: "firestore.indexes.json" }]); + const context: Record = { projectId: "test-project" }; + + await prepare(context, { ...options, only: "firestore:test-db:indexes" }); + + expect(context["firestoreRules"]).to.be.false; + expect(context["firestoreIndexes"]).to.be.true; + const firestore = context["firestore"] as { rules: unknown[]; indexes: unknown[] }; + expect(firestore.rules).to.have.length(0); + expect(firestore.indexes).to.have.length(1); + }); + }); + describe("createDatabase", () => { const projectId = "test-project"; const options = { diff --git a/src/deploy/firestore/prepare.ts b/src/deploy/firestore/prepare.ts index 8dee38dea71..d955e31714a 100644 --- a/src/deploy/firestore/prepare.ts +++ b/src/deploy/firestore/prepare.ts @@ -151,32 +151,12 @@ async function createDatabase(context: any, options: Options): Promise { export default async function (context: any, options: DeployOptions): Promise { await ensure(context.projectId, firestoreOrigin(), "firestore"); await ensure(context.projectId, firestoreOrigin(), "firestore"); - if (options.only) { - const targets = options.only.split(","); - - // Used for edge case when deploying to a named database - // https://github.com/firebase/firebase-tools/pull/6129 - const excludeRules = targets.includes("firestore:indexes"); - const excludeIndexes = targets.includes("firestore:rules"); - - // Used for edge case when deploying --only firestore:rules,firestore:indexes - // https://github.com/firebase/firebase-tools/issues/6857 - const includeRules = targets.includes("firestore:rules"); - const includeIndexes = targets.includes("firestore:indexes"); - - const onlyFirestore = targets.includes("firestore"); - - context.firestoreIndexes = !excludeIndexes || includeIndexes || onlyFirestore; - context.firestoreRules = !excludeRules || includeRules || onlyFirestore; - } else { - context.firestoreIndexes = true; - context.firestoreRules = true; - } - const firestoreConfigs: fsConfig.ParsedFirestoreConfig[] = fsConfig.getFirestoreConfig( context.projectId, options, ); + context.firestoreIndexes = firestoreConfigs.some((config) => Boolean(config.indexes)); + context.firestoreRules = firestoreConfigs.some((config) => Boolean(config.rules)); if (!firestoreConfigs || firestoreConfigs.length === 0) { return; } diff --git a/src/firestore/fsConfig.spec.ts b/src/firestore/fsConfig.spec.ts index 0c3e5c438d9..5e0cbc5bf65 100644 --- a/src/firestore/fsConfig.spec.ts +++ b/src/firestore/fsConfig.spec.ts @@ -1,50 +1,95 @@ import { expect } from "chai"; +import { FirebaseError } from "../error"; +import { Options } from "../options"; import { getFirestoreConfig } from "./fsConfig"; -describe("getFirestoreConfig", () => { - it("should return all configs when firestore:indexes is specified in only", () => { - const options: any = { - config: { - src: { - firestore: [ - { database: "(default)", rules: "firestore.rules" }, - { database: "second", rules: "firestore.second.rules" }, - ], - }, - }, - rc: { - requireTarget: () => { - return; - }, - target: () => [], +function makeOptions(only: string): Options { + return { + config: { + src: { + firestore: [ + { database: "(default)", rules: "firestore.rules", indexes: "firestore.indexes.json" }, + { + database: "second", + rules: "firestore.second.rules", + indexes: "firestore.second.indexes.json", + }, + ], }, - only: "firestore:indexes", - }; + }, + rc: { + requireTarget: () => undefined, + target: () => [], + }, + only, + } as unknown as Options; +} - const result = getFirestoreConfig("project", options); +describe("getFirestoreConfig", () => { + it("should return all configs with only indexes when firestore:indexes is specified", () => { + const result = getFirestoreConfig("project", makeOptions("firestore:indexes")); expect(result).to.have.length(2); + expect(result.map((config) => config.indexes)).to.deep.equal([ + "firestore.indexes.json", + "firestore.second.indexes.json", + ]); + expect(result.every((config) => config.rules === undefined)).to.be.true; }); - it("should return all configs when firestore:rules is specified in only", () => { - const options: any = { - config: { - src: { - firestore: [ - { database: "(default)", rules: "firestore.rules" }, - { database: "second", rules: "firestore.second.rules" }, - ], - }, + it("should return all configs with only rules when firestore:rules is specified", () => { + const result = getFirestoreConfig("project", makeOptions("firestore:rules")); + expect(result).to.have.length(2); + expect(result.map((config) => config.rules)).to.deep.equal([ + "firestore.rules", + "firestore.second.rules", + ]); + expect(result.every((config) => config.indexes === undefined)).to.be.true; + }); + + it("should return both resources for a database-scoped target", () => { + const result = getFirestoreConfig("project", makeOptions("firestore:second")); + expect(result).to.deep.equal([ + { + database: "second", + rules: "firestore.second.rules", + indexes: "firestore.second.indexes.json", }, - rc: { - requireTarget: () => { - return; - }, - target: () => [], + ]); + }); + + it("should select only rules for a database-scoped target", () => { + const result = getFirestoreConfig("project", makeOptions("firestore:second:rules")); + expect(result).to.deep.equal([ + { database: "second", rules: "firestore.second.rules", indexes: undefined }, + ]); + }); + + it("should select only indexes for a database-scoped target", () => { + const result = getFirestoreConfig("project", makeOptions("firestore:second:indexes")); + expect(result).to.deep.equal([ + { database: "second", rules: undefined, indexes: "firestore.second.indexes.json" }, + ]); + }); + + it("should combine global and database-scoped resource selections", () => { + const result = getFirestoreConfig( + "project", + makeOptions("firestore:rules,firestore:second:indexes"), + ); + expect(result).to.deep.equal([ + { database: "(default)", rules: "firestore.rules", indexes: undefined }, + { + database: "second", + rules: "firestore.second.rules", + indexes: "firestore.second.indexes.json", }, - only: "firestore:rules", - }; + ]); + }); - const result = getFirestoreConfig("project", options); - expect(result).to.have.length(2); + it("should reject unsupported database-scoped resources", () => { + expect(() => getFirestoreConfig("project", makeOptions("firestore:second:unknown"))).to.throw( + FirebaseError, + "Invalid Firestore deploy target: firestore:second:unknown", + ); }); }); diff --git a/src/firestore/fsConfig.ts b/src/firestore/fsConfig.ts index 96dd2b3b056..84699d1482d 100644 --- a/src/firestore/fsConfig.ts +++ b/src/firestore/fsConfig.ts @@ -8,6 +8,88 @@ export interface ParsedFirestoreConfig { indexes?: string; } +type FirestoreResource = "rules" | "indexes"; + +interface FirestoreSelection { + allDatabases: boolean; + allResources: Set; + byName: Map>; +} + +function parseFirestoreSelection(only: string | undefined): FirestoreSelection { + const selection: FirestoreSelection = { + allDatabases: !only, + allResources: new Set(["rules", "indexes"]), + byName: new Map(), + }; + if (!only) { + return selection; + } + + selection.allResources.clear(); + const addResources = (name: string | undefined, resources: FirestoreResource[]): void => { + const target = name + ? selection.byName.get(name) || new Set() + : selection.allResources; + for (const resource of resources) { + target.add(resource); + } + if (name) { + selection.byName.set(name, target); + } + }; + + for (const value of only.split(",")) { + if (value === "firestore") { + selection.allDatabases = true; + addResources(undefined, ["rules", "indexes"]); + continue; + } + if (!value.startsWith("firestore:")) { + continue; + } + + const parts = value.split(":"); + const name = parts[1]; + const resource = parts[2]; + if ( + !name || + parts.length > 3 || + (parts.length === 3 && !resource) || + (resource && resource !== "rules" && resource !== "indexes") + ) { + throw new FirebaseError(`Invalid Firestore deploy target: ${value}`); + } + + if (name === "rules" || name === "indexes") { + if (resource) { + throw new FirebaseError(`Invalid Firestore deploy target: ${value}`); + } + selection.allDatabases = true; + addResources(undefined, [name as FirestoreResource]); + } else { + addResources(name, resource ? [resource as FirestoreResource] : ["rules", "indexes"]); + } + } + return selection; +} + +function filterFirestoreConfig( + config: ParsedFirestoreConfig, + resources: Set, +): ParsedFirestoreConfig { + return { + ...config, + rules: resources.has("rules") ? config.rules : undefined, + indexes: resources.has("indexes") ? config.indexes : undefined, + }; +} + +/** + * Returns Firestore configuration entries selected for deployment. + * @param projectId The Firebase project ID. + * @param options The CLI options, including any Firestore deploy target. + */ export function getFirestoreConfig(projectId: string, options: Options): ParsedFirestoreConfig[] { const fsConfig = options.config.src.firestore; if (fsConfig === undefined) { @@ -15,80 +97,72 @@ export function getFirestoreConfig(projectId: string, options: Options): ParsedF } const rc = options.rc; - let allDatabases = !options.only; - const onlyDatabases = new Set(); - if (options.only) { - const split = options.only.split(","); - if (split.includes("firestore")) { - allDatabases = true; - } else { - for (const value of split) { - if (value.startsWith("firestore:")) { - const target = value.split(":")[1]; - onlyDatabases.add(target); - } - } - } - } + const selection = parseFirestoreSelection(options.only); // single DB if (!Array.isArray(fsConfig)) { if (fsConfig) { // databaseId is (default) if none provided const databaseId = fsConfig.database || `(default)`; - return [{ rules: fsConfig.rules, indexes: fsConfig.indexes, database: databaseId }]; + const resources = selection.allDatabases + ? selection.allResources + : selection.byName.get(databaseId) || new Set(); + if (!selection.allDatabases && !selection.byName.has(databaseId)) { + return []; + } + return [ + filterFirestoreConfig( + { rules: fsConfig.rules, indexes: fsConfig.indexes, database: databaseId }, + resources, + ), + ]; } else { logger.debug("Possibly invalid database config: ", JSON.stringify(fsConfig)); return []; } } - // If user specifies firestore:rules or firestore:indexes make sure we don't throw an error if this doesn't match a database name - if (onlyDatabases.has("rules")) { - onlyDatabases.delete("rules"); - allDatabases = true; - } - if (onlyDatabases.has("indexes")) { - onlyDatabases.delete("indexes"); - allDatabases = true; - } - const results: ParsedFirestoreConfig[] = []; for (const c of fsConfig) { const { database, target } = c; if (target) { - if (allDatabases || onlyDatabases.has(target)) { + const targetSelected = selection.allDatabases || selection.byName.has(target); + if (targetSelected) { // Make sure the target exists (this will throw otherwise) rc.requireTarget(projectId, "firestore", target); // Get a list of firestore instances the target maps to const databases = rc.target(projectId, "firestore", target); for (const database of databases) { - results.push({ database, rules: c.rules, indexes: c.indexes }); + const resources = new Set(selection.allResources); + for (const resource of selection.byName.get(target) || []) { + resources.add(resource); + } + results.push( + filterFirestoreConfig({ database, rules: c.rules, indexes: c.indexes }, resources), + ); } - onlyDatabases.delete(target); } } else if (database) { - if (allDatabases || onlyDatabases.has(database)) { - results.push(c as ParsedFirestoreConfig); - onlyDatabases.delete(database); + const databaseSelected = selection.allDatabases || selection.byName.has(database); + if (databaseSelected) { + const resources = new Set(selection.allResources); + for (const resource of selection.byName.get(database) || []) { + resources.add(resource); + } + results.push(filterFirestoreConfig(c as ParsedFirestoreConfig, resources)); } } else { throw new FirebaseError('Must supply either "target" or "databaseId" in firestore config'); } } - // If user specifies firestore:rules or firestore:indexes make sure we don't throw an error if this doesn't match a database name - if (onlyDatabases.has("rules")) { - onlyDatabases.delete("rules"); - } - if (onlyDatabases.has("indexes")) { - onlyDatabases.delete("indexes"); - } - - if (!allDatabases && onlyDatabases.size !== 0) { + const unmatched = [...selection.byName.keys()].filter( + (name) => !fsConfig.some((config) => config.database === name || config.target === name), + ); + if (!selection.allDatabases && unmatched.length !== 0) { throw new FirebaseError( `Could not find configurations in firebase.json for the following database targets: ${[ - ...onlyDatabases, + ...unmatched, ].join(", ")}`, ); }