Changeset View
Changeset View
Standalone View
Standalone View
src/applications/repository/query/PhabricatorRepositoryQuery.php
<?php | <?php | ||||
final class PhabricatorRepositoryQuery | final class PhabricatorRepositoryQuery | ||||
extends PhabricatorCursorPagedPolicyAwareQuery { | extends PhabricatorCursorPagedPolicyAwareQuery { | ||||
private $ids; | private $ids; | ||||
private $phids; | private $phids; | ||||
private $callsigns; | private $callsigns; | ||||
private $types; | private $types; | ||||
private $uuids; | private $uuids; | ||||
private $nameContains; | private $nameContains; | ||||
private $remoteURIs; | private $remoteURIs; | ||||
private $anyProjectPHIDs; | private $anyProjectPHIDs; | ||||
private $numericIdentifiers; | |||||
private $callsignIdentifiers; | |||||
private $phidIdentifiers; | |||||
private $identifierMap; | |||||
const STATUS_OPEN = 'status-open'; | const STATUS_OPEN = 'status-open'; | ||||
const STATUS_CLOSED = 'status-closed'; | const STATUS_CLOSED = 'status-closed'; | ||||
const STATUS_ALL = 'status-all'; | const STATUS_ALL = 'status-all'; | ||||
private $status = self::STATUS_ALL; | private $status = self::STATUS_ALL; | ||||
const ORDER_CREATED = 'order-created'; | const ORDER_CREATED = 'order-created'; | ||||
const ORDER_COMMITTED = 'order-committed'; | const ORDER_COMMITTED = 'order-committed'; | ||||
const ORDER_CALLSIGN = 'order-callsign'; | const ORDER_CALLSIGN = 'order-callsign'; | ||||
Show All 19 Lines | public function withPHIDs(array $phids) { | ||||
return $this; | return $this; | ||||
} | } | ||||
public function withCallsigns(array $callsigns) { | public function withCallsigns(array $callsigns) { | ||||
$this->callsigns = $callsigns; | $this->callsigns = $callsigns; | ||||
return $this; | return $this; | ||||
} | } | ||||
public function withIdentifiers(array $identifiers) { | |||||
$ids = array(); $callsigns = array(); $phids = array(); | |||||
foreach ($identifiers as $identifier) { | |||||
if (ctype_digit($identifier)) { | |||||
$ids[$identifier] = $identifier; | |||||
} else { | |||||
$repository_type = PhabricatorRepositoryRepositoryPHIDType::TYPECONST; | |||||
if (phid_get_type($identifier) === $repository_type) { | |||||
epriestley: I think we should either disallow this "r" stripping, or preserve it in `getIdentifierMap()`. | |||||
$phids[$identifier] = $identifier; | |||||
} else { | |||||
$callsigns[$identifier] = $identifier; | |||||
} | |||||
} | |||||
Not Done Inline ActionsYou can use this to identify repository PHIDs: $repository_type = PhabricatorRepositoryRepositoryPHIDType::TYPECONST; if (phid_get_type($identifier) === $repository_type) { // ... } (It's not perfect, but should always behave in reasonable/expected ways.) epriestley: You can use this to identify repository PHIDs:
$repository_type =… | |||||
} | |||||
$this->numericIdentifiers = $ids; | |||||
$this->callsignIdentifiers = $callsigns; | |||||
$this->phidIdentifiers = $phids; | |||||
return $this; | |||||
} | |||||
public function withStatus($status) { | public function withStatus($status) { | ||||
$this->status = $status; | $this->status = $status; | ||||
return $this; | return $this; | ||||
} | } | ||||
public function withHosted($hosted) { | public function withHosted($hosted) { | ||||
$this->hosted = $hosted; | $this->hosted = $hosted; | ||||
return $this; | return $this; | ||||
Show All 39 Lines | public function needProjectPHIDs($need_phids) { | ||||
return $this; | return $this; | ||||
} | } | ||||
public function setOrder($order) { | public function setOrder($order) { | ||||
$this->order = $order; | $this->order = $order; | ||||
return $this; | return $this; | ||||
} | } | ||||
public function getIdentifierMap() { | |||||
if ($this->identifierMap === null) { | |||||
throw new Exception( | |||||
'You must execute() the query before accessing the identifier map.'); | |||||
} | |||||
return $this->identifierMap; | |||||
} | |||||
protected function loadPage() { | protected function loadPage() { | ||||
$table = new PhabricatorRepository(); | $table = new PhabricatorRepository(); | ||||
$conn_r = $table->establishConnection('r'); | $conn_r = $table->establishConnection('r'); | ||||
if ($this->identifierMap === null) { | |||||
$this->identifierMap = array(); | |||||
} | |||||
$data = queryfx_all( | $data = queryfx_all( | ||||
$conn_r, | $conn_r, | ||||
'SELECT * FROM %T r %Q %Q %Q %Q', | 'SELECT * FROM %T r %Q %Q %Q %Q', | ||||
$table->getTableName(), | $table->getTableName(), | ||||
$this->buildJoinsClause($conn_r), | $this->buildJoinsClause($conn_r), | ||||
$this->buildWhereClause($conn_r), | $this->buildWhereClause($conn_r), | ||||
$this->buildOrderClause($conn_r), | $this->buildOrderClause($conn_r), | ||||
$this->buildLimitClause($conn_r)); | $this->buildLimitClause($conn_r)); | ||||
▲ Show 20 Lines • Show All 80 Lines • ▼ Show 20 Lines | if ($this->remoteURIs) { | ||||
$try_uris = array_fuse($try_uris); | $try_uris = array_fuse($try_uris); | ||||
foreach ($repositories as $key => $repository) { | foreach ($repositories as $key => $repository) { | ||||
if (!isset($try_uris[$repository->getNormalizedPath()])) { | if (!isset($try_uris[$repository->getNormalizedPath()])) { | ||||
unset($repositories[$key]); | unset($repositories[$key]); | ||||
} | } | ||||
} | } | ||||
} | } | ||||
// Build the identifierMap | |||||
if ($this->numericIdentifiers) { | |||||
Not Done Inline ActionsWe should initialize the identifier map in loadPage(), before executing the query: if ($this->identifierMap === null) { $this->identifierMap = array(); } Two reasons:
epriestley: We should initialize the identifier map in `loadPage()`, before executing the query:
if… | |||||
foreach ($this->numericIdentifiers as $id) { | |||||
if (isset($repositories[$id])) { | |||||
$this->identifierMap[$id] = $repositories[$id]; | |||||
} | |||||
} | |||||
} | |||||
if ($this->callsignIdentifiers) { | |||||
$repository_callsigns = mpull($repositories, null, 'getCallsign'); | |||||
foreach ($this->callsignIdentifiers as $callsign) { | |||||
if (isset($repository_callsigns[$callsign])) { | |||||
$this->identifierMap[$callsign] = $repository_callsigns[$callsign]; | |||||
} | |||||
} | |||||
} | |||||
if ($this->phidIdentifiers) { | |||||
$repository_phids = mpull($repositories, null, 'getPHID'); | |||||
foreach ($this->phidIdentifiers as $phid) { | |||||
if (isset($repository_phids[$phid])) { | |||||
$this->identifierMap[$phid] = $repository_phids[$phid]; | |||||
} | |||||
} | |||||
} | |||||
return $repositories; | return $repositories; | ||||
} | } | ||||
public function didFilterPage(array $repositories) { | public function didFilterPage(array $repositories) { | ||||
if ($this->needProjectPHIDs) { | if ($this->needProjectPHIDs) { | ||||
$type_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; | $type_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; | ||||
$edge_query = id(new PhabricatorEdgeQuery()) | $edge_query = id(new PhabricatorEdgeQuery()) | ||||
▲ Show 20 Lines • Show All 169 Lines • ▼ Show 20 Lines | private function buildWhereClause(AphrontDatabaseConnection $conn_r) { | ||||
if ($this->callsigns) { | if ($this->callsigns) { | ||||
$where[] = qsprintf( | $where[] = qsprintf( | ||||
$conn_r, | $conn_r, | ||||
'r.callsign IN (%Ls)', | 'r.callsign IN (%Ls)', | ||||
$this->callsigns); | $this->callsigns); | ||||
} | } | ||||
if ($this->numericIdentifiers || | |||||
$this->callsignIdentifiers || | |||||
$this->phidIdentifiers) { | |||||
$identifier_clause = array(); | |||||
Not Done Inline ActionsI think it might be cleaner to not reuse the ids and phids property to construct this clause. One issue with the current approach is that this: $some_query ->withIdentifiers(array('1', 'X')) ->withPHIDs(array('the-PHID-of-repository-1', 'the-phid-of-repository-Q')) ...will incorrectly select repositories r1, rX, and rQ, when it should select only repository r1. You could just define separate properties like: public function withIdentifiers(array $identifiers) { // ... split them up ... $this->numericIdentifiers = $numbery_ones; $this->callsignIdentifiers = $callsigney_ones; } ...and then do this: if ($this->numericIdentifiers || $this->callsignIdentifiers) { $identifier_clause = array(); if ($this->numericIdentifiers) { $identifier_cause[] = qsprintf(...); } if ($this->callsignIdentifiers) { $identifier_clause[] = qsprinf(...); } $where[] = '('.implode(') OR (', $identifier_clause).')'; } Then withIdentifiers() will interact as expected with calls separate calls to withIDs(), withPHIDs(), and withCallsigns(). epriestley: I think it might be cleaner to not reuse the `ids` and `phids` property to construct this… | |||||
if ($this->numericIdentifiers) { | |||||
$identifier_clause[] = qsprintf( | |||||
$conn_r, | |||||
'r.id IN (%Ld)', | |||||
Not Done Inline ActionsThis should be %Ld (list of integers) vs %Ls (list of strings). epriestley: This should be `%Ld` (list of integers) vs `%Ls` (list of strings). | |||||
$this->numericIdentifiers); | |||||
} | |||||
if ($this->callsignIdentifiers) { | |||||
$identifier_clause[] = qsprintf( | |||||
$conn_r, | |||||
'r.callsign IN (%Ls)', | |||||
$this->callsignIdentifiers); | |||||
} | |||||
if ($this->phidIdentifiers) { | |||||
$identifier_clause[] = qsprintf( | |||||
$conn_r, | |||||
'r.phid IN (%Ls)', | |||||
$this->phidIdentifiers); | |||||
} | |||||
$where = array('('.implode(' OR ', $identifier_clause).')'); | |||||
} | |||||
if ($this->types) { | if ($this->types) { | ||||
$where[] = qsprintf( | $where[] = qsprintf( | ||||
$conn_r, | $conn_r, | ||||
'r.versionControlSystem IN (%Ls)', | 'r.versionControlSystem IN (%Ls)', | ||||
$this->types); | $this->types); | ||||
} | } | ||||
if ($this->uuids) { | if ($this->uuids) { | ||||
Show All 17 Lines | if ($this->anyProjectPHIDs) { | ||||
$this->anyProjectPHIDs); | $this->anyProjectPHIDs); | ||||
} | } | ||||
$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 'PhabricatorDiffusionApplication'; | return 'PhabricatorDiffusionApplication'; | ||||
} | } | ||||
private function getNormalizedPaths() { | private function getNormalizedPaths() { | ||||
$normalized_uris = array(); | $normalized_uris = array(); | ||||
// Since we don't know which type of repository this URI is in the general | // Since we don't know which type of repository this URI is in the general | ||||
Show All 21 Lines |
I think we should either disallow this "r" stripping, or preserve it in getIdentifierMap(). That is, if you make a call like:
...and the query finds a matching result, the key r1 (i.e., the exact key you passed in) should always be present in the identifier map. Otherwise you have to trim the "r" off yourself to figure out which result matches, and you might as well do that in the first place.