Fix mongo perms

[abnegate]

Summary by CodeRabbit

• Refactor
  • Database tenant handling refactored across adapter components
  • Tenant-related method signatures updated across multiple classes
  • New getter method added for ID attribute type resolution

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR expands the tenant type system across the database layer from ?int to int|string|null. Method signatures, PHPDoc annotations, and tenant handling logic are updated in Adapter, Database, Document, Mirror, and Mongo classes to support both integer and string tenant identifiers.

Changes

Cohort / File(s)	Summary
Tenant Type Broadening - Base Adapter src/Database/Adapter.php, src/Database/Adapter/Mongo.php	Updated tenant parameter and return types from ?int to int|string|null. Adapter property, setTenant/getTenant methods, and getTenantFilters signature expanded to accept and return union types.
Tenant API Updates - Database Layer src/Database/Database.php	Added getIdAttributeType() method and updated setTenant(), getTenant(), and withTenant() signatures to use int|string|null type hints instead of ?int.
Tenant Handling - Domain Objects src/Database/Document.php, src/Database/Mirror.php	Updated getTenant() return type to int|string|null and changed logic in Document to preserve string tenants instead of casting all values to int. Mirror's setTenant() parameter type broadened similarly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Cahnge getSequence default for null #650: Modifies Document::getTenant() implementation and null-handling behavior similarly to this PR.
• Feat mongo tmp #647: Updates tenant filtering signatures in Mongo adapter and Adapter API overlapping with this PR's type changes.
• Cast internals attributes #626: Modifies how tenant identifiers are represented across the database layer including Database and adapter behavior.

Suggested reviewers

• fogelito

Poem

🐰 With whiskers twitching, types expand today,
From int alone to strings that also play,
The tenant tokens now both ways can flow,
Adapters widen—watch your garden grow! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix mongo perms' is vague and does not clearly convey the actual changes, which involve expanding tenant type support from integers to union types (int|string|null) across multiple database adapter classes.	Consider a more descriptive title such as 'Support string and null tenant types in database adapters' or 'Expand tenant parameter types to int|string|null' to better reflect the substantive changes made.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-mongo-perms

📝 Coding Plan

• Generate coding plan for human review comments

---

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Database/Database.php (1)

1239-1270: ⚠️ Potential issue | 🟠 Major

Replace three empty() tenant checks with null-safe presence validation.

The type signature now accepts int|string|null tenants (lines 1239-1270), but three locations still use empty() which incorrectly treats falsy values 0 and '0' as missing:

• Line 1967: getSizeOfCollectionOnDisk()
• Line 5469: createDocument()
• Line 8006: deleteDocuments()

Additionally, line 254 declares the internal $tenant attribute as VAR_INTEGER, creating a type mismatch with the now-widened string-accepting API. The validator at src/Database/Validator/Structure.php line 54-55 also restricts $tenant to VAR_INTEGER.

Suggested fix

Replace empty($this->adapter->getTenant()) with a dedicated check:

+private function hasTenant(): bool
+{
+    $tenant = $this->adapter->getTenant();
+    return $tenant !== null && $tenant !== '';
+}

Then update all three locations:

-if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) {
+if ($this->adapter->getSharedTables() && !$this->hasTenant()) {

Also verify that the validator's $tenant type declaration supports string values where the new API now allows them.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Database/Database.php` around lines 1239 - 1270, Three locations
(getSizeOfCollectionOnDisk(), createDocument(), deleteDocuments()) currently use
empty($this->adapter->getTenant()) which treats 0 and '0' as missing; replace
those empty() checks with a null-safe presence check (e.g.,
$this->adapter->getTenant() !== null) so numeric/string-zero tenants are
allowed. Also update the internal $tenant property declaration (was VAR_INTEGER)
to accept string values consistent with the public API and adjust the validator
in Validator/Structure.php so the tenant schema/type allows strings (or a union
of integer|string) instead of only VAR_INTEGER. Ensure
getTenant()/setTenant()/withTenant() semantics remain unchanged while fixing
those three checks and the validator/type declaration.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Database/Document.php`:
- Around line 182-186: The current numeric coercion in Document::getTenant (the
is_numeric() check and (int) cast on $tenant) can collapse distinct string
tenant IDs; remove that conversion so getTenant() returns the original $tenant
value unchanged. Locate the is_numeric($tenant) branch in the getTenant() method
and delete the conditional and cast, leaving a single return $tenant; so tenant
identity remains strictly preserved for downstream comparisons with
adapter->getTenant().

---

Outside diff comments:
In `@src/Database/Database.php`:
- Around line 1239-1270: Three locations (getSizeOfCollectionOnDisk(),
createDocument(), deleteDocuments()) currently use
empty($this->adapter->getTenant()) which treats 0 and '0' as missing; replace
those empty() checks with a null-safe presence check (e.g.,
$this->adapter->getTenant() !== null) so numeric/string-zero tenants are
allowed. Also update the internal $tenant property declaration (was VAR_INTEGER)
to accept string values consistent with the public API and adjust the validator
in Validator/Structure.php so the tenant schema/type allows strings (or a union
of integer|string) instead of only VAR_INTEGER. Ensure
getTenant()/setTenant()/withTenant() semantics remain unchanged while fixing
those three checks and the validator/type declaration.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ebf60bb8-2824-41c7-b0ca-19bc5c5f0866

📥 Commits

Reviewing files that changed from the base of the PR and between 503f13d and d9ef23f.

📒 Files selected for processing (5)

• src/Database/Adapter.php
• src/Database/Adapter/Mongo.php
• src/Database/Database.php
• src/Database/Document.php
• src/Database/Mirror.php