Skip to content

Commit fe906cf

Browse files
rich0rich0
committed
feat(dav): address review comments
Replace singular default-alarm-* integer properties with plural default-alarms-* JSON text columns. Migrate existing int defaults with two set-based QueryBuilder UPDATEs, then drop the legacy columns. No dual-write or server-side validation; propertyMap stores/retrieves JSON strings only (NC 35+ clients). Assisted-by: Grok:grok-4.5 Signed-off-by: Richard Freeman <rich@rich0.org>
1 parent 24485e2 commit fe906cf

7 files changed

Lines changed: 112 additions & 420 deletions

File tree

‎apps/dav/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
'OCA\\DAV\\CalDAV\\CalendarObject' => $baseDir . '/../lib/CalDAV/CalendarObject.php',
6262
'OCA\\DAV\\CalDAV\\CalendarProvider' => $baseDir . '/../lib/CalDAV/CalendarProvider.php',
6363
'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php',
64-
'OCA\\DAV\\CalDAV\\DefaultCalendarAlarms' => $baseDir . '/../lib/CalDAV/DefaultCalendarAlarms.php',
6564
'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => $baseDir . '/../lib/CalDAV/DefaultCalendarValidator.php',
6665
'OCA\\DAV\\CalDAV\\EmbeddedCalDavServer' => $baseDir . '/../lib/CalDAV/EmbeddedCalDavServer.php',
6766
'OCA\\DAV\\CalDAV\\EventComparisonService' => $baseDir . '/../lib/CalDAV/EventComparisonService.php',
@@ -398,6 +397,7 @@
398397
'OCA\\DAV\\Migration\\Version1038Date20260302000000' => $baseDir . '/../lib/Migration/Version1038Date20260302000000.php',
399398
'OCA\\DAV\\Migration\\Version1039Date20260408000000' => $baseDir . '/../lib/Migration/Version1039Date20260408000000.php',
400399
'OCA\\DAV\\Migration\\Version1040Date20260703120000' => $baseDir . '/../lib/Migration/Version1040Date20260703120000.php',
400+
'OCA\\DAV\\Migration\\Version1041Date20260703120100' => $baseDir . '/../lib/Migration/Version1041Date20260703120100.php',
401401
'OCA\\DAV\\Model\\ExampleEvent' => $baseDir . '/../lib/Model/ExampleEvent.php',
402402
'OCA\\DAV\\Paginate\\LimitedCopyIterator' => $baseDir . '/../lib/Paginate/LimitedCopyIterator.php',
403403
'OCA\\DAV\\Paginate\\PaginateCache' => $baseDir . '/../lib/Paginate/PaginateCache.php',

‎apps/dav/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ class ComposerStaticInitDAV
7676
'OCA\\DAV\\CalDAV\\CalendarObject' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObject.php',
7777
'OCA\\DAV\\CalDAV\\CalendarProvider' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarProvider.php',
7878
'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php',
79-
'OCA\\DAV\\CalDAV\\DefaultCalendarAlarms' => __DIR__ . '/..' . '/../lib/CalDAV/DefaultCalendarAlarms.php',
8079
'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => __DIR__ . '/..' . '/../lib/CalDAV/DefaultCalendarValidator.php',
8180
'OCA\\DAV\\CalDAV\\EmbeddedCalDavServer' => __DIR__ . '/..' . '/../lib/CalDAV/EmbeddedCalDavServer.php',
8281
'OCA\\DAV\\CalDAV\\EventComparisonService' => __DIR__ . '/..' . '/../lib/CalDAV/EventComparisonService.php',
@@ -413,6 +412,7 @@ class ComposerStaticInitDAV
413412
'OCA\\DAV\\Migration\\Version1038Date20260302000000' => __DIR__ . '/..' . '/../lib/Migration/Version1038Date20260302000000.php',
414413
'OCA\\DAV\\Migration\\Version1039Date20260408000000' => __DIR__ . '/..' . '/../lib/Migration/Version1039Date20260408000000.php',
415414
'OCA\\DAV\\Migration\\Version1040Date20260703120000' => __DIR__ . '/..' . '/../lib/Migration/Version1040Date20260703120000.php',
415+
'OCA\\DAV\\Migration\\Version1041Date20260703120100' => __DIR__ . '/..' . '/../lib/Migration/Version1041Date20260703120100.php',
416416
'OCA\\DAV\\Model\\ExampleEvent' => __DIR__ . '/..' . '/../lib/Model/ExampleEvent.php',
417417
'OCA\\DAV\\Paginate\\LimitedCopyIterator' => __DIR__ . '/..' . '/../lib/Paginate/LimitedCopyIterator.php',
418418
'OCA\\DAV\\Paginate\\PaginateCache' => __DIR__ . '/..' . '/../lib/Paginate/PaginateCache.php',

