LdapContactsSuggestions - Code Metrics - Inspection of "Merge pull request #1804 from imt-mines-albi/ldap-..." - RainLoop/rainloop-webmail - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d717b9...62e6fb )

by Rain

created 2019-03-27 22:15 UTC

LdapContactsSuggestions A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	322
Duplicated Lines	6.83 %

Coupling/Cohesion

Components	1
Dependencies	0

Importance

Changes

Metric	Value
dl	22
loc	322
rs	9.2
c	0
b	0
f	0
wmc	40
lcom	1
cbo	0

8 Methods

Rating	Name	Duplication	Size	Complexity
A	SetConfig()	0	16	2
A	SetAllowedEmails()	0	6	1
A	Process()	0	23	5
B	findNameAndEmail()	22	32	8
F	ldapSearch()	0	100	16
A	escape()	0	12	2
A	logLdapError()	0	11	4
A	SetLogger()	0	9	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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

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 LdapContactsSuggestions 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 LdapContactsSuggestions, and based on these observations, apply Extract Interface, too.

<?php

class LdapContactsSuggestions implements \RainLoop\Providers\Suggestions\ISuggestions
{
	/**
	 * @var string
	 */
	private $sHostName = '127.0.0.1';

	/**
	 * @var int
	 */
	private $iHostPort = 389;

	/**
	 * @var string
	 */
	private $sAccessDn = NULL;

	/**
	 * @var string
	 */
	private $sAccessPassword = NULL;

	/**
	 * @var string
	 */
	private $sUsersDn = '';

	/**
	 * @var string
	 */
	private $sObjectClass = 'inetOrgPerson';

	/**
	 * @var string
	 */
	private $sNameField = 'givenname';

	/**
	 * @var string
	 */
	private $sEmailField = 'mail';

	/**
	 * @var \MailSo\Log\Logger
	 */
	private $oLogger = null;

	/**
	 * @var string
	 */
	private $sAllowedEmails = '';

	/**
	 * @param string $sHostName
	 * @param int $iHostPort
	 * @param string $sAccessDn
	 * @param string $sAccessPassword
	 * @param string $sUsersDn
	 * @param string $sObjectClass
	 * @param string $sNameField
	 * @param string $sEmailField
	 *
	 * @return \LdapContactsSuggestions
	 */
	public function SetConfig($sHostName, $iHostPort, $sAccessDn, $sAccessPassword, $sUsersDn, $sObjectClass, $sNameField, $sEmailField)
	{
		$this->sHostName = $sHostName;
		$this->iHostPort = $iHostPort;
		if (0 < \strlen($sAccessDn))
		{
			$this->sAccessDn = $sAccessDn;
			$this->sAccessPassword = $sAccessPassword;
		}
		$this->sUsersDn = $sUsersDn;
		$this->sObjectClass = $sObjectClass;
		$this->sNameField = $sNameField;
		$this->sEmailField = $sEmailField;

		return $this;
	}

	/**
	 * @param string $sAllowedEmails
	 *
	 * @return \LdapContactsSuggestions
	 */
	public function SetAllowedEmails($sAllowedEmails)
	{
		$this->sAllowedEmails = $sAllowedEmails;

		return $this;
	}

	/**
	 * @param \RainLoop\Model\Account $oAccount
	 * @param string $sQuery
	 * @param int $iLimit = 20
	 *
	 * @return array
	 */
	public function Process($oAccount, $sQuery, $iLimit = 20)
	{
		$sQuery = \trim($sQuery);

		if (2 > \strlen($sQuery))
		{
			return array();
		}
		else if (!$oAccount || !\RainLoop\Plugins\Helper::ValidateWildcardValues($oAccount->Email(), $this->sAllowedEmails))
		{
			return array();
		}

		$aResult = $this->ldapSearch($oAccount, $sQuery);

		$aResult = \RainLoop\Utils::RemoveSuggestionDuplicates($aResult);
		if ($iLimit < \count($aResult))
		{
			$aResult = \array_slice($aResult, 0, $iLimit);
		}

		return $aResult;
	}

