AddMissingImagesTask::do() - Code Metrics - Inspection of "Forgot another do()->execute()" - matiasdelellis/facerecognition - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — exception_in_tasks ( 7463cd...3899e4 )

by Branko

created 2018-11-21 09:40 UTC

AddMissingImagesTask::do() A

↳ Parent: AddMissingImagesTask::execute()

Complexity

Conditions	5
Paths	9

Size

Total Lines

Duplication

Lines	7
Ratio	23.33 %

Code Coverage

Tests	0
CRAP Score	30

Importance

Changes

Metric	Value
dl	7
loc	30
ccs	0
cts	23
cp	0
rs	9.1288
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($userId, $model, $userFolder);
	}

	/**
	 * Recursively crawls given folder for a given user
	 *
	 * @param string $userId ID of the user for which we are crawling this folder for
	 * @param int $model Used model
	 * @param Folder $folder Folder to recursively search images in
	 */
	private function parseUserFolder(string $userId, 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')) {
if ($x instanceof DoesNotExist) {
    // Do something.
}
				$results = $this->getPicturesFromFolder($node, $results);
			} else if ($node instanceof File) {
if ($x instanceof DoesNotExist) {
    // Do something.
}
				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($userId, $model, $userFolder);
121		}
122
123		/**
124		* Recursively crawls given folder for a given user
125		*
126		* @param string $userId ID of the user for which we are crawling this folder for
127		* @param int $model Used model
128		* @param Folder $folder Folder to recursively search images in
129		*/
130		private function parseUserFolder(string $userId, int $model, Folder $folder) {
		0 ignored issues – show Unused Code introduced 2018-11-13 19:44 UTC by Report Bug Copy Issue Report The parameter `$userId` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
131		$nodes = $this->getPicturesFromFolder($folder);
132		foreach ($nodes as $file) {
133		$this->logDebug('Found ' . $file->getPath());
134
135		$image = new Image();
136		$image->setUser($file->getOwner()->getUid());
137		$image->setFile($file->getId());
138		$image->setModel($model);
139		// todo: this check/insert logic for each image is so inefficient it hurts my mind
140		if ($this->imageMapper->imageExists($image) == null) {
141		// todo: can we have larger transaction with bulk insert?
142		$this->imageMapper->insert($image);
143		}
144		}
145		}
146
147		/**
148		* Return all images from a given folder.
149		*
150		* TODO: It is inefficient since it copies the array recursively.
151		*
152		* @param Folder $folder Folder to get images from
153		* @return array List of all images and folders to continue recursive crawling
154		*/
155	View Code Duplication	private function getPicturesFromFolder(Folder $folder, $results = array()) {
156		$nodes = $folder->getDirectoryListing();
157
158		foreach ($nodes as $node) {
159		//if ($node->isHidden())
160		// continue;
161		// $previewImage = new \OC_Image();
162		// $previewImage->loadFromData($preview->getContent());
163		if ($node instanceof Folder and !$node->nodeExists('.nomedia')) {
		0 ignored issues – show Bug introduced 2018-11-13 19:44 UTC by Report Bug Copy Issue Report The class `OCP\Files\Folder` does not exist. Did you forget a USE statement, or did you not list all dependencies? This error could be the result of: 1. Missing dependencies PHP Analyzer uses your `composer.json` file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects the `composer.json` to be in the root folder of your repository. Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the `require` or `require-dev` section? 2. Missing use statement PHP does not complain about undefined classes in `ìnstanceof` checks. For example, the following PHP code will work perfectly fine: if ($x instanceof DoesNotExist) { // Do something. } If you have not tested against this specific condition, such errors might go unnoticed. Loading history...
164		$results = $this->getPicturesFromFolder($node, $results);
165		} else if ($node instanceof File) {
		0 ignored issues – show Bug introduced 2018-11-13 19:44 UTC by Report Bug Copy Issue Report The class `OCP\Files\File` does not exist. Did you forget a USE statement, or did you not list all dependencies? This error could be the result of: 1. Missing dependencies PHP Analyzer uses your `composer.json` file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects the `composer.json` to be in the root folder of your repository. Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the `require` or `require-dev` section? 2. Missing use statement PHP does not complain about undefined classes in `ìnstanceof` checks. For example, the following PHP code will work perfectly fine: if ($x instanceof DoesNotExist) { // Do something. } If you have not tested against this specific condition, such errors might go unnoticed. Loading history...
166		if (Requirements::isImageTypeSupported($node->getMimeType())) {
167		$results[] = $node;
168		}
169		}
170		}
171
172		return $results;
173		}
174		}
175

matiasdelellis / facerecognition

Push — exception_in_tasks ( 7463cd...3899e4 )

AddMissingImagesTask::do() A

Complexity

Size

Duplication

Code Coverage

Importance

1. Missing dependencies

2. Missing use statement

1. Missing dependencies

2. Missing use statement

Duplication Side-by-Side

Filter issues like