feat: OCC and OCS Calendar Import/Export #49995

SebastianKrupinski · 2024-12-30T20:47:00Z

Summary

This adds the ability to export calendars via the OCS and OCC.

OCC Export

Command: calendar:export
Arguments: userId calendarId format filepath

OCC Import

Command: calendar:import
Arguments: userId calendarId format filepath
Options: errors, validation, supersede, show-created, show-updated, show-skipped, show-errors

OCS Export

Endpoint: /ocs/v2.php/calendar/export
Request GET

/ocs/v2.php/calendar/export?id=personal&format=xcal&user=user1

Request: POST

{
    "id": "personal",
    "format": "xcal", (optional "ical, jcal, xcal", defaults to "ical")
	"options": {
		"rangeStart": 0,
		"rangeCount": 100
	},
	"user": "user1" (optional admin permissions required)
}

OCS Import

Endpoint: /ocs/v2.php/calendar/import
Request: POST

{
	"id": "personal",
	"options": {
		"format": "xcal", (optional "ical, jcal, xcal", defaults to "ical")
		"validation": 0, (0 - no validate, 1 - validate and skip, 2 - validate and error)
		"errors": 0, (0 - continue, 1 - fail)
		"supersede": true
	},
	"user": "user1", (optional admin permissions required)
	"data": "<?xml version=\"1.0\" encoding=\"UTF-8\"?><calendar><event><title>Meeting</title><date>2025-02-18</date></event></calendar>"
}

Performance

Export performance bottle neck seems to be CPU core max out due to single process execution, tests on dev setup with 16k of events takes about 5min or 60 items / second
Import performance bottle neck seems to be database write performance, tests on dev setup with 16k of events, takes about 30min or 10 items / second

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)

SebastianKrupinski · 2025-01-27T15:04:54Z

Hi @tcitworld

I was wondering if you would be willing to test this out with some of your larger live data calendars?

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

lib/public/AppFramework/Http/StreamGeneratorResponse.php

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

ChristophWurst

First round of feedback

ChristophWurst · 2025-02-13T07:43:22Z

apps/dav/lib/CalDAV/CalendarImpl.php

+	 *
+	 * @since 32.0.0
+	 *
+	 * @return array


can this be typed?

Yes, as long a pslam plays nice.

ChristophWurst · 2025-02-13T07:43:47Z

apps/dav/lib/CalDAV/CalendarImpl.php

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


the blank lines at a method start are strange. please try to avoid them.

ChristophWurst · 2025-02-13T07:45:07Z

apps/dav/lib/CalDAV/Export/ExportService.php

