From f6a79338d4ea66f9c341d004951b606b5310b478 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 11 Oct 2019 08:21:24 +0200 Subject: [PATCH 1/6] Make sure we create an app's Application class just once Signed-off-by: Christoph Wurst --- lib/base.php | 2 +- lib/private/Route/Router.php | 2 +- lib/private/ServerContainer.php | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/base.php b/lib/base.php index 7674c16c26..510a960e43 100644 --- a/lib/base.php +++ b/lib/base.php @@ -727,7 +727,7 @@ class OC { // Make sure that the application class is not loaded before the database is setup if ($systemConfig->getValue("installed", false)) { OC_App::loadApp('settings'); - $settings = new \OCA\Settings\AppInfo\Application(); + $settings = \OC::$server->query(\OCA\Settings\AppInfo\Application::class); $settings->register(); } diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index 3d91a33cd8..4c83f25101 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -371,7 +371,7 @@ class Router implements IRouter { $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; if (class_exists($applicationClassName)) { - $application = new $applicationClassName(); + $application = \OC::$server->query($applicationClassName); } else { $application = new App($appName); } diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index b67b4d1e70..704d207223 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -100,8 +100,9 @@ class ServerContainer extends SimpleContainer { if (!isset($this->hasNoAppContainer[$namespace])) { $applicationClassName = 'OCA\\' . $sensitiveNamespace . '\\AppInfo\\Application'; if (class_exists($applicationClassName)) { - new $applicationClassName(); + $app = new $applicationClassName(); if (isset($this->appContainers[$namespace])) { + $this->appContainers[$namespace]->offsetSet($applicationClassName, $app); return $this->appContainers[$namespace]; } } From 543190f8b3228b994ba897c9a43fdc3f61adf663 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 11 Oct 2019 08:33:09 +0200 Subject: [PATCH 2/6] Do not create Application instances directly Signed-off-by: Christoph Wurst --- apps/dav/appinfo/app.php | 3 ++- apps/encryption/appinfo/app.php | 3 ++- apps/encryption/appinfo/routes.php | 4 +++- apps/federation/appinfo/app.php | 3 ++- apps/files/appinfo/routes.php | 3 ++- apps/files_trashbin/appinfo/routes.php | 3 ++- .../lib/BackgroundJob/ExpireTrash.php | 3 ++- apps/files_trashbin/lib/Trashbin.php | 9 ++++++--- apps/files_versions/appinfo/routes.php | 3 ++- apps/files_versions/lib/Storage.php | 4 ++-- .../lib/FederatedFileSharingFactory.php | 14 ++++++++++++-- apps/settings/appinfo/routes.php | 3 ++- .../lib/Settings/Personal/PersonalInfo.php | 2 +- core/routes.php | 3 ++- 14 files changed, 42 insertions(+), 18 deletions(-) diff --git a/apps/dav/appinfo/app.php b/apps/dav/appinfo/app.php index dd9e0e9c09..a1a2377fb3 100644 --- a/apps/dav/appinfo/app.php +++ b/apps/dav/appinfo/app.php @@ -29,7 +29,8 @@ use Symfony\Component\EventDispatcher\GenericEvent; \OC_App::loadApps(['dav']); -$app = new Application(); +/** @var Application $app */ +$app = \OC::$server->query(Application::class); $app->registerHooks(); \OC::$server->registerService('CardDAVSyncService', function() use ($app) { diff --git a/apps/encryption/appinfo/app.php b/apps/encryption/appinfo/app.php index a39464e21f..97d7e8f748 100644 --- a/apps/encryption/appinfo/app.php +++ b/apps/encryption/appinfo/app.php @@ -28,7 +28,8 @@ namespace OCA\Encryption\AppInfo; $encryptionSystemReady = \OC::$server->getEncryptionManager()->isReady(); -$app = new Application(); +/** @var Application $app */ +$app = \OC::$server->query(Application::class); if ($encryptionSystemReady) { $app->registerEncryptionModule(); $app->registerHooks(); diff --git a/apps/encryption/appinfo/routes.php b/apps/encryption/appinfo/routes.php index 22f3af7fbf..4c01dd57ad 100644 --- a/apps/encryption/appinfo/routes.php +++ b/apps/encryption/appinfo/routes.php @@ -24,7 +24,9 @@ namespace OCA\Encryption\AppInfo; -(new Application())->registerRoutes($this, array('routes' => array( +/** @var Application $app */ +$app = \OC::$server->query(Application::class); +$app->registerRoutes($this, array('routes' => array( [ 'name' => 'Recovery#adminRecovery', diff --git a/apps/federation/appinfo/app.php b/apps/federation/appinfo/app.php index 6c53810dd2..23be2dd212 100644 --- a/apps/federation/appinfo/app.php +++ b/apps/federation/appinfo/app.php @@ -22,5 +22,6 @@ namespace OCA\Federation\AppInfo; -$app = new Application(); +/** @var Application $app */ +$app = \OC::$server->query(Application::class); $app->registerHooks(); diff --git a/apps/files/appinfo/routes.php b/apps/files/appinfo/routes.php index b085d79b8c..26fce8d171 100644 --- a/apps/files/appinfo/routes.php +++ b/apps/files/appinfo/routes.php @@ -29,7 +29,8 @@ declare(strict_types=1); */ namespace OCA\Files\AppInfo; -$application = new Application(); +/** @var Application $application */ +$application = \OC::$server->query(Application::class); $application->registerRoutes( $this, [ diff --git a/apps/files_trashbin/appinfo/routes.php b/apps/files_trashbin/appinfo/routes.php index 20d52adf3f..8f5530d644 100644 --- a/apps/files_trashbin/appinfo/routes.php +++ b/apps/files_trashbin/appinfo/routes.php @@ -24,7 +24,8 @@ namespace OCA\Files_Trashbin\AppInfo; -$application = new Application(); +/** @var Application $application */ +$application = \OC::$server->query(Application::class); $application->registerRoutes($this, [ 'routes' => [ [ diff --git a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php index e19b7ce604..919317a7b7 100644 --- a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php +++ b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php @@ -62,7 +62,8 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { } protected function fixDIForJobs() { - $application = new Application(); + /** @var Application $application */ + $application = \OC::$server->query(Application::class); $this->userManager = \OC::$server->getUserManager(); $this->expiration = $application->getContainer()->query('Expiration'); } diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index a06a5145d9..bb1c9c172e 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -747,7 +747,8 @@ class Trashbin { */ private static function scheduleExpire($user) { // let the admin disable auto expire - $application = new Application(); + /** @var Application $application */ + $application = \OC::$server->query(Application::class); $expiration = $application->getContainer()->query('Expiration'); if ($expiration->isEnabled()) { \OC::$server->getCommandBus()->push(new Expire($user)); @@ -764,7 +765,8 @@ class Trashbin { * @return int size of deleted files */ protected static function deleteFiles($files, $user, $availableSpace) { - $application = new Application(); + /** @var Application $application */ + $application = \OC::$server->query(Application::class); $expiration = $application->getContainer()->query('Expiration'); $size = 0; @@ -791,7 +793,8 @@ class Trashbin { * @return integer[] size of deleted files and number of deleted files */ public static function deleteExpiredFiles($files, $user) { - $application = new Application(); + /** @var Application $application */ + $application = \OC::$server->query(Application::class); $expiration = $application->getContainer()->query('Expiration'); $size = 0; $count = 0; diff --git a/apps/files_versions/appinfo/routes.php b/apps/files_versions/appinfo/routes.php index 5fe6eaaee1..95722dd778 100644 --- a/apps/files_versions/appinfo/routes.php +++ b/apps/files_versions/appinfo/routes.php @@ -26,7 +26,8 @@ namespace OCA\Files_Versions\AppInfo; -$application = new Application(); +/** @var Application $application */ +$application = \OC::$server->query(Application::class); $application->registerRoutes($this, [ 'routes' => [ [ diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index c42083ff35..168145ffec 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -850,8 +850,8 @@ class Storage { * @return Expiration */ protected static function getExpiration(){ - if (is_null(self::$application)) { - self::$application = new Application(); + if (self::$application === null) { + self::$application = \OC::$server->query(Application::class); } return self::$application->getContainer()->query(Expiration::class); } diff --git a/apps/provisioning_api/lib/FederatedFileSharingFactory.php b/apps/provisioning_api/lib/FederatedFileSharingFactory.php index e856b2a9be..0c7bb7068d 100644 --- a/apps/provisioning_api/lib/FederatedFileSharingFactory.php +++ b/apps/provisioning_api/lib/FederatedFileSharingFactory.php @@ -25,9 +25,19 @@ declare(strict_types=1); namespace OCA\Provisioning_API; use OCA\FederatedFileSharing\AppInfo\Application; +use OCP\IServerContainer; class FederatedFileSharingFactory { - public function get(): Application { - return new Application(); + + /** @var IServerContainer */ + private $serverContainer; + + public function __construct(IServerContainer $serverContainer) { + $this->serverContainer = $serverContainer; } + + public function get(): Application { + return $this->serverContainer->query(Application::class); + } + } diff --git a/apps/settings/appinfo/routes.php b/apps/settings/appinfo/routes.php index b55bea0de6..6e7ee45619 100644 --- a/apps/settings/appinfo/routes.php +++ b/apps/settings/appinfo/routes.php @@ -38,7 +38,8 @@ namespace OCA\Settings; use OCA\Settings\AppInfo\Application; -$application = new Application(); +/** @var Application $application */ +$application = \OC::$server->query(Application::class); $this->useCollection('root'); $application->registerRoutes($this, [ 'resources' => [ diff --git a/apps/settings/lib/Settings/Personal/PersonalInfo.php b/apps/settings/lib/Settings/Personal/PersonalInfo.php index b0c4d6faf4..1cf81ec965 100644 --- a/apps/settings/lib/Settings/Personal/PersonalInfo.php +++ b/apps/settings/lib/Settings/Personal/PersonalInfo.php @@ -90,7 +90,7 @@ class PersonalInfo implements ISettings { $federatedFileSharingEnabled = $this->appManager->isEnabledForUser('federatedfilesharing'); $lookupServerUploadEnabled = false; if($federatedFileSharingEnabled) { - $federatedFileSharing = new Application(); + $federatedFileSharing = \OC::$server->query(Application::class); $shareProvider = $federatedFileSharing->getFederatedShareProvider(); $lookupServerUploadEnabled = $shareProvider->isLookupServerUploadEnabled(); } diff --git a/core/routes.php b/core/routes.php index 829fa8576c..0e6efae59b 100644 --- a/core/routes.php +++ b/core/routes.php @@ -34,7 +34,8 @@ use OC\Core\Application; -$application = new Application(); +/** @var Application $application */ +$application = \OC::$server->query(Application::class); $application->registerRoutes($this, [ 'routes' => [ ['name' => 'lost#email', 'url' => '/lostpassword/email', 'verb' => 'POST'], From 12c13b86d878986e180c4383541a11dbfb90f8b8 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 14 Oct 2019 22:59:16 +0200 Subject: [PATCH 3/6] fixup! Do not create Application instances directly Signed-off-by: Roeland Jago Douma --- apps/encryption/appinfo/routes.php | 73 ++++++++++++-------------- apps/files_trashbin/appinfo/routes.php | 9 ++-- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/apps/encryption/appinfo/routes.php b/apps/encryption/appinfo/routes.php index 4c01dd57ad..f7eb7ff577 100644 --- a/apps/encryption/appinfo/routes.php +++ b/apps/encryption/appinfo/routes.php @@ -1,4 +1,5 @@ query(Application::class); -$app->registerRoutes($this, array('routes' => array( - - [ - 'name' => 'Recovery#adminRecovery', - 'url' => '/ajax/adminRecovery', - 'verb' => 'POST' - ], - [ - 'name' => 'Settings#updatePrivateKeyPassword', - 'url' => '/ajax/updatePrivateKeyPassword', - 'verb' => 'POST' - ], - [ - 'name' => 'Settings#setEncryptHomeStorage', - 'url' => '/ajax/setEncryptHomeStorage', - 'verb' => 'POST' - ], - [ - 'name' => 'Recovery#changeRecoveryPassword', - 'url' => '/ajax/changeRecoveryPassword', - 'verb' => 'POST' - ], - [ - 'name' => 'Recovery#userSetRecovery', - 'url' => '/ajax/userSetRecovery', - 'verb' => 'POST' - ], - [ - 'name' => 'Status#getStatus', - 'url' => '/ajax/getStatus', - 'verb' => 'GET' +return [ + 'routes' => [ + [ + 'name' => 'Recovery#adminRecovery', + 'url' => '/ajax/adminRecovery', + 'verb' => 'POST' + ], + [ + 'name' => 'Settings#updatePrivateKeyPassword', + 'url' => '/ajax/updatePrivateKeyPassword', + 'verb' => 'POST' + ], + [ + 'name' => 'Settings#setEncryptHomeStorage', + 'url' => '/ajax/setEncryptHomeStorage', + 'verb' => 'POST' + ], + [ + 'name' => 'Recovery#changeRecoveryPassword', + 'url' => '/ajax/changeRecoveryPassword', + 'verb' => 'POST' + ], + [ + 'name' => 'Recovery#userSetRecovery', + 'url' => '/ajax/userSetRecovery', + 'verb' => 'POST' + ], + [ + 'name' => 'Status#getStatus', + 'url' => '/ajax/getStatus', + 'verb' => 'GET' + ], ] - - -))); +]; diff --git a/apps/files_trashbin/appinfo/routes.php b/apps/files_trashbin/appinfo/routes.php index 8f5530d644..9f85e27bed 100644 --- a/apps/files_trashbin/appinfo/routes.php +++ b/apps/files_trashbin/appinfo/routes.php @@ -1,4 +1,5 @@ @@ -22,11 +23,7 @@ * */ -namespace OCA\Files_Trashbin\AppInfo; - -/** @var Application $application */ -$application = \OC::$server->query(Application::class); -$application->registerRoutes($this, [ +return [ 'routes' => [ [ 'name' => 'Preview#getPreview', @@ -34,4 +31,4 @@ $application->registerRoutes($this, [ 'verb' => 'GET', ], ], -]); +]; From 78d00ff0852a6199cc7ca33db6308f7bdd09b5a6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 31 Oct 2019 19:36:34 +0100 Subject: [PATCH 4/6] fix tests? Signed-off-by: Roeland Jago Douma --- apps/files_trashbin/tests/TrashbinTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php index ae10e27307..590e044b72 100644 --- a/apps/files_trashbin/tests/TrashbinTest.php +++ b/apps/files_trashbin/tests/TrashbinTest.php @@ -67,7 +67,7 @@ class TrashbinTest extends \Test\TestCase { // clear share hooks \OC_Hook::clear('OCP\\Share'); \OC::registerShareHooks(); - $application = new \OCA\Files_Sharing\AppInfo\Application(); + $application = \OC::$server->query(\OCA\Files_Sharing\AppInfo\Application::class); $application->registerMountProviders(); //disable encryption From e225a7bfd33f4ff986c9adf1f237a1c9c6b597a4 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 31 Oct 2019 21:27:31 +0100 Subject: [PATCH 5/6] fixup! fix tests? Signed-off-by: Roeland Jago Douma --- apps/files_trashbin/tests/TrashbinTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php index 590e044b72..3263feb4a9 100644 --- a/apps/files_trashbin/tests/TrashbinTest.php +++ b/apps/files_trashbin/tests/TrashbinTest.php @@ -152,7 +152,9 @@ class TrashbinTest extends \Test\TestCase { */ public function testExpireOldFiles() { - $currentTime = time(); + /** @var \OCP\AppFramework\Utility\ITimeFactory $time */ + $time = \OC::$server->query(\OCP\AppFramework\Utility\ITimeFactory::class); + $currentTime = $time->getTime(); $expireAt = $currentTime - 2 * 24 * 60 * 60; $expiredDate = $currentTime - 3 * 24 * 60 * 60; From 964dc0a95520efbf876e0113bfafc294f4f4b322 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 4 Nov 2019 16:54:22 +0100 Subject: [PATCH 6/6] set retention obligation on existing `expiration` in tests Signed-off-by: Robin Appelman --- apps/files_trashbin/lib/AppInfo/Application.php | 7 +------ apps/files_trashbin/lib/Expiration.php | 6 +++++- apps/files_trashbin/lib/Trashbin.php | 5 ++--- apps/files_trashbin/tests/TrashbinTest.php | 12 ++++++++---- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/apps/files_trashbin/lib/AppInfo/Application.php b/apps/files_trashbin/lib/AppInfo/Application.php index 4baa82a6b4..a2613a13b4 100644 --- a/apps/files_trashbin/lib/AppInfo/Application.php +++ b/apps/files_trashbin/lib/AppInfo/Application.php @@ -46,12 +46,7 @@ class Application extends App { /* * Register expiration */ - $container->registerService('Expiration', function($c) { - return new Expiration( - $c->query('ServerContainer')->getConfig(), - $c->query(ITimeFactory::class) - ); - }); + $container->registerAlias('Expiration', Expiration::class); /* * Register $principalBackend for the DAV collection diff --git a/apps/files_trashbin/lib/Expiration.php b/apps/files_trashbin/lib/Expiration.php index c7ad4e29f1..933375c953 100644 --- a/apps/files_trashbin/lib/Expiration.php +++ b/apps/files_trashbin/lib/Expiration.php @@ -49,7 +49,11 @@ class Expiration { public function __construct(IConfig $config,ITimeFactory $timeFactory){ $this->timeFactory = $timeFactory; - $this->retentionObligation = $config->getSystemValue('trashbin_retention_obligation', 'auto'); + $this->setRetentionObligation($config->getSystemValue('trashbin_retention_obligation', 'auto')); + } + + public function setRetentionObligation(string $obligation) { + $this->retentionObligation = $obligation; if ($this->retentionObligation !== 'disabled') { $this->parseRetentionObligation(); diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index bb1c9c172e..34138c2170 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -793,9 +793,8 @@ class Trashbin { * @return integer[] size of deleted files and number of deleted files */ public static function deleteExpiredFiles($files, $user) { - /** @var Application $application */ - $application = \OC::$server->query(Application::class); - $expiration = $application->getContainer()->query('Expiration'); + /** @var Expiration $expiration */ + $expiration = \OC::$server->query(Expiration::class); $size = 0; $count = 0; foreach ($files as $file) { diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php index 3263feb4a9..22b6e87317 100644 --- a/apps/files_trashbin/tests/TrashbinTest.php +++ b/apps/files_trashbin/tests/TrashbinTest.php @@ -76,7 +76,9 @@ class TrashbinTest extends \Test\TestCase { $config = \OC::$server->getConfig(); //configure trashbin self::$rememberRetentionObligation = $config->getSystemValue('trashbin_retention_obligation', \OCA\Files_Trashbin\Expiration::DEFAULT_RETENTION_OBLIGATION); - $config->setSystemValue('trashbin_retention_obligation', 'auto, 2'); + /** @var \OCA\Files_Trashbin\Expiration $expiration */ + $expiration = \OC::$server->query(\OCA\Files_Trashbin\Expiration::class); + $expiration->setRetentionObligation('auto, 2'); // register hooks \OCA\Files_Trashbin\Trashbin::registerHooks(); @@ -94,7 +96,9 @@ class TrashbinTest extends \Test\TestCase { $user->delete(); } - \OC::$server->getConfig()->setSystemValue('trashbin_retention_obligation', self::$rememberRetentionObligation); + /** @var \OCA\Files_Trashbin\Expiration $expiration */ + $expiration = \OC::$server->query(\OCA\Files_Trashbin\Expiration::class); + $expiration->setRetentionObligation(self::$rememberRetentionObligation); \OC_Hook::clear(); @@ -686,9 +690,9 @@ class TrashbinForTesting extends \OCA\Files_Trashbin\Trashbin { * @param OCP\Files\FileInfo[] $files * @param integer $limit */ - public function dummyDeleteExpiredFiles($files, $limit) { + public function dummyDeleteExpiredFiles($files) { // dummy value for $retention_obligation because it is not needed here - return parent::deleteExpiredFiles($files, TrashbinTest::TEST_TRASHBIN_USER1, $limit, 0); + return parent::deleteExpiredFiles($files, TrashbinTest::TEST_TRASHBIN_USER1); } /**