Repository navigation
Sync with upstream (Laravel 12/13 support) - #1
Merged
Merged
Conversation
Laravel 13 Support
…any relationships.
…-to-many Add support for many to many relation
…and switch to match
…lper + README. The trait is load-bearing for the fix's clean form, so it belongs here.
Fix many to many bugs
Eloquent's setKeysForSaveQuery and setKeysForSelectQuery use only the scalar primary key when building WHERE clauses for UPDATE, DELETE, and refresh. On tables with a composite primary key this writes to or reads the wrong row when the scalar id is not unique across the discriminator. Models can now declare a $compositeKey array. When non-empty, the extra columns are AND-ed into the save and select WHERE clauses. $primaryKey stays scalar so getKey, route binding, and queue serialization continue to work.
Two silent-failure bugs in the trait's WHERE-value resolution. First, binding null into a `=` predicate evaluates as UNKNOWN in SQL, so save/delete/refresh on a row with a null composite-key column silently affected zero rows. Fixed by emitting whereNull when the resolved value is null. Second, the `getRawOriginal($column) ?? getAttribute($column)` pattern conflated "key missing from original" with "key present with null value", so mutating a null column to a non-null value in memory made the WHERE clause use the new value instead of the original null, again missing the row. Fixed by replacing the coalesce with an explicit array_key_exists check against the model's original state. Adds 12 tests to tests/Unit/CompositeKeyWriteTest.php covering both null directions (null to value, value to null), builder-level bulk delete (which must not augment), INSERT validation gate non-firing, mass-assignment update entry point, fresh, forceDelete, restore, and a non-id scalar primary key fixture. Adds ScopedUser and CodedUser fixtures with their migration entries. README's "Composite primary keys" section gains a paragraph noting that nullable composite-key columns are supported via IS NULL translation, explaining the SQL three-valued-logic motivation.
When a model declaring $compositeKey is queued via SerializesModels,
getQueueableId() now returns a JSON-encoded array of the composite
key columns (e.g. `{"id":"u1","tenant_id":"t1"}`) instead of the
scalar key. On the worker, newQueryForRestoration() decodes that
string, verifies the shape matches the current $compositeKey columns,
and builds a query that ANDs equality (or whereNull for null values)
for each column. The exact composite row is reloaded.
The JSON-string format is required because Laravel's restoreCollection
branches on is_array($id) and would mis-route a single composite
model into the collection rehydration path. A string id stays in the
single-model branch and remains decodable on the worker side.
Old queued payloads predating this change (with a scalar id) continue
to restore via the parent path. The override only fires when the id
is a JSON string whose decoded shape matches $compositeKey exactly.
No queue drain is required on upgrade.
Adds 7 round-trip tests covering basic round-trip, null discriminator,
three-column key, scalar models unaffected, misconfigured throw on
serialize, scalar id fallback, and the raw JSON encoding shape.
Adds QueueableJobStub fixture and illuminate/queue dev dependency.
README documents the new round-trip contract in the Composite primary
keys section and removes the queue serialization exclusion from the
Scope section.
While extending newQueryForRestoration to handle the Collection restoration path, found a Laravel-side constraint that prevents EloquentCollection round-trip for composite-keyed models. After the restoration query runs, Laravel's restoreCollection re-keys loaded models by scalar getKey() and looks up by the original queued ids. Since our getQueueableId returns a JSON-encoded composite identifier, the lookup keys never match and the restored Collection is empty. Fixing this from the trait would require overriding getKey() to return JSON, which breaks find(), route binding, associate(), broadcast channel naming, and dozens of internal Eloquent paths that expect a scalar primary key. A correct fix requires a Laravel PR to restoreCollection. Adds three regression-lock tests on the single-id restoration path: wrong-shape JSON, non-JSON string, integer id. Each asserts the override falls through to the parent (scalar-key) lookup path. README's Composite primary keys section documents the limitation and recommends the workaround pattern (queue scalar keys or composite tuples as raw arrays, reload the models inside the job handler). The trait docblock on newQueryForRestoration notes the same.
Three previously untested queue surfaces now have regression locks. None were production bugs; the tests pin existing behavior so a future refactor that broke any of them would fail loudly. Multi-model: a job holding two composite-keyed model properties on separate fields round-trips each independently. SerializesModels walks all properties via reflection and applies getQueueableId per model. The test seeds two distinct composite-keyed rows and asserts both restored properties carry the correct identities. Relations: a composite-keyed model with an eager-loaded hasMany relation survives the round-trip. SerializesModels captures relation names via getQueueableRelations and reloads them via loadMissing on the restored model. The test uses a new TenantUser::notes() relation (composite-key foreign and local keys) backed by a tenant_user_notes table fixture and a TenantUserNote child model using the Compoships trait. BackedEnum cast: a composite column cast to a BackedEnum round-trips correctly. getQueueableId calls getAttribute which returns the enum instance; json_encode flattens via JsonSerializable to the backing scalar; on restore the WHERE binds the scalar and the cast re-applies on load. The test uses the existing EnumTenantUser fixture. Adds the tenant_user_notes table, TenantUserNote model, notes() relation on TenantUser, and a secondModel property on QueueableJobStub to enable the multi-model scenario without a parallel stub class. No src/ change. Trait behavior is unchanged.
Composite-keyed model collections can now be queued via a small opt-in wrapper class. Users wrap the collection at queue time via QueueableCompositeCollection::for($models) and unwrap at handler time via $this->users->restore(). The wrapper sidesteps Laravel's restoreCollection limitation by capturing composite-key tuples at queue time and rebuilding the collection via an OR-of-AND query at restore time. The wrapper is opaque to SerializesModels (it implements neither QueueableEntity nor QueueableCollection), so the trait passes it through unchanged and PHP's standard serialization captures the bag's properties directly. No trait composition, no Laravel-side fix required, no override of getKey(). Preserves the original collection order (via JSON-encoded tuple lookup map), eager-loaded relations (via with() on the restore query), and the model connection. Rejects mixed-class collections at wrap time with LogicException, mirroring Laravel's EloquentCollection::getQueueableClass behavior. Rejects misconfigured $compositeKey at wrap time with InvalidUsageException, mirroring the save-path validation gate. Promotes Compoships::getCompositeKeyValues() from protected to public so the wrapper can call it from outside the model. Backward compatible API broadening. Adds 7 tests in tests/Unit/QueueableCompositeCollectionTest.php: basic round-trip with order preservation, null discriminator, empty collection (asserts no DB query), mixed-class throw, misconfigured $compositeKey throw, loaded relations preserved, and three-column composite key. README's Composite primary keys section documents the new wrapper with a complete usage example, replacing the previous limitation paragraph.
QueueableCompositeCollection::restore() used json_encode($tuple) as
the lookup map key for re-ordering loaded models to match the
original collection order. The encoding preserved PHP-side type
identity, so int 1 in memory and string "1" returned by PDO produced
distinct keys for the same logical row. The model was silently
dropped from the restored collection with no error indication.
The mismatch fires for any composite-key column without an explicit
cast (e.g., $table->integer('tenant_id') with no $casts entry): the
in-memory model holds the typed scalar from user assignment, the
loaded model holds the string from PDO. The query returns the row
correctly; only the in-memory hash lookup misses.
Adds normalizeValueForKey($value) that coerces scalars to canonical
string form (int/float/string to string cast, bool to "1"/"0",
BackedEnum to backing value as string, null stays null) before the
canonical lookup key is built. The save and queue paths are not
affected because they bind values directly into SQL, where PDO
handles type coercion at the binding layer.
Adds test_canonical_key_handles_type_mismatched_composite_columns:
constructs a model with an integer tenant_id in memory, queues,
restores, asserts the model is present (previously dropped).
StyleCI flagged the docblocks added in src/Compoships.php and src/Queue/QueueableCompositeCollection.php. Two violations: 1. phpdoc_order: @throws must come before @return. The added docblocks had @return first. 2. phpdoc_align: @PARAM should use a single space between @PARAM, the type, and the variable name. The added docblocks used double spaces, mimicking a pattern that the maintainer had already fixed in a previous style-fix commit (dbc6175). Also adds laravel/pint as a require-dev dependency so local contributors can check style before pushing. StyleCI and Pint use slightly different rule subsets in their respective laravel presets, so Pint output should be reviewed selectively rather than applied wholesale. No behavior change. Full suite still 186/186, 555 assertions.
StyleCI enforces no space after the logical-not operator (`!$var`, not `! $var`) and aligns `@throws` exception class names across consecutive lines the same way it aligns `@param` variable names. Removes the space after `!` in six locations: getAdditionalKeyNames, getCompositeKeyValues, and newQueryForRestoration in Compoships.php (five sites total), and the relations check in QueueableCompositeCollection::restore. Aligns the two `@throws` lines on QueueableCompositeCollection::for by padding the shorter `\LogicException` line so its message column matches the longer `\Awobaz\Compoships\Exceptions\InvalidUsageException` line. No behavior change. Full suite still 186/186, 555 assertions.
Pint was added during StyleCI diagnosis but turned out to be an unreliable proxy: its `laravel` preset disagrees with StyleCI's `laravel` preset on several rules (phpdoc_order direction, blank line separation, binary_operator_spaces alignment, parens on `new`). StyleCI remains the sole style enforcement for this project.
…imary-key-writes Add composite primary key support to write and queue paths
Composite-key sync()/toggle() compared caller tuples against driver results via json_encode of raw values, so a key column read back with a different scalar type (int vs string) classified already-attached rows as "to detach", causing a detach + re-attach churn instead of an in-place pivot update. Both sides now compare through one canonical serializer (non-null scalars cast to string, null preserved, enums resolved), while attach payloads and the returned change sets keep the original values. All composite whereIn call sites (detach, newPivotStatementForId, getCurrentlyAttachedPivotsForIds) now funnel through an arity-checked tuple normalizer, and the query builder rejects wrong-arity tuples and compiles empty tuple lists to a no-match clause. Previously a flat tuple reaching whereIn expanded into more placeholders than bindings: silently NULL-filled on SQLite, SQLSTATE 08P01 on PostgreSQL.
Every existing pivot test table has an increments(id) column, so the composite branches of the pivot save/delete locators (setKeysForSelectQuery via setKeysForSaveQuery, and getDeleteQuery) were never exercised: the locator short-circuits to the surrogate key whenever the id attribute is present. Laravel convention is pivot tables without one. The new project_team_no_id table and tests prove sync updates and detaches target rows by the composite keys alone.
Added deprecation handling and error display options.
…ype-safety Fix type-sensitive composite sync/toggle and malformed tuple whereIn
…mposite-upsert Fix upsert for composite has-one and has-many relations
fix: handle Expression::getValue($grammar) for Laravel 10+
Fix StyleCI problems
Add TODO.md and run-matrix-tests.sh to export-ignore
Handle potential null values in dictionary keys(php8.5 deprecation)
Enhance whereColumn to handle '=' operator short-cut
Eloquent's dissociate() passes the foreign key to setAttribute(), which throws a TypeError when the key is an array. Null each key column instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the fork up to date with topclaudy/compoships master, which supports
illuminate/database^12|^13. The fork's only change, the composite-keydissociate(), is kept and now has a test; it is also proposed upstream so the fork can eventually be dropped.The resulting tree is identical to upstream master plus that one commit. Full suite passes (208 tests, PHP 8.4).
🤖 Generated with Claude Code