byterock

Well I was just about ready to check everything in on S3 Paws and move onto the next part of Paws that need some love and kindness when I did one last review of all the test cases I have created over the past little while when I discovered I had left one out.

I forgot to add in a request test case for the 'DeleteBucketMetricsConfiguration' action. It took the opportunity to use my new test generator a spin as well so I fired up my real world call with the new caller and much to my chagrin I got an error;

'The specified configuration does not exist.',

Which could be the case as the Bucket I was playing with may not have a 'Metrics Configuration' on it. So I dusted off another test that does both 'PUT and 'GET' and then I slapped my 'DELETE' on the end and this was the result;

ok 1 - Call S3->PutBucketMetricsConfiguration
ok 2 – Have a PutBucketMetricsConfigurationResponse
ok 3 – Call S3->GetBucketMetricsConfiguration
ok 4 – Have a GetBucketMetricsConfigurationOutput
ok 5 – Id= ImportantBlueDocuments
…
not ok Call  S3-> DeleteBucketMetricsConfiguration
not ok Have a Paws::API::Response

So something not right. I checked on my AWS account and the S3 bucket in question does have a Metric Config so the fail is not a false positive.

I looked at the API spec again;

DELETE /?metrics&Id=Id HTTP/1.1 Host: Bucket.s3.amazonaws.com

URI Request Parameters

The request requires the following URI parameters.

• Bucket
• The name of the bucket containing the metrics configuration to delete.
• id
• The ID used to identify the metrics configuration.

And there is an discrepancy there. It asks for and 'id' but in the URL we have 'Id'. I checked boto;

"Id":{
"shape":"MetricsId",
"documentation":"<p>The ID used to identify the metrics configuration.</p>",
"location":"uri",
"locationName":"id"
} 

so that is ok it might be a case of the API doc not being correct. I have seen that before. I checked what I was sending for the URI on the request;

/oneoffpaws?metrics

ok so I am missing the 'id' in the URI so that led me into the 'sub _call_uri' of my RestXmlCaller.pm and with the above boto I two 'uri' params one 'Bucket' is on the template '/{Bucket}?metrics' the other id is not and that led me to this part of the sub;

if ( $attribute->does('Paws::API::Attribute::Trait::ParamInURI') ) {
  my $att_name = $attribute->name;
  if ( $uri_attrib_is_greedy{$att_name} ) {
    $vars->{ $attribute->uri_name } =
    uri_escape_utf8( $call->$att_name, q[^A-Za-z0-9\-\._~/] );
    $uri_template =~ s{$att_name\+}{\+$att_name}g;
  }
  else {
    $vars->{ $attribute->uri_name } = $call->$att_name;
  }
} 

playing around a bit I had a look at the incoming values in the '%uri_attrib_is_greedy' hash and I got;

{

    'Bucket' => ''

};

Ah! so this;

if ( $uri_attrib_is_greedy{$att_name} ) {

will test the value of 'Bucket' key and it value is false so it does not go into this code block. This is a bug as having an empty value is perfectly valid. As So I changed that to this;

–  if ( $uri_attrib_is_greedy{$att_name} ) {
++ if (exists($uri_attrib_is_greedy{$att_name}) ) {

and when I ran my code again I got this result for the URI;

/{Bucket}?metrics&Bucket={Bucket}

ok a little too much and I am still missing the 'id' and I have an extra Bucket para. So I fixed that up to this final version;

if ( exists($uri_attrib_is_greedy{$att_name} )) {
  $vars->{ $attribute->uri_name } =
  uri_escape_utf8( $call->$att_name, q[^A-Za-z0-9\-\._~/] );
–$uri_template =~ s{$att_name\+}{\+$att_name}g;
}
else {
++$uri_template .=
++        $joiner . $attribute->uri_name . "={" . $attribute->uri_name . "}";
  $vars->{ $attribute->uri_name } = $call->$att_name;
}

So now I just escape the input value for 'Bucket' and for 'id' I add in the param to the template. So not I get an end template like this;

/{Bucket}?metrics&id={id}

which is handled fine by this code

my $t   = URI::Template->new($uri_template);
my $uri = $t->process($vars);
return $uri;

that ends out the '_call_uri' sub.

No both my real world test and my canned test pass.

I was a bit of a puzzle for my why it took so long to uncover this bug. So I went back over a number of the Delete actions and tried them out in a number of ways. Seems some will work with either Id in the params or id on the url and some will return 200 even is nothing is deleted but the ' DeleteBucketMetricsConfiguration' was the one that does neither and as I never did create a test for it and thus never ran into it till now.

That is not the end of the story as I did break 'DeleteObject' with this test as now I get a bad URL fail

want: https://s3.fake_region.amazonaws.com/oneoffpaws/one/to/delete/image.jpg 
get: https://s3.fake_region.amazonaws.com/oneoffpaws/

and I traced that back to this patch;

if ( exists($uri_attrib_is_greedy{$att_name} )) {
    $vars->{ $attribute->uri_name } =
    uri_escape_utf8( $call->$att_name, q[^A-Za-z0-9\-\._~/] );
–  $uri_template =~ s{$att_name\+}{\+$att_name}g;
}

In this case the template was coming out as

/{Bucket}/{Key+}?versionId={VersionId}

but the values that where being processed where;

 {
    'Bucket' => 'oneoffpaws',
    'Key' => 'one/to/delete/image.jpg',
    'VersionId' => 'my version id'
};

so it as not subbing out that 'Key+' . So I have to but that regex back in but only have it fire it when there is a value on the greedy bit.

if ( exists($uri_attrib_is_greedy{$att_name} )) {
    $vars->{ $attribute->uri_name } =
            uri_escape_utf8( $call->$att_name, q[^A-Za-z0-9\-\._~/] );
++    $uri_template =~ s{$att_name\+}{\+$att_name}g
++      if (($uri_attrib_is_greedy{$att_name});
}
...

and now at least all my 'DELETE' action tests pass and I think I am getting to the end of the tunnel.