From 4384f8349acf472b947141e2e770a886650610bd Mon Sep 17 00:00:00 2001 From: shbb-asdea Date: Fri, 8 May 2026 14:14:02 +0000 Subject: [PATCH] fix(proposal): avoid UID collision when converting a proposal without a known blocker convertProposal assigned the proposal's UUID to the new VEVENT before deciding between "PUT over an existing blocker" and "INSERT a new calendar object". When findCalendarBlocker missed the blocker (calendar not enumerated by the principal lookup, search backend lag, etc.) the INSERT collided with oc_calendarobjects.calobjects_by_uid_index because the orphan blocker still held the same UID, surfacing as a 500 with SQLSTATE[23505] and leaving the proposal unconverted. Defer the UID assignment until the blocker lookup result is known: when a blocker is found, keep reusing the proposal UUID so the in-place PUT preserves iCal threading; when no blocker is found, allocate a fresh UUID so the INSERT cannot clash with a stale row. Refs: nextcloud/calendar#7941 Signed-off-by: shbb-asdea --- lib/Service/Proposal/ProposalService.php | 12 +++- .../Service/Proposal/ProposalServiceTest.php | 55 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/lib/Service/Proposal/ProposalService.php b/lib/Service/Proposal/ProposalService.php index 9866993a20..8f8df6531a 100644 --- a/lib/Service/Proposal/ProposalService.php +++ b/lib/Service/Proposal/ProposalService.php @@ -341,7 +341,9 @@ public function convertProposal(IUser $user, int $proposalId, int $dateId, array $vObject = new VCalendar(); /** @var \Sabre\VObject\Component\VEvent $vEvent */ $vEvent = $vObject->add('VEVENT', []); - $vEvent->UID->setValue($proposal->getUuid() ?? Uuid::v4()->toRfc4122()); + // UID is assigned below depending on whether an existing blocker is reused (PUT in place) + // or a fresh calendar object is created. Reusing the proposal UUID for a fresh INSERT + // would collide with an orphan blocker held by oc_calendarobjects.calobjects_by_uid_index. $vEvent->add('DTSTART', $eventTimezone ? $selectedDate->getDate()->setTimezone($eventTimezone) : $selectedDate->getDate()); $vEvent->add('DTEND', (clone $vEvent->DTSTART->getDateTime())->add(new \DateInterval("PT{$proposal->getDuration()}M"))); $vEvent->add('SEQUENCE', 1); @@ -368,8 +370,16 @@ public function convertProposal(IUser $user, int $proposalId, int $dateId, array // convert existing calendar blocker to event if it exists, otherwise create a new event in the user's calendar $result = $this->findCalendarBlocker($user, $proposal); if ($result !== null) { + // blocker found in the user's calendars: reuse the proposal UUID so iCal threading + // is preserved. applyCalendarBlockersOrganizer issues a PUT against the same URI, + // which updates the existing row rather than inserting a new one. + $vEvent->UID->setValue($proposal->getUuid() ?? Uuid::v4()->toRfc4122()); $this->applyCalendarBlockersOrganizer($user, $result['calendarUri'], $result['eventUri'], $vObject); } else { + // blocker was not surfaced by the search (calendar not in the principal listing, + // search backend lag, etc.) but may still hold the proposal UID in oc_calendarobjects. + // Use a fresh UUID so the INSERT below cannot collide on calobjects_by_uid_index. + $vEvent->UID->setValue(Uuid::v4()->toRfc4122()); $userCalendar->createFromString( Uuid::v4()->toRfc4122() . '.ics', $vObject->serialize() diff --git a/tests/php/unit/Service/Proposal/ProposalServiceTest.php b/tests/php/unit/Service/Proposal/ProposalServiceTest.php index e36add0754..c15fb1abf6 100644 --- a/tests/php/unit/Service/Proposal/ProposalServiceTest.php +++ b/tests/php/unit/Service/Proposal/ProposalServiceTest.php @@ -585,6 +585,61 @@ public function testConvertProposalSuccess(): void { $this->addToAssertionCount(1); // If we reached this point, the test is successfully completed } + /** + * Regression: when no calendar blocker is found by the search, the converted event must not + * reuse the proposal's UUID. Otherwise an INSERT into oc_calendarobjects collides with a + * possibly-orphaned blocker holding the same UID and triggers SQLSTATE[23505] (#7941). + */ + public function testConvertProposalUsesFreshUuidWhenBlockerNotFound(): void { + $proposalEntry = $this->createProposalEntry(1, 'Convert Proposal'); + $proposalEntry->setDuration(60); + $proposalEntry->setUuid('9d8aa045-d371-49c1-9f70-aaaaaaaaaaaa'); + $dateEntry = new ProposalDateEntry(); + $dateEntry->setId(10); + $dateEntry->setPid(1); + $dateEntry->setUid('testuser'); + $dateEntry->setDate((new \DateTimeImmutable('+1 day'))->getTimestamp()); + + $this->proposalMapper->expects($this->once()) + ->method('fetchById') + ->with('testuser', 1) + ->willReturn($proposalEntry); + $this->proposalParticipantMapper->expects($this->once()) + ->method('fetchByProposalId') + ->with('testuser', 1) + ->willReturn([]); + $this->proposalDateMapper->expects($this->once()) + ->method('fetchByProposalId') + ->with('testuser', 1) + ->willReturn([$dateEntry]); + $this->proposalVoteMapper->expects($this->once()) + ->method('fetchByProposalId') + ->with('testuser', 1) + ->willReturn([]); + + // Simulate the bug condition: search across the user's calendars surfaces no blocker + // (even though one may still exist in oc_calendarobjects with the proposal's UID). + $this->calendarManager->method('getCalendarsForPrincipal')->willReturn([]); + + $calendar = $this->createMock(\OCP\Calendar\ICreateFromString::class); + $calendar->method('isDeleted')->willReturn(false); + $calendar->expects($this->once()) + ->method('createFromString') + ->with( + $this->callback(fn ($name) => str_ends_with($name, '.ics')), + $this->callback(fn ($data) => !str_contains($data, 'UID:' . $proposalEntry->getUuid())), + ); + $this->calendarManager->method('getPrimaryCalendar')->with('testuser')->willReturn($calendar); + + $this->proposalVoteMapper->expects($this->once())->method('deleteByProposalId')->with('testuser', 1); + $this->proposalParticipantMapper->expects($this->once())->method('deleteByProposalId')->with('testuser', 1); + $this->proposalDateMapper->expects($this->once())->method('deleteByProposalId')->with('testuser', 1); + $this->proposalMapper->expects($this->once())->method('deleteById')->with('testuser', 1); + + $this->service->convertProposal($this->user, 1, 10); + $this->addToAssertionCount(1); + } + public function testConvertProposalDateNotFound(): void { // mock objects // proposal entry