feat: Calendar Import #51925

SebastianKrupinski · 2025-04-04T00:41:29Z

Summary

This adds the ability to import calendars

Split of #49995

OCC Import

Command: calendar:import
Arguments:
uid - mandatory
cid - mandatory
format - optional defaults to ical (ical, xcal, jcal)
filepath - optional defaults to stdin
--supersede - optional (force override of existing)
--show-created - optional (show uid of created objects)
--show-updated - optional (show uid of updated objects)
--show-skipped - optional (show uid of skipped objects)
--show-errors - optional (show uid of objects with errors)
--errors (int) - optional - how to handle errors (0 - continue, 1 - fail)
--validation (int) - optional - how to handle item validation (0 - no validation, 1 - validate and skip on issue, 2 - validate and fail on issue)

Import from file:

calendar:import user1 personal ical /tmp/personal.ics

Import from pipe:

cat /tmp/personal.ics | calendar:import user1 personal ical

Import from file and override existing:

calendar:import user1 personal ical /tmp/personal.ics --supersede

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

Dexus · 2025-04-30T16:47:05Z

A small note from me: You should not only be able to overwrite entries but also delete all existent events in the same calendar that are no longer part of the ICAL files (multievents files). Otherwise such an importer makes no sense, as entries are added but never deleted if they are no longer contained in the ICS/ICALs.

SebastianKrupinski · 2025-04-30T18:49:07Z

Hi @Dexus

You mean like adding an option, --purge or --reset to empty the calendar before importing?

We can add that after this is merged, I don't want to delay this feature any more that it has been already, I will just need the team to approve it.

@ChristophWurst do you have any thoughts?

Dexus · 2025-04-30T19:29:12Z

You mean like adding an option, --purge or --reset to empty the calendar before importing?

Absolutely! In my case, it is necessary to delete outdated events that are not contained in the calendar files.

ChristophWurst · 2025-05-01T05:43:04Z

The import/export feature was requested for Migration projects AFAIK, so the current capabilities are enough. Let's merge this.

The purge can be helpful for manual syncing. This can be done too 👍

apps/dav/lib/Command/ImportCalendar.php

apps/dav/appinfo/info.xml

SebastianKrupinski

Tried to rebase merge this (hopefully it does no make you lose your review position) so that you can test it with the export command in place.

Also went through and made some of the changes you recommended in the export PR to limit the review changes

apps/dav/appinfo/info.xml

apps/dav/lib/Command/ImportCalendar.php

kesselb · 2025-05-08T19:21:20Z

apps/dav/lib/Command/ImportCalendar.php

+			if (!in_array($errors, CalendarImportOptions::ERROR_OPTIONS, true)) {
+				throw new InvalidArgumentException('Invalid errors option specified');
+			}


Suggested change

if (!in_array($errors, CalendarImportOptions::ERROR_OPTIONS, true)) {

throw new InvalidArgumentException('Invalid errors option specified');

}

Validation is done in setErrors

To be honest, I would prefer to remove the validation all together from here and the CalendarImportOptions. This would allow additional error options that are specific to each app verses forcing specific ones.

What do you think? Lets have a call to discuss the benefits.

As mentioned in our call, CalendarImportOptions.setErrors already validates the input. Whether you validate it in the command or in the options object doesn’t really matter, but please don’t do it in both places 😉 My preference would be to keep the validation in the options object.

kesselb · 2025-05-08T19:22:10Z

apps/dav/lib/Command/ImportCalendar.php

+			if (!in_array($validation, CalendarImportOptions::VALIDATE_OPTIONS, true)) {
+				throw new InvalidArgumentException('Invalid validation option specified');
+			}


Suggested change

if (!in_array($validation, CalendarImportOptions::VALIDATE_OPTIONS, true)) {

throw new InvalidArgumentException('Invalid validation option specified');

}

Validation is done in setValidation

kesselb · 2025-05-08T19:23:35Z

apps/dav/lib/Command/ImportCalendar.php

+		if (!in_array($format, ImportService::FORMATS, true)) {
+			throw new InvalidArgumentException("Format <$format> is not valid.");
+		}


