Repository: incubator-geode Updated Branches: refs/heads/develop 6a416532d -> 0b7ae4387
GEODE-247: Fix in executeQuery function to use the bucket parameter and added tests cases for it. * modification of the function executeQuery to use the parameter bucket rather than ignoring it. * added integration test QueryWithBucketParameterIntegrationTest which tests variations in the bucket parameter. * code improvements done as per the review comments in github pull request * placed MyValue, createAndPopulateSet and populateRegion into a separate file TestData.java as being reused by two test cases to avoid redundant code * modified BugJUnitTest to import the TestData package and replaced for loops with IntStreams and lambda functions. NOTE: This is an internal API which needs to be deprecated and replaced with an API without the bucket parameter. This closes #117 Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/0b7ae438 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/0b7ae438 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/0b7ae438 Branch: refs/heads/develop Commit: 0b7ae43874ec5ee82e36f0334d8c3fd6212e0e8e Parents: 6a41653 Author: nabarun <[email protected]> Authored: Thu Mar 17 17:32:23 2016 -0700 Committer: Jason Huynh <[email protected]> Committed: Tue Mar 22 10:52:02 2016 -0700 ---------------------------------------------------------------------- .../gemfire/internal/cache/LocalDataSet.java | 2 +- .../gemfire/cache/query/BugJUnitTest.java | 62 +++------ ...QueryWithBucketParameterIntegrationTest.java | 131 +++++++++++++++++++ .../gemfire/cache/query/data/TestData.java | 54 ++++++++ 4 files changed, 201 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/0b7ae438/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java index 02c8b75..f97f812 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java @@ -186,7 +186,7 @@ public final class LocalDataSet implements Region, QueryExecutor { QueryObserver indexObserver = query.startTrace(); try{ - result = this.proxy.executeQuery(query, parameters, getBucketSet()); + result = this.proxy.executeQuery(query, parameters, buckets); } finally { query.endTrace(indexObserver, startTime, result); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/0b7ae438/geode-core/src/test/java/com/gemstone/gemfire/cache/query/BugJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/BugJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/BugJUnitTest.java index 7a19321..14c2602 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/BugJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/BugJUnitTest.java @@ -22,18 +22,18 @@ */ package com.gemstone.gemfire.cache.query; +import static com.gemstone.gemfire.cache.query.data.TestData.createAndPopulateSet; import static org.junit.Assert.*; import java.io.Serializable; import java.util.*; +import java.util.stream.IntStream; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; -import junit.framework.*; - import com.gemstone.gemfire.cache.*; import com.gemstone.gemfire.cache.query.data.Portfolio; import com.gemstone.gemfire.cache.query.data.Position; @@ -44,6 +44,7 @@ import com.gemstone.gemfire.cache.query.internal.QueryUtils; import com.gemstone.gemfire.internal.cache.LocalDataSet; import com.gemstone.gemfire.internal.cache.PartitionedRegion; import com.gemstone.gemfire.test.junit.categories.IntegrationTest; +import com.gemstone.gemfire.cache.query.data.TestData.MyValue; /** * Tests reported bugs @@ -175,10 +176,7 @@ public class BugJUnitTest { CacheUtils.getLogger().fine(queryStr); r = q.execute(); CacheUtils.getLogger().fine(Utils.printResult(r)); - Set expectedSet = new HashSet(4); - for (int i = 0; i < 4; i++) { - expectedSet.add(new Integer(i)); - } + Set expectedSet = createAndPopulateSet(4); assertEquals(expectedSet, ((SelectResults)r).asSet()); // the following queries still fail because there is more than one @@ -218,9 +216,7 @@ public class BugJUnitTest { catch (TypeMismatchException e) { // expected due to bug 32251 } - - // the following queries, however, should work: // DebuggerSupport.waitForJavaDebugger(cache.getLogger()); queryStr = "Select distinct e.value.secId from /pos, getPositions(23) e"; @@ -409,16 +405,8 @@ public class BugJUnitTest { final PartitionedRegion pr2 = (PartitionedRegion)CacheUtils.getCache() .createRegion("pr2", factory.create()); - for (int i = 1; i <= 80; i++) { - pr1.put(i, new MyValue(i)); - if ((i % 2) == 0) { - pr2.put(i, new MyValue(i)); - } - } - Set<Integer> set = new HashSet<Integer>(); - for (int i = 0; i < 15; ++i) { - set.add(i); - } + createAllNumPRAndEvenNumPR(pr1, pr2, 80); + Set<Integer> set = createAndPopulateSet(15); LocalDataSet lds = new LocalDataSet(pr1, set); QueryObserverImpl observer = new QueryObserverImpl(); @@ -491,16 +479,8 @@ public class BugJUnitTest { final PartitionedRegion pr2 = (PartitionedRegion)CacheUtils.getCache() .createRegion("pr2", factory.create()); - for (int i = 1; i <= 80; i++) { - pr1.put(i, new MyValue(i)); - if ((i % 2) == 0) { - pr2.put(i, new MyValue(i)); - } - } - Set<Integer> set = new HashSet<Integer>(); - for (int i = 0; i < 15; ++i) { - set.add(i); - } + createAllNumPRAndEvenNumPR(pr1, pr2, 80); + Set<Integer> set = createAndPopulateSet(15); LocalDataSet lds = new LocalDataSet(pr1, set); QueryObserverImpl observer = new QueryObserverImpl(); @@ -521,28 +501,16 @@ public class BugJUnitTest { } - - static class MyValue implements Serializable, Comparable<MyValue> - { - public int value = 0; - - public MyValue(int value) { - this.value = value; - } - - - public int compareTo(MyValue o) - { - if(this.value > o.value) { - return 1; - }else if(this.value < o.value) { - return -1; - }else { - return 0; + private void createAllNumPRAndEvenNumPR(final PartitionedRegion pr1, final PartitionedRegion pr2, final int range) { + IntStream.rangeClosed(1,range).forEach(i -> { + pr1.put(i,new MyValue(i)); + if(i % 2 == 0){ + pr2.put(i, new MyValue(i)); } - } + }); } + class QueryObserverImpl extends QueryObserverAdapter { boolean isIndexesUsed = false; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/0b7ae438/geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryWithBucketParameterIntegrationTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryWithBucketParameterIntegrationTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryWithBucketParameterIntegrationTest.java new file mode 100644 index 0000000..f3602b9 --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryWithBucketParameterIntegrationTest.java @@ -0,0 +1,131 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.gemstone.gemfire.cache.query; + +import static com.gemstone.gemfire.cache.query.data.TestData.*; +import static org.junit.Assert.*; + +import java.io.Serializable; +import java.util.HashSet; +import java.util.Set; + +import com.gemstone.gemfire.cache.Cache; +import com.gemstone.gemfire.cache.EntryOperation; +import com.gemstone.gemfire.cache.PartitionAttributesFactory; +import com.gemstone.gemfire.cache.PartitionResolver; +import com.gemstone.gemfire.cache.RegionFactory; +import com.gemstone.gemfire.cache.RegionShortcut; +import com.gemstone.gemfire.cache.query.internal.DefaultQuery; +import com.gemstone.gemfire.internal.cache.LocalDataSet; +import com.gemstone.gemfire.internal.cache.PartitionedRegion; +import com.gemstone.gemfire.test.junit.categories.IntegrationTest; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +/** + * @implNote This test class has been designed to test the query execution using the LocalDataSet.executeQuery + * where one of the parameters passed is the bucket set to be used. + * Different variation of hashset variables are passed to the function to check for errors. + */ +@Category(IntegrationTest.class) +public class QueryWithBucketParameterIntegrationTest { + DefaultQuery queryExecutor; + LocalDataSet lds; + + public QueryWithBucketParameterIntegrationTest() {} + + @Before + public void setUp() throws Exception { + String regionName = "pr1"; + int totalBuckets = 40; + int numValues = 80; + CacheUtils.startCache(); + Cache cache = CacheUtils.getCache(); + PartitionAttributesFactory pAFactory = getPartitionAttributesFactoryWithPartitionResolver(totalBuckets); + RegionFactory rf = cache.createRegionFactory(RegionShortcut.PARTITION); + rf.setPartitionAttributes(pAFactory.create()); + PartitionedRegion pr1 = (PartitionedRegion) rf.create(regionName); + populateRegion(pr1, numValues); + QueryService qs = pr1.getCache().getQueryService(); + String query = "select distinct e1.value from /pr1 e1"; + queryExecutor = (DefaultQuery)CacheUtils.getQueryService().newQuery( + query); + Set<Integer> set = createAndPopulateSet(totalBuckets); + lds = new LocalDataSet(pr1, set); + } + + @After + public void tearDown() throws Exception { + CacheUtils.closeCache(); + } + + private PartitionAttributesFactory getPartitionAttributesFactoryWithPartitionResolver(int totalBuckets) { + PartitionAttributesFactory pAFactory = new PartitionAttributesFactory(); + pAFactory.setRedundantCopies(1).setTotalNumBuckets(totalBuckets).setPartitionResolver( + getPartitionResolver()); + return pAFactory; + } + + private PartitionResolver getPartitionResolver() { + return new PartitionResolver() { + public String getName() + { + return "PartitionResolverForTest"; + } + public Serializable getRoutingObject(EntryOperation opDetails) + { + return (Serializable)opDetails.getKey(); + } + public void close() {} + }; + } + + @Test + public void testQueryExecuteWithEmptyBucketListExpectNoResults() throws Exception + { + SelectResults r = (SelectResults)lds.executeQuery(queryExecutor, null, new HashSet<Integer>()); + assertTrue("Received: A non-empty result collection, expected : Empty result collection", r.isEmpty()); + } + + @Test + public void testQueryExecuteWithNullBucketListExpectNonEmptyResultSet() throws Exception + { + SelectResults r = (SelectResults)lds.executeQuery(queryExecutor, null, null); + assertFalse("Received: An empty result collection, expected : Non-empty result collection", r.isEmpty()); + } + + @Test + public void testQueryExecuteWithNonEmptyBucketListExpectNonEmptyResultSet() throws Exception + { + int nTestBucketNumber = 15; + Set<Integer> nonEmptySet = createAndPopulateSet(nTestBucketNumber); + SelectResults r = (SelectResults)lds.executeQuery(queryExecutor, null, nonEmptySet); + assertFalse("Received: An empty result collection, expected : Non-empty result collection", r.isEmpty()); + } + + @Test(expected = QueryInvocationTargetException.class) + public void testQueryExecuteWithLargerBucketListThanExistingExpectQueryInvocationTargetException() throws Exception + { + int nTestBucketNumber = 45; + Set<Integer> overflowSet = createAndPopulateSet(nTestBucketNumber); + SelectResults r = (SelectResults)lds.executeQuery(queryExecutor, null, overflowSet); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/0b7ae438/geode-core/src/test/java/com/gemstone/gemfire/cache/query/data/TestData.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/data/TestData.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/data/TestData.java new file mode 100644 index 0000000..6348724 --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/data/TestData.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.gemstone.gemfire.cache.query.data; + +import java.io.Serializable; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import com.gemstone.gemfire.cache.Region; + +public class TestData { + public static void populateRegion(Region region, int numValues) { + IntStream.rangeClosed(1,numValues).forEach(i -> region.put(i,new MyValue(i))); + } + + public static Set<Integer> createAndPopulateSet(int nBuckets) { + return new HashSet<Integer>(IntStream.range(0,nBuckets).boxed().collect(Collectors.toSet())); + } + + public static class MyValue implements Serializable, Comparable<MyValue> + { + public int value = 0; + public MyValue(int value) { + this.value = value; + } + public int compareTo(MyValue o) + { + if(this.value > o.value) { + return 1; + }else if(this.value < o.value) { + return -1; + }else { + return 0; + } + } + } +}
