Forget pushed event ids when another calendar is chosen (Feature 16)
Choosing a calendar other than the stored one now clears every plan day's calendar_event_id and edits each plan that had one (revision raised, synced -> changed) in the same transaction as the choice, so a stored event id is always on the chosen calendar and the next push writes the whole week there. PushEvents now runs the push's record under the same account-lock hold as its event writes, so the chosen calendar cannot change between the writes and the sync bookkeeping; a push that read a plan before the choice is refused by its revision check (ErrPlanChanged). Fixes review finding F-01 on task 5885d2b1-cf06-44ff-abf3-242d7763c3c9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+44
-13
@@ -71,7 +71,8 @@ func (s *Service) Calendars(ctx context.Context) ([]Calendar, error) {
|
||||
// and name. id must be among Calendars at the time of the call; anything
|
||||
// else, including a calendar chosen earlier that has since been deleted or
|
||||
// is no longer writable, is ErrUnknownCalendar and leaves the stored
|
||||
// choice as it was.
|
||||
// choice as it was. Choosing another calendar than the one chosen returns
|
||||
// every plan pushed to that one to not synced; see saveSelection.
|
||||
func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, error) {
|
||||
s.account.Lock()
|
||||
defer s.account.Unlock()
|
||||
@@ -84,24 +85,54 @@ func (s *Service) SelectCalendar(ctx context.Context, id string) (Selection, err
|
||||
continue
|
||||
}
|
||||
sel := Selection{CalendarID: c.ID, CalendarName: c.Summary}
|
||||
n, err := db.New(s.st.Writer).SetIntegrationsGoogleCalendar(ctx, db.SetIntegrationsGoogleCalendarParams{
|
||||
CalendarID: sql.NullString{String: sel.CalendarID, Valid: true},
|
||||
CalendarName: sql.NullString{String: sel.CalendarName, Valid: true},
|
||||
UpdatedAt: store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String(),
|
||||
})
|
||||
if err != nil {
|
||||
return Selection{}, fmt.Errorf("gcal: save calendar: %w", err)
|
||||
}
|
||||
if n != 1 {
|
||||
// The lock is held since the connection was read, so only a
|
||||
// change outside this service gets here.
|
||||
return Selection{}, ErrNotConnected
|
||||
if err := s.saveSelection(ctx, sel); err != nil {
|
||||
return Selection{}, err
|
||||
}
|
||||
return sel, nil
|
||||
}
|
||||
return Selection{}, ErrUnknownCalendar
|
||||
}
|
||||
|
||||
// saveSelection stores sel as the chosen calendar. Choosing another calendar
|
||||
// than the one stored also forgets every plan day's calendar event id, in
|
||||
// the same transaction, since those events are on the calendar no longer
|
||||
// chosen: each plan with one is edited, so a synced plan becomes changed and
|
||||
// its next push writes every day to sel. The caller holds s.account, which a
|
||||
// push also holds from its event writes through recording them, so a push
|
||||
// never records events written to another calendar than the chosen one.
|
||||
func (s *Service) saveSelection(ctx context.Context, sel Selection) error {
|
||||
return s.st.WithTx(ctx, func(tx *sql.Tx) error {
|
||||
q := db.New(tx)
|
||||
row, err := q.GetIntegrationsGoogle(ctx)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
// The lock is held since the connection was read, so only a
|
||||
// change outside this service gets here.
|
||||
return ErrNotConnected
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("gcal: read Google integration: %w", err)
|
||||
}
|
||||
now := store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String()
|
||||
if _, err := q.SetIntegrationsGoogleCalendar(ctx, db.SetIntegrationsGoogleCalendarParams{
|
||||
CalendarID: sql.NullString{String: sel.CalendarID, Valid: true},
|
||||
CalendarName: sql.NullString{String: sel.CalendarName, Valid: true},
|
||||
UpdatedAt: now,
|
||||
}); err != nil {
|
||||
return fmt.Errorf("gcal: save calendar: %w", err)
|
||||
}
|
||||
if row.CalendarID.Valid && row.CalendarID.String == sel.CalendarID {
|
||||
return nil
|
||||
}
|
||||
if err := q.MarkCalendarPlansEdited(ctx, now); err != nil {
|
||||
return fmt.Errorf("gcal: mark plans pushed to the earlier calendar edited: %w", err)
|
||||
}
|
||||
if err := q.ClearCalendarEvents(ctx, now); err != nil {
|
||||
return fmt.Errorf("gcal: forget events on the earlier calendar: %w", err)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
// calendars lists the writable calendars, each once, with the stored refresh
|
||||
// token. The caller holds s.account.
|
||||
func (s *Service) calendars(ctx context.Context) ([]Calendar, error) {
|
||||
|
||||
@@ -128,8 +128,8 @@ type Service struct {
|
||||
// account serializes every use of and change to the connected account —
|
||||
// a connect's exchange and store, a disconnect's read, revoke and delete,
|
||||
// a calendar listing's read and Calendar API calls, a calendar choice's
|
||||
// listing and store, an event push's read and event writes — across the
|
||||
// requests to Google and the database write. Google revokes the whole grant, so a revoke overlapping a
|
||||
// listing and store, an event push's read, event writes and record —
|
||||
// across the requests to Google and the database write. Google revokes the whole grant, so a revoke overlapping a
|
||||
// connect would also kill the token the connect stores.
|
||||
account sync.Mutex
|
||||
|
||||
|
||||
@@ -85,7 +85,12 @@ type eventTime struct {
|
||||
// ErrNoCalendar, a chosen calendar that was deleted or is no longer
|
||||
// writable is ErrCalendarUnavailable, a grant Google no longer accepts is
|
||||
// ErrReconnect, and any other Google failure is a *CalendarError.
|
||||
func (s *Service) PushEvents(ctx context.Context, events []Event) error {
|
||||
//
|
||||
// Once every event is written, PushEvents returns record(), which the
|
||||
// caller uses to store what it pushed. record runs under the same hold of
|
||||
// the account as the writes, so the chosen calendar cannot change between
|
||||
// them; it is not run when a write fails.
|
||||
func (s *Service) PushEvents(ctx context.Context, events []Event, record func() error) error {
|
||||
if len(events) == 0 {
|
||||
return errors.New("gcal: no events to push")
|
||||
}
|
||||
@@ -113,7 +118,7 @@ func (s *Service) PushEvents(ctx context.Context, events []Event) error {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return record()
|
||||
}
|
||||
|
||||
// writeEvent inserts e at base, the chosen calendar's events collection,
|
||||
|
||||
@@ -22,6 +22,9 @@ func chosen(t *testing.T) *harness {
|
||||
return h
|
||||
}
|
||||
|
||||
// noRecord is a PushEvents record that stores nothing.
|
||||
func noRecord() error { return nil }
|
||||
|
||||
// event is a valid event with id.
|
||||
func event(id string) gcal.Event {
|
||||
start := time.Date(2026, 10, 12, 21, 30, 0, 0, time.UTC)
|
||||
@@ -32,12 +35,12 @@ func event(id string) gcal.Event {
|
||||
func TestPushEventsInsertsThenUpdates(t *testing.T) {
|
||||
t.Parallel()
|
||||
h := chosen(t)
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), event("abcde1")}); err != nil {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), event("abcde1")}, noRecord); err != nil {
|
||||
t.Fatalf("PushEvents: %v", err)
|
||||
}
|
||||
renamed := event("abcde0")
|
||||
renamed.Summary = "Dinner: Pot Roast"
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{renamed}); err != nil {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{renamed}, noRecord); err != nil {
|
||||
t.Fatalf("PushEvents again: %v", err)
|
||||
}
|
||||
if inserts, updates := h.google.EventWrites(); inserts != 2 || updates != 1 {
|
||||
@@ -67,14 +70,14 @@ func TestPushEventsRefusesInvalidEvents(t *testing.T) {
|
||||
} {
|
||||
ev := event("abcde")
|
||||
edit(&ev)
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), ev}); err == nil {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde0"), ev}, noRecord); err == nil {
|
||||
t.Errorf("%s: PushEvents succeeded", name)
|
||||
}
|
||||
}
|
||||
if err := h.svc.PushEvents(context.Background(), nil); err == nil {
|
||||
if err := h.svc.PushEvents(context.Background(), nil, noRecord); err == nil {
|
||||
t.Error("PushEvents of no events succeeded")
|
||||
}
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event(strings.Repeat("v", 1024))}); err != nil {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event(strings.Repeat("v", 1024))}, noRecord); err != nil {
|
||||
t.Errorf("PushEvents of a 1024-character id: %v", err)
|
||||
}
|
||||
if inserts, _ := h.google.EventWrites(); inserts != 1 {
|
||||
@@ -89,7 +92,10 @@ func TestPushEventsGrantNoLongerAccepted(t *testing.T) {
|
||||
t.Parallel()
|
||||
h := chosen(t)
|
||||
h.google.FailEventWrite(1, http.StatusUnauthorized)
|
||||
err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")})
|
||||
err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, func() error {
|
||||
t.Error("record ran after a failed write")
|
||||
return nil
|
||||
})
|
||||
if !errors.Is(err, gcal.ErrReconnect) {
|
||||
t.Fatalf("PushEvents = %v, want ErrReconnect", err)
|
||||
}
|
||||
@@ -106,13 +112,13 @@ func TestPushEventsGoogleFailureIsRedacted(t *testing.T) {
|
||||
t.Parallel()
|
||||
h := chosen(t)
|
||||
if name == "update" {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}); err != nil {
|
||||
if err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
// The fake's failure body quotes the access token.
|
||||
h.google.FailEventWrite(n, http.StatusInternalServerError)
|
||||
err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")})
|
||||
err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord)
|
||||
var calendar *gcal.CalendarError
|
||||
if !errors.As(err, &calendar) {
|
||||
t.Fatalf("PushEvents = %v, want *CalendarError", err)
|
||||
@@ -134,7 +140,7 @@ func TestPushEventsHoldsTheAccount(t *testing.T) {
|
||||
arrived, release := h.google.HoldEvents()
|
||||
defer release()
|
||||
pushed := make(chan error, 1)
|
||||
go func() { pushed <- h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}) }()
|
||||
go func() { pushed <- h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, noRecord) }()
|
||||
<-arrived
|
||||
disconnected := make(chan error, 1)
|
||||
go func() { disconnected <- h.svc.Disconnect(context.Background()) }()
|
||||
@@ -154,3 +160,30 @@ func TestPushEventsHoldsTheAccount(t *testing.T) {
|
||||
t.Errorf("Disconnect: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPushEventsRecordsWhileTheCalendarIsChosen chooses another calendar
|
||||
// while a push records: the choice waits for the record, so a push never
|
||||
// records events on a calendar no longer chosen.
|
||||
func TestPushEventsRecordsWhileTheCalendarIsChosen(t *testing.T) {
|
||||
t.Parallel()
|
||||
h := chosen(t)
|
||||
selected := make(chan error, 1)
|
||||
err := h.svc.PushEvents(context.Background(), []gcal.Event{event("abcde")}, func() error {
|
||||
go func() {
|
||||
_, err := h.svc.SelectCalendar(context.Background(), family.ID)
|
||||
selected <- err
|
||||
}()
|
||||
select {
|
||||
case err := <-selected:
|
||||
t.Errorf("SelectCalendar finished (%v) while a push was recording", err)
|
||||
case <-time.After(200 * time.Millisecond):
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("PushEvents: %v", err)
|
||||
}
|
||||
if err := <-selected; err != nil {
|
||||
t.Errorf("SelectCalendar: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user