	/**
	 * @param array $aLdapItem
	 * @param array $aEmailFields
	 * @param array $aNameFields
	 *
	 * @return array
	 */
	private function findNameAndEmail($aLdapItem, $aEmailFields, $aNameFields)
	{
		$sEmail = $sName = '';
		if ($aLdapItem)

		{
			foreach ($aEmailFields as $sField)

			{
				if (!empty($aLdapItem[$sField][0]))
				{
					$sEmail = \trim($aLdapItem[$sField][0]);
					if (!empty($sEmail))
					{
						break;
					}
				}
			}

			foreach ($aNameFields as $sField)

			{
				if (!empty($aLdapItem[$sField][0]))
				{
					$sName = \trim($aLdapItem[$sField][0]);
					if (!empty($sName))
					{
						break;
					}
				}
			}
		}

		return array($sEmail, $sName);
	}

	/**
	 * @param \RainLoop\Model\Account $oAccount
	 * @param string $sQuery
	 *
	 * @return array
	 */
	private function ldapSearch($oAccount, $sQuery)
	{
		$sSearchEscaped = $this->escape($sQuery);

		$aResult = array();
		$oCon = @\ldap_connect($this->sHostName, $this->iHostPort);
		if ($oCon)
		{
			$this->oLogger->Write('ldap_connect: connected', \MailSo\Log\Enumerations\Type::INFO, 'LDAP');

			@\ldap_set_option($oCon, LDAP_OPT_PROTOCOL_VERSION, 3);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

			if (!@\ldap_bind($oCon, $this->sAccessDn, $this->sAccessPassword))
			{
				if ( is_null($this->sAccessDn) ) {
					$this->logLdapError($oCon, 'ldap_bind (anonymous)');
				} else {
					$this->logLdapError($oCon, 'ldap_bind');
				}
				return $aResult;
			}

			$sDomain = \MailSo\Base\Utils::GetDomainFromEmail($oAccount->Email());
			$sSearchDn = \strtr($this->sUsersDn, array(
				'{domain}' => $sDomain,
				'{domain:dc}' => 'dc='.\strtr($sDomain, array('.' => ',dc=')),
				'{email}' => $oAccount->Email(),
				'{email:user}' => \MailSo\Base\Utils::GetAccountNameFromEmail($oAccount->Email()),
				'{email:domain}' => $sDomain,
				'{login}' => $oAccount->Login(),
				'{imap:login}' => $oAccount->Login(),
				'{imap:host}' => $oAccount->DomainIncHost(),
				'{imap:port}' => $oAccount->DomainIncPort()
			));

			$aEmails = empty($this->sEmailField) ? array() : \explode(',', $this->sEmailField);
			$aNames = empty($this->sNameField) ? array() : \explode(',', $this->sNameField);

			$aEmails = \array_map('trim', $aEmails);
			$aNames = \array_map('trim', $aNames);

			$aFields = \array_merge($aEmails, $aNames);

			$aItems = array();
			$sSubFilter = '';
			foreach ($aFields as $sItem)
			{
				if (!empty($sItem))
				{
					$aItems[] = $sItem;
					$sSubFilter .= '('.$sItem.'=*'.$sSearchEscaped.'*)';
				}
			}

			$sFilter = '(&(objectclass='.$this->sObjectClass.')';
			$sFilter .= (1 < count($aItems) ? '(|' : '').$sSubFilter.(1 < count($aItems) ? ')' : '');
			$sFilter .= ')';

			$this->oLogger->Write('ldap_search: start: '.$sSearchDn.' / '.$sFilter, \MailSo\Log\Enumerations\Type::INFO, 'LDAP');
			$oS = @\ldap_search($oCon, $sSearchDn, $sFilter, $aItems, 0, 30, 30);
			if ($oS)
			{
				$aEntries = @\ldap_get_entries($oCon, $oS);
				if (is_array($aEntries))
				{
					if (isset($aEntries['count']))
					{
						unset($aEntries['count']);
					}

					foreach ($aEntries as $aItem)
					{
						if ($aItem)
						{
							$sName = $sEmail = '';
							list ($sEmail, $sName) = $this->findNameAndEmail($aItem, $aEmails, $aNames);
							if (!empty($sEmail))
							{
								$aResult[] = array($sEmail, $sName);
							}
						}
					}
				}
				else
				{
					$this->logLdapError($oCon, 'ldap_get_entries');
				}
			}
			else
			{
				$this->logLdapError($oCon, 'ldap_search');
			}
		}
		else
		{
			return $aResult;
		}

		return $aResult;
	}

