Skip to content

Commit

Permalink
(read_)selector handle selected part is removed out-of-band (#94)
Browse files Browse the repository at this point in the history
For some *composite* API responses, where only the subset of the response represents the resource that the user currently tracks. In this case, users use `(read_)selector` to select the tracked resource from this response. Currently, the provider will always try to apply the selector (query) on the response and return the query result, even if queried out nothing, in which case the follow up modify body process in read function will fail with errors like below:

```
│ Error: Modifying `body` during Read
...
│ unmarshal the body "": unexpected end of JSON input
```

This can be triggered by several cases, e.g.:

1. The selector is not correctly composed
2. The resource is removed out-of-band

This PR changes the behavior when the query returns nothing, that we will regard this resource as non exist. This has no problem for the 2nd case, but might introduce in-convenience for the 1st case, as user now will need to either import the resource or remove it out-of-band...

This change covers:

- Resource `restful_resource` - `read_selector`
- Data Source `restful_resource` - `selector`
  • Loading branch information
magodo authored May 16, 2024
1 parent 9c1c14e commit 27efaf7
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 31 deletions.
36 changes: 23 additions & 13 deletions internal/client/async.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,42 +14,51 @@ import (

// ValueLocator indicates where a value is located in a HTTP response.
type ValueLocator interface {
LocateValueInResp(resty.Response) string
LocateValueInResp(resty.Response) (string, bool)
String() string
}

type ExactLocator string

func (loc ExactLocator) LocateValueInResp(_ resty.Response) string {
return string(loc)
var _ ValueLocator = ExactLocator("")

func (loc ExactLocator) LocateValueInResp(_ resty.Response) (string, bool) {
return string(loc), true
}
func (loc ExactLocator) String() string {
return fmt.Sprintf(`exact.%s`, string(loc))
}

type HeaderLocator string

func (loc HeaderLocator) LocateValueInResp(resp resty.Response) string {
return resp.Header().Get(string(loc))
var _ ValueLocator = HeaderLocator("")

func (loc HeaderLocator) LocateValueInResp(resp resty.Response) (string, bool) {
v := resp.Header().Get(string(loc))
return v, true
}
func (loc HeaderLocator) String() string {
return fmt.Sprintf(`header.%s`, string(loc))
}

type BodyLocator string

func (loc BodyLocator) LocateValueInResp(resp resty.Response) string {
var _ ValueLocator = BodyLocator("")

func (loc BodyLocator) LocateValueInResp(resp resty.Response) (string, bool) {
result := gjson.GetBytes(resp.Body(), string(loc))
return result.String()
return result.String(), result.Exists()
}
func (loc BodyLocator) String() string {
return fmt.Sprintf(`body.%s`, string(loc))
}

type CodeLocator struct{}

func (loc CodeLocator) LocateValueInResp(resp resty.Response) string {
return strconv.Itoa(resp.StatusCode())
var _ ValueLocator = CodeLocator{}

func (loc CodeLocator) LocateValueInResp(resp resty.Response) (string, bool) {
return strconv.Itoa(resp.StatusCode()), true
}
func (loc CodeLocator) String() string {
return "code"
Expand Down Expand Up @@ -106,8 +115,9 @@ func NewPollableForPoll(resp resty.Response, opt PollOption) (*Pollable, error)

var rawURL string
if loc := opt.UrlLocator; loc != nil {
rawURL = loc.LocateValueInResp(resp)
if rawURL == "" {
var ok bool
rawURL, ok = loc.LocateValueInResp(resp)
if !ok {
return nil, fmt.Errorf("No polling URL found in %s", loc)
}
} else {
Expand Down Expand Up @@ -188,8 +198,8 @@ PollingLoop:
}
}

status := f.StatusLocator.LocateValueInResp(*resp)
if status == "" {
status, ok := f.StatusLocator.LocateValueInResp(*resp)
if !ok {
return fmt.Errorf("No status value found from %s", f.StatusLocator)
}
// We tolerate case difference here to be pragmatic.
Expand Down
4 changes: 2 additions & 2 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func (c *Client) setRetry(opt RetryOption) {
return true
}

status := opt.StatusLocator.LocateValueInResp(*r)
if status == "" {
status, ok := opt.StatusLocator.LocateValueInResp(*r)
if !ok {
return false
}
// We tolerate case difference here to be pragmatic.
Expand Down
28 changes: 14 additions & 14 deletions internal/provider/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/tidwall/gjson"
"github.com/magodo/terraform-provider-restful/internal/client"
)

type DataSource struct {
Expand Down Expand Up @@ -170,23 +170,23 @@ func (d *DataSource) Read(ctx context.Context, req datasource.ReadRequest, resp

b := response.Body()

if config.Selector.ValueString() != "" {
result := gjson.GetBytes(b, config.Selector.ValueString())
if !result.Exists() {
if sel := config.Selector.ValueString(); sel != "" {
bodyLocator := client.BodyLocator(sel)
sb, ok := bodyLocator.LocateValueInResp(*response)
if !ok {
if config.AllowNotExist.ValueBool() {
// Setting the input attributes to the state anyway
diags = resp.State.Set(ctx, state)
resp.Diagnostics.Append(diags...)
return
}
resp.Diagnostics.AddError(
fmt.Sprintf("Failed to select resource from response"),
fmt.Sprintf("Can't find resource with query %q", config.Selector.ValueString()),
fmt.Sprintf("`selector` failed to select from the response"),
string(response.Body()),
)
return
}
if len(result.Array()) > 1 {
resp.Diagnostics.AddError(
fmt.Sprintf("Failed to select resource from response"),
fmt.Sprintf("Multiple resources with query %q found (%d)", config.Selector.ValueString(), len(result.Array())),
)
return
}
b = []byte(result.Array()[0].Raw)
b = []byte(sb)
}

// Set output
Expand Down
19 changes: 17 additions & 2 deletions internal/provider/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,15 @@ func (r Resource) Create(ctx context.Context, req resource.CreateRequest, resp *
if sel := plan.CreateSelector.ValueString(); sel != "" {
// Guaranteed by schema
bodyLocator := client.BodyLocator(sel)
b = []byte(bodyLocator.LocateValueInResp(*response))
sb, ok := bodyLocator.LocateValueInResp(*response)
if !ok {
resp.Diagnostics.AddError(
fmt.Sprintf("`create_selector` failed to select from the response"),
string(response.Body()),
)
return
}
b = []byte(sb)
}

// Construct the resource id, which is used as the path to read the resource later on. By default, it is the same as the "path", unless "read_path" is specified.
Expand Down Expand Up @@ -776,7 +784,14 @@ func (r Resource) Read(ctx context.Context, req resource.ReadRequest, resp *reso
if sel := state.ReadSelector.ValueString(); sel != "" {
// Guaranteed by schema
bodyLocator := client.BodyLocator(sel)
b = []byte(bodyLocator.LocateValueInResp(*response))
sb, ok := bodyLocator.LocateValueInResp(*response)
// This means the tracked resource selected (filtered) from the response now disappears (deleted out of band).
if !ok {
resp.State.RemoveResource(ctx)
return
}
b = []byte(sb)

}

var writeOnlyAttributes []string
Expand Down

0 comments on commit 27efaf7

Please sign in to comment.