‎apps/dav/lib/CalDAV/CalDavBackend.php‎

Lines changed: 10 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,6 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
138138
public const CLASSIFICATION_PRIVATE = 1;
139139
public const CLASSIFICATION_CONFIDENTIAL = 2;
140140

141-
public const DEFAULT_ALARMS_PART_DAY_PROPERTY = '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarms-part-day';
142-
public const DEFAULT_ALARMS_FULL_DAY_PROPERTY = '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarms-full-day';
143-
144-
/** @var string[] DB columns for calendar properties outside propertyMap (custom serialization). */
145-
private const SERIALIZED_CALENDAR_DB_COLUMNS = [
146-
'default_alarms_pday',
147-
'default_alarms_fday',
148-
];
149-
150141
/**
151142
* List of CalDAV properties, and how they map to database field names and their type
152143
* Add your own properties by simply adding on to this array.
@@ -161,8 +152,8 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
161152
'{http://apple.com/ns/ical/}calendar-order' => ['calendarorder', 'int'],
162153
'{http://apple.com/ns/ical/}calendar-color' => ['calendarcolor', 'string'],
163154
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}deleted-at' => ['deleted_at', 'int'],
164-
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarm-part-day' => ['default_alarm_pday', 'int'],
165-
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarm-full-day' => ['default_alarm_fday', 'int'],
155+
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarms-part-day' => ['default_alarms_pday', 'string'],
156+
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}default-alarms-full-day' => ['default_alarms_fday', 'string'],
166157
];
167158

168159
/**
@@ -335,7 +326,7 @@ public function getCalendarsForUser($principalUri) {
335326
return $this->atomic(function () use ($principalUri) {
336327
$principalUriOriginal = $principalUri;
337328
$principalUri = $this->convertPrincipal($principalUri, true);
338-
$fields = $this->getCalendarMergeSerializedProperties();
329+
$fields = array_column($this->propertyMap, 0);
339330
$fields[] = 'id';
340331
$fields[] = 'uri';
341332
$fields[] = 'synctoken';
@@ -391,9 +382,10 @@ public function getCalendarsForUser($principalUri) {
391382
$principals = array_merge($principals, $this->principalBackend->getCircleMembership($principalUriOriginal));
392383
$principals[] = $principalUri;
393384

385+
$fields = array_column($this->propertyMap, 0);
394386
$fields = array_map(function (string $field) {
395387
return 'a.' . $field;
396-
}, $this->getCalendarMergeSerializedProperties());
388+
}, $fields);
397389
$fields[] = 'a.id';
398390
$fields[] = 'a.uri';
399391
$fields[] = 'a.synctoken';
@@ -476,7 +468,7 @@ public function getCalendarsForUser($principalUri) {
476468
*/
477469
public function getUsersOwnCalendars($principalUri) {
478470
$principalUri = $this->convertPrincipal($principalUri, true);
479-
$fields = $this->getCalendarMergeSerializedProperties();
471+
$fields = array_column($this->propertyMap, 0);
480472
$fields[] = 'id';
481473
$fields[] = 'uri';
482474
$fields[] = 'synctoken';
@@ -522,7 +514,7 @@ public function getUsersOwnCalendars($principalUri) {
522514
* @return array
523515
*/
524516
public function getPublicCalendars() {
525-
$fields = array_map(static fn (string $field): string => 'a.' . $field, $this->getCalendarMergeSerializedProperties());
517+
$fields = array_column($this->propertyMap, 0);
526518
$fields[] = 'a.id';
527519
$fields[] = 'a.uri';
528520
$fields[] = 'a.synctoken';
@@ -580,7 +572,7 @@ public function getPublicCalendars() {
580572
* @throws NotFound
581573
*/
582574
public function getPublicCalendar($uri) {
583-
$fields = array_map(static fn (string $field): string => 'a.' . $field, $this->getCalendarMergeSerializedProperties());
575+
$fields = array_column($this->propertyMap, 0);
584576
$fields[] = 'a.id';
585577
$fields[] = 'a.uri';
586578
$fields[] = 'a.synctoken';
@@ -639,7 +631,7 @@ public function getPublicCalendar($uri) {
639631
* @return array|null
640632
*/
641633
public function getCalendarByUri($principal, $uri) {
642-
$fields = $this->getCalendarMergeSerializedProperties();
634+
$fields = array_column($this->propertyMap, 0);
643635
$fields[] = 'id';
644636
$fields[] = 'uri';
645637
$fields[] = 'synctoken';
@@ -689,7 +681,7 @@ public function getCalendarByUri($principal, $uri) {
689681
* @return array|null
690682
*/
691683
public function getCalendarById(int $calendarId): ?array {
692-
$fields = $this->getCalendarMergeSerializedProperties();
684+
$fields = array_column($this->propertyMap, 0);
693685
$fields[] = 'id';
694686
$fields[] = 'uri';
695687
$fields[] = 'synctoken';
@@ -858,29 +850,9 @@ public function createCalendar($principalUri, $calendarUri, array $properties) {
858850
foreach ($this->propertyMap as $xmlName => [$dbName, $type]) {
859851
if (isset($properties[$xmlName])) {
860852
$values[$dbName] = $properties[$xmlName];
861-
if ($dbName === 'default_alarm_pday') {
862-
$values['default_alarms_pday'] = DefaultCalendarAlarms::encodeFromLegacyInt(
863-
$properties[$xmlName] !== null ? (int)$properties[$xmlName] : null,
864-
);
865-
} elseif ($dbName === 'default_alarm_fday') {
866-
$values['default_alarms_fday'] = DefaultCalendarAlarms::encodeFromLegacyInt(
867-
$properties[$xmlName] !== null ? (int)$properties[$xmlName] : null,
868-
);
869-
}
870853
}
871854
}
872855

873-
if (isset($properties[self::DEFAULT_ALARMS_PART_DAY_PROPERTY])) {
874-
$encoded = DefaultCalendarAlarms::validateAndEncode($properties[self::DEFAULT_ALARMS_PART_DAY_PROPERTY]);
875-
$values['default_alarms_pday'] = $encoded;
876-
$values['default_alarm_pday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
877-
}
878-
if (isset($properties[self::DEFAULT_ALARMS_FULL_DAY_PROPERTY])) {
879-
$encoded = DefaultCalendarAlarms::validateAndEncode($properties[self::DEFAULT_ALARMS_FULL_DAY_PROPERTY]);
880-
$values['default_alarms_fday'] = $encoded;
881-
$values['default_alarm_fday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
882-
}
883-
884856
[$calendarId, $calendarData] = $this->atomic(function () use ($values) {
885857
$query = $this->db->getQueryBuilder();
886858
$query->insert('calendars');
@@ -919,42 +891,18 @@ public function createCalendar($principalUri, $calendarUri, array $properties) {
919891
public function updateCalendar($calendarId, PropPatch $propPatch) {
920892
$supportedProperties = array_keys($this->propertyMap);
921893
$supportedProperties[] = '{' . Plugin::NS_CALDAV . '}schedule-calendar-transp';
922-
$supportedProperties[] = self::DEFAULT_ALARMS_PART_DAY_PROPERTY;
923-
$supportedProperties[] = self::DEFAULT_ALARMS_FULL_DAY_PROPERTY;
924894

925895
$propPatch->handle($supportedProperties, function ($mutations) use ($calendarId) {
926-
$storedDefaultAlarmsJson = $this->getCalendarDefaultAlarmsJson($calendarId);
927896
$newValues = [];
928897
foreach ($mutations as $propertyName => $propertyValue) {
929898
switch ($propertyName) {
930899
case '{' . Plugin::NS_CALDAV . '}schedule-calendar-transp':
931900
$fieldName = 'transparent';
932901
$newValues[$fieldName] = (int)($propertyValue->getValue() === 'transparent');
933902
break;
934-
case self::DEFAULT_ALARMS_PART_DAY_PROPERTY:
935-
$encoded = DefaultCalendarAlarms::validateAndEncode($propertyValue);
936-
$newValues['default_alarms_pday'] = $encoded;
937-
$newValues['default_alarm_pday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
938-
break;
939-
case self::DEFAULT_ALARMS_FULL_DAY_PROPERTY:
940-
$encoded = DefaultCalendarAlarms::validateAndEncode($propertyValue);
941-
$newValues['default_alarms_fday'] = $encoded;
942-
$newValues['default_alarm_fday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
943-
break;
944903
default:
945904
$fieldName = $this->propertyMap[$propertyName][0];
946905
$newValues[$fieldName] = $propertyValue;
947-
if ($fieldName === 'default_alarm_pday') {
948-
$newValues['default_alarms_pday'] = DefaultCalendarAlarms::mergeLegacyIntIntoJson(
949-
$storedDefaultAlarmsJson['default_alarms_pday'],
950-
$propertyValue !== null ? (int)$propertyValue : null,
951-
);
952-
} elseif ($fieldName === 'default_alarm_fday') {
953-
$newValues['default_alarms_fday'] = DefaultCalendarAlarms::mergeLegacyIntIntoJson(
954-
$storedDefaultAlarmsJson['default_alarms_fday'],
955-
$propertyValue !== null ? (int)$propertyValue : null,
956-
);
957-
}
958906
break;
959907
}
960908
}
@@ -4020,56 +3968,9 @@ private function rowToCalendar($row, array $calendar): array {
40203968
}
40213969
$calendar[$xmlName] = $value;
40223970
}
4023-
4024-
$calendar[self::DEFAULT_ALARMS_PART_DAY_PROPERTY] = DefaultCalendarAlarms::formatForCalDav(
4025-
isset($row['default_alarms_pday']) ? (string)$row['default_alarms_pday'] : null,
4026-
isset($row['default_alarm_pday']) && $row['default_alarm_pday'] !== null ? (int)$row['default_alarm_pday'] : null,
4027-
);
4028-
$calendar[self::DEFAULT_ALARMS_FULL_DAY_PROPERTY] = DefaultCalendarAlarms::formatForCalDav(
4029-
isset($row['default_alarms_fday']) ? (string)$row['default_alarms_fday'] : null,
4030-
isset($row['default_alarm_fday']) && $row['default_alarm_fday'] !== null ? (int)$row['default_alarm_fday'] : null,
4031-
);
4032-
40333971
return $calendar;
40343972
}
40353973

4036-
/**
4037-
* calendars-table SELECT columns: propertyMap DB fields plus columns for
4038-
* CalDAV properties stored outside propertyMap (custom serialization).
4039-
*
4040-
* @return string[]
4041-
*/
4042-
private function getCalendarMergeSerializedProperties(): array {
4043-
return array_merge(
4044-
array_column($this->propertyMap, 0),
4045-
self::SERIALIZED_CALENDAR_DB_COLUMNS,
4046-
);
4047-
}
4048-
4049-
/**
4050-
* @return array{default_alarms_pday: ?string, default_alarms_fday: ?string}
4051-
*/
4052-
private function getCalendarDefaultAlarmsJson(int $calendarId): array {
4053-
$query = $this->db->getQueryBuilder();
4054-
$query->select('default_alarms_pday', 'default_alarms_fday')
4055-
->from('calendars')
4056-
->where($query->expr()->eq('id', $query->createNamedParameter($calendarId, IQueryBuilder::PARAM_INT)))
4057-
->setMaxResults(1);
4058-
$row = $query->executeQuery()->fetchAssociative();
4059-
4060-
if ($row === false) {
4061-
return [
4062-
'default_alarms_pday' => null,
4063-
'default_alarms_fday' => null,
4064-
];
4065-
}
4066-
4067-
return [
4068-
'default_alarms_pday' => $row['default_alarms_pday'] !== null ? (string)$row['default_alarms_pday'] : null,
4069-
'default_alarms_fday' => $row['default_alarms_fday'] !== null ? (string)$row['default_alarms_fday'] : null,
4070-
];
4071-
}
4072-
40733974
/**
40743975
* Amend the subscription info with database row data
40753976
*

0 commit comments

Comments
 (0)