Refactor Github Repository Url Parsing, adjust PHPCS code to v1.3

beberlei · Sep 23, 2011 · 3a7f315 · stof · Sep 23, 2011 · 3a7f315
1 parent 41344b2
commit 3a7f315
Show file tree

Hide file tree

Showing 16 changed files with 153 additions and 75 deletions.
diff --git a/Command/GithubCommentCommand.php b/Command/GithubCommentCommand.php
@@ -29,11 +29,12 @@ protected function configure()
             ->setName('review-squawk:github:comment')
             ->setDescription('Create a comment on a commit')
             ->addArgument('access_token', InputArgument::REQUIRED, 'Access Token')
-            ->addArgument('username', InputArgument::REQUIRED, 'User/Organization')
+            ->addArgument('url', InputArgument::REQUIRED, 'Github Url')
             ->addArgument('repo', InputArgument::REQUIRED, 'Repository')
             ->addArgument('sha1', InputArgument::REQUIRED, 'Commit Sha1')
             ->addArgument('path', InputArgument::REQUIRED, 'Path')
             ->addArgument('line', InputArgument::REQUIRED, 'Line')
+            ->addArgument('position', InputArgument::REQUIRED, 'Position in Diff')
             ->addArgument('message', InputArgument::REQUIRED, 'Message')
             ->setHelp(<<<EOT
 The <info>review-squawk:github:comment</info> command creates a commit comment.
@@ -49,12 +50,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $api->commentCommit(
             $input->getArgument('access_token'),
-            $input->getArgument('username'),
-            $input->getArgument('repo'),
+            $input->getArgument('url'),
             $input->getArgument('sha1'),
             $input->getArgument('path'),
             $input->getArgument('line'),
-            1,
+            $input->getArgument('position'),
             $input->getArgument('message')
         );
     }

diff --git a/Command/GithubDiffCommand.php b/Command/GithubDiffCommand.php
@@ -28,8 +28,7 @@ protected function configure()
         $this
             ->setName('review-squawk:github:diff')
             ->setDescription('Create and vizualize the diffs of a Github commit')
