AddMissingImagesTask::execute() - Code Metrics - Inspection of "Merge pull request #51 from matiasdelellis/excepti..." - matiasdelellis/facerecognition - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( dce9c2...90c6b3 )

by Branko

created 2018-11-24 15:36 UTC

AddMissingImagesTask::execute() A

↳ Parent: AddMissingImagesTask

Complexity

Conditions	5
Paths	9

Size

Total Lines

Duplication

Lines	7
Ratio	21.88 %

Code Coverage

Tests	0
CRAP Score	30

Importance

Changes

Metric	Value
dl	7
loc	32
ccs	0
cts	24
cp	0
rs	9.0968
c	0
b	0
f	0
cc	5
nc	9
nop	1
crap	30

<?php
/**
 * @copyright Copyright (c) 2017, Matias De lellis <[email protected]>
 * @copyright Copyright (c) 2018, Branko Kokanovic <[email protected]>
 *
 * @author Branko Kokanovic <[email protected]>
 *
 * @license GNU AGPL version 3 or any later version
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 */
namespace OCA\FaceRecognition\BackgroundJob\Tasks;

use OCP\IConfig;
use OCP\IUser;

use OCP\Files\File;
use OCP\Files\Folder;

use OCA\FaceRecognition\BackgroundJob\FaceRecognitionBackgroundTask;
use OCA\FaceRecognition\BackgroundJob\FaceRecognitionContext;
use OCA\FaceRecognition\Db\Image;
use OCA\FaceRecognition\Db\ImageMapper;
use OCA\FaceRecognition\Helper\Requirements;
use OCA\FaceRecognition\Migration\AddDefaultFaceModel;

/**
 * Task that, for each user, crawls for all images in filesystem and insert them in database.
 * This is job that normally does file watcher, but this should be done at least once,
 * after app is installed (or re-enabled).
 */
class AddMissingImagesTask extends FaceRecognitionBackgroundTask {
	const FULL_IMAGE_SCAN_DONE_KEY = "full_image_scan_done";

	/** @var IConfig Config */
	private $config;

	/** @var ImageMapper Image mapper */
	private $imageMapper;

	/**
	 * @param IConfig $config Config
	 * @param ImageMapper $imageMapper Image mapper
	 */
	public function __construct(IConfig $config, ImageMapper $imageMapper) {
		parent::__construct();
		$this->config = $config;
		$this->imageMapper = $imageMapper;
	}

	/**
	 * @inheritdoc
	 */
	public function description() {
		return "Crawl for missing images for each user and insert them in DB";
	}

	/**
	 * @inheritdoc
	 */
	public function execute(FaceRecognitionContext $context) {
		$this->setContext($context);

		$fullImageScanDone = $this->config->getAppValue('facerecognition', AddMissingImagesTask::FULL_IMAGE_SCAN_DONE_KEY, 'false');
		if ($fullImageScanDone == 'true') {
			// Completely skip this task, seems that we already did full scan
			return true;
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());
}
		}

		$model = intval($this->config->getAppValue('facerecognition', 'model', AddDefaultFaceModel::DEFAULT_FACE_MODEL_ID));

		// Check if we are called for one user only, or for all user in instance.
		$eligable_users = array();
		if (is_null($this->context->user)) {
			$this->context->userManager->callForSeenUsers(function (IUser $user) use (&$eligable_users) {
				$eligable_users[] = $user->getUID();
			});
		} else {
			$eligable_users[] = $this->context->user->getUID();
		}

		foreach($eligable_users as $user) {
			$this->addMissingImagesForUser($user, $model);
			yield;
		}

		if (is_null($this->context->user)) {
			$this->config->setAppValue('facerecognition', AddMissingImagesTask::FULL_IMAGE_SCAN_DONE_KEY, 'true');
		}

		return true;
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());
}
	}

	/**
	 * Crawl filesystem for a given user
	 * TODO: duplicated from Queue.php, figure out how to merge
	 * (or delete this Queue.php when not needed)
	 *
	 * @param string $userId ID of the user for which to crawl images for
	 * @param int $model Used model
	 */
	private function addMissingImagesForUser(string $userId, int $model) {
		$this->logInfo(sprintf('Finding missing images for user %s', $userId));
		\OC_Util::tearDownFS();
		\OC_Util::setupFS($userId);

		$userFolder = $this->context->rootFolder->getUserFolder($userId);
		$this->parseUserFolder($model, $userFolder);
	}

	/**
	 * Recursively crawls given folder for a given user
	 *
	 * @param int $model Used model
	 * @param Folder $folder Folder to recursively search images in
	 */
	private function parseUserFolder(int $model, Folder $folder) {
		$nodes = $this->getPicturesFromFolder($folder);
		foreach ($nodes as $file) {
			$this->logDebug('Found ' . $file->getPath());

			$image = new Image();
			$image->setUser($file->getOwner()->getUid());
			$image->setFile($file->getId());
			$image->setModel($model);
			// todo: this check/insert logic for each image is so inefficient it hurts my mind
			if ($this->imageMapper->imageExists($image) == null) {
				// todo: can we have larger transaction with bulk insert?
				$this->imageMapper->insert($image);
			}
		}
	}

	/**
	 * Return all images from a given folder.
	 *
	 * TODO: It is inefficient since it copies the array recursively.
	 *
	 * @param Folder $folder Folder to get images from
	 * @return array List of all images and folders to continue recursive crawling
	 */
	private function getPicturesFromFolder(Folder $folder, $results = array()) {
		$nodes = $folder->getDirectoryListing();

		foreach ($nodes as $node) {
			//if ($node->isHidden())
			//	continue;
			// $previewImage = new \OC_Image();
			// $previewImage->loadFromData($preview->getContent());
			if ($node instanceof Folder and !$node->nodeExists('.nomedia')) {
				$results = $this->getPicturesFromFolder($node, $results);
			} else if ($node instanceof File) {
				if (Requirements::isImageTypeSupported($node->getMimeType())) {
					$results[] = $node;
				}
			}
		}

		return $results;
	}
}


