Completed
Push — master ( c88112...de5691 )
by Morris
22:03 queued 30s
created

Database   B

Complexity

Total Complexity 42

Size/Duplication

Total Lines 332
Duplicated Lines 7.53 %

Coupling/Cohesion

Components 1
Dependencies 15

Importance

Changes 0
Metric Value
dl 25
loc 332
rs 7.6142
c 0
b 0
f 0
wmc 42
lcom 1
cbo 15

17 Methods

Rating   Name   Duplication   Size   Complexity  
A getBackendName() 0 3 1
B preLoginNameUsedAsUserName() 0 18 5
A __construct() 0 4 2
A createUser() 0 19 4
A deleteUser() 11 11 3
A setPassword() 0 12 3
A setDisplayName() 11 11 2
A getDisplayName() 0 4 2
B getDisplayNames() 0 29 2
A checkPassword() 0 19 4
B loadUser() 0 32 5
A getUsers() 0 6 1
A userExists() 0 4 1
A getHome() 3 7 2
A hasUserListings() 0 3 1
A countUsers() 0 9 2
A loginName2UserName() 0 7 2

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like Database 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 Database, and based on these observations, apply Extract Interface, too.

1
<?php
2
/**
3
 * @copyright Copyright (c) 2016, ownCloud, Inc.
4
 *
5
 * @author adrien <[email protected]>
6
 * @author Aldo "xoen" Giambelluca <[email protected]>
7
 * @author Arthur Schiwon <[email protected]>
8
 * @author Bart Visscher <[email protected]>
9
 * @author Bjoern Schiessle <[email protected]>
10
 * @author Björn Schießle <[email protected]>
11
 * @author fabian <[email protected]>
12
 * @author Georg Ehrke <[email protected]>
13
 * @author Jakob Sack <[email protected]>
14
 * @author Joas Schilling <[email protected]>
15
 * @author Jörn Friedrich Dreyer <[email protected]>
16
 * @author Loki3000 <[email protected]>
17
 * @author Lukas Reschke <[email protected]>
18
 * @author Michael Gapczynski <[email protected]>
19
 * @author michag86 <[email protected]>
20
 * @author Morris Jobke <[email protected]>
21
 * @author nishiki <[email protected]>
22
 * @author Robin Appelman <[email protected]>
23
 * @author Robin McCorkell <[email protected]>
24
 * @author Roeland Jago Douma <[email protected]>
25
 * @author Thomas Müller <[email protected]>
26
 * @author Vincent Petry <[email protected]>
27
 *
28
 * @license AGPL-3.0
29
 *
30
 * This code is free software: you can redistribute it and/or modify
31
 * it under the terms of the GNU Affero General Public License, version 3,
32
 * as published by the Free Software Foundation.
33
 *
34
 * This program is distributed in the hope that it will be useful,
35
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
36
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
37
 * GNU Affero General Public License for more details.
38
 *
39
 * You should have received a copy of the GNU Affero General Public License, version 3,
40
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
41
 *
42
 */
43
44
/*
45
 *
46
 * The following SQL statement is just a help for developers and will not be
47
 * executed!
48
 *
49
 * CREATE TABLE `users` (
50
 *   `uid` varchar(64) COLLATE utf8_unicode_ci NOT NULL,
51
 *   `password` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
52
 *   PRIMARY KEY (`uid`)
53
 * ) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
54
 *
55
 */
56
57
namespace OC\User;
58
59
use OC\Cache\CappedMemoryCache;
60
use OC\DB\QueryBuilder\Literal;
61
use OCP\IUserBackend;
62
use OCP\Util;
63
use Symfony\Component\EventDispatcher\EventDispatcher;
64
use Symfony\Component\EventDispatcher\GenericEvent;
65
66
/**
67
 * Class for user management in a SQL Database (e.g. MySQL, SQLite)
68
 */
