fix: support non-default Firestore database ID in datasource configuration

[RaikaSurendra]

Description

Fixes #38121 — Firestore connection always connects to the default database, ignoring non-default database configuration.

Changes

• form.json: Added optional "Database ID" field (defaults to (default))
• FirestorePlugin.java:
  • Added getDatabaseId() helper to extract database ID from datasource properties
  • Changed FirestoreClient::getFirestore → FirestoreClient.getFirestore(app, databaseId)
• FirestorePluginTest.java: Added 3 tests for default, custom, and empty database ID scenarios

Root Cause

FirestoreClient::getFirestore (no-arg) always returns the default database.
The fix uses FirestoreClient.getFirestore(app, databaseId) from Firebase Admin SDK 9.2.0.

Testing

• Compilation verified locally (mvn compile + test-compile pass)
• Unit tests added (require Docker/Testcontainers for Firestore emulator — CI will run them)

Summary by CodeRabbit

• New Features

  • Added a Database ID setting to Firestore datasource configuration; you can specify a custom Firestore database or leave it blank to use "(default)".

• Tests

  • Added tests covering Database ID handling for various configuration scenarios (missing, null, empty, non-string, and custom values).

[coderabbitai]

Walkthrough

Reads a "databaseId" property from datasource configuration (defaulting to "(default)"), passes it into FirestoreClient initialization during datasource creation, and adds form inputs and tests to exercise this new databaseId flow.

Changes

Cohort / File(s)	Summary
Plugin implementation app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java	Imported Property, added getDatabaseId(DatasourceConfiguration) helper (defaults to "(default)"), duplicated helper in executor, and changed datasourceCreate / initialization to call FirestoreClient.getFirestore(app, databaseId) (previously called without databaseId).
Form configuration app/server/appsmith-plugins/firestorePlugin/src/main/resources/form.json	Added form fields to capture Database ID: visible text input (placeholder/default "(default)"), a hidden key field ("databaseId"), and associated property structure (three new fields inserted).
Tests app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java	Added five unit tests covering various DatasourceConfiguration property scenarios (no properties, null properties, custom databaseId, empty value, non-string value). Tests appear duplicated within the file; no public API signatures changed.

Sequence Diagram(s)

sequenceDiagram
  participant UI as "Admin UI"
  participant DS as "DatasourceConfiguration"
  participant Plugin as "FirestorePlugin / Executor"
  participant Client as "FirestoreClient"
  participant Firebase as "FirebaseApp / Firestore"

  UI->>DS: user provides service account + Database ID
  DS->>Plugin: datasourceCreate / testConnection with config
  Plugin->>Plugin: getDatabaseId(datasourceConfiguration)
  Plugin->>Client: getFirestore(app, databaseId)
  Client->>Firebase: initialize or getInstance(projectId, options, databaseId)
  Firebase-->>Client: Firestore instance bound to databaseId
  Client-->>Plugin: initialized Firestore
  Plugin-->>DS: return success / connection result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

✨ A new key whispers its name, databaseId in frame,
From form to client the path now true,
No longer stuck at default view,
Connections find the DB they claim,
Small change, big reach—hooray for you! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately and concisely describes the main change: adding support for non-default Firestore database IDs in datasource configuration.
Description check	✅ Passed	Description covers the fix, lists changes made to three files, explains root cause, and notes testing approach, but lacks explicit issue link format and DevRel/Marketing checkbox.
Linked Issues check	✅ Passed	Changes fully address issue #38121: form.json adds Database ID field, getDatabaseId() extracts it from properties, and FirestoreClient.getFirestore(app, databaseId) uses it instead of the no-arg version.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing non-default Firestore database support: form config, plugin logic, and unit tests with no extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java`:
- Around line 948-959: getDatabaseId currently casts property.getValue() to
String directly and can throw ClassCastException; change the check in the loop
inside getDatabaseId to first verify the value is a String (e.g.,
property.getValue() instanceof String) before casting and passing to
StringUtils.isEmpty, or defensively convert with
Objects.toString(property.getValue(), null) and then check emptiness; update the
logic around Property, DatasourceConfiguration, CollectionUtils and StringUtils
usage so non-String values are ignored (or handled safely) and only valid String
databaseId values are returned.

In
`@app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java`:
- Around line 1801-1859: The tests call validateDatasource() but that method
never inspects the databaseId, so add unit tests that directly exercise
getDatabaseId() (or make it package-private) and assert it returns "(default)"
when properties are null/empty, returns the custom value when set, and falls
back to "(default)" when the property exists but is empty; additionally add a
Mockito-based test for datasourceCreate() to verify
FirestoreClient.getFirestore(app, expectedDatabaseId) is invoked with the
expected databaseId for default, custom, and empty cases (referencing
getDatabaseId(), validateDatasource(), datasourceCreate(), and
FirestoreClient.getFirestore(app, databaseId) to locate the code under test).

🧹 Nitpick comments (1)

app/server/appsmith-plugins/firestorePlugin/src/main/resources/form.json (1)

29-36: Consider adding a tooltip/subtitle explaining the field.

The "Database ID" field defaults to (default) but users unfamiliar with Firestore multi-database support may not know what to put here. A short subtitle or tooltip (e.g., "Leave as (default) unless using a named Firestore database") would improve UX.

[RaikaSurendra]

Please review

[RaikaSurendra]

review