-11

conf/default.conf.php

reviewed

··· 856 856 'image/vnd.microsoft.icon' => true, 857 857 ), 858 858 859 859 - // Phabricator can proxy images from other servers so you can paste the URI 860 860 - // to a funny picture of a cat into the comment box and have it show up as an 861 861 - // image. However, this means the webserver Phabricator is running on will 862 862 - // make HTTP requests to arbitrary URIs. If the server has access to internal 863 863 - // resources, this could be a security risk. You should only enable it if you 864 864 - // are installed entirely a VPN and VPN access is required to access 865 865 - // Phabricator, or if the webserver has no special access to anything. If 866 866 - // unsure, it is safer to leave this disabled. 867 867 - 'files.enable-proxy' => false, 868 868 - 869 869 - 870 859 // -- Storage --------------------------------------------------------------- // 871 860 872 861 // Phabricator allows users to upload files, and can keep them in various

+1

resources/sql/patches/dropfileproxyimage.sql

reviewed

··· 1 1 + DROP TABLE {$NAMESPACE}_file.file_proxyimage;

+42

resources/sql/patches/liskcounters.php

reviewed

··· 1 1 + <?php 2 2 + 3 3 + // Switch PhabricatorWorkerActiveTask from autoincrement IDs to counter IDs. 4 4 + // Set the initial counter ID to be larger than any known task ID. 5 5 + 6 6 + $active_table = new PhabricatorWorkerActiveTask(); 7 7 + $archive_table = new PhabricatorWorkerArchiveTask(); 8 8 + 9 9 + $conn_w = $active_table->establishConnection('w'); 10 10 + 11 11 + $active_auto = head(queryfx_one( 12 12 + $conn_w, 13 13 + 'SELECT auto_increment FROM information_schema.tables 14 14 + WHERE table_name = %s 15 15 + AND table_schema = DATABASE()', 16 16 + $active_table->getTableName())); 17 17 + 18 18 + $active_max = head(queryfx_one( 19 19 + $conn_w, 20 20 + 'SELECT MAX(id) FROM %T', 21 21 + $active_table->getTableName())); 22 22 + 23 23 + $archive_max = head(queryfx_one( 24 24 + $conn_w, 25 25 + 'SELECT MAX(id) FROM %T', 26 26 + $archive_table->getTableName())); 27 27 + 28 28 + $initial_counter = max((int)$active_auto, (int)$active_max, (int)$archive_max); 29 29 + 30 30 + queryfx( 31 31 + $conn_w, 32 32 + 'INSERT IGNORE INTO %T (counterName, counterValue) 33 33 + VALUES (%s, %d)', 34 34 + LiskDAO::COUNTER_TABLE_NAME, 35 35 + $active_table->getTableName(), 36 36 + $initial_counter + 1); 37 37 + 38 38 + // Drop AUTO_INCREMENT from the ID column. 39 39 + queryfx( 40 40 + $conn_w, 41 41 + 'ALTER TABLE %T CHANGE id id INT UNSIGNED NOT NULL', 42 42 + $active_table->getTableName());

+9

resources/sql/patches/liskcounters.sql

reviewed

··· 1 1 + CREATE TABLE `{$NAMESPACE}_harbormaster`.`lisk_counter` ( 2 2 + counterName VARCHAR(64) COLLATE utf8_bin PRIMARY KEY, 3 3 + counterValue BIGINT UNSIGNED NOT NULL 4 4 + ) ENGINE=InnoDB DEFAULT CHARSET=utf8; 5 5 + 6 6 + CREATE TABLE `{$NAMESPACE}_worker`.`lisk_counter` ( 7 7 + counterName VARCHAR(64) COLLATE utf8_bin PRIMARY KEY, 8 8 + counterValue BIGINT UNSIGNED NOT NULL 9 9 + ) ENGINE=InnoDB DEFAULT CHARSET=utf8;

+2 -6

src/__phutil_library_map__.php

reviewed

