From 21cdf60c31553261fc1e72b79f8631ee03bbf650 Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Wed, 1 Sep 2010 13:13:52 +0000 Subject: [PATCH] Introduce a more subtle concept of validity when it comes to revision indentifiers in IDF - the SCM function isValidRevision has been replaced by a validateRevision() method which returns one of three states, valid, invalid or ambiguous. The source view can then act accordingly and display disambiguate view for the latter, so the user can select for which revision he actually wants to execute the requested action. Also, invalid revisions now lead to another separate view, telling the user that it is invalid / does not exist and pointing him optionally to the help page where he can read further how to access his repository to push the first changes into. (partially resolves issue 525) --- src/IDF/Scm.php | 29 ++- src/IDF/Scm/Git.php | 6 +- src/IDF/Scm/Mercurial.php | 9 +- src/IDF/Scm/Monotone.php | 49 ++++- src/IDF/Scm/Svn.php | 9 +- src/IDF/Views/Source.php | 208 +++++++++++------- src/IDF/conf/urls.php | 10 + src/IDF/templates/idf/source/base.html | 2 +- .../idf/source/disambiguate_revision.html | 33 +++ .../idf/source/invalid_revision.html | 17 ++ 10 files changed, 271 insertions(+), 101 deletions(-) create mode 100644 src/IDF/templates/idf/source/disambiguate_revision.html create mode 100644 src/IDF/templates/idf/source/invalid_revision.html diff --git a/src/IDF/Scm.php b/src/IDF/Scm.php index 94cf5ec..1496974 100644 --- a/src/IDF/Scm.php +++ b/src/IDF/Scm.php @@ -63,7 +63,7 @@ class IDF_Scm public $project = null; /** - * Cache storage. + * Cache storage. * * It must only be used to store data for the lifetime of the * object. For example if you need to get the list of branches in @@ -166,13 +166,28 @@ class IDF_Scm throw new Pluf_Exception_NotImplemented(); } + const REVISION_VALID = 0; + const REVISION_INVALID = 1; + const REVISION_AMBIGUOUS = 2; + /** - * Check if a revision or commit is valid. + * Check if a revision or commit is valid, invalid or ambiguous. * * @param string Revision or commit - * @return bool + * @return int One of REVISION_VALID, REVISION_INVALID or REVISION_AMBIGIOUS */ - public function isValidRevision($rev) + public function validateRevision($rev) + { + throw new Pluf_Exception_NotImplemented(); + } + + /** + * Returns an array of single commit objects for ambiguous commit identifiers + * + * @param string Ambiguous commit identifier + * @return array of objects + */ + public function disambiguateRevision($commit) { throw new Pluf_Exception_NotImplemented(); } @@ -217,7 +232,7 @@ class IDF_Scm * 'foo-branch' => 'branches/foo-branch',) * * - * @return array Branches + * @return array Branches */ public function getBranches() { @@ -282,7 +297,7 @@ class IDF_Scm * @param string Revision or commit * @param string Folder ('/') * @param string Branch (null) - * @return array + * @return array */ public function getTree($rev, $folder='/', $branch=null) { @@ -396,7 +411,7 @@ class IDF_Scm public static function syncTimeline($project, $force=false) { $cache = Pluf_Cache::factory(); - $key = 'IDF_Scm:'.$project->shortname.':lastsync'; + $key = 'IDF_Scm:'.$project->shortname.':lastsync'; if ($force or null === ($res=$cache->get($key))) { $scm = IDF_Scm::get($project); if ($scm->isAvailable()) { diff --git a/src/IDF/Scm/Git.php b/src/IDF/Scm/Git.php index 77bea39..4a3b308 100644 --- a/src/IDF/Scm/Git.php +++ b/src/IDF/Scm/Git.php @@ -296,10 +296,12 @@ class IDF_Scm_Git extends IDF_Scm } - public function isValidRevision($commit) + public function validateRevision($commit) { $type = $this->testHash($commit); - return ('commit' == $type || 'tag' == $type); + if ('commit' == $type || 'tag' == $type) + return IDF_Scm::REVISION_VALID; + return IDF_Scm::REVISION_INVALID; } /** diff --git a/src/IDF/Scm/Mercurial.php b/src/IDF/Scm/Mercurial.php index 9f34ab7..ac66464 100644 --- a/src/IDF/Scm/Mercurial.php +++ b/src/IDF/Scm/Mercurial.php @@ -87,14 +87,19 @@ class IDF_Scm_Mercurial extends IDF_Scm return sprintf(Pluf::f('mercurial_remote_url'), $project->shortname); } - public function isValidRevision($rev) + public function validateRevision($rev) { $cmd = sprintf(Pluf::f('hg_path', 'hg').' log -R %s -r %s', escapeshellarg($this->repo), escapeshellarg($rev)); $cmd = Pluf::f('idf_exec_cmd_prefix', '').$cmd; self::exec('IDF_Scm_Mercurial::isValidRevision', $cmd, $out, $ret); - return ($ret == 0) && (count($out) > 0); + + // FIXME: apparently a given hg revision can also be ambigious - + // handle this case here sometime + if ($ret == 0 && count($out) > 0) + return IDF_Scm::REVISION_VALID; + return IDF_Scm::REVISION_INVALID; } /** diff --git a/src/IDF/Scm/Monotone.php b/src/IDF/Scm/Monotone.php index f8c898f..eb44292 100644 --- a/src/IDF/Scm/Monotone.php +++ b/src/IDF/Scm/Monotone.php @@ -425,12 +425,53 @@ class IDF_Scm_Monotone extends IDF_Scm } /** - * @see IDF_Scm::isValidRevision() + * @see IDF_Scm::validateRevision() */ - public function isValidRevision($commit) + public function validateRevision($commit) { $revs = $this->_resolveSelector($commit); - return count($revs) == 1; + if (count($revs) == 0) + return IDF_Scm::REVISION_INVALID; + + if (count($revs) > 1) + return IDF_Scm::REVISION_AMBIGUOUS; + + return IDF_Scm::REVISION_VALID; + } + + /** + * @see IDF_Scm::disambiguateRevision + */ + public function disambiguateRevision($commit) + { + $revs = $this->_resolveSelector($commit); + + $out = array(); + foreach ($revs as $rev) + { + $certs = $this->_getCerts($rev); + + $log = array(); + $log['author'] = implode(', ', $certs['author']); + + $log['branch'] = implode(', ', $certs['branch']); + + $dates = array(); + foreach ($certs['date'] as $date) + $dates[] = date('Y-m-d H:i:s', strtotime($date)); + $log['date'] = implode(', ', $dates); + + $combinedChangelog = implode("\n---\n", $certs['changelog']); + $split = preg_split("/[\n\r]/", $combinedChangelog, 2); + $log['title'] = $split[0]; + $log['full_message'] = (isset($split[1])) ? trim($split[1]) : ''; + + $log['commit'] = $rev; + + $out[] = (object)$log; + } + + return $out; } /** @@ -630,7 +671,7 @@ class IDF_Scm_Monotone extends IDF_Scm --$n; $log = array(); - $log['author'] = implode(", ", $certs['author']); + $log['author'] = implode(', ', $certs['author']); $dates = array(); foreach ($certs['date'] as $date) diff --git a/src/IDF/Scm/Svn.php b/src/IDF/Scm/Svn.php index 6527bbf..fdbc340 100644 --- a/src/IDF/Scm/Svn.php +++ b/src/IDF/Scm/Svn.php @@ -138,7 +138,7 @@ class IDF_Scm_Svn extends IDF_Scm /** * Subversion revisions are either a number or 'HEAD'. */ - public function isValidRevision($rev) + public function validateRevision($rev) { if ($rev == 'HEAD') { return true; @@ -149,8 +149,11 @@ class IDF_Scm_Svn extends IDF_Scm escapeshellarg($this->repo), escapeshellarg($rev)); $cmd = Pluf::f('idf_exec_cmd_prefix', '').$cmd; - self::exec('IDF_Scm_Svn::isValidRevision', $cmd, $out, $ret); - return (0 == $ret); + self::exec('IDF_Scm_Svn::validateRevision', $cmd, $out, $ret); + + if ($ret == 0) + return IDF_Scm::REVISION_VALID; + return IDF_Scm::REVISION_INVALID; } diff --git a/src/IDF/Views/Source.php b/src/IDF/Views/Source.php index 18d79a2..dcebdab 100644 --- a/src/IDF/Views/Source.php +++ b/src/IDF/Views/Source.php @@ -59,30 +59,56 @@ class IDF_Views_Source $params, $request); } - public $changeLog_precond = array('IDF_Precondition::accessSource'); + /** + * Is displayed in case an invalid revision is requested + */ + public $invalidRevision_precond = array('IDF_Precondition::accessSource'); + public function invalidRevision($request, $match) + { + $title = sprintf(__('%s Invalid Revision'), (string) $request->project); + $commit = $match[2]; + $params = array( + 'page_title' => $title, + 'title' => $title, + 'commit' => $commit, + ); + return Pluf_Shortcuts_RenderToResponse('idf/source/invalid_revision.html', + $params, $request); + } + + /** + * Is displayed in case a revision identifier cannot be uniquely resolved + * to one single revision + */ + public $disambiguateRevision_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable'); + public function disambiguateRevision($request, $match) + { + $title = sprintf(__('%s Ambiguous Revision'), (string) $request->project); + $commit = $match[2]; + $redirect = $match[3]; + $scm = IDF_Scm::get($request->project); + $revisions = $scm->disambiguateRevision($commit); + $params = array( + 'page_title' => $title, + 'title' => $title, + 'commit' => $commit, + 'revisions' => $revisions, + 'redirect' => $redirect, + ); + return Pluf_Shortcuts_RenderToResponse('idf/source/disambiguate_revision.html', + $params, $request); + } + + public $changeLog_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function changeLog($request, $match) { $scm = IDF_Scm::get($request->project); - if (!$scm->isAvailable()) { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::help', - array($request->project->shortname)); - return new Pluf_HTTP_Response_Redirect($url); - } $branches = $scm->getBranches(); $commit = $match[2]; - if (!$scm->isValidRevision($commit)) { - if (count($branches) == 0) { - // Redirect to the project source help - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::help', - array($request->project->shortname)); - return new Pluf_HTTP_Response_Redirect($url); - } - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::changeLog', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); - } + $title = sprintf(__('%1$s %2$s Change Log'), (string) $request->project, $this->getScmType($request)); $changes = $scm->getChangeLog($commit, 25); @@ -111,22 +137,17 @@ class IDF_Views_Source $request); } - public $treeBase_precond = array('IDF_Precondition::accessSource'); + public $treeBase_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function treeBase($request, $match) { $scm = IDF_Scm::get($request->project); - if (!$scm->isAvailable()) { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::help', - array($request->project->shortname)); - return new Pluf_HTTP_Response_Redirect($url); - } $commit = $match[2]; + $cobject = $scm->getCommit($commit); if (!$cobject) { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); + throw new Exception('could not retrieve commit object for '. $commit); } $title = sprintf(__('%1$s %2$s Source Tree'), $request->project, $this->getScmType($request)); @@ -159,20 +180,14 @@ class IDF_Views_Source $request); } - public $tree_precond = array('IDF_Precondition::accessSource'); + public $tree_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function tree($request, $match) { $scm = IDF_Scm::get($request->project); $commit = $match[2]; - if (!$scm->isAvailable()) { - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::help', - array($request->project->shortname)); - return new Pluf_HTTP_Response_Redirect($url); - } - $fburl = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); $request_file = $match[3]; if (substr($request_file, -1) == '/') { $request_file = substr($request_file, 0, -1); @@ -181,13 +196,13 @@ class IDF_Views_Source $request_file)); return new Pluf_HTTP_Response_Redirect($url, 301); } - if (!$scm->isValidRevision($commit)) { - // Redirect to the first branch - return new Pluf_HTTP_Response_Redirect($fburl); - } + $request_file_info = $scm->getPathInfo($request_file, $commit); if (!$request_file_info) { - // Redirect to the first branch + // Redirect to the main branch + $fburl = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', + array($request->project->shortname, + $scm->getMainBranch())); return new Pluf_HTTP_Response_Redirect($fburl); } $branches = $scm->getBranches(); @@ -277,26 +292,17 @@ class IDF_Views_Source return ''.implode(''.$sep.'', $out).''; } - public $commit_precond = array('IDF_Precondition::accessSource'); + public $commit_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function commit($request, $match) { $scm = IDF_Scm::get($request->project); $commit = $match[2]; - if (!$scm->isValidRevision($commit)) { - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); - } $large = $scm->isCommitLarge($commit); $cobject = $scm->getCommit($commit, !$large); if (!$cobject) { - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); + throw new Exception('could not retrieve commit object for '. $commit); } $title = sprintf(__('%s Commit Details'), (string) $request->project); $page_title = sprintf(__('%s Commit Details - %s'), (string) $request->project, $commit); @@ -326,19 +332,17 @@ class IDF_Views_Source $request); } - public $downloadDiff_precond = array('IDF_Precondition::accessSource'); + public $downloadDiff_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function downloadDiff($request, $match) { $scm = IDF_Scm::get($request->project); $commit = $match[2]; - if (!$scm->isValidRevision($commit)) { - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); - } $cobject = $scm->getCommit($commit, true); + if (!$cobject) { + throw new Exception('could not retrieve commit object for '. $commit); + } $rep = new Pluf_HTTP_Response($cobject->changes, 'text/plain'); $rep->headers['Content-Disposition'] = 'attachment; filename="'.$commit.'.diff"'; return $rep; @@ -394,19 +398,14 @@ class IDF_Views_Source * Get a given file at a given commit. * */ - public $getFile_precond = array('IDF_Precondition::accessSource'); + public $getFile_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function getFile($request, $match) { $scm = IDF_Scm::get($request->project); $commit = $match[2]; $request_file = $match[3]; - if (!$scm->isValidRevision($commit)) { - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); - } $request_file_info = $scm->getPathInfo($request_file, $commit); if (!$request_file_info or $request_file_info->type == 'tree') { // Redirect to the first branch @@ -427,18 +426,13 @@ class IDF_Views_Source * Get a zip archive of the current commit. * */ - public $download_precond = array('IDF_Precondition::accessSource'); + public $download_precond = array('IDF_Precondition::accessSource', + 'IDF_Views_Source_Precondition::scmAvailable', + 'IDF_Views_Source_Precondition::revisionValid'); public function download($request, $match) { $commit = trim($match[2]); $scm = IDF_Scm::get($request->project); - if (!$scm->isValidRevision($commit)) { - // Redirect to the first branch - $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::treeBase', - array($request->project->shortname, - $scm->getMainBranch())); - return new Pluf_HTTP_Response_Redirect($url); - } $base = $request->project->shortname.'-'.$commit; $cmd = $scm->getArchiveCommand($commit, $base.'/'); $rep = new Pluf_HTTP_Response_CommandPassThru($cmd, 'application/x-zip'); @@ -447,7 +441,6 @@ class IDF_Views_Source return $rep; } - /** * Find the mime type of a requested file. * @@ -495,7 +488,6 @@ class IDF_Views_Source return $res; } - /** * Find the mime type of a file. * @@ -610,3 +602,55 @@ function IDF_Views_Source_ShortenString($string, $length) substr($string, -($length - $preflen - mb_strlen($ellipse))); } +class IDF_Views_Source_Precondition +{ + /** + * Ensures that the configured SCM for the project is available + * + * @param $request + * @return true | Pluf_HTTP_Response_Redirect + */ + static public function scmAvailable($request) + { + $scm = IDF_Scm::get($request->project); + if (!$scm->isAvailable()) { + $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::help', + array($request->project->shortname)); + return new Pluf_HTTP_Response_Redirect($url); + } + return true; + } + + /** + * Validates the revision given in the URL path and acts accordingly + * + * @param $request + * @return true | Pluf_HTTP_Response_Redirect + * @throws Exception + */ + static public function revisionValid($request) + { + list($url_info, $url_matches) = $request->view; + list(, $project, $commit) = $url_matches; + + $scm = IDF_Scm::get($request->project); + $res = $scm->validateRevision($commit); + switch ($res) { + case IDF_Scm::REVISION_VALID: + return true; + case IDF_Scm::REVISION_INVALID: + $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::invalidRevision', + array($request->project->shortname, $commit)); + return new Pluf_HTTP_Response_Redirect($url); + case IDF_Scm::REVISION_AMBIGUOUS: + $url = Pluf_HTTP_URL_urlForView('IDF_Views_Source::disambiguateRevision', + array($request->project->shortname, + $commit, + $url_info['model'].'::'.$url_info['method'])); + return new Pluf_HTTP_Response_Redirect($url); + default: + throw new Exception('unknown validation result: '. $res); + } + } +} + diff --git a/src/IDF/conf/urls.php b/src/IDF/conf/urls.php index b4f6a4c..c01e565 100644 --- a/src/IDF/conf/urls.php +++ b/src/IDF/conf/urls.php @@ -148,6 +148,16 @@ $ctl[] = array('regex' => '#^/p/([\-\w]+)/source/help/$#', 'model' => 'IDF_Views_Source', 'method' => 'help'); +$ctl[] = array('regex' => '#^/p/([\-\w]+)/source/invalid/([^/]+)/$#', + 'base' => $base, + 'model' => 'IDF_Views_Source', + 'method' => 'invalidRevision'); + +$ctl[] = array('regex' => '#^/p/([\-\w]+)/source/disambiguate/([^/]+)/from/([^/]+)/$#', + 'base' => $base, + 'model' => 'IDF_Views_Source', + 'method' => 'disambiguateRevision'); + $ctl[] = array('regex' => '#^/p/([\-\w]+)/source/tree/([^/]+)/$#', 'base' => $base, 'model' => 'IDF_Views_Source', diff --git a/src/IDF/templates/idf/source/base.html b/src/IDF/templates/idf/source/base.html index 322f6ce..0526095 100644 --- a/src/IDF/templates/idf/source/base.html +++ b/src/IDF/templates/idf/source/base.html @@ -1,7 +1,7 @@ {extends "idf/base.html"} {block tabsource} class="active"{/block} {block subtabs} -{if !$inHelp and (in_array($commit, $tree_in) or (in_array($commit, $tags_in)))}{assign $currentCommit = $commit}{else}{assign $currentCommit = $project.getScmRoot()}{/if} +{if !$inHelp and !$inError and (in_array($commit, $tree_in) or (in_array($commit, $tags_in)))}{assign $currentCommit = $commit}{else}{assign $currentCommit = $project.getScmRoot()}{/if}
{trans 'Source Tree'} | {trans 'Change Log'} diff --git a/src/IDF/templates/idf/source/disambiguate_revision.html b/src/IDF/templates/idf/source/disambiguate_revision.html new file mode 100644 index 0000000..85c3d61 --- /dev/null +++ b/src/IDF/templates/idf/source/disambiguate_revision.html @@ -0,0 +1,33 @@ +{extends "idf/source/base.html"} +{block docclass}yui-t2{assign $inError=true}{/block} +{block body} + +

{blocktrans}The revision identifier {$commit} is ambiguous and can be +expanded to multiple valid revisions - please choose one:{/blocktrans}

+ + + + + + + + + + + + +{foreach $revisions as $revision} +{aurl 'url', $redirect, array($project.shortname, $revision.commit)} + + + + + + + + +{/foreach} + +
{trans 'Title'}{trans 'Author'}{trans 'Date'}{trans 'Branch'}{trans 'Revision'}
{$revision.title}{$revision.author}{$revision.date}{$revision.branch}{$revision.commit}
+{/block} + diff --git a/src/IDF/templates/idf/source/invalid_revision.html b/src/IDF/templates/idf/source/invalid_revision.html new file mode 100644 index 0000000..4c0e966 --- /dev/null +++ b/src/IDF/templates/idf/source/invalid_revision.html @@ -0,0 +1,17 @@ +{extends "idf/source/base.html"} +{block docclass}yui-t2{assign $inError=true}{/block} +{block body} + +

{blocktrans}The revision {$commit} is not valid or does not exist +in this repository.{/blocktrans}

+ +{if $isOwner or $isMember} +{aurl 'url', 'IDF_Views_Source::help', array($project.shortname)} +

{blocktrans}If this is a new repository, the reason for this error +could be that you have not committed and / or pushed any change so far. +In this case please take a look at the Help page +how to access your repository.{/blocktrans}

+{/if} + +{/block} +