From ed46cfff3fc416332a50b5c80294762405e5efb5 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 10 Oct 2017 09:01:18 +0545 Subject: [PATCH] Refactor set and reset of capabilities --- .../features/bootstrap/AppConfiguration.php | 54 ++++++++----- .../bootstrap/CapabilitiesContext.php | 41 ++-------- .../features/bootstrap/FeatureContext.php | 10 +-- .../features/bootstrap/FederationContext.php | 15 +--- .../features/bootstrap/ShareesContext.php | 11 +-- .../features/bootstrap/Sharing.php | 75 +++---------------- 6 files changed, 55 insertions(+), 151 deletions(-) diff --git a/tests/integration/features/bootstrap/AppConfiguration.php b/tests/integration/features/bootstrap/AppConfiguration.php index ea190539091e..e82c0516b1d7 100644 --- a/tests/integration/features/bootstrap/AppConfiguration.php +++ b/tests/integration/features/bootstrap/AppConfiguration.php @@ -35,6 +35,11 @@ trait AppConfiguration { * @var string the original capabilities in XML format */ private $savedCapabilitiesXml; + + /** + * @var array the changes made to capabilities for the test scenario + */ + private $savedCapabilitiesChanges = []; /** * @param string $verb @@ -150,7 +155,7 @@ public function getParameterValueFromXml($xml, $capabilitiesApp, $capabilitiesPa * @return boolean */ public function wasCapabilitySet($capabilitiesApp, $capabilitiesParameter) { - return $this->getParameterValueFromXml( + return (bool) $this->getParameterValueFromXml( $this->savedCapabilitiesXml, $capabilitiesApp, $capabilitiesParameter @@ -164,10 +169,10 @@ public function wasCapabilitySet($capabilitiesApp, $capabilitiesParameter) { * @param string $testingApp the "app" name as understood by "testing" * @param string $testingParameter the parameter name as understood by * "testing" - * @param boolean $testingState the on|off state the parameter was set to for the test + * @param boolean $testingState the on|off state the parameter must be set to for the test * @return void */ - public function resetCapability( + public function setCapability( $capabilitiesApp, $capabilitiesParameter, $testingApp, $testingParameter, $testingState ) { $savedState = $this->wasCapabilitySet( @@ -175,12 +180,23 @@ public function resetCapability( $capabilitiesParameter ); + // Always set the config value, because sometimes enabling one config + // also changes some sub-settings. So the "interim" state as we set + // the config values could be unexpectedly different from the original + // saved state. + $this->modifyServerConfig( + $testingApp, + $testingParameter, + $testingState ? 'yes' : 'no' + ); + if ($savedState !== $testingState) { - $this->modifyServerConfig( - $testingApp, - $testingParameter, - $savedState ? 'yes' : 'no' - ); + $this->savedCapabilitiesChanges[] = + [ + 'testingApp' => $testingApp, + 'testingParameter' => $testingParameter, + 'savedState' => $savedState + ]; } } @@ -236,15 +252,7 @@ protected function setStatusTestingApp($enabled) { * * @return void */ - abstract protected function setupAppConfigs(); - - /** - * Restore any app config state. - * This will be called before each scenario. - * - * @return void - */ - abstract protected function restoreAppConfigs(); + abstract protected function resetAppConfigs(); /** * @BeforeScenario @@ -253,7 +261,7 @@ abstract protected function restoreAppConfigs(); public function prepareParametersBeforeScenario() { $user = $this->currentUser; $this->currentUser = 'admin'; - $this->setupAppConfigs(); + $this->resetAppConfigs(); $this->currentUser = $user; } @@ -264,7 +272,15 @@ public function prepareParametersBeforeScenario() { public function restoreParametersAfterScenario() { $user = $this->currentUser; $this->currentUser = 'admin'; - $this->restoreAppConfigs(); + + foreach ($this->savedCapabilitiesChanges as $capabilitiesChange) { + $this->modifyServerConfig( + $capabilitiesChange['testingApp'], + $capabilitiesChange['testingParameter'], + $capabilitiesChange['savedState'] ? 'yes' : 'no' + ); + } + $this->currentUser = $user; } } diff --git a/tests/integration/features/bootstrap/CapabilitiesContext.php b/tests/integration/features/bootstrap/CapabilitiesContext.php index 8843643c6a3f..84740f7185f8 100644 --- a/tests/integration/features/bootstrap/CapabilitiesContext.php +++ b/tests/integration/features/bootstrap/CapabilitiesContext.php @@ -59,76 +59,49 @@ public function checkCapabilitiesResponse(\Behat\Gherkin\Node\TableNode $formDat /** * @return void */ - protected function setupAppConfigs() { + protected function resetAppConfigs() { // Remember the current capabilities $this->getCapabilitiesCheckResponse(); $this->savedCapabilitiesXml = $this->getCapabilitiesXml(); // Set the required starting values for testing $this->setupCommonSharingConfigs(); $this->setupCommonFederationConfigs(); - if (!$this->wasCapabilitySet('files_sharing', 'resharing')) { - $this->modifyServerConfig('core', 'shareapi_allow_resharing', 'yes'); - } - if ($this->wasCapabilitySet('files_sharing', 'public@@@password@@@enforced')) { - $this->modifyServerConfig('core', 'shareapi_enforce_links_password', 'no'); - } - if ($this->wasCapabilitySet('files_sharing', 'public@@@send_mail')) { - $this->modifyServerConfig('core', 'shareapi_allow_public_notification', 'no'); - } - if (!$this->wasCapabilitySet('files_sharing', 'public@@@social_share')) { - $this->modifyServerConfig('core', 'shareapi_allow_social_share', 'yes'); - } - if ($this->wasCapabilitySet('files_sharing', 'public@@@expire_date@@@enabled')) { - $this->modifyServerConfig('core', 'shareapi_default_expire_date', 'no'); - } - if ($this->wasCapabilitySet('files_sharing', 'public@@@expire_date@@@enforced')) { - $this->modifyServerConfig('core', 'shareapi_enforce_expire_date', 'no'); - } - } - - /** - * @return void - */ - protected function restoreAppConfigs() { - // Restore the previous capabilities settings - $this->restoreCommonSharingConfigs(); - $this->restoreCommonFederationConfigs(); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'resharing', 'core', 'shareapi_allow_resharing', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@password@@@enforced', 'core', 'shareapi_enforce_links_password', false ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@send_mail', 'core', 'shareapi_allow_public_notification', false ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@social_share', 'core', 'shareapi_allow_social_share', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@expire_date@@@enabled', 'core', 'shareapi_default_expire_date', false ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@expire_date@@@enforced', 'core', diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index a66234758b5e..992f525b37c2 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -37,19 +37,11 @@ class FeatureContext implements Context, SnippetAcceptingContext { /** * @return void */ - protected function setupAppConfigs() { + protected function resetAppConfigs() { // Remember the current capabilities $this->getCapabilitiesCheckResponse(); $this->savedCapabilitiesXml = $this->getCapabilitiesXml(); // Set the required starting values for testing $this->setupCommonSharingConfigs(); } - - /** - * @return void - */ - protected function restoreAppConfigs() { - // Restore the previous capabilities settings - $this->restoreCommonSharingConfigs(); - } } diff --git a/tests/integration/features/bootstrap/FederationContext.php b/tests/integration/features/bootstrap/FederationContext.php index fbc5f45e93a4..2670fca0a053 100644 --- a/tests/integration/features/bootstrap/FederationContext.php +++ b/tests/integration/features/bootstrap/FederationContext.php @@ -90,25 +90,14 @@ public function acceptLastPendingShare($user, $server) { /** * @return void */ - protected function setupAppConfigs() { + protected function resetAppConfigs() { // Remember the current capabilities $this->getCapabilitiesCheckResponse(); $this->savedCapabilitiesXml = $this->getCapabilitiesXml(); // Set the required starting values for testing $this->setupCommonSharingConfigs(); $this->setupCommonFederationConfigs(); - if (!$this->wasCapabilitySet('files_sharing', 'resharing')) { - $this->modifyServerConfig('core', 'shareapi_allow_resharing', 'yes'); - } - } - - /** - * @return void - */ - protected function restoreAppConfigs() { - $this->restoreCommonSharingConfigs(); - $this->restoreCommonFederationConfigs(); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'resharing', 'core', diff --git a/tests/integration/features/bootstrap/ShareesContext.php b/tests/integration/features/bootstrap/ShareesContext.php index 59395aa63b7e..3a876a9d6e2e 100644 --- a/tests/integration/features/bootstrap/ShareesContext.php +++ b/tests/integration/features/bootstrap/ShareesContext.php @@ -105,7 +105,7 @@ public function getArrayOfShareesResponded( /** * @return void */ - protected function setupAppConfigs() { + protected function resetAppConfigs() { // Remember the current capabilities $this->getCapabilitiesCheckResponse(); $this->savedCapabilitiesXml = $this->getCapabilitiesXml(); @@ -113,13 +113,4 @@ protected function setupAppConfigs() { $this->setupCommonSharingConfigs(); $this->setupCommonFederationConfigs(); } - - /** - * @return void - */ - protected function restoreAppConfigs() { - // Restore the previous capabilities settings - $this->restoreCommonSharingConfigs(); - $this->restoreCommonFederationConfigs(); - } } diff --git a/tests/integration/features/bootstrap/Sharing.php b/tests/integration/features/bootstrap/Sharing.php index 73d60b4c7a42..90cb49087df0 100644 --- a/tests/integration/features/bootstrap/Sharing.php +++ b/tests/integration/features/bootstrap/Sharing.php @@ -876,90 +876,49 @@ private function getLastShareToken() { * @return void */ protected function setupCommonSharingConfigs() { - if (!$this->wasCapabilitySet('files_sharing', 'api_enabled')) { - $this->modifyServerConfig( - 'core', 'shareapi_enabled', 'yes' - ); - } - if (!$this->wasCapabilitySet('files_sharing', 'public@@@enabled')) { - $this->modifyServerConfig( - 'core', 'shareapi_allow_links', 'yes' - ); - } - if (!$this->wasCapabilitySet('files_sharing', 'public@@@upload')) { - $this->modifyServerConfig( - 'core', 'shareapi_allow_public_upload', 'yes' - ); - } - if (!$this->wasCapabilitySet('files_sharing', 'group_sharing')) { - $this->modifyServerConfig( - 'core', 'shareapi_allow_group_sharing', 'yes' - ); - } - if ($this->wasCapabilitySet('files_sharing', 'share_with_group_members_only')) { - $this->modifyServerConfig( - 'core', 'shareapi_only_share_with_group_members', 'no' - ); - } - if (!$this->wasCapabilitySet('files_sharing', 'user_enumeration@@@enabled')) { - $this->modifyServerConfig( - 'core', 'shareapi_allow_share_dialog_user_enumeration', 'yes' - ); - } - if ($this->wasCapabilitySet('files_sharing', 'user_enumeration@@@group_members_only')) { - $this->modifyServerConfig( - 'core', 'shareapi_share_dialog_user_enumeration_group_members', 'no' - ); - } - } - - /** - * @return void - */ - protected function restoreCommonSharingConfigs() { - $this->resetCapability( + $this->setCapability( 'files_sharing', 'api_enabled', 'core', 'shareapi_enabled', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@enabled', 'core', 'shareapi_allow_links', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'public@@@upload', 'core', 'shareapi_allow_public_upload', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'group_sharing', 'core', 'shareapi_allow_group_sharing', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'share_with_group_members_only', 'core', 'shareapi_only_share_with_group_members', false ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'user_enumeration@@@enabled', 'core', 'shareapi_allow_share_dialog_user_enumeration', true ); - $this->resetCapability( + $this->setCapability( 'files_sharing', 'user_enumeration@@@group_members_only', 'core', @@ -972,30 +931,14 @@ protected function restoreCommonSharingConfigs() { * @return void */ protected function setupCommonFederationConfigs() { - if (!$this->wasCapabilitySet('federation', 'outgoing')) { - $this->modifyServerConfig( - 'files_sharing', 'outgoing_server2server_share_enabled', 'yes' - ); - } - if (!$this->wasCapabilitySet('federation', 'incoming')) { - $this->modifyServerConfig( - 'files_sharing', 'incoming_server2server_share_enabled', 'yes' - ); - } - } - - /** - * @return void - */ - protected function restoreCommonFederationConfigs() { - $this->resetCapability( + $this->setCapability( 'federation', 'outgoing', 'files_sharing', 'outgoing_server2server_share_enabled', true ); - $this->resetCapability( + $this->setCapability( 'federation', 'incoming', 'files_sharing',