AlbumBusinessLayer::findAll() - Code Metrics - Inspection of "Another small optimization for Subsonic `getAlbumL..." - owncloud/music - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 73b23f...6d12fc )

by Pauli

created 2021-07-28 13:54 UTC

AlbumBusinessLayer::findAll() B

↳ Parent: AlbumBusinessLayer

Complexity

Conditions	7
Paths	12

Size

Total Lines	6
Code Lines	4

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
cc	7
eloc	4
c	1
b	0
f	0
nc	12
nop	8
dl	0
loc	6
rs	8.8333

How to fix Many Parameters

<?php declare(strict_types=1);

/**
 * ownCloud - Music app
 *
 * This file is licensed under the Affero General Public License version 3 or
 * later. See the COPYING file.
 *
 * @author Morris Jobke <[email protected]>
 * @author Pauli Järvinen <[email protected]>
 * @copyright Morris Jobke 2013, 2014
 * @copyright Pauli Järvinen 2016 - 2021
 */

namespace OCA\Music\BusinessLayer;

use \OCA\Music\AppFramework\BusinessLayer\BusinessLayer;
use \OCA\Music\AppFramework\BusinessLayer\BusinessLayerException;
use \OCA\Music\AppFramework\Core\Logger;

use \OCA\Music\Db\AlbumMapper;
use \OCA\Music\Db\Album;
use \OCA\Music\Db\SortBy;

use \OCA\Music\Utility\Util;

use \OCP\AppFramework\Db\Entity;

class AlbumBusinessLayer extends BusinessLayer {
	protected $mapper; // eclipse the definition from the base class, to help IDE and Scrutinizer to know the actual type
	private $logger;

	public function __construct(AlbumMapper $albumMapper, Logger $logger) {
		parent::__construct($albumMapper);
		$this->mapper = $albumMapper;
		$this->logger = $logger;
	}

	/**
	 * {@inheritdoc}
	 * @see BusinessLayer::find()
	 * @return Album
	 */
	public function find(int $albumId, string $userId) : Entity {
		$album = parent::find($albumId, $userId);
		return $this->injectExtraFields([$album], $userId)[0];
	}

