Complex classes like User often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use User, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | class User |
||
28 | { |
||
29 | use AssertionTrait; |
||
30 | /** |
||
31 | * @var FieldLayout|null |
||
32 | */ |
||
33 | private $fieldLayout; |
||
34 | /** |
||
35 | * @var Field[] |
||
36 | */ |
||
37 | private $fields = []; |
||
38 | |||
39 | /** |
||
40 | * @param SamlResponse $response |
||
41 | * @return UserElement |
||
42 | * @throws InvalidMessage |
||
43 | * @throws UserException |
||
44 | */ |
||
45 | 3 | public function getByResponse(SamlResponse $response, ProviderRecord $serviceProvider, Settings $settings) |
|
46 | { |
||
47 | |||
48 | 3 | $username = null; |
|
49 | |||
50 | 3 | if (!is_null($settings->nameIdAttributeOverride)) { |
|
51 | // use override |
||
52 | foreach ($this->getAssertions( |
||
53 | $response, |
||
54 | $serviceProvider |
||
55 | ) as $assertion) { |
||
56 | $attributes = $assertion->getAttributes(); |
||
57 | if (isset($attributes[$settings->nameIdAttributeOverride])) { |
||
58 | $attributeValue = $attributes[$settings->nameIdAttributeOverride]; |
||
59 | $username = $this->getAttributeValue($attributeValue); |
||
60 | } |
||
61 | } |
||
62 | } else { |
||
63 | // use nameid |
||
64 | 3 | $assertion = $this->getFirstAssertion($response, $serviceProvider); |
|
65 | |||
66 | 3 | if (! $assertion->getNameId()) { |
|
67 | 3 | throw new InvalidMessage('Name ID is missing.'); |
|
68 | } |
||
69 | 3 | $username = $assertion->getNameId()->getValue(); |
|
70 | |||
71 | 3 | Saml::debug('NameId: ' . $assertion->getNameId()->getValue()); |
|
72 | } |
||
73 | |||
74 | 3 | return $this->find($username); |
|
75 | } |
||
76 | |||
77 | /** |
||
78 | * @param ProviderIdentityRecord $identity |
||
79 | * @return bool |
||
80 | * @throws UserException |
||
81 | * @throws \Throwable |
||
82 | */ |
||
83 | public function login(\flipbox\saml\sp\records\ProviderIdentityRecord $identity) |
||
106 | |||
107 | /** |
||
108 | * @param UserElement $user |
||
109 | * @param SamlResponse $response |
||
110 | * @param ProviderRecord $idp |
||
111 | * @param Settings $settings |
||
112 | * @throws UserException |
||
113 | * @throws \Throwable |
||
114 | * @throws \craft\errors\ElementNotFoundException |
||
115 | * @throws \yii\base\Exception |
||
116 | */ |
||
117 | public function sync( |
||
118 | UserElement $user, |
||
119 | SamlResponse $response, |
||
120 | ProviderRecord $idp, |
||
121 | ProviderRecord $sp, |
||
122 | Settings $settings |
||
123 | ) { |
||
124 | |||
125 | // enable and transform the user |
||
126 | $this->construct( |
||
127 | $user, |
||
128 | $response, |
||
129 | $idp, |
||
130 | $sp, |
||
131 | $settings |
||
132 | ); |
||
133 | |||
134 | // Save |
||
135 | $this->save($user); |
||
136 | } |
||
137 | |||
138 | /** |
||
139 | * @param UserElement $user |
||
140 | * @return bool |
||
141 | * @throws UserException |
||
142 | * @throws \Throwable |
||
143 | * @throws \craft\errors\ElementNotFoundException |
||
144 | * @throws \yii\base\Exception |
||
145 | */ |
||
146 | protected function save(UserElement $user) |
||
147 | { |
||
148 | if (! \Craft::$app->getElements()->saveElement($user)) { |
||
149 | Saml::error( |
||
150 | 'User save failed: ' . \json_encode($user->getErrors()) |
||
151 | ); |
||
152 | throw new UserException("User save failed: " . \json_encode($user->getErrors())); |
||
153 | } |
||
154 | |||
155 | return true; |
||
156 | } |
||
157 | |||
158 | /** |
||
159 | * Response Based Methods |
||
160 | */ |
||
161 | |||
162 | /** |
||
163 | * @param UserElement $user |
||
164 | * @param SamlResponse $response |
||
165 | * @param ProviderRecord $idp |
||
166 | * @param Settings $settings |
||
167 | * @throws UserException |
||
168 | * @throws \Throwable |
||
169 | */ |
||
170 | 3 | protected function construct( |
|
210 | |||
211 | /** |
||
212 | * @param UserElement $user |
||
213 | * @param SamlResponse $response |
||
214 | * @return UserElement |
||
215 | */ |
||
216 | 3 | protected function transform( |
|
256 | |||
257 | /** |
||
258 | * @param UserElement $user |
||
259 | * @param $attributeName |
||
260 | * @param $attributeValue |
||
261 | * @param $craftProperty |
||
262 | */ |
||
263 | 3 | protected function assignProperty( |
|
299 | |||
300 | /** |
||
301 | * @param UserElement $user |
||
302 | * @return Field|null |
||
303 | */ |
||
304 | 3 | protected function getFieldLayoutField(UserElement $user, $fieldHandle) |
|
320 | |||
321 | /** |
||
322 | * @param UserElement $user |
||
323 | * @param string $name |
||
324 | * @param mixed $value |
||
325 | */ |
||
326 | 3 | private function setSimpleProperty(UserElement $user, $name, $value) |
|
345 | |||
346 | /************************************************** |
||
347 | * Craft User Methods |
||
348 | **************************************************/ |
||
349 | |||
350 | /** |
||
351 | * @param $username |
||
352 | * @return UserElement |
||
353 | * @throws UserException |
||
354 | */ |
||
355 | 3 | protected function find($username) |
|
356 | { |
||
357 | 3 | return $this->forceGet($username); |
|
358 | } |
||
359 | |||
360 | /** |
||
361 | * @param $username |
||
362 | * @return UserElement |
||
363 | * @throws UserException |
||
364 | */ |
||
365 | 3 | protected function forceGet($username) |
|
366 | { |
||
367 | |||
368 | /** |
||
369 | * Is there a user that exists already? |
||
370 | */ |
||
371 | 3 | if (!($user = $this->getByUsernameOrEmail($username))) { |
|
372 | // Should we create a new user? what's the setting say? |
||
373 | 3 | if (! Saml::getInstance()->getSettings()->createUser) { |
|
374 | throw new UserException("System doesn't have permission to create a new user."); |
||
375 | } |
||
376 | |||
377 | // new user! |
||
378 | 3 | $user = new UserElement( |
|
379 | [ |
||
380 | 3 | 'username' => $username, |
|
381 | ] |
||
382 | ); |
||
383 | } |
||
384 | |||
385 | 3 | return $user; |
|
386 | } |
||
387 | |||
388 | /** |
||
389 | * @param $usernameOrEmail |
||
390 | * @param bool $archived |
||
391 | * @return array|bool|\craft\base\ElementInterface|UserElement|null |
||
392 | */ |
||
393 | 3 | protected function getByUsernameOrEmail($usernameOrEmail, $archived = false) |
|
394 | { |
||
395 | |||
396 | 3 | return UserElement::find() |
|
397 | 3 | ->where( |
|
398 | [ |
||
399 | 3 | 'or', |
|
400 | 3 | ['username' => $usernameOrEmail], |
|
401 | 3 | ['email' => $usernameOrEmail], |
|
402 | ] |
||
403 | ) |
||
404 | 3 | ->status(null) |
|
405 | 3 | ->archived($archived) |
|
406 | 3 | ->one(); |
|
407 | } |
||
408 | |||
409 | private function getAttributeValue($attributeValue) |
||
418 | } |
||
419 |
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.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.