-            ->addArgument('username', InputArgument::REQUIRED, 'User/Organization')
-            ->addArgument('repo', InputArgument::REQUIRED, 'Repository')
+            ->addArgument('url', InputArgument::REQUIRED, 'Github Url')
             ->addArgument('sha1', InputArgument::REQUIRED, 'Commit Sha1')
             ->setHelp(<<<EOT
 The <info>review-squawk:github:diff</info> command creates and vizualizes the diff
@@ -44,7 +43,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
     {
         $api = $this->getContainer()->get('whitewashing.review_squawk.github_client');
 
-        $diffs = $api->getCommitDiffs($input->getArgument('username'), $input->getArgument('repo'), $input->getArgument('sha1'));
+        $diffs = $api->getCommitDiffs($input->getArgument('url'), $input->getArgument('sha1'));
 
         foreach ($diffs AS $diff) {
             $project = new \Whitewashing\ReviewSquawkBundle\Model\Project("http", "token", "Zend", true);

diff --git a/Controller/GithubProjectController.php b/Controller/GithubProjectController.php
@@ -54,18 +54,14 @@ public function createAction()
     public function showAction($id)
     {
         $em = $this->container->get('doctrine.orm.default_entity_manager');
-        $project = $em->find('Whitewashing\ReviewSquawkBundle\Entity\PRoject', $id);
+        $project = $em->find('Whitewashing\ReviewSquawkBundle\Entity\Project', $id);
 
         if (!$project) {
             throw $this->createNotFoundException('No project found with this id.');
         }
 
-        $parts = explode("/", $project->getRepositoryurl());
-        $repository = array_pop($parts);
-        $userOrg = array_pop($parts);
-
         $api = $this->container->get('whitewashing.review_squawk.github_client');
-        $commits = $api->getCommits($userOrg, $repository);
+        $commits = $api->getCommits($project->getRepositoryUrl());
 
         return array('project' => $project, 'commits' => $commits);
     }

diff --git a/Controller/ReviewController.php b/Controller/ReviewController.php
@@ -24,25 +24,6 @@
 
 class ReviewController extends Controller
 {
-    /**
-     * @param int $projectId
-     * @return \Whitewashing\ReviewSquawkBundle\Entity\Project
-     */
-    private function getProject($projectId)
-    {
-        $project = $this->container->get('doctrine.orm.default_entity_manager')
-                    ->find('Whitewashing\ReviewSquawkBundle\Entity\Project', $projectId);
-        if (!$project) {
-            throw $this->createNotFoundException("No project found!");
-        }
-        $user = $this->container->get('security.context')->getToken()->getUser();
-        if ($user !== $project->getUser()) {
-            throw new AccessDeniedHttpException("Not your own project");
-        }
-
-        return $project;
-    }
-
     /**
      * Creates a new commit event for the project.
      *
@@ -96,6 +77,11 @@ public function postCommitsAction($projectId)
     public function newCommitAction($projectId, $commitId)
     {
         $project = $this->getProject($projectId);
+        $currentUser = $this->container->get('security.context')->getToken()->getUser();
+
+        if ($project->getUser() != $currentUser) {
+            throw new AccessDeniedHttpException("Invalid user to review this commit.");
+        }
 
         if ($response = $this->commitExists($project->getId(), $commitId)) {
             return $response;
@@ -104,6 +90,47 @@ public function newCommitAction($projectId, $commitId)
         return array('project' => $project, 'commitId' => $commitId);
     }
 
+    /**
+     * @Route("/project/{projectId}/review/commits/{commitId}.html", name="rs_review_commit_view")
+     * @Template()
+     * @param $projectId
+     * @param $commitId
+     * @return void
+     */
+    public function getCommitAction($projectId, $commitId)
+    {
+        $project = $this->getProject($projectId);
+        $currentUser = $this->container->get('security.context')->getToken()->getUser();
+
+        if ($project->getUser() != $currentUser) {
+            throw new AccessDeniedHttpException("Invalid user to review this commit.");
+        }
+
+        $client = $this->container->get('whitewashing.review_squawk.github_client');
+        $diffs = $client->getCommitDiffs($project->getRepositoryUrl(), $commitId);
+
+        return array('project' => $project, 'diffs' => $diffs, 'commitId' => $commitId);
+    }
+
+    /**
+     * @param int $projectId
+     * @return \Whitewashing\ReviewSquawkBundle\Entity\Project
+     */
+    private function getProject($projectId)
+    {
+        $project = $this->container->get('doctrine.orm.default_entity_manager')
+                    ->find('Whitewashing\ReviewSquawkBundle\Entity\Project', $projectId);
+        if (!$project) {
+            throw $this->createNotFoundException("No project found!");
+        }
+        $user = $this->container->get('security.context')->getToken()->getUser();
+        if ($user !== $project->getUser()) {
+            throw new AccessDeniedHttpException("Not your own project");
+        }
+
+        return $project;
+    }
+
     private function commitExists($projectId, $commitId)
     {
         $em = $this->container->get('doctrine.orm.default_entity_manager');

diff --git a/Entity/Project.php b/Entity/Project.php
@@ -89,11 +89,11 @@ public function getRepositoryUrl()
     }
 
     /**
-     *
      * @param string $repositoryUrl
      */
     public function setRepositoryUrl($repositoryUrl)
     {
+        // TODO: Remove github specifices when necessary
         $repositoryUrl = rtrim($repositoryUrl, "/");
         if (strpos($repositoryUrl, "://github.com") !== false) {
             if (substr($repositoryUrl, -4) == ".git") {

diff --git a/Model/CodeSnifferService.php b/Model/CodeSnifferService.php
@@ -37,16 +37,25 @@ public function scan(Project $project, Diff $diff)
             return array();
         }
 
-        $oldViolations = $oldReport['files'][$diff->getPath()]['messages'];
-        $newViolations = $newReport['files'][$diff->getPath()]['messages'];
+        $foundViolations = $this->computeNewViolations($diff->getPath(), $oldReport['errors'], $newReport['errors']);
 
+        if ($this->showWarnings) {
+            foreach ($this->computeNewViolations($diff->getPath(), $oldReport['warnings'], $newReport['warnings']) AS $violation) {
+                $foundViolations[] = $violation;
+            }
+        }
+        return $foundViolations;
+    }
+
+    private function computeNewViolations($path, $oldViolations, $newViolations)
+    {
         $foundViolations = array();
         foreach ($newViolations AS $line => $violations) {
             foreach ($violations AS $column => $messages) {
                 if (!isset($oldViolations[$line][$column])) {
                     $oldViolations[$line][$column] = array();
                 }
-                
+
                 // both old and new code have violations at that line and column, are they the same?
                 foreach ($messages AS $idx => $message) {
                     foreach ($oldViolations[$line][$column] AS $oldMessage) {
@@ -55,7 +64,7 @@ public function scan(Project $project, Diff $diff)
                         }
                     }
 
-                    $foundViolations[] = new Violation($diff->getPath(), $line, $message['source'].": " . $message['message']);
+                    $foundViolations[] = new Violation($path, $line, $message['source'].": " . $message['message']);
                 }
             }
         }
@@ -68,14 +77,18 @@ private function getCodeReport($file, $code)
         $cli = new \PHP_CodeSniffer_CLI();
         $sniffer = new \PHP_CodeSniffer();
         $sniffer->setTokenListeners($this->standard, array());
-
+        $sniffer->populateCustomRules($this->standard);
         $sniffer->populateTokenListeners();
 
         if (strlen($code)) {
-            $sniffer->processFile($file, $code);
-            return $sniffer->prepareErrorReport($this->showWarnings);
+            $phpcsFile = $sniffer->processFile($file, $code);
+            return array(
+                'totals' => array('warnings' => $phpcsFile->getWarningCount(), 'errors' => $phpcsFile->getErrorCount()),
+                'errors' => $phpcsFile->getErrors(),
+                'warnings' => $phpcsFile->getWarnings(),
+            );
         } else {
-            return array('totals' => array('warnings' => 0, 'errors' => 0), 'files' => array($file => array('messages' => array())));
+            return array('totals' => array('warnings' => 0, 'errors' => 0), 'errors' => array(), 'warnings' => array());
         }
     }
 

diff --git a/Model/Diff.php b/Model/Diff.php
@@ -36,14 +36,14 @@ class Diff
      * @param $path
      * @param $oldCode
      * @param $newCode
-     * @param $diff
+     * @param $patch
      */
-    public function __construct($path, $oldCode, $newCode, $diff = "")
+    public function __construct($path, $oldCode, $newCode, $patch = "")
     {
         $this->path = $path;
         $this->oldCode = $oldCode;
         $this->newCode = $newCode;
-        $this->diff = $diff;
+        $this->patch = $patch;
     }
 
     public function getPath()
@@ -61,18 +61,22 @@ public function getNewCode()
         return $this->newCode;
     }
 
-    public function getDiffPositionForLine($linePos)
+    public function getDiff()
     {
-        if (strlen($this->diff) == 0) {
+        return $this->patch;
+    }
+
+    public function getPatchPositionForLine($linePos)
+    {
+        if (strlen($this->patch) == 0) {
             return $linePos;
         } else {
-            $lines = explode("\n", str_replace("\r\n", "\n", $this->diff));
+            $lines = explode("\n", str_replace("\r\n", "\n", $this->patch));
 
             $pos = 0;
             $currentLine = 0;
             for ($i = 2; $i < count($lines); $i++) {
                 if (preg_match('(@@ \-([0-9]+),([0-9]+) \+([0-9]+),([0-9]+))', $lines[$i], $match)) {
-                    var_dump($match);
                     $currentLine = $match[1];
                 } else {
                     if ($linePos == $currentLine) {

diff --git a/Model/Github/ClientAPI.php b/Model/Github/ClientAPI.php
@@ -18,17 +18,17 @@ interface ClientAPI
 {
     public function claimOAuthAccessToken($temporaryCode);
 
-    public function getCommitDiffs($username, $repository, $sha1);
+    public function getCommitDiffs($repositoryUrl, $sha1);
 
     /** http://developer.github.com/v3/repos/commits/ */
-    public function commentCommit($accessToken, $username, $repository, $sha1, $path, $line, $position, $message);
+    public function commentCommit($accessToken, $repositoryUrl, $sha1, $path, $line, $position, $message);
 
     /** http://developer.github.com/v3/pulls/comments/ */
-    public function commentPullRequest($accessToken, $username, $repository, $prId, $sha1, $path, $line, $message);
+    public function commentPullRequest($accessToken, $repositoryUrl, $prId, $sha1, $path, $line, $message);
 
     public function getCurrentUser($accessToken);
 
-    public function getProject($username, $repository);
+    public function getProject($repositoryUrl);
 
-    public function getCommits($username, $repository);
+    public function getCommits($repositoryUrl);
 }
diff --git a/Model/Github/RestV3API.php b/Model/Github/RestV3API.php
@@ -28,8 +28,18 @@ public function __construct($clientId, $clientSecret)
         $this->clientSecret = $clientSecret;
     }
 
-    public function commentCommit($accessToken, $username, $repository, $sha1, $path, $line, $position, $message)
+    private function getRepositoryParts($url)
     {
+        $parts = explode("/", $url);
+        $repository = array_pop($parts);
+        $userOrg = array_pop($parts);
+        return array($userOrg, $repository);
+    }
+
+    public function commentCommit($accessToken, $repositoryUrl, $sha1, $path, $line, $position, $message)
+    {
+        list($username, $repository) = $this->getRepositoryParts($repositoryUrl);
+
         $params = array(
             'line' => $line,
             'path' => $path,
@@ -42,7 +52,7 @@ public function commentCommit($accessToken, $username, $repository, $sha1, $path
         $this->curl->request('POST', $commentsUrl, $params);
     }
 
-    public function commentPullRequest($accessToken, $username, $repository, $prId, $sha1, $path, $line, $message)
+    public function commentPullRequest($accessToken, $repositoryUrl, $prId, $sha1, $path, $line, $message)
     {
 
     }
@@ -54,8 +64,10 @@ public function commentPullRequest($accessToken, $username, $repository, $prId,
      * @param string $sha1
      * @return Diff
      */
-    public function getCommitDiffs($username, $repository, $sha1)
+    public function getCommitDiffs($repositoryUrl, $sha1)
     {
+        list($username, $repository) = $this->getRepositoryParts($repositoryUrl);
+
         $commitUrl = "https://api.github.com/repos/" . $username . "/" . $repository . "/git/commits/" . $sha1;
         $commitResponse = $this->curl->request('GET', $commitUrl);
 
@@ -81,7 +93,7 @@ public function getCommitDiffs($username, $repository, $sha1)
             $parentTreeUrl = $commitResponse['body']['tree']['url'];
             $parentTreeResponse = $this->curl->request('GET', $parentTreeUrl . "?recursive=true");
 
-            $compareUrl = "https://api.github.com/repos/" . $username . "/" . $repository . "/compare/" . $sha1 . "..." . $parentSha;
+            $compareUrl = "https://api.github.com/repos/" . $username . "/" . $repository . "/compare/" . $parentSha . "..." .$sha1;
             $compareResponse = $this->curl->request('GET', $compareUrl);
 
             foreach ($parentTreeResponse['body']['tree'] AS $parentTreeFile) {
@@ -137,14 +149,18 @@ public function getCurrentUser($accessToken)
         return $response['body'];
     }
 
-    public function getProject($username, $repository)
+    public function getProject($repositoryUrl)
     {
+        list($username, $repository) = $this->getRepositoryParts($repositoryUrl);
+
         $response = $this->curl->request('GET', 'https://api.github.com/repos/'. $username . '/' . $repository);
         return $response['body'];
     }
 
-    public function getCommits($username, $repository)
+    public function getCommits($repositoryUrl)
     {
+        list($username, $repository) = $this->getRepositoryParts($repositoryUrl);
+
         $response = $this->curl->request('GET', 'https://api.github.com/repos/'. $username . '/' . $repository . '/commits');
         return $response['body'];
     }