	/**
	 * {@inheritdoc}
	 * @see BusinessLayer::findAll()
	 * @return Album[]
	 */
	public function findAll(string $userId, int $sortBy=SortBy::None, int $limit=null, int $offset=null,
							?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array {
		$albums = parent::findAll($userId, $sortBy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax);
		$effectivelyLimited = ($limit !== null && $limit < \count($albums));
		$everyAlbumIncluded = (!$effectivelyLimited && !$offset && !$createdMin && !$createdMax && !$updatedMin && !$updatedMax);
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
		return $this->injectExtraFields($albums, $userId, $everyAlbumIncluded);
	}

	/**
	 * Returns all albums filtered by artist (both album and track artists are considered)
	 * @param integer $artistId the id of the artist
	 * @param string $userId the name of the user
	 * @return Album[] albums
	 */
	public function findAllByArtist(int $artistId, string $userId) : array {
		$albums = $this->mapper->findAllByArtist($artistId, $userId);
		return $this->injectExtraFields($albums, $userId);
	}

	/**
	 * Returns all albums filtered by album artist
	 * @param integer $artistId the id of the artist
	 * @param string $userId the name of the user
	 * @return Album[] albums
	 */
	public function findAllByAlbumArtist(int $artistId, string $userId) : array {
		$albums = $this->mapper->findAllByAlbumArtist($artistId, $userId);
		$albums = $this->injectExtraFields($albums, $userId);
		\usort($albums, ['\OCA\Music\Db\Album', 'compareYearAndName']);
		return $albums;
	}

	/**
	 * Returns all albums filtered by genre
	 * @param int $genreId the genre to include
	 * @param string $userId the name of the user
	 * @param int|null $limit
	 * @param int|null $offset
	 * @return Album[] albums
	 */
	public function findAllByGenre(int $genreId, string $userId, int $limit=null, int $offset=null) : array {
		$albums = $this->mapper->findAllByGenre($genreId, $userId, $limit, $offset);
		return $this->injectExtraFields($albums, $userId);
	}

	/**
	 * Returns all albums filtered by release year
	 * @param int $fromYear
	 * @param int $toYear
	 * @param string $userId the name of the user
	 * @param int|null $limit
	 * @param int|null $offset
	 * @return Album[] albums
	 */
	public function findAllByYearRange(
			int $fromYear, int $toYear, string $userId, int $limit=null, int $offset=null) : array {
		$reverseOrder = false;
		if ($fromYear > $toYear) {
			$reverseOrder = true;
			Util::swap($fromYear, $toYear);
		}

		// Implement all the custom logic of this function here, without special Mapper function
		$albums = \array_filter($this->findAll($userId), function ($album) use ($fromYear, $toYear) {
			$years = $album->getYears();
			return (!empty($years) && \min($years) <= $toYear && \max($years) >= $fromYear);
		});

		\usort($albums, function ($album1, $album2) use ($reverseOrder) {
			return $reverseOrder
				? $album2->yearToAPI() - $album1->yearToAPI()
				: $album1->yearToAPI() - $album2->yearToAPI();
		});

		if ($limit !== null || $offset !== null) {
			$albums = \array_slice($albums, $offset ?: 0, $limit);
		}

		return $albums;
	}

	/**
	 * {@inheritdoc}
	 * @see BusinessLayer::findAllByName()
	 * @return Album[]
	 */
	public function findAllByName(
			string $name, string $userId, bool $fuzzy = false, int $limit=null, int $offset=null,
			?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array {
		$albums = parent::findAllByName($name, $userId, $fuzzy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax);
		return $this->injectExtraFields($albums, $userId);
	}

	/**
	 * Add performing artists, release years, genres, and disk counts to the given album objects
	 * @param Album[] $albums
	 * @param string $userId
	 * @param bool $allAlbums Set to true if $albums contains all albums of the user.
	 *                        This has now effect on the outcome but helps in optimizing
	 *                        the database query.
	 * @return Album[]
	 */
	private function injectExtraFields(array $albums, string $userId, bool $allAlbums = false) : array {
		if (\count($albums) > 0) {
			// In case we are injecting data to a lot of albums, do not limit the
			// SQL SELECTs to only those albums. Very large amount of SQL host parameters
			// could cause problems with SQLite (see #239) and probably it would be bad for
			// performance also on other DBMSs. For the proper operation of this function,
			// it doesn't matter if we fetch data for some extra albums.
			$albumIds = ($allAlbums || \count($albums) >= 999)
					? null : Util::extractIds($albums);

			$artists = $this->mapper->getPerformingArtistsByAlbumId($albumIds, $userId);
			$years = $this->mapper->getYearsByAlbumId($albumIds, $userId);
			$diskCounts = $this->mapper->getDiscCountByAlbumId($albumIds, $userId);
			$genres = $this->mapper->getGenresByAlbumId($albumIds, $userId);

			foreach ($albums as &$album) {
				$albumId = $album->getId();
				$album->setArtistIds($artists[$albumId] ?? []);
				$album->setNumberOfDisks($diskCounts[$albumId] ?? 1);
				$album->setGenres($genres[$albumId] ?? null);
				$album->setYears($years[$albumId] ?? null);
			}
		}
		return $albums;
	}

	/**
	 * Returns the count of albums where the given Artist is featured in
	 * @param integer $artistId
	 * @return integer
	 */
	public function countByArtist(int $artistId) : int {
		return $this->mapper->countByArtist($artistId);
	}

	/**
	 * Returns the count of albums where the given artist is the album artist
	 * @param integer $artistId
	 * @return integer
	 */
	public function countByAlbumArtist(int $artistId) : int {
		return $this->mapper->countByAlbumArtist($artistId);
	}

	public function findAlbumOwner(int $albumId) : string {
		$entities = $this->findById([$albumId]);
		if (\count($entities) != 1) {
			throw new BusinessLayerException(
					'Expected to find one album but got ' . \count($entities));
		} else {
			return $entities[0]->getUserId();
		}
	}

	/**
	 * Adds an album if it does not exist already or updates an existing album
	 * @param string|null $name the name of the album
	 * @param integer $albumArtistId
	 * @param string $userId
	 * @return Album The added/updated album
	 */
	public function addOrUpdateAlbum(?string $name, int $albumArtistId, string $userId) : Album {
		$album = new Album();
		$album->setName(Util::truncate($name, 256)); // some DB setups can't truncate automatically to column max size
		$album->setUserId($userId);
		$album->setAlbumArtistId($albumArtistId);

		// Generate hash from the set of fields forming the album identity to prevent duplicates.
		// The uniqueness of album name is evaluated in case-insensitive manner.
		$lowerName = \mb_strtolower($album->getName() ?? '');
		$hash = \hash('md5', "$lowerName|$albumArtistId");
		$album->setHash($hash);

		return $this->mapper->insertOrUpdate($album);
	}

	/**
	 * Check if given file is used as cover for the given album
	 * @param int $albumId
	 * @param int[] $fileIds
	 * @return boolean
	 */
	public function albumCoverIsOneOfFiles(int $albumId, array $fileIds) : bool {
		$albums = $this->findById([$albumId]);
		return (\count($albums) && \in_array($albums[0]->getCoverFileId(), $fileIds));
	}

	/**
	 * updates the cover for albums in the specified folder without cover
	 * @param integer $coverFileId the file id of the cover image
	 * @param integer $folderId the file id of the folder where the albums are looked from
	 * @return boolean True if one or more albums were influenced
	 */
	public function updateFolderCover(int $coverFileId, int $folderId) : bool {
		return $this->mapper->updateFolderCover($coverFileId, $folderId);
	}

	/**
	 * set cover file for a specified album
	 * @param int|null $coverFileId the file id of the cover image
	 * @param int $albumId the id of the album to be modified
	 */
	public function setCover(?int $coverFileId, int $albumId) {
		$this->mapper->setCover($coverFileId, $albumId);
	}

	/**
	 * removes the cover art from albums, replacement covers will be searched in a background task
	 * @param integer[] $coverFileIds the file IDs of the cover images
	 * @param string[]|null $userIds the users whose music library is targeted; all users are targeted if omitted
	 * @return Album[] albums which got modified, empty array if none
	 */
	public function removeCovers(array $coverFileIds, array $userIds=null) : array {
		return $this->mapper->removeCovers($coverFileIds, $userIds);
	}

	/**
	 * try to find cover arts for albums without covers
	 * @param string|null $userId target user; omit to target all users
	 * @return string[] users whose collections got modified
	 */
	public function findCovers(string $userId = null) : array {
		$affectedUsers = [];
		$albums = $this->mapper->getAlbumsWithoutCover($userId);
		foreach ($albums as $album) {
			if ($this->mapper->findAlbumCover($album['albumId'], $album['parentFolderId'])) {
				$affectedUsers[$album['userId']] = 1;
			}
		}
		return \array_keys($affectedUsers);
	}
}


1			<?php declare(strict_types=1);
2
3			/**
4			* ownCloud - Music app
5			*
6			* This file is licensed under the Affero General Public License version 3 or
7			* later. See the COPYING file.
8			*
9			* @author Morris Jobke <[email protected]>
10			* @author Pauli Järvinen <[email protected]>
11			* @copyright Morris Jobke 2013, 2014
12			* @copyright Pauli Järvinen 2016 - 2021
13			*/
14
15			namespace OCA\Music\BusinessLayer;
16
17			use \OCA\Music\AppFramework\BusinessLayer\BusinessLayer;
18			use \OCA\Music\AppFramework\BusinessLayer\BusinessLayerException;
19			use \OCA\Music\AppFramework\Core\Logger;
20
21			use \OCA\Music\Db\AlbumMapper;
22			use \OCA\Music\Db\Album;
23			use \OCA\Music\Db\SortBy;
24
25			use \OCA\Music\Utility\Util;
26
27			use \OCP\AppFramework\Db\Entity;
28
29			class AlbumBusinessLayer extends BusinessLayer {
30			protected $mapper; // eclipse the definition from the base class, to help IDE and Scrutinizer to know the actual type
31			private $logger;
32
33			public function __construct(AlbumMapper $albumMapper, Logger $logger) {
34			parent::__construct($albumMapper);
35			$this->mapper = $albumMapper;
36			$this->logger = $logger;
37			}
38
39			/**
40			* {@inheritdoc}
41			* @see BusinessLayer::find()
42			* @return Album
43			*/
44			public function find(int $albumId, string $userId) : Entity {
45			$album = parent::find($albumId, $userId);
46			return $this->injectExtraFields([$album], $userId)[0];
47			}
48
49			/**
50			* {@inheritdoc}
51			* @see BusinessLayer::findAll()
52			* @return Album[]
53			*/
54			public function findAll(string $userId, int $sortBy=SortBy::None, int $limit=null, int $offset=null,
55			?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array {
56			$albums = parent::findAll($userId, $sortBy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax);
57			$effectivelyLimited = ($limit !== null && $limit < \count($albums));
58			$everyAlbumIncluded = (!$effectivelyLimited && !$offset && !$createdMin && !$createdMax && !$updatedMin && !$updatedMax);
			0 ignored issues – show Bug Best Practice introduced 2021-07-28 13:57 UTC by Report Bug Copy Issue Report The expression `$offset` of type `integer\|null` is loosely compared to `false`; this is ambiguous if the integer can be 0. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
59			return $this->injectExtraFields($albums, $userId, $everyAlbumIncluded);
60			}
61
62			/**
63			* Returns all albums filtered by artist (both album and track artists are considered)
64			* @param integer $artistId the id of the artist
65			* @param string $userId the name of the user
66			* @return Album[] albums
67			*/
68			public function findAllByArtist(int $artistId, string $userId) : array {
69			$albums = $this->mapper->findAllByArtist($artistId, $userId);
70			return $this->injectExtraFields($albums, $userId);
71			}
72
73			/**
74			* Returns all albums filtered by album artist
75			* @param integer $artistId the id of the artist
76			* @param string $userId the name of the user
77			* @return Album[] albums
78			*/
79			public function findAllByAlbumArtist(int $artistId, string $userId) : array {
80			$albums = $this->mapper->findAllByAlbumArtist($artistId, $userId);
81			$albums = $this->injectExtraFields($albums, $userId);
82			\usort($albums, ['\OCA\Music\Db\Album', 'compareYearAndName']);
83			return $albums;
84			}
85
86			/**
87			* Returns all albums filtered by genre
88			* @param int $genreId the genre to include
89			* @param string $userId the name of the user
90			* @param int\|null $limit
91			* @param int\|null $offset
92			* @return Album[] albums
93			*/
94			public function findAllByGenre(int $genreId, string $userId, int $limit=null, int $offset=null) : array {
95			$albums = $this->mapper->findAllByGenre($genreId, $userId, $limit, $offset);
96			return $this->injectExtraFields($albums, $userId);
97			}
98
99			/**
100			* Returns all albums filtered by release year
101			* @param int $fromYear
102			* @param int $toYear
103			* @param string $userId the name of the user
104			* @param int\|null $limit
105			* @param int\|null $offset
106			* @return Album[] albums
107			*/
108			public function findAllByYearRange(
109			int $fromYear, int $toYear, string $userId, int $limit=null, int $offset=null) : array {
110			$reverseOrder = false;
111			if ($fromYear > $toYear) {
112			$reverseOrder = true;
113			Util::swap($fromYear, $toYear);
114			}
115
116			// Implement all the custom logic of this function here, without special Mapper function
117			$albums = \array_filter($this->findAll($userId), function ($album) use ($fromYear, $toYear) {
118			$years = $album->getYears();
119			return (!empty($years) && \min($years) <= $toYear && \max($years) >= $fromYear);
120			});
121
122			\usort($albums, function ($album1, $album2) use ($reverseOrder) {
123			return $reverseOrder
124			? $album2->yearToAPI() - $album1->yearToAPI()
125			: $album1->yearToAPI() - $album2->yearToAPI();
126			});
127
128			if ($limit !== null \|\| $offset !== null) {
129			$albums = \array_slice($albums, $offset ?: 0, $limit);
130			}
131
132			return $albums;
133			}
134
135			/**
136			* {@inheritdoc}
137			* @see BusinessLayer::findAllByName()
138			* @return Album[]
139			*/
140			public function findAllByName(
141			string $name, string $userId, bool $fuzzy = false, int $limit=null, int $offset=null,
142			?string $createdMin=null, ?string $createdMax=null, ?string $updatedMin=null, ?string $updatedMax=null) : array {
143			$albums = parent::findAllByName($name, $userId, $fuzzy, $limit, $offset, $createdMin, $createdMax, $updatedMin, $updatedMax);
144			return $this->injectExtraFields($albums, $userId);
145			}
146
147			/**
148			* Add performing artists, release years, genres, and disk counts to the given album objects
149			* @param Album[] $albums
150			* @param string $userId
151			* @param bool $allAlbums Set to true if $albums contains all albums of the user.
152			* This has now effect on the outcome but helps in optimizing
153			* the database query.
154			* @return Album[]
155			*/
156			private function injectExtraFields(array $albums, string $userId, bool $allAlbums = false) : array {
157			if (\count($albums) > 0) {
158			// In case we are injecting data to a lot of albums, do not limit the
159			// SQL SELECTs to only those albums. Very large amount of SQL host parameters
160			// could cause problems with SQLite (see #239) and probably it would be bad for
161			// performance also on other DBMSs. For the proper operation of this function,
162			// it doesn't matter if we fetch data for some extra albums.
163			$albumIds = ($allAlbums \|\| \count($albums) >= 999)
164			? null : Util::extractIds($albums);
165
166			$artists = $this->mapper->getPerformingArtistsByAlbumId($albumIds, $userId);
167			$years = $this->mapper->getYearsByAlbumId($albumIds, $userId);
168			$diskCounts = $this->mapper->getDiscCountByAlbumId($albumIds, $userId);
169			$genres = $this->mapper->getGenresByAlbumId($albumIds, $userId);
170
171			foreach ($albums as &$album) {
172			$albumId = $album->getId();
173			$album->setArtistIds($artists[$albumId] ?? []);
174			$album->setNumberOfDisks($diskCounts[$albumId] ?? 1);
175			$album->setGenres($genres[$albumId] ?? null);
176			$album->setYears($years[$albumId] ?? null);
177			}
178			}
179			return $albums;
180			}
181
182			/**
183			* Returns the count of albums where the given Artist is featured in
184			* @param integer $artistId
185			* @return integer
186			*/
187			public function countByArtist(int $artistId) : int {
188			return $this->mapper->countByArtist($artistId);
189			}
190
191			/**
192			* Returns the count of albums where the given artist is the album artist
193			* @param integer $artistId
194			* @return integer
195			*/
196			public function countByAlbumArtist(int $artistId) : int {
197			return $this->mapper->countByAlbumArtist($artistId);
198			}
199
200			public function findAlbumOwner(int $albumId) : string {
201			$entities = $this->findById([$albumId]);
202			if (\count($entities) != 1) {
203			throw new BusinessLayerException(
204			'Expected to find one album but got ' . \count($entities));
205			} else {
206			return $entities[0]->getUserId();
207			}
208			}
209
210			/**
211			* Adds an album if it does not exist already or updates an existing album
212			* @param string\|null $name the name of the album
213			* @param integer $albumArtistId
214			* @param string $userId
215			* @return Album The added/updated album
216			*/
217			public function addOrUpdateAlbum(?string $name, int $albumArtistId, string $userId) : Album {
218			$album = new Album();
219			$album->setName(Util::truncate($name, 256)); // some DB setups can't truncate automatically to column max size
220			$album->setUserId($userId);
221			$album->setAlbumArtistId($albumArtistId);
222
223			// Generate hash from the set of fields forming the album identity to prevent duplicates.
224			// The uniqueness of album name is evaluated in case-insensitive manner.
225			$lowerName = \mb_strtolower($album->getName() ?? '');
226			$hash = \hash('md5', "$lowerName\|$albumArtistId");
227			$album->setHash($hash);
228
229			return $this->mapper->insertOrUpdate($album);
230			}
231
232			/**
233			* Check if given file is used as cover for the given album
234			* @param int $albumId
235			* @param int[] $fileIds
236			* @return boolean
237			*/
238			public function albumCoverIsOneOfFiles(int $albumId, array $fileIds) : bool {
239			$albums = $this->findById([$albumId]);
240			return (\count($albums) && \in_array($albums[0]->getCoverFileId(), $fileIds));
241			}
242
243			/**
244			* updates the cover for albums in the specified folder without cover
245			* @param integer $coverFileId the file id of the cover image
246			* @param integer $folderId the file id of the folder where the albums are looked from
247			* @return boolean True if one or more albums were influenced
248			*/
249			public function updateFolderCover(int $coverFileId, int $folderId) : bool {
250			return $this->mapper->updateFolderCover($coverFileId, $folderId);
251			}
252
253			/**
254			* set cover file for a specified album
255			* @param int\|null $coverFileId the file id of the cover image
256			* @param int $albumId the id of the album to be modified
257			*/
258			public function setCover(?int $coverFileId, int $albumId) {
259			$this->mapper->setCover($coverFileId, $albumId);
260			}
261
262			/**
263			* removes the cover art from albums, replacement covers will be searched in a background task
264			* @param integer[] $coverFileIds the file IDs of the cover images
265			* @param string[]\|null $userIds the users whose music library is targeted; all users are targeted if omitted
266			* @return Album[] albums which got modified, empty array if none
267			*/
268			public function removeCovers(array $coverFileIds, array $userIds=null) : array {
269			return $this->mapper->removeCovers($coverFileIds, $userIds);
270			}
271
272			/**
273			* try to find cover arts for albums without covers
274			* @param string\|null $userId target user; omit to target all users
275			* @return string[] users whose collections got modified
276			*/
277			public function findCovers(string $userId = null) : array {
278			$affectedUsers = [];
279			$albums = $this->mapper->getAlbumsWithoutCover($userId);
280			foreach ($albums as $album) {
281			if ($this->mapper->findAlbumCover($album['albumId'], $album['parentFolderId'])) {
282			$affectedUsers[$album['userId']] = 1;
283			}
284			}
285			return \array_keys($affectedUsers);
286			}
287			}
288

owncloud / music

Push — master ( 73b23f...6d12fc )

AlbumBusinessLayer::findAll() B

Complexity

Size

Duplication

Importance

How to fix Many Parameters

Many Parameters

Duplication Side-by-Side

Filter issues like