DeployDispatcher - Code Metrics - Inspection of "[wip] Adding approval functionality" - silverstripe/deploynaut - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#681)

by Sean

created 2016-10-04 23:01 UTC

DeployDispatcher B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	366
Duplicated Lines	13.66 %

Coupling/Cohesion

Components	1
Dependencies	13

Importance

Changes

Metric	Value
wmc	43
lcom	1
cbo	13
dl	50
loc	366
rs	8.3157
c	0
b	0
f	0

14 Methods

Rating	Name	Duplication	Size	Complexity
A	init()	15	15	3
A	index()	0	3	1
A	history()	13	13	2
A	upcoming()	10	10	2
A	currentbuild()	0	7	2
A	show()	0	8	2
A	log()	0	16	3
B	createdeployment()	0	31	3
B	start()	0	39	5
A	Link()	0	3	1
A	getModel()	0	3	1
C	getDeploymentData()	0	58	9
B	getStackMemberData()	0	22	4
B	validateDeployment()	12	12	5

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

<?php

/**
 * This dispatcher takes care of updating and returning information about this
 * projects git repository
 */
class DeployDispatcher extends Dispatcher {
namespace YourVendor;

class YourClass { }

	const ACTION_DEPLOY = 'deploys';

	/**
	 * @var array
	 */
	public static $allowed_actions = [
		'history',
		'upcoming',
		'currentbuild',
		'show',
		'log',
		'create',
		'createdeployment'
	];

	/**
	 * @var \DNProject
	 */
	protected $project = null;

	/**
	 * @var \DNEnvironment
	 */
	protected $environment = null;

	/**
	 * @var array
	 */
	private static $action_types = [

		self::ACTION_DEPLOY
	];

	/**
	 * This is a per request cache of $this->project()->listMembers()
	 *
	 * @var null|array
	 */
	private static $_cache_project_members = null;

	/**
	 * This is a per request cache of $this->environment->CurrentBuild();
	 *
	 * @var null|DNDeployment
	 */
	private static $_cache_current_build = null;

