diff --git a/src/applications/pholio/engine/PholioMockTimelineEngine.php b/src/applications/pholio/engine/PholioMockTimelineEngine.php --- a/src/applications/pholio/engine/PholioMockTimelineEngine.php +++ b/src/applications/pholio/engine/PholioMockTimelineEngine.php @@ -7,10 +7,13 @@ $viewer = $this->getViewer(); $object = $this->getObject(); - PholioMockQuery::loadImages( - $viewer, - array($object), - $need_inline_comments = true); + $images = id(new PholioImageQuery()) + ->setViewer($viewer) + ->withMocks(array($object)) + ->needInlineComments(true) + ->execute(); + + $object->attachImages($images); return id(new PholioTransactionView()) ->setMock($object); diff --git a/src/applications/pholio/query/PholioImageQuery.php b/src/applications/pholio/query/PholioImageQuery.php --- a/src/applications/pholio/query/PholioImageQuery.php +++ b/src/applications/pholio/query/PholioImageQuery.php @@ -6,9 +6,9 @@ private $ids; private $phids; private $mockPHIDs; + private $mocks; private $needInlineComments; - private $mockCache = array(); public function withIDs(array $ids) { $this->ids = $ids; @@ -20,6 +20,16 @@ return $this; } + public function withMocks(array $mocks) { + assert_instances_of($mocks, 'PholioMock'); + + $mocks = mpull($mocks, null, 'getPHID'); + $this->mocks = $mocks; + $this->mockPHIDs = array_keys($mocks); + + return $this; + } + public function withMockPHIDs(array $mock_phids) { $this->mockPHIDs = $mock_phids; return $this; @@ -30,14 +40,6 @@ return $this; } - public function setMockCache($mock_cache) { - $this->mockCache = $mock_cache; - return $this; - } - public function getMockCache() { - return $this->mockCache; - } - public function newResultObject() { return new PholioImage(); } @@ -76,26 +78,40 @@ protected function willFilterPage(array $images) { assert_instances_of($images, 'PholioImage'); - if ($this->getMockCache()) { - $mocks = $this->getMockCache(); - } else { - $mock_phids = mpull($images, 'getMockPHID'); + $mock_phids = array(); + foreach ($images as $image) { + if (!$image->hasMock()) { + continue; + } + + $mock_phids[] = $image->getMockPHID(); + } + + if ($mock_phids) { + if ($this->mocks) { + $mocks = $this->mocks; + } else { + $mocks = id(new PholioMockQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($mock_phids) + ->execute(); + } - // DO NOT set needImages to true; recursion results! - $mocks = id(new PholioMockQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($mock_phids) - ->execute(); $mocks = mpull($mocks, null, 'getPHID'); - } - foreach ($images as $index => $image) { - $mock = idx($mocks, $image->getMockPHID()); - if ($mock) { + foreach ($images as $key => $image) { + if (!$image->hasMock()) { + continue; + } + + $mock = idx($mocks, $image->getMockPHID()); + if (!$mock) { + unset($images[$key]); + $this->didRejectResult($image); + continue; + } + $image->attachMock($mock); - } else { - // mock is missing or we can't see it - unset($images[$index]); } } diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -58,21 +58,14 @@ } protected function loadPage() { - $mocks = $this->loadStandardPage($this->newResultObject()); - - if ($mocks && $this->needImages) { - self::loadImages($this->getViewer(), $mocks, $this->needInlineComments); - } - - if ($mocks && $this->needCoverFiles) { - $this->loadCoverFiles($mocks); - } - - if ($mocks && $this->needTokenCounts) { - $this->loadTokenCounts($mocks); + if ($this->needInlineComments && !$this->needImages) { + throw new Exception( + pht( + 'You can not query for inline comments without also querying for '. + 'images.')); } - return $mocks; + return $this->loadStandardPage(new PholioMock()); } protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { @@ -109,58 +102,53 @@ return $where; } - public static function loadImages( - PhabricatorUser $viewer, - array $mocks, - $need_inline_comments) { - assert_instances_of($mocks, 'PholioMock'); - - $mock_map = mpull($mocks, null, 'getPHID'); - $all_images = id(new PholioImageQuery()) - ->setViewer($viewer) - ->setMockCache($mock_map) - ->withMockPHIDs(array_keys($mock_map)) - ->needInlineComments($need_inline_comments) - ->execute(); - - $image_groups = mgroup($all_images, 'getMockPHID'); - - foreach ($mocks as $mock) { - $mock_images = idx($image_groups, $mock->getPHID(), array()); - $mock->attachImages($mock_images); - } - } + protected function didFilterPage(array $mocks) { + $viewer = $this->getViewer(); - private function loadCoverFiles(array $mocks) { - assert_instances_of($mocks, 'PholioMock'); - $cover_file_phids = mpull($mocks, 'getCoverPHID'); - $cover_files = id(new PhabricatorFileQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($cover_file_phids) - ->execute(); + if ($this->needImages) { + $images = id(new PholioImageQuery()) + ->setViewer($viewer) + ->withMocks($mocks) + ->needInlineComments($this->needInlineComments) + ->execute(); - $cover_files = mpull($cover_files, null, 'getPHID'); - - foreach ($mocks as $mock) { - $file = idx($cover_files, $mock->getCoverPHID()); - if (!$file) { - $file = PhabricatorFile::loadBuiltin($this->getViewer(), 'missing.png'); + $image_groups = mgroup($images, 'getMockPHID'); + foreach ($mocks as $mock) { + $images = idx($image_groups, $mock->getPHID(), array()); + $mock->attachImages($images); } - $mock->attachCoverFile($file); } - } - private function loadTokenCounts(array $mocks) { - assert_instances_of($mocks, 'PholioMock'); + if ($this->needCoverFiles) { + $cover_files = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(mpull($mocks, 'getCoverPHID')) + ->execute(); + $cover_files = mpull($cover_files, null, 'getPHID'); + + foreach ($mocks as $mock) { + $file = idx($cover_files, $mock->getCoverPHID()); + if (!$file) { + $file = PhabricatorFile::loadBuiltin( + $viewer, + 'missing.png'); + } + $mock->attachCoverFile($file); + } + } - $phids = mpull($mocks, 'getPHID'); - $counts = id(new PhabricatorTokenCountQuery()) - ->withObjectPHIDs($phids) - ->execute(); + if ($this->needTokenCounts) { + $counts = id(new PhabricatorTokenCountQuery()) + ->withObjectPHIDs(mpull($mocks, 'getPHID')) + ->execute(); - foreach ($mocks as $mock) { - $mock->attachTokenCount(idx($counts, $mock->getPHID(), 0)); + foreach ($mocks as $mock) { + $token_count = idx($counts, $mock->getPHID(), 0); + $mock->attachTokenCount($token_count); + } } + + return $mocks; } public function getQueryApplicationClass() { diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -110,7 +110,7 @@ ); } - $ids = mpull($mock->getActiveImages(), 'getID'); + $ids = mpull($mock->getActiveImages(), null, 'getID'); if ($this->imageID && isset($ids[$this->imageID])) { $selected_id = $this->imageID; } else {