Send the meeting response only once the meeting is stored - #11
Open
r4nc0r wants to merge 1 commit into
Open
Conversation
accept() created and submitted the response as its first act, before any calendar work. Everything after that - creating the calendar item, the recurring branch, acceptException(), and in the $move path deleting the previous item before recreating it - therefore runs with the acceptance already delivered. Any failure in that window leaves the organiser with an acceptance for a meeting the attendee does not have, and the attendee with no appointment. The response cannot simply be built later: it is assembled from the request message, which the $move path copies into the calendar and then removes. So keep building it where it was, but stop short of submitting. createResponse() gains a $defer flag and returns the prepared message, accept() submits it after the calendar item exists, and doAccept() drops it in a finally block if accept() threw, so an unsent response never stays behind in the Outbox. An accept that stores nothing returns an empty entryid, and that case now withholds the response as well rather than reporting success. Measured on a gromox test rig with a failure injected immediately after createResponse(): before, the organiser's response count went 15 to 16 with no calendar item; after, it stays at 17, the request mail is still in the inbox, and the Outbox holds no unsent response. The ordinary accept path is unchanged, including when the invitation conflicts with an existing appointment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Meetingrequest::accept()creates and submits the response as its first act, before any calendar work:Any failure after that point leaves the organizer holding an acceptance for a meeting the attendee does not have, and the attendee with nothing in the calendar. The window is not small: it spans the whole recurring branch,
acceptException(), and amapi_folder_deletemessages()on the attendee's existing calendar item that happens before the replacement is created, so a failure there can also remove a meeting the attendee already had.Insufficient calendar rights are not affected —
ensureCalendarWriteAccess()throws before the response is built — but every other failure in that window is.Why the response cannot simply be built later
It is assembled from the request message:
createResponse()readsPR_SENT_REPRESENTING_*, the GlobalObjectIds, subject, start/due, recurrence pattern and timezone from$this->message, and constructs aRecurrenceover it. In the$movepath that message is copied into the calendar and the original removed, so those reads have to happen while it is still intact.The change
Keep building the response where it was; stop short of submitting it.
createResponse()gains abool $defer = falseparameter. When set it returns the prepared, saved-but-unsubmitted message instead of callingmapi_message_submitmessage(). Both existing call sites are internal, anddoDecline()is untouched.accept()stores that message and submits it at the end, once the calendar item exists.doAccept()wraps the twoaccept()calls intry/finallyand discards any still-pending response, so an unsent message never stays behind in the Outbox whenaccept()throws.createOutgoingMessage()builds it in the Outbox, so leaving it there would show the user a stuck item.Net effect: the failure mode changes from "response sent, meeting lost" to "nothing happened", which is recoverable — the request mail is still in the inbox and the user can accept again.
Testing
On a gromox test rig (php-mapi, real store), driving the same sequence
class.itemmodule.phpuses foracceptMeetingRequest, with a failure injected immediately aftercreateResponse():