Skip to content

Fix scalar type change not detected as dirty in MongoDB models#3501

Open
GromNaN wants to merge 8 commits into
mongodb:5.xfrom
GromNaN:fix/original-is-equivalent
Open

Fix scalar type change not detected as dirty in MongoDB models#3501
GromNaN wants to merge 8 commits into
mongodb:5.xfrom
GromNaN:fix/original-is-equivalent

Conversation

@GromNaN

@GromNaN GromNaN commented Apr 15, 2026

Copy link
Copy Markdown
Member

Fixes a bug in originalIsEquivalent() where changing a field from int(1) to string('1') was not reported as dirty, even though MongoDB stores types as-is and the document would change on the next save.

Changes

Replace the SQL-oriented logic (numeric coercion via is_numeric + strcmp, date/cast special cases) with BSON-aware comparison:

  • Scalars: identical values are caught by the === early return; any non-identical scalar pair returns false (dirty).
  • Complex types (UTCDateTime, arrays, embedded documents): compared via Document::fromPHP() serialization, which mirrors what MongoDB actually stores.

This is a breaking change: code that was relying on int(1) and string('1') being equivalent will now see them as dirty.

Supersede #2515

This is a rework of #2515. The approach is the same but adapted to the current codebase (DocumentModel trait, MongoDB\Laravel namespace, Document::fromPHP() instead of the deprecated fromPHP() global function).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes MongoDB Eloquent “dirty” detection so scalar type changes (e.g., 1'1') are treated as dirty, aligning comparisons with BSON/MongoDB’s type-preserving storage semantics.

Changes:

  • Reworks DocumentModel::originalIsEquivalent() to be BSON-aware (strict for scalars; BSON-serialization comparison for non-scalars).
  • Updates existing dirty-date test setup to use normal attribute assignment + syncOriginal().
  • Adds tests covering scalar type changes and embedded document (subdocument) equality.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Eloquent/DocumentModel.php Replaces SQL-oriented equivalence logic with scalar strictness + BSON-based comparison for complex values.
tests/ModelTest.php Adds/adjusts tests to validate the new dirty detection behavior for dates, scalar type changes, and embedded docs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Eloquent/DocumentModel.php Outdated
@GromNaN GromNaN force-pushed the fix/original-is-equivalent branch 2 times, most recently from a61aae0 to 28caed9 Compare April 15, 2026 20:43
@GromNaN GromNaN added the bug label Apr 15, 2026
@GromNaN GromNaN added this to the 5.8 milestone Apr 15, 2026
Comment thread src/Eloquent/DocumentModel.php Outdated
Comment thread src/Eloquent/DocumentModel.php Outdated
@GromNaN GromNaN requested a review from alcaeus May 19, 2026 07:26
Comment thread src/Eloquent/DocumentModel.php

if ($this->hasCast($key, static::$primitiveCastTypes)) {
return $this->castAttribute($key, $attribute) ===
$this->castAttribute($key, $original);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should casts be applied before we convert to BSON?

GromNaN and others added 6 commits June 18, 2026 10:10
Co-authored-by: Hamid Alaei <halaei@users.noreply.github.com>
Co-authored-by: divine <divine@users.noreply.github.com>
- Extract all dirty detection tests from ModelTest into ModelGetDirtyTest
- Add testGetDirtyDateWithoutCast: covers UTCDateTime fields without cast,
  comparing via Document::fromPHP after DateTimeInterface conversion
For fields with a primitive cast (int, bool, string, etc.), apply the
cast before comparing values, preserving Eloquent's behavior where
int(1) and string('1') are equivalent on a field cast to int.

Date casts are excluded from this path and continue to use the
DateTimeInterface -> UTCDateTime -> Document comparison.
… tests

Extract scalar cast types into a static property ($scalarCastTypes) to
make the intent explicit. Add encrypted cast variants so re-assigning
the same plaintext is not considered dirty (Eloquent encrypts on SET,
castAttribute decrypts before comparing).

Object/array/collection/json casts are excluded from this path and fall
through to the BSON Document comparison, which correctly handles
PHP-level type differences (array vs stdClass).
…alent

Replace the explicit scalar inclusion list with a shorter exclusion list
($nonScalarCastTypes) of types that produce non-scalar PHP values. Any
new scalar type added to Eloquent's $primitiveCastTypes is automatically
covered. Date types are excluded via isDateAttribute() so future date
cast additions are also covered without touching this class.
@GromNaN GromNaN force-pushed the fix/original-is-equivalent branch from 942debd to bca6952 Compare June 18, 2026 08:23
createSearchIndex triggers Atlas Search engine (mongot) initialization,
which causes a brief mongod unavailability. Add a final ping loop to
ensure MongoDB is ready again before the subsequent steps run.
@GromNaN GromNaN force-pushed the fix/original-is-equivalent branch from b26711f to 17c2ded Compare June 18, 2026 09:13
@GromNaN GromNaN force-pushed the fix/original-is-equivalent branch from 17c2ded to 25a8f06 Compare June 18, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants