From 370eebd227d90dd16a07d3091ff69c2c5722b775 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Thu, 8 Oct 2015 20:20:42 +0100 Subject: [PATCH 01/10] Fixing PHPCS error in HomeController --- PHPCI/Controller/HomeController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/PHPCI/Controller/HomeController.php b/PHPCI/Controller/HomeController.php index e6c27e14..1b0c60aa 100644 --- a/PHPCI/Controller/HomeController.php +++ b/PHPCI/Controller/HomeController.php @@ -159,5 +159,4 @@ class HomeController extends \PHPCI\Controller return $rtn; } - } From 2f859be369338aa2612699a1ddfc994efbd7168a Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Thu, 8 Oct 2015 20:22:43 +0100 Subject: [PATCH 02/10] PHPCS Fix --- PHPCI/Application.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PHPCI/Application.php b/PHPCI/Application.php index f2d40e0d..a6eb646c 100644 --- a/PHPCI/Application.php +++ b/PHPCI/Application.php @@ -136,7 +136,8 @@ class Application extends b8\Application protected function setLayoutVariables(View &$layout) { $groups = array(); - $groupList = b8\Store\Factory::getStore('ProjectGroup')->getWhere(array(), 100, 0, array(), array('title' => 'ASC')); + $groupStore = b8\Store\Factory::getStore('ProjectGroup'); + $groupList = $groupStore->getWhere(array(), 100, 0, array(), array('title' => 'ASC')); foreach ($groupList['items'] as $group) { $thisGroup = array('title' => $group->getTitle()); From af90b94a3cbe2a7c78b062b4b123c02287a52e0c Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Thu, 8 Oct 2015 20:24:32 +0100 Subject: [PATCH 03/10] PHPMD Fixes --- PHPCI/Controller/GroupController.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/PHPCI/Controller/GroupController.php b/PHPCI/Controller/GroupController.php index 970a240c..c19b0923 100644 --- a/PHPCI/Controller/GroupController.php +++ b/PHPCI/Controller/GroupController.php @@ -53,11 +53,11 @@ class GroupController extends Controller $this->view->groups = $groups; } - public function edit($id = null) + public function edit($groupId = null) { $this->requireAdmin(); - if (!is_null($id)) { + if (!is_null($groupId)) { $group = $this->groupStore->getById($id); } else { $group = new ProjectGroup(); @@ -74,7 +74,7 @@ class GroupController extends Controller $form = new Form(); $form->setMethod('POST'); - $form->setAction(PHPCI_URL . 'group/edit' . (!is_null($id) ? '/' . $id : '')); + $form->setAction(PHPCI_URL . 'group/edit' . (!is_null($groupId) ? '/' . $groupId : '')); $title = new Form\Element\Text('title'); $title->setContainerClass('form-group'); @@ -91,10 +91,10 @@ class GroupController extends Controller $this->view->form = $form; } - public function delete($id) + public function delete($groupId) { $this->requireAdmin(); - $group = $this->groupStore->getById($id); + $group = $this->groupStore->getById($groupId); $this->groupStore->delete($group); $response = new b8\Http\Response\RedirectResponse(); From 4375c524a9baf781392330dd86d2bc7d5f33d8bc Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Thu, 8 Oct 2015 20:27:40 +0100 Subject: [PATCH 04/10] PHPMD Fix --- PHPCI/Controller/GroupController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PHPCI/Controller/GroupController.php b/PHPCI/Controller/GroupController.php index c19b0923..cc4b4c51 100644 --- a/PHPCI/Controller/GroupController.php +++ b/PHPCI/Controller/GroupController.php @@ -58,7 +58,7 @@ class GroupController extends Controller $this->requireAdmin(); if (!is_null($groupId)) { - $group = $this->groupStore->getById($id); + $group = $this->groupStore->getById($groupId); } else { $group = new ProjectGroup(); } From 68249d2f5dbb803223e365985a7abe476053e3e5 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 09:16:05 +0100 Subject: [PATCH 05/10] Cleanup of new executor changes. --- PHPCI/Plugin/Util/Executor.php | 90 +++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 29 deletions(-) diff --git a/PHPCI/Plugin/Util/Executor.php b/PHPCI/Plugin/Util/Executor.php index b085ac87..bdd06b7e 100644 --- a/PHPCI/Plugin/Util/Executor.php +++ b/PHPCI/Plugin/Util/Executor.php @@ -50,10 +50,6 @@ class Executor public function executePlugins(&$config, $stage) { $success = true; - - /** @var \PHPCI\Model\Build $build */ - $build = $this->pluginFactory->getResourceFor('PHPCI\Model\Build'); - $branch = $build->getBranch(); $pluginsToExecute = array(); // If we have global plugins to execute for this stage, add them to the list to be executed: @@ -61,31 +57,7 @@ class Executor $pluginsToExecute[] = $config[$stage]; } - // If we have branch-specific plugins to execute, add them to the list to be executed: - if (isset($config['branch-' . $branch][$stage]) && is_array($config['branch-' . $branch][$stage])) { - $branchConfig = $config['branch-' . $branch]; - $runOption = isset($branchConfig['run-option']) ? $branchConfig['run-option'] : 'after'; - $plugins = $config['branch-' . $branch][$stage]; - - switch ($runOption) { - case 'replace': - $pluginsToExecute = array(); - $pluginsToExecute[] = $plugins; - break; - - case 'before': - array_unshift($pluginsToExecute, $plugins); - break; - - case 'after': - array_push($pluginsToExecute, $plugins); - break; - - default: - array_push($pluginsToExecute, $plugins); - break; - } - } + $pluginsToExecute = $this->getBranchSpecificPlugins($config, $stage, $pluginsToExecute); foreach ($pluginsToExecute as $pluginSet) { if (!$this->doExecutePlugins($pluginSet, $stage)) { @@ -96,6 +68,66 @@ class Executor return $success; } + /** + * Check the config for any plugins specific to the branch we're currently building. + * @param $config + * @param $stage + * @param $pluginsToExecute + * @return array + */ + protected function getBranchSpecificPlugins(&$config, $stage, $pluginsToExecute) + { + /** @var \PHPCI\Model\Build $build */ + $build = $this->pluginFactory->getResourceFor('PHPCI\Model\Build'); + $branch = $build->getBranch(); + + // If we don't have any branch-specific plugins: + if (!isset($config['branch-' . $branch][$stage]) || !is_array($config['branch-' . $branch][$stage])) { + return $pluginsToExecute; + } + + // If we have branch-specific plugins to execute, add them to the list to be executed: + $branchConfig = $config['branch-' . $branch]; + $plugins = $branchConfig[$stage]; + + $runOption = 'after'; + + if (!empty($branchConfig['run-option'])) { + $runOption = $branchConfig['run-option']; + } + + switch ($runOption) { + // Replace standard plugin set for this stage with just the branch-specific ones: + case 'replace': + $pluginsToExecute = array(); + $pluginsToExecute[] = $plugins; + break; + + // Run branch-specific plugins before standard plugins: + case 'before': + array_unshift($pluginsToExecute, $plugins); + break; + + // Run branch-specific plugins after standard plugins: + case 'after': + array_push($pluginsToExecute, $plugins); + break; + + default: + array_push($pluginsToExecute, $plugins); + break; + } + + return $pluginsToExecute; + } + + /** + * Execute the list of plugins found for a given testing stage. + * @param $plugins + * @param $stage + * @return bool + * @throws \Exception + */ protected function doExecutePlugins(&$plugins, $stage) { $success = true; From a0083580561acff7082df37f2222d54e2492c8cd Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 09:27:45 +0100 Subject: [PATCH 06/10] Cleaning up BuildWorker #1 --- PHPCI/Worker/BuildWorker.php | 71 ++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/PHPCI/Worker/BuildWorker.php b/PHPCI/Worker/BuildWorker.php index a6fcc536..37d2bc20 100644 --- a/PHPCI/Worker/BuildWorker.php +++ b/PHPCI/Worker/BuildWorker.php @@ -6,6 +6,7 @@ use b8\Config; use b8\Database; use b8\Store\Factory; use Monolog\Logger; +use Pheanstalk\Job; use Pheanstalk\Pheanstalk; use PHPCI\Builder; use PHPCI\BuildFactory; @@ -49,6 +50,16 @@ class BuildWorker */ protected $queue; + /** + * @var \Pheanstalk\Pheanstalk + */ + protected $pheanstalk; + + /** + * @var int + */ + protected $totalJobs = 0; + /** * @param $host * @param $queue @@ -57,6 +68,7 @@ class BuildWorker { $this->host = $host; $this->queue = $queue; + $this->pheanstalk = new Pheanstalk($this->host); } /** @@ -80,36 +92,20 @@ class BuildWorker */ public function startWorker() { - $pheanstalk = new Pheanstalk($this->host); - $pheanstalk->watch($this->queue); - $pheanstalk->ignore('default'); + $this->pheanstalk->watch($this->queue); + $this->pheanstalk->ignore('default'); $buildStore = Factory::getStore('Build'); - $jobs = 0; - while ($this->run) { // Get a job from the queue: - $job = $pheanstalk->reserve(); + $job = $this->pheanstalk->reserve(); - // Make sure we don't run more than maxJobs jobs on this worker: - $jobs++; - - if ($this->maxJobs != -1 && $this->maxJobs <= $jobs) { - $this->run = false; - } + $this->checkJobLimit(); // Get the job data and run the job: $jobData = json_decode($job->getData(), true); - if (empty($jobData) || !is_array($jobData)) { - // Probably not from PHPCI. - $pheanstalk->release($job); - continue; - } - - if (!array_key_exists('type', $jobData) || $jobData['type'] !== 'phpci.build') { - // Probably not from PHPCI. - $pheanstalk->delete($job); + if (!$this->verifyJob($job, $jobData)) { continue; } @@ -128,7 +124,7 @@ class BuildWorker $build = BuildFactory::getBuildById($jobData['build_id']); } catch (\Exception $ex) { $this->logger->addWarning('Build #' . $jobData['build_id'] . ' does not exist in the database.'); - $pheanstalk->delete($job); + $this->pheanstalk->delete($job); } try { @@ -147,7 +143,7 @@ class BuildWorker // If we've caught a PDO Exception, it is probably not the fault of the build, but of a failed // connection or similar. Release the job and kill the worker. $this->run = false; - $pheanstalk->release($job); + $this->pheanstalk->release($job); } catch (\Exception $ex) { $build->setStatus(Build::STATUS_FAILED); $build->setFinished(new \DateTime()); @@ -163,7 +159,7 @@ class BuildWorker } // Delete the job when we're done: - $pheanstalk->delete($job); + $this->pheanstalk->delete($job); } } @@ -174,4 +170,31 @@ class BuildWorker { $this->run = false; } + + protected function checkJobLimit() + { + // Make sure we don't run more than maxJobs jobs on this worker: + $this->totalJobs++; + + if ($this->maxJobs != -1 && $this->maxJobs <= $this->totalJobs) { + $this->stopWorker(); + } + } + + protected function verifyJob(Job $job, $jobData) + { + if (empty($jobData) || !is_array($jobData)) { + // Probably not from PHPCI. + $this->pheanstalk->release($job); + return false; + } + + if (!array_key_exists('type', $jobData) || $jobData['type'] !== 'phpci.build') { + // Probably not from PHPCI. + $this->pheanstalk->delete($job); + return false; + } + + return true; + } } From e61ce203a34cebaa83f946f61e454bcd3b9661be Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 09:38:55 +0100 Subject: [PATCH 07/10] Docblocks --- PHPCI/Controller/GroupController.php | 16 ++++++++++++++++ PHPCI/Controller/HomeController.php | 4 ++++ PHPCI/Store/Base/BuildMetaStoreBase.php | 15 +++++++++++++++ PHPCI/Store/Base/BuildStoreBase.php | 15 +++++++++++++++ PHPCI/Store/Base/ProjectGroupStoreBase.php | 7 +++++++ PHPCI/Store/Base/ProjectStoreBase.php | 15 +++++++++++++++ PHPCI/Store/Base/UserStoreBase.php | 15 +++++++++++++++ PHPCI/Worker/BuildWorker.php | 12 +++++++++++- 8 files changed, 98 insertions(+), 1 deletion(-) diff --git a/PHPCI/Controller/GroupController.php b/PHPCI/Controller/GroupController.php index cc4b4c51..898f9e41 100644 --- a/PHPCI/Controller/GroupController.php +++ b/PHPCI/Controller/GroupController.php @@ -28,11 +28,17 @@ class GroupController extends Controller */ protected $groupStore; + /** + * Set up this controller. + */ public function init() { $this->groupStore = b8\Store\Factory::getStore('ProjectGroup'); } + /** + * List project groups. + */ public function index() { $this->requireAdmin(); @@ -53,6 +59,11 @@ class GroupController extends Controller $this->view->groups = $groups; } + /** + * Add or edit a project group. + * @param null $groupId + * @return void|b8\Http\Response\RedirectResponse + */ public function edit($groupId = null) { $this->requireAdmin(); @@ -91,6 +102,11 @@ class GroupController extends Controller $this->view->form = $form; } + /** + * Delete a project group. + * @param $groupId + * @return b8\Http\Response\RedirectResponse + */ public function delete($groupId) { $this->requireAdmin(); diff --git a/PHPCI/Controller/HomeController.php b/PHPCI/Controller/HomeController.php index 1b0c60aa..4241d324 100644 --- a/PHPCI/Controller/HomeController.php +++ b/PHPCI/Controller/HomeController.php @@ -144,6 +144,10 @@ class HomeController extends \PHPCI\Controller return $view->render(); } + /** + * Get a summary of the project groups we have, and what projects they have in them. + * @return array + */ protected function getGroupInfo() { $rtn = array(); diff --git a/PHPCI/Store/Base/BuildMetaStoreBase.php b/PHPCI/Store/Base/BuildMetaStoreBase.php index 6c4cfc15..52665d79 100644 --- a/PHPCI/Store/Base/BuildMetaStoreBase.php +++ b/PHPCI/Store/Base/BuildMetaStoreBase.php @@ -20,11 +20,18 @@ class BuildMetaStoreBase extends Store protected $modelName = '\PHPCI\Model\BuildMeta'; protected $primaryKey = 'id'; + /** + * Get a BuildMeta by primary key (Id) + */ public function getByPrimaryKey($value, $useConnection = 'read') { return $this->getById($value, $useConnection); } + /** + * Get a single BuildMeta by Id. + * @return null|BuildMeta + */ public function getById($value, $useConnection = 'read') { if (is_null($value)) { @@ -44,6 +51,10 @@ class BuildMetaStoreBase extends Store return null; } + /** + * Get multiple BuildMeta by ProjectId. + * @return array + */ public function getByProjectId($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { @@ -72,6 +83,10 @@ class BuildMetaStoreBase extends Store } } + /** + * Get multiple BuildMeta by BuildId. + * @return array + */ public function getByBuildId($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { diff --git a/PHPCI/Store/Base/BuildStoreBase.php b/PHPCI/Store/Base/BuildStoreBase.php index 89d3a82f..b8b49cb7 100644 --- a/PHPCI/Store/Base/BuildStoreBase.php +++ b/PHPCI/Store/Base/BuildStoreBase.php @@ -20,11 +20,18 @@ class BuildStoreBase extends Store protected $modelName = '\PHPCI\Model\Build'; protected $primaryKey = 'id'; + /** + * Get a Build by primary key (Id) + */ public function getByPrimaryKey($value, $useConnection = 'read') { return $this->getById($value, $useConnection); } + /** + * Get a single Build by Id. + * @return null|Build + */ public function getById($value, $useConnection = 'read') { if (is_null($value)) { @@ -44,6 +51,10 @@ class BuildStoreBase extends Store return null; } + /** + * Get multiple Build by ProjectId. + * @return array + */ public function getByProjectId($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { @@ -72,6 +83,10 @@ class BuildStoreBase extends Store } } + /** + * Get multiple Build by Status. + * @return array + */ public function getByStatus($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { diff --git a/PHPCI/Store/Base/ProjectGroupStoreBase.php b/PHPCI/Store/Base/ProjectGroupStoreBase.php index 5b2730ea..c7cd8772 100644 --- a/PHPCI/Store/Base/ProjectGroupStoreBase.php +++ b/PHPCI/Store/Base/ProjectGroupStoreBase.php @@ -20,11 +20,18 @@ class ProjectGroupStoreBase extends Store protected $modelName = '\PHPCI\Model\ProjectGroup'; protected $primaryKey = 'id'; + /** + * Get a ProjectGroup by primary key (Id) + */ public function getByPrimaryKey($value, $useConnection = 'read') { return $this->getById($value, $useConnection); } + /** + * Get a single ProjectGroup by Id. + * @return null|ProjectGroup + */ public function getById($value, $useConnection = 'read') { if (is_null($value)) { diff --git a/PHPCI/Store/Base/ProjectStoreBase.php b/PHPCI/Store/Base/ProjectStoreBase.php index c764238d..1e2bf65b 100644 --- a/PHPCI/Store/Base/ProjectStoreBase.php +++ b/PHPCI/Store/Base/ProjectStoreBase.php @@ -20,11 +20,18 @@ class ProjectStoreBase extends Store protected $modelName = '\PHPCI\Model\Project'; protected $primaryKey = 'id'; + /** + * Get a Project by primary key (Id) + */ public function getByPrimaryKey($value, $useConnection = 'read') { return $this->getById($value, $useConnection); } + /** + * Get a single Project by Id. + * @return null|Project + */ public function getById($value, $useConnection = 'read') { if (is_null($value)) { @@ -44,6 +51,10 @@ class ProjectStoreBase extends Store return null; } + /** + * Get multiple Project by Title. + * @return array + */ public function getByTitle($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { @@ -72,6 +83,10 @@ class ProjectStoreBase extends Store } } + /** + * Get multiple Project by GroupId. + * @return array + */ public function getByGroupId($value, $limit = 1000, $useConnection = 'read') { if (is_null($value)) { diff --git a/PHPCI/Store/Base/UserStoreBase.php b/PHPCI/Store/Base/UserStoreBase.php index 90d60cb4..c1c6cdf7 100644 --- a/PHPCI/Store/Base/UserStoreBase.php +++ b/PHPCI/Store/Base/UserStoreBase.php @@ -20,11 +20,18 @@ class UserStoreBase extends Store protected $modelName = '\PHPCI\Model\User'; protected $primaryKey = 'id'; + /** + * Get a User by primary key (Id) + */ public function getByPrimaryKey($value, $useConnection = 'read') { return $this->getById($value, $useConnection); } + /** + * Get a single User by Id. + * @return null|User + */ public function getById($value, $useConnection = 'read') { if (is_null($value)) { @@ -44,6 +51,10 @@ class UserStoreBase extends Store return null; } + /** + * Get a single User by Email. + * @return null|User + */ public function getByEmail($value, $useConnection = 'read') { if (is_null($value)) { @@ -63,6 +74,10 @@ class UserStoreBase extends Store return null; } + /** + * Get a single User by Name. + * @return null|User + */ public function getByName($value, $useConnection = 'read') { if (is_null($value)) { diff --git a/PHPCI/Worker/BuildWorker.php b/PHPCI/Worker/BuildWorker.php index 37d2bc20..92ee4ced 100644 --- a/PHPCI/Worker/BuildWorker.php +++ b/PHPCI/Worker/BuildWorker.php @@ -171,6 +171,10 @@ class BuildWorker $this->run = false; } + /** + * Checks if this worker has done the amount of jobs it is allowed to do, and if so tells it to stop + * after this job completes. + */ protected function checkJobLimit() { // Make sure we don't run more than maxJobs jobs on this worker: @@ -181,11 +185,17 @@ class BuildWorker } } + /** + * Checks that the job received is actually from PHPCI, and has a valid type. + * @param Job $job + * @param $jobData + * @return bool + */ protected function verifyJob(Job $job, $jobData) { if (empty($jobData) || !is_array($jobData)) { // Probably not from PHPCI. - $this->pheanstalk->release($job); + $this->pheanstalk->delete($job); return false; } From 07b92fecf3f6d45781dd938675e55d6d36127be5 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 10:14:54 +0100 Subject: [PATCH 08/10] Fixing tests --- PHPCI/BuildFactory.php | 2 ++ PHPCI/Model/Build.php | 26 ++++++++++++++++++++++++++ composer.lock | 10 +++++----- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/PHPCI/BuildFactory.php b/PHPCI/BuildFactory.php index 1dbc4017..91e618a8 100644 --- a/PHPCI/BuildFactory.php +++ b/PHPCI/BuildFactory.php @@ -66,6 +66,8 @@ class BuildFactory case 'svn': $type = 'SubversionBuild'; break; + default: + return $build; } $class = '\\PHPCI\\Model\\Build\\' . $type; diff --git a/PHPCI/Model/Build.php b/PHPCI/Model/Build.php index be7f4a8a..9374b763 100644 --- a/PHPCI/Model/Build.php +++ b/PHPCI/Model/Build.php @@ -30,6 +30,11 @@ class Build extends BuildBase public $currentBuildPath = null; + /** + * @var \PHPCI\Model\Project $project + */ + protected $project; + /** * Get link to commit from another source (i.e. Github) */ @@ -249,4 +254,25 @@ class Build extends BuildBase exec(sprintf(IS_WIN ? 'rmdir /S /Q "%s"' : 'rm -Rf "%s"', $buildPath)); } + + /** + * @param Project $value + */ + public function setProjectObject(\PHPCI\Model\Project $value) + { + $this->project = $value; + return parent::setProjectObject($value); + } + + /** + * Get the Project model for this Build by Id. + */ + public function getProject() + { + if (empty($this->project)) { + $this->project = parent::getProject(); + } + + return $this->project; + } } diff --git a/composer.lock b/composer.lock index 0fc6a092..b399ce10 100644 --- a/composer.lock +++ b/composer.lock @@ -1781,16 +1781,16 @@ }, { "name": "sebastian/global-state", - "version": "1.0.0", + "version": "1.1.0", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/global-state.git", - "reference": "c7428acdb62ece0a45e6306f1ae85e1c05b09c01" + "reference": "23af31f402993cfd94e99cbc4b782e9a78eb0e97" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/c7428acdb62ece0a45e6306f1ae85e1c05b09c01", - "reference": "c7428acdb62ece0a45e6306f1ae85e1c05b09c01", + "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/23af31f402993cfd94e99cbc4b782e9a78eb0e97", + "reference": "23af31f402993cfd94e99cbc4b782e9a78eb0e97", "shasum": "" }, "require": { @@ -1828,7 +1828,7 @@ "keywords": [ "global state" ], - "time": "2014-10-06 09:23:50" + "time": "2015-06-21 15:11:22" }, { "name": "sebastian/recursion-context", From f747371c6ddf1d4326cdfb7260b08037fd9aef4e Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 10:29:21 +0100 Subject: [PATCH 09/10] Fixing tests #2 --- PHPCI/Model/Build.php | 13 ++++--------- Tests/PHPCI/Service/BuildServiceTest.php | 6 +++++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/PHPCI/Model/Build.php b/PHPCI/Model/Build.php index 9374b763..2f2b64cc 100644 --- a/PHPCI/Model/Build.php +++ b/PHPCI/Model/Build.php @@ -30,11 +30,6 @@ class Build extends BuildBase public $currentBuildPath = null; - /** - * @var \PHPCI\Model\Project $project - */ - protected $project; - /** * Get link to commit from another source (i.e. Github) */ @@ -260,7 +255,7 @@ class Build extends BuildBase */ public function setProjectObject(\PHPCI\Model\Project $value) { - $this->project = $value; + $this->data['project'] = $value; return parent::setProjectObject($value); } @@ -269,10 +264,10 @@ class Build extends BuildBase */ public function getProject() { - if (empty($this->project)) { - $this->project = parent::getProject(); + if (empty($this->data['project'])) { + $this->data['project'] = parent::getProject(); } - return $this->project; + return $this->data['project']; } } diff --git a/Tests/PHPCI/Service/BuildServiceTest.php b/Tests/PHPCI/Service/BuildServiceTest.php index 985f3088..4a396334 100644 --- a/Tests/PHPCI/Service/BuildServiceTest.php +++ b/Tests/PHPCI/Service/BuildServiceTest.php @@ -101,9 +101,13 @@ class BuildServiceTest extends \PHPUnit_Framework_TestCase */ public function testExecute_CreateDuplicateBuild() { + $project = new Project(); + $project->setType('hg'); + $project->setId(101); + $build = new Build(); $build->setId(1); - $build->setProjectId(101); + $build->setProject($project); $build->setCommitId('abcde'); $build->setStatus(Build::STATUS_FAILED); $build->setLog('Test'); From fbc3da59dd6a09e99b7247a1d323fba7b62f9e71 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 Oct 2015 10:37:50 +0100 Subject: [PATCH 10/10] Fix --- PHPCI/Model/Build.php | 21 --------------------- PHPCI/Service/BuildService.php | 20 ++++++++++++++------ Tests/PHPCI/Service/BuildServiceTest.php | 6 +----- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/PHPCI/Model/Build.php b/PHPCI/Model/Build.php index 2f2b64cc..be7f4a8a 100644 --- a/PHPCI/Model/Build.php +++ b/PHPCI/Model/Build.php @@ -249,25 +249,4 @@ class Build extends BuildBase exec(sprintf(IS_WIN ? 'rmdir /S /Q "%s"' : 'rm -Rf "%s"', $buildPath)); } - - /** - * @param Project $value - */ - public function setProjectObject(\PHPCI\Model\Project $value) - { - $this->data['project'] = $value; - return parent::setProjectObject($value); - } - - /** - * Get the Project model for this Build by Id. - */ - public function getProject() - { - if (empty($this->data['project'])) { - $this->data['project'] = parent::getProject(); - } - - return $this->data['project']; - } } diff --git a/PHPCI/Service/BuildService.php b/PHPCI/Service/BuildService.php index 7db8d474..dca0fc0d 100644 --- a/PHPCI/Service/BuildService.php +++ b/PHPCI/Service/BuildService.php @@ -87,9 +87,13 @@ class BuildService $build = $this->buildStore->save($build); - $build = BuildFactory::getBuild($build); - $build->sendStatusPostback(); - $this->addBuildToQueue($build); + $buildId = $build->getId(); + + if (!empty($buildId)) { + $build = BuildFactory::getBuild($build); + $build->sendStatusPostback(); + $this->addBuildToQueue($build); + } return $build; } @@ -116,9 +120,13 @@ class BuildService $build = $this->buildStore->save($build); - $build = BuildFactory::getBuild($build); - $build->sendStatusPostback(); - $this->addBuildToQueue($build); + $buildId = $build->getId(); + + if (!empty($buildId)) { + $build = BuildFactory::getBuild($build); + $build->sendStatusPostback(); + $this->addBuildToQueue($build); + } return $build; } diff --git a/Tests/PHPCI/Service/BuildServiceTest.php b/Tests/PHPCI/Service/BuildServiceTest.php index 4a396334..3c96b131 100644 --- a/Tests/PHPCI/Service/BuildServiceTest.php +++ b/Tests/PHPCI/Service/BuildServiceTest.php @@ -101,13 +101,9 @@ class BuildServiceTest extends \PHPUnit_Framework_TestCase */ public function testExecute_CreateDuplicateBuild() { - $project = new Project(); - $project->setType('hg'); - $project->setId(101); - $build = new Build(); $build->setId(1); - $build->setProject($project); + $build->setProject(101); $build->setCommitId('abcde'); $build->setStatus(Build::STATUS_FAILED); $build->setLog('Test');