Refactor downloadItem (#3534)
This refactor is being done to allow calling `downloadFile` with an URL obtained from URL cache. --- #### 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 - [ ] 🐛 Bugfix - [ ] 🗺️ Documentation - [ ] 🤖 Supportability/Tests - [ ] 💻 CI/Deployment - [x] 🧹 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.--> - [ ] 💪 Manual - [x] ⚡ Unit test - [ ] 💚 E2E
This commit is contained in:
Abhishek Pandey
GitHub
parent
cbbc8d2f6c
commit
29ef5e4b5d
@ -27,9 +27,14 @@ func downloadItem(
|
||||
ag api.Getter,
|
||||
item models.DriveItemable,
|
||||
) (io.ReadCloser, error) {
|
||||
if item == nil {
|
||||
return nil, clues.New("nil item")
|
||||
}
|
||||
|
||||
var (
|
||||
rc io.ReadCloser
|
||||
isFile = item.GetFile() != nil
|
||||
err error
|
||||
)
|
||||
|
||||
if isFile {
|
||||
@ -45,33 +50,44 @@ func downloadItem(
|
||||
}
|
||||
}
|
||||
|
||||
if len(url) == 0 {
|
||||
return nil, clues.New("extracting file url")
|
||||
}
|
||||
|
||||
resp, err := ag.Get(ctx, url, nil)
|
||||
rc, err = downloadFile(ctx, ag, url)
|
||||
if err != nil {
|
||||
return nil, clues.Wrap(err, "getting item")
|
||||
return nil, clues.Stack(err)
|
||||
}
|
||||
|
||||
if graph.IsMalwareResp(ctx, resp) {
|
||||
return nil, clues.New("malware detected").Label(graph.LabelsMalware)
|
||||
}
|
||||
|
||||
if (resp.StatusCode / 100) != 2 {
|
||||
// upstream error checks can compare the status with
|
||||
// clues.HasLabel(err, graph.LabelStatus(http.KnownStatusCode))
|
||||
return nil, clues.
|
||||
Wrap(clues.New(resp.Status), "non-2xx http response").
|
||||
Label(graph.LabelStatus(resp.StatusCode))
|
||||
}
|
||||
|
||||
rc = resp.Body
|
||||
}
|
||||
|
||||
return rc, nil
|
||||
}
|
||||
|
||||
func downloadFile(
|
||||
ctx context.Context,
|
||||
ag api.Getter,
|
||||
url string,
|
||||
) (io.ReadCloser, error) {
|
||||
if len(url) == 0 {
|
||||
return nil, clues.New("empty file url")
|
||||
}
|
||||
|
||||
resp, err := ag.Get(ctx, url, nil)
|
||||
if err != nil {
|
||||
return nil, clues.Wrap(err, "getting file")
|
||||
}
|
||||
|
||||
if graph.IsMalwareResp(ctx, resp) {
|
||||
return nil, clues.New("malware detected").Label(graph.LabelsMalware)
|
||||
}
|
||||
|
||||
if (resp.StatusCode / 100) != 2 {
|
||||
// upstream error checks can compare the status with
|
||||
// clues.HasLabel(err, graph.LabelStatus(http.KnownStatusCode))
|
||||
return nil, clues.
|
||||
Wrap(clues.New(resp.Status), "non-2xx http response").
|
||||
Label(graph.LabelStatus(resp.StatusCode))
|
||||
}
|
||||
|
||||
return resp.Body, nil
|
||||
}
|
||||
|
||||
func downloadItemMeta(
|
||||
ctx context.Context,
|
||||
gip GetItemPermissioner,
|
||||
|
||||
@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/alcionai/clues"
|
||||
@ -256,3 +257,176 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Unit tests
|
||||
|
||||
type mockGetter struct {
|
||||
GetFunc func(ctx context.Context, url string) (*http.Response, error)
|
||||
}
|
||||
|
||||
func (m mockGetter) Get(
|
||||
ctx context.Context,
|
||||
url string,
|
||||
headers map[string]string,
|
||||
) (*http.Response, error) {
|
||||
return m.GetFunc(ctx, url)
|
||||
}
|
||||
|
||||
type ItemUnitTestSuite struct {
|
||||
tester.Suite
|
||||
}
|
||||
|
||||
func TestItemUnitTestSuite(t *testing.T) {
|
||||
suite.Run(t, &ItemUnitTestSuite{Suite: tester.NewUnitSuite(t)})
|
||||
}
|
||||
|
||||
func (suite *ItemUnitTestSuite) TestDownloadItem() {
|
||||
testRc := io.NopCloser(bytes.NewReader([]byte("test")))
|
||||
url := "https://example.com"
|
||||
|
||||
table := []struct {
|
||||
name string
|
||||
itemFunc func() models.DriveItemable
|
||||
GetFunc func(ctx context.Context, url string) (*http.Response, error)
|
||||
errorExpected require.ErrorAssertionFunc
|
||||
rcExpected require.ValueAssertionFunc
|
||||
label string
|
||||
}{
|
||||
{
|
||||
name: "nil item",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
return nil
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return nil, nil
|
||||
},
|
||||
errorExpected: require.Error,
|
||||
rcExpected: require.Nil,
|
||||
},
|
||||
{
|
||||
name: "success",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
di.SetAdditionalData(map[string]interface{}{
|
||||
"@microsoft.graph.downloadUrl": url,
|
||||
})
|
||||
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Body: testRc,
|
||||
}, nil
|
||||
},
|
||||
errorExpected: require.NoError,
|
||||
rcExpected: require.NotNil,
|
||||
},
|
||||
{
|
||||
name: "success, content url set instead of download url",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
di.SetAdditionalData(map[string]interface{}{
|
||||
"@content.downloadUrl": url,
|
||||
})
|
||||
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Body: testRc,
|
||||
}, nil
|
||||
},
|
||||
errorExpected: require.NoError,
|
||||
rcExpected: require.NotNil,
|
||||
},
|
||||
{
|
||||
name: "api getter returns error",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
di.SetAdditionalData(map[string]interface{}{
|
||||
"@microsoft.graph.downloadUrl": url,
|
||||
})
|
||||
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return nil, clues.New("test error")
|
||||
},
|
||||
errorExpected: require.Error,
|
||||
rcExpected: require.Nil,
|
||||
},
|
||||
{
|
||||
name: "download url is empty",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Body: testRc,
|
||||
}, nil
|
||||
},
|
||||
errorExpected: require.Error,
|
||||
rcExpected: require.Nil,
|
||||
},
|
||||
{
|
||||
name: "malware",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
di.SetAdditionalData(map[string]interface{}{
|
||||
"@microsoft.graph.downloadUrl": url,
|
||||
})
|
||||
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
Header: http.Header{
|
||||
"X-Virus-Infected": []string{"true"},
|
||||
},
|
||||
StatusCode: http.StatusOK,
|
||||
Body: testRc,
|
||||
}, nil
|
||||
},
|
||||
errorExpected: require.Error,
|
||||
rcExpected: require.Nil,
|
||||
},
|
||||
{
|
||||
name: "non-2xx http response",
|
||||
itemFunc: func() models.DriveItemable {
|
||||
di := newItem("test", false)
|
||||
di.SetAdditionalData(map[string]interface{}{
|
||||
"@microsoft.graph.downloadUrl": url,
|
||||
})
|
||||
|
||||
return di
|
||||
},
|
||||
GetFunc: func(ctx context.Context, url string) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusNotFound,
|
||||
Body: nil,
|
||||
}, nil
|
||||
},
|
||||
errorExpected: require.Error,
|
||||
rcExpected: require.Nil,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range table {
|
||||
suite.Run(test.name, func() {
|
||||
t := suite.T()
|
||||
ctx, flush := tester.NewContext(t)
|
||||
defer flush()
|
||||
|
||||
mg := mockGetter{
|
||||
GetFunc: test.GetFunc,
|
||||
}
|
||||
rc, err := downloadItem(ctx, mg, test.itemFunc())
|
||||
test.errorExpected(t, err, clues.ToCore(err))
|
||||
test.rcExpected(t, rc)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user