From b0862a81760d60d8de61545d65a4ce9efab2b90e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 18 May 2015 13:57:20 -0700 Subject: [PATCH] Filter pastes at the query level if we're unable to load their content Summary: Fixes T8230. I think I broke this in D12354 since I missed the fact that this could filter results, because it's constructed in a sort of weird way. Try to make the logic a little more clear, while retaining the relevant properties: - Pastes with unloadable content should be discarded; - the input order should be retained in the result set. Test Plan: Faked content misses and saw things work instead of fatal. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8230 Differential Revision: https://secure.phabricator.com/D12909 --- .../paste/query/PhabricatorPasteQuery.php | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index ae74978510..d5a5daf9e3 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -90,7 +90,7 @@ final class PhabricatorPasteQuery } if ($this->needContent) { - $this->loadContent($pastes); + $pastes = $this->loadContent($pastes); } return $pastes; @@ -205,29 +205,46 @@ final class PhabricatorPasteQuery $caches = $cache->getKeys($keys); $need_raw = array(); - foreach ($pastes as $key => $paste) { + $have_cache = array(); + foreach ($pastes as $paste) { $key = $this->getContentCacheKey($paste); if (isset($caches[$key])) { $paste->attachContent(phutil_safe_html($caches[$key])); + $have_cache[$paste->getPHID()] = true; } else { $need_raw[$key] = $paste; } } if (!$need_raw) { - return; + return $pastes; } $write_data = array(); - $need_raw = $this->loadRawContent($need_raw); - foreach ($need_raw as $key => $paste) { + $have_raw = $this->loadRawContent($need_raw); + $have_raw = mpull($have_raw, null, 'getPHID'); + foreach ($pastes as $key => $paste) { + $paste_phid = $paste->getPHID(); + if (isset($have_cache[$paste_phid])) { + continue; + } + + if (empty($have_raw[$paste_phid])) { + unset($pastes[$key]); + continue; + } + $content = $this->buildContent($paste); $paste->attachContent($content); $write_data[$this->getContentCacheKey($paste)] = (string)$content; } - $cache->setKeys($write_data); + if ($write_data) { + $cache->setKeys($write_data); + } + + return $pastes; } private function buildContent(PhabricatorPaste $paste) {