FormRecovery::make() - Code Metrics - Inspection of "User logs, multi sessions features, fix, etc" - phpffcms/ffcms - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( f22cf7...1cd2c2 )

by Mihail

created 2016-04-16 08:39 UTC

FormRecovery::make() B

↳ Parent: FormRecovery

Complexity

Conditions	7
Paths	5

Size

Total Lines	59
Code Lines	37

Duplication

Lines	0
Ratio	0 %

Importance

Changes	3
Bugs	1	Features	0

Metric	Value
c	3
b	1
f	0
dl	0
loc	59
rs	7.5346
cc	7
eloc	37
nc	5
nop	0

How to fix Long Method

<?php

namespace Apps\Model\Front\User;

use Apps\ActiveRecord\UserRecovery;
use Ffcms\Core\App;
use Ffcms\Core\Arch\Model;
use Ffcms\Core\Exception\SyntaxException;
use Ffcms\Core\Helper\Date;
use Ffcms\Core\Helper\Type\Str;
use Apps\ActiveRecord\UserLog;

class FormRecovery extends Model
{
    const DELAY = 900; // delay between 2 recovery submits

    public $email;
    public $captcha;

    /**
    * Labels
    */
    public function labels()
    {
        return [
            'email' => __('Email'),
            'captcha' => __('Captcha')
        ];
    }

    /**
    * Validation rules
    */
    public function rules()
    {
        return [
            [['email', 'captcha'], 'required'],
            ['email', 'email'],
            ['captcha', 'App::$Captcha::validate'],
            ['email', 'App::$User::isMailExist']
        ];
    }

    /**
     * After validation generate new pwd, recovery token and send email
     * @throws SyntaxException
     */
    public function make()
    {
        $user = App::$User->getIdentityViaEmail($this->email);
        if ($user === null) {
            throw new SyntaxException('Email not found');
        }
        if ($user->approve_token !== '0' && Str::length($user->approve_token) > 0) {
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
            throw new SyntaxException('You must approve your account');
        }

        $rows = UserRecovery::where('user_id', '=', $user->getId())
            ->orderBy('id', 'DESC')
            ->first();

        if ($rows !== null && $rows !== false) {
            // prevent spam of recovery messages
            if (Date::convertToTimestamp($rows->created_at) > time() - self::DELAY) {
                return;
            }
        }

        // generate pwd, token and pwdCrypt
        $newPwd = Str::randomLatinNumeric(mt_rand(8, 16));
        $pwdCrypt = App::$Security->password_hash($newPwd);
        $token = Str::randomLatinNumeric(mt_rand(64, 128));

        // write new data to recovery table
        $rObject = new UserRecovery();
        $rObject->user_id = $user->id;
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
        $rObject->password = $pwdCrypt;
        $rObject->token = $token;
        $rObject->save();

        // write logs data
        $log = new UserLog();
        $log->user_id = $user->id;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $log->type = 'RECOVERY';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $log->message = __('Password recovery is initialized from: %ip%', ['ip' => App::$Request->getClientIp()]);
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $log->save();

        // generate mail template
        $mailTemplate = App::$View->render('user/mail/recovery', [
            'login' => $user->login,
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
            'email' => $this->email,
            'password' => $newPwd,
            'token' => $token,
            'id' => $rObject->id
        ]);

        $sender = App::$Properties->get('adminEmail');

        // format SWIFTMailer format
        $mailMessage = \Swift_Message::newInstance(App::$Translate->get('Profile', 'Account recovery on %site%', ['site' => App::$Request->getHost()]))
            ->setFrom([$sender])
            ->setTo([$this->email])
            ->setBody($mailTemplate, 'text/html');
        // send message
        App::$Mailer->send($mailMessage);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
    }
}

Push — master ( f22cf7...1cd2c2 )

FormRecovery::make() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Available Fixes

Available Fixes

Available Fixes

