Skip to content

Conversation

@fogelito
Copy link
Contributor

@fogelito fogelito commented Nov 24, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Fixed inverted control flow in database casting logic to correctly perform casting when adapter support is available.
    • Updated database adapter casting support flags for consistent behavior across different adapter types.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Walkthrough

Two adapter methods flip their casting support flags in opposite directions: Mongo disables casting support while SQL enables it. The Database class inverts its casting control flow to perform casting when adapter support is confirmed, reversing the previous conditional logic.

Changes

Cohort / File(s) Summary
Adapter casting support
src/Database/Adapter/Mongo.php, src/Database/Adapter/SQL.php
Mongo's getSupportForCasting() returns false (disabled); SQL's getSupportForCasting() returns true (enabled). Both methods flip their previous return values.
Database casting control flow
src/Database/Database.php
The casting method's early-return guard is inverted—casting now executes when adapter support exists, reversing the prior "skip if supported" logic.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Database
    participant Adapter

    rect rgb(240, 248, 255)
    note over Client,Adapter: Before: Inverted Logic (Skip if Supported)
    Client->>Database: cast()
    Database->>Adapter: getSupportForCasting()
    Adapter-->>Database: false
    Database->>Database: Proceed with casting
    end

    rect rgb(240, 255, 240)
    note over Client,Adapter: After: Corrected Logic (Perform if Supported)
    Client->>Database: cast()
    Database->>Adapter: getSupportForCasting()
    Adapter-->>Database: true (SQL) / false (Mongo)
    alt Supported (SQL)
        Database->>Database: Perform casting
    else Not Supported (Mongo)
        Database->>Database: Skip casting
    end
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Logic inversion in Database.php: Requires verification that the inverted conditional matches the intended behavior and that no downstream code relies on the previous logic.
  • Adapter flag toggling: Both adapters flip their casting support in opposite directions; cross-check with their actual implementation capabilities to ensure flags reflect reality.
  • Control flow dependency: Confirm that SQL adapter's new true flag aligns with its casting implementation, and that Mongo's false flag correctly disables casting where needed.

Possibly Related PRs

  • Feat mongodb #721: Adds Mongo adapter with getSupportForCasting() returning true—directly related as this PR flips that to false and modifies the same casting infrastructure.
  • Feat mongo tmp #647: Touches adapter casting surface by modifying getSupportForCasting() and Database casting control flow—overlaps with this PR's core changes.

Suggested Reviewers

  • abnegate

Poem

🐰 The casting dance did flip and sway,
Mongo off, SQL on display,
Logic inverted, guards now plain,
Support returns, not in vain!
Database logic reigns supreme! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Get support for casting' directly describes the main objective of enabling casting support across database adapters.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch casting-support

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe7a132 and 53b2324.

📒 Files selected for processing (3)
  • src/Database/Adapter/Mongo.php (1 hunks)
  • src/Database/Adapter/SQL.php (1 hunks)
  • src/Database/Database.php (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
📚 Learning: 2025-10-29T12:27:57.071Z
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.

Applied to files:

  • src/Database/Adapter/Mongo.php
📚 Learning: 2025-10-03T02:04:17.803Z
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.

Applied to files:

  • src/Database/Adapter/Mongo.php
  • src/Database/Database.php
🧬 Code graph analysis (1)
src/Database/Database.php (4)
src/Database/Adapter/Mongo.php (1)
  • getSupportForCasting (2874-2877)
src/Database/Adapter/SQL.php (1)
  • getSupportForCasting (1498-1501)
src/Database/Adapter.php (1)
  • getSupportForCasting (1002-1002)
src/Database/Adapter/Pool.php (1)
  • getSupportForCasting (375-378)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Setup & Build Docker Image
🔇 Additional comments (3)
src/Database/Adapter/SQL.php (1)

1498-1501: The casting logic change is correct and properly implemented.

The change from false to true correctly reflects that the SQL adapter requires Database-level casting since it lacks internal casting support (both castingBefore() and castingAfter() return the document unchanged, and getSupportForInternalCasting() returns false).

The Database layer correctly interprets the semantics: when getSupportForCasting() returns true, the condition if (!$this->adapter->getSupportForCasting()) evaluates to false, allowing the Database layer to proceed with casting. This contrasts with Mongo, which returns false and has internal casting support, causing the Database layer to skip casting.

The method name is somewhat misleading—it means "should Database handle casting?" not "does adapter handle casting?"—but the logic is sound and consistent across adapters.

src/Database/Adapter/Mongo.php (1)

2874-2877: Revert getSupportForCasting() to true—change breaks type conversions.

The Mongo adapter's castingBefore method only handles VAR_DATETIME conversions; it does nothing for other types (VAR_BOOLEAN, VAR_INTEGER, VAR_FLOAT, etc.). When getSupportForCasting() returns false, the Database layer skips its casting() method entirely, which handles all attribute type conversions. This disables type casting for non-datetime attributes in Mongo collections.

⛔ Skipped due to learnings
Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.
src/Database/Database.php (1)

8265-8325: Casting guard now aligns with adapter capabilities

The inverted guard makes Database::casting() run only when the adapter explicitly reports support for casting, so SQL (support=true) benefits from centralized type casting while Mongo (support=false) skips it, avoiding schema-based casts in schemaless mode. This is consistent with the adapter implementations and looks correct.


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

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

@abnegate abnegate merged commit 5e634a7 into main Nov 24, 2025
18 checks passed
@abnegate abnegate deleted the casting-support branch November 24, 2025 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants