You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2013/07/09 00:08:06 UTC
[jclouds-labs-openstack] Fixes encoding of "." when listing a single
user (#14)
Only affects users with restricted hostnames
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack fix-get-user
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/14
-- Commit Summary --
* Fixes encoding of "." when listing a single user
-- File Changes --
M openstack-trove/src/main/java/org/jclouds/openstack/trove/v1/features/UserApi.java (3)
A openstack-trove/src/main/java/org/jclouds/openstack/trove/v1/filters/EncodeDotsForUserGet.java (48)
M openstack-trove/src/test/java/org/jclouds/openstack/trove/v1/features/UserApiExpectTest.java (8)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/14.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/14.diff
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #179](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/179/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20695002
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #175](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/175/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20643653
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #7](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/7/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20640278
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #176](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/176/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20644035
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
If @demobox is ok with the latest changes, I'm going to merge this one tonight.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20842043
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Andrew Phillips <no...@github.com>.
If the match in the filter could fail, add a test case to verify that everything works in that case too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20691054
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Andrew Phillips <no...@github.com>.
> If @demobox is ok with the latest changes, I'm going to merge this one tonight.
Good to go for me. Thanks, @zack-shoylev !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20845572
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
Closed #14.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #8](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/8/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20643700
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -410,7 +410,7 @@ public void testGetUserWithHostname() {
> HttpResponse.builder().statusCode(200).payload(payloadFromResource("/user_get_withhost.json")).build()
> ).getUserApiForInstanceInZone("instanceId-1234-5678","RegionOne");
>
> - User user = api.get("exampleuser", "192.168.64.64");
> + User user = api.get("example.user", "192.168.64.64");
> assertEquals(user.getName(), "exampleuser");
Just curious, shouldn't the `user.getName()` now be `example.user`? It looks strange to have the `api.get(name, host)` returning something with a different name.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5078124
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
Merged!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20905905
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
This lgtm. Thanks @zack-shoylev !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20728792
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Zack Shoylev <no...@github.com>.
> @@ -410,7 +410,7 @@ public void testGetUserWithHostname() {
> HttpResponse.builder().statusCode(200).payload(payloadFromResource("/user_get_withhost.json")).build()
> ).getUserApiForInstanceInZone("instanceId-1234-5678","RegionOne");
>
> - User user = api.get("exampleuser", "192.168.64.64");
> + User user = api.get("example.user", "192.168.64.64");
> assertEquals(user.getName(), "exampleuser");
The result is based on the json file, if I changed that it would have required changing other tests as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5078389
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -410,7 +410,7 @@ public void testGetUserWithHostname() {
> HttpResponse.builder().statusCode(200).payload(payloadFromResource("/user_get_withhost.json")).build()
> ).getUserApiForInstanceInZone("instanceId-1234-5678","RegionOne");
>
> - User user = api.get("exampleuser", "192.168.64.64");
> + User user = api.get("example.user", "192.168.64.64");
> assertEquals(user.getName(), "exampleuser");
I think having expect tests behave like the real API, even if they are just a mock with dummy data, is usually a good idea. Along with live tests, expect ones are good examples of how to use an API, and this one caused me confusion (that's why I asked :)).
Personally, I think it would be good to change the json file even if that means changing assertions in other tests (change is quite trivial), but given that the test already tests the functionality it is okay to me as is. I was just curious :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5078774
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Zack Shoylev <no...@github.com>.
> @@ -410,7 +410,7 @@ public void testGetUserWithHostname() {
> HttpResponse.builder().statusCode(200).payload(payloadFromResource("/user_get_withhost.json")).build()
> ).getUserApiForInstanceInZone("instanceId-1234-5678","RegionOne");
>
> - User user = api.get("exampleuser", "192.168.64.64");
> + User user = api.get("example.user", "192.168.64.64");
> assertEquals(user.getName(), "exampleuser");
I agree - I did not want to push this change potentially out of scope for this simple fix. If that seems ok though, I can do the extra changes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5079038
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #178](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/178/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20694687
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #11](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/11/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20694731
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +
> +/**
> + * Encodes "." as %2e when getting a user with restricted hostname
> + *
> + * @author Zack Shoylev
> + *
> + */
> +@Singleton
> +public class EncodeDotsForUserGet implements HttpRequestFilter {
> +
> + @Override
> + public HttpRequest filter(HttpRequest request) throws HttpException {
> + String endpoint = request.getEndpoint().toString();
> + Pattern pattern = Pattern.compile("/[^/]*$"); // From last / to the end of the line
Extract as a constant?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5095965
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #12](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/12/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20694870
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Ignasi Barrera <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.openstack.trove.v1.filters;
> +
> +import java.util.regex.Matcher;
> +import java.util.regex.Pattern;
> +
> +import javax.inject.Singleton;
> +
> +import org.jclouds.http.HttpException;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +
> +/**
> + * Adds a timestamp to the query line so that cache is invalidated.
Copy/pasted javadoc?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5078378
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Zack Shoylev <no...@github.com>.
For some aesthetic reason I don't like making the Pattern const unless it's used in tight loops, but might as well do it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20699076
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #174](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/174/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20640198
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #9](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/9/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14#issuecomment-20644058
Re: [jclouds-labs-openstack] Fixes encoding of "." when listing a
single user (#14)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * Encodes "." as %2e when getting a user with restricted hostname
> + *
> + * @author Zack Shoylev
> + *
> + */
> +@Singleton
> +public class EncodeDotsForUserGet implements HttpRequestFilter {
> +
> + @Override
> + public HttpRequest filter(HttpRequest request) throws HttpException {
> + String endpoint = request.getEndpoint().toString();
> + Pattern pattern = Pattern.compile("/[^/]*$"); // From last / to the end of the line
> + Matcher matcher = pattern.matcher(endpoint);
> + matcher.find();
> + String encodable = matcher.group();
What if nothing is found? Or is the assumption that that cannot happen? If so, add a comment to that effect?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/14/files#r5096005