fix(dav): Fix atomic calendar/subscription updates #43904
Conversation
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
ChristophWurst
added
bug
3. to review
ChristophWurst
requested review from
miaulalala and
tcitworld
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| $supportedProperties = array_keys($this->propertyMap); | ||
| $supportedProperties[] = '{' . Plugin::NS_CALDAV . '}schedule-calendar-transp'; | ||
|
|
||
| $propPatch->handle($supportedProperties, function ($mutations) use ($calendarId) { |
Check notice
Code scanning / Psalm
MissingClosureParamType Note
| $supportedProperties = array_keys($this->subscriptionPropertyMap); | ||
| $supportedProperties[] = '{http://calendarserver.org/ns/}source'; | ||
|
|
||
| $propPatch->handle($supportedProperties, function ($mutations) use ($subscriptionId) { |
Check notice
Code scanning / Psalm
MissingClosureParamType Note
| return true; | ||
| }); | ||
| }, $this->db); | ||
| $this->dispatcher->dispatchTyped(new CalendarUpdatedEvent($calendarId, $calendarData, $shares, $mutations)); |
Check notice
Code scanning / Psalm
PossiblyNullArgument Note
There was a problem hiding this comment.
Strictly speaking this is correct. \OCA\DAV\CalDAV\CalDavBackend::getCalendarById returns array|null. But this was used without a null check before. Now the data is just wrapped/returned by the closure. Psalm reports here because the diff make the code appear "new".
I'd leave it as is.
|
|
||
| $subscriptionRow = $this->atomic(function () use ($subscriptionId, $newValues) { |
Check notice
Code scanning / Psalm
MissingClosureReturnType Note
Summary
Sibiling to #43903 for CalDAV.
The transaction is needed for the database queries but Sabre calls the closure asynchronously. So the transaction span has to be moved into the callback.
This is best reviewed as https://github.com/nextcloud/server/pull/43904/files?w=1
Checklist