From 8a639ee0c70272b93c6d253da8c3a9b89c74c574 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 1 Aug 2023 15:36:34 +0100 Subject: [PATCH 1/6] Base Client: support custom paging --- sdk/client/client.go | 55 +++++++++------ sdk/client/client_test.go | 100 ++++++++++++++++++++++++++- sdk/client/request_options.go | 7 +- sdk/client/resourcemanager/client.go | 1 + sdk/odata/http.go | 2 +- sdk/odata/odata.go | 4 +- sdk/odata/paging.go | 33 +++++++++ 7 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 sdk/odata/paging.go diff --git a/sdk/client/client.go b/sdk/client/client.go index 01523e054ad..dd3d5a3779a 100644 --- a/sdk/client/client.go +++ b/sdk/client/client.go @@ -80,6 +80,7 @@ type Request struct { ValidStatusFunc ValidStatusFunc Client BaseClient + Pager odata.Pager // Embed *http.Request so that we can send this to an *http.Client *http.Request @@ -302,6 +303,7 @@ func (c *Client) NewRequest(ctx context.Context, input RequestOptions) (*Request ret := Request{ Client: c, Request: req, + Pager: input.Pager, ValidStatusCodes: input.ExpectedStatusCodes, } @@ -456,50 +458,59 @@ func (c *Client) ExecutePaged(ctx context.Context, req *Request) (*Response, err return resp, fmt.Errorf("unsupported content-type %q received, only application/json is supported for paged results", contentType) } - // Read the response body and close it - respBody, err := io.ReadAll(resp.Body) + // Unmarshal the response + firstOdata, err := odata.FromResponse(resp.Response) if err != nil { - return resp, fmt.Errorf("could not parse response body") + return resp, err } - resp.Body.Close() - // Unmarshal firstOdata - var firstOdata odata.OData - if err := json.Unmarshal(respBody, &firstOdata); err != nil { - return resp, err + if firstOdata == nil { + // No results, return early + return resp, nil } + // Get results from this page firstValue, ok := firstOdata.Value.([]interface{}) - if firstOdata.NextLink == nil || firstValue == nil || !ok { - // No more pages, reassign response body and return - resp.Body = io.NopCloser(bytes.NewBuffer(respBody)) + if !ok || firstValue == nil { + // No more results on this page + return resp, nil + } + + // Get a Link for the next results page + var nextLink *odata.Link + if req.Pager == nil { + nextLink = firstOdata.NextLink + } else { + nextLink, err = odata.NextLinkFromPager(resp.Response, req.Pager) + } + if nextLink == nil { + // This is the last page return resp, nil } - // Get the next page, recursively - // TODO: may have to accommodate APIs with nonstandard paging + // Build request for the next page nextReq := req - u, err := url.Parse(string(*firstOdata.NextLink)) + u, err := url.Parse(string(*nextLink)) if err != nil { return resp, err } nextReq.URL = u + + // Retrieve the next page, descend recursively nextResp, err := c.ExecutePaged(ctx, req) if err != nil { return resp, err } - // Read the next page response body and close it - nextRespBody, err := io.ReadAll(nextResp.Body) + // Unmarshal nextOdata from the next page + nextOdata, err := odata.FromResponse(nextResp.Response) if err != nil { - return resp, fmt.Errorf("could not parse response body") + return nextResp, err } - nextResp.Body.Close() - // Unmarshal nextOdata from the next page - var nextOdata odata.OData - if err := json.Unmarshal(nextRespBody, &nextOdata); err != nil { - return nextResp, err + if nextOdata == nil { + // No more results, return early + return resp, nil } // When next page has results, append to current page diff --git a/sdk/client/client_test.go b/sdk/client/client_test.go index 6c927253954..77036c98995 100644 --- a/sdk/client/client_test.go +++ b/sdk/client/client_test.go @@ -10,12 +10,14 @@ import ( "encoding/xml" "fmt" "io" + "log" "net/http" "reflect" "testing" "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-sdk/sdk/internal/test" + "github.com/hashicorp/go-azure-sdk/sdk/odata" ) func TestAccClient(t *testing.T) { @@ -48,10 +50,106 @@ func TestAccClient(t *testing.T) { t.Fatal(err) } - _, err = req.ExecutePaged(ctx) + resp, err := req.Execute(ctx) + if err != nil { + t.Fatalf("Execute(): %v", err) + } + + fmt.Printf("%#v", resp) +} + +func TestAccClient_Paged(t *testing.T) { + test.AccTest(t) + + ctx := context.TODO() + conn := test.NewConnection(t) + api := conn.AuthConfig.Environment.MicrosoftGraph + endpoint, ok := api.Endpoint() + if !ok { + t.Fatalf("missing endpoint for microsoft graph for this environment") + } + conn.Authorize(ctx, t, api) + + c := NewClient(*endpoint, "example", "2020-01-01") + c.Authorizer = conn.Authorizer + + path := "/v1.0/servicePrincipals" + reqOpts := RequestOptions{ + ContentType: "application/json", + ExpectedStatusCodes: []int{ + http.StatusOK, + }, + HttpMethod: http.MethodGet, + OptionsObject: nil, + Path: path, + } + req, err := c.NewRequest(ctx, reqOpts) + if err != nil { + t.Fatal(err) + } + + resp, err := req.ExecutePaged(ctx) if err != nil { t.Fatalf("ExecutePaged(): %v", err) } + + fmt.Printf("%#v", resp) +} + +var _ odata.Pager = &pager{} + +type pager struct { + NextLink *odata.Link `json:"@odata.nextLink"` +} + +func (p *pager) NextPageLink() *odata.Link { + if p == nil { + log.Fatalf("pager: p was nil") + } + if p.NextLink == nil { + log.Fatalf("pager: nextLink was nil") + } + log.Printf("[DEBUG] pager: found custom nextLink %q", *p.NextLink) + return p.NextLink +} + +func TestAccClient_CustomPaged(t *testing.T) { + test.AccTest(t) + + ctx := context.TODO() + conn := test.NewConnection(t) + api := conn.AuthConfig.Environment.MicrosoftGraph + endpoint, ok := api.Endpoint() + if !ok { + t.Fatalf("missing endpoint for microsoft graph for this environment") + } + conn.Authorize(ctx, t, api) + + c := NewClient(*endpoint, "example", "2020-01-01") + c.Authorizer = conn.Authorizer + + path := "/v1.0/applications" + reqOpts := RequestOptions{ + ContentType: "application/json", + ExpectedStatusCodes: []int{ + http.StatusOK, + }, + HttpMethod: http.MethodGet, + OptionsObject: nil, + Pager: &pager{}, + Path: path, + } + req, err := c.NewRequest(ctx, reqOpts) + if err != nil { + t.Fatal(err) + } + + resp, err := req.ExecutePaged(ctx) + if err != nil { + t.Fatalf("ExecutePaged(): %v", err) + } + + fmt.Printf("%#v", resp) } func TestMarshalByteStreamAndPowerShell(t *testing.T) { diff --git a/sdk/client/request_options.go b/sdk/client/request_options.go index 015f95f5b59..0f2f52ae0cc 100644 --- a/sdk/client/request_options.go +++ b/sdk/client/request_options.go @@ -3,13 +3,18 @@ package client -import "fmt" +import ( + "fmt" + + "github.com/hashicorp/go-azure-sdk/sdk/odata" +) type RequestOptions struct { ContentType string ExpectedStatusCodes []int HttpMethod string OptionsObject Options + Pager odata.Pager Path string } diff --git a/sdk/client/resourcemanager/client.go b/sdk/client/resourcemanager/client.go index 7735b0aa5d2..16c4b7d8be9 100644 --- a/sdk/client/resourcemanager/client.go +++ b/sdk/client/resourcemanager/client.go @@ -78,6 +78,7 @@ func (c *Client) NewRequest(ctx context.Context, input client.RequestOptions) (* } req.URL.RawQuery = query.Encode() + req.Pager = input.Pager req.RetryFunc = client.RequestRetryAny(defaultRetryFunctions...) req.ValidStatusCodes = input.ExpectedStatusCodes diff --git a/sdk/odata/http.go b/sdk/odata/http.go index 4bd32a87b42..c5a129199a6 100644 --- a/sdk/odata/http.go +++ b/sdk/odata/http.go @@ -12,7 +12,7 @@ import ( "strings" ) -// FromResponse parses an http.Response and returns an unmarshaled OData +// FromResponse parses a http.Response and returns an unmarshalled OData // If no odata is present in the response, or the content type is invalid, returns nil func FromResponse(resp *http.Response) (*OData, error) { if resp == nil { diff --git a/sdk/odata/odata.go b/sdk/odata/odata.go index 9b26d87a8d7..cc41f76aed4 100644 --- a/sdk/odata/odata.go +++ b/sdk/odata/odata.go @@ -84,7 +84,7 @@ func (l *Link) UnmarshalJSON(data []byte) error { return nil } -// OData is used to unmarshall OData metadata from an API response. +// OData is used to unmarshal OData metadata from an API response. type OData struct { Context *string `json:"@odata.context"` MetadataEtag *string `json:"@odata.metadataEtag"` @@ -116,6 +116,7 @@ func (o *OData) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(data, &e); err != nil { return err } + for _, k := range []string{"error", "odata.error"} { if v, ok := e[k]; ok { var e2 Error @@ -126,5 +127,6 @@ func (o *OData) UnmarshalJSON(data []byte) error { break } } + return nil } diff --git a/sdk/odata/paging.go b/sdk/odata/paging.go new file mode 100644 index 00000000000..c7640c40dd2 --- /dev/null +++ b/sdk/odata/paging.go @@ -0,0 +1,33 @@ +package odata + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" +) + +type Pager interface { + NextPageLink() *Link +} + +func NextLinkFromPager(resp *http.Response, pager Pager) (*Link, error) { + // Read the response body and close it + respBody, err := io.ReadAll(resp.Body) + resp.Body.Close() + + // Always reassign the response body + resp.Body = io.NopCloser(bytes.NewBuffer(respBody)) + + if err != nil { + return nil, fmt.Errorf("could not read response body: %s", err) + } + + // Unmarshal + if err := json.Unmarshal(respBody, &pager); err != nil { + return nil, err + } + + return pager.NextPageLink(), nil +} From 44424926a2f2a6cc67cf85217313727e862b1a92 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 1 Aug 2023 15:52:08 +0100 Subject: [PATCH 2/6] Base Client: document exported methods and fields --- sdk/client/client.go | 5 +++++ sdk/client/interface.go | 5 +++++ sdk/client/options.go | 5 +++++ sdk/client/request_options.go | 22 +++++++++++++++++----- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/sdk/client/client.go b/sdk/client/client.go index dd3d5a3779a..ce98894d9ba 100644 --- a/sdk/client/client.go +++ b/sdk/client/client.go @@ -86,6 +86,7 @@ type Request struct { *http.Request } +// Marshal serializes a payload body and adds it to the *Request func (r *Request) Marshal(payload interface{}) error { contentType := strings.ToLower(r.Header.Get("Content-Type")) @@ -121,14 +122,17 @@ func (r *Request) Marshal(payload interface{}) error { return fmt.Errorf("internal-error: unimplemented marshal function for content type %q", contentType) } +// Execute invokes the Execute method for the Request's Client func (r *Request) Execute(ctx context.Context) (*Response, error) { return r.Client.Execute(ctx, r) } +// ExecutePaged invokes the ExecutePaged method for the Request's Client func (r *Request) ExecutePaged(ctx context.Context) (*Response, error) { return r.Client.ExecutePaged(ctx, r) } +// IsIdempotent determines whether a Request can be safely retried when encountering a connection failure func (r *Request) IsIdempotent() bool { switch strings.ToUpper(r.Method) { case http.MethodGet, http.MethodHead, http.MethodOptions: @@ -145,6 +149,7 @@ type Response struct { *http.Response } +// Unmarshal deserializes a response body into the provided model func (r *Response) Unmarshal(model interface{}) error { if model == nil { return fmt.Errorf("model was nil") diff --git a/sdk/client/interface.go b/sdk/client/interface.go index ccb14b1e307..da302e24add 100644 --- a/sdk/client/interface.go +++ b/sdk/client/interface.go @@ -11,8 +11,13 @@ import ( ) type BaseClient interface { + // Execute invokes a non-paginated API request and returns a populated *Response Execute(ctx context.Context, req *Request) (*Response, error) + + // ExecutePaged invokes a paginated API request, merges the results from all pages and returns a populated *Response with all results ExecutePaged(ctx context.Context, req *Request) (*Response, error) + + // NewRequest constructs a *Request that can be passed to Execute or ExecutePaged NewRequest(ctx context.Context, input RequestOptions) (*Request, error) } diff --git a/sdk/client/options.go b/sdk/client/options.go index 0c457b8b7ba..c85f09d4ac8 100644 --- a/sdk/client/options.go +++ b/sdk/client/options.go @@ -11,8 +11,13 @@ import ( ) type Options interface { + // ToHeaders yields a custom Headers struct to be appended to the request ToHeaders() *Headers + + // ToOData yields a custom *odata.Query struct to be appended to the request ToOData() *odata.Query + + // ToQuery yields a custom *QueryParams struct to be appended to the request ToQuery() *QueryParams } diff --git a/sdk/client/request_options.go b/sdk/client/request_options.go index 0f2f52ae0cc..1928c24d080 100644 --- a/sdk/client/request_options.go +++ b/sdk/client/request_options.go @@ -10,12 +10,24 @@ import ( ) type RequestOptions struct { - ContentType string + // ContentType is the content type of the request and should include the charset + ContentType string + + // ExpectedStatusCodes is a slice of HTTP response codes considered valid for this request ExpectedStatusCodes []int - HttpMethod string - OptionsObject Options - Pager odata.Pager - Path string + + // HttpMethod is the capitalized method verb for this request + HttpMethod string + + // OptionsObject is used for dynamically modifying the request at runtime + OptionsObject Options + + // Pager is an optional struct for handling custom pagination for this request. OData 4.0 compliant paging + // is already handled implicitly and does not require a custom pager. + Pager odata.Pager + + // Path is the absolute URI for this request, with a leading slash. + Path string } func (ro RequestOptions) Validate() error { From da17c4e04637e438349534853a79f5987e3f25e0 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 1 Aug 2023 16:06:15 +0100 Subject: [PATCH 3/6] OData: document exported types and functions --- sdk/odata/odata.go | 3 +++ sdk/odata/paging.go | 4 ++++ sdk/odata/query.go | 5 ++++- sdk/odata/types.go | 2 ++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/sdk/odata/odata.go b/sdk/odata/odata.go index cc41f76aed4..07784c21d20 100644 --- a/sdk/odata/odata.go +++ b/sdk/odata/odata.go @@ -12,8 +12,10 @@ import ( "github.com/hashicorp/go-uuid" ) +// ODataVersion describes the highest OData spec version supported by this package const ODataVersion = "4.0" // TODO: support 4.01 - https://docs.oasis-open.org/odata/odata-json-format/v4.01/cs01/odata-json-format-v4.01-cs01.html#_Toc499720587 +// Id describes the ID of an OData entity. type Id string func (id Id) MarshalJSON() ([]byte, error) { @@ -57,6 +59,7 @@ func (id *Id) UnmarshalJSON(data []byte) error { return nil } +// Link describes a URI obtained from an OData annotation. type Link string func (l *Link) UnmarshalJSON(data []byte) error { diff --git a/sdk/odata/paging.go b/sdk/odata/paging.go index c7640c40dd2..e5c4f3e461f 100644 --- a/sdk/odata/paging.go +++ b/sdk/odata/paging.go @@ -8,10 +8,14 @@ import ( "net/http" ) +// Pager handles custom paging for paginated API responses that do not follow the OData 4.0 standard for JSON services. +// The underlying type should support unmarshalling a JSON response type Pager interface { + // NextPageLink returns a *Link describing the URI for the next page of results NextPageLink() *Link } +// NextLinkFromPager unmarshalls a *http.Response into the provided Pager and invokes its NextPageLink method func NextLinkFromPager(resp *http.Response, pager Pager) (*Link, error) { // Read the response body and close it respBody, err := io.ReadAll(resp.Body) diff --git a/sdk/odata/query.go b/sdk/odata/query.go index 7eb09c5b10a..f8083f4cd91 100644 --- a/sdk/odata/query.go +++ b/sdk/odata/query.go @@ -11,12 +11,14 @@ import ( "strings" ) +// ConsistencyLevel is included in the API request headers when making advanced data queries type ConsistencyLevel string const ( ConsistencyLevelEventual ConsistencyLevel = "eventual" ) +// Metadata specifies the level of control information desired in the response for an API request and is appended to the Accept header type Metadata string const ( @@ -25,6 +27,7 @@ const ( MetadataNone Metadata = "none" ) +// Query describes OData query parameters that can be included in an API request. type Query struct { // ConsistencyLevel sets the corresponding http header ConsistencyLevel ConsistencyLevel @@ -63,7 +66,7 @@ type Query struct { DeltaToken string } -// Headers returns an http.Header map containing OData specific headers +// Headers returns a http.Header map containing OData specific headers func (q Query) Headers() http.Header { // Take extra care over canonicalization of header names headers := http.Header{} diff --git a/sdk/odata/types.go b/sdk/odata/types.go index cc404c3d426..0a35f400e94 100644 --- a/sdk/odata/types.go +++ b/sdk/odata/types.go @@ -3,6 +3,7 @@ package odata +// ShortType is the unqualified data type for an entity type ShortType = string const ( @@ -44,6 +45,7 @@ const ( ShortTypeWindowsHelloForBusinessAuthenticationMethod ShortType = "windowsHelloForBusinessAuthenticationMethod" ) +// Type is the namespace-qualified data type for an entity type Type = string const ( From 713bcc938ca84b8403e418d24a94e0b9266aa629 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 1 Aug 2023 16:17:46 +0100 Subject: [PATCH 4/6] Base client: fix acctests --- sdk/client/client_test.go | 111 ++++++++++++++++++++++++++++++-------- sdk/odata/paging.go | 3 +- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/sdk/client/client_test.go b/sdk/client/client_test.go index 77036c98995..7fd1c152166 100644 --- a/sdk/client/client_test.go +++ b/sdk/client/client_test.go @@ -12,6 +12,7 @@ import ( "io" "log" "net/http" + "net/url" "reflect" "testing" @@ -20,6 +21,48 @@ import ( "github.com/hashicorp/go-azure-sdk/sdk/odata" ) +var _ BaseClient = &testClient{} + +type testClient struct { + *Client +} + +func (c *testClient) NewRequest(ctx context.Context, input RequestOptions) (*Request, error) { + req, err := c.Client.NewRequest(ctx, input) + if err != nil { + return nil, fmt.Errorf("building %s request: %+v", input.HttpMethod, err) + } + + req.Client = c + query := url.Values{} + + if input.OptionsObject != nil { + if h := input.OptionsObject.ToHeaders(); h != nil { + for k, v := range h.Headers() { + req.Header[k] = v + } + } + + if q := input.OptionsObject.ToQuery(); q != nil { + for k, v := range q.Values() { + // we intentionally only add one of each type + query.Del(k) + query.Add(k, v[0]) + } + } + + if o := input.OptionsObject.ToOData(); o != nil { + req.Header = o.AppendHeaders(req.Header) + query = o.AppendValues(query) + } + } + + req.URL.RawQuery = query.Encode() + req.ValidStatusCodes = input.ExpectedStatusCodes + + return req, nil +} + func TestAccClient(t *testing.T) { test.AccTest(t) @@ -32,7 +75,9 @@ func TestAccClient(t *testing.T) { } conn.Authorize(ctx, t, api) - c := NewClient(*endpoint, "example", "2020-01-01") + c := &testClient{ + Client: NewClient(*endpoint, "example", "2020-01-01"), + } c.Authorizer = conn.Authorizer path := fmt.Sprintf("/v1.0/servicePrincipals/%s", conn.Claims.ObjectId) @@ -58,6 +103,16 @@ func TestAccClient(t *testing.T) { fmt.Printf("%#v", resp) } +var _ Options = &requestOptions{} + +type requestOptions struct { + query *odata.Query +} + +func (r *requestOptions) ToHeaders() *Headers { return nil } +func (r *requestOptions) ToOData() *odata.Query { return r.query } +func (r *requestOptions) ToQuery() *QueryParams { return nil } + func TestAccClient_Paged(t *testing.T) { test.AccTest(t) @@ -70,30 +125,35 @@ func TestAccClient_Paged(t *testing.T) { } conn.Authorize(ctx, t, api) - c := NewClient(*endpoint, "example", "2020-01-01") + c := &testClient{ + Client: NewClient(*endpoint, "example", "2020-01-01"), + } c.Authorizer = conn.Authorizer - path := "/v1.0/servicePrincipals" + path := "/v1.0/applications" reqOpts := RequestOptions{ ContentType: "application/json", ExpectedStatusCodes: []int{ http.StatusOK, }, - HttpMethod: http.MethodGet, - OptionsObject: nil, - Path: path, + HttpMethod: http.MethodGet, + OptionsObject: &requestOptions{ + query: &odata.Query{ + Filter: "startsWith(displayName,'acctest')", + Select: []string{"appId", "displayName"}, + Top: 10, + }, + }, + Path: path, } req, err := c.NewRequest(ctx, reqOpts) if err != nil { t.Fatal(err) } - resp, err := req.ExecutePaged(ctx) - if err != nil { + if _, err = req.ExecutePaged(ctx); err != nil { t.Fatalf("ExecutePaged(): %v", err) } - - fmt.Printf("%#v", resp) } var _ odata.Pager = &pager{} @@ -107,9 +167,13 @@ func (p *pager) NextPageLink() *odata.Link { log.Fatalf("pager: p was nil") } if p.NextLink == nil { - log.Fatalf("pager: nextLink was nil") + log.Printf("[DEBUG] pager: nextLink was nil") + } else { + log.Printf("[DEBUG] pager: found custom nextLink %q", *p.NextLink) } - log.Printf("[DEBUG] pager: found custom nextLink %q", *p.NextLink) + defer func() { + p.NextLink = nil + }() return p.NextLink } @@ -125,7 +189,9 @@ func TestAccClient_CustomPaged(t *testing.T) { } conn.Authorize(ctx, t, api) - c := NewClient(*endpoint, "example", "2020-01-01") + c := &testClient{ + Client: NewClient(*endpoint, "example", "2020-01-01"), + } c.Authorizer = conn.Authorizer path := "/v1.0/applications" @@ -134,22 +200,25 @@ func TestAccClient_CustomPaged(t *testing.T) { ExpectedStatusCodes: []int{ http.StatusOK, }, - HttpMethod: http.MethodGet, - OptionsObject: nil, - Pager: &pager{}, - Path: path, + HttpMethod: http.MethodGet, + OptionsObject: &requestOptions{ + query: &odata.Query{ + Filter: "startsWith(displayName,'acctest')", + Select: []string{"appId", "displayName"}, + Top: 10, + }, + }, + Pager: &pager{}, + Path: path, } req, err := c.NewRequest(ctx, reqOpts) if err != nil { t.Fatal(err) } - resp, err := req.ExecutePaged(ctx) - if err != nil { + if _, err = req.ExecutePaged(ctx); err != nil { t.Fatalf("ExecutePaged(): %v", err) } - - fmt.Printf("%#v", resp) } func TestMarshalByteStreamAndPowerShell(t *testing.T) { diff --git a/sdk/odata/paging.go b/sdk/odata/paging.go index e5c4f3e461f..84eb445db1f 100644 --- a/sdk/odata/paging.go +++ b/sdk/odata/paging.go @@ -11,7 +11,8 @@ import ( // Pager handles custom paging for paginated API responses that do not follow the OData 4.0 standard for JSON services. // The underlying type should support unmarshalling a JSON response type Pager interface { - // NextPageLink returns a *Link describing the URI for the next page of results + // NextPageLink returns a *Link describing the URI for the next page of results, it should also clear any state + // before returning, so that subsequent pages do not inherit the URI from the previous page. NextPageLink() *Link } From 03057a718bf602d4925fd32e0baa7ad88a71bd7e Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 2 Aug 2023 13:01:52 +0100 Subject: [PATCH 5/6] Paging: check pager is not nil --- sdk/odata/paging.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sdk/odata/paging.go b/sdk/odata/paging.go index 84eb445db1f..10a67343b2d 100644 --- a/sdk/odata/paging.go +++ b/sdk/odata/paging.go @@ -18,6 +18,10 @@ type Pager interface { // NextLinkFromPager unmarshalls a *http.Response into the provided Pager and invokes its NextPageLink method func NextLinkFromPager(resp *http.Response, pager Pager) (*Link, error) { + if pager == nil { + return nil, fmt.Errorf("internal-error: pager was nil, should be a pointer") + } + // Read the response body and close it respBody, err := io.ReadAll(resp.Body) resp.Body.Close() @@ -30,7 +34,7 @@ func NextLinkFromPager(resp *http.Response, pager Pager) (*Link, error) { } // Unmarshal - if err := json.Unmarshal(respBody, &pager); err != nil { + if err := json.Unmarshal(respBody, pager); err != nil { return nil, err } From a85ba58a616031a97557117f385806f8dc82c41c Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 9 Aug 2023 13:31:12 +0100 Subject: [PATCH 6/6] Paging: rename to CustomPager, surface custom paging errors --- sdk/client/client.go | 7 +++++-- sdk/client/client_test.go | 2 +- sdk/client/request_options.go | 2 +- sdk/odata/paging.go | 8 ++++---- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/sdk/client/client.go b/sdk/client/client.go index ce98894d9ba..22ee6a153b8 100644 --- a/sdk/client/client.go +++ b/sdk/client/client.go @@ -80,7 +80,7 @@ type Request struct { ValidStatusFunc ValidStatusFunc Client BaseClient - Pager odata.Pager + Pager odata.CustomPager // Embed *http.Request so that we can send this to an *http.Client *http.Request @@ -486,7 +486,10 @@ func (c *Client) ExecutePaged(ctx context.Context, req *Request) (*Response, err if req.Pager == nil { nextLink = firstOdata.NextLink } else { - nextLink, err = odata.NextLinkFromPager(resp.Response, req.Pager) + nextLink, err = odata.NextLinkFromCustomPager(resp.Response, req.Pager) + if err != nil { + return resp, err + } } if nextLink == nil { // This is the last page diff --git a/sdk/client/client_test.go b/sdk/client/client_test.go index 7fd1c152166..b94f8cb38c5 100644 --- a/sdk/client/client_test.go +++ b/sdk/client/client_test.go @@ -156,7 +156,7 @@ func TestAccClient_Paged(t *testing.T) { } } -var _ odata.Pager = &pager{} +var _ odata.CustomPager = &pager{} type pager struct { NextLink *odata.Link `json:"@odata.nextLink"` diff --git a/sdk/client/request_options.go b/sdk/client/request_options.go index 1928c24d080..b0e0b653bcf 100644 --- a/sdk/client/request_options.go +++ b/sdk/client/request_options.go @@ -24,7 +24,7 @@ type RequestOptions struct { // Pager is an optional struct for handling custom pagination for this request. OData 4.0 compliant paging // is already handled implicitly and does not require a custom pager. - Pager odata.Pager + Pager odata.CustomPager // Path is the absolute URI for this request, with a leading slash. Path string diff --git a/sdk/odata/paging.go b/sdk/odata/paging.go index 10a67343b2d..1042194ee2a 100644 --- a/sdk/odata/paging.go +++ b/sdk/odata/paging.go @@ -8,16 +8,16 @@ import ( "net/http" ) -// Pager handles custom paging for paginated API responses that do not follow the OData 4.0 standard for JSON services. +// CustomPager handles custom paging for paginated API responses that do not follow the OData 4.0 standard for JSON services. // The underlying type should support unmarshalling a JSON response -type Pager interface { +type CustomPager interface { // NextPageLink returns a *Link describing the URI for the next page of results, it should also clear any state // before returning, so that subsequent pages do not inherit the URI from the previous page. NextPageLink() *Link } -// NextLinkFromPager unmarshalls a *http.Response into the provided Pager and invokes its NextPageLink method -func NextLinkFromPager(resp *http.Response, pager Pager) (*Link, error) { +// NextLinkFromCustomPager unmarshalls a *http.Response into the provided CustomPager and invokes its NextPageLink method +func NextLinkFromCustomPager(resp *http.Response, pager CustomPager) (*Link, error) { if pager == nil { return nil, fmt.Errorf("internal-error: pager was nil, should be a pointer") }