Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix Owner Morph Class Resolution and Add Check for 0 in Morph Map Keys #55

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Fix Owner Morph Class Resolution and Add Check for 0 in Morph Map Keys #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fbc05a
fix: use getNumericMorphMap to resolve the numeric morph
rbondoc96 Nov 19, 2024
0c0b371
docs: update README with gotcha
rbondoc96 Nov 20, 2024
e990adb
update: add error checking for a 0 key in numeric morph map
rbondoc96 Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,15 @@ Due to the large amount of records anticipated to be created, you must create an
return [

'morphs' => [
0 => App\Models\User::class
1 => App\Models\User::class
],

];
```

> [!CAUTION]
> `0` is not a valid key for the morph map, as it will get resolved to `false` in Laravel's morphing logic.
Comment on lines +68 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify on this for anyone interested: It's specifically the buildDictionary logic that is used when eager loading.

    protected function buildDictionary(Collection $models)
    {
        foreach ($models as $model) {
            if ($model->{$this->morphType}) {
                $morphTypeKey = $this->getDictionaryKey($model->{$this->morphType});
                $foreignKeyKey = $this->getDictionaryKey($model->{$this->foreignKey});

                $this->dictionary[$morphTypeKey][$foreignKeyKey][] = $model;
            }
        }
    }

The if ($model->{$this->morphType}) { check is what get resolved to false when the morph map value is a 0. This prevents the model from being added to dictionary and ultimately results in attempts at eager loading relationships such as entity always returning null when the relationship is to the model with a morph map value of 0.

This points our `App\Models\User::class` to an enum (integer). This means our database is created with small integers vs large fully qualified namespaces.

Recommended action is creating an enum class to describe all models in your system. If an integer mapping is not detected. The system will error out with an `\InvalidArgumentException`.
Expand Down
5 changes: 3 additions & 2 deletions src/Commands/AuditModelResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Sourcetoad\Logger\Commands;

use Illuminate\Console\Command;
use Sourcetoad\Logger\Logger;
use Sourcetoad\Logger\Helpers\AuditResolver;
use Sourcetoad\Logger\Models\AuditChange;
use Sourcetoad\Logger\Models\AuditModel;
Expand All @@ -21,7 +22,7 @@ public function handle(): void

$item->processed = true;
$item->owner_id = $owner?->getKey();
$item->owner_type = $owner?->getMorphClass();
$item->owner_type = !is_null($owner) ? Logger::getNumericMorphMap($owner) : null;
$item->saveOrFail();
}
});
Expand All @@ -33,7 +34,7 @@ public function handle(): void

$item->processed = true;
$item->owner_id = $owner?->getKey();
$item->owner_type = $owner?->getMorphClass();
$item->owner_type = !is_null($owner) ? Logger::getNumericMorphMap($owner) : null;
$item->saveOrFail();
}
});
Expand Down
21 changes: 15 additions & 6 deletions src/Logger.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Facades\Request;
use InvalidArgumentException;
use Sourcetoad\Logger\Enums\ActivityType;
use Sourcetoad\Logger\Enums\HttpVerb;
use Sourcetoad\Logger\Models\AuditActivity;
Expand Down Expand Up @@ -124,7 +125,7 @@ public function logActivity(int $type, array $keys = []): AuditActivity

$data[] = [
'activity_id' => $activity->id,
'entity_type' => $this->getNumericMorphMap($model),
'entity_type' => static::getNumericMorphMap($model),
'entity_id' => $model->getKey(),
'key_id' => $keys->id,
'processed' => false,
Expand Down Expand Up @@ -156,7 +157,7 @@ public function logActivity(int $type, array $keys = []): AuditActivity

$data[] = [
'activity_id' => $activity->id,
'entity_type' => $this->getNumericMorphMap($model),
'entity_type' => static::getNumericMorphMap($model),
'entity_id' => $model->getKey(),
'processed' => false,
];
Expand All @@ -171,7 +172,7 @@ public function logActivity(int $type, array $keys = []): AuditActivity
// Private functions
//--------------------------------------------------------------------------------------------------------------

private function getNumericMorphMap(Model $model): int
public static function getNumericMorphMap(Model $model): int
{
$fcqn = get_class($model);
$morphMap = LoggerServiceProvider::$morphs;
Expand All @@ -181,11 +182,19 @@ private function getNumericMorphMap(Model $model): int
$morphableTypeId = array_search($fcqn, $morphMap, true);
}

if (is_numeric($morphableTypeId)) {
return $morphableTypeId;
if (!is_numeric($morphableTypeId)) {
throw new InvalidArgumentException(
sprintf('%s has no numeric model map. Check `morphs` in Logger.', get_class($model)),
);
}

throw new \InvalidArgumentException(get_class($model) . " has no numeric model map. Check `morphs` in Logger.");
if ($morphableTypeId === 0) {
throw new InvalidArgumentException(
sprintf('0 is not a valid morph map key for %s. Check `morphs` in Logger.', get_class($model)),
);
}

return $morphableTypeId;
}

private function getHttpVerb(string $verb): int
Expand Down