Merge pull request #1075 from Block8/dc/phpci-fixes

Fixing PHPCI Errors

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());

PHPCI/BuildFactory.php

@@ -66,6 +66,8 @@ class BuildFactory
                 case 'svn':
                     $type = 'SubversionBuild';
                     break;
+                default:
+                    return $build;
             }
 
             $class = '\\PHPCI\\Model\\Build\\' . $type;

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,12 +59,17 @@ class GroupController extends Controller
         $this->view->groups = $groups;
     }
 
-    public function edit($id = null)
+    /**
+     * Add or edit a project group.
+     * @param null $groupId
+     * @return void|b8\Http\Response\RedirectResponse
+     */
+    public function edit($groupId = null)
     {
         $this->requireAdmin();
 
-        if (!is_null($id)) {
+        if (!is_null($groupId)) {
-            $group = $this->groupStore->getById($id);
+            $group = $this->groupStore->getById($groupId);
         } else {
             $group = new ProjectGroup();
         }
@@ -74,7 +85,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 +102,15 @@ class GroupController extends Controller
         $this->view->form = $form;
     }
 
-    public function delete($id)
+    /**
+     * Delete a project group.
+     * @param $groupId
+     * @return b8\Http\Response\RedirectResponse
+     */
+    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();

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();
@@ -159,5 +163,4 @@ class HomeController extends \PHPCI\Controller
 
         return $rtn;
     }
-
 }

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:
+        $pluginsToExecute = $this->getBranchSpecificPlugins($config, $stage, $pluginsToExecute);
-        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;
-            }
-        }
 
         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;

PHPCI/Service/BuildService.php

@@ -87,9 +87,13 @@ class BuildService
 
         $build = $this->buildStore->save($build);
 
-        $build = BuildFactory::getBuild($build);
+        $buildId = $build->getId();
-        $build->sendStatusPostback();
+
-        $this->addBuildToQueue($build);
+        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);
+        $buildId = $build->getId();
-        $build->sendStatusPostback();
+
-        $this->addBuildToQueue($build);
+        if (!empty($buildId)) {
+            $build = BuildFactory::getBuild($build);
+            $build->sendStatusPostback();
+            $this->addBuildToQueue($build);
+        }
 
         return $build;
     }

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)) {

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)) {

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)) {

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)) {

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)) {

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);
+        $this->pheanstalk->watch($this->queue);
-        $pheanstalk->watch($this->queue);
+        $this->pheanstalk->ignore('default');
-        $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:
+            $this->checkJobLimit();
-            $jobs++;
-
-            if ($this->maxJobs != -1 && $this->maxJobs <= $jobs) {
-                $this->run = false;
-            }
 
             // Get the job data and run the job:
             $jobData = json_decode($job->getData(), true);
 
-            if (empty($jobData) || !is_array($jobData)) {
+            if (!$this->verifyJob($job, $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);
                 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,41 @@ 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:
+        $this->totalJobs++;
+
+        if ($this->maxJobs != -1 && $this->maxJobs <= $this->totalJobs) {
+            $this->stopWorker();
+        }
+    }
+
+    /**
+     * 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->delete($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;
+    }
 }

Tests/PHPCI/Service/BuildServiceTest.php

@@ -103,7 +103,7 @@ class BuildServiceTest extends \PHPUnit_Framework_TestCase
     {
         $build = new Build();
         $build->setId(1);
-        $build->setProjectId(101);
+        $build->setProject(101);
         $build->setCommitId('abcde');
         $build->setStatus(Build::STATUS_FAILED);
         $build->setLog('Test');

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",
+                "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/23af31f402993cfd94e99cbc4b782e9a78eb0e97",
-                "reference": "c7428acdb62ece0a45e6306f1ae85e1c05b09c01",
+                "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",