diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9639eef11e..c73cec5965 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -598,7 +598,6 @@ phutil_register_library_map(array( 'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php', 'DiffusionExternalSymbolQuery' => 'applications/diffusion/symbol/DiffusionExternalSymbolQuery.php', 'DiffusionExternalSymbolsSource' => 'applications/diffusion/symbol/DiffusionExternalSymbolsSource.php', - 'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php', 'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php', 'DiffusionFileContentQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php', 'DiffusionFindSymbolsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionFindSymbolsConduitAPIMethod.php', @@ -4571,7 +4570,6 @@ phutil_register_library_map(array( 'DiffusionExternalController' => 'DiffusionController', 'DiffusionExternalSymbolQuery' => 'Phobject', 'DiffusionExternalSymbolsSource' => 'Phobject', - 'DiffusionFileContent' => 'Phobject', 'DiffusionFileContentQuery' => 'DiffusionQuery', 'DiffusionFileContentQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionFindSymbolsConduitAPIMethod' => 'DiffusionConduitAPIMethod', diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php index b6d9d11953..0fc6391987 100644 --- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php +++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php @@ -156,9 +156,21 @@ final class DifferentialDiffExtractionEngine extends Phobject { 'commit' => $identifier, 'path' => $path, )); - $corpus = $response['corpus']; - if ($files[$file_phid]->loadFileData() != $corpus) { + $new_file_phid = $response['filePHID']; + if (!$new_file_phid) { + return true; + } + + $new_file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($new_file_phid)) + ->executeOne(); + if (!$new_file) { + return true; + } + + if ($files[$file_phid]->loadFileData() != $new_file->loadFileData()) { return true; } } else { diff --git a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php index 73ed4161fe..8088aa66b8 100644 --- a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php @@ -27,8 +27,7 @@ final class DiffusionFileContentQueryConduitAPIMethod protected function getResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); - $file_query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest) - ->setViewer($request->getUser()); + $file_query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest); $timeout = $request->getValue('timeout'); if ($timeout) { @@ -40,16 +39,44 @@ final class DiffusionFileContentQueryConduitAPIMethod $file_query->setByteLimit($byte_limit); } - $file_content = $file_query->loadFileContent(); + $content = $file_query->execute(); - $text_list = $rev_list = $blame_dict = array(); + $too_slow = (bool)$file_query->getExceededTimeLimit(); + $too_huge = (bool)$file_query->getExceededByteLimit(); - $file_content - ->setBlameDict($blame_dict) - ->setRevList($rev_list) - ->setTextList($text_list); + $file_phid = null; + if (!$too_slow && !$too_huge) { + $file = $this->newFile($drequest, $content); + $file_phid = $file->getPHID(); + } - return $file_content->toDictionary(); + return array( + 'tooSlow' => $too_slow, + 'tooHuge' => $too_huge, + 'filePHID' => $file_phid, + ); + } + + private function newFile(DiffusionRequest $drequest, $content) { + $path = $drequest->getPath(); + $name = basename($path); + + $repository = $drequest->getRepository(); + $repository_phid = $repository->getPHID(); + + $file = PhabricatorFile::buildFromFileDataOrHash( + $content, + array( + 'name' => $name, + 'ttl' => time() + phutil_units('48 hours in seconds'), + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + )); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $file->attachToObject($repository_phid); + unset($unguarded); + + return $file; } } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index e61f24173a..eef95bb1ee 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -153,44 +153,62 @@ final class DiffusionBrowseController extends DiffusionController { ); } - $file_content = DiffusionFileContent::newFromConduit( - $this->callConduitWithDiffusionRequest( - 'diffusion.filecontentquery', - $params)); - $data = $file_content->getCorpus(); + $response = $this->callConduitWithDiffusionRequest( + 'diffusion.filecontentquery', + $params); - if ($view === 'raw') { - return $this->buildRawResponse($path, $data); - } + $hit_byte_limit = $response['tooHuge']; + $hit_time_limit = $response['tooSlow']; - $this->loadLintMessages(); - $this->coverage = $drequest->loadCoverage(); - - if ($byte_limit && (strlen($data) == $byte_limit)) { + $file_phid = $response['filePHID']; + if ($hit_byte_limit) { $corpus = $this->buildErrorCorpus( pht( 'This file is larger than %s byte(s), and too large to display '. 'in the web UI.', - $byte_limit)); - } else if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) { - $file = $this->loadFileForData($path, $data); - $file_uri = $file->getBestURI(); - - if ($file->isViewableImage()) { - $corpus = $this->buildImageCorpus($file_uri); - } else { - $corpus = $this->buildBinaryCorpus($file_uri, $data); - } + phutil_format_bytes($byte_limit))); + } else if ($hit_time_limit) { + $corpus = $this->buildErrorCorpus( + pht( + 'This file took too long to load from the repository (more than '. + '%s second(s)).', + new PhutilNumber($time_limit))); } else { - // Build the content of the file. - $corpus = $this->buildCorpus( - $show_blame, - $show_color, - $file_content, - $needs_blame, - $drequest, - $path, - $data); + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($file_phid)) + ->executeOne(); + if (!$file) { + throw new Exception(pht('Failed to load content file!')); + } + + if ($view === 'raw') { + return $file->getRedirectResponse(); + } + + $data = $file->loadFileData(); + if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) { + $file_uri = $file->getBestURI(); + + if ($file->isViewableImage()) { + $corpus = $this->buildImageCorpus($file_uri); + } else { + $corpus = $this->buildBinaryCorpus($file_uri, $data); + } + } else { + $this->loadLintMessages(); + $this->coverage = $drequest->loadCoverage(); + + // Build the content of the file. + $corpus = $this->buildCorpus( + $show_blame, + $show_color, + $data, + $needs_blame, + $drequest, + $path, + $data); + } } if ($request->isAjax()) { @@ -327,23 +345,7 @@ final class DiffusionBrowseController extends DiffusionController { } $content[] = $this->buildOpenRevisions(); - - - $readme_path = $results->getReadmePath(); - if ($readme_path) { - $readme_content = $this->callConduitWithDiffusionRequest( - 'diffusion.filecontentquery', - array( - 'path' => $readme_path, - 'commit' => $drequest->getStableCommit(), - )); - if ($readme_content) { - $content[] = id(new DiffusionReadmeView()) - ->setUser($this->getViewer()) - ->setPath($readme_path) - ->setContent($readme_content['corpus']); - } - } + $content[] = $this->renderDirectoryReadme($results); $crumbs = $this->buildCrumbs( array( @@ -584,7 +586,7 @@ final class DiffusionBrowseController extends DiffusionController { private function buildCorpus( $show_blame, $show_color, - DiffusionFileContent $file_content, + $file_corpus, $needs_blame, DiffusionRequest $drequest, $path, @@ -594,7 +596,6 @@ final class DiffusionBrowseController extends DiffusionController { $blame_timeout = 15; $blame_failed = false; - $file_corpus = $file_content->getCorpus(); $highlight_limit = DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT; $blame_limit = DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT; $can_highlight = (strlen($file_corpus) <= $highlight_limit); @@ -1256,28 +1257,6 @@ final class DiffusionBrowseController extends DiffusionController { return $rows; } - private function loadFileForData($path, $data) { - $file = PhabricatorFile::buildFromFileDataOrHash( - $data, - array( - 'name' => basename($path), - 'ttl' => time() + 60 * 60 * 24, - 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, - )); - - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $file->attachToObject( - $this->getDiffusionRequest()->getRepository()->getPHID()); - unset($unguarded); - - return $file; - } - - private function buildRawResponse($path, $data) { - $file = $this->loadFileForData($path, $data); - return $file->getRedirectResponse(); - } - private function buildImageCorpus($file_uri) { $properties = new PHUIPropertyListView(); @@ -1299,7 +1278,6 @@ final class DiffusionBrowseController extends DiffusionController { } private function buildBinaryCorpus($file_uri, $data) { - $size = new PhutilNumber(strlen($data)); $text = pht('This is a binary file. It is %s byte(s) in length.', $size); $text = id(new PHUIBoxView()) diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index e398a88114..a3c661a26d 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -287,4 +287,49 @@ abstract class DiffusionController extends PhabricatorController { ->appendChild($pager); } + protected function renderDirectoryReadme(DiffusionBrowseResultSet $browse) { + $readme_path = $browse->getReadmePath(); + if ($readme_path === null) { + return null; + } + + $drequest = $this->getDiffusionRequest(); + $viewer = $this->getViewer(); + + try { + $result = $this->callConduitWithDiffusionRequest( + 'diffusion.filecontentquery', + array( + 'path' => $readme_path, + 'commit' => $drequest->getStableCommit(), + )); + } catch (Exception $ex) { + return null; + } + + $file_phid = $result['filePHID']; + if (!$file_phid) { + return null; + } + + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($file_phid)) + ->executeOne(); + if (!$file) { + return null; + } + + $corpus = $file->loadFileData(); + + if (!strlen($corpus)) { + return null; + } + + return id(new DiffusionReadmeView()) + ->setUser($this->getViewer()) + ->setPath($readme_path) + ->setContent($corpus); + } + } diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index dc6969b81d..9fc948d19a 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -161,23 +161,10 @@ final class DiffusionRepositoryController extends DiffusionController { $phids = array_keys($phids); $handles = $this->loadViewerHandles($phids); - $readme = null; if ($browse_results) { - $readme_path = $browse_results->getReadmePath(); - if ($readme_path) { - $readme_content = $this->callConduitWithDiffusionRequest( - 'diffusion.filecontentquery', - array( - 'path' => $readme_path, - 'commit' => $drequest->getStableCommit(), - )); - if ($readme_content) { - $readme = id(new DiffusionReadmeView()) - ->setUser($this->getViewer()) - ->setPath($readme_path) - ->setContent($readme_content['corpus']); - } - } + $readme = $this->renderDirectoryReadme($browse_results); + } else { + $readme = null; } $content[] = $this->buildBrowseTable( diff --git a/src/applications/diffusion/data/DiffusionFileContent.php b/src/applications/diffusion/data/DiffusionFileContent.php deleted file mode 100644 index 1680d1f964..0000000000 --- a/src/applications/diffusion/data/DiffusionFileContent.php +++ /dev/null @@ -1,63 +0,0 @@ -textList = $text_list; - return $this; - } - public function getTextList() { - if (!$this->textList) { - return phutil_split_lines($this->getCorpus(), $retain_ends = false); - } - return $this->textList; - } - - public function setRevList(array $rev_list) { - $this->revList = $rev_list; - return $this; - } - public function getRevList() { - return $this->revList; - } - - public function setBlameDict(array $blame_dict) { - $this->blameDict = $blame_dict; - return $this; - } - public function getBlameDict() { - return $this->blameDict; - } - - public function setCorpus($corpus) { - $this->corpus = $corpus; - return $this; - } - - public function getCorpus() { - return $this->corpus; - } - - public function toDictionary() { - return array( - 'corpus' => $this->getCorpus(), - 'blameDict' => $this->getBlameDict(), - 'revList' => $this->getRevList(), - 'textList' => $this->getTextList(), - ); - } - - public static function newFromConduit(array $dict) { - return id(new DiffusionFileContent()) - ->setCorpus($dict['corpus']) - ->setBlameDict($dict['blameDict']) - ->setRevList($dict['revList']) - ->setTextList($dict['textList']); - } - -} diff --git a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php index 6a9cf2e021..3ea0e12ba1 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php @@ -1,18 +1,13 @@ timeout = $timeout; return $this; @@ -31,71 +26,51 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { return $this->byteLimit; } - public function setViewer(PhabricatorUser $user) { - $this->viewer = $user; - return $this; - } - - public function getViewer() { - return $this->viewer; - } - final public static function newFromDiffusionRequest( DiffusionRequest $request) { return parent::newQueryObject(__CLASS__, $request); } - abstract public function getFileContentFuture(); - abstract protected function executeQueryFromFuture(Future $future); + final public function getExceededByteLimit() { + return $this->didHitByteLimit; + } - final public function loadFileContentFromFuture(Future $future) { + final public function getExceededTimeLimit() { + return $this->didHitTimeLimit; + } - if ($this->timeout) { - $future->setTimeout($this->timeout); + abstract protected function getFileContentFuture(); + abstract protected function resolveFileContentFuture(Future $future); + + final protected function executeQuery() { + $future = $this->getFileContentFuture(); + + if ($this->getTimeout()) { + $future->setTimeout($this->getTimeout()); } - if ($this->getByteLimit()) { - $future->setStdoutSizeLimit($this->getByteLimit()); + $byte_limit = $this->getByteLimit(); + if ($byte_limit) { + $future->setStdoutSizeLimit($byte_limit + 1); } try { - $file_content = $this->executeQueryFromFuture($future); + $file_content = $this->resolveFileContentFuture($future); } catch (CommandException $ex) { if (!$future->getWasKilledByTimeout()) { throw $ex; } - $message = pht( - '', - $this->timeout); - - $file_content = new DiffusionFileContent(); - $file_content->setCorpus($message); + $this->didHitTimeLimit = true; + $file_content = null; } - $this->fileContent = $file_content; - - $repository = $this->getRequest()->getRepository(); - $try_encoding = $repository->getDetail('encoding'); - if ($try_encoding) { - $this->fileContent->setCorpus( - phutil_utf8_convert( - $this->fileContent->getCorpus(), 'UTF-8', $try_encoding)); + if ($byte_limit && (strlen($file_content) > $byte_limit)) { + $this->didHitByteLimit = true; + $file_content = null; } - return $this->fileContent; - } - - final protected function executeQuery() { - return $this->loadFileContentFromFuture($this->getFileContentFuture()); - } - - final public function loadFileContent() { - return $this->executeQuery(); - } - - final public function getRawData() { - return $this->fileContent->getCorpus(); + return $file_content; } } diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php index 448d2886dc..a383aee922 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php @@ -2,7 +2,7 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery { - public function getFileContentFuture() { + protected function getFileContentFuture() { $drequest = $this->getRequest(); $repository = $drequest->getRepository(); @@ -15,13 +15,9 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery { $path); } - protected function executeQueryFromFuture(Future $future) { + protected function resolveFileContentFuture(Future $future) { list($corpus) = $future->resolvex(); - - $file_content = new DiffusionFileContent(); - $file_content->setCorpus($corpus); - - return $file_content; + return $corpus; } } diff --git a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php index ec49414666..539f5752c2 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php @@ -3,7 +3,7 @@ final class DiffusionMercurialFileContentQuery extends DiffusionFileContentQuery { - public function getFileContentFuture() { + protected function getFileContentFuture() { $drequest = $this->getRequest(); $repository = $drequest->getRepository(); @@ -16,13 +16,9 @@ final class DiffusionMercurialFileContentQuery $path); } - protected function executeQueryFromFuture(Future $future) { + protected function resolveFileContentFuture(Future $future) { list($corpus) = $future->resolvex(); - - $file_content = new DiffusionFileContent(); - $file_content->setCorpus($corpus); - - return $file_content; + return $corpus; } } diff --git a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php index 4976d7a27b..0f6f30b355 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php @@ -2,7 +2,7 @@ final class DiffusionSvnFileContentQuery extends DiffusionFileContentQuery { - public function getFileContentFuture() { + protected function getFileContentFuture() { $drequest = $this->getRequest(); $repository = $drequest->getRepository(); @@ -14,30 +14,9 @@ final class DiffusionSvnFileContentQuery extends DiffusionFileContentQuery { $repository->getSubversionPathURI($path, $commit)); } - protected function executeQueryFromFuture(Future $future) { - try { - list($corpus) = $future->resolvex(); - } catch (CommandException $ex) { - $stderr = $ex->getStdErr(); - if (preg_match('/path not found$/', trim($stderr))) { - // TODO: Improve user experience for this. One way to end up here - // is to have the parser behind and look at a file which was recently - // nuked; Diffusion will think it still exists and try to grab content - // at HEAD. - throw new Exception( - pht( - 'Failed to retrieve file content from Subversion. The file may '. - 'have been recently deleted, or the Diffusion cache may be out of '. - 'date.')); - } else { - throw $ex; - } - } - - $file_content = new DiffusionFileContent(); - $file_content->setCorpus($corpus); - - return $file_content; + protected function resolveFileContentFuture(Future $future) { + list($corpus) = $future->resolvex(); + return $corpus; } }