Skip to content

Commit 18b39c1

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 ea4d4ee commit 18b39c1

7 files changed

Lines changed: 113 additions & 421 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',
@@ -403,6 +402,7 @@
403402
'OCA\\DAV\\Migration\\Version1039Date20260408000000' => $baseDir . '/../lib/Migration/Version1039Date20260408000000.php',
404403
'OCA\\DAV\\Migration\\Version1040Date20260703120000' => $baseDir . '/../lib/Migration/Version1040Date20260703120000.php',
405404
'OCA\\DAV\\Migration\\Version1040Date20260805000000' => $baseDir . '/../lib/Migration/Version1040Date20260805000000.php',
405+
'OCA\\DAV\\Migration\\Version1041Date20260703120100' => $baseDir . '/../lib/Migration/Version1041Date20260703120100.php',
406406
'OCA\\DAV\\Model\\ExampleEvent' => $baseDir . '/../lib/Model/ExampleEvent.php',
407407
'OCA\\DAV\\Paginate\\LimitedCopyIterator' => $baseDir . '/../lib/Paginate/LimitedCopyIterator.php',
408408
'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',
@@ -418,6 +417,7 @@ class ComposerStaticInitDAV
418417
'OCA\\DAV\\Migration\\Version1039Date20260408000000' => __DIR__ . '/..' . '/../lib/Migration/Version1039Date20260408000000.php',
419418
'OCA\\DAV\\Migration\\Version1040Date20260703120000' => __DIR__ . '/..' . '/../lib/Migration/Version1040Date20260703120000.php',
420419
'OCA\\DAV\\Migration\\Version1040Date20260805000000' => __DIR__ . '/..' . '/../lib/Migration/Version1040Date20260805000000.php',
420+
'OCA\\DAV\\Migration\\Version1041Date20260703120100' => __DIR__ . '/..' . '/../lib/Migration/Version1041Date20260703120100.php',
421421
'OCA\\DAV\\Model\\ExampleEvent' => __DIR__ . '/..' . '/../lib/Model/ExampleEvent.php',
422422
'OCA\\DAV\\Paginate\\LimitedCopyIterator' => __DIR__ . '/..' . '/../lib/Paginate/LimitedCopyIterator.php',
423423
'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
@@ -137,15 +137,6 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
137137
public const CLASSIFICATION_PRIVATE = 1;
138138
public const CLASSIFICATION_CONFIDENTIAL = 2;
139139

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

