Handle mountpoint collisions for different providers by VicDeo · Pull Request #35319 · owncloud/core

VicDeo · 2019-05-23T13:21:20Z

Description

Check that all user mount points has unique names

Related Issue

Fixes receiving a local share is not possible when a federation share with same name was received before #35154

Motivation and Context

invisible local share when federated share with the same name exists

How Has This Been Tested?

install two servers with owncloud and configure federation sharing for me http://localhost/owncloud-10.2.0/rc2_a & http://localhost/owncloud-10.2.0/rc2_b
on server A create a user user1_on_server_a
on server B create two users user1_on_server_b user2_on_server_b
as user1_on_server_a create a folder called to-share
as user1_on_server_a share the folder to-share with user1_on_server_b via federation sharing
as user1_on_server_b accept the share
as user2_on_server_b create a folder called to-share
as user2_on_server_b share the folder to-share locally with user1_on_server_b

Expected behaviour

as user1_on_server_b there should be a received folder called to-share (fed sharing) and a folder called to share (2) local sharing

Actual behaviour

as user1_on_server_b there is only one received folder called to-share (fed sharing) and NO folder called to share (2) local sharing

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Backport (if applicable set "backport-request" label and remove when the backport was done)

codecov · 2019-05-23T13:27:51Z

Codecov Report

Merging #35319 into master will decrease coverage by 16.72%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master   #35319       +/-   ##
=============================================
- Coverage     65.81%   49.09%   -16.73%     
=============================================
  Files          1228      109     -1119     
  Lines         70982    10535    -60447     
  Branches       1289     1289               
=============================================
- Hits          46716     5172    -41544     
+ Misses        23888     4985    -18903     
  Partials        378      378

Flag	Coverage Δ	Complexity Δ
#javascript	`53.7% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`38.78% <ø> (-28.42%)`	`0 <ø> (-18818)`

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Storage/DAV.php	`59.45% <0%> (-21.64%)`	`0% <0%> (ø)`
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1108 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f52c8bd...b8053a7. Read the comment docs.

VicDeo · 2019-05-24T09:23:19Z

Well, there are existing tests that assume there could be shares with the same mount points.
😮

micbar · 2019-06-14T20:52:28Z

@VicDeo What are the next steps on this?
Do you need a review?

VicDeo · 2019-06-14T20:55:29Z

@micbar I'm stuck here. No idea how to fix the tests

micbar · 2019-06-17T06:28:50Z

@phil-davis @individual-it Could you help with the tests?

phil-davis · 2019-06-17T07:24:06Z

@micbar the acceptance tests are passing. Various unit tests are failing e.g. https://drone.owncloud.com/owncloud/core/18094/127

here were 83 errors:

1) OCA\Files_Sharing\Tests\UpdaterTest::testShareFile with data set #0 ('/')
Error: Call to a member function getShare() on null

We can have a look, however it will be some learning to understand what is going on in the unit tests here (which might be "a good thing")

micbar · 2019-07-01T10:14:57Z

@patrickjahns Could you help here with testing issues?

VicDeo · 2019-07-02T16:00:39Z

This solution causes 53 existing test cases to fail because they are not true unit tests. These tests have external dependencies :/
I'm not sure how to proceed here...

VicDeo · 2019-07-02T16:12:59Z

it;s even more interesting.
MountProviderCollection::getMountsForUser has two identical sets of shares from different providers. Let me check...

VicDeo · 2019-07-02T16:20:03Z

can of worms opened :)
3 and 6
4 and 7
are the same mount providers 😷

VicDeo · 2019-07-02T16:30:08Z

I've added a static property to register files_sharing mount providers only once.
But we might want to block duplication on MountProviderCollection level.
This will also speed up FS initialization a bit.

individual-it · 2019-07-18T05:36:41Z

with this change I have two folders as receiver, but the remote share (that was received firsts) is renamed to xx (2) the local share (that was received last) keeps its original name

tests to demonstrate the issue were added in #35870
after that test PR is merged, please rebase this PR. After fixing the issue the current tests will fail and will have to be adjusted according to the comments in the feature file

individual-it · 2019-07-19T08:09:44Z

took off QA-team label because tests were implemented

DeepDiver1975 · 2019-07-30T07:24:07Z

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

VicDeo added the 2 - Developing label May 23, 2019

VicDeo added this to the development milestone May 23, 2019

VicDeo self-assigned this May 23, 2019

VicDeo force-pushed the fix-35154 branch from d036d3f to 57a3a7f Compare May 23, 2019 13:27

VicDeo force-pushed the fix-35154 branch from 57a3a7f to c65d8ed Compare May 30, 2019 13:31

owncloud deleted a comment from codecov bot Jun 3, 2019

VicDeo force-pushed the fix-35154 branch 2 times, most recently from b9bb686 to 60d348b Compare June 6, 2019 14:19

paurakhsharma force-pushed the fix-35154 branch from 32e3b3b to b68fd7a Compare June 25, 2019 09:57

skshetry added the QA:team label Jun 26, 2019

VicDeo force-pushed the fix-35154 branch from b68fd7a to 30d3677 Compare July 2, 2019 08:47

VicDeo force-pushed the fix-35154 branch from 7fc5733 to 72fae36 Compare July 2, 2019 16:55

VicDeo added 3 commits July 4, 2019 15:28

Deduplicate mountpoints

ae8fafd

Remove unused static helper function

23a8d9c

Remove some integration tests

4ccc749

VicDeo force-pushed the fix-35154 branch 2 times, most recently from c82776f to 5d030d9 Compare July 4, 2019 12:50

Prevent registering files_sharing mount providers twice

b8053a7

VicDeo force-pushed the fix-35154 branch from 5d030d9 to b8053a7 Compare July 4, 2019 13:16

micbar requested a review from patrickjahns July 9, 2019 18:57

individual-it mentioned this pull request Jul 18, 2019

tests demonstrating issue #35154 #35870

Merged

11 tasks

individual-it removed the QA:team label Jul 19, 2019

DeepDiver1975 closed this Jul 30, 2019

VicDeo mentioned this pull request Aug 2, 2019

receiving a local share is not possible when a federation share with same name was received before #35154

Closed

jvillafanez mentioned this pull request Aug 14, 2019

Mountpoint collision #36029

Merged

11 tasks

Conversation

VicDeo commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Expected behaviour

Actual behaviour

Types of changes

Checklist:

Open tasks:

Uh oh!

codecov bot commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

VicDeo commented May 24, 2019

Uh oh!

micbar commented Jun 14, 2019

Uh oh!

VicDeo commented Jun 14, 2019

Uh oh!

micbar commented Jun 17, 2019

Uh oh!

phil-davis commented Jun 17, 2019

Uh oh!

micbar commented Jul 1, 2019

Uh oh!

VicDeo commented Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VicDeo commented Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VicDeo commented Jul 2, 2019

Uh oh!

VicDeo commented Jul 2, 2019

Uh oh!

individual-it commented Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

individual-it commented Jul 19, 2019

Uh oh!

DeepDiver1975 commented Jul 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

VicDeo commented May 23, 2019 •

edited

Loading

codecov bot commented May 23, 2019 •

edited

Loading

VicDeo commented Jul 2, 2019 •

edited

Loading

VicDeo commented Jul 2, 2019 •

edited

Loading

individual-it commented Jul 18, 2019 •

edited

Loading