Mercurial > hg > release > thermostat-1.0
changeset 1277:6f704bf1d5ac
Implement better semantic analysis of descriptors.
Reviewed-by: omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-October/008413.html
line wrap: on
line diff
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Fri Oct 04 19:34:29 2013 +0200 @@ -37,8 +37,6 @@ package com.redhat.thermostat.storage.dao; import java.util.Collection; -import java.util.List; -import java.util.Map; import com.redhat.thermostat.annotations.Service; import com.redhat.thermostat.storage.core.Category; @@ -47,6 +45,7 @@ import com.redhat.thermostat.storage.core.Key; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.model.VmInfo; +import com.redhat.thermostat.storage.model.VmInfo.KeyValuePair; @Service public interface VmInfoDAO extends Countable { @@ -60,9 +59,9 @@ static final Key<String> vmNameKey = new Key<>("vmName"); static final Key<String> vmInfoKey = new Key<>("vmInfo"); static final Key<String> vmVersionKey = new Key<>("vmVersion"); - static final Key<Map<String, String>> propertiesKey = new Key<>("properties"); - static final Key<Map<String, String>> environmentKey = new Key<>("environment"); - static final Key<List<String>> librariesKey = new Key<>("loadedNativeLibraries"); + static final Key<KeyValuePair[]> propertiesKey = new Key<>("propertiesAsArray"); + static final Key<KeyValuePair[]> environmentKey = new Key<>("environmentAsArray"); + static final Key<String[]> librariesKey = new Key<>("loadedNativeLibraries"); static final Key<Long> startTimeKey = new Key<>("startTimeStamp"); static final Key<Long> stopTimeKey = new Key<>("stopTimeStamp"); static final Key<Long> uidKey = new Key<>("uid");
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Fri Oct 04 19:34:29 2013 +0200 @@ -104,11 +104,8 @@ "'" + vmArgumentsKey.getName() + "' = ?s , " + "'" + vmInfoKey.getName() + "' = ?s , " + "'" + vmVersionKey.getName() + "' = ?s , " + - // The following two get persisted as pojo arrays. - // There is no direct key representation for what gets - // persisted. - "'propertiesAsArray' = ?p[ , " + - "'environmentAsArray' = ?p[ , " + + "'" + propertiesKey.getName() + "' = ?p[ , " + + "'" + environmentKey.getName() + "' = ?p[ , " + "'" + librariesKey.getName() + "' = ?s[ , " + "'" + uidKey.getName() + "' = ?l , " + "'" + usernameKey.getName() + "' = ?s";
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParser.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParser.java Fri Oct 04 19:34:29 2013 +0200 @@ -51,7 +51,6 @@ import com.redhat.thermostat.storage.core.Query.SortDirection; import com.redhat.thermostat.storage.core.Remove; import com.redhat.thermostat.storage.core.Replace; -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.model.Pojo; @@ -124,7 +123,7 @@ * * NOTE: Comparison expressions have equal precedence. */ -class BasicDescriptorParser<T extends Pojo> { +class BasicDescriptorParser<T extends Pojo> implements StatementDescriptorParser<T> { private static final String TOKEN_DELIMS = " \t\r\n\f"; private static final short IDX_QUERY = 0; @@ -148,14 +147,14 @@ private static final char PARAM_PLACEHOLDER = '?'; private final String[] tokens; - private final StatementDescriptor<T> desc; + protected final StatementDescriptor<T> desc; private final BackingStorage storage; private int currTokenIndex; private int placeHolderCount; // the parsed statement private ParsedStatementImpl<T> parsedStatement; - private SuffixExpression tree; - private SetList setList; + protected SuffixExpression tree; + protected SetList setList; BasicDescriptorParser(BackingStorage storage, StatementDescriptor<T> desc) { this.tokens = getTokens(desc.getDescriptor()); @@ -189,59 +188,9 @@ parsedStatement.setNumFreeParams(placeHolderCount); parsedStatement.setSetList(setList); parsedStatement.setSuffixExpression(tree); - doSemanticAnalysis(); return parsedStatement; } - private void doSemanticAnalysis() throws DescriptorParsingException { - // TODO: - // - Check that ADD/REPLACE specifies all keys judging by the Pojo - // model class. Not sure if good idea though, as this would likely - // introduce dep on beanutils. - Statement<T> stmt = parsedStatement.getRawStatement(); - if (stmt == null) { - // should never be null - throw new NullPointerException(); - } - if (stmt instanceof Add && tree.getWhereExpn() != null) { - String msg = "WHERE clause not allowed for ADD"; - throw new DescriptorParsingException(msg); - } - if (stmt instanceof Replace && tree.getWhereExpn() == null) { - String msg = "WHERE clause required for REPLACE"; - throw new DescriptorParsingException(msg); - } - if (stmt instanceof Update) { - if (tree.getWhereExpn() == null) { - // WHERE required for UPDATE - String msg = "WHERE clause required for UPDATE"; - throw new DescriptorParsingException(msg); - } - if (setList.getValues().size() == 0) { - // SET required for UPDATE - String msg = "SET list required for UPDATE"; - throw new DescriptorParsingException(msg); - } - } - if (stmt instanceof Remove && setList.getValues().size() > 0) { - String msg = "SET not allowed for REMOVE"; - throw new DescriptorParsingException(msg); - } - 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); - } - } else { - // only queries can have sort/limit expressions - if (this.tree.getLimitExpn() != null || this.tree.getSortExpn() != null) { - String msg = "LIMIT/SORT only allowed for QUERY/QUERY-COUNT"; - throw new DescriptorParsingException(msg); - } - } - } - /* * Match set list for DML statements. */
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImpl.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImpl.java Fri Oct 04 19:34:29 2013 +0200 @@ -61,12 +61,12 @@ private Query<T> query; private DataModifyingStatement<T> dmlStatement; private final PreparedParameters params; - private final ParsedStatementImpl<T> parsedStatement; + private final ParsedStatement<T> parsedStatement; public PreparedStatementImpl(BackingStorage storage, StatementDescriptor<T> desc) throws DescriptorParsingException { this.desc = desc; - BasicDescriptorParser<T> parser = new BasicDescriptorParser<>(storage, desc); - this.parsedStatement = (ParsedStatementImpl<T>)parser.parse(); + StatementDescriptorParser<T> parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + this.parsedStatement = parser.parse(); int numParams = parsedStatement.getNumParams(); params = new PreparedParameters(numParams); Statement<T> statement = parsedStatement.getRawStatement();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParser.java Fri Oct 04 19:34:29 2013 +0200 @@ -0,0 +1,177 @@ +/* + * Copyright 2012, 2013 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.internal.statement; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +import com.redhat.thermostat.storage.core.Add; +import com.redhat.thermostat.storage.core.BackingStorage; +import com.redhat.thermostat.storage.core.DataModifyingStatement; +import com.redhat.thermostat.storage.core.DescriptorParsingException; +import com.redhat.thermostat.storage.core.Key; +import com.redhat.thermostat.storage.core.ParsedStatement; +import com.redhat.thermostat.storage.core.Query; +import com.redhat.thermostat.storage.core.Remove; +import com.redhat.thermostat.storage.core.Replace; +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.model.Pojo; + +class SemanticsEnabledDescriptorParser<T extends Pojo> extends + BasicDescriptorParser<T> { + + SemanticsEnabledDescriptorParser(BackingStorage storage, StatementDescriptor<T> desc) { + super(storage, desc); + } + + public ParsedStatement<T> parse() throws DescriptorParsingException { + ParsedStatement<T> parsed = super.parse(); + doSemanticAnalysis(parsed); + return parsed; + } + + private void doSemanticAnalysis(ParsedStatement<T> parsed) throws DescriptorParsingException { + Statement<T> stmt = parsed.getRawStatement(); + // statement should never be null + Objects.requireNonNull(stmt); + if (stmt instanceof Add) { + // ADD don't take a WHERE + if (tree.getWhereExpn() != null) { + String msg = "WHERE clause not allowed for ADD"; + throw new DescriptorParsingException(msg); + } + // ADD requires all keys to be specified in the desc + ensureAllKeysSpecified(); + } + if (stmt instanceof Replace) { + // REPLACE requires a WHERE + if (tree.getWhereExpn() == null) { + String msg = "WHERE clause required for REPLACE"; + throw new DescriptorParsingException(msg); + } + // REPLACE requires all keys to be specified in the desc + ensureAllKeysSpecified(); + } + if (stmt instanceof Update) { + // WHERE required for UPDATE + if (tree.getWhereExpn() == null) { + String msg = "WHERE clause required for UPDATE"; + throw new DescriptorParsingException(msg); + } + // SET required for UPDATE + if (setList.getValues().size() == 0) { + String msg = "SET list required for UPDATE"; + throw new DescriptorParsingException(msg); + } + ensureKeyInSetIsKnown(); + } + if (stmt instanceof Remove && setList.getValues().size() > 0) { + String msg = "SET not allowed for REMOVE"; + throw new DescriptorParsingException(msg); + } + // matches for QUERY/QUERY-COUNT + 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); + } + } else { + assert(stmt instanceof DataModifyingStatement); + // only queries can have sort/limit expressions + if (this.tree.getLimitExpn() != null || this.tree.getSortExpn() != null) { + String msg = "LIMIT/SORT only allowed for QUERY/QUERY-COUNT"; + throw new DescriptorParsingException(msg); + } + } + } + + private void ensureKeyInSetIsKnown() throws DescriptorParsingException { + // retrieve the expected keys list from the category + Collection<Key<?>> keys = desc.getCategory().getKeys(); + Set<Key<?>> expectedSet = new HashSet<>(keys.size()); + expectedSet.addAll(keys); + List<String> unknownKeys = new ArrayList<>(); + try { + for (SetListValue val: setList.getValues()) { + // this may throw CCE if LHS is a free parameter. That's not + // allowed though. + Key<?> key = (Key<?>)val.getKey().getValue(); + if (!expectedSet.contains(key)) { + unknownKeys.add(key.getName()); + } + } + } catch (ClassCastException e) { + // LHS of set pair a free variable, which isn't allowed + String msg = "LHS of set list pair must not be a free variable."; + throw new DescriptorParsingException(msg); + } + if (!unknownKeys.isEmpty()) { + String msg = "Unknown key(s) in SET: '" + unknownKeys + "'"; + throw new DescriptorParsingException(msg); + } + } + + private void ensureAllKeysSpecified() throws DescriptorParsingException { + // retrieve the expected keys list from the category + Collection<Key<?>> keys = desc.getCategory().getKeys(); + Set<Key<?>> expectedSet = new HashSet<>(keys.size()); + expectedSet.addAll(keys); + Set<Key<?>> keysInSetList = new HashSet<>(keys.size()); + try { + for (SetListValue val: setList.getValues()) { + // this may throw CCE + Key<?> key = (Key<?>)val.getKey().getValue(); + keysInSetList.add(key); + } + } catch (ClassCastException e) { + // LHS of set pair a free variable, which isn't allowed + String msg = "LHS of set list pair must not be a free variable."; + throw new DescriptorParsingException(msg); + } + if (!keysInSetList.equals(expectedSet)) { + String msg = "Keys don't match keys in category. Expected the following keys: " + expectedSet; + throw new DescriptorParsingException(msg); + }; + } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java Fri Oct 04 19:34:29 2013 +0200 @@ -0,0 +1,65 @@ +/* + * Copyright 2012, 2013 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.internal.statement; + +import com.redhat.thermostat.storage.core.DescriptorParsingException; +import com.redhat.thermostat.storage.core.ParsedStatement; +import com.redhat.thermostat.storage.model.Pojo; + +/** + * Common interface for statement descriptor parsers. + * + * @see BasicDescriptorParser + * @see SemanticsEnabledDescriptorParser + * + * @param <T> The {@link Pojo} model type pertaining to this parser. + */ +interface StatementDescriptorParser<T extends Pojo> { + + /** + * Parses a descriptor, possibly performing some semantic analysis after + * parsing. + * + * @throws DescriptorParsingException + * if the descriptor failed to parse or did not pass semantic + * analysis. + * @return An intermediary representation suitable for repeated execution of + * the prepared statement. + */ + ParsedStatement<T> parse() throws DescriptorParsingException; + +}
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Fri Oct 04 19:34:29 2013 +0200 @@ -47,7 +47,6 @@ import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import org.junit.Before; @@ -67,6 +66,7 @@ import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.VmInfo; +import com.redhat.thermostat.storage.model.VmInfo.KeyValuePair; public class VmInfoDAOTest { @@ -157,9 +157,9 @@ assertTrue(keys.contains(new Key<String>("vmName"))); assertTrue(keys.contains(new Key<String>("vmInfo"))); assertTrue(keys.contains(new Key<String>("vmVersion"))); - assertTrue(keys.contains(new Key<Map<String, String>>("properties"))); - assertTrue(keys.contains(new Key<Map<String, String>>("environment"))); - assertTrue(keys.contains(new Key<List<String>>("loadedNativeLibraries"))); + assertTrue(keys.contains(new Key<KeyValuePair[]>("propertiesAsArray"))); + assertTrue(keys.contains(new Key<KeyValuePair[]>("environmentAsArray"))); + assertTrue(keys.contains(new Key<String[]>("loadedNativeLibraries"))); assertTrue(keys.contains(new Key<Long>("startTimeStamp"))); assertTrue(keys.contains(new Key<Long>("stopTimeStamp"))); assertTrue(keys.contains(new Key<Long>("uid")));
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParserTest.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/BasicDescriptorParserTest.java Fri Oct 04 19:34:29 2013 +0200 @@ -48,7 +48,6 @@ import java.util.ArrayList; import java.util.List; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -104,16 +103,6 @@ when(storage.createReplace(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockReplace); } - @After - public void teardown() { - storage = null; - mockQuery = null; - mockUpdate = null; - mockReplace = null; - mockAdd = null; - mockRemove = null; - } - @SuppressWarnings({ "rawtypes", "unchecked" }) @Test public void testParseAggregateCount() throws DescriptorParsingException { @@ -1872,139 +1861,4 @@ } } - @Test - public void rejectAddWithWhere() throws DescriptorParsingException { - String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' , 'c' = 'd' WHERE 'a' = 'b'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("ADD operation does not support WHERE clauses!"); - } catch (DescriptorParsingException e) { - // pass - assertTrue(e.getMessage().contains("WHERE clause not allowed for ADD")); - } - } - - @Test - public void rejectReplaceWithoutWhere() throws DescriptorParsingException { - String descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' , 'c' = 'd'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("REPLACE operation requires WHERE clause, and should not parse!"); - } catch (DescriptorParsingException e) { - // pass - assertTrue(e.getMessage().contains("WHERE clause required for REPLACE")); - } - } - - @Test - public void rejectUpdateWithoutWhere() throws DescriptorParsingException { - String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' , 'c' = 'd'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("UPDATE operation requires WHERE clause, and should not parse!"); - } catch (DescriptorParsingException e) { - // pass - assertTrue(e.getMessage().contains("WHERE clause required for UPDATE")); - } - } - - @Test - public void rejectRemoveWithSetList() throws DescriptorParsingException { - String descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("REMOVE does not allow SET list, and should not parse!"); - } catch (DescriptorParsingException e) { - // pass - assertTrue(e.getMessage().contains("SET not allowed for REMOVE")); - } - } - - @Test - public void rejectQueryWithSetList() throws DescriptorParsingException { - String descString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("QUERY does not allow SET list, and should not parse!"); - } catch (DescriptorParsingException e) { - // pass - assertTrue(e.getMessage().contains("SET not allowed for QUERY")); - } - } - - - private void doRejectWriteSortLimitTest(String descString, String failmsg) { - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail(failmsg); - } catch (DescriptorParsingException e) { - // pass - assertEquals("LIMIT/SORT only allowed for QUERY/QUERY-COUNT", e.getMessage()); - } - } - - @Test - public void rejectWritesWithSortLimit() throws DescriptorParsingException { - // Update rejects - String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b' SORT 'a' DSC"; - String failmsg = "SORT in UPDATE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b' LIMIT 1"; - failmsg = "LIMIT in UPDATE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - - // Remove rejects - descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' LIMIT 1"; - failmsg = "LIMIT in REMOVE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' SORT 'a' ASC"; - failmsg = "SORT in REMOVE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - - // Replace rejects - descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b' LIMIT 1"; - failmsg = "LIMIT in REPLACE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' WHERE 'a' = 'b' SORT 'a' ASC"; - failmsg = "SORT in REPLACE is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - - // Add rejects - descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' LIMIT 1"; - failmsg = "LIMIT in ADD is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b' SORT 'a' ASC"; - failmsg = "SORT in ADD is not allowed, and should not parse!"; - doRejectWriteSortLimitTest(descString, failmsg); - } - - @Test - public void rejectUpdateWithoutSet() throws DescriptorParsingException { - String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b'"; - StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); - parser = new BasicDescriptorParser<>(storage, desc); - try { - parser.parse(); - fail("UPDATE requires SET list, and should not parse!"); - } catch (DescriptorParsingException e) { - // pass - assertEquals("SET list required for UPDATE", e.getMessage()); - } - } - - // TODO: add tests where set list does not match all props of Pojo class for - // add/replace - }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImplTest.java Fri Oct 04 18:13:31 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImplTest.java Fri Oct 04 19:34:29 2013 +0200 @@ -45,8 +45,10 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Test; @@ -103,7 +105,7 @@ } @Test - public void canDoParsingPatchingAndExecution() throws Exception { + public void canDoParsingPatchingAndExecutionQuery() throws Exception { String queryString = "QUERY foo WHERE 'a' = ?s"; @SuppressWarnings("unchecked") StatementDescriptor<Pojo> desc = (StatementDescriptor<Pojo>) mock(StatementDescriptor.class); @@ -135,6 +137,9 @@ @SuppressWarnings("unchecked") Category<FooPojo> mockCategory = (Category<FooPojo>) mock(Category.class); when(desc.getCategory()).thenReturn(mockCategory); + Set<Key<?>> categoryKeys = new HashSet<>(); + categoryKeys.add(new Key<>("foo")); + when(mockCategory.getKeys()).thenReturn(categoryKeys); when(mockCategory.getDataClass()).thenReturn(FooPojo.class); when(mockCategory.getName()).thenReturn("foo-table"); BackingStorage storage = mock(BackingStorage.class); @@ -164,6 +169,9 @@ @SuppressWarnings("unchecked") Category<FancyFoo> mockCategory = (Category<FancyFoo>) mock(Category.class); when(desc.getCategory()).thenReturn(mockCategory); + Set<Key<?>> categoryKeys = new HashSet<>(); + categoryKeys.add(new Key<>("fancyFoo")); + when(mockCategory.getKeys()).thenReturn(categoryKeys); when(mockCategory.getDataClass()).thenReturn(FancyFoo.class); when(mockCategory.getName()).thenReturn("foo-table"); BackingStorage storage = mock(BackingStorage.class); @@ -204,6 +212,9 @@ when(desc.getDescriptor()).thenReturn(addString); @SuppressWarnings("unchecked") Category<FooPojo> mockCategory = (Category<FooPojo>) mock(Category.class); + Set<Key<?>> categoryKeys = new HashSet<>(); + categoryKeys.add(new Key<>("foo")); + when(mockCategory.getKeys()).thenReturn(categoryKeys); when(desc.getCategory()).thenReturn(mockCategory); when(mockCategory.getDataClass()).thenReturn(FooPojo.class); when(mockCategory.getName()).thenReturn("foo-table"); @@ -240,6 +251,9 @@ when(desc.getDescriptor()).thenReturn(addString); @SuppressWarnings("unchecked") Category<FooPojo> mockCategory = (Category<FooPojo>) mock(Category.class); + Set<Key<?>> categoryKeys = new HashSet<>(); + categoryKeys.add(new Key<>("foo")); + when(mockCategory.getKeys()).thenReturn(categoryKeys); when(desc.getCategory()).thenReturn(mockCategory); when(mockCategory.getDataClass()).thenReturn(FooPojo.class); when(mockCategory.getName()).thenReturn("foo-table");
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/SemanticsEnabledDescriptorParserTest.java Fri Oct 04 19:34:29 2013 +0200 @@ -0,0 +1,411 @@ +/* + * Copyright 2012, 2013 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.internal.statement; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +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.when; + +import org.junit.Before; +import org.junit.Test; + +import com.redhat.thermostat.storage.core.Add; +import com.redhat.thermostat.storage.core.AggregateQuery.AggregateFunction; +import com.redhat.thermostat.storage.core.BackingStorage; +import com.redhat.thermostat.storage.core.DescriptorParsingException; +import com.redhat.thermostat.storage.core.Key; +import com.redhat.thermostat.storage.core.ParsedStatement; +import com.redhat.thermostat.storage.core.Query; +import com.redhat.thermostat.storage.core.Remove; +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.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.model.AgentInformation; + +public class SemanticsEnabledDescriptorParserTest { + + private static final String COMPLETE_SET_LIST_AGENT_INFO = "SET " + + "'" + Key.AGENT_ID.getName() + "' = ?s , " + + "'" + AgentInfoDAO.START_TIME_KEY.getName() + "' = ?l , " + + "'" + AgentInfoDAO.STOP_TIME_KEY.getName() + "' = ?l , " + + "'" + AgentInfoDAO.ALIVE_KEY.getName() + "' = ?b , " + + "'" + AgentInfoDAO.CONFIG_LISTEN_ADDRESS.getName() + "' = ?s"; + + private static final String INCOMPLETE_SET_LIST_AGENT_INFO = "SET " + + "'" + Key.AGENT_ID.getName() + "' = ?s , " + + "'" + AgentInfoDAO.START_TIME_KEY.getName() + "' = ?l , " + + // stop-time key missing + "'" + AgentInfoDAO.ALIVE_KEY.getName() + "' = ?b , " + + "'" + AgentInfoDAO.CONFIG_LISTEN_ADDRESS.getName() + "' = ?s"; + + private BackingStorage storage; + private Query<AgentInformation> mockQuery; + private SemanticsEnabledDescriptorParser<AgentInformation> parser; + private Add<AgentInformation> mockAdd; + private Update<AgentInformation> mockUpdate; + private Replace<AgentInformation> mockReplace; + private Remove<AgentInformation> mockRemove; + + @SuppressWarnings("unchecked") + @Before + public void setup() { + storage = mock(BackingStorage.class); + mockQuery = mock(Query.class); + // setup for QUERY/QUERY-COUNT + when(storage.createQuery(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockQuery); + when(storage.createAggregateQuery(eq(AggregateFunction.COUNT), (eq(AgentInfoDAO.CATEGORY)))).thenReturn(mockQuery); + // setup for ADD + mockAdd = mock(Add.class); + when(storage.createAdd(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockAdd); + // setup for UPDATE + mockUpdate = mock(Update.class); + when(storage.createUpdate(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockUpdate); + // setup for REMOVE + mockRemove = mock(Remove.class); + when(storage.createRemove(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockRemove); + // setup for REPLACE + mockReplace = mock(Replace.class); + when(storage.createReplace(eq(AgentInfoDAO.CATEGORY))).thenReturn(mockReplace); + } + + @Test + public void canParseQueryWithLimit() throws DescriptorParsingException { + String descString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT 1"; + doSemanticsBasicParseTest(descString); + + descString = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName() + " LIMIT 1"; + doSemanticsBasicParseTest(descString); + } + + @Test + 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"; + doSemanticsBasicParseTest(descString); + } + + /* + * Tests whether parse succeeds if some properties are missing from the + * UPDATE descriptor. Update is the operation to be used in this case. + */ + @Test + public void canParseUpdateWithSomePropertiesMissing() throws DescriptorParsingException { + String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + + " SET '" + Key.AGENT_ID.getName() + "' = 'b' , " + + "'"+ AgentInfoDAO.ALIVE_KEY.getName() + "' = ?b" + + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s"; + doSemanticsBasicParseTest(descString); + } + + /* + * Tests whether parse succeeds if ALL properties are given in the + * ADD descriptor. + */ + @Test + public void canParseAddWithAllPropertiesGiven() throws DescriptorParsingException { + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO; + doSemanticsBasicParseTest(descString); + } + + /* + * Tests whether parse succeeds if ALL properties are given in the + * REPLACE descriptor. + */ + @Test + public void canParseReplaceWithAllPropertiesGiven() throws DescriptorParsingException { + String descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + + " " + COMPLETE_SET_LIST_AGENT_INFO + + " WHERE '" + AgentInfoDAO.CONFIG_LISTEN_ADDRESS.getName()+ "' = ?s"; + doSemanticsBasicParseTest(descString); + } + + private void doSemanticsBasicParseTest(String strDesc) throws DescriptorParsingException { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + ParsedStatement<AgentInformation> p = parser.parse(); + assertNotNull(p); + } + + @Test + public void rejectAddWithWhere() throws DescriptorParsingException { + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO + " WHERE 'a' = 'b'"; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("ADD operation does not support WHERE clauses!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("WHERE clause not allowed for ADD")); + } + } + + @Test + public void rejectReplaceWithoutWhere() throws DescriptorParsingException { + String descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("REPLACE operation requires WHERE clause, and should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("WHERE clause required for REPLACE")); + } + } + + @Test + public void rejectUpdateWithoutWhere() throws DescriptorParsingException { + String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + + " SET '" + Key.AGENT_ID.getName() + "' = 'b' , 'c' = 'd'"; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("UPDATE operation requires WHERE clause, and should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("WHERE clause required for UPDATE")); + } + } + + @Test + public void rejectRemoveWithSetList() throws DescriptorParsingException { + String descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b'" + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'"; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("REMOVE does not allow SET list, and should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue("message was: " + e.getMessage(), e.getMessage().contains("SET not allowed for REMOVE")); + } + } + + @Test + public void rejectQueryWithSetList() throws DescriptorParsingException { + String descString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = 'b'" + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'"; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("QUERY does not allow SET list, and should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("SET not allowed for QUERY")); + } + } + + + private void doRejectWriteSortLimitTest(String descString, String failmsg) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail(failmsg); + } catch (DescriptorParsingException e) { + // pass + assertEquals("LIMIT/SORT only allowed for QUERY/QUERY-COUNT", e.getMessage()); + } + } + + @Test + public void rejectWritesWithSortLimit() throws DescriptorParsingException { + // Update rejects + String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + + " SET '" + Key.AGENT_ID.getName() + "' = 'b'" + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " SORT 'a' DSC"; // this is intentionally wrong for this test + String failmsg = "SORT in UPDATE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + + " SET '" + Key.AGENT_ID.getName() + "' = 'b'" + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " LIMIT 1"; // this is intentionally wrong for this test + failmsg = "LIMIT in UPDATE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + + // Remove rejects + descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " LIMIT 1"; // this is intentionally wrong for this test + failmsg = "LIMIT in REMOVE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + descString = "REMOVE " + AgentInfoDAO.CATEGORY.getName() + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " SORT 'a' ASC"; + failmsg = "SORT in REMOVE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + + // Replace rejects + descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " " + + COMPLETE_SET_LIST_AGENT_INFO + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " LIMIT 1"; + failmsg = "LIMIT in REPLACE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + descString = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " " + + COMPLETE_SET_LIST_AGENT_INFO + + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'" + + " SORT 'a' ASC"; + failmsg = "SORT in REPLACE is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + + // Add rejects + descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO + " LIMIT 1"; + failmsg = "LIMIT in ADD is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + COMPLETE_SET_LIST_AGENT_INFO + " SORT 'a' ASC"; + failmsg = "SORT in ADD is not allowed, and should not parse!"; + doRejectWriteSortLimitTest(descString, failmsg); + } + + @Test + public void rejectUpdateWithoutSet() throws DescriptorParsingException { + String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = 'b'"; + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("UPDATE requires SET list, and should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertEquals("SET list required for UPDATE", e.getMessage()); + } + } + + /* + * Tests whether parser fails if some properties are missing from the + * ADD descriptor, but the category specified as keys. + */ + @Test + public void rejectAddWithSomePropertiesMissing() throws DescriptorParsingException { + String strDesc = "ADD " + AgentInfoDAO.CATEGORY.getName() + " " + INCOMPLETE_SET_LIST_AGENT_INFO; + doRejectIncompleteKeysTest(strDesc); + } + + /* + * Tests whether parser fails if some properties are missing from the + * REPLACE descriptor, but the category specified as keys. + */ + @Test + public void rejectReplaceWithSomePropertiesMissing() throws DescriptorParsingException { + // The following descriptor should be valid except for the one missing key. + String strDesc = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " " + + INCOMPLETE_SET_LIST_AGENT_INFO + " WHERE 'foo' = ?s"; + doRejectIncompleteKeysTest(strDesc); + } + + private void doRejectIncompleteKeysTest(String strDesc) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("stop timestamp key missing. Should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertTrue(e.getMessage().contains("Keys don't match keys in category.")); + } + } + + /* + * Tests whether parser fails if LHS of any SET pair of ADD/REPLACE is a + * free variable. We disallow this, since key completeness checks don't make + * much sense (prior patching) without this. + */ + @Test + public void rejectAddReplaceWithFreeVarOnLHS() throws DescriptorParsingException { + // second pair has ?s as LHS + String strDesc = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET " + + "'" + Key.AGENT_ID.getName() + "' = ?s , ?s = ?b"; + doRejectWithFreeVarOnLHS(strDesc); + + // ?b is the offending part + strDesc = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " SET " + + "?b = ?s WHERE 'foo' = ?s"; + doRejectWithFreeVarOnLHS(strDesc); + } + + private void doRejectWithFreeVarOnLHS(String strDesc) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("SET with free variable on LHS should not parse!"); + } catch (DescriptorParsingException e) { + // pass + assertEquals("LHS of set list pair must not be a free variable.", e.getMessage()); + } + } + + /* + * Tests whether a key set via UPDATE exists in the category. + */ + @Test + public void rejectUpdateWithUnknownKeyInSet() { + String descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'iDoNotExist' = ?s WHERE 'foo' = 'b'"; + String detail = "'[iDoNotExist]'"; + doRejectUpdateWithUnknownKeyTest(descString, detail); + descString = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'iDoNotExist' = ?s , 'notHere' = ?b WHERE 'foo' = 'b'"; + detail = "'[iDoNotExist, notHere]'"; + doRejectUpdateWithUnknownKeyTest(descString, detail); + } + + private void doRejectUpdateWithUnknownKeyTest(String descString, + String detailMsg) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new SemanticsEnabledDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail("SET for unknown key in category should not parse"); + } catch (DescriptorParsingException e) { + // pass + assertEquals("Unknown key(s) in SET: " + detailMsg, e.getMessage()); + } + } + +}