MarathonStoreJobBusinessCase::isDependencyCircular() - Code Metrics - Inspection of "Feature/add marathon support" - trivago/chapi - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#68)

by Bidesh

created 2017-02-17 15:47 UTC

isDependencyCircular() B

↳ Parent: MarathonStoreJobBusinessCase

Complexity

Conditions	8
Paths	8

Size

Total Lines	61
Code Lines	18

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	26
CRAP Score	8.0032

Importance

Changes

Metric	Value
c	0
b	0
f	0
dl	0
loc	61
ccs	26
cts	27
cp	0.963
rs	7.0047
cc	8
eloc	18
nc	8
nop	3
crap	8.0032

How to fix Long Method

<?php
/**
 *
 * @package: chapi
 *
 * @author: bthapaliya
 * @since: 2017-01-03
 *
 */

namespace Chapi\BusinessCase\JobManagement;


use Chapi\BusinessCase\Comparison\JobComparisonInterface;
use Chapi\Entity\JobEntityInterface;
use Chapi\Entity\Marathon\MarathonAppEntity;
use Chapi\Service\JobIndex\JobIndexServiceInterface;
use Chapi\Service\JobRepository\JobRepositoryInterface;
use Psr\Log\LoggerInterface;

class MarathonStoreJobBusinessCase extends AbstractStoreJobBusinessCase implements StoreJobBusinessCaseInterface
{
    public function __construct(

        JobIndexServiceInterface $oJobIndexService,
        JobRepositoryInterface $oJobRepositoryRemote,
        JobRepositoryInterface $oJobRepositoryLocal,
        JobComparisonInterface $oJobComparisonBusinessCase,
        LoggerInterface $oLogger

    )
    {

        $this->oJobIndexService = $oJobIndexService;
        $this->oLogger = $oLogger;
        $this->oJobComparisonBusinessCase = $oJobComparisonBusinessCase;
        $this->oJobRepositoryRemote = $oJobRepositoryRemote;
        $this->oJobRepositoryLocal = $oJobRepositoryLocal;
    }

    /**
     * @return void
     */
    public function storeIndexedJobs()

    {
        $_aRemoteMissingApps = $this->oJobComparisonBusinessCase->getRemoteMissingJobs();
        foreach ($_aRemoteMissingApps as $_sAppId)
        {
            $this->addRemoteMissingApp($_sAppId);
        }

        $_aLocalMissingApps = $this->oJobComparisonBusinessCase->getLocalMissingJobs();
        foreach ($_aLocalMissingApps as $_sAppId)
        {
            $this->removeLocalMissingAppInRemote($_sAppId);
        }
        $_aLocalUpdates = $this->oJobComparisonBusinessCase->getLocalJobUpdates();
        foreach ($_aLocalUpdates as $_sAppId)
        {
            $this->updateAppInRemote($_sAppId);
        }
    }

    private function addRemoteMissingApp($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            /** @var MarathonAppEntity $_oJobEntityLocal */
            $_oJobEntityLocal = $this->oJobRepositoryLocal->getJob($sAppId);

            if (!$_oJobEntityLocal instanceof MarathonAppEntity)
            {
                throw new \RuntimeException('Encountered entity that is not MarathonAppEntity');
            }

            // check if dependency is satisfied
            if ( $_oJobEntityLocal->isDependencyJob())
            {
                try {
                    $circular = $this->isDependencyCircular($_oJobEntityLocal, count($_oJobEntityLocal->dependencies));
                    if ($circular)
                    {
                        $this->oLogger->error(sprintf(
                            'The dependency for %s is circular. Please fix them.', $sAppId
                        ));
                        return false;
                    }
                }
                catch(\Exception $e)
                {
                    $this->oLogger->error(sprintf(
                        'Job %s cannot be added to remote : %s',$sAppId, $e->getMessage()
                    ));
                    return false;
                }


                foreach ($_oJobEntityLocal->dependencies as $_sDependencyKey)
                {
                    $_bAdded = $this->addRemoteMissingApp($_sDependencyKey);

                    if (!$_bAdded)
                    {
                        $this->oLogger->error(sprintf(
                            'Job "%s" is dependent on "%s" which is missing. Please add them and try again.',
                            $sAppId,
                            $_sDependencyKey
                        ));
                        $this->oJobIndexService->removeJob($_sDependencyKey);
                        return false;
                    }
                }
            }

            if ($this->oJobRepositoryRemote->addJob($_oJobEntityLocal))
            {
                $this->oJobIndexService->removeJob($_oJobEntityLocal->getKey());
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully added to marathon',
                    $_oJobEntityLocal->getKey()
                ));