··· 608 608 'PhabricatorBarePageView' => 'view/page/PhabricatorBarePageView.php', 609 609 'PhabricatorBaseEnglishTranslation' => 'infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php', 610 610 'PhabricatorBuiltinPatchList' => 'infrastructure/storage/patch/PhabricatorBuiltinPatchList.php', 611 611 + 'PhabricatorButtonsExample' => 'applications/uiexample/examples/PhabricatorButtonsExample.php', 611 612 'PhabricatorCacheDAO' => 'applications/cache/storage/PhabricatorCacheDAO.php', 612 613 'PhabricatorCalendarBrowseController' => 'applications/calendar/controller/PhabricatorCalendarBrowseController.php', 613 614 'PhabricatorCalendarController' => 'applications/calendar/controller/PhabricatorCalendarController.php', ··· 744 745 'PhabricatorFileLinkListView' => 'view/layout/PhabricatorFileLinkListView.php', 745 746 'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php', 746 747 'PhabricatorFileListController' => 'applications/files/controller/PhabricatorFileListController.php', 747 747 - 'PhabricatorFileProxyController' => 'applications/files/controller/PhabricatorFileProxyController.php', 748 748 - 'PhabricatorFileProxyImage' => 'applications/files/storage/PhabricatorFileProxyImage.php', 749 748 'PhabricatorFileQuery' => 'applications/files/query/PhabricatorFileQuery.php', 750 749 'PhabricatorFileShortcutController' => 'applications/files/controller/PhabricatorFileShortcutController.php', 751 750 'PhabricatorFileSideNavView' => 'applications/files/view/PhabricatorFileSideNavView.php', ··· 987 986 'PhabricatorRemarkupRuleObjectName' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php', 988 987 'PhabricatorRemarkupRulePaste' => 'infrastructure/markup/rule/PhabricatorRemarkupRulePaste.php', 989 988 'PhabricatorRemarkupRulePhriction' => 'infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php', 990 990 - 'PhabricatorRemarkupRuleProxyImage' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php', 991 989 'PhabricatorRemarkupRuleYoutube' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php', 992 990 'PhabricatorRepository' => 'applications/repository/storage/PhabricatorRepository.php', 993 991 'PhabricatorRepositoryArcanistProject' => 'applications/repository/storage/PhabricatorRepositoryArcanistProject.php', ··· 1824 1822 'PhabricatorBarePageView' => 'AphrontPageView', 1825 1823 'PhabricatorBaseEnglishTranslation' => 'PhabricatorTranslation', 1826 1824 'PhabricatorBuiltinPatchList' => 'PhabricatorSQLPatchList', 1825 1825 + 'PhabricatorButtonsExample' => 'PhabricatorUIExample', 1827 1826 'PhabricatorCacheDAO' => 'PhabricatorLiskDAO', 1828 1827 'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController', 1829 1828 'PhabricatorCalendarController' => 'PhabricatorController', ··· 1955 1954 'PhabricatorFileLinkListView' => 'AphrontView', 1956 1955 'PhabricatorFileLinkView' => 'AphrontView', 1957 1956 'PhabricatorFileListController' => 'PhabricatorFileController', 1958 1958 - 'PhabricatorFileProxyController' => 'PhabricatorFileController', 1959 1959 - 'PhabricatorFileProxyImage' => 'PhabricatorFileDAO', 1960 1957 'PhabricatorFileQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 1961 1958 'PhabricatorFileShortcutController' => 'PhabricatorFileController', 1962 1959 'PhabricatorFileSideNavView' => 'AphrontView', ··· 2172 2169 'PhabricatorRemarkupRuleObjectName' => 'PhutilRemarkupRule', 2173 2170 'PhabricatorRemarkupRulePaste' => 'PhabricatorRemarkupRuleObjectName', 2174 2171 'PhabricatorRemarkupRulePhriction' => 'PhutilRemarkupRule', 2175 2175 - 'PhabricatorRemarkupRuleProxyImage' => 'PhutilRemarkupRule', 2176 2172 'PhabricatorRemarkupRuleYoutube' => 'PhutilRemarkupRule', 2177 2173 'PhabricatorRepository' => 'PhabricatorRepositoryDAO', 2178 2174 'PhabricatorRepositoryArcanistProject' => 'PhabricatorRepositoryDAO',

