ActivityLogger::withProperty() - Code Metrics - Inspection of "withProperty, minor refactors" - spatie/laravel-activitylog - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( bd4385...f31fb1 )

by Sebastian

created 2016-06-27 10:00 UTC

ActivityLogger::withProperty() A

↳ Parent: ActivityLogger

Complexity

Conditions	1
Paths	1

Size

Total Lines	6
Code Lines	3

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	1
eloc	3
nc	1
nop	2
dl	0
loc	6
rs	9.4285
c	0
b	0
f	0

<?php

namespace Spatie\Activitylog;

use Illuminate\Contracts\Auth\Guard;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Collection;
use Spatie\Activitylog\Exceptions\CouldNotLogActivity;
use Spatie\Activitylog\Models\Activity;

class ActivityLogger
{
    /** @var \Illuminate\Contracts\Auth\Guard */
    protected $auth;

    /** @var \Illuminate\Database\Eloquent\Model */
    protected $performedOn;

    /** @var \Illuminate\Database\Eloquent\Model */
    protected $causedBy;

    /** @var \Illuminate\Support\Collection */
    protected $properties;

    public function __construct(Guard $auth)
    {
        $this->auth = $auth;

        $this->properties = collect();

        $this->causedBy = $auth->user();
class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
    }

    public function performedOn(Model $model)
    {
        $this->performedOn = $model;

        return $this;
    }

    public function on(Model $model)
    {
        return $this->performedOn($model);
    }

    /**
     * @param \Illuminate\Database\Eloquent\Model|int|string $modelOrId
     *
     * @return $this
     */
    public function causedBy($modelOrId)
    {
        $model = $this->normalizeCauser($modelOrId);

        $this->causedBy = $model;

        return $this;
    }

    public function by($modelOrId)
    {
        return $this->causedBy($modelOrId);
    }

    /**
     * @param array|\Illuminate\Support\Collection $properties
     *
     * @return $this
     */
    public function withProperties($properties)
    {
        $this->properties = collect($properties);

        return $this;
    }

    /**
     * @param string $key
     * @param mixed $value
     *
     * @return $this
     */
    public function withProperty(string $key, $value)
    {
        $this->properties->put($key, $value);

        return $this;
    }

    public function log(string $description)
    {
        $activity = new Activity();

        if ($this->performedOn) {
            $activity->subject()->associate($this->performedOn);
        }

        if ($this->causedBy) {
            $activity->causer()->associate($this->causedBy);
        }

        $activity->properties = $this->properties;

        $activity->description = $this->replacePlaceholders($description, $activity);

        $activity->save();
    }

    /**
     * @param \Illuminate\Database\Eloquent\Model|int|string $modelOrId
     *
     * @return \Illuminate\Database\Eloquent\Model
     *
     * @throws \Spatie\Activitylog\Exceptions\CouldNotLogActivity
     */
    protected function normalizeCauser($modelOrId): Model
    {
        if ($modelOrId instanceof Model) {
            return $modelOrId;
        }

        if ($model = $this->auth->getProvider()->retrieveById($modelOrId)) {
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            return $model;
        }

        throw CouldNotLogActivity::couldNotDetermineUser($modelOrId);
    }

    protected function replacePlaceholders(string $description, Activity $activity): string
    {
        return preg_replace_callback('/:[a-z0-9._-]+/i', function ($match) use ($activity) {

            $match = $match[0];

            $attribute = (string)string($match)->between(':', '.');

            if (! in_array($attribute, ['subject', 'causer', 'properties'])) {
                return $match;
            }

            $propertyName = substr($match, strpos($match, '.') + 1);

            $attributeValue = $activity->$attribute;

            if ($attributeValue instanceof Model) {
                $attributeValue = $attributeValue->toArray();
            }

            return array_get($attributeValue, $propertyName, $match);
        }, $description);
    }
}