                return true;
            }
            $this->oLogger->error(sprintf(
                'Failed to add job "%s" to marathon',
                $_oJobEntityLocal->getKey()
            ));
        }
        return false;

    }

    private function hasDuplicates($arr)
    {
        return !(count($arr) == count(array_unique($arr)));
    }

    private function isDependencyCircular(MarathonAppEntity $oEntity, $iImmediateChildren, &$path=[])
    {
        // Invariant: path will not have duplicates for acyclic dependency tree
        if ($this->hasDuplicates($path))
        {
            return true;
        }

        // if we hit leaf (emptyarray), and have no
        // cycle yet, then remove the leaf and return false
        // removing leaf will help maintain a proper path from root to leaf
        // For tree : A ---> B ---> D
        //                      |-> C
        // When we reach node D, path will be [A, B, D]
        // so we pop off D so that the next append will properly show [A, B, C] (legit path)
        if (empty($oEntity->dependencies))
        {
            array_pop($path);
            return false;
        }

        foreach ($oEntity->dependencies as $_sDependency)
        {
            // add this key in path as we will explore its child now
            $path[] = $oEntity->getKey();

            /** @var MarathonAppEntity $_oDependEntity */
            $_oDependEntity = $this->oJobRepositoryLocal->getJob($_sDependency);

            if (!$_oDependEntity)
            {
                throw new \Exception(sprintf('Dependency chain on non-existing app "%s"', $_sDependency));
            }

            if (!$_oDependEntity instanceof MarathonAppEntity)
            {
                throw new \RuntimeException('Expected MarathonAppEntity. Found something else');
            }


            // check if dependency has cycle
            if ($this->isDependencyCircular($_oDependEntity, count($_oDependEntity->dependencies), $path))
            {
                return true;
            }

            // tracking immediateChildren, this helps us with
            // removing knowing when to pop off key for intermediary dependency
            // For tree: A ---> B ---> D
            //              |      |-> C

            //              |->E
            // for B intermediate Child will be 2.
            // when we process D, it will be reduced to 1 and with C to 0
            // then we will pop B to generate path [A, E] when we reach E.
            $iImmediateChildren = $iImmediateChildren -1;

            if ($iImmediateChildren == 0)
            {
                array_pop($path);
            }
        }
    }


    private function removeLocalMissingAppInRemote($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            if ($this->oJobRepositoryRemote->removeJob($sAppId))
            {
                $this->oJobIndexService->removeJob($sAppId);
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully removed from marathon',
                    $sAppId
                ));

                return true;
            }
            $this->oLogger->error(sprintf(
                'Failed to remove"%s" from marathon',
                $sAppId
            ));

        }
        return false;
    }

    private function updateAppInRemote($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            $_bRemoved = $this->oJobRepositoryRemote->removeJob($sAppId);

            $_oUpdatedConfig = $this->oJobRepositoryLocal->getJob($sAppId);
            $_bAddedBack = $this->oJobRepositoryRemote->addJob($_oUpdatedConfig);

            // updated
            if ($_bRemoved && $_bAddedBack)
            {
                $this->oJobIndexService->removeJob($sAppId);
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully updated in marathon',
                    $sAppId
                ));

                return true;
            }

            $this->oLogger->error(sprintf(
                'Failed to update job "%s" in marathon',
                $sAppId
            ));
        }

        return false;
    }
}

