Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not backup shared calendars #5207

Merged
merged 2 commits into from
Feb 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
206 changes: 206 additions & 0 deletions src/internal/m365/collection/exchange/container_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package exchange
import (
"context"
"fmt"
"hash/crc32"
stdpath "path"
"testing"

"github.com/alcionai/clues"
"github.com/google/uuid"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -1017,6 +1019,210 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestAddToCache() {
assert.Equal(t, m.expectedLocation, l.String(), "location path")
}

// ---------------------------------------------------------------------------
// EventContainerCache unit tests
// ---------------------------------------------------------------------------

var _ containerGetter = mockEventContainerGetter{}

type mockEventContainerGetter struct {
// containerGetter returns graph.CalendarDisplayable, unlike containersEnumerator
// which returns models.Calendarable.
idToCalendar map[string]graph.CalendarDisplayable
err error
}

func (m mockEventContainerGetter) GetContainerByID(
ctx context.Context,
userID string,
dirID string,
) (graph.Container, error) {
return m.idToCalendar[dirID], m.err
}

var _ containersEnumerator[models.Calendarable] = mockEventContainersEnumerator{}

type mockEventContainersEnumerator struct {
containers []models.Calendarable
err error
}

func (m mockEventContainersEnumerator) EnumerateContainers(
ctx context.Context,
userID string,
baseDirID string,
) ([]models.Calendarable, error) {
return m.containers, m.err
}

type EventsContainerUnitSuite struct {
tester.Suite
}

func TestEventsContainerUnitSuite(t *testing.T) {
suite.Run(t, &EventsContainerUnitSuite{
Suite: tester.NewUnitSuite(t),
})
}

func makeCalendar(
id, name, ownerEmail string,
isDefault bool,
) *models.Calendar {
c := models.NewCalendar()

c.SetId(ptr.To(id))
c.SetName(ptr.To(name))
c.SetIsDefaultCalendar(ptr.To(isDefault))

if len(ownerEmail) > 0 {
email := models.NewEmailAddress()

email.SetAddress(ptr.To(ownerEmail))
// Set crc as the name for keeping this func simple.
eName := fmt.Sprintf("%d", crc32.ChecksumIEEE([]byte(ownerEmail)))
email.SetName(ptr.To(eName))
c.SetOwner(email)
}

return c
}

// Test if we skip backup of shared calendars. These will be backed up for
// the resource owner that owns the calendar.
func (suite *EventsContainerUnitSuite) TestPopulate_SkipSharedCalendars() {
// map of calendars
calendars := map[string]models.Calendarable{
// Default calendars Dx
"D0": makeCalendar(api.DefaultCalendar, api.DefaultCalendar, "owner@bar.com", true),
// Atypical, but creating another default calendar for testing purposes.
"D1": makeCalendar("D1", "D1", "owner@bar.com", true),
// Shared calendars Sx
"S0": makeCalendar("S0", "S0", "sharer@bar.com", false),
// Owned calendars, not default Ox
"O0": makeCalendar("O0", "O0", "owner@bar.com", false),
// Calendars with missing owner informaton
"M0": makeCalendar("M0", "M0", "", false),
}

// Always return default calendar from the getter.
getContainersByID := func() map[string]graph.CalendarDisplayable {
return map[string]graph.CalendarDisplayable{
api.DefaultCalendar: *graph.CreateCalendarDisplayable(calendars["D0"], "parentID"),
}
}

table := []struct {
name string
enumerateContainers func() []models.Calendarable
expectErr assert.ErrorAssertionFunc
assertFunc func(t *testing.T, ecc *eventContainerCache)
}{
{
name: "one default calendar, one shared",
Copy link
Contributor

Choose a reason for hiding this comment

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

excellent set of test cases! To slightly expand this, could you please add two other tests with the following:

  • default calendar has owner info set to nil and there's also a calendar the user owns and a shared calendar
  • default calendar has owner info but fields in it are empty and there's also a calendar the user owns and a shared calendar

These tests will make sure we fallback correctly if there's some missing info in the default (I think it should also be ok to backup the shared calendars in this case since we're missing info). It'll also help make sure we don't have NPEs or anything as the code changes over time. Since this set of tests always sets up things to return a properly populated default calendar it may be easiest to put them in a new Test function

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @ashmrtn! I'll add these extra tests in a followup pr. issue tracking this #5208

enumerateContainers: func() []models.Calendarable {
return []models.Calendarable{
calendars["D0"],
calendars["S0"],
}
},
expectErr: assert.NoError,
assertFunc: func(t *testing.T, ecc *eventContainerCache) {
assert.Len(t, ecc.cache, 1, "expected calendar count")
assert.NotNil(t, ecc.cache[api.DefaultCalendar], "missing default calendar")
},
},
{
name: "2 default calendars, 1 shared",
enumerateContainers: func() []models.Calendarable {
return []models.Calendarable{
calendars["D0"],
calendars["D1"],
calendars["S0"],
}
},
expectErr: assert.NoError,
assertFunc: func(t *testing.T, ecc *eventContainerCache) {
assert.Len(t, ecc.cache, 2, "expected calendar count")
assert.NotNil(t, ecc.cache[api.DefaultCalendar], "missing default calendar")
assert.NotNil(t, ecc.cache["D1"], "missing default calendar")
},
},
{
name: "1 default, 1 additional owned, 1 shared",
enumerateContainers: func() []models.Calendarable {
return []models.Calendarable{
calendars["D0"],
calendars["O0"],
calendars["S0"],
}
},
expectErr: assert.NoError,
assertFunc: func(t *testing.T, ecc *eventContainerCache) {
assert.Len(t, ecc.cache, 2, "expected calendar count")
assert.NotNil(t, ecc.cache[api.DefaultCalendar], "missing default calendar")
assert.NotNil(t, ecc.cache["O0"], "missing owned calendar")
},
},
{
name: "1 default, 1 with missing owner information",
enumerateContainers: func() []models.Calendarable {
return []models.Calendarable{
calendars["D0"],
calendars["M0"],
}
},

expectErr: assert.NoError,
assertFunc: func(t *testing.T, ecc *eventContainerCache) {
assert.Len(t, ecc.cache, 2, "expected calendar count")
assert.NotNil(t, ecc.cache[api.DefaultCalendar], "missing default calendar")
assert.NotNil(t, ecc.cache["M0"], "missing calendar with missing owner info")
},
},
{
// Unlikely to happen, but we should back up the calendar if the default owner
// cannot be determined, i.e. default calendar is missing.
name: "default owner info missing",
enumerateContainers: func() []models.Calendarable {
return []models.Calendarable{
calendars["S0"],
}
},
expectErr: assert.NoError,
assertFunc: func(t *testing.T, ecc *eventContainerCache) {
assert.Len(t, ecc.cache, 2, "expected calendar count")
assert.NotNil(t, ecc.cache[api.DefaultCalendar], "missing default calendar")
assert.NotNil(t, ecc.cache["S0"], "missing additional calendar")
},
},
}

for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()

ctx, flush := tester.NewContext(t)
defer flush()

ecc := &eventContainerCache{
userID: "test",
enumer: mockEventContainersEnumerator{containers: test.enumerateContainers()},
getter: mockEventContainerGetter{idToCalendar: getContainersByID()},
}

err := ecc.Populate(ctx, fault.New(true), "root", "root")
test.expectErr(t, err, clues.ToCore(err))

test.assertFunc(t, ecc)
})
}
}

// ---------------------------------------------------------------------------
// container resolver integration suite
// ---------------------------------------------------------------------------

type ContainerResolverIntgSuite struct {
tester.Suite
m365 its.M365IntgTestSetup
Expand Down
39 changes: 39 additions & 0 deletions src/internal/m365/collection/exchange/events_container_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package exchange

import (
"context"
"strings"
"time"

"github.com/alcionai/clues"
Expand Down Expand Up @@ -60,6 +61,16 @@ func (ecc *eventContainerCache) populateEventRoot(ctx context.Context) error {
return nil
}

func isSharedCalendar(defaultCalendarOwner string, c models.Calendarable) bool {
// If we can't determine the owner, assume the calendar is owned by the
// user.
if len(defaultCalendarOwner) == 0 || c.GetOwner() == nil {
return false
}

return !strings.EqualFold(defaultCalendarOwner, ptr.Val(c.GetOwner().GetAddress()))
}

// Populate utility function for populating eventCalendarCache.
// Executes 1 additional Graph Query
// @param baseID: ignored. Present to conform to interface
Expand Down Expand Up @@ -89,11 +100,39 @@ func (ecc *eventContainerCache) Populate(
return clues.WrapWC(ctx, err, "enumerating containers")
}

var defaultCalendarOwner string

// Determine the owner for the default calendar. We'll use this to detect and
// skip shared calendars that are not owned by this user.
for _, c := range containers {
if ptr.Val(c.GetIsDefaultCalendar()) && c.GetOwner() != nil {
defaultCalendarOwner = ptr.Val(c.GetOwner().GetAddress())
pandeyabs marked this conversation as resolved.
Show resolved Hide resolved
ctx = clues.Add(ctx, "default_calendar_owner", defaultCalendarOwner)

break
}
}

for _, c := range containers {
if el.Failure() != nil {
return el.Failure()
}

// Skip shared calendars if we have enough information to determine the owner
if isSharedCalendar(defaultCalendarOwner, c) {
var ownerEmail string
if c.GetOwner() != nil {
ownerEmail = ptr.Val(c.GetOwner().GetAddress())
}

logger.Ctx(ctx).Infow(
"skipping shared calendar",
"name", ptr.Val(c.GetName()),
"owner", ownerEmail)

continue
}

cacheFolder := graph.NewCacheFolder(
api.CalendarDisplayable{Calendarable: c},
path.Builder{}.Append(ptr.Val(c.GetId())),
Expand Down
Loading