Allow delta enum callers to specify $select properties (#4460)
<!-- PR description--> This fixes a perf regression in #4456 Context: URL cache only needs a subset of drive item properties while doing delta queries. See https://github.com/alcionai/corso/pull/4074 for details. Changes in #4456 were applying the default item property set for all delta enumerator consumers including URL cache. This PR fixes the memory regression. --- #### Does this PR need a docs update or release note? - [ ] ✅ Yes, it's included - [ ] 🕐 Yes, but in a later PR - [x] ⛔ No #### Type of change <!--- Please check the type of change your PR introduces: ---> - [ ] 🌻 Feature - [x] 🐛 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. --> * #<issue> #### Test Plan <!-- How will this be tested prior to merging.--> - [x] 💪 Manual - [ ] ⚡ Unit test - [ ] 💚 E2E
This commit is contained in:
parent
7196b5d278
commit
3656e04676
@ -298,7 +298,8 @@ func (c *Collections) Get(
|
||||
items, du, err := c.handler.EnumerateDriveItemsDelta(
|
||||
ictx,
|
||||
driveID,
|
||||
prevDeltaLink)
|
||||
prevDeltaLink,
|
||||
api.DefaultDriveItemProps())
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
|
||||
@ -86,6 +86,7 @@ type EnumerateDriveItemsDeltaer interface {
|
||||
EnumerateDriveItemsDelta(
|
||||
ctx context.Context,
|
||||
driveID, prevDeltaLink string,
|
||||
selectProps []string,
|
||||
) (
|
||||
[]models.DriveItemable,
|
||||
api.DeltaUpdate,
|
||||
|
||||
@ -137,8 +137,9 @@ func (h itemBackupHandler) IncludesDir(dir string) bool {
|
||||
func (h itemBackupHandler) EnumerateDriveItemsDelta(
|
||||
ctx context.Context,
|
||||
driveID, prevDeltaLink string,
|
||||
selectProps []string,
|
||||
) ([]models.DriveItemable, api.DeltaUpdate, error) {
|
||||
return h.ac.EnumerateDriveItemsDelta(ctx, driveID, prevDeltaLink)
|
||||
return h.ac.EnumerateDriveItemsDelta(ctx, driveID, prevDeltaLink, selectProps)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@ -140,8 +140,9 @@ func (h libraryBackupHandler) IncludesDir(dir string) bool {
|
||||
func (h libraryBackupHandler) EnumerateDriveItemsDelta(
|
||||
ctx context.Context,
|
||||
driveID, prevDeltaLink string,
|
||||
selectProps []string,
|
||||
) ([]models.DriveItemable, api.DeltaUpdate, error) {
|
||||
return h.ac.EnumerateDriveItemsDelta(ctx, driveID, prevDeltaLink)
|
||||
return h.ac.EnumerateDriveItemsDelta(ctx, driveID, prevDeltaLink, selectProps)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@ -12,6 +12,7 @@ import (
|
||||
"github.com/alcionai/corso/src/internal/common/str"
|
||||
"github.com/alcionai/corso/src/pkg/fault"
|
||||
"github.com/alcionai/corso/src/pkg/logger"
|
||||
"github.com/alcionai/corso/src/pkg/services/m365/api"
|
||||
)
|
||||
|
||||
const (
|
||||
@ -156,7 +157,11 @@ func (uc *urlCache) refreshCache(
|
||||
// Issue a delta query to graph
|
||||
logger.Ctx(ctx).Info("refreshing url cache")
|
||||
|
||||
items, du, err := uc.edid.EnumerateDriveItemsDelta(ctx, uc.driveID, uc.prevDelta)
|
||||
items, du, err := uc.edid.EnumerateDriveItemsDelta(
|
||||
ctx,
|
||||
uc.driveID,
|
||||
uc.prevDelta,
|
||||
api.URLCacheDriveItemProps())
|
||||
if err != nil {
|
||||
uc.idToProps = make(map[string]itemProps)
|
||||
return clues.Stack(err)
|
||||
|
||||
@ -97,7 +97,11 @@ func (suite *URLCacheIntegrationSuite) TestURLCacheBasic() {
|
||||
nfid := ptr.Val(newFolder.GetId())
|
||||
|
||||
// Get the previous delta to feed into url cache
|
||||
_, du, err := ac.EnumerateDriveItemsDelta(ctx, suite.driveID, "")
|
||||
_, du, err := ac.EnumerateDriveItemsDelta(
|
||||
ctx,
|
||||
suite.driveID,
|
||||
"",
|
||||
api.URLCacheDriveItemProps())
|
||||
require.NoError(t, err, clues.ToCore(err))
|
||||
require.NotEmpty(t, du.URL)
|
||||
|
||||
|
||||
@ -163,8 +163,13 @@ func (h *BackupHandler) Get(context.Context, string, map[string]string) (*http.R
|
||||
func (h BackupHandler) EnumerateDriveItemsDelta(
|
||||
ctx context.Context,
|
||||
driveID, prevDeltaLink string,
|
||||
selectProps []string,
|
||||
) ([]models.DriveItemable, api.DeltaUpdate, error) {
|
||||
return h.DriveItemEnumeration.EnumerateDriveItemsDelta(ctx, driveID, prevDeltaLink)
|
||||
return h.DriveItemEnumeration.EnumerateDriveItemsDelta(
|
||||
ctx,
|
||||
driveID,
|
||||
prevDeltaLink,
|
||||
selectProps)
|
||||
}
|
||||
|
||||
func (h BackupHandler) GetItem(ctx context.Context, _, _ string) (models.DriveItemable, error) {
|
||||
@ -282,6 +287,7 @@ type EnumeratesDriveItemsDelta struct {
|
||||
func (edi EnumeratesDriveItemsDelta) EnumerateDriveItemsDelta(
|
||||
_ context.Context,
|
||||
driveID, _ string,
|
||||
_ []string,
|
||||
) (
|
||||
[]models.DriveItemable,
|
||||
api.DeltaUpdate,
|
||||
|
||||
@ -120,8 +120,8 @@ func DefaultDriveItemProps() []string {
|
||||
"shared")
|
||||
}
|
||||
|
||||
// URL cache only needs a subset of item properties
|
||||
func DriveItemSelectURLCache() []string {
|
||||
// URL cache only needs to fetch a small subset of item properties
|
||||
func URLCacheDriveItemProps() []string {
|
||||
return idAnd(
|
||||
"content.downloadUrl",
|
||||
"deleted",
|
||||
|
||||
@ -203,12 +203,13 @@ func (c Drives) EnumerateDriveItemsDelta(
|
||||
ctx context.Context,
|
||||
driveID string,
|
||||
prevDeltaLink string,
|
||||
selectProps []string,
|
||||
) (
|
||||
[]models.DriveItemable,
|
||||
DeltaUpdate,
|
||||
error,
|
||||
) {
|
||||
pager := c.newDriveItemDeltaPager(driveID, prevDeltaLink, DefaultDriveItemProps()...)
|
||||
pager := c.newDriveItemDeltaPager(driveID, prevDeltaLink, selectProps...)
|
||||
|
||||
items, du, err := deltaEnumerateItems[models.DriveItemable](ctx, pager, prevDeltaLink)
|
||||
if err != nil {
|
||||
|
||||
@ -188,7 +188,7 @@ func (suite *DrivePagerIntgSuite) TestEnumerateDriveItems() {
|
||||
items, du, err := suite.its.
|
||||
ac.
|
||||
Drives().
|
||||
EnumerateDriveItemsDelta(ctx, suite.its.user.driveID, "")
|
||||
EnumerateDriveItemsDelta(ctx, suite.its.user.driveID, "", api.DefaultDriveItemProps())
|
||||
require.NoError(t, err, clues.ToCore(err))
|
||||
require.NotEmpty(t, items, "no items found in user's drive")
|
||||
assert.NotEmpty(t, du.URL, "should have a delta link")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user