1			<?php
2
3			namespace Spatie\Activitylog;
4
5			use Illuminate\Contracts\Auth\Guard;
6			use Illuminate\Database\Eloquent\Model;
7			use Illuminate\Support\Collection;
8			use Spatie\Activitylog\Exceptions\CouldNotLogActivity;
9			use Spatie\Activitylog\Models\Activity;
10
11			class ActivityLogger
12			{
13			/** @var \Illuminate\Contracts\Auth\Guard */
14			protected $auth;
15
16			/** @var \Illuminate\Database\Eloquent\Model */
17			protected $performedOn;
18
19			/** @var \Illuminate\Database\Eloquent\Model */
20			protected $causedBy;
21
22			/** @var \Illuminate\Support\Collection */
23			protected $properties;
24
25			public function __construct(Guard $auth)
26			{
27			$this->auth = $auth;
28
29			$this->properties = collect();
30
31			$this->causedBy = $auth->user();
			0 ignored issues – show Documentation Bug introduced 2016-06-23 22:55 UTC by Report Bug Copy Issue Report It seems like `$auth->user()` can also be of type `object<Illuminate\Contracts\Auth\Authenticatable>`. However, the property `$causedBy` is declared as type `object<Illuminate\Database\Eloquent\Model>`. Maybe add an additional type check? Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly. For example, imagine you have a variable `$accountId` that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the `id` property of an instance of the `Account` class. This class holds a proper account, so the id value must no longer be false. Either this assignment is in error or a type check should be added for that assignment. class Id { public $id; public function __construct($id) { $this->id = $id; } } class Account { /** @var Id $id */ public $id; } $account_id = false; if (starsAreRight()) { $account_id = new Id(42); } $account = new Account(); if ($account instanceof Id) { $account->id = $account_id; } Loading history...
32			}
33
34			public function performedOn(Model $model)
35			{
36			$this->performedOn = $model;
37
38			return $this;
39			}
40
41			public function on(Model $model)
42			{
43			return $this->performedOn($model);
44			}
45
46			/**
47			* @param \Illuminate\Database\Eloquent\Model\|int\|string $modelOrId
48			*
49			* @return $this
50			*/
51			public function causedBy($modelOrId)
52			{
53			$model = $this->normalizeCauser($modelOrId);
54
55			$this->causedBy = $model;
56
57			return $this;
58			}
59
60			public function by($modelOrId)
61			{
62			return $this->causedBy($modelOrId);
63			}
64
65			/**
66			* @param array\|\Illuminate\Support\Collection $properties
67			*
68			* @return $this
69			*/
70			public function withProperties($properties)
71			{
72			$this->properties = collect($properties);
73
74			return $this;
75			}
76
77			/**
78			* @param string $key
79			* @param mixed $value
80			*
81			* @return $this
82			*/
83			public function withProperty(string $key, $value)
84			{
85			$this->properties->put($key, $value);
86
87			return $this;
88			}
89
90			public function log(string $description)
91			{
92			$activity = new Activity();
93
94			if ($this->performedOn) {
95			$activity->subject()->associate($this->performedOn);
96			}
97
98			if ($this->causedBy) {
99			$activity->causer()->associate($this->causedBy);
100			}
101
102			$activity->properties = $this->properties;
103
104			$activity->description = $this->replacePlaceholders($description, $activity);
105
106			$activity->save();
107			}
108
109			/**
110			* @param \Illuminate\Database\Eloquent\Model\|int\|string $modelOrId
111			*
112			* @return \Illuminate\Database\Eloquent\Model
113			*
114			* @throws \Spatie\Activitylog\Exceptions\CouldNotLogActivity
115			*/
116			protected function normalizeCauser($modelOrId): Model
117			{
118			if ($modelOrId instanceof Model) {
119			return $modelOrId;
120			}
121
122			if ($model = $this->auth->getProvider()->retrieveById($modelOrId)) {
			0 ignored issues – show Bug introduced 2016-06-24 11:55 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Illuminate\Contracts\Auth\Guard` as the method `getProvider()` does only exist in the following implementations of said interface: `Illuminate\Auth\SessionGuard`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
123			return $model;
124			}
125
126			throw CouldNotLogActivity::couldNotDetermineUser($modelOrId);
127			}
128
129			protected function replacePlaceholders(string $description, Activity $activity): string
130			{
131			return preg_replace_callback('/:[a-z0-9._-]+/i', function ($match) use ($activity) {
132
133			$match = $match[0];
134
135			$attribute = (string)string($match)->between(':', '.');
136
137			if (! in_array($attribute, ['subject', 'causer', 'properties'])) {
138			return $match;
139			}
140
141			$propertyName = substr($match, strpos($match, '.') + 1);
142
143			$attributeValue = $activity->$attribute;
144
145			if ($attributeValue instanceof Model) {
146			$attributeValue = $attributeValue->toArray();
147			}
148
149			return array_get($attributeValue, $propertyName, $match);
150			}, $description);
151			}
152			}
153

spatie / laravel-activitylog

Push — master ( bd4385...f31fb1 )

ActivityLogger::withProperty() A

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like