From bf0a01708a3562fda5c9babd06ceba231da26de9 Mon Sep 17 00:00:00 2001 From: Keepers Date: Mon, 9 Jan 2023 15:42:17 -0700 Subject: [PATCH] remove selector printable, list discreteOwner (#2034) ## Description The selectors printable code wasn't being used in any valid way, so that's out. Backup List printing now refers to the DiscreteOwner in the embedded selector as the ResourceOwner reference. Renamed the "Selectors" column in the List print table to "Resource Owner" to better match what should be contained in that field. ## Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included ## Type of change - [x] :bug: Bugfix - [x] :broom: Tech Debt/Cleanup ## Issue(s) * #1617 ## Test Plan - [x] :muscle: Manual - [x] :zap: Unit test --- src/cli/utils/exchange_test.go | 6 - .../exchange/data_collections_test.go | 6 +- src/pkg/backup/backup.go | 26 ++-- src/pkg/backup/backup_test.go | 13 +- src/pkg/repository/repository.go | 2 +- src/pkg/selectors/exchange.go | 6 - src/pkg/selectors/helpers_test.go | 4 - src/pkg/selectors/onedrive.go | 6 - src/pkg/selectors/scopes_test.go | 27 ---- src/pkg/selectors/selectors.go | 123 ------------------ src/pkg/selectors/selectors_test.go | 122 ----------------- src/pkg/selectors/sharepoint.go | 6 - website/docs/quickstart.md | 4 +- 13 files changed, 23 insertions(+), 328 deletions(-) diff --git a/src/cli/utils/exchange_test.go b/src/cli/utils/exchange_test.go index 5559e72bd..38311c267 100644 --- a/src/cli/utils/exchange_test.go +++ b/src/cli/utils/exchange_test.go @@ -325,42 +325,36 @@ func (suite *ExchangeUtilsSuite) TestAddExchangeInclude() { }{ { name: "no inputs", - resources: empty, folders: empty, items: empty, expectIncludeLen: 0, }, { name: "single inputs", - resources: single, folders: single, items: single, expectIncludeLen: 1, }, { name: "multi inputs", - resources: multi, folders: multi, items: multi, expectIncludeLen: 1, }, { name: "folder contains", - resources: empty, folders: containsOnly, items: empty, expectIncludeLen: 1, }, { name: "folder prefixes", - resources: empty, folders: prefixOnly, items: empty, expectIncludeLen: 1, }, { name: "folder prefixes and contains", - resources: empty, folders: containsAndPrefix, items: empty, expectIncludeLen: 2, diff --git a/src/internal/connector/exchange/data_collections_test.go b/src/internal/connector/exchange/data_collections_test.go index 8b2422bc5..3d69ba79c 100644 --- a/src/internal/connector/exchange/data_collections_test.go +++ b/src/internal/connector/exchange/data_collections_test.go @@ -525,14 +525,16 @@ func (suite *DataCollectionsIntegrationSuite) TestEventsSerializationRegression( expected: DefaultCalendar, scope: selectors.NewExchangeBackup(users).EventCalendars( []string{DefaultCalendar}, - selectors.PrefixMatch())[0], + selectors.PrefixMatch(), + )[0], }, { name: "Birthday Calendar", expected: "Birthdays", scope: selectors.NewExchangeBackup(users).EventCalendars( []string{"Birthdays"}, - selectors.PrefixMatch())[0], + selectors.PrefixMatch(), + )[0], }, } diff --git a/src/pkg/backup/backup.go b/src/pkg/backup/backup.go index cef46aa3e..6d73498eb 100644 --- a/src/pkg/backup/backup.go +++ b/src/pkg/backup/backup.go @@ -28,8 +28,8 @@ type Backup struct { // Status of the operation Status string `json:"status"` - // Selectors used in this operation - Selectors selectors.Selector `json:"selectors"` + // Selector used in this operation + Selector selectors.Selector `json:"selectors"` // stats are embedded so that the values appear as top-level properties stats.Errs @@ -58,7 +58,7 @@ func New( SnapshotID: snapshotID, DetailsID: detailsID, Status: status, - Selectors: selector, + Selector: selector, ReadWrites: rw, StartAndEndTime: se, } @@ -89,14 +89,13 @@ func PrintAll(ctx context.Context, bs []*Backup) { } type Printable struct { - ID model.StableID `json:"id"` - ErrorCount int `json:"errorCount"` - StartedAt time.Time `json:"started at"` - Status string `json:"status"` - Version string `json:"version"` - Selectors selectors.Printable `json:"selectors"` - BytesRead int64 `json:"bytesRead"` - BytesUploaded int64 `json:"bytesUploaded"` + ID model.StableID `json:"id"` + ErrorCount int `json:"errorCount"` + StartedAt time.Time `json:"started at"` + Status string `json:"status"` + Version string `json:"version"` + BytesRead int64 `json:"bytesRead"` + BytesUploaded int64 `json:"bytesUploaded"` } // MinimumPrintable reduces the Backup to its minimally printable details. @@ -107,7 +106,6 @@ func (b Backup) MinimumPrintable() any { StartedAt: b.StartedAt, Status: b.Status, Version: "0", - Selectors: b.Selectors.ToPrintable(), BytesRead: b.BytesRead, BytesUploaded: b.BytesUploaded, } @@ -120,7 +118,7 @@ func (b Backup) Headers() []string { "Started At", "ID", "Status", - "Selectors", + "Resource Owner", } } @@ -134,6 +132,6 @@ func (b Backup) Values() []string { common.FormatTabularDisplayTime(b.StartedAt), string(b.ID), status, - b.Selectors.ToPrintable().Resources(), + b.Selector.DiscreteOwner, } } diff --git a/src/pkg/backup/backup_test.go b/src/pkg/backup/backup_test.go index 2e2429b4d..1ddf4f4a2 100644 --- a/src/pkg/backup/backup_test.go +++ b/src/pkg/backup/backup_test.go @@ -25,7 +25,7 @@ func TestBackupSuite(t *testing.T) { } func stubBackup(t time.Time) backup.Backup { - sel := selectors.NewExchangeBackup(selectors.Any()) + sel := selectors.NewExchangeBackup([]string{"test"}) sel.Include(sel.AllData()) return backup.Backup{ @@ -39,7 +39,7 @@ func stubBackup(t time.Time) backup.Backup { SnapshotID: "snapshot", DetailsID: "details", Status: "status", - Selectors: sel.Selector, + Selector: sel.Selector, Errs: stats.Errs{ ReadErrors: errors.New("1"), WriteErrors: errors.New("1"), @@ -66,7 +66,7 @@ func (suite *BackupSuite) TestBackup_HeadersValues() { "Started At", "ID", "Status", - "Selectors", + "Resource Owner", } hs := b.Headers() assert.Equal(t, expectHs, hs) @@ -76,7 +76,7 @@ func (suite *BackupSuite) TestBackup_HeadersValues() { nowFmt, "id", "status (2 errors)", - selectors.All, + "test", } vs := b.Values() @@ -96,11 +96,6 @@ func (suite *BackupSuite) TestBackup_MinimumPrintable() { assert.Equal(t, 2, result.ErrorCount, "error count") assert.Equal(t, now, result.StartedAt, "started at") assert.Equal(t, b.Status, result.Status, "status") - - bselp := b.Selectors.ToPrintable() - assert.Equal(t, bselp, result.Selectors, "selectors") - assert.Equal(t, bselp.Resources(), result.Selectors.Resources(), "selector resources") - assert.Equal(t, b.BytesRead, result.BytesRead, "size") assert.Equal(t, b.BytesUploaded, result.BytesUploaded, "stored size") } diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index c309723e5..e7d2a3c56 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -306,7 +306,7 @@ func (r repository) BackupDetails(ctx context.Context, backupID string) (*detail deets, err := streamstore.New( r.dataLayer, r.Account.ID(), - b.Selectors.PathService()).ReadBackupDetails(ctx, dID) + b.Selector.PathService()).ReadBackupDetails(ctx, dID) if err != nil { return nil, nil, err } diff --git a/src/pkg/selectors/exchange.go b/src/pkg/selectors/exchange.go index 79f5de9f6..109ee2ea4 100644 --- a/src/pkg/selectors/exchange.go +++ b/src/pkg/selectors/exchange.go @@ -38,7 +38,6 @@ type ( var ( _ Reducer = &ExchangeRestore{} - _ printabler = &ExchangeRestore{} _ pathCategorier = &ExchangeRestore{} ) @@ -110,11 +109,6 @@ func (sr ExchangeRestore) SplitByResourceOwner(users []string) []ExchangeRestore return ss } -// Printable creates the minimized display of a selector, formatted for human readability. -func (s exchange) Printable() Printable { - return toPrintable[ExchangeScope](s.Selector) -} - // PathCategories produces the aggregation of discrete users described by each type of scope. func (s exchange) PathCategories() selectorPathCategories { return selectorPathCategories{ diff --git a/src/pkg/selectors/helpers_test.go b/src/pkg/selectors/helpers_test.go index 32587a85e..6b28ea443 100644 --- a/src/pkg/selectors/helpers_test.go +++ b/src/pkg/selectors/helpers_test.go @@ -160,10 +160,6 @@ func stubSelector(resourceOwners []string) mockSel { } } -func (s mockSel) Printable() Printable { - return toPrintable[mockScope](s.Selector) -} - // --------------------------------------------------------------------------- // helper funcs // --------------------------------------------------------------------------- diff --git a/src/pkg/selectors/onedrive.go b/src/pkg/selectors/onedrive.go index 22660db30..69cc0687a 100644 --- a/src/pkg/selectors/onedrive.go +++ b/src/pkg/selectors/onedrive.go @@ -37,7 +37,6 @@ type ( var ( _ Reducer = &OneDriveRestore{} - _ printabler = &OneDriveRestore{} _ pathCategorier = &OneDriveRestore{} ) @@ -109,11 +108,6 @@ func (s OneDriveRestore) SplitByResourceOwner(users []string) []OneDriveRestore return ss } -// Printable creates the minimized display of a selector, formatted for human readability. -func (s oneDrive) Printable() Printable { - return toPrintable[OneDriveScope](s.Selector) -} - // PathCategories produces the aggregation of discrete users described by each type of scope. func (s oneDrive) PathCategories() selectorPathCategories { return selectorPathCategories{ diff --git a/src/pkg/selectors/scopes_test.go b/src/pkg/selectors/scopes_test.go index 86139dcee..9758b1109 100644 --- a/src/pkg/selectors/scopes_test.go +++ b/src/pkg/selectors/scopes_test.go @@ -393,33 +393,6 @@ func (suite *SelectorScopesSuite) TestMatchesPathValues() { } } -func (suite *SelectorScopesSuite) TestAddToSet() { - t := suite.T() - set := []string{} - - set = addToSet(set, []string{}) - assert.Len(t, set, 0) - - set = addToSet(set, []string{"a"}) - assert.Len(t, set, 1) - assert.Equal(t, set[0], "a") - - set = addToSet(set, []string{"a"}) - assert.Len(t, set, 1) - - set = addToSet(set, []string{"a", "b"}) - assert.Len(t, set, 2) - assert.Equal(t, set[0], "a") - assert.Equal(t, set[1], "b") - - set = addToSet(set, []string{"c", "d"}) - assert.Len(t, set, 4) - assert.Equal(t, set[0], "a") - assert.Equal(t, set[1], "b") - assert.Equal(t, set[2], "c") - assert.Equal(t, set[3], "d") -} - func (suite *SelectorScopesSuite) TestClean() { table := []struct { name string diff --git a/src/pkg/selectors/selectors.go b/src/pkg/selectors/selectors.go index b423c0405..a51ed24bb 100644 --- a/src/pkg/selectors/selectors.go +++ b/src/pkg/selectors/selectors.go @@ -3,7 +3,6 @@ package selectors import ( "context" "encoding/json" - "fmt" "strings" "github.com/pkg/errors" @@ -318,128 +317,6 @@ func selectorAsIface[T any](s Selector) (T, error) { return t, err } -// --------------------------------------------------------------------------- -// Printing Selectors for Human Reading -// --------------------------------------------------------------------------- - -type Printable struct { - ResourceOwners []string `json:"resourceOwners"` - Service string `json:"service"` - Excludes map[string][]string `json:"excludes,omitempty"` - Filters map[string][]string `json:"filters,omitempty"` - Includes map[string][]string `json:"includes,omitempty"` -} - -type printabler interface { - Printable() Printable -} - -// ToPrintable creates the minimized display of a selector, formatted for human readability. -func (s Selector) ToPrintable() Printable { - p, err := selectorAsIface[printabler](s) - if err != nil { - return Printable{} - } - - return p.Printable() -} - -// toPrintable creates the minimized display of a selector, formatted for human readability. -func toPrintable[T scopeT](s Selector) Printable { - return Printable{ - ResourceOwners: s.DiscreteResourceOwners(), - Service: s.Service.String(), - Excludes: toResourceTypeMap[T](s.Excludes), - Filters: toResourceTypeMap[T](s.Filters), - Includes: toResourceTypeMap[T](s.Includes), - } -} - -// Resources generates a tabular-readable output of the resources in Printable. -// Only the first (arbitrarily picked) resource is displayed. All others are -// simply counted. If no inclusions exist, uses Filters. If no filters exist, -// defaults to "None". -// Resource refers to the top-level entity in the service. User for Exchange, -// Site for sharepoint, etc. -func (p Printable) Resources() string { - s := resourcesShortFormat(p.ResourceOwners) - - if len(s) == 0 { - s = "None" - } - - if s == AnyTgt { - s = "All" - } - - return s -} - -// returns a string with the resources in the map. Shortened to the first resource key, -// plus, if more exist, " (len-1 more)" -func resourcesShortFormat(ros []string) string { - switch len(ros) { - case 0: - return "" - case 1: - return ros[0] - default: - return fmt.Sprintf("%s (%d more)", ros[0], len(ros)-1) - } -} - -// Transforms the slice to a single map. -// Keys are each service's rootCat value. -// Values are the set of all scopeKeyDataTypes for the resource. -func toResourceTypeMap[T scopeT](s []scope) map[string][]string { - if len(s) == 0 { - return nil - } - - r := make(map[string][]string) - - for _, sc := range s { - t := T(sc) - res := sc[t.categorizer().rootCat().String()] - k := res.Target - - if res.Target == AnyTgt { - k = All - } - - for _, sk := range split(k) { - r[sk] = addToSet(r[sk], split(sc[scopeKeyDataType].Target)) - } - } - - return r -} - -// returns v if set is empty, -// unions v with set, otherwise. -func addToSet(set []string, v []string) []string { - if len(set) == 0 { - return v - } - - for _, vv := range v { - var matched bool - - for _, s := range set { - if vv == s { - matched = true - break - } - } - - if !matched { - set = append(set, vv) - } - } - - return set -} - // --------------------------------------------------------------------------- // helpers // --------------------------------------------------------------------------- diff --git a/src/pkg/selectors/selectors_test.go b/src/pkg/selectors/selectors_test.go index 55e6a12ae..08da905d5 100644 --- a/src/pkg/selectors/selectors_test.go +++ b/src/pkg/selectors/selectors_test.go @@ -1,7 +1,6 @@ package selectors import ( - "strings" "testing" "github.com/stretchr/testify/assert" @@ -32,127 +31,6 @@ func (suite *SelectorSuite) TestBadCastErr() { assert.Error(suite.T(), err) } -func (suite *SelectorSuite) TestPrintable() { - t := suite.T() - - sel := stubSelector(Any()) - p := sel.Printable() - - assert.Equal(t, sel.Service.String(), p.Service) - assert.Equal(t, 1, len(p.Excludes)) - assert.Equal(t, 1, len(p.Filters)) - assert.Equal(t, 1, len(p.Includes)) -} - -func (suite *SelectorSuite) TestPrintable_IncludedResources() { - table := []struct { - name string - resourceOwners []string - expect func(string) bool - reason string - }{ - { - name: "distinct", - resourceOwners: []string{"foo", "smarf", "fnords"}, - expect: func(s string) bool { - return strings.HasSuffix(s, "(2 more)") - }, - reason: "should end with (2 more)", - }, - { - name: "distinct", - resourceOwners: nil, - expect: func(s string) bool { - return strings.HasSuffix(s, "None") - }, - reason: "no resource owners should produce None", - }, - } - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - sel := stubSelector(test.resourceOwners) - - stubWithResource := func(resource string) scope { - ss := stubScope("") - ss[rootCatStub.String()] = filterize(scopeConfig{}, resource) - return scope(ss) - } - - sel.Includes = []scope{} - sel.Filters = []scope{} - - for _, ro := range test.resourceOwners { - sel.Includes = append(sel.Includes, stubWithResource(ro)) - sel.Filters = append(sel.Filters, stubWithResource(ro)) - } - }) - } -} - -func (suite *SelectorSuite) TestToResourceTypeMap() { - table := []struct { - name string - input []scope - expect map[string][]string - }{ - { - name: "single scope", - input: []scope{scope(stubScope(""))}, - expect: map[string][]string{ - "All": {rootCatStub.String()}, - }, - }, - { - name: "disjoint resources", - input: []scope{ - scope(stubScope("")), - { - rootCatStub.String(): filterize(scopeConfig{}, "smarf"), - scopeKeyDataType: filterize(scopeConfig{}, unknownCatStub.String()), - }, - }, - expect: map[string][]string{ - "All": {rootCatStub.String()}, - "smarf": {unknownCatStub.String()}, - }, - }, - { - name: "multiple resources", - input: []scope{ - scope(stubScope("")), - { - rootCatStub.String(): filterize(scopeConfig{}, join("smarf", "fnords")), - scopeKeyDataType: filterize(scopeConfig{}, unknownCatStub.String()), - }, - }, - expect: map[string][]string{ - "All": {rootCatStub.String()}, - "smarf": {unknownCatStub.String()}, - "fnords": {unknownCatStub.String()}, - }, - }, - { - name: "disjoint types", - input: []scope{ - scope(stubScope("")), - { - rootCatStub.String(): filterize(scopeConfig{}, AnyTgt), - scopeKeyDataType: filterize(scopeConfig{}, "other"), - }, - }, - expect: map[string][]string{ - "All": {rootCatStub.String(), "other"}, - }, - }, - } - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - rtm := toResourceTypeMap[mockScope](test.input) - assert.Equal(t, test.expect, rtm) - }) - } -} - func (suite *SelectorSuite) TestResourceOwnersIn() { rootCat := rootCatStub.String() diff --git a/src/pkg/selectors/sharepoint.go b/src/pkg/selectors/sharepoint.go index 5701b005e..af4901ac6 100644 --- a/src/pkg/selectors/sharepoint.go +++ b/src/pkg/selectors/sharepoint.go @@ -35,7 +35,6 @@ type ( var ( _ Reducer = &SharePointRestore{} - _ printabler = &SharePointRestore{} _ pathCategorier = &SharePointRestore{} ) @@ -107,11 +106,6 @@ func (s SharePointRestore) SplitByResourceOwner(users []string) []SharePointRest return ss } -// Printable creates the minimized display of a selector, formatted for human readability. -func (s sharePoint) Printable() Printable { - return toPrintable[SharePointScope](s.Selector) -} - // PathCategories produces the aggregation of discrete users described by each type of scope. func (s sharePoint) PathCategories() selectorPathCategories { return selectorPathCategories{ diff --git a/website/docs/quickstart.md b/website/docs/quickstart.md index 89f3e69a1..d51af87b0 100644 --- a/website/docs/quickstart.md +++ b/website/docs/quickstart.md @@ -156,7 +156,7 @@ Your first backup may take some time if your mailbox is large. There will be progress indicators as the backup and, on completion, you should see output similar to: ```text - Started At ID Status Selectors + Started At ID Status Resource Owner 2022-10-20T18:28:53Z d8cd833a-fc63-4872-8981-de5c08e0661b Completed (0 errors) alice@contoso.com ``` @@ -195,7 +195,7 @@ docker run --env-file $HOME/.corso/corso.env \\ ```text - Started At ID Status Selectors + Started At ID Status Resource Owner 2022-10-20T18:28:53Z d8cd833a-fc63-4872-8981-de5c08e0661b Completed (0 errors) alice@contoso.com 2022-10-20T18:40:45Z 391ceeb3-b44d-4365-9a8e-8a8e1315b565 Completed (0 errors) alice@contoso.com ...