[10.x] Introduce Observe attribute for models

[emargareten]

This pull request introduces a new attribute class for registering model observers.

Example:

#[Observe(UserObserver::class)]
class User extends Authenticatable
{
    //
}

This change means you don't have to register the observers in a service provider. It just makes the code clearer and less confusing, making it easier to understand what's going on.

[taylorotwell]

Not saying I'm totally sold on the feature, but I wonder if the attribute should be on the observer itself?

#[Observes(User::class)]

[PerryvanderMeer]

I'd prefer auto discovery for observers, just like the current auto discovery for policies.

[emargareten]

@taylorotwell the point of having it on the model is to show right away that there are side effects for this model.

[laserhybiz]

I agree with @emargareten. Many times I've encountered unexpected behavior and spent a considerable amount of time debugging, only to realize that there is an observer on the class. Placing them at the top of the model will immediately clarify where to look when something unexpected happens.

[taylorotwell]

Updated to ObservedBy which I think makes more sense. Thanks 👍

[ollieread]

Is there the possibility to improve upon this feature? I know it was merged 2 months ago, but the solution is somewhat hacky, and obfuscates some of the functionality behind magic, and arguably invalid code.

The primary issue is that the code doesn't really use the attribute itself, instead it uses the ReflectionAttribute::getArguments() method, which, while it works, is pretty hacky. Because of this, we have the ObservedBy::__construct() method as follows:

/**
 * Create a new attribute instance.
 *
 * @param  array|string  $classes
 * @return void
 */
public function __construct(array|string $classes)
{
}

In reality, this should be:

public readonly array $classes;

/**
 * Create a new attribute instance.
 *
 * @param  string  ...$classes
 * @return void
 */
public function __construct(string ...$classes)
{
	$this->classes = $classes;
}

Then the handling code:

return collect($reflectionClass->getAttributes(ObservedBy::class))
    ->map(fn ($attribute) => $attribute->getArguments())
    ->flatten()
    ->all();

Would be:

return collect($reflectionClass->getAttributes(ObservedBy::class))
    ->map(fn ($attribute) => $attribute->newInstance()?->classes)
    ->flatten()
    ->all();

I appreciate that this may be nitpicking, but introducing unused constructor parameters that are only there so that they can be pulled out with reflection is a messy solution, and introducing code like this can, as a whole, bring down the overall standard of the frameworks code.

The same for the typing used on the constructor parameter, which actually introduces two different syntaxes for using this attribute.

#[OrderedBy(MyObserver::class)]
class SomeModel extends Model {}

and

#[OrderedBy([MyObserver::class, MyOtherObserver::class])]
class SomeModel extends Model {}

When you also consider that the attribute is repeatable, you end up with a third way.

#[OrderedBy(MyObserver::class), OrderedBy(MyOtherObserver::class)]
class SomeModel extends Model {}

Which makes the following code valid within Laravel.

#[OrderedBy(MyObserver::class)]
#[OrderedBy([MyOtherObserver::class, YetAnotherObserver::class]), OrderedBy(MoreObservers::class)]
class SomeModel extends Model {}

In reality, the only syntax that should be accepted here, is:

#[OrderedBy(MyObserver::class, MyOtherObserver::class, YetAnotherObserver::class)]
class SomeModel extends Model {}

Since attributes are not inherited, and the attribute itself does nothing and is used for nothing but a marker that contains a bit of data, there's no need for it to be repeatable.

It should also be noted that the documentation makes no reference to the fact that it's repeatable, or that there's a second syntax for providing multiple observers in one.

Ideally, the method HasEvents::resolveObservableAttributes() would be:

public static function resolveObserveAttributes()
{
    $reflectionClass = new ReflectionClass(static::class);
    $attribute = $reflectionClass->getAttributes(ObservedBy::class)[0] ?? null;
    
    return $attribute?->newInstance()->classes;
}

I'm more than happy to PR these changes myself if necessary.