Skip to content

Migrate away from OC_App and toward IAppManager. #44025

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.

Sign up for GitHub

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

Jump to bottom
Merged
come-nc merged 8 commits into from
Apr 22, 2024
Merged

Migrate away from OC_App and toward IAppManager. #44025

come-nc merged 8 commits into from
Apr 22, 2024

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Mar 6, 2024 •
edited
Loading

Summary

Migrate away from OC_App and toward IAppManager.

Checklist

@come-nc come-nc added the 2. developing Work in progress label Mar 6, 2024
@come-nc come-nc self-assigned this Mar 6, 2024
@come-nc come-nc force-pushed the fix/remove-oc-app-calls branch from 173f7f9 to f281499 Compare March 6, 2024 11:13
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 6, 2024
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 6, 2024
View reviewed changes
lib/private/App/AppManager.php
private function getInstalledAppsValues(): array {
if (!$this->installedAppsCache) {
$values = $this->appConfig->getValues(false, 'enabled');
$values = $this->getAppConfig()->getValues(false, 'enabled');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::getValues has been marked as deprecated
@come-nc come-nc force-pushed the fix/remove-oc-app-calls branch from 01b858c to babdd5a Compare March 6, 2024 15:00
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 6, 2024
View reviewed changes
lib/private/App/AppManager.php
//load the cache
if (count($this->appTypes) === 0) {
$this->appTypes = $this->appConfig->getValues(false, 'types') ?: [];
$this->appTypes = $this->getAppConfig()->getValues(false, 'types') ?: [];

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::getValues has been marked as deprecated
@come-nc come-nc changed the title Fix/remove oc app calls Migrate away from OC_App and toward IAppManager. Mar 6, 2024
@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 6, 2024
@come-nc come-nc marked this pull request as ready for review March 6, 2024 21:45
@come-nc come-nc requested review from a team, ArtificialOwl, ChristophWurst, icewind1991, nfebe and nickvergessen and removed request for a team March 7, 2024 11:45
@come-nc come-nc force-pushed the fix/remove-oc-app-calls branch from 81bc004 to 3083a2e Compare March 7, 2024 13:33
ChristophWurst
ChristophWurst approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

To avoid some kaboom we might want to merge this after the 29 branch-off

@come-nc come-nc added this to the Nextcloud 30 milestone Mar 7, 2024
@come-nc come-nc force-pushed the fix/remove-oc-app-calls branch from 3083a2e to 1b5a0c3 Compare April 22, 2024 10:22
@come-nc
Copy link
Contributor Author

come-nc commented Apr 22, 2024

Rebased and fixed conflicts

nickvergessen
nickvergessen reviewed Apr 22, 2024
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 22, 2024
View reviewed changes
lib/private/App/AppManager.php

$this->installedAppsCache[$appId] = 'yes';
$this->appConfig->setValue($appId, 'enabled', 'yes');
$this->getAppConfig()->setValue($appId, 'enabled', 'yes');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::setValue has been marked as deprecated
lib/private/App/AppManager.php

$this->installedAppsCache[$appId] = json_encode($groupIds);
$this->appConfig->setValue($appId, 'enabled', json_encode($groupIds));
$this->getAppConfig()->setValue($appId, 'enabled', json_encode($groupIds));

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::setValue has been marked as deprecated
lib/private/App/AppManager.php

if ($automaticDisabled) {
$previousSetting = $this->appConfig->getValue($appId, 'enabled', 'yes');
$previousSetting = $this->getAppConfig()->getValue($appId, 'enabled', 'yes');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::getValue has been marked as deprecated
lib/private/App/AppManager.php

unset($this->installedAppsCache[$appId]);
$this->appConfig->setValue($appId, 'enabled', 'no');
$this->getAppConfig()->setValue($appId, 'enabled', 'no');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::setValue has been marked as deprecated
lib/private/App/AppManager.php
foreach ($apps as $appId) {
$appInfo = $this->getAppInfo($appId);
$appDbVersion = $this->appConfig->getValue($appId, 'installed_version');
$appDbVersion = $this->getAppConfig()->getValue($appId, 'installed_version');

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\AppConfig::getValue has been marked as deprecated
nickvergessen
nickvergessen reviewed Apr 22, 2024
View reviewed changes
@come-nc
Copy link
Contributor Author

come-nc commented Apr 22, 2024

@nickvergessen Reverted both calls. We’ll need later to add a replacement for getAllApps.

But the maintenance:install command segfaults since today rebase. I can reproduce locally but have no idea what happens.
I tried reverting the IURLGenerator injection in the appmanager but it does not change anything.

@nickvergessen
Copy link
Member

nickvergessen commented Apr 22, 2024 •
edited
Loading

but have no idea what happens.

Circular dependencies in 99% of the cases

@come-nc
Copy link
Contributor Author

come-nc commented Apr 22, 2024

but have no idea what happens.

Circular dependencies in 99% of the cases

Yes, it was urlgenerator injection causing trouble as suspected. At first I thought it was not because I had missed one place it was pulled from.
Fixed now, should be good.

@come-nc come-nc merged commit 37c89f4 into master Apr 22, 2024
@come-nc come-nc deleted the fix/remove-oc-app-calls branch April 22, 2024 14:41
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
@come-nc come-nc mentioned this pull request Sep 12, 2024
Merged
4 tasks
@joshtrichards
Copy link
Member

joshtrichards commented Sep 26, 2024

Related to open issue: #8505 :)

@blizzz blizzz mentioned this pull request Sep 11, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@icewind1991 icewind1991 icewind1991 approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend

@nfebe nfebe Awaiting requested review from nfebe nfebe is a code owner automatically assigned from nextcloud/server-backend

Assignees

@come-nc come-nc

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

Nextcloud 30

Development

Successfully merging this pull request may close these issues.

7 participants

@come-nc @nickvergessen @joshtrichards @icewind1991 @ChristophWurst @skjnldsv