diff --git a/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php index b8392c474d..7e2f7127b1 100644 --- a/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php @@ -1,144 +1,153 @@ revisionPHIDs = $phids; return $this; } public function withObjectPHIDs(array $phids) { return $this->withRevisionPHIDs($phids); } protected function buildInlineCommentWhereClauseParts( AphrontDatabaseConnection $conn) { $where = array(); $alias = $this->getPrimaryTableAlias(); $where[] = qsprintf( $conn, 'changesetID IS NOT NULL'); return $where; } protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); $alias = $this->getPrimaryTableAlias(); if ($this->revisionPHIDs !== null) { $where[] = qsprintf( $conn, '%T.revisionPHID IN (%Ls)', $alias, $this->revisionPHIDs); } return $where; } protected function loadHiddenCommentIDs( $viewer_phid, array $comments) { $table = new DifferentialHiddenComment(); $conn = $table->establishConnection('r'); $rows = queryfx_all( $conn, 'SELECT commentID FROM %R WHERE userPHID = %s AND commentID IN (%Ld)', $table, $viewer_phid, mpull($comments, 'getID')); $id_map = ipull($rows, 'commentID'); $id_map = array_fuse($id_map); return $id_map; } + protected function newInlineContextFromCacheData(array $map) { + return PhabricatorDiffInlineCommentContext::newFromCacheData($map); + } + protected function newInlineContextMap(array $inlines) { $viewer = $this->getViewer(); - $map = array(); + $changeset_ids = mpull($inlines, 'getChangesetID'); + + $changesets = id(new DifferentialChangesetQuery()) + ->setViewer($viewer) + ->withIDs($changeset_ids) + ->needHunks(true) + ->execute(); + $changesets = mpull($changesets, null, 'getID'); + foreach ($inlines as $key => $inline) { - $changeset = id(new DifferentialChangesetQuery()) - ->setViewer($viewer) - ->withIDs(array($inline->getChangesetID())) - ->needHunks(true) - ->executeOne(); + $changeset = idx($changesets, $inline->getChangesetID()); + if (!$changeset) { continue; } $hunks = $changeset->getHunks(); $is_simple = (count($hunks) === 1) && ((int)head($hunks)->getOldOffset() <= 1) && ((int)head($hunks)->getNewOffset() <= 1); if (!$is_simple) { continue; } if ($inline->getIsNewFile()) { $vector = $changeset->getNewStatePathVector(); $filename = last($vector); $corpus = $changeset->makeNewFile(); } else { $vector = $changeset->getOldStatePathVector(); $filename = last($vector); $corpus = $changeset->makeOldFile(); } $corpus = phutil_split_lines($corpus); // Adjust the line number into a 0-based offset. $offset = $inline->getLineNumber(); $offset = $offset - 1; // Adjust the inclusive range length into a row count. $length = $inline->getLineLength(); $length = $length + 1; $head_min = max(0, $offset - 3); $head_max = $offset; $head_len = $head_max - $head_min; if ($head_len) { $head = array_slice($corpus, $head_min, $head_len, true); $head = $this->simplifyContext($head, true); } else { $head = array(); } $body = array_slice($corpus, $offset, $length, true); $tail = array_slice($corpus, $offset + $length, 3, true); $tail = $this->simplifyContext($tail, false); $context = id(new PhabricatorDiffInlineCommentContext()) ->setFilename($filename) ->setHeadLines($head) ->setBodyLines($body) ->setTailLines($tail); $map[$key] = $context; } return $map; } } diff --git a/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php b/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php index c395231808..01dc49a91e 100644 --- a/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php +++ b/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php @@ -1,73 +1,77 @@ commitPHIDs = $phids; return $this; } public function withObjectPHIDs(array $phids) { return $this->withCommitPHIDs($phids); } public function withPathIDs(array $path_ids) { $this->pathIDs = $path_ids; return $this; } protected function buildInlineCommentWhereClauseParts( AphrontDatabaseConnection $conn) { $where = array(); $alias = $this->getPrimaryTableAlias(); $where[] = qsprintf( $conn, '%T.pathID IS NOT NULL', $alias); return $where; } protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); $alias = $this->getPrimaryTableAlias(); if ($this->commitPHIDs !== null) { $where[] = qsprintf( $conn, '%T.commitPHID IN (%Ls)', $alias, $this->commitPHIDs); } if ($this->pathIDs !== null) { $where[] = qsprintf( $conn, '%T.pathID IN (%Ld)', $alias, $this->pathIDs); } return $where; } protected function loadHiddenCommentIDs( $viewer_phid, array $comments) { return array(); } protected function newInlineContextMap(array $inlines) { return array(); } + protected function newInlineContextFromCacheData(array $map) { + return PhabricatorDiffInlineCommentContext::newFromCacheData($map); + } + } diff --git a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php index 95f8473caf..f09d53092d 100644 --- a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php +++ b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php @@ -1,47 +1,67 @@ setFilename(idx($map, 'filename')); + $context->setHeadLines(idx($map, 'headLines')); + $context->setBodyLines(idx($map, 'bodyLines')); + $context->setTailLines(idx($map, 'tailLines')); + + return $context; + } + + public function newCacheDataMap() { + return array( + 'filename' => $this->getFilename(), + 'headLines' => $this->getHeadLines(), + 'bodyLines' => $this->getBodyLines(), + 'tailLines' => $this->getTailLines(), + ); + } + public function setFilename($filename) { $this->filename = $filename; return $this; } public function getFilename() { return $this->filename; } public function setHeadLines(array $head_lines) { $this->headLines = $head_lines; return $this; } public function getHeadLines() { return $this->headLines; } public function setBodyLines(array $body_lines) { $this->bodyLines = $body_lines; return $this; } public function getBodyLines() { return $this->bodyLines; } public function setTailLines(array $tail_lines) { $this->tailLines = $tail_lines; return $this; } public function getTailLines() { return $this->tailLines; } } diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php index 3d3bb6e106..76976b5929 100644 --- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php +++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php @@ -1,382 +1,392 @@ storageObject = clone $this->storageObject; } final public static function loadAndAttachVersionedDrafts( PhabricatorUser $viewer, array $inlines) { $viewer_phid = $viewer->getPHID(); if (!$viewer_phid) { return; } $inlines = mpull($inlines, null, 'getPHID'); $load = array(); foreach ($inlines as $key => $inline) { if (!$inline->getIsEditing()) { continue; } if ($inline->getAuthorPHID() !== $viewer_phid) { continue; } $load[$key] = $inline; } if (!$load) { return; } $drafts = PhabricatorVersionedDraft::loadDrafts( array_keys($load), $viewer_phid); $drafts = mpull($drafts, null, 'getObjectPHID'); foreach ($inlines as $inline) { $draft = idx($drafts, $inline->getPHID()); $inline->attachVersionedDraftForViewer($viewer, $draft); } } public function setSyntheticAuthor($synthetic_author) { $this->syntheticAuthor = $synthetic_author; return $this; } public function getSyntheticAuthor() { return $this->syntheticAuthor; } public function setStorageObject($storage_object) { $this->storageObject = $storage_object; return $this; } public function getStorageObject() { if (!$this->storageObject) { $this->storageObject = $this->newStorageObject(); } return $this->storageObject; } + public function getInlineCommentCacheFragment() { + $phid = $this->getPHID(); + + if ($phid === null) { + return null; + } + + return sprintf('inline(%s)', $phid); + } + abstract protected function newStorageObject(); abstract public function getControllerURI(); abstract public function setChangesetID($id); abstract public function getChangesetID(); abstract public function supportsHiding(); abstract public function isHidden(); public function isDraft() { return !$this->getTransactionPHID(); } public function getTransactionPHID() { return $this->getStorageObject()->getTransactionPHID(); } public function isCompatible(PhabricatorInlineComment $comment) { return ($this->getAuthorPHID() === $comment->getAuthorPHID()) && ($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) && ($this->getContent() === $comment->getContent()); } public function setIsGhost($is_ghost) { $this->isGhost = $is_ghost; return $this; } public function getIsGhost() { return $this->isGhost; } public function setContent($content) { $this->getStorageObject()->setContent($content); return $this; } public function getContent() { return $this->getStorageObject()->getContent(); } public function getID() { return $this->getStorageObject()->getID(); } public function getPHID() { return $this->getStorageObject()->getPHID(); } public function setIsNewFile($is_new) { $this->getStorageObject()->setIsNewFile($is_new); return $this; } public function getIsNewFile() { return $this->getStorageObject()->getIsNewFile(); } public function setFixedState($state) { $this->getStorageObject()->setFixedState($state); return $this; } public function setHasReplies($has_replies) { $this->getStorageObject()->setHasReplies($has_replies); return $this; } public function getHasReplies() { return $this->getStorageObject()->getHasReplies(); } public function getFixedState() { return $this->getStorageObject()->getFixedState(); } public function setLineNumber($number) { $this->getStorageObject()->setLineNumber($number); return $this; } public function getLineNumber() { return $this->getStorageObject()->getLineNumber(); } public function setLineLength($length) { $this->getStorageObject()->setLineLength($length); return $this; } public function getLineLength() { return $this->getStorageObject()->getLineLength(); } public function setAuthorPHID($phid) { $this->getStorageObject()->setAuthorPHID($phid); return $this; } public function getAuthorPHID() { return $this->getStorageObject()->getAuthorPHID(); } public function setReplyToCommentPHID($phid) { $this->getStorageObject()->setReplyToCommentPHID($phid); return $this; } public function getReplyToCommentPHID() { return $this->getStorageObject()->getReplyToCommentPHID(); } public function setIsDeleted($is_deleted) { $this->getStorageObject()->setIsDeleted($is_deleted); return $this; } public function getIsDeleted() { return $this->getStorageObject()->getIsDeleted(); } public function setIsEditing($is_editing) { $this->getStorageObject()->setAttribute('editing', (bool)$is_editing); return $this; } public function getIsEditing() { return (bool)$this->getStorageObject()->getAttribute('editing', false); } public function setDocumentEngineKey($engine_key) { $this->getStorageObject()->setAttribute('documentEngineKey', $engine_key); return $this; } public function getDocumentEngineKey() { return $this->getStorageObject()->getAttribute('documentEngineKey'); } public function setStartOffset($offset) { $this->getStorageObject()->setAttribute('startOffset', $offset); return $this; } public function getStartOffset() { return $this->getStorageObject()->getAttribute('startOffset'); } public function setEndOffset($offset) { $this->getStorageObject()->setAttribute('endOffset', $offset); return $this; } public function getEndOffset() { return $this->getStorageObject()->getAttribute('endOffset'); } public function getDateModified() { return $this->getStorageObject()->getDateModified(); } public function getDateCreated() { return $this->getStorageObject()->getDateCreated(); } public function openTransaction() { $this->getStorageObject()->openTransaction(); } public function saveTransaction() { $this->getStorageObject()->saveTransaction(); } public function save() { $this->getTransactionCommentForSave()->save(); return $this; } public function delete() { $this->getStorageObject()->delete(); return $this; } public function makeEphemeral() { $this->getStorageObject()->makeEphemeral(); return $this; } public function attachVersionedDraftForViewer( PhabricatorUser $viewer, PhabricatorVersionedDraft $draft = null) { $key = $viewer->getCacheFragment(); $this->versionedDrafts[$key] = $draft; return $this; } public function hasVersionedDraftForViewer(PhabricatorUser $viewer) { $key = $viewer->getCacheFragment(); return array_key_exists($key, $this->versionedDrafts); } public function getVersionedDraftForViewer(PhabricatorUser $viewer) { $key = $viewer->getCacheFragment(); if (!array_key_exists($key, $this->versionedDrafts)) { throw new Exception( pht( 'Versioned draft is not attached for user with fragment "%s".', $key)); } return $this->versionedDrafts[$key]; } public function isVoidComment(PhabricatorUser $viewer) { return $this->getContentStateForEdit($viewer)->isEmptyContentState(); } public function getContentStateForEdit(PhabricatorUser $viewer) { $state = $this->getContentState(); if ($this->hasVersionedDraftForViewer($viewer)) { $versioned_draft = $this->getVersionedDraftForViewer($viewer); if ($versioned_draft) { $storage_map = $versioned_draft->getProperty('inline.state'); if (is_array($storage_map)) { $state->readStorageMap($storage_map); } } } return $state; } protected function newContentState() { return new PhabricatorDiffInlineCommentContentState(); } public function newContentStateFromRequest(AphrontRequest $request) { return $this->newContentState()->readFromRequest($request); } public function getContentState() { $state = $this->newContentState(); $storage = $this->getStorageObject(); $storage_map = $storage->getAttribute('inline.state'); if (is_array($storage_map)) { $state->readStorageMap($storage_map); } $state->setContentText($this->getContent()); return $state; } public function setContentState(PhabricatorInlineCommentContentState $state) { $storage = $this->getStorageObject(); $storage_map = $state->newStorageMap(); $storage->setAttribute('inline.state', $storage_map); $this->setContent($state->getContentText()); return $this; } public function getInlineContext() { return $this->getStorageObject()->getInlineContext(); } /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ public function getMarkupFieldKey($field) { $content = $this->getMarkupText($field); return PhabricatorMarkupEngine::digestRemarkupContent($this, $content); } public function newMarkupEngine($field) { return PhabricatorMarkupEngine::newDifferentialMarkupEngine(); } public function getMarkupText($field) { return $this->getContent(); } public function didMarkupText($field, $output, PhutilMarkupEngine $engine) { return $output; } public function shouldUseMarkupCache($field) { return !$this->isDraft(); } } diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php index 85987c89a5..275a9a5aea 100644 --- a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -1,312 +1,382 @@ fixedStates = $states; return $this; } final public function needReplyToComments($need_reply_to) { $this->needReplyToComments = $need_reply_to; return $this; } final public function withPublishableComments($with_publishable) { $this->publishableComments = $with_publishable; return $this; } final public function withPublishedComments($with_published) { $this->publishedComments = $with_published; return $this; } final public function needHidden($need_hidden) { $this->needHidden = $need_hidden; return $this; } final public function needInlineContext($need_context) { $this->needInlineContext = $need_context; return $this; } final public function needAppliedDrafts($need_applied) { $this->needAppliedDrafts = $need_applied; return $this; } protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); $alias = $this->getPrimaryTableAlias(); foreach ($this->buildInlineCommentWhereClauseParts($conn) as $part) { $where[] = $part; } if ($this->fixedStates !== null) { $where[] = qsprintf( $conn, '%T.fixedState IN (%Ls)', $alias, $this->fixedStates); } $show_published = false; $show_publishable = false; if ($this->publishableComments !== null) { if (!$this->publishableComments) { throw new Exception( pht( 'Querying for comments that are "not publishable" is '. 'not supported.')); } $show_publishable = true; } if ($this->publishedComments !== null) { if (!$this->publishedComments) { throw new Exception( pht( 'Querying for comments that are "not published" is '. 'not supported.')); } $show_published = true; } if ($show_publishable || $show_published) { $clauses = array(); if ($show_published) { $clauses[] = qsprintf( $conn, '%T.transactionPHID IS NOT NULL', $alias); } if ($show_publishable) { $viewer = $this->getViewer(); $viewer_phid = $viewer->getPHID(); // If the viewer has a PHID, unpublished comments they authored and // have not deleted are visible. if ($viewer_phid) { $clauses[] = qsprintf( $conn, '%T.authorPHID = %s AND %T.isDeleted = 0 AND %T.transactionPHID IS NULL ', $alias, $viewer_phid, $alias, $alias); } } // We can end up with a known-empty query if we (for example) query for // publishable comments and the viewer is logged-out. if (!$clauses) { throw new PhabricatorEmptyQueryException(); } $where[] = qsprintf( $conn, '%LO', $clauses); } return $where; } protected function willFilterPage(array $inlines) { $viewer = $this->getViewer(); if ($this->needReplyToComments) { $reply_phids = array(); foreach ($inlines as $inline) { $reply_phid = $inline->getReplyToCommentPHID(); if ($reply_phid) { $reply_phids[] = $reply_phid; } } if ($reply_phids) { $reply_inlines = newv(get_class($this), array()) ->setViewer($this->getViewer()) ->setParentQuery($this) ->withPHIDs($reply_phids) ->execute(); $reply_inlines = mpull($reply_inlines, null, 'getPHID'); } else { $reply_inlines = array(); } foreach ($inlines as $key => $inline) { $reply_phid = $inline->getReplyToCommentPHID(); if (!$reply_phid) { $inline->attachReplyToComment(null); continue; } $reply = idx($reply_inlines, $reply_phid); if (!$reply) { $this->didRejectResult($inline); unset($inlines[$key]); continue; } $inline->attachReplyToComment($reply); } } if (!$inlines) { return $inlines; } $need_drafts = $this->needAppliedDrafts; $drop_void = $this->publishableComments; $convert_objects = ($need_drafts || $drop_void); if ($convert_objects) { $inlines = mpull($inlines, 'newInlineCommentObject'); PhabricatorInlineComment::loadAndAttachVersionedDrafts( $viewer, $inlines); if ($need_drafts) { // Don't count void inlines when considering draft state. foreach ($inlines as $key => $inline) { if ($inline->isVoidComment($viewer)) { $this->didRejectResult($inline->getStorageObject()); unset($inlines[$key]); continue; } // For other inlines: if they have a nonempty draft state, set their // content to the draft state content. We want to submit the comment // as it is currently shown to the user, not as it was stored the last // time they clicked "Save". $draft_state = $inline->getContentStateForEdit($viewer); if (!$draft_state->isEmptyContentState()) { $inline->setContentState($draft_state); } } } // If we're loading publishable comments, discard any comments that are // empty. if ($drop_void) { foreach ($inlines as $key => $inline) { if ($inline->getTransactionPHID()) { continue; } if ($inline->isVoidComment($viewer)) { $this->didRejectResult($inline->getStorageObject()); unset($inlines[$key]); continue; } } } $inlines = mpull($inlines, 'getStorageObject'); } return $inlines; } protected function didFilterPage(array $inlines) { $viewer = $this->getViewer(); if ($this->needHidden) { $viewer_phid = $viewer->getPHID(); if ($viewer_phid) { $hidden = $this->loadHiddenCommentIDs( $viewer_phid, $inlines); } else { $hidden = array(); } foreach ($inlines as $inline) { $inline->attachIsHidden(isset($hidden[$inline->getID()])); } } if ($this->needInlineContext) { $need_context = array(); foreach ($inlines as $inline) { $object = $inline->newInlineCommentObject(); if ($object->getDocumentEngineKey() !== null) { $inline->attachInlineContext(null); continue; } $need_context[] = $inline; } if ($need_context) { - $context_map = $this->newInlineContextMap($need_context); + $this->loadInlineCommentContext($need_context); + } + } + + return $inlines; + } + + private function loadInlineCommentContext(array $inlines) { + $cache_keys = array(); + foreach ($inlines as $key => $inline) { + $object = $inline->newInlineCommentObject(); + $fragment = $object->getInlineCommentCacheFragment(); + + if ($fragment === null) { + continue; + } + + $cache_keys[$key] = sprintf( + '%s.context(v%d)', + $fragment, + self::INLINE_CONTEXT_CACHE_VERSION); + } + + $cache = PhabricatorCaches::getMutableStructureCache(); + + $cache_map = $cache->getKeys($cache_keys); + + $context_map = array(); + $need_construct = array(); + + foreach ($inlines as $key => $inline) { + $cache_key = idx($cache_keys, $key); - foreach ($need_context as $key => $inline) { - $inline->attachInlineContext(idx($context_map, $key)); + if ($cache_key !== null) { + if (array_key_exists($cache_key, $cache_map)) { + $cache_data = $cache_map[$cache_key]; + $context_map[$key] = $this->newInlineContextFromCacheData( + $cache_data); + continue; } } + $need_construct[$key] = $inline; } - return $inlines; + if ($need_construct) { + $construct_map = $this->newInlineContextMap($need_construct); + + $write_map = array(); + foreach ($construct_map as $key => $context) { + if ($context === null) { + $cache_data = $context; + } else { + $cache_data = $this->newCacheDataFromInlineContext($context); + } + + $cache_key = idx($cache_keys, $key); + if ($cache_key !== null) { + $write_map[$cache_key] = $cache_data; + } + } + + if ($write_map) { + $cache->setKeys($write_map); + } + + $context_map += $construct_map; + } + + foreach ($inlines as $key => $inline) { + $inline->attachInlineContext(idx($context_map, $key)); + } + } + + protected function newCacheDataFromInlineContext( + PhabricatorInlineCommentContext $context) { + return $context->newCacheDataMap(); } final protected function simplifyContext(array $lines, $is_head) { // We want to provide the smallest amount of context we can while still // being useful, since the actual code is visible nearby and showing a // ton of context is silly. // Examine each line until we find one that looks "useful" (not just // whitespace or a single bracket). Once we find a useful piece of context // to anchor the text, discard the rest of the lines beyond it. if ($is_head) { $lines = array_reverse($lines, true); } $saw_context = false; foreach ($lines as $key => $line) { if ($saw_context) { unset($lines[$key]); continue; } $saw_context = (strlen(trim($line)) > 3); } if ($is_head) { $lines = array_reverse($lines, true); } return $lines; } }