	/**
	 * @param string $sStr
	 *
	 * @return string
	 */
	public function escape($sStr)
	{
		$aNewChars = array();
		$aChars = array('\\', '*', '(', ')', \chr(0));

		foreach ($aChars as $iIndex => $sValue)
		{
			$aNewChars[$iIndex] = '\\'.\str_pad(\dechex(\ord($sValue)), 2, '0');
		}

		return \str_replace($aChars, $aNewChars, $sStr);
	}

	/**
	 * @param mixed $oCon
	 * @param string $sCmd
	 *
	 * @return string
	 */
	public function logLdapError($oCon, $sCmd)
	{
		if ($this->oLogger)
		{
			$sError = $oCon ? @\ldap_error($oCon) : '';
			$iErrno = $oCon ? @\ldap_errno($oCon) : 0;

			$this->oLogger->Write($sCmd.' error: '.$sError.' ('.$iErrno.')',
				\MailSo\Log\Enumerations\Type::WARNING, 'LDAP');
		}
	}

	/**
	 * @param \MailSo\Log\Logger $oLogger
	 *
	 * @return \LdapContactsSuggestions
	 */
	public function SetLogger($oLogger)
	{
		if ($oLogger instanceof \MailSo\Log\Logger)
if ($x instanceof DoesNotExist) {
    // Do something.
}
		{
			$this->oLogger = $oLogger;
		}

		return $this;
	}
}


Push — master ( d717b9...62e6fb )

LdapContactsSuggestions A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

8 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

1. Missing dependencies

2. Missing use statement

