You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by pi...@apache.org on 2018/04/30 22:04:44 UTC
[geode] branch develop updated: GEODE-3881: Replace bespoke random
string generation with extant three-phrase generator. (#1880)
This is an automated email from the ASF dual-hosted git repository.
pivotalsarge pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 2334b1b GEODE-3881: Replace bespoke random string generation with extant three-phrase generator. (#1880)
2334b1b is described below
commit 2334b1b9cc4dce107e035424abfb4c4751547082
Author: Michael "Sarge" Dodge <md...@pivotal.io>
AuthorDate: Mon Apr 30 15:04:40 2018 -0700
GEODE-3881: Replace bespoke random string generation with extant three-phrase generator. (#1880)
* GEODE-3881: Replace bespoke random string generation with extant three-phrase generator.
- Add unit test for three-phrase generator to ensure compatibility with region names.
- Rename parameter to improve clarity.
- Remove dead code.
* GEODE-3881: Why does the spotlessApply task not work consistently?!
* GEODE-3881: Change assertion messages to indicate failures.
---
.../geode/redis/internal/RegionProvider.java | 27 ++++-------
.../cli/util/ThreePhraseGeneratorTest.java | 26 +++++++++--
.../java/org/apache/geode/redis/SetsJUnitTest.java | 53 ++++++++--------------
3 files changed, 51 insertions(+), 55 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/redis/internal/RegionProvider.java b/geode-core/src/main/java/org/apache/geode/redis/internal/RegionProvider.java
index da84150..a9d3cfb 100644
--- a/geode-core/src/main/java/org/apache/geode/redis/internal/RegionProvider.java
+++ b/geode-core/src/main/java/org/apache/geode/redis/internal/RegionProvider.java
@@ -56,7 +56,6 @@ import org.apache.geode.redis.internal.executor.SortedSetQuery;
* synchronized, which is done away with and abstracted by this class.
*/
public class RegionProvider implements Closeable {
-
private final ConcurrentHashMap<ByteArrayWrapper, Region<?, ?>> regions;
/**
@@ -390,24 +389,24 @@ public class RegionProvider implements Closeable {
* This method creates a Region globally with the given name. If there is an error in the
* creation, a runtime exception will be thrown.
*
- * @param key Name of Region to create
+ * @param regionPath Name of Region to create
* @return Region Region created globally
*/
- private Region<?, ?> createRegionGlobally(String key) {
+ private Region<?, ?> createRegionGlobally(String regionPath) {
Region<?, ?> r = null;
- r = cache.getRegion(key);
+ r = cache.getRegion(regionPath);
if (r != null)
return r;
do {
createRegionCmd.setCache(cache);
- Result result = createRegionCmd.createRegion(key, defaultRegionType, null, null, true, null,
- null, null, null, null, null, null, null, false, false, true, false, false, false, true,
- null, null, null, null, null, null, null, null, null, null, null, null, null, false, null,
- null, null, null, null, null, null, null, null, null, null);
+ Result result = createRegionCmd.createRegion(regionPath, defaultRegionType, null, null, true,
+ null, null, null, null, null, null, null, null, false, false, true, false, false, false,
+ true, null, null, null, null, null, null, null, null, null, null, null, null, null, false,
+ null, null, null, null, null, null, null, null, null, null, null);
- r = cache.getRegion(key);
+ r = cache.getRegion(regionPath);
if (result.getStatus() == Status.ERROR && r == null) {
- String err = "";
+ String err = "Unable to create region named \"" + regionPath + "\":\n";
while (result.hasNextLine())
err += result.nextLine();
throw new RegionCreationException(err);
@@ -419,13 +418,6 @@ public class RegionProvider implements Closeable {
public Query getQuery(ByteArrayWrapper key, Enum<?> query) {
return this.preparedQueries.get(key).get(query);
- /*
- * if (query instanceof ListQuery) { return
- * this.queryService.newQuery(((ListQuery)query).getQueryString(this.regions.get(key).
- * getFullPath())); } else { return
- * this.queryService.newQuery(((SortedSetQuery)query).getQueryString(this.regions.get(key).
- * getFullPath())); }
- */
}
/**
@@ -562,5 +554,4 @@ public class RegionProvider implements Closeable {
}
return builder.toString();
}
-
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/util/ThreePhraseGeneratorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/util/ThreePhraseGeneratorTest.java
index fca2a71..f5b2c03 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/util/ThreePhraseGeneratorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/util/ThreePhraseGeneratorTest.java
@@ -16,6 +16,7 @@ package org.apache.geode.management.internal.cli.util;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -26,10 +27,27 @@ import org.apache.geode.test.junit.categories.UnitTest;
public class ThreePhraseGeneratorTest {
@Test
public void testGenerate() {
- ThreePhraseGenerator tpg = new ThreePhraseGenerator();
- String phrase = tpg.generate('-');
+ final ThreePhraseGenerator tpg = new ThreePhraseGenerator();
- assertNotNull(phrase);
- assertNotEquals("", phrase);
+ final String phrase = tpg.generate('-');
+ assertNotNull("Generated string is null", phrase);
+ assertNotEquals("Generated string is empty", "", phrase);
+ }
+
+ @Test
+ public void testGeneratesOnlyLetters() {
+ final ThreePhraseGenerator tpg = new ThreePhraseGenerator();
+
+ // Since there is a random choice of word, try a bunch of times to get coverage.
+ for (int attempt = 1; attempt < 50000; ++attempt) {
+ final String phrase = tpg.generate('x');
+ assertTrue("Generated string does not have at least five characters", 5 <= phrase.length());
+ for (int index = 0; index < phrase.length(); ++index) {
+ final char c = phrase.charAt(index);
+ assertTrue(
+ "Character in \"" + phrase + "\" at index " + index + ", '" + c + "', is a not letter",
+ Character.isLetter(c));
+ }
+ }
}
}
diff --git a/geode-core/src/test/java/org/apache/geode/redis/SetsJUnitTest.java b/geode-core/src/test/java/org/apache/geode/redis/SetsJUnitTest.java
index eed971c..71df401 100755
--- a/geode-core/src/test/java/org/apache/geode/redis/SetsJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/redis/SetsJUnitTest.java
@@ -23,7 +23,6 @@ import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
-import java.util.Random;
import java.util.Set;
import org.junit.After;
@@ -36,6 +35,7 @@ import redis.clients.jedis.Jedis;
import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.GemFireCache;
import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
import org.apache.geode.test.junit.categories.IntegrationTest;
import org.apache.geode.test.junit.categories.RedisTest;
@@ -44,12 +44,11 @@ public class SetsJUnitTest {
private static Jedis jedis;
private static GeodeRedisServer server;
private static GemFireCache cache;
- private static Random rand;
+ private static ThreePhraseGenerator generator = new ThreePhraseGenerator();
private static int port = 6379;
@BeforeClass
public static void setUp() throws IOException {
- rand = new Random();
CacheFactory cf = new CacheFactory();
cf.set(LOG_LEVEL, "error");
cf.set(MCAST_PORT, "0");
@@ -66,9 +65,9 @@ public class SetsJUnitTest {
public void testSAddScard() {
int elements = 10;
Set<String> strings = new HashSet<String>();
- String key = randString();
+ String key = generator.generate('x');
for (int i = 0; i < elements; i++) {
- String elem = randString();
+ String elem = generator.generate('x');
strings.add(elem);
}
String[] stringArray = strings.toArray(new String[strings.size()]);
@@ -82,9 +81,9 @@ public class SetsJUnitTest {
public void testSMembersIsMember() {
int elements = 10;
Set<String> strings = new HashSet<String>();
- String key = randString();
+ String key = generator.generate('x');
for (int i = 0; i < elements; i++) {
- String elem = randString();
+ String elem = generator.generate('x');
strings.add(elem);
}
String[] stringArray = strings.toArray(new String[strings.size()]);
@@ -102,13 +101,13 @@ public class SetsJUnitTest {
@Test
public void testSMove() {
- String source = randString();
- String dest = randString();
- String test = randString();
+ String source = generator.generate('x');
+ String dest = generator.generate('x');
+ String test = generator.generate('x');
int elements = 10;
Set<String> strings = new HashSet<String>();
for (int i = 0; i < elements; i++) {
- String elem = randString();
+ String elem = generator.generate('x');
strings.add(elem);
}
String[] stringArray = strings.toArray(new String[strings.size()]);
@@ -123,7 +122,7 @@ public class SetsJUnitTest {
i++;
}
- assertTrue(jedis.smove(test, dest, randString()) == 0);
+ assertTrue(jedis.smove(test, dest, generator.generate('x')) == 0);
}
@Test
@@ -133,10 +132,10 @@ public class SetsJUnitTest {
String[] keys = new String[numSets];
ArrayList<Set<String>> sets = new ArrayList<Set<String>>();
for (int j = 0; j < numSets; j++) {
- keys[j] = randString();
+ keys[j] = generator.generate('x');
Set<String> newSet = new HashSet<String>();
for (int i = 0; i < elements; i++)
- newSet.add(randString());
+ newSet.add(generator.generate('x'));
sets.add(newSet);
}
@@ -152,7 +151,7 @@ public class SetsJUnitTest {
assertEquals(result, jedis.sdiff(keys));
- String destination = randString();
+ String destination = generator.generate('x');
jedis.sdiffstore(destination, keys);
@@ -168,10 +167,10 @@ public class SetsJUnitTest {
String[] keys = new String[numSets];
ArrayList<Set<String>> sets = new ArrayList<Set<String>>();
for (int j = 0; j < numSets; j++) {
- keys[j] = randString();
+ keys[j] = generator.generate('x');
Set<String> newSet = new HashSet<String>();
for (int i = 0; i < elements; i++)
- newSet.add(randString());
+ newSet.add(generator.generate('x'));
sets.add(newSet);
}
@@ -187,7 +186,7 @@ public class SetsJUnitTest {
assertEquals(result, jedis.sunion(keys));
- String destination = randString();
+ String destination = generator.generate('x');
jedis.sunionstore(destination, keys);
@@ -203,10 +202,10 @@ public class SetsJUnitTest {
String[] keys = new String[numSets];
ArrayList<Set<String>> sets = new ArrayList<Set<String>>();
for (int j = 0; j < numSets; j++) {
- keys[j] = randString();
+ keys[j] = generator.generate('x');
Set<String> newSet = new HashSet<String>();
for (int i = 0; i < elements; i++)
- newSet.add(randString());
+ newSet.add(generator.generate('x'));
sets.add(newSet);
}
@@ -222,7 +221,7 @@ public class SetsJUnitTest {
assertEquals(result, jedis.sinter(keys));
- String destination = randString();
+ String destination = generator.generate('x');
jedis.sinterstore(destination, keys);
@@ -231,18 +230,6 @@ public class SetsJUnitTest {
assertEquals(result, destResult);
}
- private String randString() {
- int length = rand.nextInt(8) + 5;
- StringBuilder rString = new StringBuilder();
- for (int i = 0; i < length; i++) {
- // Use random upper- and lower-case letters only. Using punctuation characters,
- // namely the underscore, can lead to spurious test failures due to invalid region
- // names, e.g., two leading underscores.
- rString.append((char) (65 + rand.nextInt(26) + (rand.nextBoolean() ? 26 : 0)));
- }
- return rString.toString();
- }
-
@After
public void flushAll() {
jedis.flushAll();
--
To stop receiving notification emails like this one, please contact
pivotalsarge@apache.org.