-
-
Notifications
You must be signed in to change notification settings - Fork 17
Add pivot chaperones and update query iteration, exception helpers and queue workers #623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e67758d
9f2f481
2fdc60f
f7f78fb
dc352b1
bc910c9
2e2b8ec
7785b79
3d83952
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,140 @@ | ||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| declare(strict_types=1); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| namespace Hypervel\Database\Eloquent\Relations\Concerns; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| use Hypervel\Database\Eloquent\Model; | ||||||||||||||||||||||||||||||
| use Hypervel\Database\Eloquent\RelationNotFoundException; | ||||||||||||||||||||||||||||||
| use Hypervel\Support\Arr; | ||||||||||||||||||||||||||||||
| use Hypervel\Support\Str; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| trait SupportsPivotInverseRelations | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * The name of the declaring model's relationship on the pivot. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected ?string $declaringInverseRelationship = null; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * The name of the related model's relationship on the pivot. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected ?string $relatedInverseRelationship = null; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Instruct Eloquent to link the declaring and related models back to the pivot after the relationship query has run. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * @return $this | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| public function chaperone(?string $declaring = null, ?string $related = null): static | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (! $this->using) { | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documented in 7785b79: using must precede chaperone. This retains the upstream no-custom-pivot no-op without adding deferred configuration state. |
||||||||||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| $pivotModel = new $this->using; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| $this->declaringInverseRelationship = $this->resolvePivotInverseRelation( | ||||||||||||||||||||||||||||||
| $pivotModel, | ||||||||||||||||||||||||||||||
| $declaring, | ||||||||||||||||||||||||||||||
| $this->foreignPivotKey, | ||||||||||||||||||||||||||||||
| $this->parent | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| $this->relatedInverseRelationship = $this->resolvePivotInverseRelation( | ||||||||||||||||||||||||||||||
| $pivotModel, | ||||||||||||||||||||||||||||||
| $related, | ||||||||||||||||||||||||||||||
| $this->relatedPivotKey, | ||||||||||||||||||||||||||||||
| $this->related | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
greptile-apps[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Both sides can guess the same model name, so keep it only on a side named | ||||||||||||||||||||||||||||||
| // explicitly or identified by its pivot key. | ||||||||||||||||||||||||||||||
| if ($this->declaringInverseRelationship !== null | ||||||||||||||||||||||||||||||
| && $this->declaringInverseRelationship === $this->relatedInverseRelationship) { | ||||||||||||||||||||||||||||||
| $relation = $this->declaringInverseRelationship; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if ($declaring === null && ($related !== null || $this->relationNameFromPivotKey($this->foreignPivotKey, $this->parent) !== $relation)) { | ||||||||||||||||||||||||||||||
| $this->declaringInverseRelationship = null; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if ($related === null && ($declaring !== null || $this->relationNameFromPivotKey($this->relatedPivotKey, $this->related) !== $relation)) { | ||||||||||||||||||||||||||||||
| $this->relatedInverseRelationship = null; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Remove the chaperone relationships for this query. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * @return $this | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| public function withoutChaperone(): static | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| $this->declaringInverseRelationship = null; | ||||||||||||||||||||||||||||||
| $this->relatedInverseRelationship = null; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Resolve the inverse relation name on the pivot for a given model. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * If an explicit name is provided and invalid, an exception is thrown. | ||||||||||||||||||||||||||||||
| * If guessing fails, null is returned. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * @throws RelationNotFoundException | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected function resolvePivotInverseRelation(Model $pivotModel, ?string $relation, string $foreignKey, Model $model): ?string | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if ($relation !== null) { | ||||||||||||||||||||||||||||||
| if (! $pivotModel->isRelation($relation)) { | ||||||||||||||||||||||||||||||
| throw RelationNotFoundException::make($pivotModel, $relation); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return $relation; | ||||||||||||||||||||||||||||||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return $this->guessPivotInverseRelation($pivotModel, $foreignKey, $model); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Attempt to guess the inverse relation name on the pivot for a given model. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected function guessPivotInverseRelation(Model $pivotModel, string $foreignKey, Model $model): ?string | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| $candidates = array_filter(array_unique([ | ||||||||||||||||||||||||||||||
| $this->relationNameFromPivotKey($foreignKey, $model), | ||||||||||||||||||||||||||||||
| Str::camel(class_basename($model)), | ||||||||||||||||||||||||||||||
|
Comment on lines
+109
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On a self-referencing Knowledge Base Used: Database access and modeling
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same owner_id/friend_id keys and owner()/user() names can also describe a valid pivot where user() belongs to friend_id. Name-based inference cannot distinguish that from an alias on owner_id. Use chaperone(declaring: 'user') for the reported alias configuration; the explicit name wins and the conflicting related guess is cleared. Calling relationship methods on an empty pivot to inspect foreign keys would change the inference contract and can depend on missing attributes or cause side effects, so no additional runtime inspection is being added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. With the ambiguous Since inspecting relationship methods would alter the inference contract and introduce attribute/side-effect concerns, the original finding is not actionable. I’m withdrawing it. |
||||||||||||||||||||||||||||||
| ])); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return Arr::first( | ||||||||||||||||||||||||||||||
| $candidates, | ||||||||||||||||||||||||||||||
| fn (string $relation): bool => $pivotModel->isRelation($relation) | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This follows the established Eloquent isRelation convention, also used by existing inverse hydration. Passing a non-relationship method is invalid API use; invoking arbitrary methods for validation could itself cause side effects. The valid self-reference inference case is fixed separately. |
||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
greptile-apps[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Derive the inverse relationship name from a pivot key. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected function relationNameFromPivotKey(string $pivotKey, Model $model): string | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| return Str::camel(Str::beforeLast($pivotKey, $model->getKeyName())); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Apply chaperone relationships to a pivot model instance. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| protected function applyChaperonesToPivot(Model $pivot, Model $declaring, Model $related): void | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if ($this->declaringInverseRelationship) { | ||||||||||||||||||||||||||||||
| $pivot->setRelation($this->declaringInverseRelationship, $declaring); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if ($this->relatedInverseRelationship) { | ||||||||||||||||||||||||||||||
| $pivot->setRelation($this->relatedInverseRelationship, $related); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: When the declaring and related inverse relation names resolve to the same string, the second Prompt for AI agents
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the valid distinct-column self-reference case in 7785b79. A shared model basename is no longer an automatic candidate, so key-derived guesses identify the correct side. Skipping the declaring assignment during hydration would still leave ambiguity and would not fix eager matching. |
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ class QueuePaused | |
| * Create a new event instance. | ||
| */ | ||
| public function __construct( | ||
| public string $connection, | ||
| public string $connectionName, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: This renames the public Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. connectionName is the current upstream API. Hypervel 0.4 is unreleased and follows that API without an alias for its earlier property. Event consumers and documentation are aligned. |
||
| public string $queue, | ||
| public DateInterval|DateTimeInterface|int|null $ttl = null, | ||
| ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,17 +13,17 @@ class WorkerStopping | |
| * Create a new event instance. | ||
| * | ||
| * @param null|float|int $memoryUsage the memory usage of the worker in megabytes | ||
| * @param bool $terminatesImmediately whether the process terminates as soon as listeners return; listeners must not start cleanup that must finish before returning when this is true | ||
| * @param bool $terminatesImmediately whether the process terminates as soon as listeners return; when true, listeners must not start cleanup that must finish after they return | ||
| */ | ||
| public function __construct( | ||
| public ?string $connectionName = null, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Reordering these promoted parameters breaks existing consumers that construct the public Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This follows the current upstream constructor order from laravel/framework#61714. Hypervel 0.4 is unreleased and does not retain compatibility with its earlier signatures. All framework callers use the current order or named arguments. |
||
| public ?string $queue = null, | ||
| public int $status = 0, | ||
| public ?WorkerOptions $workerOptions = null, | ||
| public ?WorkerStopReason $reason = null, | ||
| public ?int $jobsProcessed = null, | ||
| public float|int|null $lastJobProcessedAt = null, | ||
| public float|int|null $memoryUsage = null, | ||
| public ?string $connectionName = null, | ||
| public ?string $queue = null, | ||
| public bool $terminatesImmediately = false, | ||
| ) { | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
$itemsis shared across parents with the same key, so this loop repeatedly rewrites the same pivot inverse and leaves matched results pointing to the last parent. Give each parent its own related and pivot instances before setting the inverse.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same-key parents already share their eager-loaded related and pivot objects. The inverse refers to the correct record; independent object graphs for differently mutated in-memory copies are not guaranteed. Adding cloning only here would change existing sharing behavior, so this remains unchanged.