Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented May 12, 2022

Both deprecated since NC 14

IAppManager is the replacement for OCP\App unfortunately it can't be
dependency injected in classes used by the installer otherwise the
the database connection is initialized too early

Test plan

  1. Tests work locally
  2. psalm and cs:fix are happy
  3. The webui seems to still work :)

@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label May 12, 2022
@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone May 12, 2022
@CarlSchwan CarlSchwan requested a review from a team May 12, 2022 15:37
@CarlSchwan CarlSchwan self-assigned this May 12, 2022
@CarlSchwan CarlSchwan requested review from blizzz, come-nc and juliusknorr and removed request for a team May 12, 2022 15:37
@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from 34dfef6 to 8c50c42 Compare May 12, 2022 16:26
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

BackgroundJob was unused?
Any chance of removing OC_App?

@CarlSchwan
Copy link
Member Author

BackgroundJob was unused?

OCP\BackgroundJob was unused and did nothing. This is not the same thing as OCP\BackgroundJob\IJob and co

Any chance of removing OC_App?

I wish but let's do that step by step ;) there is a few api still missing in OCP\App\IAppManager to be a full replacement

@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from 8c50c42 to f5b7881 Compare May 20, 2022 15:18
@CarlSchwan CarlSchwan requested a review from kesselb May 20, 2022 16:39
@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from f5b7881 to 3d53626 Compare May 20, 2022 16:41
@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from 3d53626 to a36ff9e Compare May 20, 2022 20:07
@CarlSchwan CarlSchwan requested a review from kesselb May 20, 2022 20:08
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path('', 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
<link rel="icon" href="<?php print_unescaped(image_path('core', 'favicon.ico')); /* IE11+ supports png */ ?>">
<link rel="apple-touch-icon" href="<?php print_unescaped(image_path('core', 'favicon-touch.png')); ?>">
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path('core', 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
Copy link
Contributor

Choose a reason for hiding this comment

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

{
  "Exception": "RuntimeException",
  "Message": "image not found: image:favicon-mask.svg webroot: serverroot:\/drone\/src",
  "Code": 0,
  "Trace": [
    {
      "file": "\/drone\/src\/lib\/private\/legacy\/template\/functions.php",
      "line": 242,
      "function": "imagePath",
      "class": "OC\\URLGenerator",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/core\/templates\/layout.user.php",
      "line": 37,
      "function": "image_path"
    },
    {
      "file": "\/drone\/src\/lib\/private\/Template\/Base.php",
      "line": 180,
      "args": [
        "\/drone\/src\/core\/templates\/layout.user.php"
      ],
      "function": "include"
    },
    {
      "file": "\/drone\/src\/lib\/private\/Template\/Base.php",
      "line": 150,
      "function": "load",
      "class": "OC\\Template\\Base",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/private\/legacy\/OC_Template.php",
      "line": 181,
      "function": "fetchPage",
      "class": "OC\\Template\\Base",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/private\/legacy\/OC_Template.php",
      "line": 212,
      "function": "fetchPage",
      "class": "OC_Template",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/public\/AppFramework\/Http\/TemplateResponse.php",
      "line": 213,
      "function": "fetchPage",
      "class": "OC_Template",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/private\/AppFramework\/Http\/Dispatcher.php",
      "line": 178,
      "function": "render",
      "class": "OCP\\AppFramework\\Http\\TemplateResponse",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/private\/AppFramework\/App.php",
      "line": 172,
      "function": "dispatch",
      "class": "OC\\AppFramework\\Http\\Dispatcher",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/lib\/private\/Route\/Router.php",
      "line": 298,
      "function": "main",
      "class": "OC\\AppFramework\\App",
      "type": "::"
    },
    {
      "file": "\/drone\/src\/lib\/base.php",
      "line": 1021,
      "function": "match",
      "class": "OC\\Route\\Router",
      "type": "->"
    },
    {
      "file": "\/drone\/src\/index.php",
      "line": 36,
      "function": "handleRequest",
      "class": "OC",
      "type": "::"
    }
  ],
  "File": "\/drone\/src\/lib\/private\/URLGenerator.php",
  "Line": 273,
  "CustomMessage": "--"
}

computer is still a bit unhappy. some of the integration tests are failing.

kesselb
kesselb previously approved these changes May 20, 2022
@kesselb kesselb self-requested a review May 20, 2022 22:49
@kesselb kesselb dismissed their stale review May 20, 2022 22:49

wrong button ;)

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 long overdue

I suggest you grep the release tarball in case you find more usages in apps

@PVince81
Copy link
Member

and there are conflicts

@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from a36ff9e to 79930f6 Compare July 14, 2022 15:47
<tr>
<td colspan="2" bgcolor="<?php p($theme->getColorPrimary());?>">
<img src="<?php p(\OC::$server->getURLGenerator()->getAbsoluteURL(image_path('', 'logo-mail.png'))); ?>" alt="<?php p($theme->getName()); ?>"/>
<img src="<?php p(\OC::$server->getURLGenerator()->getAbsoluteURL(image_path('core', 'logo-mail.png'))); ?>" alt="<?php p($theme->getName()); ?>"/>

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Server::getURLGenerator has been marked as deprecated
@github-actions
Copy link
Contributor

Possible performance regression detected

Show Output
1 queries added

≠ /remote.php/dav/files/test with 1 queries removed
  - UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
≠ /remote.php/dav/files/test/test.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)

@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish 2. developing Work in progress and removed 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish labels Jul 22, 2022
@blizzz
Copy link
Member

blizzz commented Jul 22, 2022

CI is unhappy

@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch 2 times, most recently from bab24e7 to 6658274 Compare August 1, 2022 07:40
Both deprecated since NC 23

IAppManager is the replacement for OCP\App unfortunately it can't be
dependency injected in classes used by the installed otherwise the
database connection is initialised too early

Signed-off-by: Carl Schwan <[email protected]>
@CarlSchwan CarlSchwan force-pushed the cleanup/remove-long-deprecated-classes branch from 6658274 to 458c2fa Compare August 1, 2022 07:46
@CarlSchwan CarlSchwan merged commit f8b13ec into master Aug 8, 2022
@CarlSchwan CarlSchwan deleted the cleanup/remove-long-deprecated-classes branch August 8, 2022 15:05
@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants