properly handle collision replace behavior (#3636)

completes the item collision handling behavior in
exchange by turning replace handling into a post-delete process.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #3562

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/internal/m365/exchange/contacts_restore.go

@@ -79,9 +79,14 @@ func (h contactRestoreHandler) restore(
 		errs)
 }
 
+type contactRestorer interface {
+	postItemer[models.Contactable]
+	deleteItemer
+}
+
 func restoreContact(
 	ctx context.Context,
-	pi postItemer[models.Contactable],
+	cr contactRestorer,
 	body []byte,
 	userID, destinationID string,
 	collisionKeyToItemID map[string]string,
@@ -94,22 +99,40 @@ func restoreContact(
 	}
 
 	ctx = clues.Add(ctx, "item_id", ptr.Val(contact.GetId()))
-	collisionKey := api.ContactCollisionKey(contact)
 
-	if _, ok := collisionKeyToItemID[collisionKey]; ok {
+	var (
+		collisionKey         = api.ContactCollisionKey(contact)
+		collisionID          string
+		shouldDeleteOriginal bool
+	)
+
+	if id, ok := collisionKeyToItemID[collisionKey]; ok {
 		log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey))
 		log.Debug("item collision")
 
-		// TODO(rkeepers): Replace probably shouldn't no-op.  Just a starting point.
-		if collisionPolicy == control.Skip || collisionPolicy == control.Replace {
+		if collisionPolicy == control.Skip {
 			log.Debug("skipping item with collision")
 			return nil, graph.ErrItemAlreadyExistsConflict
 		}
+
+		collisionID = id
+		shouldDeleteOriginal = collisionPolicy == control.Replace
 	}
 
-	item, err := pi.PostItem(ctx, userID, destinationID, contact)
+	item, err := cr.PostItem(ctx, userID, destinationID, contact)
 	if err != nil {
-		return nil, graph.Wrap(ctx, err, "restoring mail message")
+		return nil, graph.Wrap(ctx, err, "restoring contact")
+	}
+
+	// contacts have no PUT request, and PATCH could retain data that's not
+	// associated with the backup item state.  Instead of updating, we
+	// post first, then delete.  In case of failure between the two calls,
+	// at least we'll have accidentally over-produced data instead of deleting
+	// the user's data.
+	if shouldDeleteOriginal {
+		if err := cr.DeleteItem(ctx, userID, collisionID); err != nil {
+			return nil, graph.Wrap(ctx, err, "deleting colliding contact")
+		}
 	}
 
 	info := api.ContactInfo(item)

src/internal/m365/exchange/contacts_restore_test.go

@@ -20,20 +20,32 @@ import (
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
 )
 
-var _ postItemer[models.Contactable] = &mockContactRestorer{}
+var _ contactRestorer = &contactRestoreMock{}
 
-type mockContactRestorer struct {
-	postItemErr error
+type contactRestoreMock struct {
+	postItemErr   error
+	calledPost    bool
+	deleteItemErr error
+	calledDelete  bool
 }
 
-func (m mockContactRestorer) PostItem(
-	ctx context.Context,
-	userID, containerID string,
-	body models.Contactable,
+func (m *contactRestoreMock) PostItem(
+	_ context.Context,
+	_, _ string,
+	_ models.Contactable,
 ) (models.Contactable, error) {
+	m.calledPost = true
 	return models.NewContact(), m.postItemErr
 }
 
+func (m *contactRestoreMock) DeleteItem(
+	_ context.Context,
+	_, _ string,
+) error {
+	m.calledDelete = true
+	return m.deleteItemErr
+}
+
 // ---------------------------------------------------------------------------
 // tests
 // ---------------------------------------------------------------------------
@@ -78,63 +90,88 @@ func (suite *ContactsRestoreIntgSuite) TestRestoreContact() {
 
 	table := []struct {
 		name         string
-		apiMock      postItemer[models.Contactable]
+		apiMock      *contactRestoreMock
 		collisionMap map[string]string
 		onCollision  control.CollisionPolicy
 		expectErr    func(*testing.T, error)
+		expectMock   func(*testing.T, *contactRestoreMock)
 	}{
 		{
 			name:         "no collision: skip",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: copy",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: replace",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: skip",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.False(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: copy",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: replace",
-			apiMock:      mockContactRestorer{},
+			apiMock:      &contactRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
-				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+			expectMock: func(t *testing.T, m *contactRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.True(t, m.calledDelete, "old item deleted")
 			},
 		},
 	}
@@ -156,6 +193,7 @@ func (suite *ContactsRestoreIntgSuite) TestRestoreContact() {
 				fault.New(true))
 
 			test.expectErr(t, err)
+			test.expectMock(t, test.apiMock)
 		})
 	}
 }

src/internal/m365/exchange/events_restore.go

@@ -105,17 +105,24 @@ func restoreEvent(
 	}
 
 	ctx = clues.Add(ctx, "item_id", ptr.Val(event.GetId()))
-	collisionKey := api.EventCollisionKey(event)
 
-	if _, ok := collisionKeyToItemID[collisionKey]; ok {
+	var (
+		collisionKey         = api.EventCollisionKey(event)
+		collisionID          string
+		shouldDeleteOriginal bool
+	)
+
+	if id, ok := collisionKeyToItemID[collisionKey]; ok {
 		log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey))
 		log.Debug("item collision")
 
-		// TODO(rkeepers): Replace probably shouldn't no-op.  Just a starting point.
-		if collisionPolicy == control.Skip || collisionPolicy == control.Replace {
+		if collisionPolicy == control.Skip {
 			log.Debug("skipping item with collision")
 			return nil, graph.ErrItemAlreadyExistsConflict
 		}
+
+		collisionID = id
+		shouldDeleteOriginal = collisionPolicy == control.Replace
 	}
 
 	event = toEventSimplified(event)
@@ -131,7 +138,18 @@ func restoreEvent(
 
 	item, err := er.PostItem(ctx, userID, destinationID, event)
 	if err != nil {
-		return nil, graph.Wrap(ctx, err, "restoring calendar item")
+		return nil, graph.Wrap(ctx, err, "restoring event")
+	}
+
+	// events have no PUT request, and PATCH could retain data that's not
+	// associated with the backup item state.  Instead of updating, we
+	// post first, then delete.  In case of failure between the two calls,
+	// at least we'll have accidentally over-produced data instead of deleting
+	// the user's data.
+	if shouldDeleteOriginal {
+		if err := er.DeleteItem(ctx, userID, collisionID); err != nil {
+			return nil, graph.Wrap(ctx, err, "deleting colliding event")
+		}
 	}
 
 	err = uploadAttachments(

src/internal/m365/exchange/events_restore_test.go

@@ -21,22 +21,34 @@ import (
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
 )
 
-var _ eventRestorer = &mockEventRestorer{}
+var _ eventRestorer = &eventRestoreMock{}
 
-type mockEventRestorer struct {
+type eventRestoreMock struct {
 	postItemErr       error
+	calledPost        bool
+	deleteItemErr     error
+	calledDelete      bool
 	postAttachmentErr error
 }
 
-func (m mockEventRestorer) PostItem(
-	ctx context.Context,
-	userID, containerID string,
-	body models.Eventable,
+func (m *eventRestoreMock) PostItem(
+	_ context.Context,
+	_, _ string,
+	_ models.Eventable,
 ) (models.Eventable, error) {
+	m.calledPost = true
 	return models.NewEvent(), m.postItemErr
 }
 
-func (m mockEventRestorer) PostSmallAttachment(
+func (m *eventRestoreMock) DeleteItem(
+	_ context.Context,
+	_, _ string,
+) error {
+	m.calledDelete = true
+	return m.deleteItemErr
+}
+
+func (m *eventRestoreMock) PostSmallAttachment(
 	_ context.Context,
 	_, _, _ string,
 	_ models.Attachmentable,
@@ -44,7 +56,7 @@ func (m mockEventRestorer) PostSmallAttachment(
 	return m.postAttachmentErr
 }
 
-func (m mockEventRestorer) PostLargeAttachment(
+func (m *eventRestoreMock) PostLargeAttachment(
 	_ context.Context,
 	_, _, _, _ string,
 	_ []byte,
@@ -52,21 +64,14 @@ func (m mockEventRestorer) PostLargeAttachment(
 	return uuid.NewString(), m.postAttachmentErr
 }
 
-func (m mockEventRestorer) DeleteAttachment(
+func (m *eventRestoreMock) DeleteAttachment(
 	ctx context.Context,
 	userID, calendarID, eventID, attachmentID string,
 ) error {
 	return nil
 }
 
-func (m mockEventRestorer) DeleteItem(
-	ctx context.Context,
-	userID, itemID string,
-) error {
-	return nil
-}
-
-func (m mockEventRestorer) GetAttachments(
+func (m *eventRestoreMock) GetAttachments(
 	_ context.Context,
 	_ bool,
 	_, _ string,
@@ -74,14 +79,14 @@ func (m mockEventRestorer) GetAttachments(
 	return []models.Attachmentable{}, nil
 }
 
-func (m mockEventRestorer) GetItemInstances(
+func (m *eventRestoreMock) GetItemInstances(
 	_ context.Context,
 	_, _, _, _ string,
 ) ([]models.Eventable, error) {
 	return []models.Eventable{}, nil
 }
 
-func (m mockEventRestorer) PatchItem(
+func (m *eventRestoreMock) PatchItem(
 	_ context.Context,
 	_, _ string,
 	_ models.Eventable,
@@ -133,63 +138,88 @@ func (suite *EventsRestoreIntgSuite) TestRestoreEvent() {
 
 	table := []struct {
 		name         string
-		apiMock      eventRestorer
+		apiMock      *eventRestoreMock
 		collisionMap map[string]string
 		onCollision  control.CollisionPolicy
 		expectErr    func(*testing.T, error)
+		expectMock   func(*testing.T, *eventRestoreMock)
 	}{
 		{
 			name:         "no collision: skip",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: copy",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: replace",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: skip",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.False(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: copy",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: replace",
-			apiMock:      mockEventRestorer{},
+			apiMock:      &eventRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
-				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+			expectMock: func(t *testing.T, m *eventRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.True(t, m.calledDelete, "old item deleted")
 			},
 		},
 	}
@@ -211,6 +241,7 @@ func (suite *EventsRestoreIntgSuite) TestRestoreEvent() {
 				fault.New(true))
 
 			test.expectErr(t, err)
+			test.expectMock(t, test.apiMock)
 		})
 	}
 }

src/internal/m365/exchange/handlers.go

@@ -138,6 +138,10 @@ type getItemsByCollisionKeyser interface {
 	) (map[string]string, error)
 }
 
+// ---------------------------------------------------------------------------
+// other interfaces
+// ---------------------------------------------------------------------------
+
 type postItemer[T any] interface {
 	PostItem(
 		ctx context.Context,
@@ -145,3 +149,10 @@ type postItemer[T any] interface {
 		body T,
 	) (T, error)
 }
+
+type deleteItemer interface {
+	DeleteItem(
+		ctx context.Context,
+		userID, itemID string,
+	) error
+}

src/internal/m365/exchange/mail_restore.go

@@ -86,6 +86,7 @@ func (h mailRestoreHandler) restore(
 
 type mailRestorer interface {
 	postItemer[models.Messageable]
+	deleteItemer
 	attachmentPoster
 }
 
@@ -104,17 +105,24 @@ func restoreMail(
 	}
 
 	ctx = clues.Add(ctx, "item_id", ptr.Val(msg.GetId()))
-	collisionKey := api.MailCollisionKey(msg)
 
-	if _, ok := collisionKeyToItemID[collisionKey]; ok {
+	var (
+		collisionKey         = api.MailCollisionKey(msg)
+		collisionID          string
+		shouldDeleteOriginal bool
+	)
+
+	if id, ok := collisionKeyToItemID[collisionKey]; ok {
 		log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey))
 		log.Debug("item collision")
 
-		// TODO(rkeepers): Replace probably shouldn't no-op.  Just a starting point.
-		if collisionPolicy == control.Skip || collisionPolicy == control.Replace {
+		if collisionPolicy == control.Skip {
 			log.Debug("skipping item with collision")
 			return nil, graph.ErrItemAlreadyExistsConflict
 		}
+
+		collisionID = id
+		shouldDeleteOriginal = collisionPolicy == control.Replace
 	}
 
 	msg = setMessageSVEPs(toMessage(msg))
@@ -128,6 +136,17 @@ func restoreMail(
 		return nil, graph.Wrap(ctx, err, "restoring mail message")
 	}
 
+	// mails have no PUT request, and PATCH could retain data that's not
+	// associated with the backup item state.  Instead of updating, we
+	// post first, then delete.  In case of failure between the two calls,
+	// at least we'll have accidentally over-produced data instead of deleting
+	// the user's data.
+	if shouldDeleteOriginal {
+		if err := mr.DeleteItem(ctx, userID, collisionID); err != nil {
+			return nil, graph.Wrap(ctx, err, "deleting colliding mail message")
+		}
+	}
+
 	err = uploadAttachments(
 		ctx,
 		mr,

src/internal/m365/exchange/mail_restore_test.go

@@ -21,22 +21,34 @@ import (
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
 )
 
-var _ mailRestorer = &mockMailRestorer{}
+var _ mailRestorer = &mailRestoreMock{}
 
-type mockMailRestorer struct {
+type mailRestoreMock struct {
 	postItemErr       error
+	calledPost        bool
+	deleteItemErr     error
+	calledDelete      bool
 	postAttachmentErr error
 }
 
-func (m mockMailRestorer) PostItem(
-	ctx context.Context,
-	userID, containerID string,
-	body models.Messageable,
+func (m *mailRestoreMock) PostItem(
+	_ context.Context,
+	_, _ string,
+	_ models.Messageable,
 ) (models.Messageable, error) {
+	m.calledPost = true
 	return models.NewMessage(), m.postItemErr
 }
 
-func (m mockMailRestorer) PostSmallAttachment(
+func (m *mailRestoreMock) DeleteItem(
+	_ context.Context,
+	_, _ string,
+) error {
+	m.calledDelete = true
+	return m.deleteItemErr
+}
+
+func (m *mailRestoreMock) PostSmallAttachment(
 	_ context.Context,
 	_, _, _ string,
 	_ models.Attachmentable,
@@ -44,7 +56,7 @@ func (m mockMailRestorer) PostSmallAttachment(
 	return m.postAttachmentErr
 }
 
-func (m mockMailRestorer) PostLargeAttachment(
+func (m *mailRestoreMock) PostLargeAttachment(
 	_ context.Context,
 	_, _, _, _ string,
 	_ []byte,
@@ -95,63 +107,88 @@ func (suite *MailRestoreIntgSuite) TestRestoreMail() {
 
 	table := []struct {
 		name         string
-		apiMock      mailRestorer
+		apiMock      *mailRestoreMock
 		collisionMap map[string]string
 		onCollision  control.CollisionPolicy
 		expectErr    func(*testing.T, error)
+		expectMock   func(*testing.T, *mailRestoreMock)
 	}{
 		{
 			name:         "no collision: skip",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: copy",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "no collision: replace",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: skip",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Skip,
 			expectErr: func(t *testing.T, err error) {
 				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.False(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: copy",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Copy,
 			expectErr: func(t *testing.T, err error) {
 				assert.NoError(t, err, clues.ToCore(err))
 			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.False(t, m.calledDelete, "old item deleted")
+			},
 		},
 		{
 			name:         "collision: replace",
-			apiMock:      mockMailRestorer{},
+			apiMock:      &mailRestoreMock{},
 			collisionMap: map[string]string{collisionKey: "smarf"},
 			onCollision:  control.Replace,
 			expectErr: func(t *testing.T, err error) {
-				assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err))
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+			expectMock: func(t *testing.T, m *mailRestoreMock) {
+				assert.True(t, m.calledPost, "new item posted")
+				assert.True(t, m.calledDelete, "old item deleted")
 			},
 		},
 	}
@@ -173,6 +210,7 @@ func (suite *MailRestoreIntgSuite) TestRestoreMail() {
 				fault.New(true))
 
 			test.expectErr(t, err)
+			test.expectMock(t, test.apiMock)
 		})
 	}
 }