GC: Ptr clean-up and Invalid NextLink logging (#2547)
## Description - Use of ptr package for next link and delta links. - Added logging to verify the presence of receiving invalid next links from M365. <!-- Insert PR description--> ## Does this PR need a docs update or release note? - [x] ⛔ No - Related to #2520 ## Type of change - [x] 🧹 Tech Debt/Cleanup ## Test Plan <!-- How will this be tested prior to merging.--> - [x] ⚡ Unit test
This commit is contained in:
parent
d395311821
commit
62d652764a
@ -3,6 +3,7 @@ package api
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"os"
|
||||||
|
|
||||||
"github.com/alcionai/clues"
|
"github.com/alcionai/clues"
|
||||||
|
|
||||||
@ -104,6 +105,11 @@ func getItemsAddedAndRemovedFromContainer(
|
|||||||
}
|
}
|
||||||
|
|
||||||
nextLink, delta := api.NextAndDeltaLink(resp)
|
nextLink, delta := api.NextAndDeltaLink(resp)
|
||||||
|
if len(os.Getenv("CORSO_URL_LOGGING")) > 0 {
|
||||||
|
if !api.IsNextLinkValid(nextLink) || api.IsNextLinkValid(delta) {
|
||||||
|
logger.Ctx(ctx).Infof("Received invalid link from M365:\nNext Link: %s\nDelta Link: %s\n", nextLink, delta)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// the deltaLink is kind of like a cursor for overall data state.
|
// the deltaLink is kind of like a cursor for overall data state.
|
||||||
// once we run through pages of nextLinks, the last query will
|
// once we run through pages of nextLinks, the last query will
|
||||||
|
|||||||
@ -1,6 +1,10 @@
|
|||||||
package api
|
package api
|
||||||
|
|
||||||
import "github.com/alcionai/corso/src/internal/common/ptr"
|
import (
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"github.com/alcionai/corso/src/internal/common/ptr"
|
||||||
|
)
|
||||||
|
|
||||||
type PageLinker interface {
|
type PageLinker interface {
|
||||||
GetOdataNextLink() *string
|
GetOdataNextLink() *string
|
||||||
@ -11,6 +15,11 @@ type DeltaPageLinker interface {
|
|||||||
GetOdataDeltaLink() *string
|
GetOdataDeltaLink() *string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// IsNextLinkValid separate check to investigate whether error is
|
||||||
|
func IsNextLinkValid(next string) bool {
|
||||||
|
return !strings.Contains(next, `users//`)
|
||||||
|
}
|
||||||
|
|
||||||
func NextLink(pl PageLinker) string {
|
func NextLink(pl PageLinker) string {
|
||||||
return ptr.Val(pl.GetOdataNextLink())
|
return ptr.Val(pl.GetOdataNextLink())
|
||||||
}
|
}
|
||||||
|
|||||||
@ -8,6 +8,7 @@ import (
|
|||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
|
|
||||||
"github.com/alcionai/corso/src/internal/connector/graph/api"
|
"github.com/alcionai/corso/src/internal/connector/graph/api"
|
||||||
|
"github.com/alcionai/corso/src/internal/tester"
|
||||||
)
|
)
|
||||||
|
|
||||||
type mockNextLink struct {
|
type mockNextLink struct {
|
||||||
@ -59,20 +60,14 @@ var (
|
|||||||
)
|
)
|
||||||
|
|
||||||
type APIUnitSuite struct {
|
type APIUnitSuite struct {
|
||||||
suite.Suite
|
tester.Suite
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAPIUnitSuite(t *testing.T) {
|
func TestAPIUnitSuite(t *testing.T) {
|
||||||
suite.Run(t, new(APIUnitSuite))
|
s := &APIUnitSuite{
|
||||||
}
|
Suite: tester.NewUnitSuite(t),
|
||||||
|
|
||||||
func (suite *APIUnitSuite) TestNextLink() {
|
|
||||||
for _, test := range nextLinkInputs {
|
|
||||||
suite.T().Run(test.name, func(t *testing.T) {
|
|
||||||
l := mockNextLink{nextLink: test.inputLink}
|
|
||||||
assert.Equal(t, test.expectedLink, api.NextLink(l))
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
suite.Run(t, s)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *APIUnitSuite) TestNextAndDeltaLink() {
|
func (suite *APIUnitSuite) TestNextAndDeltaLink() {
|
||||||
@ -112,3 +107,39 @@ func (suite *APIUnitSuite) TestNextAndDeltaLink() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestIsLinkValid check to verify is nextLink guard check for logging
|
||||||
|
// Related to: https://github.com/alcionai/corso/issues/2520
|
||||||
|
//
|
||||||
|
//nolint:lll
|
||||||
|
func (suite *APIUnitSuite) TestIsLinkValid() {
|
||||||
|
invalidString := `https://graph.microsoft.com/v1.0/users//mailFolders//messages/microsoft.graph.delta()?$select=id%2CisRead`
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
inputString string
|
||||||
|
isValid assert.BoolAssertionFunc
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "Empty",
|
||||||
|
inputString: emptyLink,
|
||||||
|
isValid: assert.True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Invalid",
|
||||||
|
inputString: invalidString,
|
||||||
|
isValid: assert.False,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Valid",
|
||||||
|
inputString: `https://graph.microsoft.com/v1.0/users/aPerson/mailFolders/AMessage/messages/microsoft.graph.delta()?$select=id%2CisRead`,
|
||||||
|
isValid: assert.True,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, test := range tests {
|
||||||
|
suite.T().Run(test.name, func(t *testing.T) {
|
||||||
|
got := api.IsNextLinkValid(test.inputString)
|
||||||
|
test.isValid(t, got)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@ -12,6 +12,7 @@ import (
|
|||||||
"github.com/pkg/errors"
|
"github.com/pkg/errors"
|
||||||
"golang.org/x/exp/maps"
|
"golang.org/x/exp/maps"
|
||||||
|
|
||||||
|
"github.com/alcionai/corso/src/internal/common/ptr"
|
||||||
"github.com/alcionai/corso/src/internal/connector/graph"
|
"github.com/alcionai/corso/src/internal/connector/graph"
|
||||||
gapi "github.com/alcionai/corso/src/internal/connector/graph/api"
|
gapi "github.com/alcionai/corso/src/internal/connector/graph/api"
|
||||||
"github.com/alcionai/corso/src/internal/connector/onedrive/api"
|
"github.com/alcionai/corso/src/internal/connector/onedrive/api"
|
||||||
@ -126,7 +127,7 @@ func drives(
|
|||||||
|
|
||||||
drives = append(drives, tmp...)
|
drives = append(drives, tmp...)
|
||||||
|
|
||||||
nextLink := gapi.NextLink(page)
|
nextLink := ptr.Val(page.GetOdataNextLink())
|
||||||
if len(nextLink) == 0 {
|
if len(nextLink) == 0 {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user