From 9a7743e8d3b7017425ac412f03b54b1b39ef8b3d Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Mon, 14 Jul 2014 14:01:29 +0100 Subject: [PATCH 01/37] Initial work on adding a Services layer to PHPCI, for better testability. Starting with Projects --- PHPCI/Controller/ProjectController.php | 47 +++++---- PHPCI/Service/ProjectService.php | 105 +++++++++++++++++++++ Tests/PHPCI/Service/MockProjectStore.php | 15 +++ Tests/PHPCI/Service/ProjectServiceTest.php | 101 ++++++++++++++++++++ phpunit.xml | 3 + 5 files changed, 245 insertions(+), 26 deletions(-) create mode 100644 PHPCI/Service/ProjectService.php create mode 100644 Tests/PHPCI/Service/MockProjectStore.php create mode 100644 Tests/PHPCI/Service/ProjectServiceTest.php diff --git a/PHPCI/Controller/ProjectController.php b/PHPCI/Controller/ProjectController.php index 38986c59..1906fb8c 100644 --- a/PHPCI/Controller/ProjectController.php +++ b/PHPCI/Controller/ProjectController.php @@ -20,6 +20,7 @@ use PHPCI\Helper\Github; use PHPCI\Helper\SshKey; use PHPCI\Model\Build; use PHPCI\Model\Project; +use PHPCI\Service\ProjectService; /** * Project Controller - Allows users to create, edit and view projects. @@ -39,10 +40,16 @@ class ProjectController extends \PHPCI\Controller */ protected $projectStore; + /** + * @var \PHPCI\Service\ProjectService + */ + protected $projectService; + public function init() { $this->buildStore = Store\Factory::getStore('Build'); $this->projectStore = Store\Factory::getStore('Project'); + $this->projectService = new ProjectService($this->projectStore); } /** @@ -179,35 +186,23 @@ class ProjectController extends \PHPCI\Controller return $view->render(); } else { - return $this->addProject($form); + $title = $this->getParam('title', 'New Project'); + $reference = $this->getParam('reference', null); + $type = $this->getParam('type', null); + + $options = array( + 'ssh_private_key' => $this->getParam('key', null), + 'ssh_public_key' => $this->getParam('pubkey', null), + 'build_config' => $this->getParam('build_config', null), + 'allow_public_status' => $this->getParam('allow_public_status', 0), + ); + + $project = $this->projectService->createProject($title, $reference, $type, $options); + header('Location: '.PHPCI_URL.'project/view/' . $project->getId()); + die; } } - protected function addProject(Form $form) - { - $values = $form->getValues(); - - $matches = array(); - if ($values['type'] == "gitlab" && preg_match('`^(.*)@(.*):(.*)/(.*)\.git`', $values['reference'], $matches)) { - $info = array(); - $info['user'] = $matches[1]; - $info['domain'] = $matches[2]; - $values['access_information'] = serialize($info); - $values['reference'] = $matches[3]."/".$matches[4]; - } - - $values['ssh_private_key'] = $values['key']; - $values['ssh_public_key'] = $values['pubkey']; - - $project = new Project(); - $project->setValues($values); - - $project = $this->projectStore->save($project); - - header('Location: '.PHPCI_URL.'project/view/' . $project->getId()); - die; - } - /** * Edit a project. Handles both the form and processing. */ diff --git a/PHPCI/Service/ProjectService.php b/PHPCI/Service/ProjectService.php new file mode 100644 index 00000000..ee2ef7b2 --- /dev/null +++ b/PHPCI/Service/ProjectService.php @@ -0,0 +1,105 @@ +projectStore = $projectStore; + } + + /** + * Create a new project model and use the project store to save it. + * @param string $title + * @param string $type + * @param string $reference + * @param array $options + * @return \PHPCI\Model\Project + */ + public function createProject($title, $type, $reference, $options = array()) + { + // Create base project and use updateProject() to set its properties: + $project = new Project(); + return $this->updateProject($project, $title, $type, $reference, $options); + } + + /** + * Update the properties of a given project. + * @param Project $project + * @param string $title + * @param string $type + * @param string $reference + * @param array $options + * @return \PHPCI\Model\Project + */ + public function updateProject(Project $project, $title, $type, $reference, $options = array()) + { + // Set basic properties: + $project->setTitle($title); + $project->setType($type); + $project->setReference($reference); + + // Handle extra project options: + if (array_key_exists('ssh_private_key', $options)) { + $project->setSshPrivateKey($options['ssh_private_key']); + } + + if (array_key_exists('ssh_public_key', $options)) { + $project->setSshPublicKey($options['ssh_public_key']); + } + + if (array_key_exists('build_config', $options)) { + $project->setBuildConfig($options['build_config']); + } + + if (array_key_exists('allow_public_status', $options)) { + $project->setAllowPublicStatus((int)$options['allow_public_status']); + } + + // Allow certain project types to set access information: + $this->processAccessInformation($project); + + // Save and return the project: + return $this->projectStore->save($project); + } + + /** + * In circumstances where it is necessary, populate access information based on other project properties. + * @see ProjectService::createProject() + * @param Project $project + */ + protected function processAccessInformation(Project &$project) + { + $matches = array(); + $reference = $project->getReference(); + + if ($project->getType() == 'gitlab' && preg_match('`^(.*)@(.*):(.*)/(.*)\.git`', $reference, $matches)) { + $info = array(); + $info['user'] = $matches[1]; + $info['domain'] = $matches[2]; + + /** @todo At a later date, we need to find a way to replace this serialized data with JSON */ + $project->setAccessInformation(serialize($info)); + $project->setReference($matches[3] . '/' . $matches[4]); + } + } +} diff --git a/Tests/PHPCI/Service/MockProjectStore.php b/Tests/PHPCI/Service/MockProjectStore.php new file mode 100644 index 00000000..a19f5e69 --- /dev/null +++ b/Tests/PHPCI/Service/MockProjectStore.php @@ -0,0 +1,15 @@ +setId(1); + return $project; + } +} diff --git a/Tests/PHPCI/Service/ProjectServiceTest.php b/Tests/PHPCI/Service/ProjectServiceTest.php new file mode 100644 index 00000000..b0ff30d6 --- /dev/null +++ b/Tests/PHPCI/Service/ProjectServiceTest.php @@ -0,0 +1,101 @@ + + */ +class ProjectServiceTest extends \PHPUnit_Framework_TestCase +{ + + /** + * @var ProjectService $testedService + */ + protected $testedService; + + /** + * @var \ $mockProjectStore + */ + protected $mockProjectStore; + + public function setUp() + { + $this->mockProjectStore = new MockProjectStore(); + $this->testedService = new ProjectService($this->mockProjectStore); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateBasicProject() + { + $returnValue = $this->testedService->createProject('Test Project', 'github', 'block8/phpci'); + + $this->assertEquals('Test Project', $returnValue->getTitle()); + $this->assertEquals('github', $returnValue->getType()); + $this->assertEquals('block8/phpci', $returnValue->getReference()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateProjectWithOptions() + { + $options = array( + 'ssh_private_key' => 'private', + 'ssh_public_key' => 'public', + 'allow_public_status' => 1, + 'build_config' => 'config', + ); + + $returnValue = $this->testedService->createProject('Test Project', 'github', 'block8/phpci', $options); + + $this->assertEquals('private', $returnValue->getSshPrivateKey()); + $this->assertEquals('public', $returnValue->getSshPublicKey()); + $this->assertEquals('config', $returnValue->getBuildConfig()); + $this->assertEquals(1, $returnValue->getAllowPublicStatus()); + } + + /** + * @link https://github.com/Block8/PHPCI/issues/484 + * @covers PHPUnit::execute + */ + public function testExecute_CreateGitlabProjectWithoutPort() + { + $reference = 'git@gitlab.block8.net:block8/phpci.git'; + $returnValue = $this->testedService->createProject('Gitlab', 'gitlab', $reference); + + $this->assertEquals('git', $returnValue->getAccessInformation('user')); + $this->assertEquals('gitlab.block8.net', $returnValue->getAccessInformation('domain')); + $this->assertEquals('block8/phpci', $returnValue->getReference()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_UpdateExistingProject() + { + $project = new Project(); + $project->setTitle('Before Title'); + $project->setReference('Before Reference'); + $project->setType('github'); + + $returnValue = $this->testedService->updateProject($project, 'After Title', 'bitbucket', 'After Reference'); + + $this->assertEquals('After Title', $returnValue->getTitle()); + $this->assertEquals('After Reference', $returnValue->getReference()); + $this->assertEquals('bitbucket', $returnValue->getType()); + } +} diff --git a/phpunit.xml b/phpunit.xml index 15bd43aa..0e1d6810 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -20,5 +20,8 @@ ./Tests/PHPCI/Plugin + + ./Tests/PHPCI/Service + From 2bff0270cf66d93620cc624ce4053f9005a886a8 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Mon, 14 Jul 2014 14:37:51 +0100 Subject: [PATCH 02/37] More work on ProjectService and its tests --- PHPCI/Controller/ProjectController.php | 29 +++++++-------- PHPCI/Service/ProjectService.php | 11 ++++++ Tests/PHPCI/Service/MockProjectStore.php | 15 -------- Tests/PHPCI/Service/ProjectServiceTest.php | 42 ++++++++++++++++++++-- 4 files changed, 64 insertions(+), 33 deletions(-) delete mode 100644 Tests/PHPCI/Service/MockProjectStore.php diff --git a/PHPCI/Controller/ProjectController.php b/PHPCI/Controller/ProjectController.php index 1906fb8c..7683e49e 100644 --- a/PHPCI/Controller/ProjectController.php +++ b/PHPCI/Controller/ProjectController.php @@ -119,7 +119,7 @@ class ProjectController extends \PHPCI\Controller } $project = $this->projectStore->getById($projectId); - $this->projectStore->delete($project); + $this->projectService->delete($project); header('Location: '.PHPCI_URL); exit; @@ -197,7 +197,7 @@ class ProjectController extends \PHPCI\Controller 'allow_public_status' => $this->getParam('allow_public_status', 0), ); - $project = $this->projectService->createProject($title, $reference, $type, $options); + $project = $this->projectService->createProject($title, $type, $reference, $options); header('Location: '.PHPCI_URL.'project/view/' . $project->getId()); die; } @@ -247,21 +247,18 @@ class ProjectController extends \PHPCI\Controller return $view->render(); } - $values = $form->getValues(); - $values['ssh_private_key'] = $values['key']; - $values['ssh_public_key'] = $values['pubkey']; + $title = $this->getParam('title', 'New Project'); + $reference = $this->getParam('reference', null); + $type = $this->getParam('type', null); - if ($values['type'] == "gitlab") { - preg_match('`^(.*)@(.*):(.*)/(.*)\.git`', $values['reference'], $matches); - $info = array(); - $info["user"] = $matches[1]; - $info["domain"] = $matches[2]; - $values['access_information'] = serialize($info); - $values['reference'] = $matches[3] . "/" . $matches[4]; - } + $options = array( + 'ssh_private_key' => $this->getParam('key', null), + 'ssh_public_key' => $this->getParam('pubkey', null), + 'build_config' => $this->getParam('build_config', null), + 'allow_public_status' => $this->getParam('allow_public_status', 0), + ); - $project->setValues($values); - $project = $this->projectStore->save($project); + $project = $this->projectService->updateProject($project, $title, $type, $reference, $options); header('Location: '.PHPCI_URL.'project/view/' . $project->getId()); die; @@ -332,7 +329,7 @@ class ProjectController extends \PHPCI\Controller $field = Form\Element\Checkbox::create('allow_public_status', $label, false); $field->setContainerClass('form-group'); $field->setCheckedValue(1); - $field->setValue(1); + $field->setValue(0); $form->addField($field); $field = new Form\Element\Submit(); diff --git a/PHPCI/Service/ProjectService.php b/PHPCI/Service/ProjectService.php index ee2ef7b2..26954ddc 100644 --- a/PHPCI/Service/ProjectService.php +++ b/PHPCI/Service/ProjectService.php @@ -57,6 +57,7 @@ class ProjectService $project->setTitle($title); $project->setType($type); $project->setReference($reference); + $project->setAllowPublicStatus(0); // Handle extra project options: if (array_key_exists('ssh_private_key', $options)) { @@ -82,6 +83,16 @@ class ProjectService return $this->projectStore->save($project); } + /** + * Delete a given project. + * @param Project $project + * @return bool + */ + public function deleteProject(Project $project) + { + return $this->projectStore->delete($project); + } + /** * In circumstances where it is necessary, populate access information based on other project properties. * @see ProjectService::createProject() diff --git a/Tests/PHPCI/Service/MockProjectStore.php b/Tests/PHPCI/Service/MockProjectStore.php deleted file mode 100644 index a19f5e69..00000000 --- a/Tests/PHPCI/Service/MockProjectStore.php +++ /dev/null @@ -1,15 +0,0 @@ -setId(1); - return $project; - } -} diff --git a/Tests/PHPCI/Service/ProjectServiceTest.php b/Tests/PHPCI/Service/ProjectServiceTest.php index b0ff30d6..146eb97c 100644 --- a/Tests/PHPCI/Service/ProjectServiceTest.php +++ b/Tests/PHPCI/Service/ProjectServiceTest.php @@ -11,7 +11,6 @@ namespace PHPCI\Service\Tests; use PHPCI\Model\Project; use PHPCI\Service\ProjectService; -use Tests\PHPCI\Service\MockProjectStore; /** * Unit tests for the ProjectService class. @@ -32,7 +31,11 @@ class ProjectServiceTest extends \PHPUnit_Framework_TestCase public function setUp() { - $this->mockProjectStore = new MockProjectStore(); + $this->mockProjectStore = $this->getMock('PHPCI\Store\ProjectStore'); + $this->mockProjectStore->expects($this->any()) + ->method('save') + ->will($this->returnArgument(0)); + $this->testedService = new ProjectService($this->mockProjectStore); } @@ -98,4 +101,39 @@ class ProjectServiceTest extends \PHPUnit_Framework_TestCase $this->assertEquals('After Reference', $returnValue->getReference()); $this->assertEquals('bitbucket', $returnValue->getType()); } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_EmptyPublicStatus() + { + $project = new Project(); + $project->setAllowPublicStatus(1); + + $options = array( + 'ssh_private_key' => 'private', + 'ssh_public_key' => 'public', + 'build_config' => 'config', + ); + + $returnValue = $this->testedService->updateProject($project, 'Test Project', 'github', 'block8/phpci', $options); + + $this->assertEquals(0, $returnValue->getAllowPublicStatus()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_DeleteProject() + { + $store = $this->getMock('PHPCI\Store\ProjectStore'); + $store->expects($this->once()) + ->method('delete') + ->will($this->returnValue(true)); + + $service = new ProjectService($store); + $project = new Project(); + + $this->assertEquals(true, $service->deleteProject($project)); + } } From cb53ec9c6d68fa2d4bbd3be4d665a4846fc17605 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Mon, 14 Jul 2014 14:59:04 +0100 Subject: [PATCH 03/37] Fixing some issues with project default branches, adding tests for that too --- PHPCI/Model/Base/ProjectBase.php | 18 ++--- PHPCI/Model/Project.php | 14 ++++ PHPCI/Service/ProjectService.php | 4 ++ Tests/PHPCI/Model/ProjectTest.php | 78 ++++++++++++++++++++++ Tests/PHPCI/Service/ProjectServiceTest.php | 3 + phpunit.xml | 3 + 6 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 Tests/PHPCI/Model/ProjectTest.php diff --git a/PHPCI/Model/Base/ProjectBase.php b/PHPCI/Model/Base/ProjectBase.php index b56279b5..5e1f4f37 100644 --- a/PHPCI/Model/Base/ProjectBase.php +++ b/PHPCI/Model/Base/ProjectBase.php @@ -36,6 +36,7 @@ class ProjectBase extends Model 'id' => null, 'title' => null, 'reference' => null, + 'branch' => null, 'ssh_private_key' => null, 'ssh_public_key' => null, 'type' => null, @@ -200,17 +201,15 @@ class ProjectBase extends Model } /** - * Get the value of Branch / branch. - * - * @return string - */ + * Get the value of Branch / branch. + * + * @return string + */ public function getBranch() { - if (empty($this->data['branch'])) { - return $this->getType() === 'hg' ? 'default' : 'master'; - } else { - return $this->data['branch']; - } + $rtn = $this->data['branch']; + + return $rtn; } /** @@ -365,6 +364,7 @@ class ProjectBase extends Model */ public function setBranch($value) { + $this->_validateNotNull('Branch', $value); $this->_validateString('Branch', $value); if ($this->data['branch'] === $value) { diff --git a/PHPCI/Model/Project.php b/PHPCI/Model/Project.php index 55df01c5..9047de02 100644 --- a/PHPCI/Model/Project.php +++ b/PHPCI/Model/Project.php @@ -58,4 +58,18 @@ class Project extends ProjectBase return $rtn; } + + /** + * Get the value of Branch / branch. + * + * @return string + */ + public function getBranch() + { + if (empty($this->data['branch'])) { + return $this->getType() === 'hg' ? 'default' : 'master'; + } else { + return $this->data['branch']; + } + } } diff --git a/PHPCI/Service/ProjectService.php b/PHPCI/Service/ProjectService.php index 26954ddc..1ab24aae 100644 --- a/PHPCI/Service/ProjectService.php +++ b/PHPCI/Service/ProjectService.php @@ -76,6 +76,10 @@ class ProjectService $project->setAllowPublicStatus((int)$options['allow_public_status']); } + if (array_key_exists('branch', $options)) { + $project->setBranch($options['branch']); + } + // Allow certain project types to set access information: $this->processAccessInformation($project); diff --git a/Tests/PHPCI/Model/ProjectTest.php b/Tests/PHPCI/Model/ProjectTest.php new file mode 100644 index 00000000..367eaa77 --- /dev/null +++ b/Tests/PHPCI/Model/ProjectTest.php @@ -0,0 +1,78 @@ + + */ +class ProjectTest extends \PHPUnit_Framework_TestCase +{ + public function setUp() + { + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestGitDefaultBranch() + { + $project = new Project(); + $project->setType('git'); + + $this->assertEquals('master', $project->getBranch()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestGithubDefaultBranch() + { + $project = new Project(); + $project->setType('github'); + + $this->assertEquals('master', $project->getBranch()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestGitlabDefaultBranch() + { + $project = new Project(); + $project->setType('gitlab'); + + $this->assertEquals('master', $project->getBranch()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestBitbucketDefaultBranch() + { + $project = new Project(); + $project->setType('bitbucket'); + + $this->assertEquals('master', $project->getBranch()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestMercurialDefaultBranch() + { + $project = new Project(); + $project->setType('hg'); + + $this->assertEquals('default', $project->getBranch()); + } +} diff --git a/Tests/PHPCI/Service/ProjectServiceTest.php b/Tests/PHPCI/Service/ProjectServiceTest.php index 146eb97c..abb06d8f 100644 --- a/Tests/PHPCI/Service/ProjectServiceTest.php +++ b/Tests/PHPCI/Service/ProjectServiceTest.php @@ -49,6 +49,7 @@ class ProjectServiceTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Test Project', $returnValue->getTitle()); $this->assertEquals('github', $returnValue->getType()); $this->assertEquals('block8/phpci', $returnValue->getReference()); + $this->assertEquals('master', $returnValue->getBranch()); } /** @@ -61,6 +62,7 @@ class ProjectServiceTest extends \PHPUnit_Framework_TestCase 'ssh_public_key' => 'public', 'allow_public_status' => 1, 'build_config' => 'config', + 'branch' => 'testbranch', ); $returnValue = $this->testedService->createProject('Test Project', 'github', 'block8/phpci', $options); @@ -68,6 +70,7 @@ class ProjectServiceTest extends \PHPUnit_Framework_TestCase $this->assertEquals('private', $returnValue->getSshPrivateKey()); $this->assertEquals('public', $returnValue->getSshPublicKey()); $this->assertEquals('config', $returnValue->getBuildConfig()); + $this->assertEquals('testbranch', $returnValue->getBranch()); $this->assertEquals(1, $returnValue->getAllowPublicStatus()); } diff --git a/phpunit.xml b/phpunit.xml index 0e1d6810..78685417 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -20,6 +20,9 @@ ./Tests/PHPCI/Plugin + + ./Tests/PHPCI/Model + ./Tests/PHPCI/Service From 4d2583e536a89a22093f0326ec7149d611ce933b Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Mon, 14 Jul 2014 16:02:36 +0100 Subject: [PATCH 04/37] More service layer functionality, builds now. Also some extra tests for projects and build models. --- PHPCI/Controller/BuildController.php | 23 ++-- PHPCI/Controller/ProjectController.php | 27 ++-- PHPCI/Model/Base/BuildBase.php | 157 +++++++++++++++++++++++ PHPCI/Model/Build.php | 28 ++-- PHPCI/Service/BuildService.php | 112 ++++++++++++++++ PHPCI/Store/Base/BuildStoreBase.php | 32 +++++ Tests/PHPCI/Model/BuildTest.php | 84 ++++++++++++ Tests/PHPCI/Model/ProjectTest.php | 33 ++++- Tests/PHPCI/Service/BuildServiceTest.php | 148 +++++++++++++++++++++ 9 files changed, 602 insertions(+), 42 deletions(-) create mode 100644 PHPCI/Service/BuildService.php create mode 100644 Tests/PHPCI/Model/BuildTest.php create mode 100644 Tests/PHPCI/Service/BuildServiceTest.php diff --git a/PHPCI/Controller/BuildController.php b/PHPCI/Controller/BuildController.php index 145236d5..600c8508 100644 --- a/PHPCI/Controller/BuildController.php +++ b/PHPCI/Controller/BuildController.php @@ -13,6 +13,7 @@ use b8; use b8\Exception\HttpException\NotFoundException; use PHPCI\BuildFactory; use PHPCI\Model\Build; +use PHPCI\Service\BuildService; /** * Build Controller - Allows users to run and view builds. @@ -26,10 +27,16 @@ class BuildController extends \PHPCI\Controller * @var \PHPCI\Store\BuildStore */ protected $buildStore; + + /** + * @var \PHPCI\Service\BuildService + */ + protected $buildService; public function init() { - $this->buildStore = b8\Store\Factory::getStore('Build'); + $this->buildStore = b8\Store\Factory::getStore('Build'); + $this->buildService = new BuildService($this->buildStore); } /** @@ -123,17 +130,7 @@ class BuildController extends \PHPCI\Controller throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); } - $build = new Build(); - $build->setProjectId($copy->getProjectId()); - $build->setCommitId($copy->getCommitId()); - $build->setStatus(Build::STATUS_NEW); - $build->setBranch($copy->getBranch()); - $build->setCreated(new \DateTime()); - $build->setCommitterEmail($copy->getCommitterEmail()); - $build->setCommitMessage($copy->getCommitMessage()); - $build->setExtra(json_encode($copy->getExtra())); - - $build = $this->buildStore->save($build); + $build = $this->buildService->createDuplicateBuild($copy); header('Location: '.PHPCI_URL.'build/view/' . $build->getId()); exit; @@ -154,7 +151,7 @@ class BuildController extends \PHPCI\Controller throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); } - $this->buildStore->delete($build); + $this->buildService->delete($build); header('Location: '.PHPCI_URL.'project/view/' . $build->getProjectId()); exit; diff --git a/PHPCI/Controller/ProjectController.php b/PHPCI/Controller/ProjectController.php index 7683e49e..5add67a8 100644 --- a/PHPCI/Controller/ProjectController.php +++ b/PHPCI/Controller/ProjectController.php @@ -20,6 +20,7 @@ use PHPCI\Helper\Github; use PHPCI\Helper\SshKey; use PHPCI\Model\Build; use PHPCI\Model\Project; +use PHPCI\Service\BuildService; use PHPCI\Service\ProjectService; /** @@ -30,11 +31,6 @@ use PHPCI\Service\ProjectService; */ class ProjectController extends \PHPCI\Controller { - /** - * @var \PHPCI\Store\BuildStore - */ - protected $buildStore; - /** * @var \PHPCI\Store\ProjectStore */ @@ -45,11 +41,22 @@ class ProjectController extends \PHPCI\Controller */ protected $projectService; + /** + * @var \PHPCI\Store\BuildStore + */ + protected $buildStore; + + /** + * @var \PHPCI\Service\BuildService + */ + protected $buildService; + public function init() { $this->buildStore = Store\Factory::getStore('Build'); $this->projectStore = Store\Factory::getStore('Project'); $this->projectService = new ProjectService($this->projectStore); + $this->buildService = new BuildService($this->buildStore); } /** @@ -95,15 +102,7 @@ class ProjectController extends \PHPCI\Controller throw new NotFoundException('Project with id: ' . $projectId . ' not found'); } - $build = new Build(); - $build->setProjectId($projectId); - $build->setCommitId('Manual'); - $build->setStatus(Build::STATUS_NEW); - $build->setBranch($project->getBranch()); - $build->setCreated(new \DateTime()); - $build->setCommitterEmail($_SESSION['user']->getEmail()); - - $build = $this->buildStore->save($build); + $build = $this->buildService->createBuild($project, null, null, $_SESSION['user']->getEmail()); header('Location: '.PHPCI_URL.'build/view/' . $build->getId()); exit; diff --git a/PHPCI/Model/Base/BuildBase.php b/PHPCI/Model/Base/BuildBase.php index 97b427b5..648bb18c 100644 --- a/PHPCI/Model/Base/BuildBase.php +++ b/PHPCI/Model/Base/BuildBase.php @@ -45,6 +45,8 @@ class BuildBase extends Model 'committer_email' => null, 'commit_message' => null, 'extra' => null, + 'parent_id' => null, + 'engine' => null, ); /** @@ -64,9 +66,12 @@ class BuildBase extends Model 'committer_email' => 'getCommitterEmail', 'commit_message' => 'getCommitMessage', 'extra' => 'getExtra', + 'parent_id' => 'getParentId', + 'engine' => 'getEngine', // Foreign key getters: 'Project' => 'getProject', + 'Parent' => 'getParent', ); /** @@ -86,9 +91,12 @@ class BuildBase extends Model 'committer_email' => 'setCommitterEmail', 'commit_message' => 'setCommitMessage', 'extra' => 'setExtra', + 'parent_id' => 'setParentId', + 'engine' => 'setEngine', // Foreign key setters: 'Project' => 'setProject', + 'Parent' => 'setParent', ); /** @@ -159,6 +167,18 @@ class BuildBase extends Model 'nullable' => true, 'default' => null, ), + 'parent_id' => array( + 'type' => 'int', + 'length' => 11, + 'nullable' => true, + 'default' => null, + ), + 'engine' => array( + 'type' => 'varchar', + 'length' => 50, + 'nullable' => true, + 'default' => null, + ), ); /** @@ -168,6 +188,7 @@ class BuildBase extends Model 'PRIMARY' => array('unique' => true, 'columns' => 'id'), 'project_id' => array('columns' => 'project_id'), 'idx_status' => array('columns' => 'status'), + 'parent_id' => array('columns' => 'parent_id'), ); /** @@ -181,6 +202,13 @@ class BuildBase extends Model 'table' => 'project', 'col' => 'id' ), + 'build_ibfk_2' => array( + 'local_col' => 'parent_id', + 'update' => 'CASCADE', + 'delete' => 'CASCADE', + 'table' => 'build', + 'col' => 'id' + ), ); /** @@ -339,6 +367,30 @@ class BuildBase extends Model return $rtn; } + /** + * Get the value of ParentId / parent_id. + * + * @return int + */ + public function getParentId() + { + $rtn = $this->data['parent_id']; + + return $rtn; + } + + /** + * Get the value of Engine / engine. + * + * @return string + */ + public function getEngine() + { + $rtn = $this->data['engine']; + + return $rtn; + } + /** * Set the value of Id / id. * @@ -563,6 +615,42 @@ class BuildBase extends Model $this->_setModified('extra'); } + /** + * Set the value of ParentId / parent_id. + * + * @param $value int + */ + public function setParentId($value) + { + $this->_validateInt('ParentId', $value); + + if ($this->data['parent_id'] === $value) { + return; + } + + $this->data['parent_id'] = $value; + + $this->_setModified('parent_id'); + } + + /** + * Set the value of Engine / engine. + * + * @param $value string + */ + public function setEngine($value) + { + $this->_validateString('Engine', $value); + + if ($this->data['engine'] === $value) { + return; + } + + $this->data['engine'] = $value; + + $this->_setModified('engine'); + } + /** * Get the Project model for this Build by Id. * @@ -620,6 +708,75 @@ class BuildBase extends Model return $this->setProjectId($value->getId()); } + /** + * Get the Build model for this Build by Id. + * + * @uses \PHPCI\Store\BuildStore::getById() + * @uses \PHPCI\Model\Build + * @return \PHPCI\Model\Build + */ + public function getParent() + { + $key = $this->getParentId(); + + if (empty($key)) { + return null; + } + + $cacheKey = 'Cache.Build.' . $key; + $rtn = $this->cache->get($cacheKey, null); + + if (empty($rtn)) { + $rtn = Factory::getStore('Build', 'PHPCI')->getById($key); + $this->cache->set($cacheKey, $rtn); + } + + return $rtn; + } + + /** + * Set Parent - Accepts an ID, an array representing a Build or a Build model. + * + * @param $value mixed + */ + public function setParent($value) + { + // Is this an instance of Build? + if ($value instanceof \PHPCI\Model\Build) { + return $this->setParentObject($value); + } + + // Is this an array representing a Build item? + if (is_array($value) && !empty($value['id'])) { + return $this->setParentId($value['id']); + } + + // Is this a scalar value representing the ID of this foreign key? + return $this->setParentId($value); + } + + /** + * Set Parent - Accepts a Build model. + * + * @param $value \PHPCI\Model\Build + */ + public function setParentObject(\PHPCI\Model\Build $value) + { + return $this->setParentId($value->getId()); + } + + /** + * Get Build models by ParentId for this Build. + * + * @uses \PHPCI\Store\BuildStore::getByParentId() + * @uses \PHPCI\Model\Build + * @return \PHPCI\Model\Build[] + */ + public function getParentBuilds() + { + return Factory::getStore('Build', 'PHPCI')->getByParentId($this->getId()); + } + /** * Get BuildMeta models by BuildId for this Build. * diff --git a/PHPCI/Model/Build.php b/PHPCI/Model/Build.php index 4ca3cf9e..0286b3b8 100644 --- a/PHPCI/Model/Build.php +++ b/PHPCI/Model/Build.php @@ -38,15 +38,6 @@ class Build extends BuildBase return '#'; } - /** - * @return string - */ - public function getProjectTitle() - { - $project = $this->getProject(); - return $project ? $project->getTitle() : ""; - } - /** * Get link to branch from another source (i.e. Github) */ @@ -55,6 +46,11 @@ class Build extends BuildBase return '#'; } + public function getFileLinkTemplate() + { + return null; + } + /** * Send status updates to any relevant third parties (i.e. Github) */ @@ -63,6 +59,15 @@ class Build extends BuildBase return; } + /** + * @return string + */ + public function getProjectTitle() + { + $project = $this->getProject(); + return $project ? $project->getTitle() : ""; + } + /** * Store build metadata */ @@ -160,11 +165,6 @@ class Build extends BuildBase return $config; } - public function getFileLinkTemplate() - { - return null; - } - public function getExtra($key = null) { $data = json_decode($this->data['extra'], true); diff --git a/PHPCI/Service/BuildService.php b/PHPCI/Service/BuildService.php new file mode 100644 index 00000000..99f06ec2 --- /dev/null +++ b/PHPCI/Service/BuildService.php @@ -0,0 +1,112 @@ +buildStore = $buildStore; + } + + /** + * @param Project $project + * @param string|null $commitId + * @param string|null $branch + * @param string|null $committerEmail + * @param string|null $commitMessage + * @param string|null $extra + * @return \PHPCI\Model\Build + */ + public function createBuild( + Project $project, + $commitId = null, + $branch = null, + $committerEmail = null, + $commitMessage = null, + $extra = null + ) + { + $build = new Build(); + $build->setCreated(new \DateTime()); + $build->setProject($project); + $build->setStatus(0); + + if (!is_null($commitId)) { + $build->setCommitId($commitId); + } else { + $build->setCommitId('Manual'); + } + + if (!is_null($branch)) { + $build->setBranch($branch); + } else { + $build->setBranch($project->getBranch()); + } + + if (!is_null($committerEmail)) { + $build->setCommitterEmail($committerEmail); + } + + if (!is_null($commitMessage)) { + $build->setCommitMessage($commitMessage); + } + + if (!is_null($extra)) { + $build->setExtra(json_encode($extra)); + } + + return $this->buildStore->save($build); + } + + /** + * @param Build $copyFrom + * @return \PHPCI\Model\Build + */ + public function createDuplicateBuild(Build $copyFrom) + { + $data = $copyFrom->getDataArray(); + + // Clean up unwanted properties from the original build: + unset($data['id']); + unset($data['status']); + unset($data['log']); + unset($data['started']); + unset($data['finished']); + + $build = new Build(); + $build->setValues($data); + $build->setCreated(new \DateTime()); + + return $this->buildStore->save($build); + } + + /** + * Delete a given build. + * @param Build $build + * @return bool + */ + public function deleteBuild(Build $build) + { + return $this->buildStore->delete($build); + } +} diff --git a/PHPCI/Store/Base/BuildStoreBase.php b/PHPCI/Store/Base/BuildStoreBase.php index b67d5f73..ff21155a 100644 --- a/PHPCI/Store/Base/BuildStoreBase.php +++ b/PHPCI/Store/Base/BuildStoreBase.php @@ -107,4 +107,36 @@ class BuildStoreBase extends Store return array('items' => array(), 'count' => 0); } } + + public function getByParentId($value, $limit = null, $useConnection = 'read') + { + if (is_null($value)) { + throw new HttpException('Value passed to ' . __FUNCTION__ . ' cannot be null.'); + } + + $add = ''; + + if ($limit) { + $add .= ' LIMIT ' . $limit; + } + + $count = null; + + $query = 'SELECT * FROM `build` WHERE `parent_id` = :parent_id' . $add; + $stmt = Database::getConnection($useConnection)->prepare($query); + $stmt->bindValue(':parent_id', $value); + + if ($stmt->execute()) { + $res = $stmt->fetchAll(\PDO::FETCH_ASSOC); + + $map = function ($item) { + return new Build($item); + }; + $rtn = array_map($map, $res); + + return array('items' => $rtn, 'count' => $count); + } else { + return array('items' => array(), 'count' => 0); + } + } } diff --git a/Tests/PHPCI/Model/BuildTest.php b/Tests/PHPCI/Model/BuildTest.php new file mode 100644 index 00000000..b8ee3432 --- /dev/null +++ b/Tests/PHPCI/Model/BuildTest.php @@ -0,0 +1,84 @@ + + */ +class BuildTest extends \PHPUnit_Framework_TestCase +{ + public function setUp() + { + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestIsAValidModel() + { + $build = new Build(); + $this->assertTrue($build instanceof \b8\Model); + $this->assertTrue($build instanceof Model); + $this->assertTrue($build instanceof Model\Base\BuildBase); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestBaseBuildDefaults() + { + $build = new Build(); + $this->assertEquals('#', $build->getCommitLink()); + $this->assertEquals('#', $build->getBranchLink()); + $this->assertEquals(null, $build->getFileLinkTemplate()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestIsSuccessful() + { + $build = new Build(); + $build->setStatus(Build::STATUS_NEW); + $this->assertFalse($build->isSuccessful()); + + $build->setStatus(Build::STATUS_RUNNING); + $this->assertFalse($build->isSuccessful()); + + $build->setStatus(Build::STATUS_FAILED); + $this->assertFalse($build->isSuccessful()); + + $build->setStatus(Build::STATUS_SUCCESS); + $this->assertTrue($build->isSuccessful()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestBuildExtra() + { + $info = array( + 'item1' => 'Item One', + 'item2' => 2, + ); + + $build = new Build(); + $build->setExtra(json_encode($info)); + + $this->assertEquals('Item One', $build->getExtra('item1')); + $this->assertEquals(2, $build->getExtra('item2')); + $this->assertNull($build->getExtra('item3')); + $this->assertEquals($info, $build->getExtra()); + } +} diff --git a/Tests/PHPCI/Model/ProjectTest.php b/Tests/PHPCI/Model/ProjectTest.php index 367eaa77..95cf933c 100644 --- a/Tests/PHPCI/Model/ProjectTest.php +++ b/Tests/PHPCI/Model/ProjectTest.php @@ -10,9 +10,10 @@ namespace PHPCI\Model\Tests; use PHPCI\Model\Project; +use PHPCI\Model; /** - * Unit tests for the ProjectService class. + * Unit tests for the Project model class. * @author Dan Cryer */ class ProjectTest extends \PHPUnit_Framework_TestCase @@ -21,6 +22,17 @@ class ProjectTest extends \PHPUnit_Framework_TestCase { } + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestIsAValidModel() + { + $project = new Project(); + $this->assertTrue($project instanceof \b8\Model); + $this->assertTrue($project instanceof Model); + $this->assertTrue($project instanceof Model\Base\ProjectBase); + } + /** * @covers PHPUnit::execute */ @@ -75,4 +87,23 @@ class ProjectTest extends \PHPUnit_Framework_TestCase $this->assertEquals('default', $project->getBranch()); } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestProjectAccessInformation() + { + $info = array( + 'item1' => 'Item One', + 'item2' => 2, + ); + + $project = new Project(); + $project->setAccessInformation(serialize($info)); + + $this->assertEquals('Item One', $project->getAccessInformation('item1')); + $this->assertEquals(2, $project->getAccessInformation('item2')); + $this->assertNull($project->getAccessInformation('item3')); + $this->assertEquals($info, $project->getAccessInformation()); + } } diff --git a/Tests/PHPCI/Service/BuildServiceTest.php b/Tests/PHPCI/Service/BuildServiceTest.php new file mode 100644 index 00000000..9b1097a3 --- /dev/null +++ b/Tests/PHPCI/Service/BuildServiceTest.php @@ -0,0 +1,148 @@ + + */ +class BuildServiceTest extends \PHPUnit_Framework_TestCase +{ + + /** + * @var BuildService $testedService + */ + protected $testedService; + + /** + * @var \ $mockBuildStore + */ + protected $mockBuildStore; + + public function setUp() + { + $this->mockBuildStore = $this->getMock('PHPCI\Store\BuildStore'); + $this->mockBuildStore->expects($this->any()) + ->method('save') + ->will($this->returnArgument(0)); + + $this->testedService = new BuildService($this->mockBuildStore); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateBasicBuild() + { + $project = new Project(); + $project->setType('github'); + $project->setId(101); + + $returnValue = $this->testedService->createBuild($project); + + $this->assertEquals(101, $returnValue->getProjectId()); + $this->assertEquals(Build::STATUS_NEW, $returnValue->getStatus()); + $this->assertNull($returnValue->getStarted()); + $this->assertNull($returnValue->getFinished()); + $this->assertNull($returnValue->getLog()); + $this->assertNull($returnValue->getCommitMessage()); + $this->assertNull($returnValue->getCommitterEmail()); + $this->assertNull($returnValue->getExtra()); + $this->assertEquals('master', $returnValue->getBranch()); + $this->assertInstanceOf('DateTime', $returnValue->getCreated()); + $this->assertEquals('Manual', $returnValue->getCommitId()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateBuildWithOptions() + { + $project = new Project(); + $project->setType('hg'); + $project->setId(101); + + $returnValue = $this->testedService->createBuild($project, '123', 'testbranch', 'test@example.com', 'test'); + + $this->assertEquals('testbranch', $returnValue->getBranch()); + $this->assertEquals('123', $returnValue->getCommitId()); + $this->assertEquals('test', $returnValue->getCommitMessage()); + $this->assertEquals('test@example.com', $returnValue->getCommitterEmail()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateBuildWithExtra() + { + $project = new Project(); + $project->setType('bitbucket'); + $project->setId(101); + + $returnValue = $this->testedService->createBuild($project, null, null, null, null, array('item1' => 1001)); + + $this->assertEquals(1001, $returnValue->getExtra('item1')); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateDuplicateBuild() + { + $build = new Build(); + $build->setId(1); + $build->setProjectId(101); + $build->setCommitId('abcde'); + $build->setStatus(Build::STATUS_FAILED); + $build->setLog('Test'); + $build->setBranch('example_branch'); + $build->setStarted(new \DateTime()); + $build->setFinished(new \DateTime()); + $build->setCommitMessage('test'); + $build->setCommitterEmail('test@example.com'); + $build->setExtra(json_encode(array('item1' => 1001))); + + $returnValue = $this->testedService->createDuplicateBuild($build); + + $this->assertNotEquals($build->getId(), $returnValue->getId()); + $this->assertEquals($build->getProjectId(), $returnValue->getProjectId()); + $this->assertEquals($build->getCommitId(), $returnValue->getCommitId()); + $this->assertNotEquals($build->getStatus(), $returnValue->getStatus()); + $this->assertEquals(Build::STATUS_NEW, $returnValue->getStatus()); + $this->assertNull($returnValue->getLog()); + $this->assertEquals($build->getBranch(), $returnValue->getBranch()); + $this->assertNotEquals($build->getCreated(), $returnValue->getCreated()); + $this->assertNull($returnValue->getStarted()); + $this->assertNull($returnValue->getFinished()); + $this->assertEquals('test', $returnValue->getCommitMessage()); + $this->assertEquals('test@example.com', $returnValue->getCommitterEmail()); + $this->assertEquals($build->getExtra('item1'), $returnValue->getExtra('item1')); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_DeleteBuild() + { + $store = $this->getMock('PHPCI\Store\BuildStore'); + $store->expects($this->once()) + ->method('delete') + ->will($this->returnValue(true)); + + $service = new BuildService($store); + $build = new Build(); + + $this->assertEquals(true, $service->deleteBuild($build)); + } +} From a20cf49bf9cd698c2e517bf0dbba4720af369e56 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Mon, 14 Jul 2014 16:08:19 +0100 Subject: [PATCH 05/37] Updating webhooks to use build service --- PHPCI/Controller/WebhookController.php | 32 +++++++++++++++----------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/PHPCI/Controller/WebhookController.php b/PHPCI/Controller/WebhookController.php index e95a5597..4d4501ed 100644 --- a/PHPCI/Controller/WebhookController.php +++ b/PHPCI/Controller/WebhookController.php @@ -13,6 +13,7 @@ use b8; use b8\Store; use PHPCI\BuildFactory; use PHPCI\Model\Build; +use PHPCI\Service\BuildService; /** * Webhook Controller - Processes webhook pings from BitBucket, Github, Gitlab, etc. @@ -29,9 +30,21 @@ class WebhookController extends \PHPCI\Controller */ protected $buildStore; + /** + * @var \PHPCI\Store\ProjectStore + */ + protected $projectStore; + + /** + * @var \PHPCI\Service\BuildService + */ + protected $buildService; + public function init() { $this->buildStore = Store\Factory::getStore('Build'); + $this->projectStore = Store\Factory::getStore('Project'); + $this->buildService = new BuildService($this->buildStore); } /** @@ -238,22 +251,15 @@ class WebhookController extends \PHPCI\Controller return true; } - // If not, create a new build job for it: - $build = new Build(); - $build->setProjectId($projectId); - $build->setCommitId($commitId); - $build->setStatus(Build::STATUS_NEW); - $build->setLog(''); - $build->setCreated(new \DateTime()); - $build->setBranch($branch); - $build->setCommitterEmail($committer); - $build->setCommitMessage($commitMessage); + $project = $this->projectStore->getById($projectId); - if (!is_null($extra)) { - $build->setExtra(json_encode($extra)); + if (empty($project)) { + throw new \Exception('Project does not exist:' . $projectId); } - $build = BuildFactory::getBuild($this->buildStore->save($build)); + // If not, create a new build job for it: + $build = $this->buildService->createBuild($project, $commitId, $branch, $committer, $commitMessage, $extra); + $build = BuildFactory::getBuild($build); // Send a status postback if the build type provides one: $build->sendStatusPostback(); From bfc7a5819577ad4eb5f39783a522e4b4c7270849 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 23 Jul 2014 14:54:49 +0100 Subject: [PATCH 06/37] Fixing PHPCI issues for this branch --- PHPCI/Service/BuildService.php | 3 +-- phpci.yml | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/PHPCI/Service/BuildService.php b/PHPCI/Service/BuildService.php index 99f06ec2..300df728 100644 --- a/PHPCI/Service/BuildService.php +++ b/PHPCI/Service/BuildService.php @@ -44,8 +44,7 @@ class BuildService $committerEmail = null, $commitMessage = null, $extra = null - ) - { + ) { $build = new Build(); $build->setCreated(new \DateTime()); $build->setProject($project); diff --git a/phpci.yml b/phpci.yml index 4522d0fe..20419bda 100644 --- a/phpci.yml +++ b/phpci.yml @@ -6,6 +6,7 @@ build_settings: - "PHPCI/Command" # PHPMD complains about un-used parameters, but they are required. - "public/install.php" # PHPCS really doesn't like PHP mixed with HTML (and so it shouldn't) - "PHPCI/Migrations" # Ignore the migrations directory, as both PHPMD and PHPCS can't cope with them. + - "PHPCI/Model/Base" # These files are auto-generated, and sometimes hit PHPMD complexity thresholds. setup: composer: From 06ccdd1937f050958dbe689fb65e464b9d1de6f7 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 23 Jul 2014 15:50:34 +0100 Subject: [PATCH 07/37] Moving user controller to using a service class --- PHPCI/Controller/UserController.php | 48 +++++++++++------------ PHPCI/Service/UserService.php | 61 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 PHPCI/Service/UserService.php diff --git a/PHPCI/Controller/UserController.php b/PHPCI/Controller/UserController.php index 86fa8662..f9c4fced 100644 --- a/PHPCI/Controller/UserController.php +++ b/PHPCI/Controller/UserController.php @@ -15,6 +15,7 @@ use b8\Exception\HttpException\NotFoundException; use b8\Form; use PHPCI\Controller; use PHPCI\Model\User; +use PHPCI\Service\UserService; /** * User Controller - Allows an administrator to view, add, edit and delete users. @@ -29,9 +30,15 @@ class UserController extends Controller */ protected $userStore; + /** + * @var \PHPCI\Service\UserService + */ + protected $userService; + public function init() { $this->userStore = b8\Store\Factory::getStore('User'); + $this->userService = new UserService($this->userStore); } /** @@ -53,16 +60,11 @@ class UserController extends Controller $values = $user->getDataArray(); if ($this->request->getMethod() == 'POST') { - $values = $this->getParams(); + $name = $this->getParam('name', null); + $email = $this->getParam('email', null); + $password = $this->getParam('password', null); - if (!empty($values['password'])) { - $values['hash'] = password_hash($values['password'], PASSWORD_DEFAULT); - } - - $this->view->updated = true; - - $user->setValues($values); - $_SESSION['user'] = $this->userStore->save($user); + $_SESSION['user'] = $this->userService->updateUser($name, $email, $password); } $form = new Form(); @@ -132,13 +134,13 @@ class UserController extends Controller return $view->render(); } - $values = $form->getValues(); - $values['hash'] = password_hash($values['password'], PASSWORD_DEFAULT); - $user = new User(); - $user->setValues($values); + $name = $this->getParam('name', null); + $email = $this->getParam('email', null); + $password = $this->getParam('password', null); + $isAdmin = (int)$this->getParam('is_admin', 0); - $user = $this->userStore->save($user); + $this->userService->createUser($name, $email, $password, $isAdmin); header('Location: '.PHPCI_URL.'user'); die; @@ -172,18 +174,12 @@ class UserController extends Controller return $view->render(); } - if (!empty($values['password'])) { - $values['hash'] = password_hash($values['password'], PASSWORD_DEFAULT); - } + $name = $this->getParam('name', null); + $email = $this->getParam('email', null); + $password = $this->getParam('password', null); + $isAdmin = (int)$this->getParam('is_admin', 0); - $user->setValues($values); - - $isAdmin = $this->getParam('is_admin'); - if (empty($isAdmin)) { - $user->setIsAdmin(0); - } - - $this->userStore->save($user); + $this->userService->updateUser($user, $name, $email, $password, $isAdmin); header('Location: '.PHPCI_URL.'user'); die; @@ -258,7 +254,7 @@ class UserController extends Controller throw new NotFoundException('User with ID: ' . $userId . ' does not exist.'); } - $this->userStore->delete($user); + $this->userService->delete($user); header('Location: '.PHPCI_URL.'user'); die; diff --git a/PHPCI/Service/UserService.php b/PHPCI/Service/UserService.php new file mode 100644 index 00000000..8f20044f --- /dev/null +++ b/PHPCI/Service/UserService.php @@ -0,0 +1,61 @@ +store = $store; + } + + public function createUser($name, $emailAddress, $password, $isAdmin = false) + { + $user = new User(); + $user->setName($name); + $user->setEmail($emailAddress); + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + $user->setIsAdmin(($isAdmin ? 1 : 0)); + + return $this->store->save($user); + } + + public function updateUser(User $user, $name, $emailAddress, $password = null, $isAdmin = null) + { + $user->setName($name); + $user->setEmail($emailAddress); + + if (!empty($password)) { + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + } + + if (!is_null($isAdmin)) { + $user->setIsAdmin(($isAdmin ? 1 : 0)); + } + + return $this->store->save($user); + } + + public function deleteUser(User $user) + { + return $this->store->delete($user); + } +} From f24f5c0a45e2ccfa364173c5b4308f34ff46651a Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 23 Jul 2014 15:56:23 +0100 Subject: [PATCH 08/37] Migrating install and create-admin console commands to use userservice --- PHPCI/Command/CreateAdminCommand.php | 15 +++++---------- PHPCI/Command/InstallCommand.php | 11 +++-------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/PHPCI/Command/CreateAdminCommand.php b/PHPCI/Command/CreateAdminCommand.php index 30d800b1..fc40c307 100644 --- a/PHPCI/Command/CreateAdminCommand.php +++ b/PHPCI/Command/CreateAdminCommand.php @@ -9,6 +9,7 @@ namespace PHPCI\Command; +use PHPCI\Service\UserService; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -37,7 +38,9 @@ class CreateAdminCommand extends Command */ protected function execute(InputInterface $input, OutputInterface $output) { - + $userStore = Factory::getStore('User'); + $userService = new UserService($userStore); + require(PHPCI_DIR . 'bootstrap.php'); // Try to create a user account: @@ -51,15 +54,7 @@ class CreateAdminCommand extends Command $adminName = $this->ask('Admin name: '); try { - $user = new \PHPCI\Model\User(); - $user->setEmail($adminEmail); - $user->setName($adminName); - $user->setIsAdmin(1); - $user->setHash(password_hash($adminPass, PASSWORD_DEFAULT)); - - $store = \b8\Store\Factory::getStore('User'); - $store->save($user); - + $userService->createUser($adminName, $adminEmail, $adminPass, 1); print 'User account created!' . PHP_EOL; } catch (\Exception $ex) { print 'There was a problem creating your account. :(' . PHP_EOL; diff --git a/PHPCI/Command/InstallCommand.php b/PHPCI/Command/InstallCommand.php index 43495dc0..e5317261 100644 --- a/PHPCI/Command/InstallCommand.php +++ b/PHPCI/Command/InstallCommand.php @@ -236,16 +236,11 @@ class InstallCommand extends Command $adminName = $dialog->ask($output, 'Enter your name: '); try { - $user = new User(); - $user->setEmail($adminEmail); - $user->setName($adminName); - $user->setIsAdmin(1); - $user->setHash(password_hash($adminPass, PASSWORD_DEFAULT)); - $this->reloadConfig(); - $store = Factory::getStore('User'); - $store->save($user); + $userStore = Factory::getStore('User'); + $userService = new UserService($userStore); + $userService->createUser($adminName, $adminEmail, $adminPass, 1); $output->writeln('User account created!'); } catch (\Exception $ex) { From 6b8f008cfe4958f253a5e278886b825fc13ef8ed Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 23 Jul 2014 16:11:47 +0100 Subject: [PATCH 09/37] Adding some tests for the UserService class. --- Tests/PHPCI/Service/UserServiceTest.php | 116 ++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 Tests/PHPCI/Service/UserServiceTest.php diff --git a/Tests/PHPCI/Service/UserServiceTest.php b/Tests/PHPCI/Service/UserServiceTest.php new file mode 100644 index 00000000..1358bd7b --- /dev/null +++ b/Tests/PHPCI/Service/UserServiceTest.php @@ -0,0 +1,116 @@ + + */ +class UserServiceTest extends \PHPUnit_Framework_TestCase +{ + + /** + * @var UserService $testedService + */ + protected $testedService; + + /** + * @var \ $mockBuildStore + */ + protected $mockUserStore; + + public function setUp() + { + $this->mockUserStore = $this->getMock('PHPCI\Store\UserStore'); + $this->mockUserStore->expects($this->any()) + ->method('save') + ->will($this->returnArgument(0)); + + $this->testedService = new UserService($this->mockUserStore); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateNonAdminUser() + { + $user = $this->testedService->createUser('Test', 'test@example.com', 'testing', 0); + + $this->assertEquals('Test', $user->getName()); + $this->assertEquals('test@example.com', $user->getEmail()); + $this->assertEquals(0, $user->getIsAdmin()); + $this->assertTrue(password_verify('testing', $user->getHash())); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_CreateAdminUser() + { + $user = $this->testedService->createUser('Test', 'test@example.com', 'testing', 1); + $this->assertEquals(1, $user->getIsAdmin()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_RevokeAdminStatus() + { + $user = new User(); + $user->setEmail('test@example.com'); + $user->setName('Test'); + $user->setIsAdmin(1); + + $user = $this->testedService->updateUser($user, 'Test', 'test@example.com', 'testing', 0); + $this->assertEquals(0, $user->getIsAdmin()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_GrantAdminStatus() + { + $user = new User(); + $user->setEmail('test@example.com'); + $user->setName('Test'); + $user->setIsAdmin(0); + + $user = $this->testedService->updateUser($user, 'Test', 'test@example.com', 'testing', 1); + $this->assertEquals(1, $user->getIsAdmin()); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_ChangesPasswordIfNotEmpty() + { + $user = new User(); + $user->setHash(password_hash('testing', PASSWORD_DEFAULT)); + + $user = $this->testedService->updateUser($user, 'Test', 'test@example.com', 'newpassword', 0); + $this->assertFalse(password_verify('testing', $user->getHash())); + $this->assertTrue(password_verify('newpassword', $user->getHash())); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_DoesNotChangePasswordIfEmpty() + { + $user = new User(); + $user->setHash(password_hash('testing', PASSWORD_DEFAULT)); + + $user = $this->testedService->updateUser($user, 'Test', 'test@example.com', '', 0); + $this->assertTrue(password_verify('testing', $user->getHash())); + } +} From 84e78993d82dedba3e455250812b45ce3e266e8b Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 23 Jul 2014 16:19:45 +0100 Subject: [PATCH 10/37] Reverting accidental addition of engine and parent_id columns to the Build table. --- PHPCI/Model/Base/BuildBase.php | 157 ---------------------------- PHPCI/Store/Base/BuildStoreBase.php | 32 ------ 2 files changed, 189 deletions(-) diff --git a/PHPCI/Model/Base/BuildBase.php b/PHPCI/Model/Base/BuildBase.php index 648bb18c..97b427b5 100644 --- a/PHPCI/Model/Base/BuildBase.php +++ b/PHPCI/Model/Base/BuildBase.php @@ -45,8 +45,6 @@ class BuildBase extends Model 'committer_email' => null, 'commit_message' => null, 'extra' => null, - 'parent_id' => null, - 'engine' => null, ); /** @@ -66,12 +64,9 @@ class BuildBase extends Model 'committer_email' => 'getCommitterEmail', 'commit_message' => 'getCommitMessage', 'extra' => 'getExtra', - 'parent_id' => 'getParentId', - 'engine' => 'getEngine', // Foreign key getters: 'Project' => 'getProject', - 'Parent' => 'getParent', ); /** @@ -91,12 +86,9 @@ class BuildBase extends Model 'committer_email' => 'setCommitterEmail', 'commit_message' => 'setCommitMessage', 'extra' => 'setExtra', - 'parent_id' => 'setParentId', - 'engine' => 'setEngine', // Foreign key setters: 'Project' => 'setProject', - 'Parent' => 'setParent', ); /** @@ -167,18 +159,6 @@ class BuildBase extends Model 'nullable' => true, 'default' => null, ), - 'parent_id' => array( - 'type' => 'int', - 'length' => 11, - 'nullable' => true, - 'default' => null, - ), - 'engine' => array( - 'type' => 'varchar', - 'length' => 50, - 'nullable' => true, - 'default' => null, - ), ); /** @@ -188,7 +168,6 @@ class BuildBase extends Model 'PRIMARY' => array('unique' => true, 'columns' => 'id'), 'project_id' => array('columns' => 'project_id'), 'idx_status' => array('columns' => 'status'), - 'parent_id' => array('columns' => 'parent_id'), ); /** @@ -202,13 +181,6 @@ class BuildBase extends Model 'table' => 'project', 'col' => 'id' ), - 'build_ibfk_2' => array( - 'local_col' => 'parent_id', - 'update' => 'CASCADE', - 'delete' => 'CASCADE', - 'table' => 'build', - 'col' => 'id' - ), ); /** @@ -367,30 +339,6 @@ class BuildBase extends Model return $rtn; } - /** - * Get the value of ParentId / parent_id. - * - * @return int - */ - public function getParentId() - { - $rtn = $this->data['parent_id']; - - return $rtn; - } - - /** - * Get the value of Engine / engine. - * - * @return string - */ - public function getEngine() - { - $rtn = $this->data['engine']; - - return $rtn; - } - /** * Set the value of Id / id. * @@ -615,42 +563,6 @@ class BuildBase extends Model $this->_setModified('extra'); } - /** - * Set the value of ParentId / parent_id. - * - * @param $value int - */ - public function setParentId($value) - { - $this->_validateInt('ParentId', $value); - - if ($this->data['parent_id'] === $value) { - return; - } - - $this->data['parent_id'] = $value; - - $this->_setModified('parent_id'); - } - - /** - * Set the value of Engine / engine. - * - * @param $value string - */ - public function setEngine($value) - { - $this->_validateString('Engine', $value); - - if ($this->data['engine'] === $value) { - return; - } - - $this->data['engine'] = $value; - - $this->_setModified('engine'); - } - /** * Get the Project model for this Build by Id. * @@ -708,75 +620,6 @@ class BuildBase extends Model return $this->setProjectId($value->getId()); } - /** - * Get the Build model for this Build by Id. - * - * @uses \PHPCI\Store\BuildStore::getById() - * @uses \PHPCI\Model\Build - * @return \PHPCI\Model\Build - */ - public function getParent() - { - $key = $this->getParentId(); - - if (empty($key)) { - return null; - } - - $cacheKey = 'Cache.Build.' . $key; - $rtn = $this->cache->get($cacheKey, null); - - if (empty($rtn)) { - $rtn = Factory::getStore('Build', 'PHPCI')->getById($key); - $this->cache->set($cacheKey, $rtn); - } - - return $rtn; - } - - /** - * Set Parent - Accepts an ID, an array representing a Build or a Build model. - * - * @param $value mixed - */ - public function setParent($value) - { - // Is this an instance of Build? - if ($value instanceof \PHPCI\Model\Build) { - return $this->setParentObject($value); - } - - // Is this an array representing a Build item? - if (is_array($value) && !empty($value['id'])) { - return $this->setParentId($value['id']); - } - - // Is this a scalar value representing the ID of this foreign key? - return $this->setParentId($value); - } - - /** - * Set Parent - Accepts a Build model. - * - * @param $value \PHPCI\Model\Build - */ - public function setParentObject(\PHPCI\Model\Build $value) - { - return $this->setParentId($value->getId()); - } - - /** - * Get Build models by ParentId for this Build. - * - * @uses \PHPCI\Store\BuildStore::getByParentId() - * @uses \PHPCI\Model\Build - * @return \PHPCI\Model\Build[] - */ - public function getParentBuilds() - { - return Factory::getStore('Build', 'PHPCI')->getByParentId($this->getId()); - } - /** * Get BuildMeta models by BuildId for this Build. * diff --git a/PHPCI/Store/Base/BuildStoreBase.php b/PHPCI/Store/Base/BuildStoreBase.php index ff21155a..b67d5f73 100644 --- a/PHPCI/Store/Base/BuildStoreBase.php +++ b/PHPCI/Store/Base/BuildStoreBase.php @@ -107,36 +107,4 @@ class BuildStoreBase extends Store return array('items' => array(), 'count' => 0); } } - - public function getByParentId($value, $limit = null, $useConnection = 'read') - { - if (is_null($value)) { - throw new HttpException('Value passed to ' . __FUNCTION__ . ' cannot be null.'); - } - - $add = ''; - - if ($limit) { - $add .= ' LIMIT ' . $limit; - } - - $count = null; - - $query = 'SELECT * FROM `build` WHERE `parent_id` = :parent_id' . $add; - $stmt = Database::getConnection($useConnection)->prepare($query); - $stmt->bindValue(':parent_id', $value); - - if ($stmt->execute()) { - $res = $stmt->fetchAll(\PDO::FETCH_ASSOC); - - $map = function ($item) { - return new Build($item); - }; - $rtn = array_map($map, $res); - - return array('items' => $rtn, 'count' => $count); - } else { - return array('items' => array(), 'count' => 0); - } - } } From fa1fd4038ac74349b11d4d8ac3c962d8cf6788c7 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 25 Jul 2014 20:49:42 +0100 Subject: [PATCH 11/37] Fixing #527 --- PHPCI/Command/InstallCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PHPCI/Command/InstallCommand.php b/PHPCI/Command/InstallCommand.php index e5317261..e4af4722 100644 --- a/PHPCI/Command/InstallCommand.php +++ b/PHPCI/Command/InstallCommand.php @@ -21,7 +21,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Helper\DialogHelper; -use PHPCI\Model\User; +use PHPCI\Service\UserService; /** From 46c03e030ecb9c7e39fd7950f1a8b5a12c01b347 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 15:07:26 +0100 Subject: [PATCH 12/37] Fixing incorrect method call BuildService::delete() and making the project page reload when a build is deleted. Fixes #528 --- PHPCI/Controller/BuildController.php | 2 +- public/assets/js/init.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/PHPCI/Controller/BuildController.php b/PHPCI/Controller/BuildController.php index 600c8508..fb9957cb 100644 --- a/PHPCI/Controller/BuildController.php +++ b/PHPCI/Controller/BuildController.php @@ -151,7 +151,7 @@ class BuildController extends \PHPCI\Controller throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); } - $this->buildService->delete($build); + $this->buildService->deleteBuild($build); header('Location: '.PHPCI_URL.'project/view/' . $build->getProjectId()); exit; diff --git a/public/assets/js/init.js b/public/assets/js/init.js index c851f648..0ef49c4f 100644 --- a/public/assets/js/init.js +++ b/public/assets/js/init.js @@ -20,7 +20,7 @@ function bindAppDeleteEvents () { e.preventDefault(); confirmDelete(e.target.href, 'Build').onClose = function () { - refreshBuildsTable(); + window.location.reload(); }; return false; From ad1a75ec05e684b5b322dfe4877d16b8f8f58bbd Mon Sep 17 00:00:00 2001 From: Daren Chandisingh Date: Tue, 29 Jul 2014 14:18:02 +0100 Subject: [PATCH 13/37] Add build path to phploc directory --- PHPCI/Plugin/PhpLoc.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/PHPCI/Plugin/PhpLoc.php b/PHPCI/Plugin/PhpLoc.php index 26ec8204..8efd0eee 100644 --- a/PHPCI/Plugin/PhpLoc.php +++ b/PHPCI/Plugin/PhpLoc.php @@ -43,7 +43,9 @@ class PhpLoc implements PHPCI\Plugin, PHPCI\ZeroConfigPlugin { $this->phpci = $phpci; $this->build = $build; - $this->directory = isset($options['directory']) ? $options['directory'] : $phpci->buildPath; + $this->directory = isset($options['directory']) ? + $phpci->buildPath . '/' . $options['directory'] : + $phpci->buildPath; } /** From 1f41e50639356eb2d12f73de5cae1835b8612da3 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 15:31:27 +0100 Subject: [PATCH 14/37] Quick fix for the PHPLoc directory setting --- PHPCI/Plugin/PhpLoc.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/PHPCI/Plugin/PhpLoc.php b/PHPCI/Plugin/PhpLoc.php index 8efd0eee..6e402cc7 100644 --- a/PHPCI/Plugin/PhpLoc.php +++ b/PHPCI/Plugin/PhpLoc.php @@ -43,9 +43,11 @@ class PhpLoc implements PHPCI\Plugin, PHPCI\ZeroConfigPlugin { $this->phpci = $phpci; $this->build = $build; - $this->directory = isset($options['directory']) ? - $phpci->buildPath . '/' . $options['directory'] : - $phpci->buildPath; + $this->directory = $phpci->buildPath; + + if (isset($options['directory'])) { + $this->directory .= $options['directory']; + } } /** From 926ef097d413aec04fa973aa5d96272e25604617 Mon Sep 17 00:00:00 2001 From: Tobias van Beek Date: Mon, 21 Jul 2014 11:26:06 +0200 Subject: [PATCH 15/37] Fixes for SSH key generation. Closes #514, fixes #512 --- PHPCI/Helper/SshKey.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/PHPCI/Helper/SshKey.php b/PHPCI/Helper/SshKey.php index 087b0ed3..f36940ca 100644 --- a/PHPCI/Helper/SshKey.php +++ b/PHPCI/Helper/SshKey.php @@ -29,7 +29,7 @@ class SshKey mkdir($tempPath); } - $return = array(); + $return = array('private_key' => '', 'public_key' => ''); if ($this->canGenerateKeys()) { shell_exec('ssh-keygen -q -t rsa -b 2048 -f '.$keyFile.' -N "" -C "deploy@phpci"'); @@ -37,15 +37,13 @@ class SshKey $pub = file_get_contents($keyFile . '.pub'); $prv = file_get_contents($keyFile); - if (empty($pub)) { - $pub = ''; + if (!empty($pub)) { + $return['public_key'] = $pub; } - if (empty($prv)) { - $prv = ''; + if (!empty($prv)) { + $return['private_key'] = $prv; } - - $return = array('private_key' => $prv, 'public_key' => $pub); } return $return; From 6324c40757adc23ecb77d2de0543909a57e93bcc Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 15:51:53 +0100 Subject: [PATCH 16/37] Partial fix for #504 - No error checking in the User helper --- PHPCI/Helper/User.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/PHPCI/Helper/User.php b/PHPCI/Helper/User.php index 726003fd..065deacf 100644 --- a/PHPCI/Helper/User.php +++ b/PHPCI/Helper/User.php @@ -20,6 +20,11 @@ class User public function __call($method, $params = array()) { $user = $_SESSION['user']; + + if (!is_object($user)) { + return null; + } + return call_user_func_array(array($user, $method), $params); } } From 7137e3921f89f9dce9cc13069393c98791f128c5 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 16:18:36 +0100 Subject: [PATCH 17/37] Remove call to non-existant function getPlugins() from the build status page. Fixes #455 --- PHPCI/View/BuildStatus/view.phtml | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/PHPCI/View/BuildStatus/view.phtml b/PHPCI/View/BuildStatus/view.phtml index 2b81cf45..e8536bf6 100644 --- a/PHPCI/View/BuildStatus/view.phtml +++ b/PHPCI/View/BuildStatus/view.phtml @@ -165,21 +165,7 @@ getBranch(); ?> - getPlugins(), true); - - if ( !is_array($plugins) ) { - $plugins = array(); - } - if ( 0 === count($plugins) ) { - ?> - $pluginstatus): - $subcls = $pluginstatus?'label label-success':'label label-danger'; - ?> Build()->formatPluginName($plugin); ?> -
+ From b6e7624bf6cc8aab2c6ed1412de4188aa715b88e Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 16:23:48 +0100 Subject: [PATCH 18/37] Adding the ability to use a project's composer bin directory even if it isn't defined in the composer.json (/vendor/bin) --- PHPCI/Helper/BaseCommandExecutor.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PHPCI/Helper/BaseCommandExecutor.php b/PHPCI/Helper/BaseCommandExecutor.php index 554d9083..d5984a06 100644 --- a/PHPCI/Helper/BaseCommandExecutor.php +++ b/PHPCI/Helper/BaseCommandExecutor.php @@ -170,8 +170,11 @@ abstract class BaseCommandExecutor implements CommandExecutor $composer = $path.'/composer.json'; if (is_file($composer)) { $json = json_decode(file_get_contents($composer)); + if (isset($json->config->{"bin-dir"})) { return $path.'/'.$json->config->{"bin-dir"}; + } elseif (is_dir($path . '/vendor/bin')) { + return $path . '/vendor/bin'; } } } From 9db02ced295af3342fe97668c6ab9af93aaad650 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 17:08:17 +0100 Subject: [PATCH 19/37] Adding testing requirements to composer, so that they are loaded when PHPCI tests PHPCI. --- composer.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0e06e7e1..64f9c36c 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,11 @@ "require-dev": { "phpunit/phpunit": "~4.0", - "phpspec/prophecy-phpunit": "~1.0" + "phpspec/prophecy-phpunit": "~1.0", + "phpmd/phpmd": "~2.0", + "squizlabs/php_codesniffer": "~1.5", + "block8/php-docblock-checker": "~1.0", + "phploc/phploc": "~2.0" }, "suggest": { From 0d189f6f4c59494730f0a95e614682b1174d3fc8 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 17:19:37 +0100 Subject: [PATCH 20/37] Fixing links to files on Github when viewing a pull request build. Fixes #423 --- PHPCI/Model/Build/GithubBuild.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/PHPCI/Model/Build/GithubBuild.php b/PHPCI/Model/Build/GithubBuild.php index 6d3f217b..e0ff0fc0 100644 --- a/PHPCI/Model/Build/GithubBuild.php +++ b/PHPCI/Model/Build/GithubBuild.php @@ -109,8 +109,19 @@ class GithubBuild extends RemoteGitBuild public function getFileLinkTemplate() { - $link = 'https://github.com/' . $this->getProject()->getReference() . '/'; - $link .= 'blob/' . $this->getBranch() . '/'; + $reference = $this->getProject()->getReference(); + $branch = $this->getBranch(); + + if ($this->getExtra('build_type') == 'pull_request') { + $matches = array(); + preg_match('/\/([a-zA-Z0-9_\-]+\/[a-zA-Z0-9_\-]+)/', $this->getExtra('remote_url'), $matches); + + $reference = $matches[1]; + $branch = $this->getExtra('remote_branch'); + } + + $link = 'https://github.com/' . $reference . '/'; + $link .= 'blob/' . $branch . '/'; $link .= '{FILE}'; $link .= '#L{LINE}'; From 26ebad78f721e0298b820681b3eee3c1c1273595 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Tue, 29 Jul 2014 17:24:13 +0100 Subject: [PATCH 21/37] Allow users to turn off SMTP encryption. Fixes #495 --- PHPCI/Controller/SettingsController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PHPCI/Controller/SettingsController.php b/PHPCI/Controller/SettingsController.php index ebcab7e8..b8dcb556 100644 --- a/PHPCI/Controller/SettingsController.php +++ b/PHPCI/Controller/SettingsController.php @@ -75,6 +75,8 @@ class SettingsController extends Controller public function email() { $this->settings['phpci']['email_settings'] = $this->getParams(); + $this->settings['phpci']['email_settings']['smtp_encryption'] = $this->getParam('smtp_encryption', 0); + $error = $this->storeSettings(); if ($error) { From a89f686372ca04ccc702bb7a340132554ec646a2 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 30 Jul 2014 11:58:10 +0100 Subject: [PATCH 22/37] Adding some tests for MailerFactory to ensure it works as expected with a provided configuration. Hopefully will help in debugging #523 --- PHPCI/Helper/MailerFactory.php | 2 +- Tests/PHPCI/Helper/MailerFactoryTest.php | 71 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 Tests/PHPCI/Helper/MailerFactoryTest.php diff --git a/PHPCI/Helper/MailerFactory.php b/PHPCI/Helper/MailerFactory.php index 81d37831..b0ec4860 100644 --- a/PHPCI/Helper/MailerFactory.php +++ b/PHPCI/Helper/MailerFactory.php @@ -40,7 +40,7 @@ class MailerFactory return \Swift_Mailer::newInstance($transport); } - protected function getMailConfig($configName) + public function getMailConfig($configName) { if (isset($this->emailConfig[$configName]) && $this->emailConfig[$configName] != "") { return $this->emailConfig[$configName]; diff --git a/Tests/PHPCI/Helper/MailerFactoryTest.php b/Tests/PHPCI/Helper/MailerFactoryTest.php new file mode 100644 index 00000000..d4140c55 --- /dev/null +++ b/Tests/PHPCI/Helper/MailerFactoryTest.php @@ -0,0 +1,71 @@ + + */ +class MailerFactoryTest extends \PHPUnit_Framework_TestCase +{ + public function setUp() + { + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestGetMailConfig() + { + $config = array( + 'smtp_address' => 'mail.example.com', + 'smtp_port' => 225, + 'smtp_encryption' => true, + 'smtp_username' => 'example.user', + 'smtp_password' => 'examplepassword', + 'default_mailto_address' => 'phpci@example.com', + ); + + $factory = new MailerFactory(array('email_settings' => $config)); + + $this->assertEquals($config['smtp_address'], $factory->getMailConfig('smtp_address')); + $this->assertEquals($config['smtp_port'], $factory->getMailConfig('smtp_port')); + $this->assertEquals($config['smtp_encryption'], $factory->getMailConfig('smtp_encryption')); + $this->assertEquals($config['smtp_username'], $factory->getMailConfig('smtp_username')); + $this->assertEquals($config['smtp_password'], $factory->getMailConfig('smtp_password')); + $this->assertEquals($config['default_mailto_address'], $factory->getMailConfig('default_mailto_address')); + } + + /** + * @covers PHPUnit::execute + */ + public function testExecute_TestMailer() + { + $config = array( + 'smtp_address' => 'mail.example.com', + 'smtp_port' => 225, + 'smtp_encryption' => true, + 'smtp_username' => 'example.user', + 'smtp_password' => 'examplepassword', + 'default_mailto_address' => 'phpci@example.com', + ); + + $factory = new MailerFactory(array('email_settings' => $config)); + $mailer = $factory->getSwiftMailerFromConfig(); + + $this->assertEquals($config['smtp_address'], $mailer->getTransport()->getHost()); + $this->assertEquals($config['smtp_port'], $mailer->getTransport()->getPort()); + $this->assertEquals('tls', $mailer->getTransport()->getEncryption()); + $this->assertEquals($config['smtp_username'], $mailer->getTransport()->getUsername()); + $this->assertEquals($config['smtp_password'], $mailer->getTransport()->getPassword()); + } +} From d798b5f672b017e3d53cd9555e3e4cc637dd803e Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 30 Jul 2014 13:34:45 +0100 Subject: [PATCH 23/37] Updating the CommandExecutor::executeCommand() method to properly catch stderr output from commands. Fixes #456 --- PHPCI/Helper/BaseCommandExecutor.php | 35 ++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/PHPCI/Helper/BaseCommandExecutor.php b/PHPCI/Helper/BaseCommandExecutor.php index d5984a06..1b1d757f 100644 --- a/PHPCI/Helper/BaseCommandExecutor.php +++ b/PHPCI/Helper/BaseCommandExecutor.php @@ -30,6 +30,7 @@ abstract class BaseCommandExecutor implements CommandExecutor protected $verbose; protected $lastOutput; + protected $lastError; public $logExecOutput = true; @@ -78,16 +79,42 @@ abstract class BaseCommandExecutor implements CommandExecutor } $status = 0; - exec($command, $this->lastOutput, $status); + $descriptorSpec = array( + 0 => array("pipe", "r"), // stdin + 1 => array("pipe", "w"), // stdout + 2 => array("pipe", "w"), // stderr + ); - foreach ($this->lastOutput as &$lastOutput) { - $lastOutput = trim($lastOutput, '"'); + $pipes = array(); + + $process = proc_open($command, $descriptorSpec, $pipes, dirname($this->buildPath), null); + + if (is_resource($process)) { + fclose($pipes[0]); + + $this->lastOutput = stream_get_contents($pipes[1]); + $this->lastError = stream_get_contents($pipes[2]); + + fclose($pipes[1]); + fclose($pipes[2]); + + $status = proc_close($process); } - if ($this->logExecOutput && !empty($this->lastOutput) && ($this->verbose|| $status != 0)) { + $this->lastOutput = explode(PHP_EOL, $this->lastOutput); + $this->lastError = "\033[0;31m" . $this->lastError . "\033[0m"; + + $shouldOutput = ($this->logExecOutput && ($this->verbose || $status != 0)); + + if ($shouldOutput && !empty($this->lastOutput)) { $this->logger->log($this->lastOutput); } + if (!empty($this->lastError)) { + $this->logger->log('Error trying to execute: ' . $command); + $this->logger->log($this->lastError, LogLevel::ERROR); + } + $rtn = false; if ($status == 0) { From f1115ba722da437a625ea72275e8da45bdb76649 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 30 Jul 2014 13:57:29 +0100 Subject: [PATCH 24/37] Fixed a small logic bug introduced in the last commit --- PHPCI/Helper/BaseCommandExecutor.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/PHPCI/Helper/BaseCommandExecutor.php b/PHPCI/Helper/BaseCommandExecutor.php index 1b1d757f..8084cfe3 100644 --- a/PHPCI/Helper/BaseCommandExecutor.php +++ b/PHPCI/Helper/BaseCommandExecutor.php @@ -102,7 +102,6 @@ abstract class BaseCommandExecutor implements CommandExecutor } $this->lastOutput = explode(PHP_EOL, $this->lastOutput); - $this->lastError = "\033[0;31m" . $this->lastError . "\033[0m"; $shouldOutput = ($this->logExecOutput && ($this->verbose || $status != 0)); @@ -111,8 +110,8 @@ abstract class BaseCommandExecutor implements CommandExecutor } if (!empty($this->lastError)) { - $this->logger->log('Error trying to execute: ' . $command); - $this->logger->log($this->lastError, LogLevel::ERROR); + $this->logger->log("\033[0;31m" . 'Error trying to execute: ' . $command . "\033[0m", LogLevel::ERROR); + $this->logger->log("\033[0;31m" . $this->lastError . "\033[0m", LogLevel::ERROR); } $rtn = false; From 1bb5c20962fdc1d05d6d0af766323c7a9d82ebc4 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 30 Jul 2014 14:03:55 +0100 Subject: [PATCH 25/37] Fixing failed test. --- PHPCI/Helper/BaseCommandExecutor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PHPCI/Helper/BaseCommandExecutor.php b/PHPCI/Helper/BaseCommandExecutor.php index 8084cfe3..418b664f 100644 --- a/PHPCI/Helper/BaseCommandExecutor.php +++ b/PHPCI/Helper/BaseCommandExecutor.php @@ -101,7 +101,7 @@ abstract class BaseCommandExecutor implements CommandExecutor $status = proc_close($process); } - $this->lastOutput = explode(PHP_EOL, $this->lastOutput); + $this->lastOutput = array_filter(explode(PHP_EOL, $this->lastOutput)); $shouldOutput = ($this->logExecOutput && ($this->verbose || $status != 0)); From 4d4912a09d62966124612492d40345912d0d40f4 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Wed, 30 Jul 2014 14:32:38 +0100 Subject: [PATCH 26/37] Adding handling of HTTP 401 status codes in Ajax requests. Fixes #504 --- PHPCI/View/Home/index.phtml | 22 +++++++++++--- PHPCI/View/Project/view.phtml | 11 ++++++- public/assets/js/phpci.js | 54 ++++++++++++++++++++++++----------- 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/PHPCI/View/Home/index.phtml b/PHPCI/View/Home/index.phtml index a103b413..bcac0775 100644 --- a/PHPCI/View/Home/index.phtml +++ b/PHPCI/View/Home/index.phtml @@ -66,12 +66,26 @@