1		<?php
2
3		class LdapContactsSuggestions implements \RainLoop\Providers\Suggestions\ISuggestions
4		{
5		/**
6		* @var string
7		*/
8		private $sHostName = '127.0.0.1';
9
10		/**
11		* @var int
12		*/
13		private $iHostPort = 389;
14
15		/**
16		* @var string
17		*/
18		private $sAccessDn = NULL;
19
20		/**
21		* @var string
22		*/
23		private $sAccessPassword = NULL;
24
25		/**
26		* @var string
27		*/
28		private $sUsersDn = '';
29
30		/**
31		* @var string
32		*/
33		private $sObjectClass = 'inetOrgPerson';
34
35		/**
36		* @var string
37		*/
38		private $sNameField = 'givenname';
39
40		/**
41		* @var string
42		*/
43		private $sEmailField = 'mail';
44
45		/**
46		* @var \MailSo\Log\Logger
47		*/
48		private $oLogger = null;
49
50		/**
51		* @var string
52		*/
53		private $sAllowedEmails = '';
54
55		/**
56		* @param string $sHostName
57		* @param int $iHostPort
58		* @param string $sAccessDn
59		* @param string $sAccessPassword
60		* @param string $sUsersDn
61		* @param string $sObjectClass
62		* @param string $sNameField
63		* @param string $sEmailField
64		*
65		* @return \LdapContactsSuggestions
66		*/
67		public function SetConfig($sHostName, $iHostPort, $sAccessDn, $sAccessPassword, $sUsersDn, $sObjectClass, $sNameField, $sEmailField)
68		{
69		$this->sHostName = $sHostName;
70		$this->iHostPort = $iHostPort;
71		if (0 < \strlen($sAccessDn))
72		{
73		$this->sAccessDn = $sAccessDn;
74		$this->sAccessPassword = $sAccessPassword;
75		}
76		$this->sUsersDn = $sUsersDn;
77		$this->sObjectClass = $sObjectClass;
78		$this->sNameField = $sNameField;
79		$this->sEmailField = $sEmailField;
80
81		return $this;
82		}
83
84		/**
85		* @param string $sAllowedEmails
86		*
87		* @return \LdapContactsSuggestions
88		*/
89		public function SetAllowedEmails($sAllowedEmails)
90		{
91		$this->sAllowedEmails = $sAllowedEmails;
92
93		return $this;
94		}
95
96		/**
97		* @param \RainLoop\Model\Account $oAccount
98		* @param string $sQuery
99		* @param int $iLimit = 20
100		*
101		* @return array
102		*/
103		public function Process($oAccount, $sQuery, $iLimit = 20)
104		{
105		$sQuery = \trim($sQuery);
106
107		if (2 > \strlen($sQuery))
108		{
109		return array();
110		}
111		else if (!$oAccount \|\| !\RainLoop\Plugins\Helper::ValidateWildcardValues($oAccount->Email(), $this->sAllowedEmails))
112		{
113		return array();
114		}
115
116		$aResult = $this->ldapSearch($oAccount, $sQuery);
117
118		$aResult = \RainLoop\Utils::RemoveSuggestionDuplicates($aResult);
119		if ($iLimit < \count($aResult))
120		{
121		$aResult = \array_slice($aResult, 0, $iLimit);
122		}
123
124		return $aResult;
125		}
126
127		/**
128		* @param array $aLdapItem
129		* @param array $aEmailFields
130		* @param array $aNameFields
131		*
132		* @return array
133		*/
134		private function findNameAndEmail($aLdapItem, $aEmailFields, $aNameFields)
135		{
136		$sEmail = $sName = '';
137		if ($aLdapItem)
		0 ignored issues – show Bug Best Practice introduced 2017-10-06 18:56 UTC by Report Bug Copy Issue Report The expression `$aLdapItem` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `! empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history...
138		{
139	View Code Duplication	foreach ($aEmailFields as $sField)
		0 ignored issues – show Duplication introduced 2017-10-06 18:56 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
140		{
141		if (!empty($aLdapItem[$sField][0]))
142		{
143		$sEmail = \trim($aLdapItem[$sField][0]);
144		if (!empty($sEmail))
145		{
146		break;
147		}
148		}
149		}
150
151	View Code Duplication	foreach ($aNameFields as $sField)
		0 ignored issues – show Duplication introduced 2017-10-06 18:56 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
152		{
153		if (!empty($aLdapItem[$sField][0]))
154		{
155		$sName = \trim($aLdapItem[$sField][0]);
156		if (!empty($sName))
157		{
158		break;
159		}
160		}
161		}
162		}
163
164		return array($sEmail, $sName);
165		}
166
167		/**
168		* @param \RainLoop\Model\Account $oAccount
169		* @param string $sQuery
170		*
171		* @return array
172		*/
173		private function ldapSearch($oAccount, $sQuery)
174		{
175		$sSearchEscaped = $this->escape($sQuery);
176
177		$aResult = array();
178		$oCon = @\ldap_connect($this->sHostName, $this->iHostPort);
179		if ($oCon)
180		{
181		$this->oLogger->Write('ldap_connect: connected', \MailSo\Log\Enumerations\Type::INFO, 'LDAP');
182
183		@\ldap_set_option($oCon, LDAP_OPT_PROTOCOL_VERSION, 3);
		0 ignored issues – show Security Best Practice introduced 2017-10-06 18:56 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
184
185		if (!@\ldap_bind($oCon, $this->sAccessDn, $this->sAccessPassword))
186		{
187		if ( is_null($this->sAccessDn) ) {
188		$this->logLdapError($oCon, 'ldap_bind (anonymous)');
189		} else {
190		$this->logLdapError($oCon, 'ldap_bind');
191		}
192		return $aResult;
193		}
194
195		$sDomain = \MailSo\Base\Utils::GetDomainFromEmail($oAccount->Email());
196		$sSearchDn = \strtr($this->sUsersDn, array(
197		'{domain}' => $sDomain,
198		'{domain:dc}' => 'dc='.\strtr($sDomain, array('.' => ',dc=')),
199		'{email}' => $oAccount->Email(),
200		'{email:user}' => \MailSo\Base\Utils::GetAccountNameFromEmail($oAccount->Email()),
201		'{email:domain}' => $sDomain,
202		'{login}' => $oAccount->Login(),
203		'{imap:login}' => $oAccount->Login(),
204		'{imap:host}' => $oAccount->DomainIncHost(),
205		'{imap:port}' => $oAccount->DomainIncPort()
206		));
207
208		$aEmails = empty($this->sEmailField) ? array() : \explode(',', $this->sEmailField);
209		$aNames = empty($this->sNameField) ? array() : \explode(',', $this->sNameField);
210
211		$aEmails = \array_map('trim', $aEmails);
212		$aNames = \array_map('trim', $aNames);
213
214		$aFields = \array_merge($aEmails, $aNames);
215
216		$aItems = array();
217		$sSubFilter = '';
218		foreach ($aFields as $sItem)
219		{
220		if (!empty($sItem))
221		{
222		$aItems[] = $sItem;
223		$sSubFilter .= '('.$sItem.'='.$sSearchEscaped.')';
224		}
225		}
226
227		$sFilter = '(&(objectclass='.$this->sObjectClass.')';
228		$sFilter .= (1 < count($aItems) ? '(\|' : '').$sSubFilter.(1 < count($aItems) ? ')' : '');
229		$sFilter .= ')';
230
231		$this->oLogger->Write('ldap_search: start: '.$sSearchDn.' / '.$sFilter, \MailSo\Log\Enumerations\Type::INFO, 'LDAP');
232		$oS = @\ldap_search($oCon, $sSearchDn, $sFilter, $aItems, 0, 30, 30);
233		if ($oS)
234		{
235		$aEntries = @\ldap_get_entries($oCon, $oS);
236		if (is_array($aEntries))
237		{
238		if (isset($aEntries['count']))
239		{
240		unset($aEntries['count']);
241		}
242
243		foreach ($aEntries as $aItem)
244		{
245		if ($aItem)
246		{
247		$sName = $sEmail = '';
248		list ($sEmail, $sName) = $this->findNameAndEmail($aItem, $aEmails, $aNames);
249		if (!empty($sEmail))
250		{
251		$aResult[] = array($sEmail, $sName);
252		}
253		}
254		}
255		}
256		else
257		{
258		$this->logLdapError($oCon, 'ldap_get_entries');
259		}
260		}
261		else
262		{
263		$this->logLdapError($oCon, 'ldap_search');
264		}
265		}
266		else
267		{
268		return $aResult;
269		}
270
271		return $aResult;
272		}
273
274		/**
275		* @param string $sStr
276		*
277		* @return string
278		*/
279		public function escape($sStr)
280		{
281		$aNewChars = array();
282		$aChars = array('\\', '*', '(', ')', \chr(0));
283
284		foreach ($aChars as $iIndex => $sValue)
285		{
286		$aNewChars[$iIndex] = '\\'.\str_pad(\dechex(\ord($sValue)), 2, '0');
287		}
288
289		return \str_replace($aChars, $aNewChars, $sStr);
290		}
291
292		/**
293		* @param mixed $oCon
294		* @param string $sCmd
295		*
296		* @return string
297		*/
298		public function logLdapError($oCon, $sCmd)
299		{
300		if ($this->oLogger)
301		{
302		$sError = $oCon ? @\ldap_error($oCon) : '';
303		$iErrno = $oCon ? @\ldap_errno($oCon) : 0;
304
305		$this->oLogger->Write($sCmd.' error: '.$sError.' ('.$iErrno.')',
306		\MailSo\Log\Enumerations\Type::WARNING, 'LDAP');
307		}
308		}
309
310		/**
311		* @param \MailSo\Log\Logger $oLogger
312		*
313		* @return \LdapContactsSuggestions
314		*/
315		public function SetLogger($oLogger)
316		{
317		if ($oLogger instanceof \MailSo\Log\Logger)
		0 ignored issues – show Bug introduced 2017-10-06 18:56 UTC by Report Bug Copy Issue Report The class `MailSo\Log\Logger` 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...
318		{
319		$this->oLogger = $oLogger;
320		}
321
322		return $this;
323		}
324		}
325

RainLoop / rainloop-webmail

Push — master ( d717b9...62e6fb )

LdapContactsSuggestions A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

8 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

1. Missing dependencies

2. Missing use statement

Duplication Side-by-Side

Filter issues like