BrooklynNetworkUtils.portRulesToRanges: code tidy Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/9886c695 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/9886c695 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/9886c695
Branch: refs/heads/master Commit: 9886c695d38b1ee1fda028ae4767b5fcc2f1a2eb Parents: 2a950a5 Author: Aled Sage <aled.s...@gmail.com> Authored: Thu Aug 11 20:24:20 2016 +0100 Committer: Aled Sage <aled.s...@gmail.com> Committed: Thu Aug 11 20:24:20 2016 +0100 ---------------------------------------------------------------------- .../util/core/BrooklynNetworkUtils.java | 15 ++--- .../apache/brooklyn/util/net/Networking.java | 17 ++++-- .../org/apache/brooklyn/util/text/Strings.java | 11 ++++ .../brooklyn/util/net/NetworkingUtilsTest.java | 62 +++++++++++--------- .../apache/brooklyn/util/text/StringsTest.java | 10 ++++ 5 files changed, 70 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9886c695/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java index 2451844..33733b1 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java @@ -18,22 +18,19 @@ */ package org.apache.brooklyn.util.core; -import com.google.common.base.Preconditions; -import com.google.common.collect.Range; -import com.google.common.collect.RangeSet; -import com.google.common.collect.TreeRangeSet; +import java.net.InetAddress; + import org.apache.brooklyn.core.location.geo.LocalhostExternalIpLoader; import org.apache.brooklyn.core.server.BrooklynServiceAttributes; import org.apache.brooklyn.util.JavaGroovyEquivalents; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.net.Networking; -import java.net.InetAddress; -import java.util.Collection; - /** * <tt>BrooklynNetworkUtils</tt> is for utility methods that rely on some other part(s) of Brooklyn, * or seem too custom in how they are used/configured to be considered a "common utility". + * + * See {@link Networking} for more generic network utilities. */ public class BrooklynNetworkUtils { @@ -47,8 +44,4 @@ public class BrooklynNetworkUtils { return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(), Networking.getLocalHost()), InetAddress.class); } - - private static Range<Integer> closedRange(String from, String to) { - return Range.closed(Integer.parseInt(from), Integer.parseInt(to)); - } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9886c695/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java index a561032..5d3c69f 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java @@ -43,6 +43,7 @@ import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.text.Identifiers; +import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,7 +55,8 @@ import com.google.common.net.HostAndPort; import com.google.common.primitives.UnsignedBytes; /** - * <tt>Networking</tt> is for generic network utility methods. + * Generic network utility methods (which are generally not specific to Brooklyn configuration, or + * brooklyn use-cases). */ public class Networking { @@ -240,7 +242,10 @@ public class Networking { for (String portRule : portRules) { if (portRule.contains("-")) { String[] fromTo = portRule.split("-"); - Preconditions.checkState(fromTo.length == 2, "Invalid port range '%s'", portRule); + checkArgument(fromTo.length == 2, "Invalid port range '%s'", portRule); + checkArgument(Strings.countOccurrences(portRule, '-') == 1, "Invalid port range '%s'", portRule); + checkArgument(Strings.isNonEmpty(fromTo[0]), "Invalid port range '%s'", portRule); + checkArgument(Strings.isNonEmpty(fromTo[1]), "Invalid port range '%s'", portRule); result.add(closedRange(fromTo[0], fromTo[1])); } else { result.add(closedRange(portRule, portRule)); @@ -252,10 +257,10 @@ public class Networking { private static Range<Integer> closedRange(String from, String to) { Integer fromPort = Integer.parseInt(from); Integer toPort = Integer.parseInt(to); - Preconditions.checkArgument(fromPort >= MIN_PORT_NUMBER && fromPort <= MAX_PORT_NUMBER, "fromPort %s should be a number between %s and %s", fromPort, MIN_PORT_NUMBER, MAX_PORT_NUMBER); - Preconditions.checkArgument(toPort >= MIN_PORT_NUMBER && toPort <= MAX_PORT_NUMBER, "toPort %s should be a number between %s and %s", toPort, MIN_PORT_NUMBER, MAX_PORT_NUMBER); - Preconditions.checkArgument(fromPort <= toPort, "fromNumber should be less or equal than toPort %s <= %s", fromPort, toPort); - return Range.closed(Integer.parseInt(from), Integer.parseInt(to)); + checkArgument(isPortValid(fromPort), "fromPort %s should be a number between %s and %s", fromPort, MIN_PORT_NUMBER, MAX_PORT_NUMBER); + checkArgument(isPortValid(toPort), "toPort %s should be a number between %s and %s", toPort, MIN_PORT_NUMBER, MAX_PORT_NUMBER); + checkArgument(fromPort <= toPort, "fromPort %s should be less than or equal to toPort %s", fromPort, toPort); + return Range.closed(fromPort, toPort); } /** http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9886c695/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java index 6e3afab..b9168cc 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java @@ -937,4 +937,15 @@ public class Strings { public static List<String> parseCsv(String csv) { return parseCsv(csv, ","); } + + public static int countOccurrences(String phrase, char target) { + if (phrase == null) return 0; + int result = 0; + for (int i = 0; i < phrase.length(); i++) { + if (target == phrase.charAt(i)) { + result++; + } + } + return result; + } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9886c695/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java index 80fbe67..1afffd7 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/net/NetworkingUtilsTest.java @@ -33,22 +33,20 @@ import java.net.UnknownHostException; import java.util.Collection; import java.util.concurrent.TimeUnit; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableRangeSet; -import com.google.common.collect.Range; -import com.google.common.collect.RangeSet; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.javalang.JavaClassNames; -import org.apache.brooklyn.util.net.Networking; import org.apache.brooklyn.util.text.Identifiers; -import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.Test; import com.google.common.base.Stopwatch; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableRangeSet; +import com.google.common.collect.Range; +import com.google.common.collect.RangeSet; import com.google.common.net.HostAndPort; public class NetworkingUtilsTest { @@ -98,8 +96,7 @@ public class NetworkingUtilsTest { @Test public void testPortRulesToRanges() throws Exception { RangeSet<Integer> actualRangeSet = Networking.portRulesToRanges(ImmutableList.of( - "22", "23", "5000-6000", "8081", "80-90", "90-100", "23", - "8081")); + "22", "23", "5000-6000", "8081", "80-90", "90-100", "23", "8081")); Asserts.assertEquals(actualRangeSet, ImmutableRangeSet.<Integer>builder() .add(Range.closed(22, 22)) @@ -111,30 +108,39 @@ public class NetworkingUtilsTest { } @Test - public void testPortRulesToRangesWithWrongRanges() throws Exception { - try { - Networking.portRulesToRanges(ImmutableList.of("-1")); - fail("Have to fail parsing"); - } catch (IllegalArgumentException e) { - Asserts.assertTrue(e instanceof NumberFormatException); - Asserts.assertEquals(e.getMessage(), "For input string: \"\""); - } - - assertCheckException(ImmutableList.of("68000"), Strings.format("fromPort 68000 should be a number between %s and %s", - Networking.MIN_PORT_NUMBER, Networking.MAX_PORT_NUMBER).toString()); + public void testPortRulesToRangesWithInvalidRanges() throws Exception { + assertPortRulesException(ImmutableList.of("-1"), "Invalid port range '-1'"); + assertPortRulesException(ImmutableList.of("1-"), "Invalid port range '1-'"); + assertPortRulesException(ImmutableList.of("-"), "Invalid port range '-'"); + assertPortRulesException(ImmutableList.of("1-2-"), "Invalid port range '1-2-'"); + assertPortRulesException(ImmutableList.of("-1-2"), "Invalid port range '-1-2'"); + assertPortRulesException(ImmutableList.of("1-2-3"), "Invalid port range '1-2-3'"); + assertPortRulesException(ImmutableList.of("a"), NumberFormatException.class, "For input string: \"a\""); + assertPortRulesException(ImmutableList.of("1-a"), NumberFormatException.class, "For input string: \"a\""); + assertPortRulesException(ImmutableList.of("a-2"), NumberFormatException.class, "For input string: \"a\""); + } - assertCheckException(ImmutableList.of("67000-67000"), "fromPort 67000 should be a number between 1 and 65535"); - assertCheckException(ImmutableList.of("0-67000"), "fromPort 0 should be a number between 1 and 65535"); - assertCheckException(ImmutableList.of("300-67000"), "toPort 67000 should be a number between 1 and 65535"); - assertCheckException(ImmutableList.of("2-1"), "fromNumber should be less or equal than toPort 2 <= 1"); + @Test + public void testPortRulesToRangesWithOutOfRange() throws Exception { + assertPortRulesException(ImmutableList.of("68000"), String.format("fromPort 68000 should be a number between %s and %s", + Networking.MIN_PORT_NUMBER, Networking.MAX_PORT_NUMBER)); + assertPortRulesException(ImmutableList.of("67000-67000"), "fromPort 67000 should be a number between 1 and 65535"); + assertPortRulesException(ImmutableList.of("0-67000"), "fromPort 0 should be a number between 1 and 65535"); + assertPortRulesException(ImmutableList.of("300-67000"), "toPort 67000 should be a number between 1 and 65535"); + assertPortRulesException(ImmutableList.of("2-1"), "fromPort 2 should be less than or equal to toPort 1"); } - private void assertCheckException(Collection<String> ports, String message) { + private void assertPortRulesException(Collection<String> portRules, String message) { + assertPortRulesException(portRules, IllegalArgumentException.class, message); + } + + private void assertPortRulesException(Collection<String> portRules, Class<? extends Exception> type, String message) { try { - Networking.portRulesToRanges(ports); - fail("Should have failed"); - } catch (IllegalArgumentException e) { - Asserts.assertEquals(e.getMessage(), message); + Networking.portRulesToRanges(portRules); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + if (!type.isInstance(e)) throw e; + Asserts.expectedFailureContains(e, message); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9886c695/utils/common/src/test/java/org/apache/brooklyn/util/text/StringsTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/text/StringsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/text/StringsTest.java index cc758ab..c3f6d92 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/text/StringsTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/text/StringsTest.java @@ -385,4 +385,14 @@ public class StringsTest extends FixedLocaleTest { Assert.assertEquals(0, Strings.parseCsv("", ",").size()); Assert.assertEquals(0, Strings.parseCsv(" ", ",").size()); } + + @Test + public void testCountOccurrences() throws Exception { + assertEquals(Strings.countOccurrences(null, 'a'), 0); + assertEquals(Strings.countOccurrences("", 'a'), 0); + assertEquals(Strings.countOccurrences("b", 'a'), 0); + assertEquals(Strings.countOccurrences("a", 'a'), 1); + assertEquals(Strings.countOccurrences("aa", 'a'), 2); + assertEquals(Strings.countOccurrences("abba", 'a'), 2); + } }
