Skip to content

Commit

Permalink
Stop using AppEngine to check database mode (#5261)
Browse files Browse the repository at this point in the history
* Stop using AppEngine to check database mode

* fixing tests

* Removing unnecessary check for cloud resource location

* remove outdated test

* add changelog
  • Loading branch information
joehan authored Jan 26, 2023
1 parent 06b8bad commit d59b78b
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 23 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
- Refactor Functions Emulator. (#5422)
- Fix race condition when discovering functions. (#5444)
- Refactors Functions Emulator. (#5422)
- Fixes race condition when discovering functions. (#5444)
- Fixes issue where `init firestore` was unecessarilly checking for default resource location. (#5230 and #5452)
19 changes: 11 additions & 8 deletions src/firestore/checkDatabaseType.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
import { appengineOrigin } from "../api";
import { firestoreOrigin } from "../api";
import { Client } from "../apiv2";
import { logger } from "../logger";

/**
* Determine the Firestore database type for a given project. One of:
* - DATABASE_TYPE_UNSPECIFIED (unspecified)
* - CLOUD_DATASTORE (Datastore legacy)
* - CLOUD_FIRESTORE (Firestore native mode)
* - CLOUD_DATASTORE_COMPATIBILITY (Firestore datastore mode)
* - DATASTORE_MODE(Datastore legacy)
* - FIRESTORE_NATIVE (Firestore native mode)
*
* @param projectId the Firebase project ID.
*/
export async function checkDatabaseType(projectId: string): Promise<string | undefined> {
export async function checkDatabaseType(
projectId: string
): Promise<"DATASTORE_MODE" | "FIRESTORE_NATIVE" | "DATABASE_TYPE_UNSPECIFIED" | undefined> {
try {
const client = new Client({ urlPrefix: appengineOrigin, apiVersion: "v1" });
const resp = await client.get<{ databaseType?: string }>(`/apps/${projectId}`);
return resp.body.databaseType;
const client = new Client({ urlPrefix: firestoreOrigin, apiVersion: "v1" });
const resp = await client.get<{
type?: "DATASTORE_MODE" | "FIRESTORE_NATIVE" | "DATABASE_TYPE_UNSPECIFIED";
}>(`/projects/${projectId}/databases/(default)`);
return resp.body.type;
} catch (err: any) {
logger.debug("error getting database type", err);
return undefined;
Expand Down
4 changes: 1 addition & 3 deletions src/init/features/firestore/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { logger } from "../../../logger";
import * as apiEnabled from "../../../ensureApiEnabled";
import { ensureLocationSet } from "../../../ensureCloudResourceLocation";
import { requirePermissions } from "../../../requirePermissions";
import { checkDatabaseType } from "../../../firestore/checkDatabaseType";
import * as rules from "./rules";
Expand Down Expand Up @@ -36,14 +35,13 @@ async function checkProjectSetup(setup: any, config: any, options: any) {

if (!dbType) {
throw firestoreUnusedError;
} else if (dbType !== "CLOUD_FIRESTORE") {
} else if (dbType !== "FIRESTORE_NATIVE") {
throw new FirebaseError(
`It looks like this project is using Cloud Datastore or Cloud Firestore in Datastore mode. The Firebase CLI can only manage projects using Cloud Firestore in Native mode. For more information, visit https://cloud.google.com/datastore/docs/firestore-or-datastore`,
{ exit: 1 }
);
}

ensureLocationSet(setup.projectLocation, "Cloud Firestore");
await requirePermissions({ ...options, project: setup.projectId });
}

Expand Down
11 changes: 1 addition & 10 deletions src/test/init/features/firestore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("firestore", () => {

// By default, mock Firestore enabled in Native mode
checkApiStub.returns(true);
checkDbTypeStub.returns("CLOUD_FIRESTORE");
checkDbTypeStub.returns("FIRESTORE_NATIVE");
});

afterEach(() => {
Expand All @@ -46,15 +46,6 @@ describe("firestore", () => {
expect(_.get(setup, "config.firestore")).to.deep.equal({});
});

it("should error when cloud resource location is not set", async () => {
const setup = { config: {}, projectId: "my-project-123" };

await expect(firestore.doSetup(setup, {}, {})).to.eventually.be.rejectedWith(
FirebaseError,
"Cloud resource location is not set"
);
});

it("should error when the firestore API is not enabled", async () => {
checkApiStub.returns(false);

Expand Down

0 comments on commit d59b78b

Please sign in to comment.