pscott-au / WebService-GoogleAPI-Client

Perl WebService::GoogleAPI::Client Module
https://pscott-au.github.io/WebService-GoogleAPI-Client/
Other
1 stars 2 forks source link

Accept parameters which are repeatable within Google API #16

Closed bgilly1 closed 5 years ago

bgilly1 commented 5 years ago

In working with the Sheets API I noticed there are some parameters (e.g. sheets.spreadsheets.get.ranges) which can appear in the query string multiple times in order to return multiple cell ranges from different spreadsheet locations using a single call. The CHI-stored api spec from google identifies these parameters as "repeated" The code in Client.pm sub _interpolate_path_parameters_append_query_params_and_return_errors didn't account for this and only retained the last value for the "ranges" parameter.

I added this code to allow for api-query options to also accept and anonymous array for repeated values and add them to the @get_query_params array in Client.pm starting at line 232.

      if ( defined $method_discovery_struct->{ parameters }{ $meth_param_spec }{ repeated } ) {
        if ( $method_discovery_struct->{ parameters }{ $meth_param_spec }{ repeated } ) {
            if ( ref $params->{ options }{ $meth_param_spec } eq 'ARRAY' ) {
                    foreach my $repeated_param (@{$params->{ options }{ $meth_param_spec }}) {
                        push( @get_query_params, "$meth_param_spec=$repeated_param" ) if defined $params->{ options }{ $meth_param_spec };
                    }
            }
            else {
                push( @get_query_params, "$meth_param_spec=$params->{options}{$meth_param_spec}" ) if defined $params->{ options }{ $meth_param_spec };
            }
        }
      }
      else {
          push( @get_query_params, "$meth_param_spec=$params->{options}{$meth_param_spec}" ) if defined $params->{ options }{ $meth_param_spec };
      }
rabbiveesh commented 5 years ago

Wow, I missed that when I was reading through the rpc spec. If you plan on making a pull request, could you write tests for it also?

On Wed, Feb 6, 2019 at 5:30 PM bgilly1 notifications@github.com wrote:

In working with the Sheets API I noticed there are some parameters (e.g. sheets.spreadsheets.get.ranges) which can appear in the query string multiple times in order to return multiple cell ranges from different spreadsheet locations using a single call. The CHI-stored api spec from google identifies these parameters as "repeated" The code in Client.pm sub _interpolate_path_parameters_append_query_params_and_return_errors didn't account for this and only retained the last value for the "ranges" parameter.

I added this code to allow for api-query options to also accept and anonymous array for repeated values and add them to the @get_query_params array in Client.pm starting at line 232.

if ( defined $method_discovery_struct->{ parameters }{ $meth_param_spec }{ repeated } ) {
  if ( $method_discovery_struct->{ parameters }{ $meth_param_spec }{ repeated } ) {
      if ( ref $params->{ options }{ $meth_param_spec } eq 'ARRAY' ) {
              foreach my $repeated_param (@{$params->{ options }{ $meth_param_spec }}) {
                  push( @get_query_params, "$meth_param_spec=$repeated_param" ) if defined $params->{ options }{ $meth_param_spec };
              }
      }
      else {
          push( @get_query_params, "$meth_param_spec=$params->{options}{$meth_param_spec}" ) if defined $params->{ options }{ $meth_param_spec };
      }
  }
}
else {
    push( @get_query_params, "$meth_param_spec=$params->{options}{$meth_param_spec}" ) if defined $params->{ options }{ $meth_param_spec };
}

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pscott-au/WebService-GoogleAPI-Client/issues/16, or mute the thread https://github.com/notifications/unsubscribe-auth/AWkXqpLiuKcPuPBnI1-lmlBMJQFmEF_Iks5vKvUVgaJpZM4alezf .

pscott-au commented 5 years ago

Nice pickup . am hoping to get some time to look into this in the next week. Pull requests welcome.

rabbiveesh commented 5 years ago

Hey, I just looked into this. We already interpolate arrayrefs into a list of arguments, because that's what Mojo::Parameters does. The proper way to do so is to pass an array ref to the field that you want repeated. In your case, it would mean

  { 
     api_endpoint_id => 'sheets:v4.spreadsheets.values.batchGet',
     options => {
          ranges => [ 'first_range', 'second_range' ],
      }
  }

Also, the endpoint you're looking for is batchGet, not get, but that's an aside.

Since we already support this, I'm closing this issue.