1			<?php
2
3			namespace Apps\Model\Front\User;
4
5			use Apps\ActiveRecord\UserRecovery;
6			use Ffcms\Core\App;
7			use Ffcms\Core\Arch\Model;
8			use Ffcms\Core\Exception\SyntaxException;
9			use Ffcms\Core\Helper\Date;
10			use Ffcms\Core\Helper\Type\Str;
11			use Apps\ActiveRecord\UserLog;
12
13			class FormRecovery extends Model
14			{
15			const DELAY = 900; // delay between 2 recovery submits
16
17			public $email;
18			public $captcha;
19
20			/**
21			* Labels
22			*/
23			public function labels()
24			{
25			return [
26			'email' => __('Email'),
27			'captcha' => __('Captcha')
28			];
29			}
30
31			/**
32			* Validation rules
33			*/
34			public function rules()
35			{
36			return [
37			[['email', 'captcha'], 'required'],
38			['email', 'email'],
39			['captcha', 'App::$Captcha::validate'],
40			['email', 'App::$User::isMailExist']
41			];
42			}
43
44			/**
45			* After validation generate new pwd, recovery token and send email
46			* @throws SyntaxException
47			*/
48			public function make()
49			{
50			$user = App::$User->getIdentityViaEmail($this->email);
51			if ($user === null) {
52			throw new SyntaxException('Email not found');
53			}
54			if ($user->approve_token !== '0' && Str::length($user->approve_token) > 0) {
			0 ignored issues – show Bug introduced 2015-11-21 16:45 UTC by Report Bug Copy Issue Report Accessing `approve_token` on the interface `Ffcms\Core\Interfaces\iUser` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
55			throw new SyntaxException('You must approve your account');
56			}
57
58			$rows = UserRecovery::where('user_id', '=', $user->getId())
59			->orderBy('id', 'DESC')
60			->first();
61
62			if ($rows !== null && $rows !== false) {
63			// prevent spam of recovery messages
64			if (Date::convertToTimestamp($rows->created_at) > time() - self::DELAY) {
65			return;
66			}
67			}
68
69			// generate pwd, token and pwdCrypt
70			$newPwd = Str::randomLatinNumeric(mt_rand(8, 16));
71			$pwdCrypt = App::$Security->password_hash($newPwd);
72			$token = Str::randomLatinNumeric(mt_rand(64, 128));
73
74			// write new data to recovery table
75			$rObject = new UserRecovery();
76			$rObject->user_id = $user->id;
			0 ignored issues – show Bug introduced 2015-11-21 16:45 UTC by Report Bug Copy Issue Report Accessing `id` on the interface `Ffcms\Core\Interfaces\iUser` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
77			$rObject->password = $pwdCrypt;
78			$rObject->token = $token;
79			$rObject->save();
80
81			// write logs data
82			$log = new UserLog();
83			$log->user_id = $user->id;
			0 ignored issues – show Documentation introduced 2016-04-16 08:43 UTC by Report Bug Copy Issue Report The property `user_id` does not exist on `object<Apps\ActiveRecord\UserLog>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history... Bug introduced 2016-04-16 08:43 UTC by Report Bug Copy Issue Report Accessing `id` on the interface `Ffcms\Core\Interfaces\iUser` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
84			$log->type = 'RECOVERY';
			0 ignored issues – show Documentation introduced 2016-04-16 08:43 UTC by Report Bug Copy Issue Report The property `type` does not exist on `object<Apps\ActiveRecord\UserLog>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
85			$log->message = __('Password recovery is initialized from: %ip%', ['ip' => App::$Request->getClientIp()]);
			0 ignored issues – show Documentation introduced 2016-04-16 08:43 UTC by Report Bug Copy Issue Report The property `message` does not exist on `object<Apps\ActiveRecord\UserLog>`. Since you implemented `__set`, maybe consider adding a @property annotation. Since your code implements the magic setter `_set`, this function will be called for any write access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } Since the property has write access only, you can use the @property-write annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
86			$log->save();
87
88			// generate mail template
89			$mailTemplate = App::$View->render('user/mail/recovery', [
90			'login' => $user->login,
			0 ignored issues – show Bug introduced 2015-11-21 16:45 UTC by Report Bug Copy Issue Report Accessing `login` on the interface `Ffcms\Core\Interfaces\iUser` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
91			'email' => $this->email,
92			'password' => $newPwd,
93			'token' => $token,
94			'id' => $rObject->id
95			]);
96
97			$sender = App::$Properties->get('adminEmail');
98
99			// format SWIFTMailer format
100			$mailMessage = \Swift_Message::newInstance(App::$Translate->get('Profile', 'Account recovery on %site%', ['site' => App::$Request->getHost()]))
101			->setFrom([$sender])
102			->setTo([$this->email])
103			->setBody($mailTemplate, 'text/html');
104			// send message
105			App::$Mailer->send($mailMessage);
			0 ignored issues – show Documentation introduced 2016-03-01 13:05 UTC by Report Bug Copy Issue Report `$mailMessage` is of type `object<Swift_Mime_MimePart>`, but the function expects a `object<Swift_Mime_Message>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
106			}
107			}

phpffcms / ffcms

Push — master ( f22cf7...1cd2c2 )

FormRecovery::make() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like