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

Add an API for calendar providers #28970

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

miaulalala
Copy link
Contributor

@miaulalala miaulalala commented Sep 27, 2021

Right now user calendars are preemptively loaded in every request. This comes with two downsides

  • We create calendar objects that are possibly never used
  • Any queries against the calendars always go against the current user's calendars. You can't look into someone elses calendar. You can't look into a calendar from a background or CLI job

This flips the logic so that calendar back-end apps (mainly the dav app but possibly other "virtual" calendars like in deck) register that they provide calendars. Then, when calendars are needed, we ask all the registered providers for the calendars of the given principal, the providers give back a list and that list can be used for the queries.

This means we

  • Don't load anything unless we need it
  • It works in any user context, background jobs and CLI

Todo

  • Deprecate the old preemptive APIs that register calendars or calendar closures
  • Add a new, lazy and user context independent mechanism

Follow-up tasks

Required for nextcloud/calendar#3477

@ChristophWurst ChristophWurst changed the title Add Public Calendar Provider Add an API for calendar providers Sep 27, 2021
lib/private/AppFramework/Bootstrap/RegistrationContext.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Bootstrap/IRegistrationContext.php Outdated Show resolved Hide resolved
lib/public/Calendar/ICalendarProvider.php Outdated Show resolved Hide resolved
lib/public/Calendar/ICalendarProvider.php Outdated Show resolved Hide resolved
lib/private/Calendar/Manager.php Outdated Show resolved Hide resolved
@ChristophWurst ChristophWurst marked this pull request as draft September 27, 2021 13:14
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Sep 27, 2021
lib/private/Calendar/CalendarQuery.php Outdated Show resolved Hide resolved
lib/private/Calendar/CalendarQuery.php Outdated Show resolved Hide resolved
}

public function setTimerangeStart(\DateTime $startTime): void {
$this->options['timerange']['start'] = $startTime;
Copy link
Member

Choose a reason for hiding this comment

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

you want to initialize options with an empty array otherwise you access an offset of null here

Copy link
Member

Choose a reason for hiding this comment

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

timerange is not initialized in the constructor

lib/public/Calendar/ICalendarQuery.php Outdated Show resolved Hide resolved
lib/public/Calendar/ICalendarQuery.php Outdated Show resolved Hide resolved
@ChristophWurst
Copy link
Member

Is this ready for a final review? :)

@miaulalala miaulalala added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 13, 2021
@miaulalala miaulalala force-pushed the enhancement/calendar-appointments branch from d0269da to efc762e Compare October 13, 2021 15:26
@miaulalala miaulalala requested review from ChristophWurst, a team, ArtificialOwl, skjnldsv, come-nc, kesselb and nickvergessen and removed request for a team October 13, 2021 15:26
@ChristophWurst ChristophWurst marked this pull request as ready for review October 13, 2021 15:27
@ChristophWurst
Copy link
Member

@miaulalala please add the deprecations to #27846

@blizzz blizzz removed the 3. to review Waiting for reviews label Oct 13, 2021
@kesselb kesselb added 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish labels Oct 13, 2021
@ChristophWurst

This comment has been minimized.

@ChristophWurst ChristophWurst force-pushed the enhancement/calendar-appointments branch from efc762e to dfe4c6c Compare October 13, 2021 17:42
@ChristophWurst ChristophWurst force-pushed the enhancement/calendar-appointments branch from dfe4c6c to 116140b Compare October 13, 2021 17:48
@miaulalala miaulalala requested a review from kesselb October 13, 2021 17:53
@miaulalala miaulalala force-pushed the enhancement/calendar-appointments branch from 116140b to d5025ad Compare October 13, 2021 17:57
@kesselb kesselb added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 13, 2021
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Look clean

@miaulalala miaulalala force-pushed the enhancement/calendar-appointments branch from d5025ad to 41d5841 Compare October 13, 2021 18:58
@miaulalala miaulalala force-pushed the enhancement/calendar-appointments branch from 41d5841 to 767e518 Compare October 13, 2021 20:42
Signed-off-by: Anna Larch <[email protected]>
Signed-off-by: Christoph Wurst <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants