Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

use delta queries for calendar events #2154

Merged
merged 4 commits into from
Jan 19, 2023
Merged

use delta queries for calendar events #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c94ffdb
use delta queries for calendar events
ryanfkeepers Jan 17, 2023
2f973c4
comment out events test
ryanfkeepers Jan 18, 2023
00047de
Merge branch 'main' into issue-2022-poc
aviator-app[bot] Jan 18, 2023
60c45a3
correct test expectations for restore quant
ryanfkeepers Jan 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/internal/connector/data_collections_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection
getSelector func(t *testing.T) selectors.Selector
}{
{
name: suite.user + " Email",
name: "Email",
getSelector: func(t *testing.T) selectors.Selector {
sel := selectors.NewExchangeBackup(selUsers)
sel.Include(sel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch()))
Expand All @@ -77,7 +77,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection
},
},
{
name: suite.user + " Contacts",
name: "Contacts",
getSelector: func(t *testing.T) selectors.Selector {
sel := selectors.NewExchangeBackup(selUsers)
sel.Include(sel.ContactFolders([]string{exchange.DefaultContactFolder}, selectors.PrefixMatch()))
Expand All @@ -86,7 +86,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection
},
},
// {
// name: suite.user + " Events",
// name: "Events",
// getSelector: func(t *testing.T) selectors.Selector {
// sel := selectors.NewExchangeBackup(selUsers)
// sel.Include(sel.EventCalendars([]string{exchange.DefaultCalendar}, selectors.PrefixMatch()))
Expand Down
61 changes: 44 additions & 17 deletions src/internal/connector/exchange/api/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,29 +144,25 @@ func (c Events) EnumerateContainers(
// item pager
// ---------------------------------------------------------------------------

type eventWrapper struct {
models.EventCollectionResponseable
}

func (ew eventWrapper) GetOdataDeltaLink() *string {
return nil
}

var _ itemPager = &eventPager{}

const (
eventBetaDeltaURLTemplate = "https://graph.microsoft.com/beta/users/%s/calendars/%s/events/delta"
)

type eventPager struct {
gs graph.Servicer
builder *users.ItemCalendarsItemEventsRequestBuilder
options *users.ItemCalendarsItemEventsRequestBuilderGetRequestConfiguration
builder *users.ItemCalendarsItemEventsDeltaRequestBuilder
options *users.ItemCalendarsItemEventsDeltaRequestBuilderGetRequestConfiguration
}

func (p *eventPager) getPage(ctx context.Context) (pageLinker, error) {
resp, err := p.builder.Get(ctx, p.options)
return eventWrapper{resp}, err
return resp, err
}

func (p *eventPager) setNext(nextLink string) {
p.builder = users.NewItemCalendarsItemEventsRequestBuilder(nextLink, p.gs.Adapter())
p.builder = users.NewItemCalendarsItemEventsDeltaRequestBuilder(nextLink, p.gs.Adapter())
}

func (p *eventPager) valuesIn(pl pageLinker) ([]getIDAndAddtler, error) {
Expand All @@ -182,23 +178,54 @@ func (c Events) GetAddedAndRemovedItemIDs(
return nil, nil, DeltaUpdate{}, err
}

var errs *multierror.Error
var (
resetDelta bool
errs *multierror.Error
)

options, err := optionsForEventsByCalendar([]string{"id"})
options, err := optionsForEventsByCalendarDelta([]string{"id"})
if err != nil {
return nil, nil, DeltaUpdate{}, err
}

builder := service.Client().UsersById(user).CalendarsById(calendarID).Events()
if len(oldDelta) > 0 {
builder := users.NewItemCalendarsItemEventsDeltaRequestBuilder(oldDelta, service.Adapter())
pgr := &eventPager{service, builder, options}

added, removed, deltaURL, err := getItemsAddedAndRemovedFromContainer(ctx, pgr)
// note: happy path, not the error condition
if err == nil {
return added, removed, DeltaUpdate{deltaURL, false}, errs.ErrorOrNil()
}
// only return on error if it is NOT a delta issue.
// on bad deltas we retry the call with the regular builder
if graph.IsErrInvalidDelta(err) == nil {
return nil, nil, DeltaUpdate{}, err
}

resetDelta = true
errs = nil
}

// Graph SDK only supports delta queries against events on the beta version, so we're
// manufacturing use of the beta version url to make the call instead.
// See: https://learn.microsoft.com/ko-kr/graph/api/event-delta?view=graph-rest-beta&tabs=http
// Note that the delta item body is skeletal compared to the actual event struct. Lucky
// for us, we only need the item ID. As a result, even though we hacked the version, the
Comment on lines +213 to +214
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you check that items don't get skipped accidentally due to us selecting the bare minimum fields? Tbh, I'm not sure if this is a feature or a "bug" in how Graph works, but using $select can sometimes cause a delta endpoint to skip returning results if they don't match what's in the $select. For example, not having isRead in the select for emails will cause emails where the isRead flag has been changed to be skipped because it doesn't update the mod time. This will take some playing around and is probably easiest to do with GraphExplorer/postman

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good call out. I'll do some manual testing. This might be especially interesting in this case, because the delta body for an event are only able to contain the id, startTime, and endTime. Hopefully the delta will catch changes on unrepresented properties, like renames and etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, two findings.

  1. everything works. Changes to name, desc, attendees, etc, are all picked up by the delta.
  2. we're not allowed to select values in the first place (see the error below). If anything, we're lucky that the client is tossing the use of options for us.
    "error": {
        "code": "ErrorInvalidUrlQuery",
        "message": "The following parameters are not supported with change tracking over the 'SyncEvents' resource: '$orderby, $filter, $select, $expand, $search'."
    }

// response body parses properly into the v1.0 structs and complies with our wanted interfaces.
// Likewise, the NextLink and DeltaLink odata tags carry our hack forward, so the rest of the code
// works as intended (until, at least, we want to _not_ call the beta anymore).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

rawURL := fmt.Sprintf(eventBetaDeltaURLTemplate, user, calendarID)
builder := users.NewItemCalendarsItemEventsDeltaRequestBuilder(rawURL, service.Adapter())
pgr := &eventPager{service, builder, options}

added, _, _, err := getItemsAddedAndRemovedFromContainer(ctx, pgr)
added, removed, deltaURL, err := getItemsAddedAndRemovedFromContainer(ctx, pgr)
if err != nil {
return nil, nil, DeltaUpdate{}, err
}

// Events don't have a delta endpoint so just return an empty string.
return added, nil, DeltaUpdate{}, errs.ErrorOrNil()
return added, removed, DeltaUpdate{deltaURL, resetDelta}, errs.ErrorOrNil()
}

// ---------------------------------------------------------------------------
Expand Down
8 changes: 4 additions & 4 deletions src/internal/connector/exchange/api/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,19 +214,19 @@ func optionsForContactFoldersItemDelta(
}

// optionsForEvents ensures a valid option inputs for `exchange.Events` when selected from within a Calendar
func optionsForEventsByCalendar(
func optionsForEventsByCalendarDelta(
moreOps []string,
) (*users.ItemCalendarsItemEventsRequestBuilderGetRequestConfiguration, error) {
) (*users.ItemCalendarsItemEventsDeltaRequestBuilderGetRequestConfiguration, error) {
selecting, err := buildOptions(moreOps, fieldsForEvents)
if err != nil {
return nil, err
}

requestParameters := &users.ItemCalendarsItemEventsRequestBuilderGetQueryParameters{
requestParameters := &users.ItemCalendarsItemEventsDeltaRequestBuilderGetQueryParameters{
Select: selecting,
}

options := &users.ItemCalendarsItemEventsRequestBuilderGetRequestConfiguration{
options := &users.ItemCalendarsItemEventsDeltaRequestBuilderGetRequestConfiguration{
QueryParameters: requestParameters,
}

Expand Down
7 changes: 7 additions & 0 deletions src/internal/connector/exchange/data_collections_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,13 @@ func (suite *DataCollectionsIntegrationSuite) TestDelta() {
selectors.PrefixMatch(),
)[0],
},
{
name: "Events",
scope: selectors.NewExchangeBackup(users).EventCalendars(
[]string{DefaultCalendar},
selectors.PrefixMatch(),
)[0],
},
}
for _, test := range tests {
suite.T().Run(test.name, func(t *testing.T) {
Expand Down
70 changes: 60 additions & 10 deletions src/internal/operations/backup_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"time"

"github.com/google/uuid"
msuser "github.com/microsoftgraph/msgraph-sdk-go/users"
"github.com/microsoftgraph/msgraph-sdk-go/users"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -507,7 +507,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchange() {
ctx, flush := tester.NewContext()
defer flush()

users := []string{suite.user}
owners := []string{suite.user}

tests := []struct {
name string
Expand All @@ -520,7 +520,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchange() {
{
name: "Mail",
selector: func() *selectors.ExchangeBackup {
sel := selectors.NewExchangeBackup(users)
sel := selectors.NewExchangeBackup(owners)
sel.Include(sel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch()))
sel.DiscreteOwner = suite.user

Expand All @@ -534,7 +534,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchange() {
{
name: "Contacts",
selector: func() *selectors.ExchangeBackup {
sel := selectors.NewExchangeBackup(users)
sel := selectors.NewExchangeBackup(owners)
sel.Include(sel.ContactFolders([]string{exchange.DefaultContactFolder}, selectors.PrefixMatch()))
return sel
},
Expand All @@ -546,7 +546,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchange() {
{
name: "Calendar Events",
selector: func() *selectors.ExchangeBackup {
sel := selectors.NewExchangeBackup(users)
sel := selectors.NewExchangeBackup(owners)
sel.Include(sel.EventCalendars([]string{exchange.DefaultCalendar}, selectors.PrefixMatch()))
return sel
},
Expand Down Expand Up @@ -639,10 +639,12 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
ffs = control.Toggles{}
mb = evmock.NewBus()
now = common.Now()
users = []string{suite.user}
owners = []string{suite.user}
categories = map[path.CategoryType][]string{
path.EmailCategory: exchange.MetadataFileNames(path.EmailCategory),
path.ContactsCategory: exchange.MetadataFileNames(path.ContactsCategory),
// TODO: not currently functioning; cannot retrieve generated calendars
// path.EventsCategory: exchange.MetadataFileNames(path.EventsCategory),
}
container1 = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 1, now)
container2 = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 2, now)
Expand Down Expand Up @@ -688,6 +690,13 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
)
}

eventDBF := func(id, timeStamp, subject, body string) []byte {
return mockconnector.GetMockEventWith(
suite.user, subject, body, body,
now, now, false)
}

// test data set
dataset := map[path.CategoryType]struct {
dbf dataBuilderFunc
dests map[string]contDeets
Expand All @@ -706,8 +715,17 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
container2: {},
},
},
// TODO: not currently functioning; cannot retrieve generated calendars
// path.EventsCategory: {
// dbf: eventDBF,
// dests: map[string]contDeets{
// container1: {},
// container2: {},
// },
// },
}

// populate initial test data
for category, gen := range dataset {
for destName := range gen.dests {
deets := generateContainerOfItems(
Expand All @@ -717,7 +735,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
path.ExchangeService,
acct,
category,
selectors.NewExchangeRestore(users).Selector,
selectors.NewExchangeRestore(owners).Selector,
m365.AzureTenantID, suite.user, destName,
2,
gen.dbf)
Expand All @@ -726,6 +744,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
}
}

// verify test data was populated, and track it for comparisons
for category, gen := range dataset {
qp := graph.QueryParams{
Category: category,
Expand All @@ -752,7 +771,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
// later on during the tests. Putting their identifiers into the selector
// at this point is harmless.
containers := []string{container1, container2, container3, containerRename}
sel := selectors.NewExchangeBackup(users)
sel := selectors.NewExchangeBackup(owners)
sel.Include(
sel.MailFolders(containers, selectors.PrefixMatch()),
sel.ContactFolders(containers, selectors.PrefixMatch()),
Expand Down Expand Up @@ -788,7 +807,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
toContainer := dataset[path.EmailCategory].dests[container1].containerID
fromContainer := dataset[path.EmailCategory].dests[container2].containerID

body := msuser.NewItemMailFoldersItemMovePostRequestBody()
body := users.NewItemMailFoldersItemMovePostRequestBody()
body.SetDestinationId(&toContainer)

_, err := gc.Service.
Expand Down Expand Up @@ -820,6 +839,11 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
t,
cli.ContactFoldersById(containerID).Delete(ctx, nil),
"deleting a contacts folder")
case path.EventsCategory:
require.NoError(
t,
cli.CalendarsById(containerID).Delete(ctx, nil),
"deleting a calendar")
}
}
},
Expand All @@ -837,7 +861,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
path.ExchangeService,
acct,
category,
selectors.NewExchangeRestore(users).Selector,
selectors.NewExchangeRestore(owners).Selector,
m365.AzureTenantID, suite.user, container3,
2,
gen.dbf)
Expand Down Expand Up @@ -897,6 +921,16 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {
body.SetDisplayName(&containerRename)
_, err = ccf.Patch(ctx, body, nil)
require.NoError(t, err, "updating contact folder name")

case path.EventsCategory:
ccf := cli.CalendarsById(containerID)

body, err := ccf.Get(ctx, nil)
require.NoError(t, err, "getting calendar")

body.SetName(&containerRename)
_, err = ccf.Patch(ctx, body, nil)
require.NoError(t, err, "updating calendar name")
}
}
},
Expand Down Expand Up @@ -926,6 +960,14 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {

_, err = cli.ContactFoldersById(containerID).Contacts().Post(ctx, body, nil)
require.NoError(t, err, "posting contact item")

case path.EventsCategory:
_, itemData := generateItemData(t, category, suite.user, eventDBF)
body, err := support.CreateEventFromBytes(itemData)
require.NoError(t, err, "transforming event bytes to eventable")

_, err = cli.CalendarsById(containerID).Events().Post(ctx, body, nil)
require.NoError(t, err, "posting events item")
}
}
},
Expand Down Expand Up @@ -955,6 +997,14 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() {

err = cli.ContactsById(ids[0]).Delete(ctx, nil)
require.NoError(t, err, "deleting contact item: %s", support.ConnectorStackErrorTrace(err))

case path.EventsCategory:
ids, _, _, err := ac.Events().GetAddedAndRemovedItemIDs(ctx, suite.user, containerID, "")
require.NoError(t, err, "getting event ids")
require.NotEmpty(t, ids, "event ids in folder")

err = cli.CalendarsById(ids[0]).Delete(ctx, nil)
require.NoError(t, err, "deleting calendar: %s", support.ConnectorStackErrorTrace(err))
}
}
},
Expand Down
4 changes: 2 additions & 2 deletions src/internal/operations/restore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ func (suite *RestoreOpIntegrationSuite) SetupSuite() {
require.NotEmpty(t, bo.Results.BackupID)

suite.backupID = bo.Results.BackupID
// Discount metadata files (3 paths, 2 deltas) as
// Discount metadata files (3 paths, 3 deltas) as
// they are not part of the data restored.
suite.numItems = bo.Results.ItemsWritten - 5
suite.numItems = bo.Results.ItemsWritten - 6
}

func (suite *RestoreOpIntegrationSuite) TearDownSuite() {
Expand Down