D14970.diff
No OneTemporary
Actions

Size

21 KB

Referenced Files

None

Subscribers

None

D14970.diff
View Options

	diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
	--- a/src/__phutil_library_map__.php
	+++ b/src/__phutil_library_map__.php
	@@ -598,7 +598,6 @@
	'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 @@
	'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
	--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
	+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
	@@ -156,9 +156,21 @@
	'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
	--- a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php
	+++ b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php
	@@ -27,8 +27,7 @@
	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 @@
	$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 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_content->toDictionary();
	+ return $file;
	}

	}
	diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php
	--- a/src/applications/diffusion/controller/DiffusionBrowseController.php
	+++ b/src/applications/diffusion/controller/DiffusionBrowseController.php
	@@ -153,44 +153,62 @@
	);
	}

	- $file_content = DiffusionFileContent::newFromConduit(
	- $this->callConduitWithDiffusionRequest(
	- 'diffusion.filecontentquery',
	- $params));
	- $data = $file_content->getCorpus();
	-
	- if ($view === 'raw') {
	- return $this->buildRawResponse($path, $data);
	- }
	+ $response = $this->callConduitWithDiffusionRequest(
	+ 'diffusion.filecontentquery',
	+ $params);

	- $this->loadLintMessages();
	- $this->coverage = $drequest->loadCoverage();
	+ $hit_byte_limit = $response['tooHuge'];
	+ $hit_time_limit = $response['tooSlow'];

	- 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();
	+ 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 {
	+ $file = id(new PhabricatorFileQuery())
	+ ->setViewer($viewer)
	+ ->withPHIDs(array($file_phid))
	+ ->executeOne();
	+ if (!$file) {
	+ throw new Exception(pht('Failed to load content file!'));
	+ }

	- if ($file->isViewableImage()) {
	- $corpus = $this->buildImageCorpus($file_uri);
	+ 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 {
	- $corpus = $this->buildBinaryCorpus($file_uri, $data);
	+ $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);
	}
	- } else {
	- // Build the content of the file.
	- $corpus = $this->buildCorpus(
	- $show_blame,
	- $show_color,
	- $file_content,
	- $needs_blame,
	- $drequest,
	- $path,
	- $data);
	}

	if ($request->isAjax()) {
	@@ -327,23 +345,7 @@
	}

	$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 @@
	private function buildCorpus(
	$show_blame,
	$show_color,
	- DiffusionFileContent $file_content,
	+ $file_corpus,
	$needs_blame,
	DiffusionRequest $drequest,
	$path,
	@@ -594,7 +596,6 @@
	$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 @@
	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 @@
	}

	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
	--- a/src/applications/diffusion/controller/DiffusionController.php
	+++ b/src/applications/diffusion/controller/DiffusionController.php
	@@ -287,4 +287,49 @@
	->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
	--- a/src/applications/diffusion/controller/DiffusionRepositoryController.php
	+++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php
	@@ -161,23 +161,10 @@
	$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
	--- a/src/applications/diffusion/data/DiffusionFileContent.php
	+++ /dev/null
	@@ -1,63 +0,0 @@
	-<?php
	-
	-final class DiffusionFileContent extends Phobject {
	-
	- private $corpus;
	- private $blameDict;
	- private $revList;
	- private $textList;
	-
	- public function setTextList(array $text_list) {
	- $this->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
	--- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php
	+++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php
	@@ -1,18 +1,13 @@
	<?php

	-/**
	- * NOTE: this class should only be used where local access to the repository
	- * is guaranteed and NOT from within the Diffusion application. Diffusion
	- * should use Conduit method 'diffusion.filecontentquery' to get this sort
	- * of data.
	- */
	abstract class DiffusionFileContentQuery extends DiffusionQuery {

	- private $fileContent;
	- private $viewer;
	private $timeout;
	private $byteLimit;

	+ private $didHitByteLimit = false;
	+ private $didHitTimeLimit = false;
	+
	public function setTimeout($timeout) {
	$this->timeout = $timeout;
	return $this;
	@@ -31,71 +26,51 @@
	return $this->byteLimit;
	}

	- public function setViewer(PhabricatorUser $user) {
	- $this->viewer = $user;
	- return $this;
	+ final public static function newFromDiffusionRequest(
	+ DiffusionRequest $request) {
	+ return parent::newQueryObject(__CLASS__, $request);
	}

	- public function getViewer() {
	- return $this->viewer;
	+ final public function getExceededByteLimit() {
	+ return $this->didHitByteLimit;
	}

	- final public static function newFromDiffusionRequest(
	- DiffusionRequest $request) {
	- return parent::newQueryObject(__CLASS__, $request);
	+ final public function getExceededTimeLimit() {
	+ return $this->didHitTimeLimit;
	}

	- abstract public function getFileContentFuture();
	- abstract protected function executeQueryFromFuture(Future $future);
	+ abstract protected function getFileContentFuture();
	+ abstract protected function resolveFileContentFuture(Future $future);

	- final public function loadFileContentFromFuture(Future $future) {
	+ final protected function executeQuery() {
	+ $future = $this->getFileContentFuture();

	- if ($this->timeout) {
	- $future->setTimeout($this->timeout);
	+ 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(
	- '<Attempt to load this file was terminated after %s second(s).>',
	- $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
	--- 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 @@
	$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
	--- 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 @@
	$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
	--- 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 @@
	$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;
	}

	}

File Metadata

Mime Type: text/plain
Expires: Mon, Mar 10, 10:14 PM (1 d, 4 h ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 7475881
Default Alt Text: D14970.diff (21 KB)

D14970.diffNo OneTemporaryActions

D14970.diffView Options

File Metadata

Event Timeline

D14970.diff
No OneTemporary
Actions

D14970.diff
View Options