Mercurial > hg > release > thermostat-0.15
changeset 1264:e103fd511693
Add support for more types which are allowed as free variables.
Reviewed-by: omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-September/008294.html
line wrap: on
line diff
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/PreparedParameters.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/PreparedParameters.java Wed Sep 18 18:15:22 2013 +0200 @@ -36,6 +36,8 @@ package com.redhat.thermostat.storage.core; +import com.redhat.thermostat.storage.model.Pojo; + /** * * Shared class for setting prepared parameters. @@ -60,23 +62,58 @@ } @Override + public void setLongList(int paramIndex, long[] paramValue) { + setType(paramIndex, paramValue, Long[].class); + } + + @Override public void setInt(int paramIndex, int paramValue) { setType(paramIndex, paramValue, Integer.class); } @Override + public void setIntList(int paramIndex, int[] paramValue) { + setType(paramIndex, paramValue, Integer[].class); + } + + @Override + public void setBoolean(int paramIndex, boolean paramValue) { + setType(paramIndex, paramValue, Boolean.class); + } + + @Override + public void setBooleanList(int paramIndex, boolean[] paramValue) { + setType(paramIndex, paramValue, Boolean[].class); + } + + @Override + public void setString(int paramIndex, String paramValue) { + setType(paramIndex, paramValue, String.class); + } + + @Override public void setStringList(int paramIndex, String[] paramValue) { setType(paramIndex, paramValue, String[].class); } - + @Override - public void setBoolean(int paramIndex, boolean paramValue) { - setType(paramIndex, paramValue, Boolean.class); + public void setDouble(int paramIndex, double paramValue) { + setType(paramIndex, paramValue, Double.class); } @Override - public void setString(int paramIndex, String paramValue) { - setType(paramIndex, paramValue, String.class); + public void setDoubleList(int paramIndex, double[] paramValue) { + setType(paramIndex, paramValue, Double[].class); + } + + @Override + public void setPojo(int paramIndex, Pojo paramValue) { + setType(paramIndex, paramValue, Pojo.class); + } + + @Override + public void setPojoList(int paramIndex, Pojo[] paramValue) { + setType(paramIndex, paramValue, Pojo[].class); } private void setType(int paramIndex, Object paramValue, Class<?> paramType) {
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/PreparedStatementSetter.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/PreparedStatementSetter.java Wed Sep 18 18:15:22 2013 +0200 @@ -36,6 +36,8 @@ package com.redhat.thermostat.storage.core; +import com.redhat.thermostat.storage.model.Pojo; + /** * Package private interface in order to ensure consistency of * setters between {@link PreparedStatement} and {@link PreparedParamenters}. @@ -45,12 +47,26 @@ void setBoolean(int paramIndex, boolean paramValue); + void setBooleanList(int paramIndex, boolean[] paramValue); + void setLong(int paramIndex, long paramValue); + void setLongList(int paramIndex, long[] paramValue); + void setInt(int paramIndex, int paramValue); + void setIntList(int paramIndex, int[] paramValue); + void setString(int paramIndex, String paramValue); void setStringList(int paramIndex, String[] paramValue); + void setDouble(int paramIndex, double paramValue); + + void setDoubleList(int paramIndex, double[] paramValue); + + void setPojo(int paramIndex, Pojo paramValue); + + void setPojoList(int paramIndex, Pojo[] paramValue); + }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImpl.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImpl.java Wed Sep 18 18:15:22 2013 +0200 @@ -89,11 +89,21 @@ } @Override + public void setLongList(int paramIndex, long[] paramValue) { + params.setLongList(paramIndex, paramValue); + } + + @Override public void setInt(int paramIndex, int paramValue) { params.setInt(paramIndex, paramValue); } @Override + public void setIntList(int paramIndex, int[] paramValue) { + params.setIntList(paramIndex, paramValue); + } + + @Override public void setStringList(int paramIndex, String[] paramValue) { params.setStringList(paramIndex, paramValue); } @@ -104,6 +114,31 @@ } @Override + public void setBooleanList(int paramIndex, boolean[] paramValue) { + params.setBooleanList(paramIndex, paramValue); + } + + @Override + public void setDouble(int paramIndex, double paramValue) { + params.setDouble(paramIndex, paramValue); + } + + @Override + public void setDoubleList(int paramIndex, double[] paramValue) { + params.setDoubleList(paramIndex, paramValue); + } + + @Override + public void setPojo(int paramIndex, Pojo paramValue) { + params.setPojo(paramIndex, paramValue); + } + + @Override + public void setPojoList(int paramIndex, Pojo[] paramValue) { + params.setPojoList(paramIndex, paramValue); + } + + @Override public int execute() throws StatementExecutionException { if (dmlStatement == null) { throw new IllegalStateException(
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java Wed Sep 18 18:15:22 2013 +0200 @@ -144,6 +144,7 @@ private static final String KEYWORD_LIMIT = "LIMIT"; private static final String KEYWORD_ASC = "ASC"; private static final String KEYWORD_DSC = "DSC"; + private static final String POJO_FREE_PARAMETER_TYPE = "?p"; private static final char PARAM_PLACEHOLDER = '?'; private final String[] tokens; @@ -284,7 +285,7 @@ private void matchValuePair(SetList setList) throws DescriptorParsingException { SetListValue value = new SetListValue(); TerminalNode lval = new TerminalNode(null); - matchTerm(lval, true); + matchTerm(lval, true, true); value.setKey(lval); if (tokens[currTokenIndex].equals("=")) { currTokenIndex++; // = @@ -293,7 +294,7 @@ throw new DescriptorParsingException(msg); } TerminalNode rval = new TerminalNode(null); - matchTerm(rval, false); + matchTerm(rval, false, true); value.setValue(rval); setList.addValue(value); } @@ -499,6 +500,7 @@ String term = getTerm(); if (term.charAt(0) == PARAM_PLACEHOLDER) { assert(placeHolderCount > 0); + ensureValidType(term, "SORT"); if (term.charAt(1) != 's') { String msg = "Sort parameters only accept string types. Placeholder was: " + term; throw new DescriptorParsingException(msg); @@ -512,10 +514,32 @@ member.setSortKey(stringTerm); } + /* + * Check if the free parameter type is valid in a given context. Currently, + * list and pojo free type parameters are invalid for LIMIT, SORT and WHERE. + * + * Currently, no list types and no Pojo types are allowed. + */ + private void ensureValidType(String term, String contextName) throws DescriptorParsingException { + if (term.length() > 2) { + // Don't allow list types for invalid contexts + // Only list type free variables have 3 characters + String format = "List free variable type not allowed in %s context"; + String msg = String.format(format, contextName); + throw new DescriptorParsingException(msg); + } + if (term.equals(POJO_FREE_PARAMETER_TYPE)) { + String format = "Pojo free variable type not allowed in %s context"; + String msg = String.format(format, contextName); + throw new DescriptorParsingException(msg); + } + } + private void matchTerm(LimitExpression expn) throws DescriptorParsingException { String term = getTerm(); if (term.charAt(0) == PARAM_PLACEHOLDER) { assert(placeHolderCount > 0); + ensureValidType(term, "LIMIT"); if (term.charAt(1) != 'i') { String msg = "Limit parameters only accept integer types. Placeholder was: " + term; throw new DescriptorParsingException(msg); @@ -533,11 +557,40 @@ } expn.setValue(limitVal); } + + /** + * Calls {@link #matchTerm(TerminalNode, boolean, boolean)} with a + * {@code false isSetListContext} parameter. + */ + private void matchTerm(TerminalNode node, boolean isLHS) throws DescriptorParsingException { + // default to false + matchTerm(node, isLHS, false); + } - private void matchTerm(TerminalNode node, boolean isLHS) throws DescriptorParsingException { + /** + * Match a terminal in a TerminalNode context. Only where expression, and + * set list context take this code path. + * + * @param node + * The terminal node which is parsed at this point. + * @param isLHS + * {@code true} if and only if the node is the left hand side of + * a binary expression. + * @param isSetListContext + * {@code true} if and only if the node is in a set list context + * or conversely NOT in a where expression context. + * @throws DescriptorParsingException + * If and error was encountered parsing the terminal string + * token. + */ + private void matchTerm(TerminalNode node, boolean isLHS, + boolean isSetListContext) throws DescriptorParsingException { String term = getTerm(); if (term.charAt(0) == PARAM_PLACEHOLDER) { assert(placeHolderCount > 0); + if (!isSetListContext) { + ensureValidType(term, "WHERE"); + } UnfinishedValueNode patchNode = new UnfinishedValueNode(); patchNode.setParameterIndex(placeHolderCount - 1); patchNode.setLHS(isLHS); @@ -611,16 +664,25 @@ // illegal type return null; } - assert(term.equals("i") || term.equals("l") || term.equals("s") || term.equals("s[") || term.equals("b")); + // free variable types can have 1 or 2 characters. + assert(term.length() > 0 && term.length() < 3); char switchChar = term.charAt(0); Class<?> type = null; switch (switchChar) { case 'i': { - type = Integer.class; + if (term.length() == 1) { + type = Integer.class; + } else if (term.length() == 2 && term.charAt(1) == '[') { + type = Integer[].class; + } break; } case 'l': { - type = Long.class; + if (term.length() == 1) { + type = Long.class; + } else if (term.length() == 2 && term.charAt(1) == '[') { + type = Long[].class; + } break; } case 's': { @@ -632,11 +694,31 @@ break; } case 'b': { - type = Boolean.class; + if (term.length() == 1) { + type = Boolean.class; + } else if (term.length() == 2 && term.charAt(1) == '[') { + type = Boolean[].class; + } + break; + } + case 'd': { + if (term.length() == 1) { + type = Double.class; + } else if (term.length() == 2 && term.charAt(1) == '[') { + type = Double[].class; + } + break; + } + case 'p': { + if (term.length() == 1) { + type = Pojo.class; + } else if (term.length() == 2 && term.charAt(1) == '[') { + type = Pojo[].class; + } break; } default: - assert (type == null); + assert(type == null); break; } return type;
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/ParsedStatementImplTest.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/ParsedStatementImplTest.java Wed Sep 18 18:15:22 2013 +0200 @@ -37,6 +37,7 @@ 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; @@ -352,7 +353,7 @@ @Test public void canPatchBasicSetListAdd() throws IllegalPatchException { // create the parsedStatementImpl we are going to use - DataModifyingStatement<TestPojo> stmt = new TestAdd(); + DataModifyingStatement<TestPojo> stmt = new TestAdd<>(); ParsedStatementImpl<TestPojo> parsedStmt = new ParsedStatementImpl<>(stmt, TestPojo.class); SuffixExpression suffixExpn = new SuffixExpression(); SetList setList = buildSetList(); @@ -369,7 +370,7 @@ // finally test the patching Add<TestPojo> add = (Add<TestPojo>)parsedStmt.patchStatement(params); assertTrue(add instanceof TestAdd); - TestAdd q = (TestAdd)add; + TestAdd<TestPojo> q = (TestAdd<TestPojo>)add; Pojo testPojo = q.pojo; assertTrue(testPojo instanceof TestPojo); TestPojo tPojo = (TestPojo)testPojo; @@ -379,6 +380,67 @@ /* * Test for patching of: + * "ADD something SET 'someList' = ?p[" + */ + @Test + public void canPatchSetListAddWithPojoList() throws IllegalPatchException { + // create the parsedStatementImpl we are going to use + DataModifyingStatement<FancyPojo> stmt = new TestAdd<>(); + ParsedStatementImpl<FancyPojo> parsedStmt = new ParsedStatementImpl<>(stmt, FancyPojo.class); + SuffixExpression suffixExpn = new SuffixExpression(); + SetList setList = new SetList(); + parsedStmt.setSetList(setList); + TerminalNode someProperty = new TerminalNode(null); + someProperty.setValue(new Key<>("someList")); + TerminalNode somePropertyVal = new TerminalNode(null); + UnfinishedValueNode unfinishedPojoList = new UnfinishedValueNode(); + unfinishedPojoList.setLHS(false); + unfinishedPojoList.setType(Pojo[].class); + unfinishedPojoList.setParameterIndex(0); + somePropertyVal.setValue(unfinishedPojoList); + SetListValue value = new SetListValue(); + value.setKey(someProperty); + value.setValue(somePropertyVal); + setList.addValue(value); + suffixExpn.setLimitExpn(null); + suffixExpn.setSortExpn(null); + suffixExpn.setWhereExpn(null); + parsedStmt.setSuffixExpression(suffixExpn); + assertEquals(1, setList.getValues().size()); + PreparedStatementImpl<Pojo> preparedStatement = new PreparedStatementImpl<>(1); + TestPojo elem1 = new TestPojo(); + elem1.setFooTimeStamp(-300); + elem1.setWriterId("elem1"); + TestPojo elem2 = new TestPojo(); + elem2.setFooTimeStamp(-301); + elem2.setWriterId("elem2"); + TestPojo[] elems = new TestPojo[] { + elem1, elem2 + }; + preparedStatement.setPojoList(0, elems); + PreparedParameter[] params = preparedStatement.getParams(); + // finally test the patching + Add<FancyPojo> add = (Add<FancyPojo>)parsedStmt.patchStatement(params); + assertTrue(add instanceof TestAdd); + TestAdd<FancyPojo> q = (TestAdd<FancyPojo>)add; + Pojo testPojo = q.pojo; + assertTrue(testPojo instanceof FancyPojo); + FancyPojo tPojo = (FancyPojo)testPojo; + assertEquals(null, tPojo.getWriterId()); + assertNotNull(tPojo.getSomeList()); + assertEquals(2, tPojo.getSomeList().length); + TestPojo first = tPojo.getSomeList()[0]; + TestPojo second = tPojo.getSomeList()[1]; + assertEquals(elem1, first); + assertEquals(elem2, second); + assertEquals("elem1", first.getWriterId()); + assertEquals(-300, first.getFooTimeStamp()); + assertEquals("elem2", second.getWriterId()); + assertEquals(-301, second.getFooTimeStamp()); + } + + /* + * Test for patching of: * "REPLACE something SET ?s = 'foo-bar', 'fooTimeStamp' = ?l WHERE 'foo' = ?i" */ @Test @@ -571,7 +633,7 @@ @Test public void failPatchSetListAddWrongType() throws IllegalPatchException { // create the parsedStatementImpl we are going to use - DataModifyingStatement<TestPojo> stmt = new TestAdd(); + DataModifyingStatement<TestPojo> stmt = new TestAdd<>(); ParsedStatementImpl<TestPojo> parsedStmt = new ParsedStatementImpl<>(stmt, TestPojo.class); SuffixExpression suffixExpn = new SuffixExpression(); SetList setList = buildSetList(); @@ -599,7 +661,7 @@ @Test public void failPatchSetListAddInsufficientParams() throws IllegalPatchException { // create the parsedStatementImpl we are going to use - DataModifyingStatement<TestPojo> stmt = new TestAdd(); + DataModifyingStatement<TestPojo> stmt = new TestAdd<>(); ParsedStatementImpl<TestPojo> parsedStmt = new ParsedStatementImpl<>(stmt, TestPojo.class); SuffixExpression suffixExpn = new SuffixExpression(); SetList setList = buildSetList(); @@ -756,7 +818,7 @@ } - private static class TestAdd implements Add<TestPojo> { + private static class TestAdd<T extends Pojo> implements Add<T> { private Pojo pojo; @@ -839,6 +901,20 @@ this.fooTimeStamp = fooTimeStamp; } + } + + public static class FancyPojo extends TestPojo { + + private TestPojo[] someList; + + public TestPojo[] getSomeList() { + return someList; + } + + public void setSomeList(TestPojo[] someList) { + this.someList = someList; + } + } }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImplTest.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/PreparedStatementImplTest.java Wed Sep 18 18:15:22 2013 +0200 @@ -136,7 +136,7 @@ when(mockCategory.getDataClass()).thenReturn(FooPojo.class); when(mockCategory.getName()).thenReturn("foo-table"); BackingStorage storage = mock(BackingStorage.class); - TestAdd add = new TestAdd(); + TestAdd<FooPojo> add = new TestAdd<>(); when(storage.createAdd(mockCategory)).thenReturn(add); PreparedStatement<FooPojo> preparedStatement = new PreparedStatementImpl<FooPojo>(storage, desc); preparedStatement.setString(0, "foo-val"); @@ -155,6 +155,47 @@ } @Test + public void canDoParsingPatchingAndExecutionForAddInvolvingFancyPojo() throws Exception { + String addString = "ADD foo-table SET 'fancyFoo' = ?p["; + @SuppressWarnings("unchecked") + StatementDescriptor<FancyFoo> desc = (StatementDescriptor<FancyFoo>) mock(StatementDescriptor.class); + when(desc.getDescriptor()).thenReturn(addString); + @SuppressWarnings("unchecked") + Category<FancyFoo> mockCategory = (Category<FancyFoo>) mock(Category.class); + when(desc.getCategory()).thenReturn(mockCategory); + when(mockCategory.getDataClass()).thenReturn(FancyFoo.class); + when(mockCategory.getName()).thenReturn("foo-table"); + BackingStorage storage = mock(BackingStorage.class); + TestAdd<FancyFoo> add = new TestAdd<>(); + when(storage.createAdd(mockCategory)).thenReturn(add); + PreparedStatement<FancyFoo> preparedStatement = new PreparedStatementImpl<FancyFoo>(storage, desc); + FooPojo one = new FooPojo(); + one.setFoo("one"); + FooPojo two = new FooPojo(); + two.setFoo("two"); + FooPojo[] list = new FooPojo[] { + one, two + }; + preparedStatement.setPojoList(0, list); + assertFalse(add.executed); + try { + // this should call add.apply(); + preparedStatement.execute(); + } catch (StatementExecutionException e) { + fail(e.getMessage()); + } + assertTrue(add.pojo != null); + assertTrue(add.pojo instanceof FancyFoo); + assertTrue(add.executed); + FancyFoo fooPojo = (FancyFoo)add.pojo; + assertEquals(2, fooPojo.getFancyFoo().length); + FooPojo first = fooPojo.getFancyFoo()[0]; + FooPojo second = fooPojo.getFancyFoo()[1]; + assertEquals("one", first.getFoo()); + assertEquals("two", second.getFoo()); + } + + @Test public void canDoParsingPatchingAndExecutionForUpdate() throws Exception { String addString = "UPDATE foo-table SET 'foo' = ?s WHERE 'foo' = ?s"; @SuppressWarnings("unchecked") @@ -281,7 +322,7 @@ } } - private static class TestAdd implements Add<FooPojo> { + private static class TestAdd<T extends Pojo> implements Add<T> { private Pojo pojo; private boolean executed = false; @@ -380,6 +421,20 @@ } } + public static class FancyFoo extends FooPojo { + + private FooPojo[] fancyFoo; + + public FooPojo[] getFancyFoo() { + return fancyFoo; + } + + public void setFancyFoo(FooPojo[] fancyFoo) { + this.fancyFoo = fancyFoo; + } + + } + private static class StubQuery implements Query<Pojo> { private Expression expr;
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParserTest.java Tue Sep 03 14:19:10 2013 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParserTest.java Wed Sep 18 18:15:22 2013 +0200 @@ -45,6 +45,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.List; import org.junit.After; @@ -69,6 +70,7 @@ import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.model.AgentInformation; import com.redhat.thermostat.storage.model.AggregateCount; +import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.storage.query.BinaryComparisonOperator; import com.redhat.thermostat.storage.query.BinaryLogicalOperator; @@ -226,6 +228,150 @@ } @Test + public void testParseDoubleTypeFreeVarInWhere() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' != ?d"; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Double.class); + unfinished.setParameterIndex(0); + doTestType(descrString, unfinished, 1); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * String list free variable type in ADD. + */ + @Test + public void testParseStringListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?s["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(String[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * integer list free variable type in ADD. + */ + @Test + public void testParseDoubleListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?d["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Double[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * double list free variable type in ADD. + */ + @Test + public void testParseIntListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?i["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Integer[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * boolean list free variable type in ADD. + */ + @Test + public void testParseBooleanListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?b["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Boolean[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * long list free variable type in ADD. + */ + @Test + public void testParseLongListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?l["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Long[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * long list free variable type in ADD. + */ + @Test + public void testParsePojoTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support pojo type. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?p"; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Pojo.class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + /* + * SET clauses allow for setting list types. Test that it works for a + * Pojo list free variable type in ADD. + */ + @Test + public void testParsePojoListTypeFreeVarInSetlist() throws DescriptorParsingException { + // use ADD since WHERE should not support list types. + String descString = "ADD " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?p["; + UnfinishedValueNode unfinished = new UnfinishedValueNode(); + unfinished.setLHS(false); + unfinished.setType(Pojo[].class); + unfinished.setParameterIndex(0); + doListTypeTest(descString, unfinished); + } + + private void doListTypeTest(String descString, UnfinishedValueNode unfinished) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, descString); + parser = new StatementDescriptorParser<>(storage, desc); + ParsedStatementImpl<AgentInformation> statement = null; + try { + statement = (ParsedStatementImpl<AgentInformation>)parser.parse(); + } catch (DescriptorParsingException e) { + e.printStackTrace(); + fail(e.getMessage()); + } + assertTrue(statement.getRawStatement() instanceof Add); + SuffixExpression suffix = statement.getSuffixExpression(); + assertNotNull(suffix); + assertNull(suffix.getLimitExpn()); + assertNull(suffix.getSortExpn()); + assertNull(suffix.getWhereExpn()); + SetList list = statement.getSetList(); + assertNotNull(list); + List<SetListValue> mems = list.getValues(); + assertEquals(1, mems.size()); + SetListValue val = mems.get(0); + TerminalNode aKey = new TerminalNode(null); + aKey.setValue(new Key<>("a")); + assertEquals(aKey, val.getKey()); + TerminalNode aVal = new TerminalNode(null); + aVal.setValue(unfinished); + assertEquals(aVal, val.getValue()); + } + + @Test public void testParseIntTypeFreeVarInWhere() throws DescriptorParsingException { String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' != ?i"; UnfinishedValueNode unfinished = new UnfinishedValueNode(); @@ -1440,6 +1586,102 @@ } } + /* + * At this point list types don't make sense in WHERE. + * What should "'a' != [ 'a', 'b' ]" evaluate to? + * How about this? "'key' > [ 1, 2 ]" + * + * The only reasonable use case in WHERE would be 'a' IN [ 'a', 'b' ... ]. + * 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. + */ + @Test + public void rejectListTypesAsFreeVarInInvalidContext() throws DescriptorParsingException { + List<String> illegalDescs = new ArrayList<>(); + + // Where should not support list types/Pojos. + String[] illegalWheres = new String[] { + " WHERE 'a' = ?s[", + " WHERE 'a' = ?i[", + " WHERE 'a' = ?p[", + " WHERE 'a' = ?d[", + }; + + // Make sure we test for QUERY, QUERY-COUNT, REPLACE, UPDATE, REMOVE + // i.e. all that accept a WHERE. + String basicQuery = "QUERY " + AgentInfoDAO.CATEGORY.getName(); + String basicQueryCount = "QUERY-COUNT " + AgentInfoDAO.CATEGORY.getName(); + // note SET clause is legal + String basicUpdate = "UPDATE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?s["; + // note SET clause is legal + String basicReplace = "REPLACE " + AgentInfoDAO.CATEGORY.getName() + " SET 'a' = ?i["; + String basicRemove = "REMOVE " + AgentInfoDAO.CATEGORY.getName(); + for (String where: illegalWheres) { + illegalDescs.add(basicQuery + where); + illegalDescs.add(basicQueryCount + where); + illegalDescs.add(basicUpdate + where); + illegalDescs.add(basicReplace + where); + illegalDescs.add(basicRemove + where); + } + + // Test all illegal descs involving WHERE + doIllegalDescsTest(illegalDescs, "WHERE", "List"); + + // Test limit too + illegalDescs.clear(); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT ?i["); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT ?s["); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT ?p["); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT ?d["); + + doIllegalDescsTest(illegalDescs, "LIMIT", "List"); + + // Test sort + illegalDescs.clear(); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT ?i[ ASC"); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT ?s[ ASC"); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT ?p[ ASC"); + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT ?d[ ASC"); + + doIllegalDescsTest(illegalDescs, "SORT", "List"); + } + + /* + * Pojos don't make sense in a WHERE, LIMIT and SORT clause. This test makes + * sure we reject this right away. + * + */ + @Test + public void rejectPojoTypesAsFreeVarInInvalidContext() throws DescriptorParsingException { + List<String> illegalDescs = new ArrayList<>(); + + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = ?p"); + doIllegalDescsTest(illegalDescs, "WHERE", "Pojo"); + illegalDescs.clear(); + + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " LIMIT ?p"); + doIllegalDescsTest(illegalDescs, "LIMIT", "Pojo"); + illegalDescs.clear(); + + illegalDescs.add("QUERY " + AgentInfoDAO.CATEGORY.getName() + " SORT ?p DSC"); + doIllegalDescsTest(illegalDescs, "SORT", "Pojo"); + } + + private void doIllegalDescsTest(List<String> descs, String errorMsgContext, String type) { + for (String strDesc : descs) { + StatementDescriptor<AgentInformation> desc = new StatementDescriptor<>(AgentInfoDAO.CATEGORY, strDesc); + parser = new StatementDescriptorParser<>(storage, desc); + try { + parser.parse(); + fail(strDesc + " should not parse"); + } catch (DescriptorParsingException e) { + // pass + assertEquals(strDesc + " did not provide correct error message.", + type + " free variable type not allowed in " + errorMsgContext + " context", e.getMessage()); + } + } + } + @Test public void rejectIllegalFreeParamType() throws DescriptorParsingException { // ? should be one of '?i', '?l', '?s', '?b', '?s['
--- a/web/common/src/main/java/com/redhat/thermostat/web/common/WebPreparedStatement.java Tue Sep 03 14:19:10 2013 +0200 +++ b/web/common/src/main/java/com/redhat/thermostat/web/common/WebPreparedStatement.java Wed Sep 18 18:15:22 2013 +0200 @@ -80,16 +80,31 @@ } @Override + public void setBooleanList(int paramIndex, boolean[] paramValue) { + params.setBooleanList(paramIndex, paramValue); + } + + @Override public void setLong(int paramIndex, long paramValue) { params.setLong(paramIndex, paramValue); } @Override + public void setLongList(int paramIndex, long[] paramValue) { + params.setLongList(paramIndex, paramValue); + } + + @Override public void setInt(int paramIndex, int paramValue) { params.setInt(paramIndex, paramValue); } @Override + public void setIntList(int paramIndex, int[] paramValue) { + params.setIntList(paramIndex, paramValue); + } + + @Override public void setString(int paramIndex, String paramValue) { params.setString(paramIndex, paramValue); } @@ -100,6 +115,26 @@ } @Override + public void setDouble(int paramIndex, double paramValue) { + params.setDouble(paramIndex, paramValue); + } + + @Override + public void setDoubleList(int paramIndex, double[] paramValue) { + params.setDoubleList(paramIndex, paramValue); + } + + @Override + public void setPojo(int paramIndex, Pojo paramValue) { + params.setPojo(paramIndex, paramValue); + } + + @Override + public void setPojoList(int paramIndex, Pojo[] paramValue) { + params.setPojoList(paramIndex, paramValue); + } + + @Override public int execute() throws StatementExecutionException { // actual implementation should override this throw new IllegalStateException();