69
class Database extends Backend implements IUserBackend {
70
	/** @var CappedMemoryCache */
71
	private $cache;
72
73
	/** @var EventDispatcher */
74
	private $eventDispatcher;
75
76
	/**
77
	 * \OC\User\Database constructor.
78
	 *
79
	 * @param EventDispatcher $eventDispatcher
80
	 */
81
	public function __construct($eventDispatcher = null) {
82
		$this->cache = new CappedMemoryCache();
83
		$this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->getEventDispatcher();
0 ignored issues
show
Documentation Bug introduced by
It seems like $eventDispatcher ? $even...r->getEventDispatcher() can also be of type object<Symfony\Component...entDispatcherInterface>. However, the property $eventDispatcher is declared as type object<Symfony\Component...atcher\EventDispatcher>. 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...
84
	}
85
86
	/**
87
	 * Create a new user
88
	 *
89
	 * @param string $uid The username of the user to create
90
	 * @param string $password The password of the new user
91
	 * @return bool
92
	 *
93
	 * Creates a new user. Basic checking of username is done in OC_User
94
	 * itself, not in its subclasses.
95
	 */
96
	public function createUser($uid, $password) {
97
		if (!$this->userExists($uid)) {
98
			$event = new GenericEvent($password);
99
			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
100
			$query = \OC_DB::prepare('INSERT INTO `*PREFIX*users` ( `uid`, `password` ) VALUES( ?, ? )');
101
			try {
102
				$result = $query->execute(array($uid, \OC::$server->getHasher()->hash($password)));
103
			} catch (\Exception $e) {
104
				$result = false;
105
			}
106
107
			// Clear cache
108
			unset($this->cache[$uid]);
109
110
			return $result ? true : false;
111
		}
112
113
		return false;
114
	}
115
116
	/**
117
	 * delete a user
118
	 *
119
	 * @param string $uid The username of the user to delete
120
	 * @return bool
121
	 *
122
	 * Deletes a user
123
	 */
124 View Code Duplication
	public function deleteUser($uid) {
125
		// Delete user-group-relation
126
		$query = \OC_DB::prepare('DELETE FROM `*PREFIX*users` WHERE `uid` = ?');
127
		$result = $query->execute(array($uid));
128
129
		if (isset($this->cache[$uid])) {
130
			unset($this->cache[$uid]);
131
		}
132
133
		return $result ? true : false;
134
	}
135
136
	/**
137
	 * Set password
138
	 *
139
	 * @param string $uid The username
140
	 * @param string $password The new password
141
	 * @return bool
142
	 *
143
	 * Change the password of a user
144
	 */
145
	public function setPassword($uid, $password) {
146
		if ($this->userExists($uid)) {
147
			$event = new GenericEvent($password);
148
			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
149
			$query = \OC_DB::prepare('UPDATE `*PREFIX*users` SET `password` = ? WHERE `uid` = ?');
150
			$result = $query->execute(array(\OC::$server->getHasher()->hash($password), $uid));
151
152
			return $result ? true : false;
153
		}
154
155
		return false;
156
	}
157
158
	/**
159
	 * Set display name
160
	 *
161
	 * @param string $uid The username
162
	 * @param string $displayName The new display name
163
	 * @return bool
164
	 *
165
	 * Change the display name of a user
166
	 */
167 View Code Duplication
	public function setDisplayName($uid, $displayName) {
168
		if ($this->userExists($uid)) {
169
			$query = \OC_DB::prepare('UPDATE `*PREFIX*users` SET `displayname` = ? WHERE LOWER(`uid`) = LOWER(?)');
170
			$query->execute(array($displayName, $uid));
171
			$this->cache[$uid]['displayname'] = $displayName;
172
173
			return true;
174
		}
175
176
		return false;
177
	}
178
179
	/**
180
	 * get display name of the user
181
	 *
182
	 * @param string $uid user ID of the user
183
	 * @return string display name
184
	 */
185
	public function getDisplayName($uid) {
186
		$this->loadUser($uid);
187
		return empty($this->cache[$uid]['displayname']) ? $uid : $this->cache[$uid]['displayname'];
188
	}
189
190
	/**
191
	 * Get a list of all display names and user ids.
192
	 *
193
	 * @param string $search
194
	 * @param string|null $limit
195
	 * @param string|null $offset
196
	 * @return array an array of all displayNames (value) and the corresponding uids (key)
197
	 */
198
	public function getDisplayNames($search = '', $limit = null, $offset = null) {
199
		$connection = \OC::$server->getDatabaseConnection();
200
201
		$query = $connection->getQueryBuilder();
202
203
		$query->select('uid', 'displayname')
204
			->from('users', 'u')
205
			->leftJoin('u', 'preferences', 'p', $query->expr()->andX(
0 ignored issues
show
Documentation introduced by
$query->expr()->andX($qu...)->eq('userid', 'uid')) is of type object<OCP\DB\QueryBuilder\ICompositeExpression>, but the function expects a string|null.

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...
206
				$query->expr()->eq('userid', 'uid')),
207
				$query->expr()->eq('appid', new Literal('settings')),
208
				$query->expr()->eq('configkey', new Literal('email'))
209
			)
210
			// sqlite doesn't like re-using a single named parameter here
211
			->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
212
			->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
213
			->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
214
			->orderBy($query->func()->lower('displayname'), 'ASC')
215
			->orderBy($query->func()->lower('uid'), 'ASC')
216
			->setMaxResults($limit)
217
			->setFirstResult($offset);
218
219
		$result = $query->execute();
220
		$displayNames = [];
221
		while ($row = $result->fetch()) {
222
			$displayNames[$row['uid']] = $row['displayname'];
223
		}
224
225
		return $displayNames;
226
	}
227
228
	/**
229
	 * Check if the password is correct
230
	 *
231
	 * @param string $uid The username
232
	 * @param string $password The password
233
	 * @return string
234
	 *
235
	 * Check if the password is correct without logging in the user
236
	 * returns the user id or false
237
	 */
238
	public function checkPassword($uid, $password) {
239
		$query = \OC_DB::prepare('SELECT `uid`, `password` FROM `*PREFIX*users` WHERE LOWER(`uid`) = LOWER(?)');
240
		$result = $query->execute(array($uid));
241
242
		$row = $result->fetchRow();
243
		if ($row) {
244
			$storedHash = $row['password'];
245
			$newHash = '';
246
			if (\OC::$server->getHasher()->verify($password, $storedHash, $newHash)) {
247
				if (!empty($newHash)) {
248
					$this->setPassword($uid, $password);
249
				}
250
				return $row['uid'];
251
			}
252
253
		}
254
255
		return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by OC\User\Database::checkPassword of type string.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

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

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
256
	}