It would be nice to also move the validation to the options object like for errors and validate.

kesselb · 2025-05-08T19:40:06Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importText(CalendarImportOptions $options): Generator {


$options seems unused

Done on purpose, so that server code can be changed in the future with additional options but the function signature stays the same for any application that implements this.

As the ability to overwrite the import functionality is gone, there's also no need any longer to pass the unused options arguments. If we need it later, we can just add it again without breaking anything, as the implementation is now private.

kesselb · 2025-05-08T19:40:11Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importXml(CalendarImportOptions $options): Generator {


$options seems unused

See above comment

Ref https://github.com/nextcloud/server/pull/51925/files#r2277535274

kesselb · 2025-05-08T19:40:16Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importJson(CalendarImportOptions $options): Generator {


$options seems unused

See above comment

Ref https://github.com/nextcloud/server/pull/51925/files#r2277535274

kesselb · 2025-05-08T19:45:37Z

apps/dav/lib/CalDAV/CalendarImpl.php

+	 *
+	 * @return array<string,array<string,string|array<string>>>
+	 */
+	public function import(CalendarImportOptions $options, callable $generator): array {


I'm not super happy with the import method on the calendar object. I know we did the same for export, my bad for not catching it earlier. The issue is that the ImportService and the import method in the calendar are quite tightly coupled. I think it would be nicer to inject the CalDavBackend into the ImportService and let the service handle the import directly, without going through CalendarImpl.

I did this on purpose, so that any app that supports ICalendarImport can import data. If the service imports directly to the caldavbackend then a 3rd party apps can not use this code to import data, with the current design the DECK app for instance, can extend this own calendar object to import data, and save it, in anyway or format that it wants.

To be honest, now that I think about this, I think the outcome should also be passed in as a callback and the outcome result should be created in the service or command/controller layer, this would allow for streaming output in the command line or even api end point.

@kesselb but the import service is not exposed, is it? Wouldn't that be necessary to make the import service usable as entry point?

The import service is not exposed.

Right now, the import method doesn't use any private methods or fields from the Calendar object that aren’t already exposed, so there’s no technical reason to implement import and export functionality directly in the Calendar object.

I understand the idea behind ICalendarImport, and that apps might be able to override it, but honestly, that’s a use case I just don’t see.

Take the Deck example: a deck card is much more than just a calendar event. I’d be really surprised if the Deck team started subclassing Calendar and implementing their own marshalling to wrap a deck card into an iCal object. Sure, that might technically be possible; but why would they do that? ;)

Same goes for the Tasks and Pools apps, they don’t even use a calendar provider and rely on our code for CalDAV. If something’s missing for them when importing/exporting, it needs to be fixed here.

So we’re introducing a more complex implementation to support extensibility that no one has asked for yet, and it’s unclear whether anyone ever will. That’s why my suggestion is: don’t expose the functionality, and remove the related exporter interface again before the 32 freeze.

Unless there’s a concrete use case, I’d rather avoid introducing complexity without a clear benefit.

Okay, that makes sense. I agree that this extensibility is not on the horizon, so let's not prepare for it but cross that bridge when we get to it.

I also agree. I would prefer not having to maintain the public API for imports until we or some other app/team really needs it.

kesselb · 2025-05-21T08:16:22Z

apps/dav/lib/Command/ImportCalendar.php

+			->addArgument('uid', InputArgument::REQUIRED, 'Id of system user')
+			->addArgument('cid', InputArgument::REQUIRED, 'Id of calendar')
+			->addOption('format', null, InputOption::VALUE_REQUIRED, 'Format of input (ical, jcal, xcal) defaults to ical', 'ical')
+			->addOption('location', null, InputOption::VALUE_REQUIRED, 'Location of where to write the input. defaults to stdin')


I suggested using an option for the location to keep it in line with the export command, but it feels a bit off here, since most people probably just want to save it to a file anyway.

Using an optional argument for the location makes more sense in this case, and I might update the export command later to match. The 'format' option should stay as is, though, because of the default.

Also, please don’t forget to update the execute function; it currently treats format and location as arguments, which causes an error when running the command. In most other commands, we usually define the arguments first and then the options, so please update that as well.

So I can make this an argument instead of a option, but it would be optional in either case, as the code supports piping a file directly in to the command, this can't be enforced. So does it really make a difference if this an argument or a option?

I’d agree, it basically comes down to:

A: occ calendar:import alice personal --location /mnt/testfiles/lustige_feiertage.ics
B: occ calendar:import alice personal /mnt/testfiles/lustige_feiertage.ics

I assume most people will use the command to import from a file, such as a backup, so my preference is the version without --location.

That said, both versions are fine, and whether location is an option or an argument isn’t a blocker.

You're right, most people will most likely define a file to import, so there is no point in making them type in --location, i've changed it to a optional argument

kesselb · 2025-05-21T08:19:22Z

apps/dav/lib/Command/ImportCalendar.php

+			if (!in_array($errors, CalendarImportOptions::ERROR_OPTIONS, true)) {
+				throw new InvalidArgumentException('Invalid errors option specified');
+			}


As mentioned in our call, CalendarImportOptions.setErrors already validates the input. Whether you validate it in the command or in the options object doesn’t really matter, but please don’t do it in both places 😉 My preference would be to keep the validation in the options object.

kesselb · 2025-05-21T08:49:51Z

apps/dav/lib/CalDAV/CalendarImpl.php

+	 *
+	 * @return array<string,array<string,string|array<string>>>
+	 */
+	public function import(CalendarImportOptions $options, callable $generator): array {


The import service is not exposed.

Right now, the import method doesn't use any private methods or fields from the Calendar object that aren’t already exposed, so there’s no technical reason to implement import and export functionality directly in the Calendar object.

I understand the idea behind ICalendarImport, and that apps might be able to override it, but honestly, that’s a use case I just don’t see.

Take the Deck example: a deck card is much more than just a calendar event. I’d be really surprised if the Deck team started subclassing Calendar and implementing their own marshalling to wrap a deck card into an iCal object. Sure, that might technically be possible; but why would they do that? ;)

Same goes for the Tasks and Pools apps, they don’t even use a calendar provider and rely on our code for CalDAV. If something’s missing for them when importing/exporting, it needs to be fixed here.

So we’re introducing a more complex implementation to support extensibility that no one has asked for yet, and it’s unclear whether anyone ever will. That’s why my suggestion is: don’t expose the functionality, and remove the related exporter interface again before the 32 freeze.

Unless there’s a concrete use case, I’d rather avoid introducing complexity without a clear benefit.

SebastianKrupinski · 2025-07-21T15:42:00Z

@kesselb

I moved the code from the calendar to the service

kesselb · 2025-08-14T19:23:03Z

lib/public/Calendar/CalendarImportOptions.php

+	 * @param 'ical'|'jcal'|'xcal' $format
+	 */
+	public function setFormat(string $value): void {
+		if (!in_array($value, ImportService::FORMATS, true)) {


Psalm is raising a warning because \OCA\DAV\CalDAV\Import\ImportService is private, but referenced by a class part of the public interface. The easiest way to resolve the warning is to move the formats const over here.

Okay, this feels odd, the service should define the formats it supports. But I've changes it.

Huhu,

If you’d rather not move the formats constant over here, I’m fine with a different approach too. The main thing is to keep psalm happy ;)

The psalm warning

lib/public/Calendar/CalendarImportOptions.php:56:25: UndefinedClass: Class, interface or enum named OCA\DAV\CalDAV\Import\ImportService does not exist

means that part of the public API is referencing an implementation detail.

For me, formats, validate_options, and error_options are similar in the sense that the service uses them to alter its behavior, so I’d prefer to keep them together in one place. Whether that’s in the Options object or in the service itself doesn’t matter.

If you prefer having them in the service, then you can’t use them in the Options object. I haven’t tested it, but moving the Options object to OCA\DAV\CalDAV\Import could be one solution. Another option would be, similar to what you did with the exporter, to avoid any validation in the Options object. That way, you wouldn’t run into issues with referencing private code in public.

At this point it does not matter, lets just keep it where it is.

lib/public/Calendar/CalendarImportOptions.php

kesselb · 2025-08-14T19:25:47Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importText(CalendarImportOptions $options): Generator {


As the ability to overwrite the import functionality is gone, there's also no need any longer to pass the unused options arguments. If we need it later, we can just add it again without breaking anything, as the implementation is now private.

kesselb · 2025-08-14T19:26:16Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importXml(CalendarImportOptions $options): Generator {


Ref https://github.com/nextcloud/server/pull/51925/files#r2277535274

kesselb · 2025-08-14T19:27:29Z

apps/dav/lib/CalDAV/Import/ImportService.php

+	 *
+	 * @return Generator<\Sabre\VObject\Component\VCalendar>
+	 */
+	private function importJson(CalendarImportOptions $options): Generator {


Ref https://github.com/nextcloud/server/pull/51925/files#r2277535274

apps/dav/lib/Command/ImportCalendar.php

kesselb

php-cs is unhappy
please merge master

kesselb · 2025-08-20T10:37:50Z

apps/dav/lib/CalDAV/Import/ImportService.php

+		$calendarId = $calendar->getKey();
+		$principalUri = $calendar->getPrincipalUri();
+		$outcome = [];
+		foreach ($generator($options) as $vObject) {


Suggested change

foreach ($generator($options) as $vObject) {

foreach ($generator() as $vObject) {

Oops. Fixed

apps/dav/appinfo/info.xml

kesselb · 2025-08-26T19:38:37Z

apps/dav/lib/CalDAV/Import/ImportService.php

+ */
+class ImportService {
+
+	private $source;


Suggested change

private $source;

/** @var resource */

private $source;

apps/dav/lib/CalDAV/Import/ImportService.php

apps/dav/lib/CalDAV/Import/TextImporter.php

apps/dav/lib/CalDAV/Import/XmlImporter.php

apps/dav/lib/Command/ImportCalendar.php

apps/dav/composer/composer/autoload_classmap.php

kesselb · 2025-08-27T11:25:15Z

Scenario: Exporting a calendar for alice and then importing it into a different calendar under the same account does not work.

occ calendar:export --format xcal --location /tmp/cal_export.xcal alice personal
occ dav:create-calendar alice test-123
occ calendar:import --format xcal alice test-123 /tmp/cal_export.xcal

My expectation would be that importing such a file into another calendar of the same user should work. Technically, we find the existing event through getCalendarObjectByUID and then either skip or update it. What’s missing is a check to see if the import is targeting the same calendar or a different one. I still approved the PR, but maybe we should consider adding such a check / making that case work too.

SebastianKrupinski · 2025-08-27T12:14:28Z

Scenario: Exporting a calendar for alice and then importing it into a different calendar under the same account does not work.

Good find, I didn't even think of this, I've corrected the issue

Signed-off-by: SebastianKrupinski <[email protected]>

SebastianKrupinski · 2025-09-21T21:20:15Z

Documentation nextcloud/documentation#13691

SebastianKrupinski added the 3. to review Waiting for reviews label Apr 4, 2025

SebastianKrupinski requested review from ChristophWurst, hamza221, kesselb and st3iny April 4, 2025 00:41

SebastianKrupinski self-assigned this Apr 4, 2025

SebastianKrupinski requested review from a team and tcitworld as code owners April 4, 2025 00:41

SebastianKrupinski added this to 💌 📅 👥 Groupware team Apr 4, 2025

SebastianKrupinski requested review from Altahrim, nfebe and yemkareems and removed request for a team April 4, 2025 00:41

github-project-automation bot moved this to 🏗️ In progress in 💌 📅 👥 Groupware team Apr 4, 2025

susnux added enhancement feature: caldav Related to CalDAV internals labels Apr 4, 2025

susnux added this to the Nextcloud 32 milestone Apr 4, 2025

ChristophWurst approved these changes Apr 30, 2025

View reviewed changes

SebastianKrupinski mentioned this pull request Apr 30, 2025

webdav DbalException An exception occurred while executing a query: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'uri' at row 1 nextcloud/calendar#6925

Open

kesselb reviewed May 7, 2025

View reviewed changes

apps/dav/lib/Command/ImportCalendar.php Outdated Show resolved Hide resolved

apps/dav/lib/Command/ImportCalendar.php Outdated Show resolved Hide resolved

kesselb reviewed May 7, 2025

View reviewed changes

apps/dav/appinfo/info.xml Outdated Show resolved Hide resolved

SebastianKrupinski force-pushed the feat/issue-563-calendar-import branch from aa32cd8 to be42c1e Compare May 8, 2025 18:17

SebastianKrupinski commented May 8, 2025

View reviewed changes

apps/dav/appinfo/info.xml Outdated Show resolved Hide resolved

apps/dav/lib/Command/ImportCalendar.php Outdated Show resolved Hide resolved

apps/dav/lib/Command/ImportCalendar.php Outdated Show resolved Hide resolved

kesselb reviewed May 8, 2025

View reviewed changes

kesselb reviewed May 21, 2025

View reviewed changes

kesselb mentioned this pull request Jun 2, 2025

[Bug]: Importing an event is not possible if an event with the same UID exists in the trash #53262

Open

ChristophWurst mentioned this pull request Jul 8, 2025

feat: Calendar Import #53823

Closed

SebastianKrupinski force-pushed the feat/issue-563-calendar-import branch 2 times, most recently from d72a25d to 0bfb9cc Compare July 21, 2025 15:14

SebastianKrupinski requested a review from kesselb July 21, 2025 15:42

kesselb reviewed Aug 14, 2025

View reviewed changes

SebastianKrupinski force-pushed the feat/issue-563-calendar-import branch from 6cf58f4 to 9037de4 Compare August 20, 2025 04:20

SebastianKrupinski requested a review from kesselb August 20, 2025 04:41

kesselb reviewed Aug 20, 2025

View reviewed changes

SebastianKrupinski requested a review from kesselb August 20, 2025 13:11

This was referenced Aug 22, 2025

32.0.0 beta 2 #54581

Merged

32.0.0 beta 3 #54606

Merged

kesselb reviewed Aug 26, 2025

View reviewed changes

SebastianKrupinski requested a review from kesselb August 27, 2025 00:24

kesselb approved these changes Aug 27, 2025

View reviewed changes

feat: Calendar Import

46e624a

Signed-off-by: SebastianKrupinski <[email protected]>

SebastianKrupinski force-pushed the feat/issue-563-calendar-import branch from 43d3c31 to 46e624a Compare August 27, 2025 12:31

SebastianKrupinski merged commit 9e9f3b9 into master Aug 27, 2025
215 of 219 checks passed

SebastianKrupinski deleted the feat/issue-563-calendar-import branch August 27, 2025 13:04

github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 💌 📅 👥 Groupware team Aug 27, 2025

nextcloud-bot mentioned this pull request Aug 28, 2025

32.0.0 beta 4 #54688

Merged

SebastianKrupinski mentioned this pull request Sep 21, 2025

feat: calendar import + export nextcloud/documentation#13691

Merged

skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025

SebastianKrupinski mentioned this pull request Oct 15, 2025

Import: check if UID already exists, check which one is newest and update if necessary. nextcloud/calendar#1508

Open

	if (!in_array($errors, CalendarImportOptions::ERROR_OPTIONS, true)) {
	throw new InvalidArgumentException('Invalid errors option specified');
	}

	if (!in_array($validation, CalendarImportOptions::VALIDATE_OPTIONS, true)) {
	throw new InvalidArgumentException('Invalid validation option specified');
	}

	foreach ($generator($options) as $vObject) {
	foreach ($generator() as $vObject) {

Uh oh!

feat: Calendar Import #51925

feat: Calendar Import #51925

Uh oh!

Conversation

SebastianKrupinski commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

OCC Import

Checklist

Uh oh!

Dexus commented Apr 30, 2025

Uh oh!

SebastianKrupinski commented Apr 30, 2025

Uh oh!

Dexus commented Apr 30, 2025

Uh oh!

ChristophWurst commented May 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SebastianKrupinski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianKrupinski May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianKrupinski commented Apr 4, 2025 •

edited

Loading

SebastianKrupinski May 8, 2025 •

edited

Loading

SebastianKrupinski commented Jul 21, 2025 •

edited

Loading