Kill some of the old sharing code related to share backends #26608

PVince81 · 2016-11-11T12:12:16Z

Description

Remove the "File" and "Folder" item type and share backends from files_sharing.
These should not be needed any more when using the OCS Share API.

Also removes any reference to file/folder in the old share API in \OC\Share (where possible), so that the old sharing stays only for other types like contacts/etc. (another approach would be to rewire these API to the new API if a file/folder item type is given ?)

Also remove most methods in \OC:Share class

Related Issue

General cleanup issue: #22209
Issue about those backends using oc_filecache: #26607

Motivation and Context

Removes the old code,

How Has This Been Tested?

Quick test in the web UI, sharing still seems to work, so does "unshare from self".
A quick run of integration tests locally showed that many calls still work (but my env is a bit broken so want to see that run on CI).
Will need manual testing of all share scenarios (and/or complete these as integration tests)
Retest fed sharing

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

TODOs:

port all file share related tests that use shareItem with file/folder to use the share manager instead

@DeepDiver1975 @jvillafanez

mention-bot · 2016-11-11T12:12:19Z

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @schiessle, @DeepDiver1975 and @nickvergessen to be potential reviewers.

PVince81 · 2016-11-11T16:11:13Z

Looks like View::isTargetAllowed is a big blocker for many tests.
Might want to move this to the SharedMount implementation and add that method on the MoveableMount interface.

But the bigger problem is that it seems that the share manager has no replacement for \OCP\Share::getItemShared

PVince81 · 2016-11-11T16:35:48Z

Solved a few test issues by porting the tests to the new API.
Unfortunately most are failing due to #26608 (comment). Need a good solution.

PVince81 · 2016-11-11T17:33:55Z

port publicpreview.php to share 2.0: Port publicpreview.php to the share 2.0 API #26611 (also included in this PR)

PVince81 · 2016-11-11T17:57:55Z

Remaining to port:

apps/files_sharing/lib/ExpireSharesJob.php:                     \OCP\Share::unshare($share['item_type'], $share['file_source'], \OCP\Share::SHARE_TYPE_LINK, null, $share['uid_owner']);
apps/files_sharing/lib/Helper.php:              $linkItem = \OCP\Share::getShareByToken($token, !$password);
apps/files_sharing/lib/Helper.php:              $rootLinkItem = \OCP\Share::resolveReShare($linkItem);
lib/private/Encryption/File.php:                        $resultForParents = \OCP\Share::getUsersSharingFile($parent, $owner);
lib/private/Encryption/File.php:                $resultForFile = \OCP\Share::getUsersSharingFile($ownerPath, $owner, false, false, false);
lib/private/Files/View.php:             $shares = \OCP\Share::getItemShared(
lib/private/Share/Share.php:            if ($checkPasswordProtection && !\OCP\Share::checkPasswordProtectedShare($row)) {

PVince81 · 2016-11-11T18:05:34Z

getting rid of Maintainer.php: Remove config change hook that deletes link shares #26612

PVince81 · 2016-11-11T19:28:40Z

This should help getting rid of getItemShared => Port isTargetAllowed to share 2.0 #26615

PVince81 · 2016-11-14T08:32:16Z

kill \OCP\Share altogether as @DeepDiver1975 told me that it's not used by calendar and contacts any more 🎉 => can't kill before ajax/share.php is gone due to gallery app

PVince81 · 2016-11-29T17:37:45Z

Rebased as the isTargetAllowed fix was merged. Now there should be much less test failures.

PVince81 · 2016-12-16T12:47:21Z

18:44:53 1) OCA\Files_Sharing\Tests\ApiTest::testDefaultExpireDate
18:44:53 Exception: Sharing backend for file not found
18:44:53 
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:603
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:267
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ApiTest.php:1399
18:44:53 
18:44:53 2) OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare with data set #2 (true, 'P1D', false, true)
18:44:53 Exception: Sharing backend for folder not found
18:44:53 
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:863
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:281
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/lib/ExpireSharesJob.php:71
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ExpireSharesJobTest.php:175
18:44:53 
18:44:53 3) OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare with data set #4 (true, 'P1W', false, true)
18:44:53 Exception: Sharing backend for folder not found
18:44:53 
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:863
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:281
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/lib/ExpireSharesJob.php:71
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ExpireSharesJobTest.php:175
18:44:53 
18:44:53 4) OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare with data set #6 (true, 'P1M', false, true)
18:44:53 Exception: Sharing backend for folder not found
18:44:53 
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:863
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:281
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/lib/ExpireSharesJob.php:71
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ExpireSharesJobTest.php:175
18:44:53 
18:44:53 5) OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare with data set #8 (true, 'P1Y', false, true)
18:44:53 Exception: Sharing backend for folder not found
18:44:53 
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:863
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:281
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/lib/ExpireSharesJob.php:71
18:44:53 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ExpireSharesJobTest.php:175

