Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor OC\Server::getAppManager #40113

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions core/register_command.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
use Psr\Log\LoggerInterface;

use OCP\App\IAppManager;
use Psr\Log\LoggerInterface;
Comment on lines +52 to +53
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use OCP\App\IAppManager;
use Psr\Log\LoggerInterface;
use OCP\App\IAppManager;
use Psr\Log\LoggerInterface;


$application->add(new \Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand());
$application->add(new OC\Core\Command\Status(\OC::$server->get(\OCP\IConfig::class), \OC::$server->get(\OCP\Defaults::class)));
Expand All @@ -72,12 +74,12 @@


if (\OC::$server->getConfig()->getSystemValue('installed', false)) {
$application->add(new OC\Core\Command\App\Disable(\OC::$server->getAppManager()));
$application->add(new OC\Core\Command\App\Enable(\OC::$server->getAppManager(), \OC::$server->getGroupManager()));
$application->add(new OC\Core\Command\App\Disable(\OC::$server->get(IAppManager::class)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$application->add(new OC\Core\Command\App\Disable(\OC::$server->get(IAppManager::class)));
$application->add(\OCP\Server::get(\OC\Core\Command\App\Disable::class));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for all others of course.

Copy link
Contributor Author

@summersab summersab Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add these changes to this PR if you would like, but should it be in its own PR? Either way works for me.

Shouldn't it be this (or something similar), though?

$application->add(\OCP\Server::get(\OC\Core\Command\App\Disable::class)(\OC::$server->get(IAppManager::class)));

Copy link
Member

@nickvergessen nickvergessen Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well see this urgent comment before continuing the work: #40083 (comment)

Shouldn't it be this (or something similar), though?

The code you provided does not even parse in PHP. No, my suggested code basically removes all "manual injection of dependencies" and replaces it by "automatically injecting the actual command" which then on it's own will get all it's dependencies, instead of us listing hundreds of get() parameters here.

It might not work for all commands (some seem to get non-DI-able objects like new View() as a parameter), but should work for most of them.

$application->add(new OC\Core\Command\App\Enable(\OC::$server->get(IAppManager::class), \OC::$server->getGroupManager()));
summersab marked this conversation as resolved.
Show resolved Hide resolved
$application->add(new OC\Core\Command\App\Install());
$application->add(new OC\Core\Command\App\GetPath());
$application->add(new OC\Core\Command\App\ListApps(\OC::$server->getAppManager()));
$application->add(new OC\Core\Command\App\Remove(\OC::$server->getAppManager(), \OC::$server->query(\OC\Installer::class), \OC::$server->get(LoggerInterface::class)));
$application->add(new OC\Core\Command\App\ListApps(\OC::$server->get(IAppManager::class)));
$application->add(new OC\Core\Command\App\Remove(\OC::$server->get(IAppManager::class), \OC::$server->query(\OC\Installer::class), \OC::$server->get(LoggerInterface::class)));
summersab marked this conversation as resolved.
Show resolved Hide resolved
$application->add(\OC::$server->query(\OC\Core\Command\App\Update::class));

$application->add(\OC::$server->query(\OC\Core\Command\TwoFactorAuth\Cleanup::class));
Expand Down Expand Up @@ -116,7 +118,7 @@
if (\OC::$server->getConfig()->getSystemValueBool('debug', false)) {
$application->add(new OC\Core\Command\Db\Migrations\StatusCommand(\OC::$server->get(\OC\DB\Connection::class)));
$application->add(new OC\Core\Command\Db\Migrations\MigrateCommand(\OC::$server->get(\OC\DB\Connection::class)));
$application->add(new OC\Core\Command\Db\Migrations\GenerateCommand(\OC::$server->get(\OC\DB\Connection::class), \OC::$server->getAppManager()));
$application->add(new OC\Core\Command\Db\Migrations\GenerateCommand(\OC::$server->get(\OC\DB\Connection::class), \OC::$server->get(IAppManager::class)));
$application->add(new OC\Core\Command\Db\Migrations\ExecuteCommand(\OC::$server->get(\OC\DB\Connection::class), \OC::$server->getConfig()));
}

Expand All @@ -125,10 +127,10 @@
$application->add(new OC\Core\Command\Encryption\ListModules(\OC::$server->getEncryptionManager(), \OC::$server->getConfig()));
$application->add(new OC\Core\Command\Encryption\SetDefaultModule(\OC::$server->getEncryptionManager(), \OC::$server->getConfig()));
$application->add(new OC\Core\Command\Encryption\Status(\OC::$server->getEncryptionManager()));
$application->add(new OC\Core\Command\Encryption\EncryptAll(\OC::$server->getEncryptionManager(), \OC::$server->getAppManager(), \OC::$server->getConfig(), new \Symfony\Component\Console\Helper\QuestionHelper()));
$application->add(new OC\Core\Command\Encryption\EncryptAll(\OC::$server->getEncryptionManager(), \OC::$server->get(IAppManager::class), \OC::$server->getConfig(), new \Symfony\Component\Console\Helper\QuestionHelper()));
summersab marked this conversation as resolved.
Show resolved Hide resolved
summersab marked this conversation as resolved.
Show resolved Hide resolved
$application->add(new OC\Core\Command\Encryption\DecryptAll(
\OC::$server->getEncryptionManager(),
\OC::$server->getAppManager(),
\OC::$server->get(IAppManager::class),
\OC::$server->getConfig(),
new \OC\Encryption\DecryptAll(\OC::$server->getEncryptionManager(), \OC::$server->getUserManager(), new \OC\Files\View()),
new \Symfony\Component\Console\Helper\QuestionHelper())
Expand Down Expand Up @@ -174,7 +176,7 @@
new \OC\Repair([], \OC::$server->get(\OCP\EventDispatcher\IEventDispatcher::class), \OC::$server->get(LoggerInterface::class)),
\OC::$server->getConfig(),
\OC::$server->get(\OCP\EventDispatcher\IEventDispatcher::class),
\OC::$server->getAppManager()
\OC::$server->get(IAppManager::class)
));
$application->add(\OC::$server->query(OC\Core\Command\Maintenance\RepairShareOwnership::class));

Expand All @@ -188,7 +190,7 @@
$application->add(new OC\Core\Command\User\Enable(\OC::$server->getUserManager()));
$application->add(new OC\Core\Command\User\LastSeen(\OC::$server->getUserManager()));
$application->add(\OC::$server->get(\OC\Core\Command\User\Report::class));
$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager(), \OC::$server->getAppManager()));
$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager(), \OC::$server->get(IAppManager::class)));
$application->add(new OC\Core\Command\User\Setting(\OC::$server->getUserManager(), \OC::$server->getConfig()));
$application->add(new OC\Core\Command\User\ListCommand(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
$application->add(new OC\Core\Command\User\Info(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
Expand Down
3 changes: 2 additions & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
use OC\ServerContainer;
use OC\Settings\AuthorizedGroupMapper;
use OCA\WorkflowEngine\Manager;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\IOutput;
use OCP\AppFramework\IAppContainer;
use OCP\AppFramework\QueryException;
Expand Down Expand Up @@ -255,7 +256,7 @@ public function __construct(string $appName, array $urlParams = [], ServerContai
$server->getUserSession()->isLoggedIn(),
$this->getUserId() !== null && $server->getGroupManager()->isAdmin($this->getUserId()),
$server->getUserSession()->getUser() !== null && $server->query(ISubAdmin::class)->isSubAdmin($server->getUserSession()->getUser()),
$server->getAppManager(),
$server->get(IAppManager::class),
$server->getL10N('lib'),
$c->get(AuthorizedGroupMapper::class),
$server->get(IUserSession::class)
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ public function isDownloaded($name) {
*/
public function removeApp($appId) {
if ($this->isDownloaded($appId)) {
if (\OC::$server->getAppManager()->isShipped($appId)) {
if (\OC::$server->get(IAppManager::class)->isShipped($appId)) {
return false;
}
$appDir = OC_App::getInstallPath() . '/' . $appId;
Expand Down Expand Up @@ -537,7 +537,7 @@ public function installAppBundle(Bundle $bundle) {
* @return array Array of error messages (appid => Exception)
*/
public static function installShippedApps($softErrors = false) {
$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
$config = \OC::$server->getConfig();
$errors = [];
foreach (\OC::$APPSROOTS as $app_dir) {
Expand Down
9 changes: 5 additions & 4 deletions lib/private/Share20/ProviderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use OCA\ShareByMail\Settings\SettingsManager;
use OCA\ShareByMail\ShareByMailProvider;
use OCA\Talk\Share\RoomShareProvider;
use OCP\App\IAppManager;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IServerContainer;
Expand Down Expand Up @@ -121,7 +122,7 @@ protected function federatedShareProvider() {
/*
* Check if the app is enabled
*/
$appManager = $this->serverContainer->getAppManager();
$appManager = $this->serverContainer->get(IAppManager::class);
if (!$appManager->isEnabledForUser('federatedfilesharing')) {
return null;
}
Expand Down Expand Up @@ -178,7 +179,7 @@ protected function getShareByMailProvider() {
/*
* Check if the app is enabled
*/
$appManager = $this->serverContainer->getAppManager();
$appManager = $this->serverContainer->get(IAppManager::class);
if (!$appManager->isEnabledForUser('sharebymail')) {
return null;
}
Expand Down Expand Up @@ -220,7 +221,7 @@ protected function getShareByCircleProvider() {
return null;
}

if (!$this->serverContainer->getAppManager()->isEnabledForUser('circles') ||
if (!$this->serverContainer->get(IAppManager::class)->isEnabledForUser('circles') ||
!class_exists('\OCA\Circles\ShareByCircleProvider')
) {
$this->circlesAreNotAvailable = true;
Expand Down Expand Up @@ -252,7 +253,7 @@ protected function getRoomShareProvider() {
/*
* Check if the app is enabled
*/
$appManager = $this->serverContainer->getAppManager();
$appManager = $this->serverContainer->get(IAppManager::class);
if (!$appManager->isEnabledForUser('spreed')) {
return null;
}
Expand Down
13 changes: 7 additions & 6 deletions lib/private/TemplateLayout.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
use OC\Template\CSSResourceLocator;
use OC\Template\JSConfigHelper;
use OC\Template\JSResourceLocator;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\Defaults;
use OCP\IConfig;
Expand Down Expand Up @@ -87,7 +88,7 @@ public function __construct($renderAs, $appId = '') {

// Add fallback theming variables if theming is disabled
if ($renderAs !== TemplateResponse::RENDER_AS_USER
|| !\OC::$server->getAppManager()->isEnabledForUser('theming')) {
|| !\OC::$server->get(IAppManager::class)->isEnabledForUser('theming')) {
// TODO cache generated default theme if enabled for fallback if server is erroring ?
Util::addStyle('theming', 'default');
}
Expand All @@ -113,7 +114,7 @@ public function __construct($renderAs, $appId = '') {

// Set body data-theme
$this->assign('enabledThemes', []);
if (\OC::$server->getAppManager()->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) {
if (\OC::$server->get(IAppManager::class)->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) {
/** @var \OCA\Theming\Service\ThemesService */
$themesService = \OC::$server->get(\OCA\Theming\Service\ThemesService::class);
$this->assign('enabledThemes', $themesService->getEnabledThemes());
Expand All @@ -124,8 +125,8 @@ public function __construct($renderAs, $appId = '') {
$this->assign('logoUrl', $logoUrl);

// Set default app name
$defaultApp = \OC::$server->getAppManager()->getDefaultAppForUser();
$defaultAppInfo = \OC::$server->getAppManager()->getAppInfo($defaultApp);
$defaultApp = \OC::$server->get(IAppManager::class)->getDefaultAppForUser();
$defaultAppInfo = \OC::$server->get(IAppManager::class)->getAppInfo($defaultApp);
$l10n = \OC::$server->getL10NFactory()->get($defaultApp);
$this->assign('defaultAppName', $l10n->t($defaultAppInfo['name']));

Expand Down Expand Up @@ -227,7 +228,7 @@ public function __construct($renderAs, $appId = '') {
$jsConfigHelper = new JSConfigHelper(
\OC::$server->getL10N('lib'),
\OCP\Server::get(Defaults::class),
\OC::$server->getAppManager(),
\OC::$server->get(IAppManager::class),
\OC::$server->getSession(),
\OC::$server->getUserSession()->getUser(),
$this->config,
Expand Down Expand Up @@ -312,7 +313,7 @@ protected function getVersionHashSuffix($path = false, $file = false) {
$v = [];

if ($this->config->getSystemValueBool('installed', false)) {
if (\OC::$server->getAppManager()->isInstalled('theming')) {
if (\OC::$server->get(IAppManager::class)->isInstalled('theming')) {
$themingSuffix = '-' . $this->config->getAppValue('theming', 'cachebuster', '0');
}
$v = \OC_App::getAppVersions();
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Updater.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ private function doUpgrade(string $currentVersion, string $installedVersion): vo
\OC::$server->getAppFetcher()->setVersion($currentVersion);

/** @var AppManager $appManager */
$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);

// upgrade appstore apps
$this->upgradeAppStoreApps($appManager->getInstalledApps());
Expand Down Expand Up @@ -382,7 +382,7 @@ private function checkAppsRequirements(): void {
$isCoreUpgrade = $this->isCodeUpgrade();
$apps = OC_App::getEnabledApps();
$version = implode('.', Util::getVersion());
$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
foreach ($apps as $app) {
// check if the app is compatible with this version of Nextcloud
$info = $appManager->getAppInfo($app);
Expand Down
22 changes: 11 additions & 11 deletions lib/private/legacy/OC_App.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public static function isType(string $app, array $types): bool {
* read app types from info.xml and cache them in the database
*/
public static function setAppTypes(string $app) {
$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
$appData = $appManager->getAppInfo($app);
if (!is_array($appData)) {
return;
Expand Down Expand Up @@ -221,7 +221,7 @@ public static function getEnabledApps(bool $forceRefresh = false, bool $all = fa
}
// in incognito mode or when logged out, $user will be false,
// which is also the case during an upgrade
$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
if ($all) {
$user = null;
} else {
Expand Down Expand Up @@ -264,7 +264,7 @@ public function enable(string $appId,

$installer->installApp($appId);

$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
if ($groups !== []) {
$groupManager = \OC::$server->getGroupManager();
$groupsList = [];
Expand Down Expand Up @@ -354,7 +354,7 @@ public static function findAppInDirectories(string $appId, bool $ignoreCache = f
* @param string $appId
* @param bool $refreshAppPath should be set to true only during install/upgrade
* @return string|false
* @deprecated 11.0.0 use \OC::$server->getAppManager()->getAppPath()
* @deprecated 11.0.0 use \OC::$server->get(\OCP\App\IAppManager::class)->getAppPath()
*/
public static function getAppPath(string $appId, bool $refreshAppPath = false) {
if ($appId === null || trim($appId) === '') {
Expand All @@ -373,7 +373,7 @@ public static function getAppPath(string $appId, bool $refreshAppPath = false) {
*
* @param string $appId
* @return string|false
* @deprecated 18.0.0 use \OC::$server->getAppManager()->getAppWebPath()
* @deprecated 18.0.0 use \OC::$server->get(\OCP\App\IAppManager::class)->getAppWebPath()
*/
public static function getAppWebPath(string $appId) {
if (($dir = self::findAppInDirectories($appId)) != false) {
Expand All @@ -390,7 +390,7 @@ public static function getAppWebPath(string $appId) {
*/
public static function getAppVersionByPath(string $path): string {
$infoFile = $path . '/appinfo/info.xml';
$appData = \OC::$server->getAppManager()->getAppInfo($infoFile, true);
$appData = \OC::$server->get(IAppManager::class)->getAppInfo($infoFile, true);
return isset($appData['version']) ? $appData['version'] : '';
}

Expand Down Expand Up @@ -556,7 +556,7 @@ public function getSupportedApps(): array {
public function listAllApps(): array {
$installedApps = OC_App::getAllApps();

$appManager = \OC::$server->getAppManager();
$appManager = \OC::$server->get(IAppManager::class);
//we don't want to show configuration for these
$blacklist = $appManager->getAlwaysEnabledApps();
$appList = [];
Expand Down Expand Up @@ -756,7 +756,7 @@ public static function updateApp(string $appId): bool {
return false;
}

\OC::$server->getAppManager()->clearAppsCache();
\OC::$server->get(IAppManager::class)->clearAppsCache();
$l = \OC::$server->getL10N('core');
$appData = \OCP\Server::get(\OCP\App\IAppManager::class)->getAppInfo($appId, false, $l->getLanguageCode());

Expand All @@ -778,8 +778,8 @@ public static function updateApp(string $appId): bool {
self::executeRepairSteps($appId, $appData['repair-steps']['post-migration']);
self::setupLiveMigrations($appId, $appData['repair-steps']['live-migration']);
// update appversion in app manager
\OC::$server->getAppManager()->clearAppsCache();
\OC::$server->getAppManager()->getAppVersion($appId, false);
\OC::$server->get(IAppManager::class)->clearAppsCache();
\OC::$server->get(IAppManager::class)->getAppVersion($appId, false);

self::setupBackgroundJobs($appData['background-jobs']);

Expand Down Expand Up @@ -862,7 +862,7 @@ private static function setupLiveMigrations(string $appId, array $steps) {
* @return \OC\Files\View|false
*/
public static function getStorage(string $appId) {
if (\OC::$server->getAppManager()->isEnabledForUser($appId)) { //sanity check
if (\OC::$server->get(IAppManager::class)->isEnabledForUser($appId)) { //sanity check
if (\OC::$server->getUserSession()->isLoggedIn()) {
$view = new \OC\Files\View('/' . OC_User::getUser());
if (!$view->file_exists($appId)) {
Expand Down
5 changes: 4 additions & 1 deletion lib/private/legacy/OC_JSON.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

use OCP\App\IAppManager;

class OC_JSON {
/**
* Check if the app is enabled, send json error msg if not
Expand All @@ -35,7 +38,7 @@ class OC_JSON {
* @suppress PhanDeprecatedFunction
*/
public static function checkAppEnabled($app) {
if (!\OC::$server->getAppManager()->isEnabledForUser($app)) {
if (!\OC::$server->get(IAppManager::class)->isEnabledForUser($app)) {
$l = \OC::$server->getL10N('lib');
self::error([ 'data' => [ 'message' => $l->t('Application is not enabled'), 'error' => 'application_not_enabled' ]]);
exit();
Expand Down
3 changes: 2 additions & 1 deletion lib/private/legacy/OC_Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
*
*/
use OC\TemplateLayout;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\TemplateResponse;

require_once __DIR__.'/template/functions.php';
Expand Down Expand Up @@ -240,7 +241,7 @@ public static function printGuestPage($application, $name, $parameters = []) {
* @suppress PhanAccessMethodInternal
*/
public static function printErrorPage($error_msg, $hint = '', $statusCode = 500) {
if (\OC::$server->getAppManager()->isEnabledForUser('theming') && !\OC_App::isAppLoaded('theming')) {
if (\OC::$server->get(IAppManager::class)->isEnabledForUser('theming') && !\OC_App::isAppLoaded('theming')) {
\OC_App::loadApp('theming');
}

Expand Down
3 changes: 2 additions & 1 deletion lib/private/legacy/OC_Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

use bantu\IniGetWrapper\IniGetWrapper;
use OC\Files\SetupManager;
use OCP\App\IAppManager;
use OCP\Files\Template\ITemplateManager;
use OCP\IConfig;
use OCP\IGroupManager;
Expand All @@ -83,7 +84,7 @@ class OC_Util {
private static $versionCache = null;

protected static function getAppManager() {
return \OC::$server->getAppManager();
return \OC::$server->get(IAppManager::class);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion ocm-provider/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@

require_once __DIR__ . '/../lib/base.php';

use OCP\App\IAppManager;

header('Content-Type: application/json');

$server = \OC::$server;

$isEnabled = $server->getAppManager()->isEnabledForUser('cloud_federation_api');
$isEnabled = $server->get(IAppManager::class)->isEnabledForUser('cloud_federation_api');

if ($isEnabled) {
// Make sure the routes are loaded
Expand Down
4 changes: 3 additions & 1 deletion ocs-provider/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@

require_once __DIR__ . '/../lib/base.php';

use OCP\App\IAppManager;

header('Content-Type: application/json');

$server = \OC::$server;

$controller = new \OC\OCS\Provider(
'ocs_provider',
$server->getRequest(),
$server->getAppManager()
$server->get(IAppManager::class)
);
echo $controller->buildProviderList()->render();
Loading
Loading