+/**
+ * Calendar Export Service
+ *
+ * @since 32.0.0


FYI the @since annotations are used for public API. You don't have to add this for internal structures, because they won't be used in apps.

TBH, I add these as a way of tracking when something was added, so when I look though our code I can see how new or old something is, and if there are fixes to back port is easy to see how far back you can go.

Do you want me to remove them?

ChristophWurst · 2025-02-13T07:45:20Z

apps/dav/lib/CalDAV/Export/ExportService.php

+	public function __construct() {
+	}


Does this mean fire as in, this is great or burn it? Lol

ChristophWurst · 2025-02-13T07:46:24Z

apps/dav/lib/Command/ExportCalendar.php

+		// construct options object
+		$options = new CalendarExportOptions();
+		// evaluate if provided format is supported
+		if ($format !== null && !in_array($format, $this->exportService::FORMATS)) {


always use in_array in strict mode

ChristophWurst · 2025-02-13T07:50:30Z

apps/dav/lib/Controller/CalendarExportController.php

+	#[ApiRoute(verb: 'GET', url: '/export', root: '/calendar')]
+	#[ApiRoute(verb: 'POST', url: '/export', root: '/calendar')]


I don't get why it is GET and POST

ChristophWurst · 2025-02-13T07:51:10Z

apps/dav/lib/Controller/CalendarExportController.php

+				return new DataResponse(['error' => 'user not found'], Http::STATUS_BAD_REQUEST);
+			}
+		} else {
+			$userId = $this->userSession->getUser()->getUID();


opening the route unauthenticated with no userId passed will lead to a NPE when calling getUID

I tested this with Insomnia, by sending a request without authentication or cookies.

Requests that are NOT authenticated get rejected by the initial check as expected.

if (!$this->userSession->isLoggedIn()) { return new DataResponse([], Http::STATUS_UNAUTHORIZED); }

Is this sufficient or am I missing something?

ChristophWurst · 2025-02-13T07:51:51Z

apps/dav/lib/Controller/CalendarExportController.php

+		};
+
+		return new StreamGeneratorResponse($this->exportService->export($calendar, $options), $contentType);
+


unnecessary blank line

ChristophWurst · 2025-02-13T07:52:27Z

apps/dav/lib/Controller/CalendarImportController.php

+	#[ApiRoute(verb: 'POST', url: '/import', root: '/calendar')]
+	#[UserRateLimit(limit: 1, period: 60)]
+	#[NoAdminRequired]
+	public function index(string $id, array $options, string $data, ?string $user = null) {


missing return type hint

missing types for options

ChristophWurst · 2025-02-13T07:58:08Z

apps/dav/lib/Controller/CalendarImportController.php

+				return new DataResponse(['error' => 'user not found'], Http::STATUS_BAD_REQUEST);
+			}
+		} else {
+			$userId = $this->userSession->getUser()->getUID();


See comments on the CalendarExportController.

ChristophWurst · 2025-02-13T08:03:39Z

When adding a new controller for HTTP routes I'm wondering if we shouldn't got for OCS and openapi specs right away. Could you please check how doable that is with the different input/output formats you want to support?

SebastianKrupinski · 2025-02-17T13:53:36Z

Found a new internal use for this, this morning, calendar subscriptions can use the same import code to reduce redundant code, complexity and increase performance.

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

SebastianKrupinski · 2025-02-19T00:33:11Z

When adding a new controller for HTTP routes I'm wondering if we shouldn't got for OCS and openapi specs right away. Could you please check how doable that is with the different input/output formats you want to support?

I think we can make it work, Import is good already, just struggling getting a proper phpdoc export

SebastianKrupinski added the 2. developing Work in progress label Dec 30, 2024

SebastianKrupinski self-assigned this Dec 30, 2024

SebastianKrupinski mentioned this pull request Dec 30, 2024

feat: OCC and OCS Calendar Import/Export nextcloud/calendar#6596

Closed

1 task

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from b0ee614 to 5198d6b Compare January 18, 2025 19:26

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from c69a4fb to 8a6dd83 Compare January 26, 2025 18:15

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from 59a2223 to 67dc029 Compare January 27, 2025 21:35

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from 67dc029 to 8ad0991 Compare February 4, 2025 17:18

SebastianKrupinski marked this pull request as ready for review February 4, 2025 17:19

SebastianKrupinski requested review from st3iny and tcitworld as code owners February 4, 2025 17:19

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from 8ad0991 to 213c1aa Compare February 4, 2025 17:20

feat: OCC and OCS Calendar Import/Export

8eff6e4

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

SebastianKrupinski force-pushed the enh/issues-563-calendar-import-export branch from a2f9ff4 to 8eff6e4 Compare February 11, 2025 19:37

SebastianKrupinski requested a review from kesselb February 11, 2025 19:39

kesselb reviewed Feb 11, 2025

View reviewed changes

lib/public/AppFramework/Http/StreamGeneratorResponse.php Outdated Show resolved Hide resolved

fixup! feat: OCC and OCS Calendar Import/Export

bd1c20a

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

ChristophWurst reviewed Feb 13, 2025

View reviewed changes

SebastianKrupinski added 5 commits February 18, 2025 15:17

fixup! feat: OCC and OCS Calendar Import/Export

b43b7d5

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

fixup! feat: OCC and OCS Calendar Import/Export

8ed0c90

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

fixup! feat: OCC and OCS Calendar Import/Export

292165a

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

fixup! feat: OCC and OCS Calendar Import/Export

4d79357

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

fixup! feat: OCC and OCS Calendar Import/Export

e9b0cfe

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: OCC and OCS Calendar Import/Export #49995

feat: OCC and OCS Calendar Import/Export #49995

SebastianKrupinski commented Dec 30, 2024 •

edited

Loading

SebastianKrupinski commented Jan 27, 2025

ChristophWurst left a comment

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 17, 2025

ChristophWurst Feb 13, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025 •

edited

Loading

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst Feb 13, 2025

ChristophWurst Feb 13, 2025

ChristophWurst Feb 13, 2025

SebastianKrupinski Feb 18, 2025

ChristophWurst commented Feb 13, 2025

SebastianKrupinski commented Feb 17, 2025

SebastianKrupinski commented Feb 19, 2025

		#[ApiRoute(verb: 'GET', url: '/export', root: '/calendar')]
		#[ApiRoute(verb: 'POST', url: '/export', root: '/calendar')]

		};

		return new StreamGeneratorResponse($this->exportService->export($calendar, $options), $contentType);

feat: OCC and OCS Calendar Import/Export #49995

Are you sure you want to change the base?

feat: OCC and OCS Calendar Import/Export #49995

Conversation

SebastianKrupinski commented Dec 30, 2024 • edited Loading

Summary

OCC Export

OCC Import

OCS Export

OCS Import

Performance

Checklist

SebastianKrupinski commented Jan 27, 2025

ChristophWurst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SebastianKrupinski Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristophWurst commented Feb 13, 2025

SebastianKrupinski commented Feb 17, 2025

SebastianKrupinski commented Feb 19, 2025

SebastianKrupinski commented Dec 30, 2024 •

edited

Loading

SebastianKrupinski Feb 18, 2025 •

edited

Loading