don't handle meta files in details (#2606)

## Description

Remove the display and interaction with .meta
files from details entries.

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

- [x]  Yes, it's included

## Type of change

- [x] 🐛 Bugfix

## Test Plan

- [x]  Unit test
This commit is contained in:
Keepers 2023-02-22 21:14:17 -07:00 committed by GitHub
parent 6c7623041c
commit cb12329d0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 250 additions and 81 deletions

View File

@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Show owner information when doing backup list in json format
### Fixed
- Corso-generated .meta files and permissions no longer appear in the backup details.
### Known Issues
- Folders and Calendars containing zero items or subfolders are not included in the backup.

View File

@ -15,8 +15,8 @@ import (
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/mockconnector"
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/credentials"
@ -94,7 +94,7 @@ func generateAndRestoreItems(
Infof(ctx, "Generating %d %s items in %s\n", howMany, cat, Destination)
return gc.RestoreDataCollections(ctx, backup.Version, acct, sel, dest, opts, dataColls, errs)
return gc.RestoreDataCollections(ctx, version.Backup, acct, sel, dest, opts, dataColls, errs)
}
// ------------------------------------------------------------------------------------------

View File

@ -712,6 +712,7 @@ func compareOneDriveItem(
}
name := item.UUID()
if strings.HasSuffix(name, onedrive.MetaFileSuffix) ||
strings.HasSuffix(name, onedrive.DirMetaFileSuffix) {
var (
@ -1050,10 +1051,12 @@ func collectionsForInfo(
allInfo []colInfo,
backupVersion int,
) (int, int, []data.RestoreCollection, map[string]map[string][]byte) {
collections := make([]data.RestoreCollection, 0, len(allInfo))
expectedData := make(map[string]map[string][]byte, len(allInfo))
totalItems := 0
kopiaEntries := 0
var (
collections = make([]data.RestoreCollection, 0, len(allInfo))
expectedData = make(map[string]map[string][]byte, len(allInfo))
totalItems = 0
kopiaEntries = 0
)
for _, info := range allInfo {
pth := mustToDataLayerPath(

View File

@ -14,8 +14,8 @@ import (
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/onedrive"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/path"
)
@ -175,9 +175,7 @@ func (c *onedriveCollection) withFile(
name+onedrive.DataFileSuffix,
fileData))
case 1:
fallthrough
case 2:
case 1, 2, 3:
c.items = append(c.items, onedriveItemWithData(
c.t,
name+onedrive.DataFileSuffix,
@ -209,9 +207,7 @@ func (c *onedriveCollection) withFolder(
case 0:
return c
case 1:
fallthrough
case 2:
case 1, 2, 3:
c.items = append(
c.items,
onedriveMetadata(
@ -219,8 +215,7 @@ func (c *onedriveCollection) withFolder(
"",
name+onedrive.DirMetaFileSuffix,
user,
roles),
)
roles))
default:
assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion)
@ -237,7 +232,7 @@ func (c *onedriveCollection) withPermissions(
) *onedriveCollection {
// These versions didn't store permissions for the folder or didn't store them
// in the folder's collection.
if c.backupVersion < 3 {
if c.backupVersion < version.OneDriveXIncludesPermissions {
return c
}
@ -281,7 +276,7 @@ type onedriveColInfo struct {
type onedriveTest struct {
name string
// Version this test first be run for. Will run from
// [startVersion, backup.Version] inclusive.
// [startVersion, version.Backup] inclusive.
startVersion int
cols []onedriveColInfo
}
@ -467,17 +462,17 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Multip
}
for _, test := range table {
expected := testDataForInfo(suite.T(), test.cols, backup.Version)
expected := testDataForInfo(suite.T(), test.cols, version.Backup)
for version := test.startVersion; version <= backup.Version; version++ {
suite.T().Run(fmt.Sprintf("%s_Version%d", test.name, version), func(t *testing.T) {
input := testDataForInfo(t, test.cols, version)
for vn := test.startVersion; vn <= version.Backup; vn++ {
suite.T().Run(fmt.Sprintf("%s_Version%d", test.name, vn), func(t *testing.T) {
input := testDataForInfo(t, test.cols, vn)
testData := restoreBackupInfoMultiVersion{
service: path.OneDriveService,
resource: Users,
backupVersion: version,
countMeta: version == 0,
backupVersion: vn,
countMeta: vn == 0,
collectionsPrevious: input,
collectionsLatest: expected,
}
@ -691,17 +686,17 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
}
for _, test := range table {
expected := testDataForInfo(suite.T(), test.cols, backup.Version)
expected := testDataForInfo(suite.T(), test.cols, version.Backup)
for version := test.startVersion; version <= backup.Version; version++ {
suite.T().Run(fmt.Sprintf("%s_Version%d", test.name, version), func(t *testing.T) {
input := testDataForInfo(t, test.cols, version)
for vn := test.startVersion; vn <= version.Backup; vn++ {
suite.T().Run(fmt.Sprintf("%s_Version%d", test.name, vn), func(t *testing.T) {
input := testDataForInfo(t, test.cols, vn)
testData := restoreBackupInfoMultiVersion{
service: path.OneDriveService,
resource: Users,
backupVersion: version,
countMeta: version == 0,
backupVersion: vn,
countMeta: vn == 0,
collectionsPrevious: input,
collectionsLatest: expected,
}
@ -764,17 +759,17 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsBackupAndNoR
}
for _, test := range table {
expected := testDataForInfo(suite.T(), test.cols, backup.Version)
expected := testDataForInfo(suite.T(), test.cols, version.Backup)
for version := test.startVersion; version <= backup.Version; version++ {
suite.T().Run(fmt.Sprintf("Version%d", version), func(t *testing.T) {
input := testDataForInfo(t, test.cols, version)
for vn := test.startVersion; vn <= version.Backup; vn++ {
suite.T().Run(fmt.Sprintf("Version%d", vn), func(t *testing.T) {
input := testDataForInfo(t, test.cols, vn)
testData := restoreBackupInfoMultiVersion{
service: path.OneDriveService,
resource: Users,
backupVersion: version,
countMeta: version == 0,
backupVersion: vn,
countMeta: vn == 0,
collectionsPrevious: input,
collectionsLatest: expected,
}
@ -811,7 +806,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
test := restoreBackupInfoMultiVersion{
service: path.OneDriveService,
resource: Users,
backupVersion: backup.Version,
backupVersion: version.Backup,
countMeta: false,
collectionsPrevious: []colInfo{
newOneDriveCollection(
@ -821,7 +816,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
driveID,
"root:",
},
backup.Version,
version.Backup,
).
withFile(
fileName,
@ -843,7 +838,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
"root:",
folderBName,
},
backup.Version,
version.Backup,
).
withFile(
fileName,
@ -865,7 +860,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
driveID,
"root:",
},
backup.Version,
version.Backup,
).
withFile(
fileName,
@ -887,7 +882,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
"root:",
folderBName,
},
backup.Version,
version.Backup,
).
withFile(
fileName,

View File

@ -17,8 +17,8 @@ import (
"github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path"
@ -237,7 +237,7 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreFailsBadService() {
deets, err := suite.connector.RestoreDataCollections(
ctx,
backup.Version,
version.Backup,
acct,
sel,
dest,
@ -314,7 +314,7 @@ func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() {
deets, err := suite.connector.RestoreDataCollections(
ctx,
backup.Version,
version.Backup,
suite.acct,
test.sel,
dest,
@ -527,13 +527,13 @@ func runRestoreBackupTest(
t,
config,
test.collections,
backup.Version)
version.Backup)
runRestore(
t,
ctx,
config,
backup.Version,
version.Backup,
collections,
totalItems)
@ -590,7 +590,7 @@ func runRestoreBackupTestVersions(
t,
config,
test.collectionsLatest,
backup.Version)
version.Backup)
runBackupAndCompare(
t,
@ -930,7 +930,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames
suite.user,
dest,
[]colInfo{collection},
backup.Version,
version.Backup,
)
allItems += totalItems
@ -948,7 +948,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames
restoreGC := loadConnector(ctx, t, graph.HTTPClient(graph.NoTimeout()), test.resource)
deets, err := restoreGC.RestoreDataCollections(
ctx,
backup.Version,
version.Backup,
suite.acct,
restoreSel,
dest,

View File

@ -406,7 +406,7 @@ func (oc *Collection) populateItems(ctx context.Context) {
// TODO(meain): Remove this once we change to always
// backing up permissions. Until then we cannot rely
// on weather the previous data is what we need as the
// on whether the previous data is what we need as the
// user might have not backup up permissions in the
// previous run.
metaItemInfo := details.ItemInfo{}
@ -415,6 +415,7 @@ func (oc *Collection) populateItems(ctx context.Context) {
ItemName: itemInfo.OneDrive.ItemName,
DriveName: itemInfo.OneDrive.DriveName,
ItemType: itemInfo.OneDrive.ItemType,
IsMeta: true,
Modified: time.Now(), // set to current time to always refresh
Owner: itemInfo.OneDrive.Owner,
ParentPath: itemInfo.OneDrive.ParentPath,

View File

@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"io"
"math"
"runtime/trace"
"sort"
"strings"
@ -20,6 +19,7 @@ import (
"github.com/alcionai/corso/src/internal/data"
D "github.com/alcionai/corso/src/internal/diagnostics"
"github.com/alcionai/corso/src/internal/observe"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/fault"
@ -27,23 +27,10 @@ import (
"github.com/alcionai/corso/src/pkg/path"
)
const (
// copyBufferSize is used for chunked upload
// Microsoft recommends 5-10MB buffers
// https://docs.microsoft.com/en-us/graph/api/driveitem-createuploadsession?view=graph-rest-1.0#best-practices
copyBufferSize = 5 * 1024 * 1024
// versionWithDataAndMetaFiles is the corso backup format version
// in which we split from storing just the data to storing both
// the data and metadata in two files.
versionWithDataAndMetaFiles = 1
// versionWithNameInMeta points to the backup format version where we begin
// storing files in kopia with their item ID instead of their OneDrive file
// name.
// TODO(ashmrtn): Update this to a real value when we merge the file name
// change. Set to MAXINT for now to keep the if-check using it working.
versionWithNameInMeta = math.MaxInt
)
const copyBufferSize = 5 * 1024 * 1024
func getParentPermissions(
parentPath path.Path,
@ -288,7 +275,7 @@ func RestoreCollection(
continue
}
if source == OneDriveSource && backupVersion >= versionWithDataAndMetaFiles {
if source == OneDriveSource && backupVersion >= version.OneDrive1DataAndMetaFiles {
name := itemData.UUID()
if strings.HasSuffix(name, DataFileSuffix) {
@ -300,7 +287,7 @@ func RestoreCollection(
err error
)
if backupVersion < versionWithNameInMeta {
if backupVersion < version.OneDriveXNameInMeta {
itemInfo, err = restoreV1File(
ctx,
source,

View File

@ -27,6 +27,7 @@ import (
"github.com/alcionai/corso/src/internal/kopia"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/backup/details"
@ -341,7 +342,7 @@ func generateContainerOfItems(
deets, err := gc.RestoreDataCollections(
ctx,
backup.Version,
version.Backup,
acct,
sel,
dest,

View File

@ -0,0 +1,37 @@
package version
import "math"
const Backup = 3
// Various labels to refer to important version changes.
// Labels don't need 1:1 service:version representation. Add a new
// label when it's important to mark a delta in behavior that's handled
// somewhere in the logic.
// Labels should state their application, the backup version number,
// and the colloquial purpose of the label.
const (
// OneDrive1DataAndMetaFiles is the corso backup format version
// in which we split from storing just the data to storing both
// the data and metadata in two files.
OneDrive1DataAndMetaFiles = 1
// OneDrive3IsMetaMarker is a small improvement on
// VersionWithDataAndMetaFiles, but has a marker IsMeta which
// specifies if the file is a meta file or a data file.
OneDrive3IsMetaMarker = 3
// OneDrive4IncludesPermissions includes permissions in the backup.
// Note that this is larger than the current backup version. That's
// because it isn't implemented yet. But we have tests based on this,
// so maybe we just keep bumping the verson ahead of the backup until
// it gets implemented.
OneDriveXIncludesPermissions = Backup + 1
// OneDriveXNameInMeta points to the backup format version where we begin
// storing files in kopia with their item ID instead of their OneDrive file
// name.
// TODO(ashmrtn): Update this to a real value when we merge the file name
// change. Set to MAXINT for now to keep the if-check using it working.
OneDriveXNameInMeta = math.MaxInt
)

View File

@ -10,12 +10,11 @@ import (
"github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/internal/stats"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/selectors"
)
const Version = 2
// Backup represents the result of a backup operation
type Backup struct {
model.BaseModel
@ -72,7 +71,7 @@ func New(
Errors: errs.Data(),
ReadWrites: rw,
StartAndEndTime: se,
Version: Version,
Version: version.Backup,
}
}

View File

@ -35,10 +35,12 @@ type DetailsModel struct {
// Print writes the DetailModel Entries to StdOut, in the format
// requested by the caller.
func (dm DetailsModel) PrintEntries(ctx context.Context) {
sl := dm.FilterMetaFiles()
if print.JSONFormat() {
printJSON(ctx, dm)
printJSON(ctx, sl)
} else {
printTable(ctx, dm)
printTable(ctx, sl)
}
}
@ -71,13 +73,13 @@ func printJSON(ctx context.Context, dm DetailsModel) {
print.All(ctx, ents...)
}
// Paths returns the list of Paths for non-folder items extracted from the
// Entries slice.
// Paths returns the list of Paths for non-folder and non-meta items extracted
// from the Entries slice.
func (dm DetailsModel) Paths() []string {
r := make([]string, 0, len(dm.Entries))
for _, ent := range dm.Entries {
if ent.Folder != nil {
if ent.Folder != nil || ent.isMetaFile() {
continue
}
@ -89,21 +91,49 @@ func (dm DetailsModel) Paths() []string {
// Items returns a slice of *ItemInfo that does not contain any FolderInfo
// entries. Required because not all folders in the details are valid resource
// paths.
// paths, and we want to slice out metadata.
func (dm DetailsModel) Items() []*DetailsEntry {
res := make([]*DetailsEntry, 0, len(dm.Entries))
for i := 0; i < len(dm.Entries); i++ {
if dm.Entries[i].Folder != nil {
ent := dm.Entries[i]
if ent.Folder != nil || ent.isMetaFile() {
continue
}
res = append(res, &dm.Entries[i])
res = append(res, &ent)
}
return res
}
// FilterMetaFiles returns a copy of the Details with all of the
// .meta files removed from the entries.
func (dm DetailsModel) FilterMetaFiles() DetailsModel {
d2 := DetailsModel{
Entries: []DetailsEntry{},
}
for _, ent := range dm.Entries {
if !ent.isMetaFile() {
d2.Entries = append(d2.Entries, ent)
}
}
return d2
}
// Check if a file is a metadata file. These are used to store
// additional data like permissions in case of OneDrive and are not to
// be treated as regular files.
func (de DetailsEntry) isMetaFile() bool {
return de.ItemInfo.OneDrive != nil && de.ItemInfo.OneDrive.IsMeta
}
// ---------------------------------------------------------------------------
// Builder
// ---------------------------------------------------------------------------
// Builder should be used to create a details model.
type Builder struct {
d Details
@ -583,6 +613,7 @@ type OneDriveInfo struct {
ItemName string `json:"itemName,omitempty"`
DriveName string `json:"driveName,omitempty"`
ItemType ItemType `json:"itemType,omitempty"`
IsMeta bool `json:"isMeta,omitempty"`
Modified time.Time `json:"modified,omitempty"`
Owner string `json:"owner,omitempty"`
ParentPath string `json:"parentPath"`

View File

@ -224,6 +224,69 @@ var pathItemsTable = []struct {
expectRepoRefs: []string{"abcde", "12345"},
expectLocationRefs: []string{"locationref", "locationref2"},
},
{
name: "multiple entries with meta file",
ents: []DetailsEntry{
{
RepoRef: "abcde",
LocationRef: "locationref",
},
{
RepoRef: "foo.meta",
LocationRef: "locationref.dirmeta",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: false},
},
},
{
RepoRef: "is-meta-file",
LocationRef: "locationref-meta-file",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: true},
},
},
},
expectRepoRefs: []string{"abcde", "foo.meta"},
expectLocationRefs: []string{"locationref", "locationref.dirmeta"},
},
{
name: "multiple entries with folder and meta file",
ents: []DetailsEntry{
{
RepoRef: "abcde",
LocationRef: "locationref",
},
{
RepoRef: "12345",
LocationRef: "locationref2",
},
{
RepoRef: "foo.meta",
LocationRef: "locationref.dirmeta",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: false},
},
},
{
RepoRef: "is-meta-file",
LocationRef: "locationref-meta-file",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: true},
},
},
{
RepoRef: "deadbeef",
LocationRef: "locationref3",
ItemInfo: ItemInfo{
Folder: &FolderInfo{
DisplayName: "test folder",
},
},
},
},
expectRepoRefs: []string{"abcde", "12345", "foo.meta"},
expectLocationRefs: []string{"locationref", "locationref2", "locationref.dirmeta"},
},
}
func (suite *DetailsUnitSuite) TestDetailsModel_Path() {
@ -259,6 +322,38 @@ func (suite *DetailsUnitSuite) TestDetailsModel_Items() {
}
}
func (suite *DetailsUnitSuite) TestDetailsModel_FilterMetaFiles() {
t := suite.T()
d := &DetailsModel{
Entries: []DetailsEntry{
{
RepoRef: "a.data",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: false},
},
},
{
RepoRef: "b.meta",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: false},
},
},
{
RepoRef: "c.meta",
ItemInfo: ItemInfo{
OneDrive: &OneDriveInfo{IsMeta: true},
},
},
},
}
d2 := d.FilterMetaFiles()
assert.Len(t, d2.Entries, 2)
assert.Len(t, d.Entries, 3)
}
func (suite *DetailsUnitSuite) TestDetails_AddFolders() {
itemTime := time.Date(2022, 10, 21, 10, 0, 0, 0, time.UTC)
folderTimeOlderThanItem := time.Date(2022, 9, 21, 10, 0, 0, 0, time.UTC)

View File

@ -2,6 +2,7 @@ package repository
import (
"context"
"strings"
"time"
"github.com/alcionai/clues"
@ -9,12 +10,14 @@ import (
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common/crash"
"github.com/alcionai/corso/src/internal/connector/onedrive"
"github.com/alcionai/corso/src/internal/events"
"github.com/alcionai/corso/src/internal/kopia"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/internal/observe"
"github.com/alcionai/corso/src/internal/operations"
"github.com/alcionai/corso/src/internal/streamstore"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/backup/details"
@ -358,6 +361,20 @@ func (r repository) BackupDetails(
return nil, nil, errs.Fail(err)
}
// Retroactively fill in isMeta information for items in older
// backup versions without that info
// version.Restore2 introduces the IsMeta flag, so only v1 needs a check.
if b.Version >= version.OneDrive1DataAndMetaFiles && b.Version < version.OneDrive3IsMetaMarker {
for _, d := range deets.Entries {
if d.OneDrive != nil {
if strings.HasSuffix(d.RepoRef, onedrive.MetaFileSuffix) ||
strings.HasSuffix(d.RepoRef, onedrive.DirMetaFileSuffix) {
d.OneDrive.IsMeta = true
}
}
}
}
return deets, b, errs
}