+25

src/applications/differential/field/specification/DifferentialFieldSpecification.php

reviewed

··· 383 383 return $key; 384 384 } 385 385 386 386 + /* -( Extending the Search Interface )------------------------------------ */ 387 387 + 388 388 + /** 389 389 + * @task search 390 390 + */ 391 391 + public function shouldAddToSearchIndex() { 392 392 + return false; 393 393 + } 394 394 + 395 395 + /** 396 396 + * @task search 397 397 + */ 398 398 + public function getValueForSearchIndex() { 399 399 + throw new DifferentialFieldSpecificationIncompleteException($this); 400 400 + } 401 401 + 402 402 + /** 403 403 + * NOTE: Keys *must be* 4 characters for 404 404 + * @{class:PhabricatorSearchEngineMySQL}. 405 405 + * 406 406 + * @task search 407 407 + */ 408 408 + public function getKeyForSearchIndex() { 409 409 + throw new DifferentialFieldSpecificationIncompleteException($this); 410 410 + } 386 411 387 412 /* -( Extending Commit Messages )------------------------------------------ */ 388 413

+12

src/applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php

reviewed

··· 95 95 return $value; 96 96 } 97 97 98 98 + public function shouldAddToSearchIndex() { 99 99 + return true; 100 100 + } 101 101 + 102 102 + public function getValueForSearchIndex() { 103 103 + return $this->value; 104 104 + } 105 105 + 106 106 + public function getKeyForSearchIndex() { 107 107 + return 'rpln'; 108 108 + } 109 109 + 98 110 }

+12

src/applications/differential/field/specification/DifferentialSummaryFieldSpecification.php

reviewed

··· 70 70 return $this->summary; 71 71 } 72 72 73 73 + public function shouldAddToSearchIndex() { 74 74 + return true; 75 75 + } 76 76 + 77 77 + public function getValueForSearchIndex() { 78 78 + return $this->summary; 79 79 + } 80 80 + 81 81 + public function getKeyForSearchIndex() { 82 82 + return PhabricatorSearchField::FIELD_BODY; 83 83 + } 84 84 + 73 85 }

+14

src/applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php

reviewed

··· 103 103 return "TEST PLAN\n".preg_replace('/^/m', ' ', $this->plan); 104 104 } 105 105 106 106 + public function shouldAddToSearchIndex() { 107 107 + return true; 108 108 + } 109 109 + 110 110 + public function getValueForSearchIndex() { 111 111 + return $this->plan; 112 112 + } 113 113 + 114 114 + public function getKeyForSearchIndex() { 115 115 + return 'tpln'; 116 116 + } 117 117 + 106 118 private function isRequired() { 107 119 return PhabricatorEnv::getEnvConfig('differential.require-test-plan-field'); 108 120 } 121 121 + 122 122 + 109 123 110 124 }

-54

src/applications/files/controller/PhabricatorFileProxyController.php

reviewed

