Enforce site filter for Groups restore (#4496)

This PR reworks the groups restore to and makes the site to restore to mandatory. This also updates some missing filtering capabilities in groups export.

---

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

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* closes https://github.com/alcionai/corso/issues/4462

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

CHANGELOG.md

@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Catch and report cases where a protected resource is locked out of access.  SDK consumers have a new errs sentinel that allows them to check for this case.
 - Fix a case where missing item LastModifiedTimes could cause incremental backups to fail.
 - Email size metadata was incorrectly set to the size of the last attachment.  Emails will now correctly report the size of the mail content plus the size of all attachments.
+- Improves the filtering capabilities for Groups restore and backup
+
+### Changed
+- Groups restore now expects the site whose backup we should restore
 
 ## [v0.14.0] (beta) - 2023-10-09
 

src/cli/backup/sharepoint.go

@@ -76,8 +76,8 @@ func addSharePointCommands(cmd *cobra.Command) *cobra.Command {
 		c.Use = c.Use + " " + sharePointServiceCommandCreateUseSuffix
 		c.Example = sharePointServiceCommandCreateExamples
 
-		flags.AddSiteFlag(c)
+		flags.AddSiteFlag(c, true)
-		flags.AddSiteIDFlag(c)
+		flags.AddSiteIDFlag(c, true)
 		flags.AddDataFlag(c, []string{flags.DataLibraries}, true)
 		flags.AddFailFastFlag(c)
 		flags.AddDisableIncrementalsFlag(c)

src/cli/export/groups.go

@@ -27,6 +27,9 @@ func addGroupsCommands(cmd *cobra.Command) *cobra.Command {
 		fs.SortFlags = false
 
 		flags.AddBackupIDFlag(c, true)
+		flags.AddSiteFlag(c, false)
+		flags.AddSiteIDFlag(c, false)
+		flags.AddSharePointDetailsAndRestoreFlags(c)
 		flags.AddGroupDetailsAndRestoreFlags(c)
 		flags.AddExportConfigFlags(c)
 		flags.AddFailFastFlag(c)
@@ -89,7 +92,7 @@ func exportGroupsCmd(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	if err := utils.ValidateGroupsRestoreFlags(flags.BackupIDFV, opts); err != nil {
+	if err := utils.ValidateGroupsRestoreFlags(flags.BackupIDFV, opts, false); err != nil {
 		return err
 	}
 

src/cli/flags/sharepoint.go

@@ -95,24 +95,28 @@ func AddSharePointDetailsAndRestoreFlags(cmd *cobra.Command) {
 // AddSiteIDFlag adds the --site-id flag, which accepts site ID values.
 // This flag is hidden, since we expect users to prefer the --site url
 // and do not want to encourage confusion.
-func AddSiteIDFlag(cmd *cobra.Command) {
+func AddSiteIDFlag(cmd *cobra.Command, multiple bool) {
 	fs := cmd.Flags()
 
+	message := "ID of the site to operate on"
+	if multiple {
+		//nolint:lll
+		message += "; accepts '" + Wildcard + "' to select all sites.  Args cannot be comma-delimited and must use multiple flags."
+	}
+
 	// note string ARRAY var.  IDs naturally contain commas, so we cannot accept
 	// duplicate values within a flag declaration.  ie: --site-id a,b,c does not
 	// work.  Users must call --site-id a --site-id b --site-id c.
-	fs.StringArrayVar(
+	fs.StringArrayVar(&SiteIDFV, SiteIDFN, nil, message)
-		&SiteIDFV,
-		SiteIDFN, nil,
-		//nolint:lll
-		"Backup data by site ID; accepts '"+Wildcard+"' to select all sites.  Args cannot be comma-delimited and must use multiple flags.")
 	cobra.CheckErr(fs.MarkHidden(SiteIDFN))
 }
 
 // AddSiteFlag adds the --site flag, which accepts webURL values.
-func AddSiteFlag(cmd *cobra.Command) {
+func AddSiteFlag(cmd *cobra.Command, multiple bool) {
-	cmd.Flags().StringSliceVar(
+	message := "Web URL of the site to operate on"
-		&WebURLFV,
+	if multiple {
-		SiteFN, nil,
+		message += "; accepts '" + Wildcard + "' to select all sites."
-		"Backup data by site URL; accepts '"+Wildcard+"' to select all sites.")
+	}
+
+	cmd.Flags().StringSliceVar(&WebURLFV, SiteFN, nil, message)
 }

src/cli/flags/testdata/flags.go

@@ -10,6 +10,7 @@ func FlgInputs(in []string) string { return strings.Join(in, ",") }
 
 var (
 	BackupInput = "backup-id"
+	SiteInput   = "site-id"
 
 	GroupsInput  = []string{"team1", "group2"}
 	MailboxInput = []string{"mailbox1", "mailbox2"}

src/cli/restore/groups.go

@@ -27,9 +27,10 @@ func addGroupsCommands(cmd *cobra.Command) *cobra.Command {
 		fs.SortFlags = false
 
 		flags.AddBackupIDFlag(c, true)
+		flags.AddSiteFlag(c, false)
+		flags.AddSiteIDFlag(c, false)
 		flags.AddNoPermissionsFlag(c)
-		flags.AddSharePointDetailsAndRestoreFlags(c) // for sp restores
+		flags.AddSharePointDetailsAndRestoreFlags(c)
-		flags.AddSiteIDFlag(c)
 		flags.AddRestoreConfigFlags(c, false)
 		flags.AddFailFastFlag(c)
 	}
@@ -83,7 +84,7 @@ func restoreGroupsCmd(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	if err := utils.ValidateGroupsRestoreFlags(flags.BackupIDFV, opts); err != nil {
+	if err := utils.ValidateGroupsRestoreFlags(flags.BackupIDFV, opts, true); err != nil {
 		return err
 	}
 

src/cli/restore/groups_test.go

@@ -52,6 +52,7 @@ func (suite *GroupsUnitSuite) TestAddGroupsCommands() {
 					[]string{
 						"--" + flags.RunModeFN, flags.RunModeFlagTest,
 						"--" + flags.BackupFN, flagsTD.BackupInput,
+						"--" + flags.SiteFN, flagsTD.SiteInput,
 						"--" + flags.LibraryFN, flagsTD.LibraryInput,
 						"--" + flags.FileFN, flagsTD.FlgInputs(flagsTD.FileNameInput),
 						"--" + flags.FolderFN, flagsTD.FlgInputs(flagsTD.FolderPathInput),

src/cli/utils/groups.go

@@ -21,6 +21,7 @@ type GroupsOpts struct {
 	MessageLastReplyBefore string
 
 	SiteID             []string
+	WebURL             []string
 	Library            string
 	FileName           []string // for libraries, to duplicate onedrive interface
 	FolderPath         []string // for libraries, to duplicate onedrive interface
@@ -70,6 +71,7 @@ func MakeGroupsOpts(cmd *cobra.Command) GroupsOpts {
 		Groups:   flags.GroupFV,
 		Channels: flags.ChannelFV,
 		Messages: flags.MessageFV,
+		WebURL:   flags.WebURLFV,
 		SiteID:   flags.SiteIDFV,
 
 		Library:                flags.LibraryFV,
@@ -101,11 +103,22 @@ func MakeGroupsOpts(cmd *cobra.Command) GroupsOpts {
 }
 
 // ValidateGroupsRestoreFlags checks common flags for correctness and interdependencies
-func ValidateGroupsRestoreFlags(backupID string, opts GroupsOpts) error {
+func ValidateGroupsRestoreFlags(backupID string, opts GroupsOpts, isRestore bool) error {
 	if len(backupID) == 0 {
 		return clues.New("a backup ID is required")
 	}
 
+	// The user has to explicitly specify which resource to restore. In
+	// this case, since we can only restore sites, the user is supposed
+	// to specify which site to restore.
+	if isRestore {
+		if len(opts.WebURL)+len(opts.SiteID) == 0 {
+			return clues.New("web URL of the site to restore is required. Use --" + flags.SiteFN + " to provide one.")
+		} else if len(opts.WebURL)+len(opts.SiteID) > 1 {
+			return clues.New("only a single site can be selected for restore")
+		}
+	}
+
 	if _, ok := opts.Populated[flags.FileCreatedAfterFN]; ok && !IsValidTimeFormat(opts.FileCreatedAfter) {
 		return clues.New("invalid time format for " + flags.FileCreatedAfterFN)
 	}
@@ -164,8 +177,6 @@ func IncludeGroupsRestoreDataSelectors(ctx context.Context, opts GroupsOpts) *se
 		llf, lli    = len(opts.ListFolder), len(opts.ListItem)
 		lpf, lpi    = len(opts.PageFolder), len(opts.Page)
 		lg, lch, lm = len(opts.Groups), len(opts.Channels), len(opts.Messages)
-		// TODO(meain): handle sites once we add non-root site backup
-		// ls := len(opts.SiteID)
 	)
 
 	if lg == 0 {
@@ -259,6 +270,23 @@ func FilterGroupsRestoreInfoSelectors(
 	sel *selectors.GroupsRestore,
 	opts GroupsOpts,
 ) {
+	var site string
+
+	if len(opts.SiteID) > 0 {
+		site = opts.SiteID[0]
+	} else {
+		// using else instead of else if so that it would have a hard
+		// fail in case we somehow miss checking this earlier
+		site = opts.WebURL[0]
+	}
+
+	// sel.Site can accept both ID and URL and so irrespective of
+	// which flag the user uses, we can process both weburl and siteid
+	// Also since the url will start with `https://` in the data that
+	// we store and the id is a uuid, we can grantee that there will be
+	// no collisions.
+	AddGroupsFilter(sel, site, sel.Site)
+
 	AddGroupsFilter(sel, opts.Library, sel.Library)
 	AddGroupsFilter(sel, opts.FileCreatedAfter, sel.CreatedAfter)
 	AddGroupsFilter(sel, opts.FileCreatedBefore, sel.CreatedBefore)

src/cli/utils/groups_test.go

@@ -71,7 +71,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 			opts: utils.GroupsOpts{
 				FileName:   empty,
 				FolderPath: containsOnly,
-				SiteID:     empty,
 			},
 			expectIncludeLen: 1,
 		},
@@ -80,7 +79,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 			opts: utils.GroupsOpts{
 				FileName:   empty,
 				FolderPath: prefixOnly,
-				SiteID:     empty,
 			},
 			expectIncludeLen: 1,
 		},
@@ -89,7 +87,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 			opts: utils.GroupsOpts{
 				FileName:   empty,
 				FolderPath: containsAndPrefix,
-				SiteID:     empty,
 			},
 			expectIncludeLen: 2,
 		},
@@ -100,7 +97,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 				FolderPath: empty,
 				ListItem:   empty,
 				ListFolder: containsOnly,
-				SiteID:     empty,
 			},
 			expectIncludeLen: 1,
 		},
@@ -123,7 +119,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 			opts: utils.GroupsOpts{
 				FileName:   empty,
 				FolderPath: empty,
-				// SiteID:     empty,  // TODO(meain): Update once we support multiple sites
 			},
 			expectIncludeLen: 2,
 		},
@@ -132,7 +127,6 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 			opts: utils.GroupsOpts{
 				FileName:   empty,
 				FolderPath: empty,
-				// SiteID:     empty, // TODO(meain): update once we support multiple sites
 			},
 			expectIncludeLen: 2,
 		},
@@ -231,11 +225,29 @@ func (suite *GroupsUtilsSuite) TestValidateGroupsRestoreFlags() {
 		expect   assert.ErrorAssertionFunc
 	}{
 		{
-			name:     "no opts",
+			name:     "just site",
 			backupID: "id",
-			opts:     utils.GroupsOpts{},
+			opts:     utils.GroupsOpts{WebURL: []string{"site"}}, // site is mandatory
 			expect:   assert.NoError,
 		},
+		{
+			name:     "just siteid",
+			backupID: "id",
+			opts:     utils.GroupsOpts{SiteID: []string{"site-id"}},
+			expect:   assert.NoError,
+		},
+		{
+			name:     "multiple sites",
+			backupID: "id",
+			opts:     utils.GroupsOpts{SiteID: []string{"site-id1", "site-id2"}},
+			expect:   assert.Error,
+		},
+		{
+			name:     "site and siteid",
+			backupID: "id",
+			opts:     utils.GroupsOpts{SiteID: []string{"site-id"}, WebURL: []string{"site"}},
+			expect:   assert.Error,
+		},
 		{
 			name:     "no backupID",
 			backupID: "",
@@ -246,6 +258,7 @@ func (suite *GroupsUtilsSuite) TestValidateGroupsRestoreFlags() {
 			name:     "all valid",
 			backupID: "id",
 			opts: utils.GroupsOpts{
+				WebURL:                 []string{"site"},
 				FileCreatedAfter:       dttm.Now(),
 				FileCreatedBefore:      dttm.Now(),
 				FileModifiedAfter:      dttm.Now(),
@@ -362,7 +375,7 @@ func (suite *GroupsUtilsSuite) TestValidateGroupsRestoreFlags() {
 	for _, test := range table {
 		suite.Run(test.name, func() {
 			t := suite.T()
-			test.expect(t, utils.ValidateGroupsRestoreFlags(test.backupID, test.opts))
+			test.expect(t, utils.ValidateGroupsRestoreFlags(test.backupID, test.opts, true))
 		})
 	}
 }

src/pkg/selectors/groups.go

@@ -261,6 +261,21 @@ func (s *groups) ChannelMessages(channels, messages []string, opts ...option) []
 	return scopes
 }
 
+// Sites produces one or more Groups site scopes, where the site
+// matches upon a given site by ID or URL.
+// If any slice contains selectors.Any, that slice is reduced to [selectors.Any]
+// If any slice contains selectors.None, that slice is reduced to [selectors.None]
+// If any slice is empty, it defaults to [selectors.None]
+func (s *groups) Site(site string) []GroupsScope {
+	return []GroupsScope{
+		makeInfoScope[GroupsScope](
+			GroupsLibraryItem,
+			GroupsInfoSite,
+			[]string{site},
+			filters.Equal),
+	}
+}
+
 // Library produces one or more Group library scopes, where the library
 // matches upon a given drive by ID or Name.  In order to ensure library selection
 // this should always be embedded within the Filter() set; include(Library()) will
@@ -519,6 +534,7 @@ const (
 	GroupsInfoLibraryItemModifiedBefore groupsCategory = "GroupsInfoLibraryItemModifiedBefore"
 
 	// channel and drive selection
+	GroupsInfoSite             groupsCategory = "GroupsInfoSite"
 	GroupsInfoSiteLibraryDrive groupsCategory = "GroupsInfoSiteLibraryDrive"
 
 	// data contained within details.ItemInfo
@@ -562,7 +578,7 @@ func (c groupsCategory) leafCat() categorizer {
 		GroupsInfoChannelMessageCreatedAfter, GroupsInfoChannelMessageCreatedBefore, GroupsInfoChannelMessageCreator,
 		GroupsInfoChannelMessageLastReplyAfter, GroupsInfoChannelMessageLastReplyBefore:
 		return GroupsChannelMessage
-	case GroupsLibraryFolder, GroupsLibraryItem, GroupsInfoSiteLibraryDrive,
+	case GroupsLibraryFolder, GroupsLibraryItem, GroupsInfoSite, GroupsInfoSiteLibraryDrive,
 		GroupsInfoLibraryItemCreatedAfter, GroupsInfoLibraryItemCreatedBefore,
 		GroupsInfoLibraryItemModifiedAfter, GroupsInfoLibraryItemModifiedBefore:
 		return GroupsLibraryItem
@@ -781,6 +797,18 @@ func (s GroupsScope) matchesInfo(dii details.ItemInfo) bool {
 	}
 
 	switch infoCat {
+	case GroupsInfoSite:
+		ds := []string{}
+
+		if len(info.SiteID) > 0 {
+			ds = append(ds, info.SiteID)
+		}
+
+		if len(info.WebURL) > 0 {
+			ds = append(ds, info.WebURL)
+		}
+
+		return matchesAny(s, GroupsInfoSite, ds)
 	case GroupsInfoSiteLibraryDrive:
 		ds := []string{}
 

src/pkg/selectors/groups_test.go

@@ -394,6 +394,8 @@ func (suite *GroupsSelectorSuite) TestGroupsScope_MatchesInfo() {
 		{"file modified before epoch", dspl, user, sel.ModifiedBefore(dttm.Format(now)), assert.False},
 		{"in library", dspl, user, sel.Library("included-library"), assert.True},
 		{"not in library", dspl, user, sel.Library("not-included-library"), assert.False},
+		{"site id", dspl, user, sel.Site("site1"), assert.True},
+		{"web url", dspl, user, sel.Site(user), assert.True},
 		{"library id", dspl, user, sel.Library("1234"), assert.True},
 		{"not library id", dspl, user, sel.Library("abcd"), assert.False},
 
@@ -430,6 +432,7 @@ func (suite *GroupsSelectorSuite) TestGroupsScope_MatchesInfo() {
 					LastReplyAt:    mod,
 					DriveName:      "included-library",
 					DriveID:        "1234",
+					SiteID:         "site1",
 				},
 			}
 
@@ -457,6 +460,7 @@ func (suite *GroupsSelectorSuite) TestCategory_PathType() {
 		{GroupsLibraryFolder, path.LibrariesCategory},
 		{GroupsLibraryItem, path.LibrariesCategory},
 		{GroupsInfoSiteLibraryDrive, path.LibrariesCategory},
+		{GroupsInfoSite, path.LibrariesCategory},
 	}
 	for _, test := range table {
 		suite.Run(test.cat.String(), func() {