From 7198bd7db784d4a2dc4890106e586f0b607ce080 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 27 Aug 2019 15:27:25 -0700 Subject: [PATCH 1/5] When "utf8mb4" is available, use it as the default client charset when invoking standalone "mysql" commands Summary: Fixes T13390. We have some old code which doesn't dynamically select between "utf8mb4" and "utf8". This can lead to dumping utf8mb4 data over a utf8 connection in `bin/storage dump`, which possibly corrupts some emoji/whales. Instead, prefer "utf8mb4" if it's available. Test Plan: Ran `bin/storage dump` and `bin/storage shell`, saw sub-commands select utf8mb4 as the client charset. Maniphest Tasks: T13390 Differential Revision: https://secure.phabricator.com/D20742 --- .../management/PhabricatorStorageManagementAPI.php | 8 ++++++++ .../workflow/PhabricatorStorageManagementDumpWorkflow.php | 4 +++- .../PhabricatorStorageManagementShellWorkflow.php | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php b/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php index e66ba784f7..b838c8a5d9 100644 --- a/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php +++ b/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php @@ -298,6 +298,14 @@ final class PhabricatorStorageManagementAPI extends Phobject { return self::isCharacterSetAvailableOnConnection($character_set, $conn); } + public function getClientCharset() { + if ($this->isCharacterSetAvailable('utf8mb4')) { + return 'utf8mb4'; + } else { + return 'utf8'; + } + } + public static function isCharacterSetAvailableOnConnection( $character_set, AphrontDatabaseConnection $conn) { diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index 28b188a873..3a18578a30 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -179,7 +179,9 @@ final class PhabricatorStorageManagementDumpWorkflow $argv = array(); $argv[] = '--hex-blob'; $argv[] = '--single-transaction'; - $argv[] = '--default-character-set=utf8'; + + $argv[] = '--default-character-set'; + $argv[] = $api->getClientCharset(); if ($args->getArg('for-replica')) { $argv[] = '--master-data'; diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementShellWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementShellWorkflow.php index 0bf185a086..f376ea3e14 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementShellWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementShellWorkflow.php @@ -31,8 +31,8 @@ final class PhabricatorStorageManagementShellWorkflow } return phutil_passthru( - 'mysql --protocol=TCP --default-character-set=utf8mb4 '. - '-u %s %C -h %s %C', + 'mysql --protocol=TCP --default-character-set %R -u %s %C -h %s %C', + $api->getClientCharset(), $api->getUser(), $flag_password, $host, From 0943561dcb780596a13a6c34b9090bfebe825dfa Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 27 Aug 2019 07:05:11 -0700 Subject: [PATCH 2/5] Fix incorrect construction of subtype map when validating "subtype" transactions against non-subtypable objects Summary: Fixes T13389. Currently, we try to "newSubtypeMap()" unconditionally, even if the underlying object does not support subtypes. - Only try to build a subtype map if subtype transactions are actually being applied. - When subtype transactions are applied to a non-subtypable object, fail more explicitly. Test Plan: Clicked "Make Editable" in a fresh Calendar transaction form, got an editable form instead of a fatal from "newSubtypeMap()". (Calendar events are not currently subtypable.) Maniphest Tasks: T13389 Differential Revision: https://secure.phabricator.com/D20741 --- ...habricatorEditEngineSubtypeTransaction.php | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php b/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php index 53ec221631..a2a8538115 100644 --- a/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php +++ b/src/applications/transactions/xaction/PhabricatorEditEngineSubtypeTransaction.php @@ -25,11 +25,30 @@ final class PhabricatorEditEngineSubtypeTransaction } public function validateTransactions($object, array $xactions) { - $map = $object->getEngine() + $errors = array(); + + if (!$xactions) { + return $errors; + } + + $engine = $object->getEngine(); + + if (!$engine->supportsSubtypes()) { + foreach ($xactions as $xaction) { + $errors[] = $this->newInvalidError( + pht( + 'Edit engine (of class "%s") does not support subtypes, so '. + 'subtype transactions can not be applied to it.', + get_class($engine)), + $xaction); + } + return $errors; + } + + $map = $engine ->setViewer($this->getActor()) ->newSubtypeMap(); - $errors = array(); foreach ($xactions as $xaction) { $new = $xaction->getNewValue(); From c6642213d57f82dda6927001312fcbde5a18c34e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 28 Aug 2019 07:34:03 -0700 Subject: [PATCH 3/5] Straighten out replication/cache behavior in "bin/storage dump" Summary: Fixes T13336. - Prevent `--no-indexes` from being combined with `--for-replica`, since combining these options can only lead to heartbreak. - In `--for-replica` mode, dump caches too. See discussion in T13336. It is probably "safe" to not dump these today, but fragile and not correct. - Mark the "MarkupCache" table as having "Cache" persistence, not "Data" persistence (no need to back it up, since it can be fully regenerated from other datasources). Test Plan: Ran `bin/storage dump` with various combinations of flags. Maniphest Tasks: T13336 Differential Revision: https://secure.phabricator.com/D20743 --- .../cache/storage/PhabricatorMarkupCache.php | 4 ++++ .../schema/PhabricatorConfigSchemaSpec.php | 10 +++++++- src/infrastructure/storage/lisk/LiskDAO.php | 4 ++++ ...abricatorStorageManagementDumpWorkflow.php | 23 ++++++++++++++----- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/applications/cache/storage/PhabricatorMarkupCache.php b/src/applications/cache/storage/PhabricatorMarkupCache.php index 03a4b08681..e008a18ee1 100644 --- a/src/applications/cache/storage/PhabricatorMarkupCache.php +++ b/src/applications/cache/storage/PhabricatorMarkupCache.php @@ -30,4 +30,8 @@ final class PhabricatorMarkupCache extends PhabricatorCacheDAO { ) + parent::getConfiguration(); } + public function getSchemaPersistence() { + return PhabricatorConfigTableSchema::PERSISTENCE_CACHE; + } + } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php index 8e67391b59..adccdc1267 100644 --- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php @@ -48,11 +48,19 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject { abstract public function buildSchemata(); protected function buildLiskObjectSchema(PhabricatorLiskDAO $object) { + $index_options = array(); + + $persistence = $object->getSchemaPersistence(); + if ($persistence !== null) { + $index_options['persistence'] = $persistence; + } + $this->buildRawSchema( $object->getApplicationName(), $object->getTableName(), $object->getSchemaColumns(), - $object->getSchemaKeys()); + $object->getSchemaKeys(), + $index_options); } protected function buildFerretIndexSchema(PhabricatorFerretEngine $engine) { diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php index 81005ab30d..0bbbdd83a7 100644 --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -1881,6 +1881,10 @@ abstract class LiskDAO extends Phobject ->getMaximumByteLengthForDataType($data_type); } + public function getSchemaPersistence() { + return null; + } + /* -( AphrontDatabaseTableRefInterface )----------------------------------- */ diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index 3a18578a30..ebe1c77f40 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -14,7 +14,8 @@ final class PhabricatorStorageManagementDumpWorkflow 'name' => 'for-replica', 'help' => pht( 'Add __--master-data__ to the __mysqldump__ command, '. - 'generating a CHANGE MASTER statement in the output.'), + 'generating a CHANGE MASTER statement in the output. This '. + 'option also dumps all data, including caches.'), ), array( 'name' => 'output', @@ -54,6 +55,8 @@ final class PhabricatorStorageManagementDumpWorkflow $output_file = $args->getArg('output'); $is_compress = $args->getArg('compress'); $is_overwrite = $args->getArg('overwrite'); + $is_noindex = $args->getArg('no-indexes'); + $is_replica = $args->getArg('for-replica'); if ($is_compress) { if ($output_file === null) { @@ -79,6 +82,14 @@ final class PhabricatorStorageManagementDumpWorkflow } } + if ($is_replica && $is_noindex) { + throw new PhutilArgumentUsageException( + pht( + 'The "--for-replica" flag can not be used with the '. + '"--no-indexes" flag. Replication dumps must contain a complete '. + 'representation of database state.')); + } + if ($output_file !== null) { if (Filesystem::pathExists($output_file)) { if (!$is_overwrite) { @@ -94,8 +105,6 @@ final class PhabricatorStorageManagementDumpWorkflow $api = $this->getSingleAPI(); $patches = $this->getPatches(); - $with_indexes = !$args->getArg('no-indexes'); - $applied = $api->getAppliedPatches(); if ($applied === null) { throw new PhutilArgumentUsageException( @@ -119,6 +128,9 @@ final class PhabricatorStorageManagementDumpWorkflow $schemata = $actual_map[$ref_key]; $expect = $expect_map[$ref_key]; + $with_caches = $is_replica; + $with_indexes = !$is_noindex; + $targets = array(); foreach ($schemata->getDatabases() as $database_name => $database) { $expect_database = $expect->getDatabase($database_name); @@ -143,7 +155,7 @@ final class PhabricatorStorageManagementDumpWorkflow // When dumping tables, leave the data in cache tables in the // database. This will be automatically rebuild after the data // is restored and does not need to be persisted in backups. - $with_data = false; + $with_data = $with_caches; break; case PhabricatorConfigTableSchema::PERSISTENCE_INDEX: // When dumping tables, leave index data behind of the caller @@ -183,7 +195,7 @@ final class PhabricatorStorageManagementDumpWorkflow $argv[] = '--default-character-set'; $argv[] = $api->getClientCharset(); - if ($args->getArg('for-replica')) { + if ($is_replica) { $argv[] = '--master-data'; } @@ -342,7 +354,6 @@ final class PhabricatorStorageManagementDumpWorkflow return 0; } - private function writeData($data, $file, $is_compress, $output_file) { if (!strlen($data)) { return; From 3c26e384872ac8c1193b304434428eb3b42ff2f7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 29 Aug 2019 12:55:39 -0700 Subject: [PATCH 4/5] Provide a simple read-only maintenance mode for repositories Summary: Ref T13393. While doing a shard migration in the Phacility cluster, we'd like to stop writes to the migrating repository. It's safe to continue serving reads. Add a simple maintenance mode for making repositories completely read-only during maintenance. Test Plan: Put a repository into read-only mode, tried to write via HTTP + SSH. Viewed web UI. Took it back out of maintenance mode. Maniphest Tasks: T13393 Differential Revision: https://secure.phabricator.com/D20748 --- src/__phutil_library_map__.php | 4 + .../DiffusionRepositoryController.php | 25 ++++- .../controller/DiffusionServeController.php | 6 + .../diffusion/ssh/DiffusionSSHWorkflow.php | 4 + .../PhabricatorRepositoryPullEngine.php | 7 ++ ...epositoryManagementMaintenanceWorkflow.php | 104 ++++++++++++++++++ .../storage/PhabricatorRepository.php | 35 ++++++ ...icatorRepositoryMaintenanceTransaction.php | 43 ++++++++ 8 files changed, 223 insertions(+), 5 deletions(-) create mode 100644 src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php create mode 100644 src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 81ac776d5d..5d9a0aed3c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4470,6 +4470,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryIdentityTransaction' => 'applications/repository/storage/PhabricatorRepositoryIdentityTransaction.php', 'PhabricatorRepositoryIdentityTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryIdentityTransactionQuery.php', 'PhabricatorRepositoryIdentityTransactionType' => 'applications/repository/xaction/PhabricatorRepositoryIdentityTransactionType.php', + 'PhabricatorRepositoryMaintenanceTransaction' => 'applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php', 'PhabricatorRepositoryManagementCacheWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementCacheWorkflow.php', 'PhabricatorRepositoryManagementClusterizeWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementClusterizeWorkflow.php', 'PhabricatorRepositoryManagementDiscoverWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php', @@ -4478,6 +4479,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryManagementListPathsWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListPathsWorkflow.php', 'PhabricatorRepositoryManagementListWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php', 'PhabricatorRepositoryManagementLookupUsersWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementLookupUsersWorkflow.php', + 'PhabricatorRepositoryManagementMaintenanceWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php', 'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMarkImportedWorkflow.php', 'PhabricatorRepositoryManagementMarkReachableWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMarkReachableWorkflow.php', 'PhabricatorRepositoryManagementMirrorWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php', @@ -10911,6 +10913,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryIdentityTransaction' => 'PhabricatorModularTransaction', 'PhabricatorRepositoryIdentityTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorRepositoryIdentityTransactionType' => 'PhabricatorModularTransactionType', + 'PhabricatorRepositoryMaintenanceTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryManagementCacheWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementClusterizeWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementDiscoverWorkflow' => 'PhabricatorRepositoryManagementWorkflow', @@ -10919,6 +10922,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryManagementListPathsWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementListWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementLookupUsersWorkflow' => 'PhabricatorRepositoryManagementWorkflow', + 'PhabricatorRepositoryManagementMaintenanceWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementMarkReachableWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementMirrorWorkflow' => 'PhabricatorRepositoryManagementWorkflow', diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index 12aaefe12a..f00521257e 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -145,13 +145,26 @@ final class DiffusionRepositoryController extends DiffusionController { ->setRight(array($this->branchButton, $actions_button, $clone_button)) ->addClass('diffusion-action-bar'); + $status_view = null; + if ($repository->isReadOnly()) { + $status_view = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors( + array( + phutil_escape_html_newlines( + $repository->getReadOnlyMessageForDisplay()), + )); + } + $view = id(new PHUITwoColumnView()) ->setHeader($header) - ->setFooter(array( - $bar, - $description, - $content, - )); + ->setFooter( + array( + $status_view, + $bar, + $description, + $content, + )); if ($page_has_content) { $view->setTabs($tabs); @@ -327,6 +340,8 @@ final class DiffusionRepositoryController extends DiffusionController { if (!$repository->isTracked()) { $header->setStatus('fa-ban', 'dark', pht('Inactive')); + } else if ($repository->isReadOnly()) { + $header->setStatus('fa-wrench', 'indigo', pht('Under Maintenance')); } else if ($repository->isImporting()) { $ratio = $repository->loadImportProgress(); $percentage = sprintf('%.2f%%', 100 * $ratio); diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index aea901f100..60d5c1578d 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -302,6 +302,12 @@ final class DiffusionServeController extends DiffusionController { } if ($is_push) { + if ($repository->isReadOnly()) { + return new PhabricatorVCSResponse( + 503, + $repository->getReadOnlyMessageForDisplay()); + } + $can_write = $repository->canServeProtocol($proto_https, true) || $repository->canServeProtocol($proto_http, true); diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php index 57dc83953d..08144eb0c9 100644 --- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php @@ -255,6 +255,10 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow { 'user account.')); } + if ($repository->isReadOnly()) { + throw new Exception($repository->getReadOnlyMessageForDisplay()); + } + $protocol = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_SSH; if ($repository->canServeProtocol($protocol, true)) { $can_push = PhabricatorPolicyFilter::hasCapability( diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php index 2c6ac8e83f..ea70f380aa 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -52,6 +52,13 @@ final class PhabricatorRepositoryPullEngine $repository = $this->getRepository(); $viewer = PhabricatorUser::getOmnipotentUser(); + if ($repository->isReadOnly()) { + $this->skipPull( + pht( + "Skipping pull on read-only repository.\n\n%s", + $repository->getReadOnlyMessageForDisplay())); + } + $is_hg = false; $is_git = false; $is_svn = false; diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php new file mode 100644 index 0000000000..65fe0adaad --- /dev/null +++ b/src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php @@ -0,0 +1,104 @@ +setName('maintenance') + ->setExamples( + "**maintenance** --start __message__ __repository__ ...\n". + "**maintenance** --stop __repository__") + ->setSynopsis( + pht('Set or clear read-only mode for repository maintenance.')) + ->setArguments( + array( + array( + 'name' => 'start', + 'param' => 'message', + 'help' => pht( + 'Put repositories into maintenance mode.'), + ), + array( + 'name' => 'stop', + 'help' => pht( + 'Take repositories out of maintenance mode, returning them '. + 'to normal serice.'), + ), + array( + 'name' => 'repositories', + 'wildcard' => true, + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + $repositories = $this->loadRepositories($args, 'repositories'); + if (!$repositories) { + throw new PhutilArgumentUsageException( + pht('Specify one or more repositories to act on.')); + } + + $message = $args->getArg('start'); + $is_start = (bool)strlen($message); + $is_stop = $args->getArg('stop'); + + if (!$is_start && !$is_stop) { + throw new PhutilArgumentUsageException( + pht( + 'Use "--start " to put repositories into maintenance '. + 'mode, or "--stop" to take them out of maintenance mode.')); + } + + if ($is_start && $is_stop) { + throw new PhutilArgumentUsageException( + pht( + 'Specify either "--start" or "--stop", but not both.')); + } + + $content_source = $this->newContentSource(); + $diffusion_phid = id(new PhabricatorDiffusionApplication())->getPHID(); + + if ($is_start) { + $new_value = $message; + } else { + $new_value = null; + } + + foreach ($repositories as $repository) { + $xactions = array(); + + $xactions[] = $repository->getApplicationTransactionTemplate() + ->setTransactionType( + PhabricatorRepositoryMaintenanceTransaction::TRANSACTIONTYPE) + ->setNewValue($new_value); + + $repository->getApplicationTransactionEditor() + ->setActor($viewer) + ->setActingAsPHID($diffusion_phid) + ->setContentSource($content_source) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($repository, $xactions); + + if ($is_start) { + echo tsprintf( + "%s\n", + pht( + 'Put repository "%s" into maintenance mode.', + $repository->getDisplayName())); + } else { + echo tsprintf( + "%s\n", + pht( + 'Took repository "%s" out of maintenance mode.', + $repository->getDisplayName())); + } + } + + return 0; + } + +} diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 9661584851..bd15e3e8de 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1410,6 +1410,12 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO } } + if ($write) { + if ($this->isReadOnly()) { + return false; + } + } + return false; } @@ -2266,6 +2272,35 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $this->isGit(); } + public function isReadOnly() { + return (bool)$this->getDetail('read-only'); + } + + public function setReadOnly($read_only) { + return $this->setDetail('read-only', $read_only); + } + + public function getReadOnlyMessage() { + return $this->getDetail('read-only-message'); + } + + public function setReadOnlyMessage($message) { + return $this->setDetail('read-only-message', $message); + } + + public function getReadOnlyMessageForDisplay() { + $parts = array(); + $parts[] = pht( + 'This repository is currently in read-only maintenance mode.'); + + $message = $this->getReadOnlyMessage(); + if ($message !== null) { + $parts[] = $message; + } + + return implode("\n\n", $parts); + } + /* -( Repository URIs )---------------------------------------------------- */ diff --git a/src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php new file mode 100644 index 0000000000..caf9e84527 --- /dev/null +++ b/src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php @@ -0,0 +1,43 @@ +getReadOnlyMessage(); + } + + public function applyInternalEffects($object, $value) { + if ($value === null) { + $object + ->setReadOnly(false) + ->setReadOnlyMessage(null); + } else { + $object + ->setReadOnly(true) + ->setReadOnlyMessage($value); + } + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if (strlen($old) && !strlen($new)) { + return pht( + '%s took this repository out of maintenance mode.', + $this->renderAuthor()); + } else if (!strlen($old) && strlen($new)) { + return pht( + '%s put this repository into maintenance mode.', + $this->renderAuthor()); + } else { + return pht( + '%s updated the maintenance message for this repository.', + $this->renderAuthor()); + } + } + +} From 533a5535b6f0104c13266400735cb563d64525e7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 30 Aug 2019 09:12:57 -0700 Subject: [PATCH 5/5] Remove the "grant authority" mechanism from users Summary: Ref T13393. See some previous discussion in T13366. Caching is hard and all approaches here have downsides, but the request cache likely has fewer practical downsides for this kind of policy check than other approaches. In particular, the grant approach (at least, as previously used in Phortune) has a major downside that "Query" classes can no longer fully enforce policies. Since Phortune no longer depends on grants and they've now been removed from instances, drop the mechanism completely. Test Plan: Grepped for callsites, found none. Maniphest Tasks: T13393 Differential Revision: https://secure.phabricator.com/D20754 --- .../people/storage/PhabricatorUser.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index c675878747..cce4ffa58a 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -59,7 +59,6 @@ final class PhabricatorUser private $rawCacheData = array(); private $usableCacheData = array(); - private $authorities = array(); private $handlePool; private $csrfSalt; @@ -705,23 +704,6 @@ final class PhabricatorUser } - /** - * Grant a user a source of authority, to let them bypass policy checks they - * could not otherwise. - */ - public function grantAuthority($authority) { - $this->authorities[] = $authority; - return $this; - } - - - /** - * Get authorities granted to the user. - */ - public function getAuthorities() { - return $this->authorities; - } - public function hasConduitClusterToken() { return ($this->conduitClusterToken !== self::ATTACHABLE); }