You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2013/05/29 19:36:27 UTC
[jclouds] Remove strings2 replaceall (#21)
Soliciting feedback since this is an API change.
You can merge this Pull Request by running:
git pull https://github.com/maginatics/jclouds remove-strings2-replaceall
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/21
-- Commit Summary --
* Remove Strings2.replaceAll(String, char, String)
* Remove Strings2.replaceAll(String, Pattern, String)
-- File Changes --
M apis/atmos/src/main/java/org/jclouds/atmos/filters/SignRequest.java (7)
M apis/s3/src/main/java/org/jclouds/s3/xml/ListBucketHandler.java (3)
M blobstore/src/main/java/org/jclouds/blobstore/functions/HttpGetOptionsListToGetOptions.java (5)
M common/azure/src/main/java/org/jclouds/azure/storage/filters/SharedKeyLiteAuthentication.java (2)
M common/trmk/src/main/java/org/jclouds/trmk/vcloud_0_8/binders/BindAddInternetServiceToXmlPayload.java (4)
M common/trmk/src/main/java/org/jclouds/trmk/vcloud_0_8/binders/BindAddNodeServiceToXmlPayload.java (2)
M core/src/main/java/org/jclouds/util/Strings2.java (27)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/21.patch
https://github.com/jclouds/jclouds/pull/21.diff
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Phillips <no...@github.com>.
> @@ -149,8 +146,8 @@ private void appendCanonicalizedHeaders(HttpRequest request, StringBuilder toSig
> // replacing any
> // newline characters and extra embedded white spaces in the value.
> for (String value : request.getHeaders().get(header)) {
> - value = Strings2.replaceAll(value, TWO_SPACE_PATTERN, " ");
> - value = Strings2.replaceAll(value, NEWLINE_PATTERN, "");
> + value = value.replace(" ", " ");
What should happen here if the input contains _three_ spaces? `"foo bar".replace(" ", " ")` returns `"foo bar"`, i.e. a string that still has two spaces.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21/files#r4441419
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-7-pull-requests #424 FAILURE
Some odd Checkstyle bug?
```
ERROR: Processing failed due to a bug in the code. Please report this to jenkinsci-users@googlegroups.com
java.lang.NullPointerException
at hudson.plugins.violations.hudson.maven.ViolationsMavenReporter.end(ViolationsMavenReporter.java:96)
at hudson.maven.AbstractMavenBuilder.end(AbstractMavenBuilder.java:103)
at hudson.maven.MavenModuleSetBuild$MavenModuleSetBuildExecution.doRun(MavenModuleSetBuild.java:794)
at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:510)
at hudson.model.Run.execute(Run.java:1505)
at hudson.maven.MavenModuleSetBuild.run(MavenModuleSetBuild.java:477)
at hudson.model.ResourceController.execute(ResourceController.java:88)
at hudson.model.Executor.run(Executor.java:237)
project=hudson.maven.MavenModuleSet@7d291c65[jclouds-java-7-pull-requests]
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21#issuecomment-18636344
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Gaul <no...@github.com>.
> @@ -149,8 +146,8 @@ private void appendCanonicalizedHeaders(HttpRequest request, StringBuilder toSig
> // replacing any
> // newline characters and extra embedded white spaces in the value.
> for (String value : request.getHeaders().get(header)) {
> - value = Strings2.replaceAll(value, TWO_SPACE_PATTERN, " ");
> - value = Strings2.replaceAll(value, NEWLINE_PATTERN, "");
> + value = value.replace(" ", " ");
Matches existing behavior.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21/files#r4488280
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #424](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/424/) FAILURE
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/pull/21#issuecomment-18635846
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Gaul <no...@github.com>.
This commit removes a low-value API and replaces calls with more succinct and idiomatic calls.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21#issuecomment-18782965
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Phillips <no...@github.com>.
@andrewgaul Thanks for the responses. Good to go for me!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21#issuecomment-18787320
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #57](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/57/) 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/pull/21#issuecomment-18635810
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Gaul <no...@github.com>.
Closed #21.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Gaul <no...@github.com>.
> - input = CHAR_TO_PATTERN.get(match).matcher(input).replaceAll(replacement);
> - } catch (ExecutionException e) {
> - throw new IllegalStateException("error creating pattern: " + match, e);
> - }
> - }
> - return input;
> - }
> -
> - private static final LoadingCache<Character, Pattern> CHAR_TO_PATTERN = CacheBuilder.newBuilder()
> - .<Character, Pattern> build(new CacheLoader<Character, Pattern>() {
> - @Override
> - public Pattern load(Character plain) {
> - return Pattern.compile(plain + "");
> - }
> - });
> -
Caching is now the caller's responsibility. In most cases they should allocate a static Pattern.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21/files#r4488279
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Phillips <no...@github.com>.
Reasoning behind this change? Is there a problem with `Strings2`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21#issuecomment-18636233
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Phillips <no...@github.com>.
> - input = CHAR_TO_PATTERN.get(match).matcher(input).replaceAll(replacement);
> - } catch (ExecutionException e) {
> - throw new IllegalStateException("error creating pattern: " + match, e);
> - }
> - }
> - return input;
> - }
> -
> - private static final LoadingCache<Character, Pattern> CHAR_TO_PATTERN = CacheBuilder.newBuilder()
> - .<Character, Pattern> build(new CacheLoader<Character, Pattern>() {
> - @Override
> - public Pattern load(Character plain) {
> - return Pattern.compile(plain + "");
> - }
> - });
> -
Getting rid of the pattern cache here...is that replaced by an internal cache in the JDK code somewhere? Or is caching here no longer required?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21/files#r4441484
Re: [jclouds] Remove strings2 replaceall (#21)
Posted by Andrew Gaul <no...@github.com>.
Committed as 64e9a4e4c6bd7aee05dd645510fd886484c7057c and d113b0ba63e1335ca4aa007036e0b0678934841f. I will submit a deprecation for 1.6.x after the 1.6.1 release settles.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/21#issuecomment-18874433