Differential D13070 Diff 32319 src/applications/diviner/query/DivinerBookQuery.php

Changeset View

Standalone View

src/applications/diviner/query/DivinerBookQuery.php

<?php		<?php

final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery {		final class DivinerBookQuery extends PhabricatorCursorPagedPolicyAwareQuery {

private $ids;		private $ids;
private $phids;		private $phids;
private $names;		private $names;
		private $repositoryPHIDs;

private $needProjectPHIDs;		private $needProjectPHIDs;
		private $needRepositories;

public function withIDs(array $ids) {		public function withIDs(array $ids) {
$this->ids = $ids;		$this->ids = $ids;
return $this;		return $this;
}		}

public function withPHIDs(array $phids) {		public function withPHIDs(array $phids) {
$this->phids = $phids;		$this->phids = $phids;
return $this;		return $this;
}		}

public function withNames(array $names) {		public function withNames(array $names) {
$this->names = $names;		$this->names = $names;
return $this;		return $this;
}		}

		public function withRepositoryPHIDs(array $repository_phids) {
		$this->repositoryPHIDs = $repository_phids;
		return $this;
		}

public function needProjectPHIDs($need_phids) {		public function needProjectPHIDs($need_phids) {
$this->needProjectPHIDs = $need_phids;		$this->needProjectPHIDs = $need_phids;
return $this;		return $this;
}		}
		epriestleyUnsubmitted Done Inline Actions I think we should always load these and perform policy filtering; if you can't see a repository, you should not be able to see its books (similar to how revisions work). (This might create some issues with generating public documentation from private repositories, but I don't think that's a major use case and it shouldn't be the default behavior.) epriestley: I think we should always load these and perform policy filtering; if you can't see a repository…

		public function needRepositories($need_repositories) {
		$this->needRepositories = $need_repositories;
		return $this;
		}

protected function loadPage() {		protected function loadPage() {
$table = new DivinerLiveBook();		$table = new DivinerLiveBook();
$conn_r = $table->establishConnection('r');		$conn_r = $table->establishConnection('r');

$data = queryfx_all(		$data = queryfx_all(
$conn_r,		$conn_r,
'SELECT * FROM %T %Q %Q %Q',		'SELECT * FROM %T %Q %Q %Q',
$table->getTableName(),		$table->getTableName(),
$this->buildWhereClause($conn_r),		$this->buildWhereClause($conn_r),
$this->buildOrderClause($conn_r),		$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));		$this->buildLimitClause($conn_r));

return $table->loadAllFromArray($data);		return $table->loadAllFromArray($data);
}		}

protected function didFilterPage(array $books) {		protected function didFilterPage(array $books) {
assert_instances_of($books, 'DivinerLiveBook');		assert_instances_of($books, 'DivinerLiveBook');

		epriestleyUnsubmitted Done Inline Actions For consistency, do this in `willFilterPage()`. epriestley: For consistency, do this in `willFilterPage()`.
		if ($this->needRepositories) {
		epriestleyUnsubmitted Not Done Inline Actions This will technically work fine, but `mpull($books, 'getRepositoryPHID')` might return something like `array(null, null, null)`. This might be slightly clearer (although also slightly longer) with more effort to handle that case. epriestley: This will technically work fine, but `mpull($books, 'getRepositoryPHID')` might return…
		$repositories = id(new PhabricatorRepositoryQuery())
		->setViewer($this->getViewer())
		->withPHIDs(mpull($books, 'getRepositoryPHID'))
		->execute();
		$repositories = mpull($repositories, null, 'getPHID');

		foreach ($books as $key => $book) {
		if ($book->getRepositoryPHID() === null) {
		epriestleyUnsubmitted Done Inline Actions This likely needs to `$book->attachRepository(null)` to avoid throwing when loading books with no repository and later calling `getRepository()` on them. (That will imply a corresponding `PhabricatorRepository $repository = null` default parameter value in `attachRepository()`.) epriestley: This likely needs to `$book->attachRepository(null)` to avoid throwing when loading books with…
		$book->attachRepository(null);
		continue;
		}

		$repository = idx($repositories, $book->getRepositoryPHID());

		if (!$repository) {
		epriestleyUnsubmitted Not Done Inline Actions Per that recent handle stuff, should `didRejectResult()` here. epriestley: Per that recent handle stuff, should `didRejectResult()` here.
		$this->didRejectResult($book);
		unset($books[$key]);
		continue;
		}

		$book->attachRepository($repository);
		}
		}

if ($this->needProjectPHIDs) {		if ($this->needProjectPHIDs) {
$edge_query = id(new PhabricatorEdgeQuery())		$edge_query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(mpull($books, 'getPHID'))		->withSourcePHIDs(mpull($books, 'getPHID'))
->withEdgeTypes(		->withEdgeTypes(
array(		array(
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,		PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
));		));
$edge_query->execute();		$edge_query->execute();
Show All 29 Lines	protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {

if ($this->names) {		if ($this->names) {
$where[] = qsprintf(		$where[] = qsprintf(
$conn_r,		$conn_r,
'name IN (%Ls)',		'name IN (%Ls)',
$this->names);		$this->names);
}		}

		if ($this->repositoryPHIDs !== null) {
		epriestleyUnsubmitted Done Inline Actions Prefer `!== null`, so `setRepositoryPHIDs($phids)` is an error when `$phids` is `array()`, instead of an accidental query for all results. epriestley: Prefer `!== null`, so `setRepositoryPHIDs($phids)` is an error when `$phids` is `array()`…
		$where[] = qsprintf(
		$conn_r,
		'repositoryPHID IN (%Ls)',
		$this->repositoryPHIDs);
		}

$where[] = $this->buildPagingClause($conn_r);		$where[] = $this->buildPagingClause($conn_r);

return $this->formatWhereClause($where);		return $this->formatWhereClause($where);
}		}

public function getQueryApplicationClass() {		public function getQueryApplicationClass() {
return 'PhabricatorDivinerApplication';		return 'PhabricatorDivinerApplication';
}		}

}		}
		epriestleyUnsubmitted Done Inline Actions If the repository fails to load, I think we should filter the book out (i.e., same deal as revisions: if you can't see a repository, you can't see its stuff). epriestley: If the repository fails to load, I think we should filter the book out (i.e., same deal as…