Merge pull request #67 from HannesHil/fix/deleted-source-event-bug

Fixed Missing Source Event bug

internal/adapter/google/adapter.go

@@ -24,7 +24,7 @@ type GoogleCalendarClient interface {
 	CreateEvent(ctx context.Context, event models.Event) error
 	UpdateEvent(ctx context.Context, event models.Event) error
 	DeleteEvent(ctx context.Context, event models.Event) error
-	GetSourceID() string
+	GetCalendarID() string
 	InitGoogleCalendarClient(calId string, log *log.Logger) error
 }
 
@@ -219,12 +219,12 @@ func (c *CalendarAPI) Name() string {
 	return "Google Calendar"
 }
 
-// GetSourceID calculates a unique SourceID for this adapter based on the current calendar.
+// GetCalendarID calculates a unique ID for this adapter based on the current calendar.
 // This is used to distinguish between adapters in order to not overwrite or delete events
 // which are maintained by different adapters.
 // A simple use-case for this is if you have multiple google calendars as source adapters configured.
-func (c *CalendarAPI) GetSourceID() string {
-	return c.gcalClient.GetSourceID()
+func (c *CalendarAPI) GetCalendarID() string {
+	return c.gcalClient.GetCalendarID()
 }
 
 func (c *CalendarAPI) SetLogger(logger *log.Logger) {

internal/adapter/google/client.go

@@ -67,7 +67,7 @@ func (g *GCalClient) ListEvents(ctx context.Context, starttime time.Time, endtim
 
 	var loadedEvents []models.Event
 	for _, event := range eventList.Items {
-		loadedEvents = append(loadedEvents, calendarEventToEvent(event, g.GetSourceID()))
+		loadedEvents = append(loadedEvents, calendarEventToEvent(event, g.GetCalendarID()))
 	}
 
 	// if the responses 'nextPageToken' is set, the result is paginated and more data to be loaded recursively
@@ -77,7 +77,7 @@ func (g *GCalClient) ListEvents(ctx context.Context, starttime time.Time, endtim
 			return nil, err
 		}
 		for _, pageEvent := range eventList.Items {
-			loadedEvents = append(loadedEvents, calendarEventToEvent(pageEvent, g.GetSourceID()))
+			loadedEvents = append(loadedEvents, calendarEventToEvent(pageEvent, g.GetCalendarID()))
 		}
 	}
 	return loadedEvents, nil
@@ -214,15 +214,11 @@ func (g *GCalClient) loadPages(listCall *calendar.EventsListCall, events *[]*cal
 	return g.loadPages(listCall, events, pageEvents.NextPageToken)
 }
 
-// TODO
-// it is not ideal to have the GetSourceID func here as well, it is not used in the CalendarAPI Client anymore but we still need it to satisfy the "Source" interface
-// defined in sync/sychronisation.go
-
-// GetSourceID calculates a unique SourceID for this adapter based on the current calendar.
+// GetCalendarID calculates a unique ID for this adapter based on the current calendar.
 // This is used to distinguish between adapters in order to not overwrite or delete events
 // which are maintained by different adapters.
 // A simple use-case for this is if you have multiple google calendars as source adapters configured.
-func (g *GCalClient) GetSourceID() string {
+func (g *GCalClient) GetCalendarID() string {
 	var id []byte
 
 	sum := sha1.Sum([]byte(g.CalendarId))

internal/adapter/outlook_http/adapter.go

@@ -25,7 +25,7 @@ type OutlookCalendarClient interface {
 	CreateEvent(ctx context.Context, event models.Event) error
 	UpdateEvent(ctx context.Context, event models.Event) error
 	DeleteEvent(ctx context.Context, event models.Event) error
-	GetSourceID() string
+	GetCalendarID() string
 }
 
 type CalendarAPI struct {
@@ -212,8 +212,8 @@ func (c *CalendarAPI) DeleteEvent(ctx context.Context, e models.Event) error {
 	return nil
 }
 
-func (c *CalendarAPI) GetSourceID() string {
-	return c.outlookClient.GetSourceID()
+func (c *CalendarAPI) GetCalendarID() string {
+	return c.outlookClient.GetCalendarID()
 }
 
 func (c *CalendarAPI) Name() string {

internal/adapter/outlook_http/client.go

@@ -76,7 +76,7 @@ func (o *OutlookClient) ListEvents(ctx context.Context, start time.Time, end tim
 
 	var events []models.Event
 	for _, evt := range eventList.Events {
-		evt, err := o.outlookEventToEvent(evt, o.GetSourceID())
+		evt, err := o.outlookEventToEvent(evt, o.GetCalendarID())
 		if err != nil {
 			return nil, err
 		}
@@ -168,7 +168,7 @@ func (o *OutlookClient) DeleteEvent(ctx context.Context, event models.Event) err
 	return nil
 }
 
-func (o OutlookClient) GetSourceID() string {
+func (o OutlookClient) GetCalendarID() string {
 	var id []byte
 	sum := sha1.Sum([]byte(o.CalendarID))
 	id = append(id, sum[:]...)

internal/adapter/sink_adapter.go

@@ -102,3 +102,7 @@ func (a SinkAdapter) DeleteEvent(ctx context.Context, e models.Event) error {
 func (a SinkAdapter) EventsInTimeframe(ctx context.Context, start time.Time, end time.Time) ([]models.Event, error) {
 	return a.client.EventsInTimeframe(ctx, start, end)
 }
+
+func (a SinkAdapter) GetCalendarID() string {
+	return a.client.GetCalendarID()
+}

internal/adapter/source_adapter.go

@@ -90,8 +90,8 @@ func (a SourceAdapter) Name() string {
 func (a SourceAdapter) CalendarID() string {
 	return a.calendarID
 }
-func (a SourceAdapter) GetSourceID() string {
-	return a.client.GetSourceID()
+func (a SourceAdapter) GetCalendarID() string {
+	return a.client.GetCalendarID()
 }
 
 func (a SourceAdapter) EventsInTimeframe(ctx context.Context, start time.Time, end time.Time) ([]models.Event, error) {

internal/adapter/zep/client.go

@@ -36,11 +36,11 @@ type CalendarAPI struct {
 	homeSet   string
 }
 
-func (zep *CalendarAPI) GetSourceID() string {
-	return zep.generateSourceID()
+func (zep *CalendarAPI) GetCalendarID() string {
+	return zep.generateCalendarID()
 }
 
-func (zep *CalendarAPI) generateSourceID() string {
+func (zep *CalendarAPI) generateCalendarID() string {
 	var id []byte
 	components := []string{zep.username, zep.homeSet, zep.calendarID}
 
@@ -104,7 +104,7 @@ func (zep *CalendarAPI) EventsInTimeframe(ctx context.Context, start time.Time,
 				Description: v.Description,
 				StartTime:   v.Start,
 				EndTime:     v.End,
-				Metadata:    models.NewEventMetadata(v.ID, "", zep.GetSourceID()),
+				Metadata:    models.NewEventMetadata(v.ID, "", zep.GetCalendarID()),
 			})
 	}
 

internal/sync/controller.go

@@ -159,7 +159,7 @@ func (p Controller) CleanUp(ctx context.Context, start time.Time, end time.Time)
 	for _, event := range sink {
 		// Check if the sink event was synced by us, if there's no metadata the event may
 		// be there because we were invited or because it is not managed by us
-		if event.Metadata.SourceID == p.source.GetSourceID() {
+		if event.Metadata.SourceID == p.source.GetCalendarID() {
 			// redefine to let the closure capture individual variables
 			event := event
 			tasks = append(tasks, func() error {
@@ -193,14 +193,26 @@ func (p Controller) diffEvents(sourceEvents []models.Event, sinkEvents []models.
 
 		switch {
 		case !exists:
+			// Don't sync synced events back to their original calendar to prevent resurrecting
+			// deleted events.
+			// Problem:
+			// - Sync event (from calendar A) to create eventCopy (calendar B, SourceID = calendar A).
+			// - Delete event (in calendar A)
+			// - Run sync from calendar B to calendar A. This will copy (and thereby resurrect) the event.
+			//
+			// Solution: Ignore events the originate from the sink, but no longer exist there.
+			if event.Metadata.SourceID == p.sink.GetCalendarID() {
+				p.logger.Info("skipping event as it originates from the sink, but no longer exists there", logFields(event)...)
+				continue
+			}
 			p.logger.Info("new event, needs sync", logFields(event)...)
 			createEvents = append(createEvents, event)
 
-		case sinkEvent.Metadata.SourceID != p.source.GetSourceID():
+		case sinkEvent.Metadata.SourceID != p.source.GetCalendarID():
 			p.logger.Info("event was not synced by this source adapter, skipping", logFields(event)...)
 
 			// Only update the event if the event differs AND we synced it prior and set the correct metadata
-		case !models.IsSameEvent(event, sinkEvent) && sinkEvent.Metadata.SourceID == p.source.GetSourceID():
+		case !models.IsSameEvent(event, sinkEvent) && sinkEvent.Metadata.SourceID == p.source.GetCalendarID():
 			p.logger.Info("event content changed, needs sync", logFields(event)...)
 			updateEvents = append(updateEvents, sinkEvent.Overwrite(event))
 
@@ -219,7 +231,7 @@ func (p Controller) diffEvents(sourceEvents []models.Event, sinkEvents []models.
 		case exists:
 			// Nothing to do
 
-		case event.Metadata.SourceID == p.source.GetSourceID():
+		case event.Metadata.SourceID == p.source.GetCalendarID():
 			p.logger.Info("sinkEvent is not (anymore) in sourceEvents, marked for removal", logFields(event)...)
 			deleteEvents = append(deleteEvents, event)
 

internal/sync/controller_test.go

@@ -99,7 +99,8 @@ func (suite *ControllerTestSuite) TestDryRun() {
 	suite.source.On("EventsInTimeframe", ctx, startTime, endTime).Return(sourceEvents, nil)
 	suite.sink.On("EventsInTimeframe", ctx, startTime, endTime).Return(sinkEvents, nil)
 	suite.sink.On("DeleteEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
-	suite.source.On("GetSourceID").Return("sourceID")
+	suite.sink.On("GetCalendarID").Return("sinkID")
+	suite.source.On("GetCalendarID").Return("sourceID")
 
 	err := suite.controller.SynchroniseTimeframe(ctx, startTime, endTime, true)
 	assert.NoError(suite.T(), err)
@@ -175,7 +176,8 @@ func (suite *ControllerTestSuite) TestCleanUp() {
 	suite.source.On("EventsInTimeframe", ctx, startTime, endTime).Return(sourceEvents, nil)
 	suite.sink.On("EventsInTimeframe", ctx, startTime, endTime).Return(sinkEvents, nil)
 	suite.sink.On("DeleteEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
-	suite.source.On("GetSourceID").Return("sourceID")
+	suite.sink.On("GetCalendarID").Return("sinkID")
+	suite.source.On("GetCalendarID").Return("sourceID")
 
 	err := suite.controller.CleanUp(ctx, startTime, endTime)
 	assert.NoError(suite.T(), err)
@@ -220,6 +222,7 @@ func (suite *ControllerTestSuite) TestCreateEventsEmptySink() {
 	suite.source.On("EventsInTimeframe", ctx, startTime, endTime).Return(eventsToCreate, nil)
 	suite.sink.On("EventsInTimeframe", ctx, startTime, endTime).Return(nil, nil)
 	suite.sink.On("CreateEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
+	suite.sink.On("GetCalendarID").Return("sinkID")
 
 	err := suite.controller.SynchroniseTimeframe(ctx, startTime, endTime, false)
 	assert.NoError(suite.T(), err)
@@ -231,7 +234,7 @@ func (suite *ControllerTestSuite) TestCreateEventsEmptySink() {
 	suite.sink.AssertNotCalled(suite.T(), "DeleteEvent", ctx, mock.AnythingOfType("models.Event"))
 }
 
-// TestDeleteEventsNotInSink verifies that if events are present in the source-adapter, but not in the sink, these
+// TestDeleteEventsNotInSink verifies that if events are present in the sink-adapter, but not in the source, these
 // events are deleted in the sink.
 func (suite *ControllerTestSuite) TestDeleteEventsNotInSink() {
 	ctx := context.Background()
@@ -304,7 +307,8 @@ func (suite *ControllerTestSuite) TestDeleteEventsNotInSink() {
 	suite.sink.On("DeleteEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
 	// UpdateEvent gets called because the remaining event in the sink will get updated because there are no transformers configured
 	suite.sink.On("UpdateEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
-	suite.source.On("GetSourceID").Return("sourceID")
+	suite.sink.On("GetCalendarID").Return("sinkID")
+	suite.source.On("GetCalendarID").Return("sourceID")
 
 	err := suite.controller.SynchroniseTimeframe(ctx, startTime, endTime, false)
 	assert.NoError(suite.T(), err)
@@ -314,6 +318,43 @@ func (suite *ControllerTestSuite) TestDeleteEventsNotInSink() {
 	suite.sink.AssertNumberOfCalls(suite.T(), "DeleteEvent", expectedDelete)
 }
 
+// TestDoNotResurrectEvents verifies that if events are present in the source-adapter that originated
+// from the sink, but have been deleted there, these events are not copied to the sink.
+// This ensures that for two calendars A and B, with sync A->B and B->A, that an event
+// will not be restored if the original event was deleted.
+func (suite *ControllerTestSuite) TestDoNotResurrectEvents() {
+	ctx := context.Background()
+	startTime := time.Now()
+	endTime := startTime.Add(2 * time.Hour)
+	sourceEvents := []models.Event{
+		{
+			ICalUID:     "testID",
+			ID:          "testUID",
+			Title:       "Title 1",
+			Description: "Description",
+			StartTime:   startTime,
+			EndTime:     endTime,
+			AllDay:      false,
+			// originates from sink
+			Metadata:  models.NewEventMetadata("seed1", "uri", "sinkID"),
+			Reminders: []models.Reminder{{Actions: models.ReminderActionDisplay, Trigger: models.ReminderTrigger{PointInTime: startTime.Add(-10 * time.Minute)}}},
+		},
+	}
+	sinkEvents := []models.Event{}
+
+	suite.source.On("EventsInTimeframe", ctx, startTime, endTime).Return(sourceEvents, nil)
+	suite.sink.On("EventsInTimeframe", ctx, startTime, endTime).Return(sinkEvents, nil)
+	suite.sink.On("GetCalendarID").Return("sinkID")
+	suite.source.On("GetCalendarID").Return("sourceID")
+
+	err := suite.controller.SynchroniseTimeframe(ctx, startTime, endTime, false)
+	assert.NoError(suite.T(), err)
+
+	suite.sink.AssertNotCalled(suite.T(), "CreateEvent", ctx, mock.AnythingOfType("models.Event"))
+	suite.sink.AssertNotCalled(suite.T(), "UpdateEvent", ctx, mock.AnythingOfType("models.Event"))
+	suite.sink.AssertNotCalled(suite.T(), "DeleteEvent", ctx, mock.AnythingOfType("models.Event"))
+}
+
 // TestUpdateEventsPrefilledSink asserts that, given that the sink does contain any events,
 // the controller properly updates the events in the sink.
 // and leaves unmanaged events as they are
@@ -437,7 +478,8 @@ func (suite *ControllerTestSuite) TestUpdateEventsPrefilledSink() {
 	suite.source.On("EventsInTimeframe", ctx, startTime, endTime).Return(sourceEvents, nil)
 	suite.sink.On("EventsInTimeframe", ctx, startTime, endTime).Return(sinkEvents, nil)
 	suite.sink.On("UpdateEvent", ctx, mock.AnythingOfType("models.Event")).Return(nil)
-	suite.source.On("GetSourceID").Return("sourceID")
+	suite.sink.On("GetCalendarID").Return("sinkID")
+	suite.source.On("GetCalendarID").Return("sourceID")
 
 	err := suite.controller.SynchroniseTimeframe(ctx, startTime, endTime, false)
 	assert.NoError(suite.T(), err)
@@ -708,15 +750,32 @@ func TestController_diffEvents(t *testing.T) {
 			expectedUpdateEvents: []models.Event{},
 			expectedDeleteEvents: []models.Event{},
 		},
+		{
+			name: "should not resurrect events already deleted from source when syncing back from sink",
+			source: []models.Event{{
+				Metadata: &models.Metadata{
+					SyncID:   "unicorn",
+					SourceID: "sinkID",
+				},
+				Title: "Foo",
+			}},
+			expectedCreateEvents: []models.Event{},
+			expectedUpdateEvents: []models.Event{},
+			expectedDeleteEvents: []models.Event{},
+		},
 	}
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			var source mocks.Source
-			source.On("GetSourceID").Return("sourceID")
+			source.On("GetCalendarID").Return("sourceID")
+
+			var sink mocks.Sink
+			sink.On("GetCalendarID").Return("sinkID")
 
 			var controller = Controller{
 				source: &source,
+				sink:   &sink,
 				logger: log.Default(),
 			}