Skip to content

Modernize the dav app #30335

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
Closed
tcitworld wants to merge 23 commits into from
Closed

Modernize the dav app #30335

tcitworld wants to merge 23 commits into from

Conversation

@tcitworld
Copy link
Member

@tcitworld tcitworld commented Dec 19, 2021 •
edited
Loading

(This was built on top of #20766)

  • Update most of depreciated calls
  • Move every event to proper listeners
  • Move uses of ILogger to LoggerInterface (except background jobs)
  • Fix tests
  • Factorize some EventListeners
  • Extract changes to EventListeners in their own PR
  • Add myself to authors in every file

@tcitworld tcitworld added 2. developing Work in progress feature: dav feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals labels Dec 19, 2021
@tcitworld tcitworld added this to the Nextcloud 24 milestone Dec 19, 2021
This was referenced Dec 19, 2021
Merged
Closed
@tcitworld tcitworld force-pushed the modernize-dav branch 3 times, most recently from dd872a8 to 1a5c0d8 Compare January 20, 2022 13:50
tcitworld
tcitworld commented Jan 29, 2022
View reviewed changes
apps/dav/appinfo/v1/caldav.php
\OC::$server->getRequest(),
\OC::$server->getTwoFactorAuthManager(),
\OC::$server->getBruteForceThrottler(),
$userSession->getSession(),
Copy link
Member Author

@tcitworld tcitworld Jan 29, 2022

Choose a reason for hiding this comment

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

What's the recommended way to get the actual ISession object there? getSession exists on OC\User\Session but not IUserSession.

Copy link
Member

@ChristophWurst ChristophWurst Jan 31, 2022

Choose a reason for hiding this comment

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

does \OC::$server->get(IUserSession::class) work?

Copy link
Member Author

@tcitworld tcitworld Jan 31, 2022

Choose a reason for hiding this comment

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

This needs an ISession, not an IUserSession.

Copy link
Member

@ArtificialOwl ArtificialOwl Feb 15, 2022

Choose a reason for hiding this comment

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

\OC::$server->get(ISession::class); should work

Copy link
Member Author

@tcitworld tcitworld Feb 15, 2022

Choose a reason for hiding this comment

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

Ah yes, it's registered indeed

server/lib/private/Server.php

Lines 1353 to 1355 in 9026455

$this->registerService(\OCP\ISession::class, function (ContainerInterface $c) {
return $c->get(\OCP\IUserSession::class)->getSession();
}, false);

tcitworld
tcitworld commented Jan 31, 2022
View reviewed changes
apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
* @throws ContainerExceptionInterface
* @throws NotFoundExceptionInterface
*/
public function testGetQuotaInfoSpecific() {
Copy link
Member Author

@tcitworld tcitworld Jan 31, 2022

Choose a reason for hiding this comment

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

This is the last test failing that I've no idea how to fix, since the call to getQuotaInfo goes deeply into legacy stuff.

@tcitworld tcitworld mentioned this pull request Feb 14, 2022
Merged
@tcitworld tcitworld force-pushed the modernize-dav branch 2 times, most recently from 896b71e to 6275668 Compare February 15, 2022 09:08
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
Merged
@tcitworld tcitworld modified the milestones: Nextcloud 24, Nextcloud 25 Mar 24, 2022
@tcitworld
Move createSubscription and deleteSubscription event handlers to
c8c8f93 
proper listeners

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Remove uses of LegacyDispatcher for create/delete subscriptions
bfdfd3e 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Move every event to listeners and remove all uses of legacy dispatcher
dfa2cc3 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Add tests for new Listeners
10af3ab 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Remove usages of legacyDispatcher in CardDAVBackend
180e695 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Update some depreciated calls
ab07629 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Fix doctype of param passed to deleteContact
08abaae 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Improve OCA\DAV\Connector\Sabre\Principal
4827bc7 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Improve OCA\DAV\Tests\unit\CardDAV\AddressBookImplTest
aa7994f 
Signed-off-by: Thomas Citharel <[email protected]>
tcitworld added 14 commits March 31, 2022 10:28
@tcitworld
Move uses of ILogger to LoggerInterface
3a3a1a0 
Except for BackgroundJobs

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Move some events to IEventDispatcher
fed15f9 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Replace calls to Sabre's depreciated getPropertiesForPath with getPro…
c304561 
…pertiesIteratorForPath

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Remove more depreciated calls
981b99c 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Remove HookManager and replace with UserChangeListener
dcc9c70 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Modernize endpoints
7a5f4af 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Modernize Sabre Connector
d739f92 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Add @template-implements metadata for listeners
244aaa7 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Remove CalendarManager registration
57ddf28 
registerCalendarProvider is now used

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Various fixes
fccc89d 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Improve tests
041526d 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Improve CommentsEntityEvent
3d42336 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Use PHP 7.4 typed properties
9246d06 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Rebasing, cs and more typing
6d6008d 
Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld tcitworld closed this May 12, 2022
@ChristophWurst ChristophWurst deleted the modernize-dav branch December 15, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ArtificialOwl ArtificialOwl ArtificialOwl left review comments

@ChristophWurst ChristophWurst ChristophWurst left review comments

@SebastianKrupinski SebastianKrupinski Awaiting requested review from SebastianKrupinski SebastianKrupinski will be requested when the pull request is marked ready for review SebastianKrupinski is a code owner

@nickvergessen nickvergessen Awaiting requested review from nickvergessen nickvergessen will be requested when the pull request is marked ready for review nickvergessen is a code owner

@hamza221 hamza221 Awaiting requested review from hamza221 hamza221 will be requested when the pull request is marked ready for review hamza221 is a code owner

Assignees

No one assigned

Labels

2. developing Work in progress feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals feature: dav technical debt

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

4 participants

@tcitworld @ArtificialOwl @ChristophWurst