Mercurial > hg > release > thermostat-1.2
changeset 1568:73fdadf117c5
Implement distinct query support.
Reviewed-by: vanaltj, omajid, neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-November/011462.html
Original review thread started here:
http://icedtea.classpath.org/pipermail/thermostat/2014-June/009985.html
line wrap: on
line diff
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/AggregateQuery.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/AggregateQuery.java Thu Jun 05 09:33:45 2014 +0200 @@ -48,12 +48,16 @@ /** * Aggregate records by counting them. */ - COUNT + COUNT, + /** + * Find distinct values for a {@link Key} + */ + DISTINCT, } protected final Query<T> queryToAggregate; private final AggregateFunction function; - + public AggregateQuery(AggregateFunction function, Query<T> queryToAggregate) { this.function = function; this.queryToAggregate = queryToAggregate; @@ -87,6 +91,6 @@ public AggregateFunction getAggregateFunction() { return this.function; } - + }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/BackingStorage.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/BackingStorage.java Thu Jun 05 09:33:45 2014 +0200 @@ -53,6 +53,7 @@ <T extends Pojo> Query<T> createQuery(Category<T> category); + // FIXME: Thermostat 2.0 return AggregateQuery2 (or merged version of AggregateQuery) instead <T extends Pojo> Query<T> createAggregateQuery(AggregateFunction function, Category<T> category); <T extends Pojo> Add<T> createAdd(Category<T> category);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/experimental/AggregateQuery2.java Thu Jun 05 09:33:45 2014 +0200 @@ -0,0 +1,79 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.core.experimental; + +import java.util.Objects; + +import com.redhat.thermostat.storage.core.AggregateQuery; +import com.redhat.thermostat.storage.core.Key; +import com.redhat.thermostat.storage.core.Query; +import com.redhat.thermostat.storage.model.Pojo; + +/** + * Successor of {@link AggregateQuery} with improved support for agregating on + * a key value. + * + * @param <T> + */ +// FIXME: Thermostat 2.0 Merge into AggregateQuery +public abstract class AggregateQuery2<T extends Pojo> extends AggregateQuery<T> { + + // optional Key to aggregate values for + private Key<?> aggregateKey; + + public AggregateQuery2(AggregateFunction function, Query<T> queryToAggregate) { + super(function, queryToAggregate); + } + + /** + * + * @return An optional {@link Key} to aggregate values for. May be + * {@code null}; + */ + public Key<?> getAggregateKey() { + return aggregateKey; + } + + /** + * Sets an optional {@link Key} to aggregate values for. + * @param aggregateKey Must not be {@code null}. + * @throws NullPointerException If the aggregate key was {@code null} + */ + public void setAggregateKey(Key<?> aggregateKey) { + this.aggregateKey = Objects.requireNonNull(aggregateKey); + } +}
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParser.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParser.java Thu Jun 05 09:33:45 2014 +0200 @@ -39,7 +39,12 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.storage.core.Add; import com.redhat.thermostat.storage.core.AggregateQuery.AggregateFunction; import com.redhat.thermostat.storage.core.BackingStorage; @@ -53,6 +58,7 @@ import com.redhat.thermostat.storage.core.Replace; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.storage.query.BinaryComparisonOperator; import com.redhat.thermostat.storage.query.BinaryLogicalOperator; @@ -125,16 +131,23 @@ */ class BasicDescriptorParser<T extends Pojo> implements StatementDescriptorParser<T> { + private static final Logger logger = LoggingUtils.getLogger(BasicDescriptorParser.class); private static final String TOKEN_DELIMS = " \t\r\n\f"; private static final short IDX_QUERY = 0; - private static final short IDX_QUERY_COUNT = 1; - private static final short IDX_ADD = 2; - private static final short IDX_REPLACE = 3; - private static final short IDX_UPDATE = 4; - private static final short IDX_REMOVE = 5; + private static final short IDX_ADD = 1; + private static final short IDX_REPLACE = 2; + private static final short IDX_UPDATE = 3; + private static final short IDX_REMOVE = 4; private static final String[] KNOWN_STATEMENT_TYPES = new String[] { - "QUERY", "QUERY-COUNT", "ADD", "REPLACE", "UPDATE", "REMOVE" + "QUERY", "ADD", "REPLACE", "UPDATE", "REMOVE" }; + + // package-private for testing + static final String AGGREGATE_PARAM_REGEXP = "(?:\\(([a-zA-Z_]+)\\))?$"; + private static final String QUERY_COUNT_REGEXP = "QUERY-COUNT" + AGGREGATE_PARAM_REGEXP; + private static final String QUERY_DISTINCT_REGEXP = "QUERY-DISTINCT" + AGGREGATE_PARAM_REGEXP; + private static final Pattern QUERY_COUNT_PATTERN = Pattern.compile(QUERY_COUNT_REGEXP); + private static final Pattern QUERY_DISTINCT_PATTERN = Pattern.compile(QUERY_DISTINCT_REGEXP); private static final String SORTLIST_SEP = ","; private static final String SETLIST_SEP = SORTLIST_SEP; private static final String KEYWORD_SET = "SET"; @@ -754,14 +767,12 @@ // matchStatementType and matchCategory advanced currTokenIndex, // lets use idx of 0 here. final String statementType = tokens[0]; + Matcher queryCountMatcher = QUERY_COUNT_PATTERN.matcher(statementType); + Matcher queryDistinctMatcher = QUERY_DISTINCT_PATTERN.matcher(statementType); if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_QUERY])) { // regular query case Query<T> query = storage.createQuery(desc.getCategory()); this.parsedStatement = new ParsedStatementImpl<>(query); - } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_QUERY_COUNT])) { - // create aggregate count query - Query<T> query = storage.createAggregateQuery(AggregateFunction.COUNT, desc.getCategory()); - this.parsedStatement = new ParsedStatementImpl<>(query); } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_ADD])) { // create add Add<T> add = storage.createAdd(desc.getCategory()); @@ -778,10 +789,37 @@ // create remove Remove<T> remove = storage.createRemove(desc.getCategory()); this.parsedStatement = new ParsedStatementImpl<>(remove); + } else if (queryCountMatcher.matches()) { + this.parsedStatement = createAggregatePreparedStatement(AggregateFunction.COUNT, queryCountMatcher); + } else if (queryDistinctMatcher.matches()) { + this.parsedStatement = createAggregatePreparedStatement(AggregateFunction.DISTINCT, queryDistinctMatcher); } else { throw new IllegalStateException("Don't know how to create statement type '" + statementType + "'"); } } + + private ParsedStatementImpl<T> createAggregatePreparedStatement(final AggregateFunction function, final Matcher matcher) { + // create aggregate query + Query<T> query = storage.createAggregateQuery(function, desc.getCategory()); + if (!(query instanceof AggregateQuery2)) { + // FIXME: Thermostat 2.0 BackingStorage.createAggregateQuery() should + // return a merged version of AggregateQuery (i.e. AggregateQuery2). + // Thus, this check can go away then. + // For 1.2 we have this in order to be API backwards compatible. + logger.log(Level.WARNING, "Expected AggregateQuery2. This will no longer work for Thermostat 2.0. Stmt was: " + desc); + return new ParsedStatementImpl<>(query); + } + AggregateQuery2<T> aggregateQuery = (AggregateQuery2<T>)query; + // We'll always have a match for at least one group. That group + // will be the keyName to use (if any). For old query descriptors + // the keyName may be null + String keyName = matcher.group(1); // groups start at 1 + if (keyName != null) { + Key<?> aggKey = new Key<>(keyName); + aggregateQuery.setAggregateKey(aggKey); + } + return new ParsedStatementImpl<>(aggregateQuery); + } private void matchCategory() throws DescriptorParsingException { if (currTokenIndex >= tokens.length) { @@ -799,26 +837,32 @@ } private void matchStatementType() throws DescriptorParsingException { - if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_QUERY])) { + final String statementType = tokens[currTokenIndex]; + Matcher queryCountMatcher = QUERY_COUNT_PATTERN.matcher(statementType); + Matcher queryDistinctMatcher = QUERY_DISTINCT_PATTERN.matcher(statementType); + if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_QUERY])) { // QUERY currTokenIndex++; - } else if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_QUERY_COUNT])) { - // QUERY-COUNT - currTokenIndex++; - } else if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_ADD])) { + } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_ADD])) { // ADD currTokenIndex++; - } else if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_REPLACE])) { + } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_REPLACE])) { // REPLACE currTokenIndex++; - } else if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_UPDATE])) { + } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_UPDATE])) { // UPDATE currTokenIndex++; - } else if (tokens[currTokenIndex].equals(KNOWN_STATEMENT_TYPES[IDX_REMOVE])) { + } else if (statementType.equals(KNOWN_STATEMENT_TYPES[IDX_REMOVE])) { // REMOVE currTokenIndex++; + } else if (queryCountMatcher.matches()) { + // QUERY-COUNT + currTokenIndex++; + } else if (queryDistinctMatcher.matches()) { + // QUERY-DISTINCT + currTokenIndex++; } else { - throw new DescriptorParsingException("Unknown statement type: '" + tokens[currTokenIndex] + "'"); + throw new DescriptorParsingException("Unknown statement type: '" + statementType + "'"); } } }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParser.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParser.java Thu Jun 05 09:33:45 2014 +0200 @@ -55,6 +55,7 @@ import com.redhat.thermostat.storage.core.Statement; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.model.Pojo; class SemanticsEnabledDescriptorParser<T extends Pojo> extends @@ -109,13 +110,28 @@ String msg = "SET not allowed for REMOVE"; throw new DescriptorParsingException(msg); } - // matches for QUERY/QUERY-COUNT + // matches for QUERY/QUERY-COUNT/QUERY-DISTINCT if (stmt instanceof Query) { if (setList.getValues().size() > 0) { // Must not have SET for QUERYs String msg = "SET not allowed for QUERY/QUERY-COUNT"; throw new DescriptorParsingException(msg); } + if (stmt instanceof AggregateQuery2) { + AggregateQuery2<T> aggQuery = (AggregateQuery2<T>)stmt; + switch (aggQuery.getAggregateFunction()) { + case COUNT: + // count queries need a sane key param if present + performKeyParamChecksAllowNull(aggQuery); + break; + case DISTINCT: + // distinct queries must have a known key + performKeyParamChecks(aggQuery); + break; + default: + throw new IllegalStateException("Unknown aggregate function: " + aggQuery.getAggregateFunction()); + } + } } else { assert(stmt instanceof DataModifyingStatement); // only queries can have sort/limit expressions @@ -126,6 +142,26 @@ } } + private void performKeyParamChecksAllowNull(AggregateQuery2<T> aggQuery) throws DescriptorParsingException { + if (aggQuery.getAggregateKey() != null) { + performKeyParamChecks(aggQuery); + } + } + + private void performKeyParamChecks(AggregateQuery2<T> aggQuery) throws DescriptorParsingException { + Key<?> optionalKey = aggQuery.getAggregateKey(); + if (optionalKey == null) { + throw new DescriptorParsingException("Aggregate key for " + + aggQuery.getAggregateFunction() + " must not be null."); + } + // non-null case + String name = optionalKey.getName(); + Key<?> aggKey = desc.getCategory().getKey(name); + if (aggKey == null) { + throw new DescriptorParsingException("Unknown aggregate key '" + name + "'"); + } + } + private void ensureKeyInSetIsKnown() throws DescriptorParsingException { // retrieve the expected keys list from the category Collection<Key<?>> keys = desc.getCategory().getKeys();
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java Thu Jun 05 09:33:45 2014 +0200 @@ -37,14 +37,10 @@ package com.redhat.thermostat.storage.model; import java.util.Objects; -import java.util.logging.Level; -import java.util.logging.Logger; -import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.storage.core.Entity; import com.redhat.thermostat.storage.core.Persist; -import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor; import com.redhat.thermostat.storage.core.experimental.BatchCursor; /** @@ -84,36 +80,6 @@ public <T extends Pojo> Cursor<T> getCursor() { return (BatchCursor<T>) new AggregateCursor<>(this); } - - private static class AggregateCursor<T extends Pojo> extends BasicBatchCursor<T> { - private static final Logger logger = LoggingUtils.getLogger(AggregateCursor.class); - private boolean available = true; - private final T count; - - private AggregateCursor(T count) { - this.count = count; - } - - @Override - public boolean hasNext() { - return available; - } - - @Override - public T next() { - if (available) { - available = false; - return count; - } else { - // FIXME: Thermostat 2.0: Change to throwing NoSuchElementException - String warning = "No next element but next() is being called. " + - "This will throw NoSuchElementException in the next release!"; - logger.log(Level.WARNING, warning); - return null; - } - } - - } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCursor.java Thu Jun 05 09:33:45 2014 +0200 @@ -0,0 +1,81 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.model; + +import java.util.logging.Level; +import java.util.logging.Logger; + +import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor; + +/** + * A cursor which returns true on {@link #hasNext()} once and only once. + * {@link #next()} will return the instance given when the cursor was + * constructed. + * + * @param <T> + */ +class AggregateCursor<T extends Pojo> extends BasicBatchCursor<T> { + + private static final Logger logger = LoggingUtils.getLogger(AggregateCursor.class); + private boolean available = true; + private final T result; + + AggregateCursor(T value) { + this.result = value; + } + + @Override + public boolean hasNext() { + return available; + } + + @Override + public T next() { + if (available) { + available = false; + return result; + } else { + // FIXME: Thermostat 2.0: Change to throwing NoSuchElementException + String warning = "No next element but next() is being called. " + + "This will throw NoSuchElementException in the next release!"; + logger.log(Level.WARNING, warning); + return null; + } + } + +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/DistinctResult.java Thu Jun 05 09:33:45 2014 +0200 @@ -0,0 +1,80 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.model; + +import com.redhat.thermostat.storage.core.Cursor; +import com.redhat.thermostat.storage.core.Entity; +import com.redhat.thermostat.storage.core.Key; +import com.redhat.thermostat.storage.core.Persist; + +/** + * Model class for aggregated query results based on distinct values + * of a key. + * + */ +@Entity +public class DistinctResult implements AggregateResult { + + private String[] values; + private Key<?> key; + + @Persist + public String[] getValues() { + return values; + } + + @Persist + public void setValues(String[] values) { + this.values = values; + } + + @Persist + public Key<?> getKey() { + return key; + } + + @Persist + public void setKey(Key<?> key) { + this.key = key; + } + + @SuppressWarnings("unchecked") + public <T extends Pojo> Cursor<T> getCursor() { + return (Cursor<T>) new AggregateCursor<>(this); + } + +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/experimental/AggregateQuery2Test.java Thu Jun 05 09:33:45 2014 +0200 @@ -0,0 +1,93 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.core.experimental; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +import org.junit.Test; + +import com.redhat.thermostat.storage.core.Cursor; +import com.redhat.thermostat.storage.core.Key; +import com.redhat.thermostat.storage.core.Query; +import com.redhat.thermostat.storage.core.Statement; +import com.redhat.thermostat.storage.core.AggregateQuery.AggregateFunction; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; +import com.redhat.thermostat.storage.model.AggregateCount; + +public class AggregateQuery2Test { + + @Test + public void testSetAggregateKey() { + @SuppressWarnings("unchecked") + Query<AggregateCount> query = mock(Query.class); + TestAggregateQuery aggregate = new TestAggregateQuery(AggregateFunction.COUNT, query); + assertNull(aggregate.getAggregateKey()); + try { + aggregate.setAggregateKey(null); + fail("Expected NPE."); + } catch (NullPointerException e) { + // pass + } + @SuppressWarnings("rawtypes") + Key<?> fooKey = new Key("foo"); + aggregate.setAggregateKey(fooKey); + assertEquals(fooKey, aggregate.getAggregateKey()); + } + + private static class TestAggregateQuery extends AggregateQuery2<AggregateCount> { + + public TestAggregateQuery( + AggregateFunction function, + Query<AggregateCount> queryToAggregate) { + super(function, queryToAggregate); + } + + @Override + public Cursor<AggregateCount> execute() { + throw new AssertionError("not implemented"); + } + + @Override + public Statement<AggregateCount> getRawDuplicate() { + throw new AssertionError("not implemented"); + } + + } +}
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParserTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParserTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -37,16 +37,21 @@ package com.redhat.thermostat.storage.internal.statement; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.any; import java.util.ArrayList; import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.junit.Before; import org.junit.Test; @@ -66,6 +71,7 @@ import com.redhat.thermostat.storage.core.Replace; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.model.AgentInformation; import com.redhat.thermostat.storage.model.AggregateCount; @@ -103,17 +109,101 @@ when(storage.createReplace(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockReplace); } + @Test + public void verifyMatchingAggregateCountParamRegexp() { + Pattern p = Pattern.compile(BasicDescriptorParser.AGGREGATE_PARAM_REGEXP); + String empty = ""; + Matcher matcher = p.matcher(empty); + assertTrue("Should match empty == no params", matcher.matches()); + assertEquals(1, matcher.groupCount()); + assertNull(matcher.group(1)); + String parensWithName = "(foo)"; + matcher = p.matcher(parensWithName); + assertTrue("Should match for a parameter name: " + parensWithName, matcher.matches()); + assertEquals(1, matcher.groupCount()); + assertEquals("foo", matcher.group(1)); + String upLowUnderscore = "(Foo_Bar)"; + matcher = p.matcher(upLowUnderscore); + assertTrue("Should match for a parameter name with upper/lowercase and underscore: " + + upLowUnderscore, matcher.matches()); + assertEquals(1, matcher.groupCount()); + assertEquals("Foo_Bar", matcher.group(1)); + } + + @Test + public void verifyRejectingAggregateCountParamRegexp() { + Pattern p = Pattern.compile(BasicDescriptorParser.AGGREGATE_PARAM_REGEXP); + String parensOnly = "()"; + Matcher matcher = p.matcher(parensOnly); + assertFalse(parensOnly + " should not match", matcher.matches()); + String spaceString = "( foo )"; + matcher = p.matcher(spaceString); + assertFalse(spaceString + " should not match because it has spaces", matcher.matches()); + } + + /* + * We allow QUERY-COUNT without any parameter for backwards compatibility + * Contrast this with QUERY-COUNT(foo). + */ + @Test + public void testParseAggregateCount() throws DescriptorParsingException { + String descFormat = "QUERY-COUNT %s WHERE 'a' = 'b'"; + runAggregateCountTest(descFormat, AggregateFunction.COUNT); + } + + /* + * Query count with a specific optional key. Tests basic parsing. + */ + @Test + public void testParseAggregateCountWithParam() throws DescriptorParsingException { + String descrString = "QUERY-COUNT(a) %s WHERE 'a' = 'b'"; + runAggregateCountTest(descrString, AggregateFunction.COUNT); + } + + /* + * Distinct with a specific optional key. Tests basic parsing. + */ + @Test + public void testParseAggregateDistinctWithParam() throws DescriptorParsingException { + String descrString = "QUERY-DISTINCT(a) %s WHERE 'a' = 'b'"; + runAggregateCountTest(descrString, AggregateFunction.DISTINCT); + } + @SuppressWarnings({ "rawtypes", "unchecked" }) @Test - public void testParseAggregateCount() throws DescriptorParsingException { - Query<AggregateCount> query = mock(AggregateQuery.class); + public void testParseAggregateCountNoWhereClause() throws DescriptorParsingException { + AggregateQuery<AggregateCount> query = mock(AggregateQuery.class); ArgumentCaptor<Category> captor = ArgumentCaptor.forClass(Category.class); when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), captor.capture())).thenReturn(query); // first adapt from the target category in order to be able to produce the // right aggregate query with a different result type. CategoryAdapter<AgentInformation, AggregateCount> adapter = new CategoryAdapter<>(AgentInfoDAO.CATEGORY); Category<AggregateCount> aggregateCategory = adapter.getAdapted(AggregateCount.class); - String descrString = "QUERY-COUNT " + aggregateCategory.getName() + " WHERE 'a' = 'b'"; + String descrString = "QUERY-COUNT " + aggregateCategory.getName(); + StatementDescriptor<AggregateCount> desc = new StatementDescriptor<>(aggregateCategory, descrString); + BasicDescriptorParser<AggregateCount> parser = new BasicDescriptorParser<>(storage, desc); + ParsedStatementImpl<AggregateCount> statement = (ParsedStatementImpl<AggregateCount>)parser.parse(); + assertEquals(0, statement.getNumParams()); + assertTrue(statement.getRawStatement() instanceof AggregateQuery); + Category<AggregateCount> capturedCategory = captor.getValue(); + assertEquals(aggregateCategory, capturedCategory); + SuffixExpression expn = statement.getSuffixExpression(); + assertNotNull(expn); + assertNull(expn.getWhereExpn()); + assertNull(expn.getSortExpn()); + assertNull(expn.getLimitExpn()); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private void runAggregateCountTest(String descriptorFormat, AggregateFunction function) throws DescriptorParsingException { + AggregateQuery2<AggregateCount> query = mock(AggregateQuery2.class); + ArgumentCaptor<Category> captor = ArgumentCaptor.forClass(Category.class); + when(storage.createAggregateQuery(eq(function), captor.capture())).thenReturn(query); + // first adapt from the target category in order to be able to produce the + // right aggregate query with a different result type. + CategoryAdapter<AgentInformation, AggregateCount> adapter = new CategoryAdapter<>(AgentInfoDAO.CATEGORY); + Category<AggregateCount> aggregateCategory = adapter.getAdapted(AggregateCount.class); + String descrString = String.format(descriptorFormat, aggregateCategory.getName()); StatementDescriptor<AggregateCount> desc = new StatementDescriptor<>(aggregateCategory, descrString); BasicDescriptorParser<AggregateCount> parser = new BasicDescriptorParser<>(storage, desc); ParsedStatementImpl<AggregateCount> statement = (ParsedStatementImpl<AggregateCount>)parser.parse(); @@ -130,6 +220,45 @@ } @Test + public void testParseQueryCountWithAssertedParamValue() throws DescriptorParsingException { + String formatedDesc = "QUERY-COUNT(a) %s"; + runParamAggregateAssertedValueTest(formatedDesc, AggregateFunction.COUNT); + } + + @Test + public void testParseQueryDistinctWithAssertedParamValue() throws DescriptorParsingException { + String formatedDesc = "QUERY-DISTINCT(a) %s"; + runParamAggregateAssertedValueTest(formatedDesc, AggregateFunction.DISTINCT); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void runParamAggregateAssertedValueTest(String aggregateDescFormat, AggregateFunction function) throws DescriptorParsingException { + AggregateQuery2<AggregateCount> query = mock(AggregateQuery2.class); + ArgumentCaptor<Category> captor = ArgumentCaptor.forClass(Category.class); + when(storage.createAggregateQuery(eq(function), captor.capture())).thenReturn(query); + // first adapt from the target category in order to be able to produce the + // right aggregate query with a different result type. + CategoryAdapter<AgentInformation, AggregateCount> adapter = new CategoryAdapter<>(AgentInfoDAO.CATEGORY); + Category<AggregateCount> aggregateCategory = adapter.getAdapted(AggregateCount.class); + String descrString = String.format(aggregateDescFormat, aggregateCategory.getName()); + StatementDescriptor<AggregateCount> desc = new StatementDescriptor<>(aggregateCategory, descrString); + BasicDescriptorParser<AggregateCount> parser = new BasicDescriptorParser<>(storage, desc); + ParsedStatementImpl<AggregateCount> statement = (ParsedStatementImpl<AggregateCount>)parser.parse(); + assertEquals(0, statement.getNumParams()); + assertTrue(statement.getRawStatement() instanceof AggregateQuery); + Key<?> aKey = new Key<>("a"); + // Make sure the optional key is set + verify(query).setAggregateKey(eq(aKey)); + Category<AggregateCount> capturedCategory = captor.getValue(); + assertEquals(aggregateCategory, capturedCategory); + SuffixExpression expn = statement.getSuffixExpression(); + assertNotNull(expn); + assertNull("No where expression expected", expn.getWhereExpn()); + assertNull(expn.getSortExpn()); + assertNull(expn.getLimitExpn()); + } + + @Test public void testParseQuerySimple() throws DescriptorParsingException { String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName(); StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descrString); @@ -1575,6 +1704,41 @@ } } + @Test + public void rejectSpaceInQueryCountParam() { + String descFormat = "QUERY-COUNT(' a) %s"; + String expnMsg = "Unknown statement type: 'QUERY-COUNT(''"; + runRejectQueryCountTest(descFormat, expnMsg); + } + + @Test + public void rejectSpaceInQueryCountParam2() { + String descFormat = "QUERY-COUNT(Abc ) %s"; + String expnMsg = "Unknown statement type: 'QUERY-COUNT(Abc'"; + runRejectQueryCountTest(descFormat, expnMsg); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void runRejectQueryCountTest(String descFormat, String expnMsg) { + AggregateQuery<AggregateCount> query = mock(AggregateQuery.class); + ArgumentCaptor<Category> captor = ArgumentCaptor.forClass(Category.class); + when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), captor.capture())).thenReturn(query); + // first adapt from the target category in order to be able to produce the + // right aggregate query with a different result type. + CategoryAdapter<AgentInformation, AggregateCount> adapter = new CategoryAdapter<>(AgentInfoDAO.CATEGORY); + Category<AggregateCount> aggregateCategory = adapter.getAdapted(AggregateCount.class); + String descrString = String.format(descFormat, aggregateCategory.getName()); + StatementDescriptor<AggregateCount> desc = new StatementDescriptor<>(aggregateCategory, descrString); + BasicDescriptorParser<AggregateCount> parser = new BasicDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("shouldn't have parsed correctly"); + } catch (DescriptorParsingException e) { + assertTrue(e.getMessage().contains(expnMsg)); + } + } + + /* * At this point list types don't make sense in WHERE. * What should "'a' != [ 'a', 'b' ]" evaluate to? @@ -1584,6 +1748,7 @@ * We don't support this in a prepared context at this point. Should this * change in future, this test needs to be carefully re-crafted. */ + @SuppressWarnings("unchecked") @Test public void rejectListTypesAsFreeVarInInvalidContext() throws DescriptorParsingException { List<String> illegalDescs = new ArrayList<>(); @@ -1596,10 +1761,13 @@ " WHERE 'a' = ?d[", }; - // Make sure we test for QUERY, QUERY-COUNT, REPLACE, UPDATE, REMOVE + // Make sure we test for QUERY, QUERY-COUNT, QUERY-DISTINCT, REPLACE, UPDATE, REMOVE // i.e. all that accept a WHERE. String basicQuery = "QUERY " + AgentInfoDAO.CATEGORY.getName(); String basicQueryCount = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName(); + String basicQueryDistinct = "QUERY-DISTINCT(foo) " + AgentInfoDAO.CATEGORY.getName(); + when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), eq(AgentInfoDAO.CATEGORY))).thenReturn(mock(AggregateQuery.class)); + when(storage.createAggregateQuery(eq(AggregateFunction.DISTINCT), eq(AgentInfoDAO.CATEGORY))).thenReturn(mock(AggregateQuery.class)); // note SET clause is legal String basicUpdate = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?s["; // note SET clause is legal @@ -1608,6 +1776,7 @@ for (String where: illegalWheres) { illegalDescs.add(basicQuery + where); illegalDescs.add(basicQueryCount + where); + illegalDescs.add(basicQueryDistinct + where); illegalDescs.add(basicUpdate + where); illegalDescs.add(basicReplace + where); illegalDescs.add(basicRemove + where); @@ -1658,7 +1827,7 @@ private void doIllegalDescsTest(List<String> descs, String errorMsgContext, String type) { for (String strDesc : descs) { - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); parser = new BasicDescriptorParser<>(storage, desc); try { parser.parse(); @@ -1855,5 +2024,21 @@ } } + @SuppressWarnings("unchecked") + @Test + public void testCompatibilityNonAggregateQuery() throws DescriptorParsingException { + when(storage.createAggregateQuery(any(AggregateFunction.class), any(Category.class))).thenReturn(mock(Query.class)); + String descString = "QUERY-COUNT(foo) " + AgentInfoDAO.CATEGORY.getName(); + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new BasicDescriptorParser<>(storage, desc); + try { + parser.parse(); + // pass + } catch (Exception e) { + e.printStackTrace(); + fail("Expected to parse even though createAggregateQuery returned a simple Query instance instead of AggregateQuery"); + } + } + }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParserTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParserTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -38,6 +38,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.eq; @@ -58,6 +59,7 @@ import com.redhat.thermostat.storage.core.Replace; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.model.AgentInformation; @@ -79,6 +81,7 @@ private BackingStorage storage; private Query<AgentInformation> mockQuery; + private AggregateQuery2<AgentInformation> aggQuery; private SemanticsEnabledDescriptorParser<AgentInformation> parser; private Add<AgentInformation> mockAdd; private Update<AgentInformation> mockUpdate; @@ -90,9 +93,11 @@ public void setup() { storage = mock(BackingStorage.class); mockQuery = mock(Query.class); - // setup for QUERY/QUERY-COUNT + aggQuery = mock(AggregateQuery2.class); + // setup for QUERY/QUERY-COUNT/QUERY-DISTINCT when(storage.createQuery(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockQuery); - when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), (eq(AgentInfoDAO.CATEGORY)))).thenReturn(mockQuery); + when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), (eq(AgentInfoDAO.CATEGORY)))).thenReturn(aggQuery); + when(storage.createAggregateQuery(eq(AggregateFunction.DISTINCT), (eq(AgentInfoDAO.CATEGORY)))).thenReturn(aggQuery); // setup for ADD mockAdd = mock(Add.class); when(storage.createAdd(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockAdd); @@ -112,7 +117,30 @@ String descString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT 1"; doSemanticsBasicParseTest(descString); - descString = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName() + " LIMIT 1"; + } + + @Test + public void catParseQueryCountWithLimit() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.COUNT); + String descString = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName() + " LIMIT 1"; + doSemanticsBasicParseTest(descString); + } + + @Test + public void canParseQueryCountWithCorrectKeyParam() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.COUNT); + String descString = "QUERY-COUNT(" + Key.AGENT_ID.getName() + ") " + + AgentInfoDAO.CATEGORY.getName(); + doSemanticsBasicParseTest(descString); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + @Test + public void canParseQueryDistinctWithCorrectKeyParam() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.DISTINCT); + when(aggQuery.getAggregateKey()).thenReturn((Key)Key.AGENT_ID); + String descString = "QUERY-DISTINCT(" + Key.AGENT_ID.getName() + ") " + + AgentInfoDAO.CATEGORY.getName(); doSemanticsBasicParseTest(descString); } @@ -120,8 +148,12 @@ public void canParseQueryWithSort() throws DescriptorParsingException { String descString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT 'foo' DSC"; doSemanticsBasicParseTest(descString); - - descString = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName() + " SORT 'foo' DSC"; + } + + @Test + public void catParseQueryCountWithSort() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.COUNT); + String descString = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName() + " SORT 'foo' DSC"; doSemanticsBasicParseTest(descString); } @@ -167,6 +199,68 @@ assertNotNull(p); } + /* + * Tests whether we appropriately reject a provided key parameter in a + * query-count aggregate for which the key is unknown in the given category. + */ + @Test + public void rejectQueryCountWithUnknownKeyAsParam() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.COUNT); + doRejectQueryAggregateTestWithParam(AggregateFunction.COUNT); + } + + /* + * Tests whether we appropriately reject a provided key parameter in a + * query-distinct aggregate for which the key is unknown in the given category. + */ + @Test + public void rejectQueryDistinctWithUnknownKeyAsParam() throws DescriptorParsingException { + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.DISTINCT); + doRejectQueryAggregateTestWithParam(AggregateFunction.DISTINCT); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private void doRejectQueryAggregateTestWithParam(AggregateFunction function) { + String unknownKeyName = "unknown_key"; + Key unknownKey = new Key(unknownKeyName); + when(aggQuery.getAggregateKey()).thenReturn(unknownKey); + String format = "QUERY-%s(%s) %s"; + String descString = String.format(format, function.name(), unknownKeyName, AgentInfoDAO.CATEGORY.getName()); + assertNull("Precondition failed. unknown_key in AgentInfo category?", + AgentInfoDAO.CATEGORY.getKey(unknownKeyName)); + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail(String.format("QUERY-%s with an unknown key as param should not parse", function.name())); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("Unknown aggregate key '" + unknownKeyName + "'")); + } + } + + /* + * Distinct queries aggregate by selecting all distinct values for a + * given key. Not providing a key for which to find distinct values for + * doesn't make sense. + */ + @Test + public void rejectQueryDistinctWithoutKeyParam() throws DescriptorParsingException { + String descString = "QUERY-DISTINCT " + AgentInfoDAO.CATEGORY.getName(); + + when(aggQuery.getAggregateKey()).thenReturn(null); + when(aggQuery.getAggregateFunction()).thenReturn(AggregateFunction.DISTINCT); + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("QUERY-DISTINCT without a key for which to produce distinct values for should not parse"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("Aggregate key for DISTINCT must not be null")); + } + } + @Test public void rejectAddWithWhere() throws DescriptorParsingException { String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO + " WHERE 'a' = 'b'";
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCountTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCountTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -38,41 +38,40 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import org.junit.Test; -import com.redhat.thermostat.storage.core.Cursor; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; - public class AggregateCountTest { - + @Test - public void testCursor() { + public void testEquals() { AggregateCount c = new AggregateCount(); - c.setCount(10); - Cursor<AggregateCount> cursor = c.getCursor(); - assertTrue(cursor.hasNext()); - AggregateCount actual = cursor.next(); - assertEquals(10, actual.getCount()); - assertFalse(cursor.hasNext()); - assertNull(cursor.next()); + assertTrue("should be equal to self", c.equals(c)); + c.setCount(-1); + assertTrue("should be equal to self", c.equals(c)); + AggregateCount d = new AggregateCount(); + d.setCount(-1); + assertTrue("c + d have equal count", c.equals(d)); + d.setCount(10); + assertFalse("c has count -1, d 10", c.equals(d)); + assertFalse("string is no agg-count", c.equals("foo")); + assertTrue(new AggregateCount().equals(new AggregateCount())); } - /** - * Setting the batch size for single result lists should be no-op. - * This just makes sure that nothing bad happens (no exceptions being thrown) - */ @Test - public void testCursorBatchSize() { - AggregateCount count = new AggregateCount(); - count.setCount(3); - Cursor<AggregateCount> cursor = count.getCursor(); - BatchCursor<AggregateCount> advCursor = (BatchCursor<AggregateCount>)cursor; - advCursor.setBatchSize(500); - AggregateCount result = advCursor.next(); - assertEquals(3, result.getCount()); + public void testHashCode() { + AggregateCount c = new AggregateCount(); + assertEquals(c.hashCode(), c.hashCode()); + c.setCount(-1); + assertEquals(c.hashCode(), c.hashCode()); + AggregateCount d = new AggregateCount(); + d.setCount(-1); + assertEquals(d.hashCode(), c.hashCode()); + d.setCount(100); + assertFalse(d.hashCode() == c.hashCode()); + assertFalse("foo".hashCode() == d.hashCode()); + assertEquals(new AggregateCount().hashCode(), new AggregateCount().hashCode()); } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCursorTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -0,0 +1,83 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.model; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.redhat.thermostat.storage.core.Cursor; +import com.redhat.thermostat.storage.core.experimental.BatchCursor; + +public class AggregateCursorTest { + + @Test + public void testCursor() { + AggregateTest t = new AggregateTest(); + Cursor<AggregateTest> cursor = t.getCursor(); + assertTrue(cursor.hasNext()); + AggregateTest actual = cursor.next(); + assertSame(t, actual); + assertFalse(cursor.hasNext()); + assertNull(cursor.next()); + } + + /** + * Setting the batch size for single result lists should be no-op. + * This just makes sure that nothing bad happens (no exceptions being thrown) + */ + @Test + public void testCursorBatchSize() { + AggregateTest t = new AggregateTest(); + Cursor<AggregateTest> cursor = t.getCursor(); + BatchCursor<AggregateTest> advCursor = (BatchCursor<AggregateTest>)cursor; + advCursor.setBatchSize(500); + assertEquals(500, advCursor.getBatchSize()); + } + + private static class AggregateTest implements AggregateResult { + + @SuppressWarnings("unchecked") + private <T extends Pojo> Cursor<T> getCursor() { + return (Cursor<T>)new AggregateCursor<>(this); + } + } +}
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/model/BackendInformationTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/model/BackendInformationTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -37,14 +37,11 @@ package com.redhat.thermostat.storage.model; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.fail; import java.util.Map; import org.junit.Test; -import com.redhat.thermostat.storage.model.BackendInformation; - public class BackendInformationTest { @Test
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Thu Jun 05 09:33:45 2014 +0200 @@ -79,9 +79,11 @@ import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.AggregateResult; +import com.redhat.thermostat.storage.model.DistinctResult; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.storage.query.Expression; @@ -92,7 +94,33 @@ */ public class MongoStorage implements BackingStorage, SchemaInfoInserter { - private class MongoCountQuery<T extends Pojo> extends AggregateQuery<T> { + private class MongoDistinctQuery<T extends Pojo> extends AggregateQuery2<T> { + + private final Category<T> category; + + private MongoDistinctQuery(MongoQuery<T> queryToAggregate, Category<T> category) { + super(AggregateFunction.DISTINCT, queryToAggregate); + this.category = category; + } + + @Override + public Cursor<T> execute() { + return executeDistinctQuery(this, category, (MongoQuery<T>)queryToAggregate); + } + + @Override + public Statement<T> getRawDuplicate() { + MongoQuery<T> query = (MongoQuery<T>) this.queryToAggregate; + MongoDistinctQuery<T> aggQuery = new MongoDistinctQuery<>(query, category); + // Distinct queries require this param. It's static so pass this + // on to duplicates. + aggQuery.setAggregateKey(getAggregateKey()); + return aggQuery; + } + + } + + private class MongoCountQuery<T extends Pojo> extends AggregateQuery2<T> { private final Category<T> category; @@ -109,7 +137,13 @@ @Override public Statement<T> getRawDuplicate() { MongoQuery<T> query = (MongoQuery<T>) this.queryToAggregate; - return new MongoCountQuery<>(query, category); + MongoCountQuery<T> dupe = new MongoCountQuery<>(query, category); + // Count aggregates have an optional key to aggregate. Optional for + // backwards compat reasons. Be sure to copy it over if it was set. + if (getAggregateKey() != null) { + dupe.setAggregateKey(getAggregateKey()); + } + return dupe; } } @@ -332,22 +366,6 @@ this(new MongoConnection(url, creds, sslConf)); } - public <T extends Pojo> Cursor<T> executeGetCount(Category<T> category, MongoQuery<T> queryToAggregate) { - try { - DBCollection coll = getCachedCollection(category); - long count = 0L; - DBObject query = queryToAggregate.getGeneratedQuery(); - if (coll != null) { - count = coll.getCount(query); - } - AggregateCount result = new AggregateCount(); - result.setCount(count); - return result.getCursor(); - } catch (MongoException me) { - throw new StorageException(me); - } - } - @Override public Connection getConnection() { return conn; @@ -365,6 +383,56 @@ return replace; } + private <T extends Pojo> Cursor<T> executeGetCount(Category<T> category, MongoQuery<T> queryToAggregate) { + try { + DBCollection coll = getCachedCollection(category); + long count = 0L; + DBObject query = queryToAggregate.getGeneratedQuery(); + if (coll != null) { + count = coll.getCount(query); + } + AggregateCount result = new AggregateCount(); + result.setCount(count); + return result.getCursor(); + } catch (MongoException me) { + throw new StorageException(me); + } + } + + private <T extends Pojo> Cursor<T> executeDistinctQuery(MongoDistinctQuery<T> aggQuery, Category<T> category, MongoQuery<T> queryToAggregate) { + try { + DBCollection coll = getCachedCollection(category); + DBObject query = queryToAggregate.getGeneratedQuery(); + String[] distinctValues; + Key<?> aggregateKey = aggQuery.getAggregateKey(); + if (coll != null) { + String key = aggregateKey.getName(); + List<?> values = coll.distinct(key, query); + distinctValues = convertToStringList(values, key); + } else { + distinctValues = new String[0]; + } + DistinctResult result = new DistinctResult(); + result.setKey(aggregateKey); + result.setValues(distinctValues); + return result.getCursor(); + } catch (MongoException me) { + throw new StorageException(me); + } + } + + private String[] convertToStringList(List<?> values, String keyName) { + List<String> stringList = new ArrayList<>(); + for (int i = 0; i < values.size(); i++) { + Object val = values.get(i); + // Mongodb might give us null values. + if (val != null) { + stringList.add(val.toString()); + } + } + return stringList.toArray(new String[0]); + } + private <T extends Pojo> int addImpl(final Category<T> cat, final DBObject values) { try { DBCollection coll = getCachedCollection(cat); @@ -601,12 +669,14 @@ } @Override - public <T extends Pojo> Query<T> createAggregateQuery( + public <T extends Pojo> AggregateQuery<T> createAggregateQuery( AggregateFunction function, Category<T> category) { + MongoQuery<T> query = (MongoQuery<T>)createQuery(category); switch (function) { case COUNT: - MongoQuery<T> query = (MongoQuery<T>)createQuery(category); return new MongoCountQuery<>(query, category); + case DISTINCT: + return new MongoDistinctQuery<>(query, category); default: throw new IllegalStateException("function not supported: " + function);
--- a/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -55,6 +55,7 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -84,9 +85,11 @@ import com.mongodb.gridfs.GridFSInputFile; import com.redhat.thermostat.shared.config.SSLConfiguration; import com.redhat.thermostat.storage.core.Add; +import com.redhat.thermostat.storage.core.AggregateQuery; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.Category; import com.redhat.thermostat.storage.core.CategoryAdapter; +import com.redhat.thermostat.storage.core.AggregateQuery.AggregateFunction; import com.redhat.thermostat.storage.core.Connection.ConnectionListener; import com.redhat.thermostat.storage.core.Connection.ConnectionStatus; import com.redhat.thermostat.storage.core.Cursor; @@ -97,11 +100,14 @@ import com.redhat.thermostat.storage.core.Remove; import com.redhat.thermostat.storage.core.Replace; import com.redhat.thermostat.storage.core.SchemaInfo; +import com.redhat.thermostat.storage.core.Statement; import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.Update; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.BasePojo; +import com.redhat.thermostat.storage.model.DistinctResult; import com.redhat.thermostat.storage.model.HostInfo; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.storage.query.Expression; @@ -522,6 +528,102 @@ assertEquals("val1", pojoVal.get("key1")); assertEquals("val3", pojoVal.get("key3")); } + + @Test + public void verifyAggregateDistinct() throws Exception { + // setup + List<Integer> mockList = new ArrayList<>(); + mockList.add(-1); + mockList.add(200); + when(testCollection.distinct(eq(key1.getName()), any(DBObject.class))).thenReturn(mockList); + + MongoStorage storage = makeStorage(); + CategoryAdapter<TestClass, DistinctResult> adapter = new CategoryAdapter<>(testCategory); + Category<DistinctResult> adaptedCategory = adapter.getAdapted(DistinctResult.class); + AggregateQuery<DistinctResult> aggQuery = storage.createAggregateQuery(AggregateFunction.DISTINCT, adaptedCategory); + assertTrue(aggQuery instanceof AggregateQuery2); + AggregateQuery2<DistinctResult> aggQuery2 = (AggregateQuery2<DistinctResult>)aggQuery; + aggQuery2.setAggregateKey(key1); + Cursor<DistinctResult> cursor = aggQuery.execute(); + assertTrue(cursor.hasNext()); + DistinctResult r = cursor.next(); + String[] expected = new String[] { "-1", "200" }; + assertArrayEquals(expected, r.getValues()); + assertEquals(key1, r.getKey()); + + // do it again with a list of booleans + List<Boolean> boolList = new ArrayList<>(); + boolList.add(false); + boolList.add(true); + boolList.add(true); + when(testCollection.distinct(eq(key2.getName()), any(DBObject.class))).thenReturn(boolList); + aggQuery = storage.createAggregateQuery(AggregateFunction.DISTINCT, adaptedCategory); + assertTrue(aggQuery instanceof AggregateQuery2); + aggQuery2 = (AggregateQuery2<DistinctResult>)aggQuery; + aggQuery2.setAggregateKey(key2); + cursor = aggQuery2.execute(); + assertTrue(cursor.hasNext()); + r = cursor.next(); + expected = new String[] { "false", "true", "true" }; + assertArrayEquals(expected, r.getValues()); + assertEquals(key2, r.getKey()); + } + + /* + * The statement descriptor subsystem uses getRawDuplicate. That duplicate + * needs to keep the key parameter set for distinct aggregate queries. + */ + @Test + public void canDuplicateAggregateDistinct() throws Exception { + MongoStorage storage = makeStorage(); + CategoryAdapter<TestClass, DistinctResult> adapter = new CategoryAdapter<>(testCategory); + Category<DistinctResult> adaptedCategory = adapter.getAdapted(DistinctResult.class); + AggregateQuery<DistinctResult> aggQuery = storage.createAggregateQuery(AggregateFunction.DISTINCT, adaptedCategory); + assertTrue(aggQuery instanceof AggregateQuery2); + AggregateQuery2<DistinctResult> aggQuery2 = (AggregateQuery2<DistinctResult>)aggQuery; + assertNull(aggQuery2.getAggregateKey()); + aggQuery2.setAggregateKey(key1); + + Statement<DistinctResult> stmt = aggQuery2.getRawDuplicate(); + assertTrue(stmt instanceof AggregateQuery2); + AggregateQuery2<DistinctResult> rawCopy = (AggregateQuery2<DistinctResult>) stmt; + assertEquals("Expected key to be copied for dupe", key1, rawCopy.getAggregateKey()); + } + + /* + * The statement descriptor subsystem uses getRawDuplicate. That duplicate + * needs to keep the key parameter if set for count aggregates. + */ + @Test + public void canDuplicateAggregateCountWithKey() throws Exception { + MongoStorage storage = makeStorage(); + CategoryAdapter<TestClass, AggregateCount> adapter = new CategoryAdapter<>(testCategory); + Category<AggregateCount> adaptedCategory = adapter.getAdapted(AggregateCount.class); + AggregateQuery<AggregateCount> aggQuery = storage.createAggregateQuery(AggregateFunction.COUNT, adaptedCategory); + + // Produce duplicate without key set + assertTrue(aggQuery instanceof AggregateQuery2); + AggregateQuery2<AggregateCount> aggQuery2 = (AggregateQuery2<AggregateCount>)aggQuery; + assertNull(aggQuery2.getAggregateKey()); + Statement<AggregateCount> stmt = aggQuery2.getRawDuplicate(); + assertTrue(stmt instanceof AggregateQuery2); + AggregateQuery2<AggregateCount> rawCopy = (AggregateQuery2<AggregateCount>) stmt; + + // Duplicate with key set + aggQuery2.setAggregateKey(key1); + assertNotNull(aggQuery2.getAggregateKey()); + stmt = aggQuery2.getRawDuplicate(); + assertTrue(stmt instanceof AggregateQuery2); + rawCopy = (AggregateQuery2<AggregateCount>) stmt; + assertEquals("Expected key to be copied for dupe", key1, rawCopy.getAggregateKey()); + } + + private static void assertArrayEquals(String[] expected, String[] actual) { + assertEquals(expected.length, actual.length); + for (int i = 0; i < expected.length; i++) { + assertEquals(expected[i], actual[i]); + } + } @Test public void verifyPutChunkDoesNotUseGlobalAgent() throws Exception {
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Wed Nov 26 11:56:35 2014 +0100 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Thu Jun 05 09:33:45 2014 +0200 @@ -103,7 +103,6 @@ import com.google.gson.GsonBuilder; import com.google.gson.reflect.TypeToken; import com.redhat.thermostat.storage.core.Add; -import com.redhat.thermostat.storage.core.AggregateQuery; import com.redhat.thermostat.storage.core.AggregateQuery.AggregateFunction; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.Categories; @@ -121,6 +120,7 @@ import com.redhat.thermostat.storage.core.auth.CategoryRegistration; import com.redhat.thermostat.storage.core.auth.DescriptorMetadata; import com.redhat.thermostat.storage.core.auth.StatementDescriptorRegistration; +import com.redhat.thermostat.storage.core.experimental.AggregateQuery2; import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; @@ -805,7 +805,7 @@ AggregateCount count = new AggregateCount(); count.setCount(500); // prepare-statement does this under the hood - Query<AggregateCount> mockMongoQuery = mock(AggregateQuery.class); + AggregateQuery2<AggregateCount> mockMongoQuery = mock(AggregateQuery2.class); Category<AggregateCount> adapted = new CategoryAdapter(category).getAdapted(AggregateCount.class); registerCategory(adapted, "no-matter", "no-matter"); when(mockStorage.createAggregateQuery(eq(AggregateFunction.COUNT), eq(adapted))).thenReturn(mockMongoQuery);