167158
/**
@@ -334,7 +325,7 @@ public function getCalendarsForUser($principalUri) {
334325
return $this->atomic(function () use ($principalUri) {
335326
$principalUriOriginal = $principalUri;
336327
$principalUri = $this->convertPrincipal($principalUri, true);
337-
$fields = $this->getCalendarMergeSerializedProperties();
328+
$fields = array_column($this->propertyMap, 0);
338329
$fields[] = 'id';
339330
$fields[] = 'uri';
340331
$fields[] = 'synctoken';
@@ -390,9 +381,10 @@ public function getCalendarsForUser($principalUri) {
390381
$principals = array_merge($principals, $this->principalBackend->getCircleMembership($principalUriOriginal));
391382
$principals[] = $principalUri;
392383

384+
$fields = array_column($this->propertyMap, 0);
393385
$fields = array_map(function (string $field) {
394386
return 'a.' . $field;
395-
}, $this->getCalendarMergeSerializedProperties());
387+
}, $fields);
396388
$fields[] = 'a.id';
397389
$fields[] = 'a.uri';
398390
$fields[] = 'a.synctoken';
@@ -475,7 +467,7 @@ public function getCalendarsForUser($principalUri) {
475467
*/
476468
public function getUsersOwnCalendars($principalUri) {
477469
$principalUri = $this->convertPrincipal($principalUri, true);
478-
$fields = $this->getCalendarMergeSerializedProperties();
470+
$fields = array_column($this->propertyMap, 0);
479471
$fields[] = 'id';
480472
$fields[] = 'uri';
481473
$fields[] = 'synctoken';
@@ -521,7 +513,7 @@ public function getUsersOwnCalendars($principalUri) {
521513
* @return array
522514
*/
523515
public function getPublicCalendars() {
524-
$fields = array_map(static fn (string $field): string => 'a.' . $field, $this->getCalendarMergeSerializedProperties());
516+
$fields = array_column($this->propertyMap, 0);
525517
$fields[] = 'a.id';
526518
$fields[] = 'a.uri';
527519
$fields[] = 'a.synctoken';
@@ -579,7 +571,7 @@ public function getPublicCalendars() {
579571
* @throws NotFound
580572
*/
581573
public function getPublicCalendar($uri) {
582-
$fields = array_map(static fn (string $field): string => 'a.' . $field, $this->getCalendarMergeSerializedProperties());
574+
$fields = array_column($this->propertyMap, 0);
583575
$fields[] = 'a.id';
584576
$fields[] = 'a.uri';
585577
$fields[] = 'a.synctoken';
@@ -638,7 +630,7 @@ public function getPublicCalendar($uri) {
638630
* @return array|null
639631
*/
640632
public function getCalendarByUri($principal, $uri) {
641-
$fields = $this->getCalendarMergeSerializedProperties();
633+
$fields = array_column($this->propertyMap, 0);
642634
$fields[] = 'id';
643635
$fields[] = 'uri';
644636
$fields[] = 'synctoken';
@@ -688,7 +680,7 @@ public function getCalendarByUri($principal, $uri) {
688680
* @return array|null
689681
*/
690682
public function getCalendarById(int $calendarId): ?array {
691-
$fields = $this->getCalendarMergeSerializedProperties();
683+
$fields = array_column($this->propertyMap, 0);
692684
$fields[] = 'id';
693685
$fields[] = 'uri';
694686
$fields[] = 'synctoken';
@@ -857,29 +849,9 @@ public function createCalendar($principalUri, $calendarUri, array $properties) {
857849
foreach ($this->propertyMap as $xmlName => [$dbName, $type]) {
858850
if (isset($properties[$xmlName])) {
859851
$values[$dbName] = $properties[$xmlName];
860-
if ($dbName === 'default_alarm_pday') {
861-
$values['default_alarms_pday'] = DefaultCalendarAlarms::encodeFromLegacyInt(
862-
$properties[$xmlName] !== null ? (int)$properties[$xmlName] : null,
863-
);
864-
} elseif ($dbName === 'default_alarm_fday') {
865-
$values['default_alarms_fday'] = DefaultCalendarAlarms::encodeFromLegacyInt(
866-
$properties[$xmlName] !== null ? (int)$properties[$xmlName] : null,
867-
);
868-
}
869852
}
870853
}
871854

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

924894
$propPatch->handle($supportedProperties, function ($mutations) use ($calendarId) {
925-
$storedDefaultAlarmsJson = $this->getCalendarDefaultAlarmsJson($calendarId);
926895
$newValues = [];
927896
foreach ($mutations as $propertyName => $propertyValue) {
928897
switch ($propertyName) {
929898
case '{' . Plugin::NS_CALDAV . '}schedule-calendar-transp':
930899
$fieldName = 'transparent';
931900
$newValues[$fieldName] = (int)($propertyValue->getValue() === 'transparent');
932901
break;
933-
case self::DEFAULT_ALARMS_PART_DAY_PROPERTY:
934-
$encoded = DefaultCalendarAlarms::validateAndEncode($propertyValue);
935-
$newValues['default_alarms_pday'] = $encoded;
936-
$newValues['default_alarm_pday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
937-
break;
938-
case self::DEFAULT_ALARMS_FULL_DAY_PROPERTY:
939-
$encoded = DefaultCalendarAlarms::validateAndEncode($propertyValue);
940-
$newValues['default_alarms_fday'] = $encoded;
941-
$newValues['default_alarm_fday'] = DefaultCalendarAlarms::legacyIntFromJson($encoded);
942-
break;
943902
default:
944903
$fieldName = $this->propertyMap[$propertyName][0];
945904
$newValues[$fieldName] = $propertyValue;
946-
if ($fieldName === 'default_alarm_pday') {
947-
$newValues['default_alarms_pday'] = DefaultCalendarAlarms::mergeLegacyIntIntoJson(
948-
$storedDefaultAlarmsJson['default_alarms_pday'],
949-
$propertyValue !== null ? (int)$propertyValue : null,
950-
);
951-
} elseif ($fieldName === 'default_alarm_fday') {
952-
$newValues['default_alarms_fday'] = DefaultCalendarAlarms::mergeLegacyIntIntoJson(
953-
$storedDefaultAlarmsJson['default_alarms_fday'],
954-
$propertyValue !== null ? (int)$propertyValue : null,
955-
);
956-
}
957905
break;
958906
}
959907
}
@@ -4045,56 +3993,9 @@ private function rowToCalendar($row, array $calendar): array {
40453993
}
40463994
$calendar[$xmlName] = $value;
40473995
}
4048-
4049-
$calendar[self::DEFAULT_ALARMS_PART_DAY_PROPERTY] = DefaultCalendarAlarms::formatForCalDav(
4050-
isset($row['default_alarms_pday']) ? (string)$row['default_alarms_pday'] : null,
4051-
isset($row['default_alarm_pday']) && $row['default_alarm_pday'] !== null ? (int)$row['default_alarm_pday'] : null,
4052-
);
4053-
$calendar[self::DEFAULT_ALARMS_FULL_DAY_PROPERTY] = DefaultCalendarAlarms::formatForCalDav(
4054-
isset($row['default_alarms_fday']) ? (string)$row['default_alarms_fday'] : null,
4055-
isset($row['default_alarm_fday']) && $row['default_alarm_fday'] !== null ? (int)$row['default_alarm_fday'] : null,
4056-
);
4057-
40583996
return $calendar;
40593997
}
40603998

4061-
/**
4062-
* calendars-table SELECT columns: propertyMap DB fields plus columns for
4063-
* CalDAV properties stored outside propertyMap (custom serialization).
4064-
*
4065-
* @return string[]
4066-
*/
4067-
private function getCalendarMergeSerializedProperties(): array {
4068-
return array_merge(
4069-
array_column($this->propertyMap, 0),
4070-
self::SERIALIZED_CALENDAR_DB_COLUMNS,
4071-
);
4072-
}
4073-
4074-
/**
4075-
* @return array{default_alarms_pday: ?string, default_alarms_fday: ?string}
4076-
*/
4077-
private function getCalendarDefaultAlarmsJson(int $calendarId): array {
4078-
$query = $this->db->getQueryBuilder();
4079-
$query->select('default_alarms_pday', 'default_alarms_fday')
4080-
->from('calendars')
4081-
->where($query->expr()->eq('id', $query->createNamedParameter($calendarId, IQueryBuilder::PARAM_INT)))
4082-
->setMaxResults(1);
4083-
$row = $query->executeQuery()->fetchAssociative();
4084-
4085-
if ($row === false) {
4086-
return [
4087-
'default_alarms_pday' => null,
4088-
'default_alarms_fday' => null,
4089-
];
4090-
}
4091-
4092-
return [
4093-
'default_alarms_pday' => $row['default_alarms_pday'] !== null ? (string)$row['default_alarms_pday'] : null,
4094-
'default_alarms_fday' => $row['default_alarms_fday'] !== null ? (string)$row['default_alarms_fday'] : null,
4095-
];
4096-
}
4097-
40983999
/**
40994000
* Amend the subscription info with database row data
41004001
*

0 commit comments

Comments
 (0)