Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Federation 2.0 #9345

Merged
merged 39 commits into from Jul 3, 2018
Merged

Federation 2.0 #9345

merged 39 commits into from Jul 3, 2018

Conversation

schiessle
Copy link
Member

@schiessle schiessle commented Apr 30, 2018

  • Make Federated Sharing OCM compatible (with some small additions/modifications)

  • Allow the implementation of federated calendar and contact sharing

  • basic infrastructure

  • receive shares over new API

  • send shares over new API

  • receive notifications over new API

    • permissions change
    • ask for re-share
    • share accepted
    • share declined
    • owner unshare
    • undo reshare
  • send notifications over new API

    • permissions change
    • ask for re-share
    • share accepted
    • share declined
    • owner unshare
    • undo reshare

@codecov
Copy link

codecov bot commented May 8, 2018

Codecov Report

Merging #9345 into master will decrease coverage by 45.46%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master   #9345       +/-   ##
============================================
- Coverage      51.8%   6.33%   -45.47%     
- Complexity    26222   26393      +171     
============================================
  Files          1673    1690       +17     
  Lines         96928   97627      +699     
  Branches       1290    1290               
============================================
- Hits          50209    6184    -44025     
- Misses        46719   91443    +44724
Impacted Files Coverage Δ Complexity Δ
...dfilesharing/composer/composer/autoload_static.php 0% <ø> (ø) 1 <0> (ø) ⬇️
lib/private/Server.php 1.56% <0%> (-80.43%) 286 <6> (+6)
...s/federatedfilesharing/lib/AppInfo/Application.php 0% <0%> (-63.64%) 6 <1> (+1)
apps/files_sharing/lib/Hooks.php 0% <0%> (-47.37%) 4 <0> (ø)
apps/files_sharing/lib/AppInfo/Application.php 0% <0%> (-48.92%) 15 <0> (ø)
...on_api/lib/Controller/RequestHandlerController.php 0% <0%> (ø) 33 <33> (?)
lib/private/Federation/CloudFederationFactory.php 0% <0%> (ø) 2 <2> (?)
...lesharing/lib/ocm/CloudFederationProviderFiles.php 0% <0%> (ø) 74 <74> (?)
...s/cloud_federation_api/lib/AppInfo/Application.php 0% <0%> (ø) 1 <1> (?)
core/routes.php 0% <0%> (ø) 0 <0> (ø) ⬇️
... and 961 more

@schiessle schiessle force-pushed the federation20 branch 5 times, most recently from 20ccb81 to cbfd266 Compare May 14, 2018 10:45
@schiessle schiessle force-pushed the federation20 branch 2 times, most recently from 5e06045 to 0e76f93 Compare May 30, 2018 15:46
@schiessle schiessle force-pushed the federation20 branch 9 times, most recently from a0c0cc9 to a4e71fd Compare June 12, 2018 14:59
@schiessle schiessle added this to the Nextcloud 14 milestone Jun 12, 2018
@schiessle schiessle added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 12, 2018
parent::__construct('cloud_federation_api');

$container = $this->getContainer();
$container->registerCapability(Capabilities::class);
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer if this was moved to a separate register method instead of having the constructor have side effects

Copy link
Member Author

Choose a reason for hiding this comment

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

most apps I checked, e.g. notifications and activity register the capabilities in the constructor. What side-effects are you referring too? I don't see a real different if I call it manually each time I instantiate the application or if it is done automatically in the constructor.

public function addShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, $protocol, $shareType, $resourceType) {

// check if all required parameters are set
if ($shareWith === null ||
Copy link
Member

Choose a reason for hiding this comment

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

use strict type hints instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

not really an option here, because I want to be able to control the exception in case of a missing parameter.

throw new BadRequestException(['permission']);
}

error_log("new permissions: " . $ncPermissions);
Copy link
Member

Choose a reason for hiding this comment

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

left over debug

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
…ares are enabled for the specific resource type

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle schiessle force-pushed the federation20 branch 2 times, most recently from 18606a3 to da256da Compare July 2, 2018 09:54
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle
Copy link
Member Author

I think the failing tests can be ignored, they fail randomly and I don't see how they are connected to this PR. Looking for your final 👍 😉

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Huge and scarry. But seems to work. Lets do this.

@daraelmin
Copy link

Not sur, but I think this pull request should sollve issue #1440 (Allow the implementation of federated calendar and contact sharing) and... there's a bounty on it. Maybe someone should take it.

https://www.bountysource.com/issues/37963414-federated-calendar-and-contacts-sharing-210-00

Many many many thanks to the dev for your work !
Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants