added path_query_string parameter

[NamrathaG]

Issue : #103

Helps the user add a query string to API requests.

[DRuggeri]

Thanks, @NamrathaG!
I haven't tried it myself, but is a separate configuration item needed? Since this is just concatenating the path with ? and then the parameter, it seems the same can be accomplished by just using 'path'?

[DRuggeri]

Oh goodness - I've confused myself. Yep - this makes sense. Can you make just one modification, please, to name the parameter to query_string? This is to keep the restapi resource and datasource in alignment as far as configurations

[sirlatrom]

@DRuggeri Is this good to go?

[sirlatrom]

@NamrathaG May I suggest you add the query_string to the data source as well, so that once the searched key has been matched, the read (GET) request for the resource also uses the query_string value?

[NamrathaG]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

[sirlatrom]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

@NamrathaG How about the former, with the latter as an option for overriding in the cases when a different or an empty query string is required?

[NamrathaG]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

@NamrathaG How about the former, with the latter as an option for overriding in the cases when a different or an empty query string is required?

Yes, that makes sense. Will update the PR.

[NamrathaG]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

@NamrathaG How about the former, with the latter as an option for overriding in the cases when a different or an empty query string is required?

@sirlatrom In the above comment by empty you mean the user will explicitly set read_query_string = ""?

[sirlatrom]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

@NamrathaG How about the former, with the latter as an option for overriding in the cases when a different or an empty query string is required?

@sirlatrom In the above comment by empty you mean the user will explicitly set read_query_string = ""?

Yes, that would reflect how the provider otherwise builds reasonable default effective values for e.g. read_path etc.

[NamrathaG]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?

@NamrathaG How about the former, with the latter as an option for overriding in the cases when a different or an empty query string is required?

@sirlatrom In the above comment by empty you mean the user will explicitly set read_query_string = ""?

Yes, that would reflect how the provider otherwise builds reasonable default effective values for e.g. read_path etc.

But then how to differentiate between the case read_query_string="" and the case where read_query_string is not set at all ?
One way is to set some default value to read_query_string like "not-set" :

	"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Optional:     true,
				Default:      "not-set"
				Description:  "By default set to the same value as query_string. This key allows settig a different or empty query string for reading the object."
			}

Is there a better way?

[sirlatrom]

@sirlatrom I see data source already has an optional query_string option which is used for searching, but not for reading. Was thinking if we should use the same query_string to read as well or create a different option like read_query_string?
But then how to differentiate between the case read_query_string="" and the case where read_query_string is not set at all ?
One way is to set some default value to read_query_string like "not-set" :

	"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Optional:     true,
				Default:      "not-set"
				Description:  "By default set to the same value as query_string. This key allows settig a different or empty query string for reading the object."
			}

Is there a better way?

I believe the Provider SDK has methods for knowing whether an argument is set. Maybe it's called d.IsSet()?

[NamrathaG]

I believe the Provider SDK has methods for knowing whether an argument is set. Maybe it's called d.IsSet()?

The closest I could find was GetOk and GetOkExists, but both return "" and false when a TypeString option is set to "" or not set at all.

[sirlatrom]

I believe the Provider SDK has methods for knowing whether an argument is set. Maybe it's called d.IsSet()?

The closest I could find was GetOk and GetOkExists, but both return "" and false when a TypeString option is set to "" or not set at all.

I must have confused it with https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/terraform#ResourceConfig.IsSet, which I don't know if is available in the method you're implementing.

[NamrathaG]

I must have confused it with https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/terraform#ResourceConfig.IsSet, which I don't know if is available in the method you're implementing.

The function (func dataSourceRestApiRead(d *schema.ResourceData, meta interface{}) error {) I am working with does not support that.
Should I go ahead with the default value idea? 🤔

	"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Optional:     true,
				Default:      "not-set"
				Description:  "By default set to the same value as query_string. This key allows settig a different or empty query string for reading the object."
			}

[sirlatrom]

The function (func dataSourceRestApiRead(d *schema.ResourceData, meta interface{}) error {) I am working with does not support that.
Should I go ahead with the default value idea? 🤔

	"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Optional:     true,
				Default:      "not-set"
				Description:  "By default set to the same value as query_string. This key allows settig a different or empty query string for reading the object."
			}

@NamrathaG I recommend changing the type of the query_string field in the struct to *string and only setting/reading it if it has a value. Use the GetOk method and check the second return value. Check for nil value instead of empty string.

PS: It will still be of the TypeString type in the Terraform schema.

[NamrathaG]

@NamrathaG I recommend changing the type of the query_string field in the struct to *string and only setting/reading it if it has a value. Use the GetOk method and check the second return value. Check for nil value instead of empty string.

PS: It will still be of the TypeString type in the Terraform schema.

@sirlatrom But I don't understand how this solves our problem of identifying if read_query_string is set to "" or not set in the configuration.
Identifying this is important because if read_query_string is set to "" we want to send an empty query string value for reading object, and if it is not set at all then we want to send the value of query_string(the one used for finding object) for reading object.
Am I missing something?

[sirlatrom]

@NamrathaG My bad, I meant the read_query_string field should be made a *string, which allows comparing with nil vs. only "" now, and can be combined with the GetOk() method to only set it to a non-nil value if the 2nd return value is true.

Additionally, if a map like read_search is set, you can use the map lookup function that returns 2 values, e.g.

		query_string, query_string_exists := obj.read_search["read_query_string"]
		if read_query_string_exists {
			if obj.debug {
				log.Printf("api_object.go: Adding read query string '%s'", obj.query_string)
			}
			query_string = fmt.Sprintf("%s&%s", obj.read_search["read_query_string"], obj.query_string)
		}

[NamrathaG]

@sirlatrom I understood that if a variable's type is set to *string then we can differentiate between the cases set to "" and not set.

But I don't think setting read_query_string to *string in the apiObjectOpts struct will be of much use, because the first time we read it's value is in the function func dataSourceRestApiRead(d *schema.ResourceData, meta interface{}) error { and inorder to get the value from d we have to use d.Get("read_query_string") or d.GetOk("read_query_string"), which will give "" regardless of whether it is set to "" or not set. And GetOk's second result is false in both cases.
So we don't know if the user set it to "" or did not set it, and only after reading it, we can populate the apiObjectOpts structure accordingly.

I am assuming we are going to set the datasource schema as below

func dataSourceRestApi() *schema.Resource {
	return &schema.Resource{
		Read: dataSourceRestApiRead,

		Schema: map[string]*schema.Schema{
			"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Description:  "By default set to the same value as query_string. 
				This key allows setting a different or empty query string for reading the object.",
			    Optional:     true,
			},...

[sirlatrom]

@sirlatrom I understood that if a variable's type is set to *string then we can differentiate between the cases set to "" and not set.

But I don't think setting read_query_string to *string in the apiObjectOpts struct will be of much use, because the first time we read it's value is in the function func dataSourceRestApiRead(d *schema.ResourceData, meta interface{}) error { and inorder to get the value from d we have to use d.Get("read_query_string") or d.GetOk("read_query_string"), which will give "" regardless of whether it is set to "" or not set. And GetOk's second result is false in both cases.
So we don't know if the user set it to "" or did not set it, and only after reading it, we can populate the apiObjectOpts structure accordingly.

I am assuming we are going to set the datasource schema as below

func dataSourceRestApi() *schema.Resource {
	return &schema.Resource{
		Read: dataSourceRestApiRead,

		Schema: map[string]*schema.Schema{
			"read_query_string": &schema.Schema{
				Type:         schema.TypeString, 
				Description:  "By default set to the same value as query_string. 
				This key allows setting a different or empty query string for reading the object.",
			    Optional:     true,
			},...

@NamrathaG The schema looks right.

Use valueRaw, ok := d.GetOk("read_query_string") and check if ok if true or false. If true, there is a value, otherwise there isn't. You can since use value := valueRaw.(string) to coerce to a string.

[NamrathaG]

@sirlatrom Yeah, I tried this but getting the same output for both the cases below
Case 1:

data "restapi_object" "John" {
  path = "/api/objects"
  read_query_string = ""
}

Case 2:

data "restapi_object" "John" {
  path = "/api/objects"
}

When I run the below snippet for both the cases I am getting the same result( ok is false and valueRaw is empty("" I think))

valueRaw, ok := d.GetOk("read_query_string")`
log.Printf("%s", ok)
log.Printf("%s", valueRaw)  

[sirlatrom]

When I run the below snippet for both the cases I am getting the same result( ok is false and valueRaw is empty("" I think))

valueRaw, ok := d.GetOk("read_query_string")`
log.Printf("%s", ok)
log.Printf("%s", valueRaw)  

I think I finally get it now, sorry for taking so long 😅

So, in the docs it says

GetOk returns the data for the given key and whether or not the key has been set to a non-zero value at some point.

which means a string value of "" will yield false in the 2nd return value from GetOk().

I think that means your suggestion of a placeholder default value is the best approach.

[DRuggeri]

Thanks for the awesome discussion, all. Much appreciated!

I'm waiting for the day an issue gets logged where a user would like to set the query string of not-set... but until then, this is great :-)