1			<?php
2			/**
3			*
4			* @package: chapi
5			*
6			* @author: bthapaliya
7			* @since: 2017-01-03
8			*
9			*/
10
11			namespace Chapi\BusinessCase\JobManagement;
12
13
14			use Chapi\BusinessCase\Comparison\JobComparisonInterface;
15			use Chapi\Entity\JobEntityInterface;
16			use Chapi\Entity\Marathon\MarathonAppEntity;
17			use Chapi\Service\JobIndex\JobIndexServiceInterface;
18			use Chapi\Service\JobRepository\JobRepositoryInterface;
19			use Psr\Log\LoggerInterface;
20
21			class MarathonStoreJobBusinessCase extends AbstractStoreJobBusinessCase implements StoreJobBusinessCaseInterface
22			{
23	12	View Code Duplication	public function __construct(
			0 ignored issues – show Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
24			JobIndexServiceInterface $oJobIndexService,
25			JobRepositoryInterface $oJobRepositoryRemote,
26			JobRepositoryInterface $oJobRepositoryLocal,
27			JobComparisonInterface $oJobComparisonBusinessCase,
28			LoggerInterface $oLogger
29
30			)
31			{
32
33	12		$this->oJobIndexService = $oJobIndexService;
34	12		$this->oLogger = $oLogger;
35	12		$this->oJobComparisonBusinessCase = $oJobComparisonBusinessCase;
36	12		$this->oJobRepositoryRemote = $oJobRepositoryRemote;
37	12		$this->oJobRepositoryLocal = $oJobRepositoryLocal;
38	12		}
39
40			/**
41			* @return void
42			*/
43	7	View Code Duplication	public function storeIndexedJobs()
			0 ignored issues – show Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
44			{
45	7		$_aRemoteMissingApps = $this->oJobComparisonBusinessCase->getRemoteMissingJobs();
46	7		foreach ($_aRemoteMissingApps as $_sAppId)
47			{
48	5		$this->addRemoteMissingApp($_sAppId);
49	7		}
50
51	7		$_aLocalMissingApps = $this->oJobComparisonBusinessCase->getLocalMissingJobs();
52	7		foreach ($_aLocalMissingApps as $_sAppId)
53			{
54	1		$this->removeLocalMissingAppInRemote($_sAppId);
55	7		}
56	7		$_aLocalUpdates = $this->oJobComparisonBusinessCase->getLocalJobUpdates();
57	7		foreach ($_aLocalUpdates as $_sAppId)
58			{
59	1		$this->updateAppInRemote($_sAppId);
60	7		}
61	7		}
62
63	5		private function addRemoteMissingApp($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function addRemoteMissingApp()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history...
64			{
65	5		if ($this->oJobIndexService->isJobInIndex($sAppId))
66	5		{
67			/** @var MarathonAppEntity $_oJobEntityLocal */
68	5		$_oJobEntityLocal = $this->oJobRepositoryLocal->getJob($sAppId);
69
70	5		if (!$_oJobEntityLocal instanceof MarathonAppEntity)
71	5		{
72			throw new \RuntimeException('Encountered entity that is not MarathonAppEntity');
73			}
74
75			// check if dependency is satisfied
76	5		if ( $_oJobEntityLocal->isDependencyJob())
77	5		{
78			try {
79	4		$circular = $this->isDependencyCircular($_oJobEntityLocal, count($_oJobEntityLocal->dependencies));
80			if ($circular)
81	3		{
82	1		$this->oLogger->error(sprintf(
83	1		'The dependency for %s is circular. Please fix them.', $sAppId
84	1		));
85	1		return false;
86			}
87			}
88	3		catch(\Exception $e)
89			{
90	1		$this->oLogger->error(sprintf(
91	1		'Job %s cannot be added to remote : %s',$sAppId, $e->getMessage()
92	1		));
93	1		return false;
94			}
95
96
97	2		foreach ($_oJobEntityLocal->dependencies as $_sDependencyKey)
98			{
99	2		$_bAdded = $this->addRemoteMissingApp($_sDependencyKey);
100
101	2		if (!$_bAdded)
102	2		{
103	1		$this->oLogger->error(sprintf(
104	1		'Job "%s" is dependent on "%s" which is missing. Please add them and try again.',
105	1		$sAppId,
106			$_sDependencyKey
107	1		));
108	1		$this->oJobIndexService->removeJob($_sDependencyKey);
109	1		return false;
110			}
111	1		}
112	1		}
113
114	2		if ($this->oJobRepositoryRemote->addJob($_oJobEntityLocal))
115	2		{
116	2		$this->oJobIndexService->removeJob($_oJobEntityLocal->getKey());
117	2		$this->oLogger->notice(sprintf(
118	2		'Job "%s" successfully added to marathon',
119	2		$_oJobEntityLocal->getKey()
120	2		));
121
122	2		return true;
123			}
124			$this->oLogger->error(sprintf(
125			'Failed to add job "%s" to marathon',
126			$_oJobEntityLocal->getKey()
127			));
128			}
129	2		return false;
130
131			}
132
133	4		private function hasDuplicates($arr)
134			{
135	4		return !(count($arr) == count(array_unique($arr)));
136			}
137
138	4		private function isDependencyCircular(MarathonAppEntity $oEntity, $iImmediateChildren, &$path=[])
139			{
140			// Invariant: path will not have duplicates for acyclic dependency tree
141	4		if ($this->hasDuplicates($path))
142	4		{
143	1		return true;
144			}
145
146			// if we hit leaf (emptyarray), and have no
147			// cycle yet, then remove the leaf and return false
148			// removing leaf will help maintain a proper path from root to leaf
149			// For tree : A ---> B ---> D
150			// \|-> C
151			// When we reach node D, path will be [A, B, D]
152			// so we pop off D so that the next append will properly show [A, B, C] (legit path)
153	4		if (empty($oEntity->dependencies))
154	4		{
155	2		array_pop($path);
156	2		return false;
157			}
158
159	4		foreach ($oEntity->dependencies as $_sDependency)
160			{
161			// add this key in path as we will explore its child now
162	4		$path[] = $oEntity->getKey();
163
164			/** @var MarathonAppEntity $_oDependEntity */
165	4		$_oDependEntity = $this->oJobRepositoryLocal->getJob($_sDependency);
166
167	4		if (!$_oDependEntity)
168	4		{
169	1		throw new \Exception(sprintf('Dependency chain on non-existing app "%s"', $_sDependency));
170			}
171
172	3		if (!$_oDependEntity instanceof MarathonAppEntity)
173	3		{
174			throw new \RuntimeException('Expected MarathonAppEntity. Found something else');
175			}
176
177
178			// check if dependency has cycle
179	3		if ($this->isDependencyCircular($_oDependEntity, count($_oDependEntity->dependencies), $path))
180	3		{
181	1		return true;
182			}
183
184			// tracking immediateChildren, this helps us with
185			// removing knowing when to pop off key for intermediary dependency
186			// For tree: A ---> B ---> D
187			// \| \|-> C
			0 ignored issues – show Unused Code Comprehensibility introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `43%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
188			// \|->E
189			// for B intermediate Child will be 2.
190			// when we process D, it will be reduced to 1 and with C to 0
191			// then we will pop B to generate path [A, E] when we reach E.
192	2		$iImmediateChildren = $iImmediateChildren -1;
			0 ignored issues – show Coding Style introduced 2017-02-16 13:15 UTC by Report Bug Copy Issue Report Decrement operators should be used where possible; found "$iImmediateChildren = $iImmediateChildren -1;" but expected "$iImmediateChildren--" Loading history...
193	2		if ($iImmediateChildren == 0)
194	2		{
195	2		array_pop($path);
196	2		}
197	2		}
198	2		}
199
200
201	1	View Code Duplication	private function removeLocalMissingAppInRemote($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function removeLocalMissingAppInRemote()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history... Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
202			{
203	1		if ($this->oJobIndexService->isJobInIndex($sAppId))
204	1		{
205	1		if ($this->oJobRepositoryRemote->removeJob($sAppId))
206	1		{
207	1		$this->oJobIndexService->removeJob($sAppId);
208	1		$this->oLogger->notice(sprintf(
209	1		'Job "%s" successfully removed from marathon',
210			$sAppId
211	1		));
212
213	1		return true;
214			}
215			$this->oLogger->error(sprintf(
216			'Failed to remove"%s" from marathon',
217			$sAppId
218			));
219
220			}
221			return false;
222			}
223
224	1		private function updateAppInRemote($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function updateAppInRemote()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history...
225			{
226	1		if ($this->oJobIndexService->isJobInIndex($sAppId))
227	1		{
228	1		$_bRemoved = $this->oJobRepositoryRemote->removeJob($sAppId);
229
230	1		$_oUpdatedConfig = $this->oJobRepositoryLocal->getJob($sAppId);
231	1		$_bAddedBack = $this->oJobRepositoryRemote->addJob($_oUpdatedConfig);
232
233			// updated
234	1		if ($_bRemoved && $_bAddedBack)
235	1		{
236	1		$this->oJobIndexService->removeJob($sAppId);
237	1		$this->oLogger->notice(sprintf(
238	1		'Job "%s" successfully updated in marathon',
239			$sAppId
240	1		));
241
242	1		return true;
243			}
244
245			$this->oLogger->error(sprintf(
246			'Failed to update job "%s" in marathon',
247			$sAppId
248			));
249			}
250
251			return false;
252			}
253			}

trivago / chapi

GitHub Access Token became invalid

Pull Request — master (#68)

isDependencyCircular() B

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like