PVince81 · 2016-12-16T13:59:40Z

apps/files_sharing/tests/ApiTest.php will need more work as it seems to test older APIs

PVince81 · 2016-12-23T09:04:11Z

Note: we can't kill OCP\Share without killing ajax/share.php yet, so I'd do that in a separate PR.
And we can't kill ajax/share.php as the gallery app is still using some of these old endpoints.

PVince81 · 2016-12-23T09:10:53Z

Last failure:

10:06:59 1) OCA\Files_Sharing\Tests\ApiTest::testDefaultExpireDate
10:06:59 Exception: Sharing backend for file not found
10:06:59 
10:06:59 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:1474
10:06:59 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/private/Share/Share.php:603
10:06:59 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/lib/public/Share.php:267
10:06:59 /var/lib/jenkins/workspace/owncloud-core_core_PR-26608-7HTQSAV7ASDXHK3HEP454Z4KEUYQXFTEM4XUGRAL27I4X6LVM6NQ/apps/files_sharing/tests/ApiTest.php:1399

Some old tests can be deleted but need to check that the new code has these tests too for the new sharing API.

PVince81 · 2016-12-23T09:15:41Z

It is also likely that merging this PR would break gallery sharing, so need to verify that it doesn't.

PVince81 · 2018-10-18T06:45:35Z

Rebased since #32494 was merged. Expecting all tests to be green now.

PVince81 · 2018-10-18T12:37:33Z

seems I forgot to grep in "settings" when killing APIs:

