Page MenuHomePhabricator
Files F15410437

D8879.id21061.diff
No OneTemporary
Actions

D8879.id21061.diff
View Options

diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php
--- a/src/applications/diffusion/query/DiffusionCommitQuery.php
+++ b/src/applications/diffusion/query/DiffusionCommitQuery.php
@@ -19,7 +19,6 @@
const AUDIT_STATUS_ANY = 'audit-status-any';
const AUDIT_STATUS_OPEN = 'audit-status-open';
const AUDIT_STATUS_CONCERN = 'audit-status-concern';
- private $loadAuditIds;
private $needCommitData;
@@ -88,13 +87,28 @@
return $this;
}
- public function getAuditRequests() {
+ /**
+ * Retuns true if we should join the audit table, either because we're
+ * interested in the information if it's available or because matching
+ * rows must always have it.
+ */
+ private function shouldJoinAudits() {
return
$this->needAuditRequests ||
+ $this->auditStatus ||
+ $this->rowsMustHaveAudits();
+ }
+
+
+ /**
+ * Return true if we should `JOIN` (vs `LEFT JOIN`) the audit table, because
+ * matching commits will always have audit rows.
+ */
+ private function rowsMustHaveAudits() {
+ return
$this->auditIDs ||
$this->auditorPHIDs ||
- $this->auditAwaitingUser ||
- $this->auditStatus;
+ $this->auditAwaitingUser;
}
public function withAuditIDs(array $ids) {
@@ -141,31 +155,17 @@
$data = queryfx_all(
$conn_r,
- 'SELECT commit.* %Q FROM %T commit %Q %Q %Q %Q',
- $this->buildAuditSelect($conn_r),
+ 'SELECT commit.* FROM %T commit %Q %Q %Q %Q %Q',
$table->getTableName(),
$this->buildJoinClause($conn_r),
$this->buildWhereClause($conn_r),
+ $this->buildGroupClause($conn_r),
$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));
- if ($this->getAuditRequests()) {
- $this->loadAuditIds = ipull($data, 'audit_id');
- }
-
return $table->loadAllFromArray($data);
}
- private function buildAuditSelect($conn_r) {
- if ($this->getAuditRequests()) {
- return qsprintf(
- $conn_r,
- ', audit.id as audit_id');
- }
-
- return '';
- }
-
protected function willFilterPage(array $commits) {
$repository_ids = mpull($commits, 'getRepositoryID', 'getRepositoryID');
$repos = id(new PhabricatorRepositoryQuery())
@@ -239,9 +239,10 @@
}
}
- if ($this->getAuditRequests()) {
+ if ($this->needAuditRequests) {
+ $commit_phids = mpull($commits, 'getPHID');
$requests = id(new PhabricatorRepositoryAuditRequest())
- ->loadAllWhere('id IN (%Ld)', $this->loadAuditIds);
+ ->loadAllWhere('commitPHID IN (%Ls)', $commit_phids);
$requests = mgroup($requests, 'getCommitPHID');
foreach ($commits as $commit) {
@@ -259,35 +260,35 @@
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();
- if ($this->ids) {
+ if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'commit.id IN (%Ld)',
$this->ids);
}
- if ($this->phids) {
+ if ($this->phids !== null) {
$where[] = qsprintf(
$conn_r,
'commit.phid IN (%Ls)',
$this->phids);
}
- if ($this->repositoryIDs) {
+ if ($this->repositoryIDs !== null) {
$where[] = qsprintf(
$conn_r,
'commit.repositoryID IN (%Ld)',
$this->repositoryIDs);
}
- if ($this->authorPHIDs) {
+ if ($this->authorPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'commit.authorPHID IN (%Ls)',
$this->authorPHIDs);
}
- if ($this->identifiers) {
+ if ($this->identifiers !== null) {
$min_unqualified = PhabricatorRepository::MINIMUM_UNQUALIFIED_HASH;
$min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH;
@@ -378,14 +379,14 @@
$where[] = '('.implode(' OR ', $sql).')';
}
- if ($this->auditIDs) {
+ if ($this->auditIDs !== null) {
$where[] = qsprintf(
$conn_r,
'audit.id IN (%Ld)',
$this->auditIDs);
}
- if ($this->auditorPHIDs) {
+ if ($this->auditorPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'audit.auditorPHID IN (%Ls)',
@@ -454,14 +455,23 @@
}
}
+ private function buildGroupClause($conn_r) {
+ if ($this->shouldJoinAudits()) {
+ return qsprintf($conn_r, 'GROUP BY commit.id');
+ } else {
+ return '';
+ }
+ }
+
private function buildJoinClause($conn_r) {
$joins = array();
$audit_request = new PhabricatorRepositoryAuditRequest();
- if ($this->getAuditRequests()) {
+ if ($this->shouldJoinAudits()) {
$joins[] = qsprintf(
$conn_r,
- 'JOIN %T audit ON commit.phid = audit.commitPHID',
+ '%Q %T audit ON commit.phid = audit.commitPHID',
+ ($this->rowsMustHaveAudits() ? 'JOIN' : 'LEFT JOIN'),
$audit_request->getTableName());
}

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 20, 7:24 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7707699
Default Alt Text
D8879.id21061.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility