Add prefix support to swift test double

Add prefix support to swift test double

The swift test double did not support using prefix=xxx to filter container listings.
This branch adds that support which is required to make some juju-core tests pass.

https://code.launchpad.net/~wallyworld/goose/swift-double-prefix-support/+merge/150265

(do not edit description out of merge proposal)

[wallyworld, 2013-02-25 06:35:51]

Please take a look.

[dimitern, 2013-02-25 07:16:22]

LGTM, just a few trivials.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service.go:102: // ListContainer lists the objects in
the given container.
Update the comment about the new params?

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service.go:111: sorted := make([]string, 0,
len(items))
why not just: var sorted []string ?
you'll fill it in in the loop anyway

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_http.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http.go:58: contents, err :=
s.ListContainer(container, params)
So it'll work without any params as well?

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_http_test.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http_test.go:57: urlParams := make(url.Values,
len(params))
I don't think you need to specify the length here, since Set() will just work.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http_test.go:180: s.removeContainer("test", c)
defer this just after s.ensureContainer ?

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_test.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_test.go:95: containerData, err :=
s.service.ListContainer("test", map[string]string{})
Sending nil instead of an empty map should work.

[jameinel, 2013-02-25 07:42:34]

LGTM

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service.go:111: sorted := make([]string, 0,
len(items))
He is pre-allocating enough items in the array so it doesn't have to resize
during the loop.

It isn't a huge deal, but IMO it is a good practice to pre-allocate when you
know the final size.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_http.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http.go:58: contents, err :=
s.ListContainer(container, params)
On 2013/02/25 07:16:22, dimitern wrote:
> So it'll work without any params as well?

I'm pretty sure len(urlParams) can be done, in which case you just have an empty
map. Which all the code I've seen just iterates over the map, so they should all
be fine.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_http_test.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http_test.go:57: urlParams := make(url.Values,
len(params))
On 2013/02/25 07:16:22, dimitern wrote:
> I don't think you need to specify the length here, since Set() will just work.


It *is* optional, but if you know in advance, you usually want to pass it.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http_test.go:180: s.removeContainer("test", c)
On 2013/02/25 07:16:22, dimitern wrote:
> defer this just after s.ensureContainer ?

Agreed, defering the cleanup as soon as you can should help keep things clean
even if an Assertion in the middle fails.

[wallyworld, 2013-02-26 01:04:21]

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service.go:102: // ListContainer lists the objects in
the given container.
On 2013/02/25 07:16:22, dimitern wrote:
> Update the comment about the new params?

Done.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service.go:111: sorted := make([]string, 0,
len(items))
On 2013/02/25 07:42:34, jameinel wrote:
> He is pre-allocating enough items in the array so it doesn't have to resize
> during the loop.
> 
> It isn't a huge deal, but IMO it is a good practice to pre-allocate when you
> know the final size.

The final size isn't know until the loop ends, and the loop is appending to the
slice in question so I took the decision to pre-allocate the known maximum size
as the capacity. The initial length is 0 as expected so I think it's a
reasonable decision to make.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_http.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_http.go:58: contents, err :=
s.ListContainer(container, params)
On 2013/02/25 07:42:34, jameinel wrote:
> On 2013/02/25 07:16:22, dimitern wrote:
> > So it'll work without any params as well?
> 
> I'm pretty sure len(urlParams) can be done, in which case you just have an
empty
> map. Which all the code I've seen just iterates over the map, so they should
all
> be fine.

Yes, works fine without params.

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
File testservices/swiftservice/service_test.go (right):

https://codereview.appspot.com/7388055/diff/1/testservices/swiftservice/servi...
testservices/swiftservice/service_test.go:95: containerData, err :=
s.service.ListContainer("test", map[string]string{})
On 2013/02/25 07:16:22, dimitern wrote:
> Sending nil instead of an empty map should work.

Good pickup, thanks.