{"reqId":"teYh83ZViCZbzxzQ9rlo","level":3,"time":"2018-10-18T12:36:58+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"index","method":"GET","url":"\/owncloud\/index.php\/settings\/admin?sectionid=sharing","message":"Exception: {\"Exception\":\"Error\",\"Message\":\"Call to undefined method OC\\\\Share\\\\Share::shareWithGroupMembersOnly()\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/settings\\\/Panels\\\/Admin\\\/FileSharing.php(96): OC\\\\Settings\\\\Panels\\\\Helper->shareWithGroupMembersOnly()\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/settings\\\/Controller\\\/SettingsPageController.php(167): OC\\\\Settings\\\\Panels\\\\Admin\\\\FileSharing->getPanel()\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/settings\\\/Controller\\\/SettingsPageController.php(104): OC\\\\Settings\\\\Controller\\\\SettingsPageController->getPanelsData(Array)\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/AppFramework\\\/Http\\\/Dispatcher.php(153): OC\\\\Settings\\\\Controller\\\\SettingsPageController->getAdmin('sharing')\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/AppFramework\\\/Http\\\/Dispatcher.php(85): OC\\\\AppFramework\\\\Http\\\\Dispatcher->executeController(Object(OC\\\\Settings\\\\Controller\\\\SettingsPageController), 'getAdmin')\\n#5 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/AppFramework\\\/App.php(100): OC\\\\AppFramework\\\\Http\\\\Dispatcher->dispatch(Object(OC\\\\Settings\\\\Controller\\\\SettingsPageController), 'getAdmin')\\n#6 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/AppFramework\\\/Routing\\\/RouteActionHandler.php(47): OC\\\\AppFramework\\\\App::main('SettingsPageCon...', 'getAdmin', Object(OC\\\\AppFramework\\\\DependencyInjection\\\\DIContainer), Array)\\n#7 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/Route\\\/Router.php(342): OC\\\\AppFramework\\\\Routing\\\\RouteActionHandler->__invoke(Array)\\n#8 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/base.php(907): OC\\\\Route\\\\Router->match('\\\/settings\\\/admin')\\n#9 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/index.php(54): OC::handleRequest()\\n#10 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/settings\\\/Panels\\\/Helper.php\",\"Line\":38}"}
{"reqId":"MaWKdI7n8MhzZCM75Ijw","level":0,"time":"2018-10-18T12:36:59+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"cron","method":"GET","url":"\/owncloud\/index.php\/cron","message":"Started background job of class : OCA\\Files\\BackgroundJob\\CleanupPersistentFileLocks with arguments : "}

Remove shareWithGroupMembersOnly helper and read from app config directly.

PVince81 · 2018-10-18T13:46:51Z

all green 🎉

I'd appreciate some review before merging @DeepDiver1975 @jvillafanez @VicDeo

see "Description" in the original post and also the list of spaghettis I managed to untangle: #26608 (comment)

there are still some spaghettis to untangle which must be adressed separately as they are more involved

PVince81 · 2018-10-18T14:45:41Z

raised #33219 for part 2 with remaining items

VicDeo · 2018-10-18T14:52:10Z

@PVince81 please note that \OCP\Share, \OCP\Share_Backend, \OCP\Share_Backend_Collection, \OCP\Share_Backend_File_Dependent should be marked as deprecated in stable10

PVince81 · 2018-10-18T15:04:55Z

@VicDeo thanks for the reminder, I'll submit a PR

PVince81 · 2018-10-18T15:09:46Z

deprecation PR for stable10: #33220

PVince81 added 2 - Developing feature:sharing labels Nov 11, 2016

PVince81 added this to the 9.2 milestone Nov 11, 2016

This was referenced Nov 11, 2016

Sharing 2.0 internals #22209

Open

Sharing does not work with custom Storages, expects oc_filecache #26607

Closed

PVince81 force-pushed the remove-file-share-backends branch from 8b2a6c4 to 354e756 Compare November 11, 2016 16:34

This was referenced Nov 11, 2016

Port sharing related tests to Share2.0 #26610

Merged

Port publicpreview.php to the share 2.0 API #26611

Merged

PVince81 mentioned this pull request Nov 11, 2016

Use ShareManager to delete orphaned and expired shares #26265

Closed

PVince81 force-pushed the remove-file-share-backends branch from 354e756 to cb9184f Compare November 11, 2016 17:58

PVince81 force-pushed the remove-file-share-backends branch from cb9184f to 0b07d32 Compare November 11, 2016 18:40

PVince81 mentioned this pull request Nov 14, 2016

[stable9] Add repair step to fix file share permissions #26618

Merged

PVince81 changed the title ~~Remove old file share backends~~ Kill old sharing code Nov 14, 2016

PVince81 force-pushed the remove-file-share-backends branch from 0b07d32 to bd2008a Compare November 21, 2016 10:09

PVince81 force-pushed the remove-file-share-backends branch from bd2008a to 2d2c5f5 Compare November 29, 2016 17:37

PVince81 force-pushed the remove-file-share-backends branch from 2d2c5f5 to 423cbe8 Compare December 23, 2016 07:09

Vincent Petry added 6 commits October 18, 2018 08:45

Remove old Share::getItemsShared()

c0b2e4f

Delete share backend classes and more Share 1.0 cleanup

058e1a3

More Share backend removal

e89431b

Fix ShareesController test

7722a40

Remove obsolete Share.php tests

057568b

Remove more obsolete legacy sharing code

152ef00

PVince81 force-pushed the remove-file-share-backends branch from 2a1ccb0 to 152ef00 Compare October 18, 2018 06:45

Share panel read setting from config directly

6f33db4

Remove shareWithGroupMembersOnly helper and read from app config directly.

PVince81 added 3 - To Review and removed 2 - Developing labels Oct 18, 2018

PVince81 requested a review from VicDeo October 18, 2018 12:49

PVince81 modified the milestones: maybe some day, development Oct 18, 2018

PVince81 self-assigned this Oct 18, 2018

DeepDiver1975 approved these changes Oct 18, 2018

View reviewed changes

DeepDiver1975 merged commit a698db1 into master Oct 18, 2018

DeepDiver1975 deleted the remove-file-share-backends branch October 18, 2018 14:19

PVince81 mentioned this pull request Oct 18, 2018

Remove old sharing code part 2 #33219

Open

14 tasks

PVince81 mentioned this pull request Oct 18, 2018

[stable10] Deprecate Sharing 1.0 APIs #33220

Merged

DeepDiver1975 mentioned this pull request Nov 5, 2018

no need to add sharing scripts if the server is not ye… #33414

Merged

10 tasks

phil-davis added the only-in-master label Mar 18, 2019

phil-davis mentioned this pull request Jul 8, 2019

[stable10] Introduce phpstan #35774

Merged

10 tasks

lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kill some of the old sharing code related to share backends #26608

Kill some of the old sharing code related to share backends #26608

PVince81 commented Nov 11, 2016 •

edited

Loading

mention-bot commented Nov 11, 2016

PVince81 commented Nov 11, 2016

PVince81 commented Nov 11, 2016

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 14, 2016 •

edited

Loading

PVince81 commented Nov 29, 2016

PVince81 commented Dec 16, 2016

PVince81 commented Dec 16, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

VicDeo commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

Kill some of the old sharing code related to share backends #26608

Kill some of the old sharing code related to share backends #26608

Conversation

PVince81 commented Nov 11, 2016 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

TODOs:

mention-bot commented Nov 11, 2016

PVince81 commented Nov 11, 2016

PVince81 commented Nov 11, 2016

PVince81 commented Nov 11, 2016 • edited Loading

PVince81 commented Nov 11, 2016 • edited Loading

PVince81 commented Nov 11, 2016 • edited Loading

PVince81 commented Nov 11, 2016 • edited Loading

PVince81 commented Nov 14, 2016 • edited Loading

PVince81 commented Nov 29, 2016

PVince81 commented Dec 16, 2016

PVince81 commented Dec 16, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Dec 23, 2016

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

VicDeo commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Oct 18, 2018

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 11, 2016 •

edited

Loading

PVince81 commented Nov 14, 2016 •

edited

Loading