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.

Sign up for GitHub

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

Jump to bottom

Update Sabre (and friends) to 4.0 #19325

Merged
merged 6 commits into from
Mar 30, 2020
Merged

Update Sabre (and friends) to 4.0 #19325

merged 6 commits into from
Mar 30, 2020

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Feb 6, 2020 •
edited
Loading

I'm doing this to to help @skjnldsv. If tests fail I'm out 🙊

@ChristophWurst ChristophWurst added this to the Nextcloud 19 milestone Feb 6, 2020
@ChristophWurst ChristophWurst self-assigned this Feb 6, 2020
@rullzer rullzer force-pushed the dependachristoph/composer/sabre-4.0 branch from d913ef9 to c79b7b5 Compare February 11, 2020 20:35
@ChristophWurst ChristophWurst force-pushed the dependachristoph/composer/sabre-4.0 branch from c79b7b5 to 6896985 Compare March 6, 2020 14:56
@ChristophWurst
Copy link
Member Author

Changelog at https://github.com/sabre-io/dav/releases/tag/4.0.0 does not sound like there are any breaking changes for us.

@ChristophWurst
Copy link
Member Author

Files load but folder icons are missing and permissions are incorrect. A DAV expert needs to look into this

@skjnldsv
Copy link
Member

skjnldsv commented Mar 6, 2020

Files load but folder icons are missing and permissions are incorrect. A DAV expert needs to look into this

Pinging Dr @georgehrke :D

@georgehrke
Copy link
Member

Looking at the Changelog of version 4, this point will probably cause us most work:

Event listeners that in the past listened to beforeMethod or method
no longer get called. They must listen to beforeMethod:* and method:* now.

@georgehrke georgehrke force-pushed the dependachristoph/composer/sabre-4.0 branch 2 times, most recently from 33a16b8 to 747e2be Compare March 9, 2020 15:38
@kesselb
Copy link
Contributor

kesselb commented Mar 9, 2020

https://github.com/nextcloud/user_saml/blob/6734601db801fc4ee232ed643d94a92f37dc1398/lib/DavPlugin.php#L51

https://github.com/nextcloud/end_to_end_encryption/blob/9e71e2d282ccd106cbe3f5f62ad60abd5d05d072/lib/Connector/Sabre/LockPlugin.php#L100

Please don't forget ;)

This was referenced Mar 9, 2020
Merged
Merged
@georgehrke
Copy link
Member

@georgehrke
Copy link
Member

georgehrke commented Mar 9, 2020 •
edited
Loading

As far as i can tell, the beforeMethod thing was pretty much the only change necessary.
Let's wait to see if Litmus is happy and do more in-depth testing.

@ChristophWurst
Copy link
Member Author

We can also check how similar projects did the upgrade :)

@georgehrke
Copy link
Member

We already fixed all the usages of URIUtil::split before: fc369dd :)

@georgehrke georgehrke force-pushed the dependachristoph/composer/sabre-4.0 branch from 747e2be to 5f8c63a Compare March 10, 2020 08:51
@ChristophWurst
Copy link
Member Author

👍 (can't approve my own PR 😀 )

@georgehrke georgehrke force-pushed the dependachristoph/composer/sabre-4.0 branch from 5f8c63a to 9a7d944 Compare March 10, 2020 09:07
@georgehrke georgehrke added the 2. developing Work in progress label Mar 10, 2020
@ChristophWurst ChristophWurst mentioned this pull request Mar 11, 2020
Closed
@georgehrke
Copy link
Member

please test @icewind1991 @rullzer @juliushaertl

@rullzer rullzer force-pushed the dependachristoph/composer/sabre-4.0 branch from 36f936b to b6f9217 Compare March 23, 2020 20:43
@rullzer
Copy link
Member

rullzer commented Mar 23, 2020

Looks like I don't have permissions to upload files

@rullzer
Copy link
Member

rullzer commented Mar 23, 2020

Looks like I don't have permissions to upload files

Or at least that is what the js thinks ;)

@georgehrke georgehrke force-pushed the dependachristoph/composer/sabre-4.0 branch 2 times, most recently from 36f936b to 0ff1de6 Compare March 24, 2020 09:28
@icewind1991
Copy link
Member

Apps that will require updating their beforeMethod hooks:

  • guests
  • user_saml

@icewind1991
Copy link
Member

Looks like I don't have permissions to upload files

Or at least that is what the js thinks ;)

works for me

@ChristophWurst
Copy link
Member Author

user_saml

That nextcloud/user_saml#407?

ChristophWurst and others added 6 commits March 27, 2020 12:52
@ChristophWurst @rullzer
Update Sabre (and friends) to 4.0
8648ee1 
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@georgehrke @rullzer
Sabre/DAV 4.0: beforeMethod is now beforeMethod:*
52ba75f 
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke @rullzer
Fix usage of Sabre\HTTP\Request in unit tests
102a328 
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke @rullzer
Set custom ProdId in Birthday service, do not depend on Sabre Versio …
5e2c802 
…number for unit tests

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke @rullzer
Adjust FakeLockerPluginTest to new Sabre version
80b7ef9 
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke @rullzer
RequestTestCase: Default type for headers needs to be array
55081b7 
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@rullzer rullzer force-pushed the dependachristoph/composer/sabre-4.0 branch from 0ff1de6 to 55081b7 Compare March 27, 2020 11:53
rullzer
rullzer approved these changes Mar 27, 2020
View reviewed changes
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.

Does the trick for me. Lets do this.

@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 27, 2020
@georgehrke georgehrke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 30, 2020
@rullzer rullzer merged commit 2200cc8 into master Mar 30, 2020
@rullzer rullzer deleted the dependachristoph/composer/sabre-4.0 branch March 30, 2020 07:34
@kesselb kesselb mentioned this pull request May 7, 2020
Closed
@phil-davis phil-davis mentioned this pull request Jan 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgehrke georgehrke georgehrke approved these changes

@rullzer rullzer rullzer approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

@georgehrke georgehrke

Labels
4. to release Ready to be released and/or waiting for tests to finish feature: dependencies
Projects
None yet
Milestone
Nextcloud 19
Development

Successfully merging this pull request may close these issues.
6 participants
@ChristophWurst @skjnldsv @georgehrke @kesselb @rullzer @icewind1991