257
258
	/**
259
	 * Load an user in the cache
260
	 *
261
	 * @param string $uid the username
262
	 * @return boolean true if user was found, false otherwise
263
	 */
264
	private function loadUser($uid) {
265
		$uid = (string)$uid;
266
		if (!isset($this->cache[$uid])) {
267
			//guests $uid could be NULL or ''
268
			if ($uid === '') {
269
				$this->cache[$uid] = false;
270
				return true;
271
			}
272
273
			$query = \OC_DB::prepare('SELECT `uid`, `displayname` FROM `*PREFIX*users` WHERE LOWER(`uid`) = LOWER(?)');
274
			$result = $query->execute(array($uid));
275
276
			if ($result === false) {
277
				Util::writeLog('core', \OC_DB::getErrorMessage(), Util::ERROR);
278
				return false;
279
			}
280
281
			$this->cache[$uid] = false;
282
283
			// "uid" is primary key, so there can only be a single result
284
			if ($row = $result->fetchRow()) {
285
				$this->cache[$uid]['uid'] = $row['uid'];
286
				$this->cache[$uid]['displayname'] = $row['displayname'];
287
				$result->closeCursor();
0 ignored issues
show
Documentation Bug introduced by
The method closeCursor does not exist on object<OC_DB_StatementWrapper>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
288
			} else {
289
				$result->closeCursor();
0 ignored issues
show
Documentation Bug introduced by
The method closeCursor does not exist on object<OC_DB_StatementWrapper>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
290
				return false;
291
			}
292
		}
293
294
		return true;
295
	}
296
297
	/**
298
	 * Get a list of all users
299
	 *
300
	 * @param string $search
301
	 * @param null|int $limit
302
	 * @param null|int $offset
303
	 * @return string[] an array of all uids
304
	 */
305
	public function getUsers($search = '', $limit = null, $offset = null) {
306
		$users = $this->getDisplayNames($search, $limit, $offset);
307
		$userIds = array_keys($users);
308
		sort($userIds, SORT_STRING | SORT_FLAG_CASE);
309
		return $userIds;
310
	}
311
312
	/**
313
	 * check if a user exists
314
	 *
315
	 * @param string $uid the username
316
	 * @return boolean
317
	 */
318
	public function userExists($uid) {
319
		$this->loadUser($uid);
320
		return $this->cache[$uid] !== false;
321
	}
322
323
	/**
324
	 * get the user's home directory
325
	 *
326
	 * @param string $uid the username
327
	 * @return string|false
328
	 */
329
	public function getHome($uid) {
330 View Code Duplication
		if ($this->userExists($uid)) {
331
			return \OC::$server->getConfig()->getSystemValue("datadirectory", \OC::$SERVERROOT . "/data") . '/' . $uid;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return \OC::$server->get... '/data') . '/' . $uid; (string) is incompatible with the return type of the parent method OC\User\Backend::getHome of type boolean.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

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

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
332
		}
333
334
		return false;
335
	}
336
337
	/**
338
	 * @return bool
339
	 */
340
	public function hasUserListings() {
341
		return true;
342
	}
343
344
	/**
345
	 * counts the users in the database
346
	 *
347
	 * @return int|bool
348
	 */
349
	public function countUsers() {
350
		$query = \OC_DB::prepare('SELECT COUNT(*) FROM `*PREFIX*users`');
351
		$result = $query->execute();
352
		if ($result === false) {
353
			Util::writeLog('core', \OC_DB::getErrorMessage(), Util::ERROR);
354
			return false;
355
		}
356
		return $result->fetchOne();
357
	}
358
359
	/**
360
	 * returns the username for the given login name in the correct casing
361
	 *
362
	 * @param string $loginName
363
	 * @return string|false
364
	 */
365
	public function loginName2UserName($loginName) {
366
		if ($this->userExists($loginName)) {
367
			return $this->cache[$loginName]['uid'];
368
		}
369
370
		return false;
371
	}
372
373
	/**
374
	 * Backend name to be shown in user management
375
	 *
376
	 * @return string the name of the backend to be shown
377
	 */
378
	public function getBackendName() {
379
		return 'Database';
380
	}
381
382
	public static function preLoginNameUsedAsUserName($param) {
383
		if (!isset($param['uid'])) {
384
			throw new \Exception('key uid is expected to be set in $param');
385
		}
386
387
		$backends = \OC::$server->getUserManager()->getBackends();
388
		foreach ($backends as $backend) {
389
			if ($backend instanceof Database) {
390
				/** @var \OC\User\Database $backend */
391
				$uid = $backend->loginName2UserName($param['uid']);
392
				if ($uid !== false) {
393
					$param['uid'] = $uid;
394
					return;
395
				}
396
			}
397
		}
398
399
	}
400
}
401