[stable9] Fix post_unshareFromSelf hook parameter format #26389

Merged
merged 2 commits into from Oct 19, 2016

Projects

None yet

3 participants

@PVince81
Collaborator

Forward port of #26364 to stable9

@PVince81 PVince81 Fix post_unshareFromSelf hook parameter format
When unsharing from self in a group share situation, the share items
passed to the post_unshareFromSelf hook were using the wrong format in
which the attribute names (ex: "share_type") have non camel-case format.

This fix makes sure that in group sharing case we use the correct
format. It looks like the code was already producing it but in
array_merge it was not using it and adding the unprocessed one.
fcda88c
@PVince81 PVince81 added this to the 9.0.6 milestone Oct 17, 2016
@mention-bot

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

@jvillafanez
Contributor

👍

@PVince81
Collaborator

oh noes, failing tests

@PVince81
Collaborator

can't reproduce the problem with a manual test... hmmm

@PVince81
Collaborator

Weiiird... I debugged into the code.

The code that combines etags based on all mount points is inside FileInfo->addSubEntry() (and later FileInfo->getEtag). But the combination is only done if the data passed to addSubEntry() contains an "etag" attribute. For some reason during the unit tests the "etag" attribute is missing from the passed file info... this makes the etag combination fail and the test fails.

@PVince81
Collaborator

It looks like the ownerView belongs to user2, for the shared storage connected to "files/test" in user3's point. This is wrong, the owner is user1. So because in the test user2 unshares from self, so when checking if the folder is still visible it does it with user2's view where it's obviously gone instead of user1...

Need to find out why... possibly another bug. Also strange that it doesn't happen in stable9.1...

@PVince81
Collaborator

So... I tried reproducing this in the web UI but the correct owner is added to the database.
However when running these unit tests, the owner in the DB is user2 instead of user1, which is wrong.

So I suspect that the web UI is using the new APIs to create the share, which create it correctly, while the test still uses the old static APIs, which might not do it correctly. But since it works in stable9.1, then it means there is a fix that might need backporting.

@PVince81
Collaborator

Yep, found it... the GroupEtagPropagation test in stable9.1 is using the new API and stable9 is using the old one... So the old API is broken, probably related to #24981

@PVince81
Collaborator

For the sake of finishing this PR, I'll try and backport the part that makes use of the new sharing API

@PVince81
Collaborator

@jvillafanez I had to partially backport the etag propagation test cases from stable9.1 which use the new sharing API for creating shares while testing, see a3bcee8

We'll eventually need to look into the actual issue with the old sharing API here: #24981 (or ideally get rid of it completely...)

@PVince81 PVince81 Partial backport of etag propagation tests
Took the etag propagation test cases from
92abb5f to make sure it uses the new
API when creating shares.
1144cf0
@PVince81
Collaborator

@jvillafanez mind approving the extra backport ?

@PVince81 PVince81 merged commit a9e8b7d into stable9 Oct 19, 2016

3 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@PVince81 PVince81 deleted the stable9-fix-unsharefromself-hookvalues branch Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment