Share API 2

[MTGap]

This isn't ready to be merged yet, because this is only the backend changes. At this time I would just like a code review of the architecture. You can read this page to get a general overview: https://github.com/owncloud/core/wiki/Share-API

Besides all the new code, the largest change is storing multiple parent references for reshares because it is possible to share to the same user multiple times through group and user shares. By not referencing all parents of a reshare there were unexpected permission and unsharing issues.

This took so long, because I wrote unit tests for 'almost' all of the code. It is currently at 95.62% code coverage and the nice thing is that the tests are fast, all ~500 assertions. One test is failing because of the groups backend and is unrelated to the Share API.

Todo:

• Update frontend to use new API and write acceptance tests for it
• Update public links
• Integrate with files
• Integrate with contacts
• Integrate with calendar
• Integrate with encryption app
• User hook listener
• Group hook listener
• Updater

I really want to kill all sharing bugs once and for all...

[MTGap]

@karlitschek @icewind1991 @bartv2 @Raydiation @DeepDiver1975

[icewind1991]

get_class() returns the namespace + classname without a leading \ (OC\Share\Shares) for consistency sake I think it's best to always have a leading \ for emitter scopes

[icewind1991]

Maybe using a more general scope like \OC\Share is better since the classname isn't consistence with the multiple sub classes that can exists

[MTGap]

My intention is to have it just be the classname, is there a function for just that?

[MTGap]

I want to have it based on the individual sub classes so you didn't have to listen to all sharing events and filter afterwards.

[icewind1991]

There isn't a need to use the scope to distinguish the source that emitted the events, if you want to only listen to events from only one backend you can simple only attach the listeners to that one backend.

(also see my comments about making \OC\Share\Manager an emitter)

[MTGap]

You're right, I'll just switch it to a general scope.

[DeepDiver1975]

@MTGap What about the pubic api? Do you provide a compatibility layer?

[MTGap]

@MTGap What about the pubic api? Do you provide a compatibility layer?

No, I see no need to do that, because I was the main person to integrate it with apps originally anyways. It will only cause more pain.

[DeepDiver1975]

No, I see no need to do that,

Well - we have no clue on which and how many apps exist out there relying on the public api.
Breaking the API that way is very unkind and can cause many mad dev to scream at us.

[bartv2]

if (method_exists($instance, $method)) {

[MTGap]

I prefer that the exception is thrown, because it shows that something is wrong.

[ghost]

Test failed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/477/

[ghost]

Test failed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/635/

[ghost]

Test failed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/667/

[ghost]

Test failed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/731/

[jancborchardt]

Closed? @MTGap what’s up with this, any follow-up pull request?

[Niduroki]

@MTGap: Seconding @jancborchardt's question.

[karlitschek]

@Kondou-ger This branch is dead. We will start over for ownCloud 7 and improve sharing. But this time not with a complete rewrite that is 10x as big but with an iterative approach.