From 429405809e2337c49e7338d44a12544e23f8d568 Mon Sep 17 00:00:00 2001 From: Dmitry Khomutov Date: Fri, 23 Feb 2018 17:40:56 +0700 Subject: [PATCH] Refactored build type 'Pull request' logic. --- .../Controller/WebhookController.php | 9 +-- .../20140730143702_fix_database_columns.php | 1 - ...4958_unique_email_and_name_user_fields.php | 20 +++---- ...0151009100610_remove_unique_name_index.php | 6 +- .../20151015124825_convert_errors.php | 4 +- ...23052715_added_request_branch_to_build.php | 56 +++++++++++++++++++ src/PHPCensor/Model/Build.php | 13 +++-- src/PHPCensor/Model/Build/BitbucketBuild.php | 35 +++++++----- .../Model/Build/BitbucketHgBuild.php | 19 ++++--- src/PHPCensor/Model/Build/GithubBuild.php | 47 ++++++++-------- src/PHPCensor/Model/Build/RemoteGitBuild.php | 2 +- tests/PHPCensor/Service/BuildServiceTest.php | 11 ++-- 12 files changed, 143 insertions(+), 80 deletions(-) create mode 100644 src/PHPCensor/Migrations/20180223052715_added_request_branch_to_build.php diff --git a/src/PHPCensor/Controller/WebhookController.php b/src/PHPCensor/Controller/WebhookController.php index 03a00744..8221c2a0 100644 --- a/src/PHPCensor/Controller/WebhookController.php +++ b/src/PHPCensor/Controller/WebhookController.php @@ -207,7 +207,6 @@ class WebhookController extends Controller $message = $commit['message']; $extra = [ - 'build_type' => 'pull_request', 'pull_request_number' => $payload['pullrequest']['id'], 'remote_branch' => $payload['pullrequest']['source']['branch']['name'], 'remote_reference' => $payload['pullrequest']['source']['repository']['full_name'], @@ -437,7 +436,7 @@ class WebhookController extends Controller foreach ($commits as $commit) { // Skip all but the current HEAD commit ID: $id = $commit['sha']; - if ($id != $payload['pull_request']['head']['sha']) { + if ($id !== $payload['pull_request']['head']['sha']) { $results[$id] = ['status' => 'ignored', 'message' => 'not branch head']; continue; } @@ -447,14 +446,10 @@ class WebhookController extends Controller $committer = $commit['commit']['author']['email']; $message = $commit['commit']['message']; - $remoteUrlKey = $payload['pull_request']['head']['repo']['private'] ? 'ssh_url' : 'clone_url'; - $extra = [ - 'build_type' => 'pull_request', - 'pull_request_id' => $payload['pull_request']['id'], 'pull_request_number' => $payload['number'], 'remote_branch' => $payload['pull_request']['head']['ref'], - 'remote_url' => $payload['pull_request']['head']['repo'][$remoteUrlKey], + 'remote_reference' => $payload['pull_request']['head']['repo']['full_name'], ]; $results[$id] = $this->createBuild( diff --git a/src/PHPCensor/Migrations/20140730143702_fix_database_columns.php b/src/PHPCensor/Migrations/20140730143702_fix_database_columns.php index 11b3540d..48eaa494 100644 --- a/src/PHPCensor/Migrations/20140730143702_fix_database_columns.php +++ b/src/PHPCensor/Migrations/20140730143702_fix_database_columns.php @@ -50,6 +50,5 @@ class FixDatabaseColumns extends AbstractMigration public function down() { - } } diff --git a/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php b/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php index 5a19ff16..ea9a3291 100644 --- a/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php +++ b/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php @@ -6,27 +6,27 @@ class UniqueEmailAndNameUserFields extends AbstractMigration { public function up() { - $user_table = $this->table('user'); + $table = $this->table('user'); - if (!$user_table->hasIndex('email', ['unique' => true])) { - $user_table->addIndex('email', ['unique' => true])->save(); + if (!$table->hasIndex('email')) { + $table->addIndex('email', ['unique' => true])->save(); } - if (!$user_table->hasIndex('name', ['unique' => true])) { - $user_table->addIndex('name', ['unique' => true])->save(); + if (!$table->hasIndex('name')) { + $table->addIndex('name', ['unique' => true])->save(); } } public function down() { - $user_table = $this->table('user'); + $table = $this->table('user'); - if ($user_table->hasIndex('email', ['unique' => true])) { - $user_table->removeIndex(['email'], ['unique' => true])->save(); + if ($table->hasIndex('email')) { + $table->removeIndex(['email'])->save(); } - if ($user_table->hasIndex('name', ['unique' => true])) { - $user_table->removeIndex(['name'], ['unique' => true])->save(); + if ($table->hasIndex('name')) { + $table->removeIndex(['name'])->save(); } } } diff --git a/src/PHPCensor/Migrations/20151009100610_remove_unique_name_index.php b/src/PHPCensor/Migrations/20151009100610_remove_unique_name_index.php index 72c56a8a..a58cdc20 100644 --- a/src/PHPCensor/Migrations/20151009100610_remove_unique_name_index.php +++ b/src/PHPCensor/Migrations/20151009100610_remove_unique_name_index.php @@ -8,11 +8,11 @@ class RemoveUniqueNameIndex extends AbstractMigration { $user = $this->table('user'); - if ($user->hasIndex('name', ['unique' => true])) { - $user->removeIndex(['name'], ['unique' => true])->save(); + if ($user->hasIndex('name')) { + $user->removeIndex(['name'])->save(); } - if (!$user->hasIndex('name', ['unique' => true])) { + if (!$user->hasIndex('name')) { $user->addIndex(['name'], ['unique' => false])->save(); } } diff --git a/src/PHPCensor/Migrations/20151015124825_convert_errors.php b/src/PHPCensor/Migrations/20151015124825_convert_errors.php index 7cb07683..40efd1a3 100644 --- a/src/PHPCensor/Migrations/20151015124825_convert_errors.php +++ b/src/PHPCensor/Migrations/20151015124825_convert_errors.php @@ -24,8 +24,8 @@ class ConvertErrors extends AbstractMigration $this->metaStore = Factory::getStore('BuildMeta'); $this->errorStore = Factory::getStore('BuildError'); - while ($count === 100) { - $data = $this->metaStore->getErrorsForUpgrade(100); + while ($count >= 100) { + $data = $this->metaStore->getErrorsForUpgrade(100); $count = count($data); /** @var \PHPCensor\Model\BuildMeta $meta */ diff --git a/src/PHPCensor/Migrations/20180223052715_added_request_branch_to_build.php b/src/PHPCensor/Migrations/20180223052715_added_request_branch_to_build.php new file mode 100644 index 00000000..9466b5fe --- /dev/null +++ b/src/PHPCensor/Migrations/20180223052715_added_request_branch_to_build.php @@ -0,0 +1,56 @@ += 100) { + $builds = $buildStore->getBuilds(100, $offset); + $offset =+ 100; + $count = count($builds); + + /** @var Build $build */ + foreach ($builds as &$build) { + $extra = $build->getExtra(); + if (isset($extra['build_type']) && 'pull_request' === $extra['build_type']) { + unset($extra['build_type']); + + $build->setSource(Build::SOURCE_WEBHOOK_PULL_REQUEST); + } + + if (!empty($extra['remote_url'])) { + preg_match( + '/[\/:]([a-zA-Z0-9_\-]+\/[a-zA-Z0-9_\-]+)/', + $extra['remote_url'], + $matches + ); + $remoteReference = $matches[1]; + + if ($remoteReference && empty($extra['remote_reference'])) { + $extra['remote_reference'] = $remoteReference; + } + + unset($extra['remote_url']); + } + + $build->setExtra($extra); + $buildStore->save($build); + } + unset($build); + } + } + + public function down() + { + } +} diff --git a/src/PHPCensor/Model/Build.php b/src/PHPCensor/Model/Build.php index 256be244..71403356 100644 --- a/src/PHPCensor/Model/Build.php +++ b/src/PHPCensor/Model/Build.php @@ -849,13 +849,14 @@ class Build extends Model /** * Allows specific build types (e.g. Github) to report violations back to their respective services. + * * @param Builder $builder - * @param $plugin - * @param $message - * @param int $severity - * @param null $file - * @param null $lineStart - * @param null $lineEnd + * @param string $plugin + * @param string $message + * @param integer $severity + * @param string $file + * @param integer $lineStart + * @param integer $lineEnd */ public function reportError( Builder $builder, diff --git a/src/PHPCensor/Model/Build/BitbucketBuild.php b/src/PHPCensor/Model/Build/BitbucketBuild.php index 0268fd3a..b56ce4ca 100644 --- a/src/PHPCensor/Model/Build/BitbucketBuild.php +++ b/src/PHPCensor/Model/Build/BitbucketBuild.php @@ -19,6 +19,8 @@ class BitbucketBuild extends RemoteGitBuild { /** * Get link to commit from another source (i.e. BitBucket) + * + * @return string */ public function getCommitLink() { @@ -27,6 +29,8 @@ class BitbucketBuild extends RemoteGitBuild /** * Get link to branch from another source (i.e. BitBucket) + * + * @return string */ public function getBranchLink() { @@ -35,6 +39,8 @@ class BitbucketBuild extends RemoteGitBuild /** * Get link to tag from another source (i.e. BitBucket) + * + * @return string */ public function getTagLink() { @@ -97,7 +103,7 @@ class BitbucketBuild extends RemoteGitBuild $url = sprintf( '/2.0/repositories/%s/commit/%s/statuses/build', - $this->getExtra('build_type') == 'pull_request' + Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource() ? $this->getExtra('remote_reference') : $project->getReference(), $this->getCommitId() @@ -128,6 +134,8 @@ class BitbucketBuild extends RemoteGitBuild /** * Get the URL to be used to clone this remote repository. + * + * @return string */ protected function getCloneUrl() { @@ -149,7 +157,7 @@ class BitbucketBuild extends RemoteGitBuild { $reference = $this->getProject()->getReference(); - if ($this->getExtra('build_type') == 'pull_request') { + if (Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource()) { $reference = $this->getExtra('remote_reference'); } @@ -162,21 +170,15 @@ class BitbucketBuild extends RemoteGitBuild } /** - * Handle any post-clone tasks, like applying a pull request patch on top of the branch. - * @param Builder $builder - * @param $cloneTo - * @param array $extra - * @return bool + * @inheritdoc */ protected function postCloneSetup(Builder $builder, $cloneTo, array $extra = null) { - $buildType = $this->getExtra('build_type'); - - $success = true; + $success = true; $skipGitFinalization = false; try { - if (!empty($buildType) && $buildType == 'pull_request') { + if (Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource()) { $helper = new Bitbucket(); $diff = $helper->getPullRequestDiff( $this->getProject()->getReference(), @@ -206,7 +208,8 @@ class BitbucketBuild extends RemoteGitBuild /** * Create an diff file on disk for this build. * - * @param string $cloneTo + * @param string $cloneTo + * @param string $diff * * @return string */ @@ -271,10 +274,12 @@ class BitbucketBuild extends RemoteGitBuild /** * Uses git diff to figure out what the diff line position is, based on the error line number. + * * @param Builder $builder - * @param $file - * @param $line - * @return int|null + * @param string $file + * @param integer $line + * + * @return integer|null */ protected function getDiffLineNumber(Builder $builder, $file, $line) { diff --git a/src/PHPCensor/Model/Build/BitbucketHgBuild.php b/src/PHPCensor/Model/Build/BitbucketHgBuild.php index 37e832d1..d67dd2d8 100644 --- a/src/PHPCensor/Model/Build/BitbucketHgBuild.php +++ b/src/PHPCensor/Model/Build/BitbucketHgBuild.php @@ -2,15 +2,19 @@ namespace PHPCensor\Model\Build; +use PHPCensor\Model\Build; + /** - * BitBucket Build Model - * + * BitbucketHgBuild Build Model + * * @author Artem Bochkov */ class BitbucketHgBuild extends MercurialBuild { /** * Get link to commit from another source (i.e. BitBucket) + * + * @return string */ public function getCommitLink() { @@ -19,6 +23,8 @@ class BitbucketHgBuild extends MercurialBuild /** * Get link to branch from another source (i.e. BitBucket) + * + * @return string */ public function getBranchLink() { @@ -27,6 +33,8 @@ class BitbucketHgBuild extends MercurialBuild /** * Get the URL to be used to clone this remote repository. + * + * @return string */ protected function getCloneUrl() { @@ -48,11 +56,8 @@ class BitbucketHgBuild extends MercurialBuild { $reference = $this->getProject()->getReference(); - if ($this->getExtra('build_type') == 'pull_request') { - $matches = []; - preg_match('/[\/:]([a-zA-Z0-9_\-]+\/[a-zA-Z0-9_\-]+)/', $this->getExtra('remote_url'), $matches); - - $reference = $matches[1]; + if (Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource()) { + $reference = $this->getExtra('remote_reference'); } $link = 'https://bitbucket.org/' . $reference . '/'; diff --git a/src/PHPCensor/Model/Build/GithubBuild.php b/src/PHPCensor/Model/Build/GithubBuild.php index cb6b4053..911ade0c 100644 --- a/src/PHPCensor/Model/Build/GithubBuild.php +++ b/src/PHPCensor/Model/Build/GithubBuild.php @@ -18,16 +18,20 @@ use PHPCensor\Model\BuildError; class GithubBuild extends RemoteGitBuild { /** - * Get link to commit from another source (i.e. Github) - */ + * Get link to commit from another source (i.e. Github) + * + * @return string + */ public function getCommitLink() { return 'https://github.com/' . $this->getProject()->getReference() . '/commit/' . $this->getCommitId(); } /** - * Get link to branch from another source (i.e. Github) - */ + * Get link to branch from another source (i.e. Github) + * + * @return string + */ public function getBranchLink() { return 'https://github.com/' . $this->getProject()->getReference() . '/tree/' . $this->getBranch(); @@ -35,6 +39,8 @@ class GithubBuild extends RemoteGitBuild /** * Get link to tag from another source (i.e. Github) + * + * @return string */ public function getTagLink() { @@ -117,8 +123,10 @@ class GithubBuild extends RemoteGitBuild } /** - * Get the URL to be used to clone this remote repository. - */ + * Get the URL to be used to clone this remote repository. + * + * @return string + */ protected function getCloneUrl() { $key = trim($this->getProject()->getSshPrivateKey()); @@ -160,11 +168,8 @@ class GithubBuild extends RemoteGitBuild { $reference = $this->getProject()->getReference(); - if ($this->getExtra('build_type') == 'pull_request') { - $matches = []; - preg_match('/[\/:]([a-zA-Z0-9_\-]+\/[a-zA-Z0-9_\-]+)/', $this->getExtra('remote_url'), $matches); - - $reference = $matches[1]; + if (Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource()) { + $reference = $this->getExtra('remote_reference'); } $link = 'https://github.com/' . $reference . '/'; @@ -176,20 +181,14 @@ class GithubBuild extends RemoteGitBuild } /** - * Handle any post-clone tasks, like applying a pull request patch on top of the branch. - * @param Builder $builder - * @param $cloneTo - * @param array $extra - * @return bool + * @inheritdoc */ protected function postCloneSetup(Builder $builder, $cloneTo, array $extra = null) { - $buildType = $this->getExtra('build_type'); - $success = true; try { - if (!empty($buildType) && $buildType == 'pull_request') { + if (Build::SOURCE_WEBHOOK_PULL_REQUEST === $this->getSource()) { $pullRequestId = $this->getExtra('pull_request_number'); $cmd = 'cd "%s" && git checkout -b php-censor/' . $this->getId() @@ -211,7 +210,7 @@ class GithubBuild extends RemoteGitBuild } /** - * @inheritDoc + * @inheritdoc */ public function reportError( Builder $builder, @@ -259,10 +258,12 @@ class GithubBuild extends RemoteGitBuild /** * Uses git diff to figure out what the diff line position is, based on the error line number. + * * @param Builder $builder - * @param $file - * @param $line - * @return int|null + * @param string $file + * @param integer $line + * + * @return integer|null */ protected function getDiffLineNumber(Builder $builder, $file, $line) { diff --git a/src/PHPCensor/Model/Build/RemoteGitBuild.php b/src/PHPCensor/Model/Build/RemoteGitBuild.php index d1938ec9..49753393 100644 --- a/src/PHPCensor/Model/Build/RemoteGitBuild.php +++ b/src/PHPCensor/Model/Build/RemoteGitBuild.php @@ -135,7 +135,7 @@ class RemoteGitBuild extends Build * @param string $cloneTo * @param array $extra * - * @return bool + * @return boolean */ protected function postCloneSetup(Builder $builder, $cloneTo, array $extra = null) { diff --git a/tests/PHPCensor/Service/BuildServiceTest.php b/tests/PHPCensor/Service/BuildServiceTest.php index ef158480..ba0dd00d 100644 --- a/tests/PHPCensor/Service/BuildServiceTest.php +++ b/tests/PHPCensor/Service/BuildServiceTest.php @@ -3,7 +3,6 @@ namespace Tests\PHPCensor\Service; use PHPCensor\Model\Build; -use PHPCensor\Model\Project; use PHPCensor\Service\BuildService; /** @@ -32,12 +31,14 @@ class BuildServiceTest extends \PHPUnit\Framework\TestCase public function setUp() { $this->mockBuildStore = $this->getMockBuilder('PHPCensor\Store\BuildStore')->getMock(); - $this->mockBuildStore->expects($this->any()) - ->method('save') - ->will($this->returnArgument(0)); + $this->mockBuildStore + ->expects($this->any()) + ->method('save') + ->will($this->returnArgument(0)); $this->mockEnvironmentStore = $this->getMockBuilder('PHPCensor\Store\EnvironmentStore')->getMock(); - $this->mockEnvironmentStore->expects($this->any()) + $this->mockEnvironmentStore + ->expects($this->any()) ->method('getByProjectId') ->will($this->returnValue(['items' => [], 'count' => 0]));