	public function init() {

		parent::init();

		$this->project = $this->getCurrentProject();

		if (!$this->project) {
			return $this->project404Response();
		}

		// Performs canView permission check by limiting visible projects
		$this->environment = $this->getCurrentEnvironment($this->project);
		if (!$this->environment) {
			return $this->environment404Response();
		}
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \HTMLText|\SS_HTTPResponse
	 */
	public function index(\SS_HTTPRequest $request) {
		return $this->redirect(\Controller::join_links($this->Link(), 'history'), 302);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function history(SS_HTTPRequest $request) {

		$data = [];

		$list = $this->environment->DeployHistory('DeployStarted');

		foreach ($list as $deployment) {
			$data[] = $this->getDeploymentData($deployment);
		}

		return $this->getAPIResponse([
			'list' => $data,
		], 200);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function upcoming(SS_HTTPRequest $request) {

		$data = [];
		$list = $this->environment->UpcomingDeployments();
		foreach ($list as $deployment) {
			$data[] = $this->getDeploymentData($deployment);
		}
		return $this->getAPIResponse([
			'list' => $data,
		], 200);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function currentbuild(SS_HTTPRequest $request) {
		$currentBuild = $this->environment->CurrentBuild();
		if (!$currentBuild) {
			return $this->getAPIResponse(['deployment' => []], 200);
		}
		return $this->getAPIResponse(['deployment' => $this->getDeploymentData($currentBuild)], 200);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function show(SS_HTTPRequest $request) {
		$deployment = DNDeployment::get()->byId($request->param('ID'));
		$errorResponse = $this->validateDeployment($deployment);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		if ($errorResponse instanceof \SS_HTTPResponse) {
			return $errorResponse;
		}
		return $this->getAPIResponse(['deployment' => $this->getDeploymentData($deployment)], 200);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function log(SS_HTTPRequest $request) {
		$deployment = DNDeployment::get()->byId($request->param('ID'));
		$errorResponse = $this->validateDeployment($deployment);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		if ($errorResponse instanceof \SS_HTTPResponse) {
			return $errorResponse;
		}
		$log = $deployment->log();
		$content = $log->exists() ? $log->content() : 'Waiting for action to start';
		$lines = explode(PHP_EOL, $content);

		return $this->getAPIResponse([
			'message' => $lines,
			'status' => $deployment->Status,
			'deployment' => $this->getDeploymentData($deployment),

		], 200);
	}

	/**
	 * Create deployment. Can't use {@link create()} as it's taken by Object.
	 *
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function createdeployment(SS_HTTPRequest $request) {
		if ($request->httpMethod() !== 'POST') {
			return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
		}

		$this->checkSecurityToken();

		// @todo the strategy should have been saved when there has been a request for an
		// approval or a bypass. This saved state needs to be checked if it's invalidated
		// if another deploy happens before this one
		$isBranchDeploy = (int) $request->postVar('ref_type') === GitDispatcher::REF_TYPE_BRANCH;

		$options = [
			'sha' => $request->postVar('ref'),
			'ref_type' => $request->postVar('ref_type'),
			'branch' => $isBranchDeploy ? $request->postVar('ref_name') : null,
			'summary' => $request->postVar('summary')
		];
		$strategy = $this->environment->Backend()->planDeploy($this->environment, $options);

		$strategy->fromArray($request->postVars());
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		$deployment = $strategy->createDeployment();

		$deployment->getMachine()->apply(DNDeployment::TR_SUBMIT);

		return $this->getAPIResponse([
			'message' => 'Deployment has been created',
			'id' => $deployment->ID,
			'deployment' => $this->getDeploymentData($deployment),
		], 201);
	}

	/**
	 * @param \SS_HTTPRequest $request
	 * @return \SS_HTTPResponse
	 */
	public function start(SS_HTTPRequest $request) {
		if ($request->httpMethod() !== 'POST') {
			return $this->getAPIResponse(['message' => 'Method not allowed, requires POST'], 405);
		}

		$this->checkSecurityToken();

		$deployment = DNDeployment::get()->byId($request->postVar('id'));
		if (!$deployment || !$deployment->exists()) {
			return $this->getAPIResponse(['message' => 'This deployment does not exist'], 404);
		}

		// The deployment cannot be started until it has been approved, or bypassed straight to approved state
		if ($deployment->State != DNDeployment::STATE_APPROVED) {
			return $this->getAPIResponse(['message' => 'This deployment has not been approved. Cannot deploy'], 403);
		}

		// until we have a system that can invalidate currently scheduled deployments due
		// to emergency deploys etc, replan the deployment to check if it's still valid.
		$options = $deployment->getDeploymentStrategy()->getOptions();
		$strategy = $this->environment->Backend()->planDeploy($this->environment, $options);
		$deployment->Strategy = $strategy->toJSON();
		$deployment->write();

		$deployment->getMachine()->apply(DNDeployment::TR_QUEUE);

		$location = \Controller::join_links(Director::absoluteBaseURL(), $this->Link('log'), $deployment->ID);

		$response = $this->getAPIResponse([
			'message' => 'Deployment has been queued',
			'id' => $deployment->ID,
			'location' => $location,
			'deployment' => $this->getDeploymentData($deployment),

		], 201);

		$response->addHeader('Location', $location);

		return $response;
	}

	/**
	 * @param string $action
	 * @return string
	 */
	public function Link($action = '') {
		return \Controller::join_links($this->environment->Link(), self::ACTION_DEPLOY, $action);
	}

	/**
	 * @param string $name
	 * @return array
	 */
	public function getModel($name = '') {
		return [];
	}

	/**
	 * Return data about a single deployment for use in API response.
	 * @param DNDeployment $deployment
	 * @return array
	 */
	protected function getDeploymentData(DNDeployment $deployment) {
		if (self::$_cache_current_build === null) {
			self::$_cache_current_build = $this->environment->CurrentBuild();
class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
		}

		$deployer = $deployment->Deployer();
		$deployerData = null;
		if ($deployer && $deployer->exists()) {
			$deployerData = $this->getStackMemberData($deployer);
		}
		$approver = $deployment->Approver();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
		$approverData = null;
		if ($approver && $approver->exists()) {
			$approverData = $this->getStackMemberData($approver);
		}

		// failover for older deployments
		$started = $deployment->Created;
		$startedNice = $deployment->obj('Created')->Nice();
		if($deployment->DeployStarted) {
			$started = $deployment->DeployStarted;
			$startedNice = $deployment->obj('DeployStarted')->Nice();
		}

		$requested = $deployment->Created;
		$requestedNice = $deployment->obj('Created')->Nice();
		if($deployment->DeployRequested) {
			$requested = $deployment->DeployRequested;
			$requestedNice = $deployment->obj('DeployRequested')->Nice();
		}

		$isCurrentBuild = self::$_cache_current_build ? ($deployment->ID === self::$_cache_current_build->ID) : false;

		return [
			'id' => $deployment->ID,
			'date_created' => $deployment->Created,
			'date_created_nice' => $deployment->obj('Created')->Nice(),
			'date_started' => $started,
			'date_started_nice' => $startedNice,
			'date_requested' => $requested,
			'date_requested_nice' => $requestedNice,
			'date_updated' => $deployment->LastEdited,
			'date_updated_nice' => $deployment->obj('LastEdited')->Nice(),
			'summary' => $deployment->Summary,

			'branch' => $deployment->Branch,
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
			'tags' => $deployment->getTags()->toArray(),
			'changes' => $deployment->getDeploymentStrategy()->getChanges(),
			'sha' => $deployment->SHA,
			'short_sha' => substr($deployment->SHA, 0, 7),
			'ref_type' => $deployment->RefType,
			'commit_message' => $deployment->getCommitMessage(),
			'commit_url' => $deployment->getCommitURL(),
			'deployer' => $deployerData,
			'approver' => $approverData,
			'state' => $deployment->State,
			'is_current_build' => $isCurrentBuild
		];
	}

	/**
	 * Return data about a particular {@link Member} of the stack for use in API response.
	 * Note that role can be null in the response. This is the case of an admin, or an operations
	 * user who can create the deployment but is not part of the stack roles.
	 *
	 * @param Member $member
	 * @return array
	 */
	protected function getStackMemberData(Member $member) {
		if (self::$_cache_project_members === null) {
			self::$_cache_project_members = $this->project->listMembers();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
		}

		$role = null;

		foreach (self::$_cache_project_members as $stackMember) {
			if ($stackMember['MemberID'] !== $member->ID) {
				continue;
			}

			$role = $stackMember['RoleTitle'];
		}

		return [
			'id' => $member->ID,
			'email' => $member->Email,
			'role' => $role,
			'name' => $member->getName()
		];
	}

	/**
	 * Check if a DNDeployment exists and do permission checks on it. If there is something wrong it will return
	 * an APIResponse with the error, otherwise null.
	 *
	 * @param \DNDeployment $deployment
	 *
	 * @return null|SS_HTTPResponse
	 */
	protected function validateDeployment(\DNDeployment $deployment) {

		if (!$deployment || !$deployment->exists()) {
			return $this->getAPIResponse(['message' => 'This deployment does not exist'], 404);
		}
		if ($deployment->EnvironmentID != $this->environment->ID) {
			return $this->getAPIResponse(['message' => 'This deployment does not belong to the environment'], 403);
		}
		if (!$deployment->canView()) {
			return $this->getAPIResponse(['message' => 'You are not authorised to view this deployment'], 403);
		}
		return null;
	}

}


silverstripe / deploynaut