Make Changeset ID for render cache explicit

Summary:
DifferentialChangesetParser currently takes the Changeset object to mean a bunch
of different and mutually conflicting things implicitly:

- Changeset ID is used to access the render cache.
- Changeset ID is also used to tell the ajax endpoint what to render when
clicking "show more".
- Changeset object has the actual changes.
- Changeset ID and "oldChangesetID" are used to choose where to show inline
comments and how to attach new ones.

This indirectly causes a bunch of problems, like T141 and T132. Move toward
making all these separate things explicit. I want to have the changeset object
only mean the actual changes to display.

Test Plan:
Looked at changesets and verified the render cache was accessed correctly (and
not accessed in other cases).

Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 228

epriestley 15 years ago af06bfb1 85b09c5c

+37 -6

2 changed files

expand all

src

applications

differential

controller

changesetview

DifferentialChangesetViewController.php

parser

changeset

DifferentialChangesetParser.php

src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php

··· 48 48 $right_new = true; 49 49 $left_source = $right->getID(); 50 50 $left_new = false; 51 + 52 + $render_cache_key = $right->getID(); 51 53 } else if ($vs == -1) { 52 54 $right = null; 53 55 $left = $changeset; ··· 56 58 $right_new = false; 57 59 $left_source = $left->getID(); 58 60 $left_new = true; 61 + 62 + $render_cache_key = null; 59 63 } else { 60 64 $right = $changeset; 61 65 $left = $vs_changeset; ··· 64 68 $right_new = true; 65 69 $left_source = $left->getID(); 66 70 $left_new = true; 71 + 72 + $render_cache_key = null; 67 73 } 68 74 69 75 if ($left) { ··· 130 136 131 137 $parser = new DifferentialChangesetParser(); 132 138 $parser->setChangeset($changeset); 139 + $parser->setRenderCacheKey($render_cache_key); 133 140 $parser->setRightSideCommentMapping($right_source, $right_new); 134 141 $parser->setLeftSideCommentMapping($left_source, $left_new); 135 142 $parser->setWhitespaceMode($request->getStr('whitespace'));

+30 -6

src/applications/differential/parser/changeset/DifferentialChangesetParser.php

··· 42 42 protected $oldChangesetID = null; 43 43 protected $noHighlight; 44 44 45 + protected $renderCacheKey = null; 46 + 45 47 private $handles; 46 48 private $user; 47 49 ··· 64 66 65 67 public function setLeftSideCommentMapping($id, $is_new) { 66 68 69 + } 70 + 71 + /** 72 + * Set a key for identifying this changeset in the render cache. If set, the 73 + * parser will attempt to use the changeset render cache, which can improve 74 + * performance for frequently-viewed changesets. 75 + * 76 + * By default, there is no render cache key and parsers do not use the cache. 77 + * This is appropriate for rarely-viewed changesets. 78 + * 79 + * NOTE: Currently, this key must be a valid Differential Changeset ID. 80 + * 81 + * @param string Key for identifying this changeset in the render cache. 82 + * @return this 83 + */ 84 + public function setRenderCacheKey($key) { 85 + $this->renderCacheKey = $key; 86 + return $this; 87 + } 88 + 89 + private function getRenderCacheKey() { 90 + return $this->renderCacheKey; 67 91 } 68 92 69 93 public function setChangeset($changeset) { ··· 427 451 } 428 452 429 453 public function loadCache() { 430 - if (!$this->changesetID) { 454 + $render_cache_key = $this->getRenderCacheKey(); 455 + if (!$render_cache_key) { 431 456 return false; 432 457 } 433 458 ··· 439 464 $conn_r, 440 465 'SELECT * FROM %T WHERE id = %d', 441 466 $changeset->getTableName().'_parse_cache', 442 - $this->changesetID); 467 + $render_cache_key); 443 468 444 469 if (!$data) { 445 470 return false; ··· 488 513 } 489 514 490 515 public function saveCache() { 491 - if (!$this->changesetID) { 516 + $render_cache_key = $this->getRenderCacheKey(); 517 + if (!$render_cache_key) { 492 518 return false; 493 519 } 494 520 ··· 516 542 'INSERT INTO %T (id, cache) VALUES (%d, %s) 517 543 ON DUPLICATE KEY UPDATE cache = VALUES(cache)', 518 544 $changeset->getTableName().'_parse_cache', 519 - $this->changesetID, 545 + $render_cache_key, 520 546 $cache); 521 547 } catch (AphrontQueryException $ex) { 522 548 // TODO: uhoh ··· 704 730 PhabricatorEnv::getEnvConfig('pygments.enabled')); 705 731 706 732 $this->tryCacheStuff(); 707 - 708 - $changeset_id = $this->changesetID; 709 733 710 734 $feedback_mask = array(); 711 735

Configure Feed

Configure Feed