··· 1 1 - <?php 2 2 - 3 3 - final class PhabricatorFileProxyController extends PhabricatorFileController { 4 4 - 5 5 - private $uri; 6 6 - 7 7 - public function processRequest() { 8 8 - 9 9 - if (!PhabricatorEnv::getEnvConfig('files.enable-proxy')) { 10 10 - return new Aphront400Response(); 11 11 - } 12 12 - 13 13 - $request = $this->getRequest(); 14 14 - $uri = $request->getStr('uri'); 15 15 - 16 16 - $proxy = id(new PhabricatorFileProxyImage())->loadOneWhere( 17 17 - 'uri = %s', 18 18 - $uri); 19 19 - 20 20 - if (!$proxy) { 21 21 - // This write is fine to skip CSRF checks for, we're just building a 22 22 - // cache of some remote image. 23 23 - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 24 24 - 25 25 - $file = PhabricatorFile::newFromFileDownload( 26 26 - $uri, 27 27 - nonempty(basename($uri), 'proxied-file')); 28 28 - if ($file) { 29 29 - $proxy = new PhabricatorFileProxyImage(); 30 30 - $proxy->setURI($uri); 31 31 - $proxy->setFilePHID($file->getPHID()); 32 32 - $proxy->save(); 33 33 - } 34 34 - 35 35 - unset($unguarded); 36 36 - } 37 37 - 38 38 - if ($proxy) { 39 39 - $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', 40 40 - $proxy->getFilePHID()); 41 41 - if ($file) { 42 42 - $view_uri = $file->getBestURI(); 43 43 - } else { 44 44 - $bad_phid = $proxy->getFilePHID(); 45 45 - throw new Exception( 46 46 - "Unable to load file with phid {$bad_phid}." 47 47 - ); 48 48 - } 49 49 - return id(new AphrontRedirectResponse())->setURI($view_uri); 50 50 - } 51 51 - 52 52 - return new Aphront400Response(); 53 53 - } 54 54 - }

-18

src/applications/files/storage/PhabricatorFileProxyImage.php

reviewed

··· 1 1 - <?php 2 2 - 3 3 - final class PhabricatorFileProxyImage extends PhabricatorFileDAO { 4 4 - 5 5 - protected $uri; 6 6 - protected $filePHID; 7 7 - 8 8 - public function getConfiguration() { 9 9 - return array( 10 10 - self::CONFIG_TIMESTAMPS => false, 11 11 - ) + parent::getConfiguration(); 12 12 - } 13 13 - 14 14 - static public function getProxyImageURI($uri) { 15 15 - return '/file/proxy/?uri='.phutil_escape_uri($uri); 16 16 - } 17 17 - } 18 18 -

+1

src/applications/maniphest/controller/ManiphestTaskListController.php

reviewed

··· 543 543 $owner_phids, 544 544 $author_phids, 545 545 $project_group_phids, 546 546 + $any_project_phids, 546 547 array_mergev(mpull($data, 'getProjectPHIDs'))); 547 548 $handles = id(new PhabricatorObjectHandleData($handle_phids)) 548 549 ->loadHandles();

-1

src/applications/search/constants/PhabricatorSearchField.php

reviewed

··· 7 7 8 8 const FIELD_TITLE = 'titl'; 9 9 const FIELD_BODY = 'body'; 10 10 - const FIELD_TEST_PLAN = 'tpln'; 11 10 const FIELD_COMMENT = 'cmnt'; 12 11 13 12 }

+17 -6

src/applications/search/index/indexer/PhabricatorSearchDifferentialIndexer.php

reviewed

··· 14 14 $doc->setDocumentCreated($rev->getDateCreated()); 15 15 $doc->setDocumentModified($rev->getDateModified()); 16 16 17 17 - $doc->addField( 18 18 - PhabricatorSearchField::FIELD_BODY, 19 19 - $rev->getSummary()); 20 20 - $doc->addField( 21 21 - PhabricatorSearchField::FIELD_TEST_PLAN, 22 22 - $rev->getTestPlan()); 17 17 + $aux_fields = DifferentialFieldSelector::newSelector() 18 18 + ->getFieldSpecifications(); 19 19 + foreach ($aux_fields as $key => $aux_field) { 20 20 + if (!$aux_field->shouldAddToSearchIndex()) { 21 21 + unset($aux_fields[$key]); 22 22 + } 23 23 + } 24 24 + 25 25 + $aux_fields = DifferentialAuxiliaryField::loadFromStorage( 26 26 + $rev, 27 27 + $aux_fields); 28 28 + foreach ($aux_fields as $aux_field) { 29 29 + $doc->addField( 30 30 + $aux_field->getKeyForSearchIndex(), 31 31 + $aux_field->getValueForSearchIndex() 32 32 + ); 33 33 + } 23 34 24 35 $doc->addRelationship( 25 36 PhabricatorSearchRelationship::RELATIONSHIP_AUTHOR,

+45

src/applications/uiexample/examples/PhabricatorButtonsExample.php

reviewed

··· 1 1 + <?php 2 2 + 3 3 + final class PhabricatorButtonsExample extends PhabricatorUIExample { 4 4 + 5 5 + public function getName() { 6 6 + return 'Buttons'; 7 7 + } 8 8 + 9 9 + public function getDescription() { 10 10 + return 'Use <tt>&lt;button&gt;</tt> to render buttons.'; 11 11 + } 12 12 + 13 13 + public function renderExample() { 14 14 + $request = $this->getRequest(); 15 15 + $user = $request->getUser(); 16 16 + 17 17 + $colors = array('', 'green', 'grey', 'disabled'); 18 18 + $sizes = array('', 'small'); 19 19 + $tags = array('a', 'button'); 20 20 + 21 21 + $view = array(); 22 22 + foreach ($tags as $tag) { 23 23 + foreach ($colors as $color) { 24 24 + foreach ($sizes as $size) { 25 25 + $class = implode(' ', array($color, $size)); 26 26 + 27 27 + if ($tag == 'a') { 28 28 + $class .= ' button'; 29 29 + } 30 30 + 31 31 + $view[] = phutil_render_tag( 32 32 + $tag, 33 33 + array( 34 34 + 'class' => $class, 35 35 + ), 36 36 + phutil_escape_html(ucwords($size.' '.$color.' '.$tag))); 37 37 + 38 38 + $view[] = '<br /><br />'; 39 39 + } 40 40 + } 41 41 + } 42 42 + 43 43 + return '<div style="margin: 1em 2em;">'.implode('', $view).'</div>'; 44 44 + } 45 45 + }

+4 -6

src/docs/userguide/remarkup.diviner

reviewed

··· 307 307 308 308 = Embedding Media = 309 309 310 310 - If you set configuration flags, you can embed media directly in text: 310 310 + If you set a configuration flag, you can embed media directly in text: 311 311 312 312 - - **files.enable-proxy**: allows you to paste in image URLs and have them 313 313 - render inline. 314 312 - **remarkup.enable-embedded-youtube**: allows you to paste in YouTube videos 315 313 and have them render inline. 316 314 317 317 - These options are disabled by default because they have security and/or 318 318 - silliness implications, read their descriptions in ##default.conf.php## before 319 319 - enabling them. 315 315 + This option is disabled by default because it has security and/or 316 316 + silliness implications. Read the description in ##default.conf.php## before 317 317 + enabling it. 320 318 321 319 = Image Macros = 322 320

+1

src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php

reviewed

··· 11 11 12 12 public function getConfiguration() { 13 13 return array( 14 14 + self::CONFIG_IDS => self::IDS_COUNTER, 14 15 self::CONFIG_TIMESTAMPS => false, 15 16 ) + parent::getConfiguration(); 16 17 }

+1 -7

src/infrastructure/markup/PhabricatorMarkupEngine.php

reviewed

··· 41 41 42 42 private $objects = array(); 43 43 private $viewer; 44 44 - private $version = 0; 44 44 + private $version = 1; 45 45 46 46 47 47 /* -( Markup Pipeline )---------------------------------------------------- */ ··· 286 286 return self::newMarkupEngine( 287 287 array( 288 288 'macros' => false, 289 289 - 'fileproxy' => false, 290 289 'youtube' => false, 291 290 292 291 )); ··· 345 344 private static function getMarkupEngineDefaultConfiguration() { 346 345 return array( 347 346 'pygments' => PhabricatorEnv::getEnvConfig('pygments.enabled'), 348 348 - 'fileproxy' => PhabricatorEnv::getEnvConfig('files.enable-proxy'), 349 347 'youtube' => PhabricatorEnv::getEnvConfig( 350 348 'remarkup.enable-embedded-youtube'), 351 349 'custom-inline' => array(), ··· 393 391 } 394 392 395 393 $rules[] = new PhutilRemarkupRuleDocumentLink(); 396 396 - 397 397 - if ($options['fileproxy']) { 398 398 - $rules[] = new PhabricatorRemarkupRuleProxyImage(); 399 399 - } 400 394 401 395 if ($options['youtube']) { 402 396 $rules[] = new PhabricatorRemarkupRuleYoutube();

-45

src/infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php

reviewed

··· 1 1 - <?php 2 2 - 3 3 - /** 4 4 - * @group markup 5 5 - */ 6 6 - final class PhabricatorRemarkupRuleProxyImage 7 7 - extends PhutilRemarkupRule { 8 8 - 9 9 - public function apply($text) { 10 10 - 11 11 - $filetypes = '\.(?:jpe?g|png|gif)'; 12 12 - 13 13 - $text = preg_replace_callback( 14 14 - '@[<](\w{3,}://.+?'.$filetypes.')[>]@', 15 15 - array($this, 'markupProxyImage'), 16 16 - $text); 17 17 - 18 18 - $text = preg_replace_callback( 19 19 - '@(?<=^|\s)(\w{3,}://\S+'.$filetypes.')(?=\s|$)@', 20 20 - array($this, 'markupProxyImage'), 21 21 - $text); 22 22 - 23 23 - return $text; 24 24 - } 25 25 - 26 26 - public function markupProxyImage($matches) { 27 27 - 28 28 - $uri = PhabricatorFileProxyImage::getProxyImageURI($matches[1]); 29 29 - 30 30 - return $this->getEngine()->storeText( 31 31 - phutil_render_tag( 32 32 - 'a', 33 33 - array( 34 34 - 'href' => $uri, 35 35 - 'target' => '_blank', 36 36 - ), 37 37 - phutil_render_tag( 38 38 - 'img', 39 39 - array( 40 40 - 'src' => $uri, 41 41 - 'class' => 'remarkup-proxy-image', 42 42 - )))); 43 43 - } 44 44 - 45 45 - }

+66 -6

src/infrastructure/storage/lisk/LiskDAO.php

reviewed

··· 177 177 const SERIALIZATION_PHP = 'php'; 178 178 179 179 const IDS_AUTOINCREMENT = 'ids-auto'; 180 180 + const IDS_COUNTER = 'ids-counter'; 180 181 const IDS_PHID = 'ids-phid'; 181 182 const IDS_MANUAL = 'ids-manual'; 183 183 + 184 184 + const COUNTER_TABLE_NAME = 'lisk_counter'; 182 185 183 186 private $__dirtyFields = array(); 184 187 private $__missingFields = array(); ··· 327 330 * Lisk objects need to have a unique identifying ID. The three mechanisms 328 331 * available for generating this ID are IDS_AUTOINCREMENT (default, assumes 329 332 * the ID column is an autoincrement primary key), IDS_PHID (to generate a 330 330 - * unique PHID for each object) or IDS_MANUAL (you are taking full 331 331 - * responsibility for ID management). 333 333 + * unique PHID for each object), IDS_MANUAL (you are taking full 334 334 + * responsibility for ID management), or IDS_COUNTER (see below). 335 335 + * 336 336 + * InnoDB does not persist the value of `auto_increment` across restarts, 337 337 + * and instead initializes it to `MAX(id) + 1` during startup. This means it 338 338 + * may reissue the same autoincrement ID more than once, if the row is deleted 339 339 + * and then the database is restarted. To avoid this, you can set an object to 340 340 + * use a counter table with IDS_COUNTER. This will generally behave like 341 341 + * IDS_AUTOINCREMENT, except that the counter value will persist across 342 342 + * restarts and inserts will be slightly slower. If a database stores any 343 343 + * DAOs which use this mechanism, you must create a table there with this 344 344 + * schema: 345 345 + * 346 346 + * CREATE TABLE lisk_counter ( 347 347 + * counterName VARCHAR(64) COLLATE utf8_bin PRIMARY KEY, 348 348 + * counterValue BIGINT UNSIGNED NOT NULL 349 349 + * ) ENGINE=InnoDB DEFAULT CHARSET=utf8; 332 350 * 333 351 * CONFIG_TIMESTAMPS 334 352 * Lisk can automatically handle keeping track of a `dateCreated' and ··· 364 382 * by Lisk (use readField and writeField instead), and you should not 365 383 * directly access or assign protected members of your class (use the getters 366 384 * and setters). 367 367 - * 368 385 * 369 386 * @return dictionary Map of configuration options to values. 370 387 * ··· 1181 1198 return $this; 1182 1199 } 1183 1200 1184 1184 - 1185 1201 /** 1186 1202 * Internal implementation of INSERT and REPLACE. 1187 1203 * ··· 1193 1209 $this->willSaveObject(); 1194 1210 $data = $this->getPropertyValues(); 1195 1211 1212 1212 + $conn = $this->establishConnection('w'); 1213 1213 + 1196 1214 $id_mechanism = $this->getConfigOption(self::CONFIG_IDS); 1197 1215 switch ($id_mechanism) { 1198 1216 case self::IDS_AUTOINCREMENT: ··· 1204 1222 unset($data[$id_key]); 1205 1223 } 1206 1224 break; 1225 1225 + case self::IDS_COUNTER: 1226 1226 + // If we are using counter IDs, assign a new ID if we don't already have 1227 1227 + // one. 1228 1228 + $id_key = $this->getIDKeyForUse(); 1229 1229 + if (empty($data[$id_key])) { 1230 1230 + $counter_name = $this->getTableName(); 1231 1231 + $id = self::loadNextCounterID($conn, $counter_name); 1232 1232 + $this->setID($id); 1233 1233 + $data[$id_key] = $id; 1234 1234 + } 1235 1235 + break; 1207 1236 case self::IDS_PHID: 1208 1237 if (empty($data[$this->getIDKeyForUse()])) { 1209 1238 $phid = $this->generatePHID(); ··· 1218 1247 } 1219 1248 1220 1249 $this->willWriteData($data); 1221 1221 - 1222 1222 - $conn = $this->establishConnection('w'); 1223 1250 1224 1251 $columns = array_keys($data); 1225 1252 ··· 1759 1786 public function __set($name, $value) { 1760 1787 phlog('Wrote to undeclared property '.get_class($this).'::$'.$name.'.'); 1761 1788 $this->$name = $value; 1789 1789 + } 1790 1790 + 1791 1791 + /** 1792 1792 + * Increments a named counter and returns the next value. 1793 1793 + * 1794 1794 + * @param AphrontDatabaseConnection Database where the counter resides. 1795 1795 + * @param string Counter name to create or increment. 1796 1796 + * @return int Next counter value. 1797 1797 + * 1798 1798 + * @task util 1799 1799 + */ 1800 1800 + public static function loadNextCounterID( 1801 1801 + AphrontDatabaseConnection $conn_w, 1802 1802 + $counter_name) { 1803 1803 + 1804 1804 + // NOTE: If an insert does not touch an autoincrement row or call 1805 1805 + // LAST_INSERT_ID(), MySQL normally does not change the value of 1806 1806 + // LAST_INSERT_ID(). This can cause a counter's value to leak to a 1807 1807 + // new counter if the second counter is created after the first one is 1808 1808 + // updated. To avoid this, we insert LAST_INSERT_ID(1), to ensure the 1809 1809 + // LAST_INSERT_ID() is always updated and always set correctly after the 1810 1810 + // query completes. 1811 1811 + 1812 1812 + queryfx( 1813 1813 + $conn_w, 1814 1814 + 'INSERT INTO %T (counterName, counterValue) VALUES 1815 1815 + (%s, LAST_INSERT_ID(1)) 1816 1816 + ON DUPLICATE KEY UPDATE 1817 1817 + counterValue = LAST_INSERT_ID(counterValue + 1)', 1818 1818 + self::COUNTER_TABLE_NAME, 1819 1819 + $counter_name); 1820 1820 + 1821 1821 + return $conn_w->getInsertID(); 1762 1822 } 1763 1823 1764 1824 }

+35

src/infrastructure/storage/lisk/__tests__/LiskFixtureTestCase.php

reviewed

··· 95 95 $this->assertEqual(true, (bool)$load->load((string)$id)); 96 96 } 97 97 98 98 + public function testCounters() { 99 99 + $obj = new HarbormasterObject(); 100 100 + $conn_w = $obj->establishConnection('w'); 101 101 + 102 102 + // Test that the counter bascially behaves as expected. 103 103 + $this->assertEqual(1, LiskDAO::loadNextCounterID($conn_w, 'a')); 104 104 + $this->assertEqual(2, LiskDAO::loadNextCounterID($conn_w, 'a')); 105 105 + $this->assertEqual(3, LiskDAO::loadNextCounterID($conn_w, 'a')); 106 106 + 107 107 + // This first insert is primarily a test that the previous LAST_INSERT_ID() 108 108 + // value does not bleed into the creation of a new counter. 109 109 + $this->assertEqual(1, LiskDAO::loadNextCounterID($conn_w, 'b')); 110 110 + $this->assertEqual(2, LiskDAO::loadNextCounterID($conn_w, 'b')); 111 111 + 112 112 + // These inserts alternate database connections. Since unit tests are 113 113 + // transactional by default, we need to break out of them or we'll deadlock 114 114 + // since the transactions don't normally close until we exit the test. 115 115 + LiskDAO::endIsolateAllLiskEffectsToTransactions(); 116 116 + try { 117 117 + 118 118 + $conn_1 = $obj->establishConnection('w', $force_new = true); 119 119 + $conn_2 = $obj->establishConnection('w', $force_new = true); 120 120 + 121 121 + $this->assertEqual(1, LiskDAO::loadNextCounterID($conn_1, 'z')); 122 122 + $this->assertEqual(2, LiskDAO::loadNextCounterID($conn_2, 'z')); 123 123 + $this->assertEqual(3, LiskDAO::loadNextCounterID($conn_1, 'z')); 124 124 + $this->assertEqual(4, LiskDAO::loadNextCounterID($conn_2, 'z')); 125 125 + $this->assertEqual(5, LiskDAO::loadNextCounterID($conn_1, 'z')); 126 126 + 127 127 + LiskDAO::beginIsolateAllLiskEffectsToTransactions(); 128 128 + } catch (Exception $ex) { 129 129 + LiskDAO::beginIsolateAllLiskEffectsToTransactions(); 130 130 + throw $ex; 131 131 + } 132 132 + } 98 133 99 134 }

+12

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

reviewed

··· 1012 1012 'type' => 'sql', 1013 1013 'name' => $this->getPatchPath('drydockresourcetype.sql'), 1014 1014 ), 1015 1015 + 'liskcounters.sql' => array( 1016 1016 + 'type' => 'sql', 1017 1017 + 'name' => $this->getPatchPath('liskcounters.sql'), 1018 1018 + ), 1019 1019 + 'liskcounters.php' => array( 1020 1020 + 'type' => 'php', 1021 1021 + 'name' => $this->getPatchPath('liskcounters.php'), 1022 1022 + ), 1023 1023 + 'dropfileproxyimage.sql' => array( 1024 1024 + 'type' => 'sql', 1025 1025 + 'name' => $this->getPatchPath('dropfileproxyimage.sql'), 1026 1026 + ), 1015 1027 ); 1016 1028 } 1017 1029