1		<?php
2		/**
3		* @copyright Copyright (c) 2017, Matias De lellis <[email protected]>
4		* @copyright Copyright (c) 2018, Branko Kokanovic <[email protected]>
5		*
6		* @author Branko Kokanovic <[email protected]>
7		*
8		* @license GNU AGPL version 3 or any later version
9		*
10		* This program is free software: you can redistribute it and/or modify
11		* it under the terms of the GNU Affero General Public License as
12		* published by the Free Software Foundation, either version 3 of the
13		* License, or (at your option) any later version.
14		*
15		* This program is distributed in the hope that it will be useful,
16		* but WITHOUT ANY WARRANTY; without even the implied warranty of
17		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18		* GNU Affero General Public License for more details.
19		*
20		* You should have received a copy of the GNU Affero General Public License
21		* along with this program. If not, see <http://www.gnu.org/licenses/>.
22		*
23		*/
24		namespace OCA\FaceRecognition\BackgroundJob\Tasks;
25
26		use OCP\IConfig;
27		use OCP\IUser;
28
29		use OCP\Files\File;
30		use OCP\Files\Folder;
31
32		use OCA\FaceRecognition\BackgroundJob\FaceRecognitionBackgroundTask;
33		use OCA\FaceRecognition\BackgroundJob\FaceRecognitionContext;
34		use OCA\FaceRecognition\Db\Image;
35		use OCA\FaceRecognition\Db\ImageMapper;
36		use OCA\FaceRecognition\Helper\Requirements;
37		use OCA\FaceRecognition\Migration\AddDefaultFaceModel;
38
39		/**
40		* Task that, for each user, crawls for all images in filesystem and insert them in database.
41		* This is job that normally does file watcher, but this should be done at least once,
42		* after app is installed (or re-enabled).
43		*/
44		class AddMissingImagesTask extends FaceRecognitionBackgroundTask {
45		const FULL_IMAGE_SCAN_DONE_KEY = "full_image_scan_done";
46
47		/** @var IConfig Config */
48		private $config;
49
50		/** @var ImageMapper Image mapper */
51		private $imageMapper;
52
53		/**
54		* @param IConfig $config Config
55		* @param ImageMapper $imageMapper Image mapper
56		*/
57		public function __construct(IConfig $config, ImageMapper $imageMapper) {
58		parent::__construct();
59		$this->config = $config;
60		$this->imageMapper = $imageMapper;
61		}
62
63		/**
64		* @inheritdoc
65		*/
66		public function description() {
67		return "Crawl for missing images for each user and insert them in DB";
68		}
69
70		/**
71		* @inheritdoc
72		*/
73		public function execute(FaceRecognitionContext $context) {
74		$this->setContext($context);
75
76		$fullImageScanDone = $this->config->getAppValue('facerecognition', AddMissingImagesTask::FULL_IMAGE_SCAN_DONE_KEY, 'false');
77		if ($fullImageScanDone == 'true') {
78		// Completely skip this task, seems that we already did full scan
79		return true;
		0 ignored issues – show Bug Best Practice introduced 2018-11-21 09:42 UTC by Report Bug Copy Issue Report The return type of `return true;` (`boolean`) is incompatible with the return type documented by `OCA\FaceRecognition\Back...singImagesTask::execute` of type `Generator`. 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...
80		}
81
82		$model = intval($this->config->getAppValue('facerecognition', 'model', AddDefaultFaceModel::DEFAULT_FACE_MODEL_ID));
83
84		// Check if we are called for one user only, or for all user in instance.
85		$eligable_users = array();
86	View Code Duplication	if (is_null($this->context->user)) {
87		$this->context->userManager->callForSeenUsers(function (IUser $user) use (&$eligable_users) {
88		$eligable_users[] = $user->getUID();
89		});
90		} else {
91		$eligable_users[] = $this->context->user->getUID();
92		}
93
94		foreach($eligable_users as $user) {
95		$this->addMissingImagesForUser($user, $model);
96		yield;
97		}
98
99		if (is_null($this->context->user)) {
100		$this->config->setAppValue('facerecognition', AddMissingImagesTask::FULL_IMAGE_SCAN_DONE_KEY, 'true');
101		}
102
103		return true;
		0 ignored issues – show Bug Best Practice introduced 2018-11-21 09:42 UTC by Report Bug Copy Issue Report The return type of `return true;` (`boolean`) is incompatible with the return type documented by `OCA\FaceRecognition\Back...singImagesTask::execute` of type `Generator`. 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...
104		}
105
106		/**
107		* Crawl filesystem for a given user
108		* TODO: duplicated from Queue.php, figure out how to merge
109		* (or delete this Queue.php when not needed)
110		*
111		* @param string $userId ID of the user for which to crawl images for
112		* @param int $model Used model
113		*/
114		private function addMissingImagesForUser(string $userId, int $model) {
115		$this->logInfo(sprintf('Finding missing images for user %s', $userId));
116		\OC_Util::tearDownFS();
117		\OC_Util::setupFS($userId);
118
119		$userFolder = $this->context->rootFolder->getUserFolder($userId);
120		$this->parseUserFolder($model, $userFolder);
121		}
122
123		/**
124		* Recursively crawls given folder for a given user
125		*
126		* @param int $model Used model
127		* @param Folder $folder Folder to recursively search images in
128		*/
129		private function parseUserFolder(int $model, Folder $folder) {
130		$nodes = $this->getPicturesFromFolder($folder);
131		foreach ($nodes as $file) {
132		$this->logDebug('Found ' . $file->getPath());
133
134		$image = new Image();
135		$image->setUser($file->getOwner()->getUid());
136		$image->setFile($file->getId());
137		$image->setModel($model);
138		// todo: this check/insert logic for each image is so inefficient it hurts my mind
139		if ($this->imageMapper->imageExists($image) == null) {
140		// todo: can we have larger transaction with bulk insert?
141		$this->imageMapper->insert($image);
142		}
143		}
144		}
145
146		/**
147		* Return all images from a given folder.
148		*
149		* TODO: It is inefficient since it copies the array recursively.
150		*
151		* @param Folder $folder Folder to get images from
152		* @return array List of all images and folders to continue recursive crawling
153		*/
154	View Code Duplication	private function getPicturesFromFolder(Folder $folder, $results = array()) {
155		$nodes = $folder->getDirectoryListing();
156
157		foreach ($nodes as $node) {
158		//if ($node->isHidden())
159		// continue;
160		// $previewImage = new \OC_Image();
161		// $previewImage->loadFromData($preview->getContent());
162		if ($node instanceof Folder and !$node->nodeExists('.nomedia')) {
163		$results = $this->getPicturesFromFolder($node, $results);
164		} else if ($node instanceof File) {
165		if (Requirements::isImageTypeSupported($node->getMimeType())) {
166		$results[] = $node;
167		}
168		}
169		}
170
171		return $results;
172		}
173		}
174

matiasdelellis / facerecognition

Push — master ( dce9c2...90c6b3 )

AddMissingImagesTask::execute() A

Complexity

Size

Duplication

Code Coverage

Importance

Duplication Side-by-Side

Filter issues like