From 38fef212e45f84e89eee9a2ae7a08b3333dafd05 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 21 Jun 2016 15:11:15 +0200 Subject: [PATCH] DATACASS-192 - Fix syntax error when adding columns using CQL generators. The Add-Column CQL generator now generates correct CQL statements. Original pull request: #69. --- .../cassandra/core/cql/CqlIdentifier.java | 20 ++- .../cql/generator/AddColumnCqlGenerator.java | 3 +- .../generator/AlterColumnCqlGenerator.java | 2 +- .../core/keyspace/AddColumnSpecification.java | 18 ++ .../keyspace/ColumnChangeSpecification.java | 28 +++- .../ColumnTypeChangeSpecification.java | 27 ++- ...lterTableCqlGeneratorIntegrationTests.java | 125 ++++++++++++++ .../AlterTableCqlGeneratorUnitTests.java | 156 ++++-------------- .../TableLifecycleIntegrationTests.java | 30 ---- 9 files changed, 241 insertions(+), 168 deletions(-) create mode 100644 spring-cql/src/test/java/org/springframework/cassandra/core/cql/generator/AlterTableCqlGeneratorIntegrationTests.java diff --git a/spring-cql/src/main/java/org/springframework/cassandra/core/cql/CqlIdentifier.java b/spring-cql/src/main/java/org/springframework/cassandra/core/cql/CqlIdentifier.java index 4e9997836..6b6def5a8 100644 --- a/spring-cql/src/main/java/org/springframework/cassandra/core/cql/CqlIdentifier.java +++ b/spring-cql/src/main/java/org/springframework/cassandra/core/cql/CqlIdentifier.java @@ -1,3 +1,18 @@ +/* + * Copyright 2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.cassandra.core.cql; import java.io.Serializable; @@ -23,6 +38,7 @@ import com.datastax.driver.core.TableMetadata; * @see #toString() * @author John McPeek * @author Matthew T. Adams + * @author Mark Paluch */ public final class CqlIdentifier implements Comparable, Serializable { @@ -110,10 +126,10 @@ public final class CqlIdentifier implements Comparable, Serializa */ private void setIdentifier(CharSequence identifier, boolean forceQuoting) { - Assert.notNull(identifier); + Assert.notNull(identifier, "Identifier must not be null"); String string = identifier.toString(); - Assert.hasText(string); + Assert.hasText(string, "Identifier must not be empty"); if (forceQuoting || isQuotedIdentifier(string)) { this.unquoted = string; diff --git a/spring-cql/src/main/java/org/springframework/cassandra/core/cql/generator/AddColumnCqlGenerator.java b/spring-cql/src/main/java/org/springframework/cassandra/core/cql/generator/AddColumnCqlGenerator.java index 2849ab6ad..cf0705956 100644 --- a/spring-cql/src/main/java/org/springframework/cassandra/core/cql/generator/AddColumnCqlGenerator.java +++ b/spring-cql/src/main/java/org/springframework/cassandra/core/cql/generator/AddColumnCqlGenerator.java @@ -23,6 +23,7 @@ import org.springframework.cassandra.core.keyspace.AddColumnSpecification; * CQL generator for generating an ADD clause of an ALTER TABLE statement. * * @author Matthew T. Adams + * @author Mark Paluch */ public class AddColumnCqlGenerator extends ColumnChangeCqlGenerator { @@ -32,6 +33,6 @@ public class AddColumnCqlGenerator extends ColumnChangeCqlGenerator);"); + + AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation", + DataType.list(DataType.varchar())); + + execute(spec); + + ColumnMetadata column = getTableMetadata("addamsFamily").getColumn("lastKnownLocation"); + assertThat(column.getType(), is(equalTo((DataType) DataType.list(DataType.varchar())))); + } + + /** + * @see DATACASS-192 + */ + @Test + public void alterTableAddColumn() { + + session.execute("CREATE TABLE addamsFamily (name varchar PRIMARY KEY, gender varchar,\n" + + " lastknownlocation varchar);"); + + AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").add("gravesite", + DataType.varchar()); + + execute(spec); + + ColumnMetadata column = getTableMetadata("addamsFamily").getColumn("gravesite"); + assertThat(column.getType(), is(equalTo(DataType.varchar()))); + } + + /** + * @see DATACASS-192 + */ + @Test + public void alterTableAddListColumn() { + + session.execute("CREATE TABLE users (user_name varchar PRIMARY KEY);"); + + AlterTableSpecification spec = AlterTableSpecification.alterTable("users").add("top_places", + DataType.list(DataType.ascii())); + + execute(spec); + + ColumnMetadata column = getTableMetadata("users").getColumn("top_places"); + assertThat(column.getType(), is(equalTo((DataType) DataType.list(DataType.ascii())))); + } + + private void execute(AlterTableSpecification spec) { + session.execute(new AlterTableCqlGenerator(spec).toCql()); + } + + private TableMetadata getTableMetadata(String table) { + + KeyspaceMetadata keyspace = session.getCluster().getMetadata().getKeyspace(session.getLoggedKeyspace()); + return keyspace.getTable(table); + } +} diff --git a/spring-cql/src/test/java/org/springframework/cassandra/core/cql/generator/AlterTableCqlGeneratorUnitTests.java b/spring-cql/src/test/java/org/springframework/cassandra/core/cql/generator/AlterTableCqlGeneratorUnitTests.java index e4be74c06..bc63c710f 100644 --- a/spring-cql/src/test/java/org/springframework/cassandra/core/cql/generator/AlterTableCqlGeneratorUnitTests.java +++ b/spring-cql/src/test/java/org/springframework/cassandra/core/cql/generator/AlterTableCqlGeneratorUnitTests.java @@ -15,21 +15,11 @@ */ package org.springframework.cassandra.core.cql.generator; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; -import java.util.LinkedHashMap; -import java.util.Map; - import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.cassandra.core.keyspace.AlterTableSpecification; -import org.springframework.cassandra.core.keyspace.Option; -import org.springframework.cassandra.core.keyspace.TableOption; -import org.springframework.cassandra.core.keyspace.TableOption.CachingOption; -import org.springframework.cassandra.core.keyspace.TableOption.CompactionOption; -import org.springframework.cassandra.core.keyspace.TableOption.CompressionOption; -import org.springframework.cassandra.core.keyspace.TableOption.KeyCachingOption; import com.datastax.driver.core.DataType; @@ -38,139 +28,59 @@ import com.datastax.driver.core.DataType; * * @author Matthew T. Adams * @author David Webb + * @author Mark Paluch */ public class AlterTableCqlGeneratorUnitTests { - private final static Logger log = LoggerFactory.getLogger(AlterTableCqlGeneratorUnitTests.class); - /** - * Asserts that the preamble is first & correctly formatted in the given CQL string. + * @see DATACASS-192 */ - public static void assertPreamble(String tableName, String cql) { - assertTrue(cql.startsWith("ALTER TABLE " + tableName + " ")); + @Test + public void alterTableAlterColumnType() { + + AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation", + DataType.uuid()); + + assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ALTER lastknownlocation TYPE uuid;"))); } /** - * Asserts that the given list of columns definitions are contained in the given CQL string properly. - * - * @param columnSpec IE, "foo text, bar blob" + * @see DATACASS-192 */ - public static void assertColumnChanges(String columnSpec, String cql) { - assertTrue(cql.contains("")); + @Test + public void alterTableAlterListColumnType() { + + AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation", + DataType.list(DataType.ascii())); + + assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ALTER lastknownlocation TYPE list;"))); } /** - * Convenient base class that other test classes can use so as not to repeat the generics declarations. + * @see DATACASS-192 */ - public static abstract class AlterTableTest - extends AbstractTableOperationCqlGeneratorTest {} + @Test + public void alterTableAddColumn() { - public static class BasicTest extends AlterTableTest { + AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").add("gravesite", + DataType.varchar()); - public String name = "mytable"; - public DataType alteredType = DataType.text(); - public String altered = "altered"; - - public DataType addedType = DataType.text(); - public String added = "added"; - - public String dropped = "dropped"; - - @Override - public AlterTableSpecification specification() { - return AlterTableSpecification.alterTable().name(name).alter(altered, alteredType).add(added, addedType); - } - - @Override - public AlterTableCqlGenerator generator() { - return new AlterTableCqlGenerator(specification); - } - - @Test - public void test() { - prepare(); - - assertPreamble(name, cql); - assertColumnChanges( - String.format("ALTER %s TYPE %s, ADD %s %s, DROP %s", altered, alteredType, added, addedType, dropped), cql); - } + assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ADD gravesite varchar;"))); } /** - * Fully test all available create table options - * - * @author David Webb + * @see DATACASS-192 */ - public static class MultipleOptionsTest extends AlterTableTest { + @Test + public void alterTableAddListColumn() { - public String name = "timeseries_table"; - public DataType partitionKeyType0 = DataType.timeuuid(); - public String partitionKey0 = "tid"; - public DataType partitionKeyType1 = DataType.timestamp(); - public String partitionKey1 = "create_timestamp"; - public DataType columnType1 = DataType.text(); - public String column1 = "data_point"; - public Double readRepairChance = 0.6; - public Double dcLocalReadRepairChance = 0.8; - public Double bloomFilterFpChance = 0.002; - public Boolean replcateOnWrite = Boolean.FALSE; - public Long gcGraceSeconds = 1200l; - public String comment = "This is My Table"; - public Map compactionMap = new LinkedHashMap(); - public Map compressionMap = new LinkedHashMap(); - public Map cachingMap = new LinkedHashMap(); + AlterTableSpecification spec = AlterTableSpecification.alterTable("users").add("top_places", + DataType.list(DataType.ascii())); - @Override - public AlterTableSpecification specification() { + assertThat(toCql(spec), is(equalTo("ALTER TABLE users ADD top_places list;"))); + } - // Compaction - compactionMap.put(CompactionOption.CLASS, "SizeTieredCompactionStrategy"); - compactionMap.put(CompactionOption.MIN_THRESHOLD, "4"); - // Compression - compressionMap.put(CompressionOption.SSTABLE_COMPRESSION, "SnappyCompressor"); - compressionMap.put(CompressionOption.CHUNK_LENGTH_KB, 128); - compressionMap.put(CompressionOption.CRC_CHECK_CHANCE, 0.75); - // Caching - cachingMap.put(CachingOption.KEYS, KeyCachingOption.ALL); - cachingMap.put(CachingOption.ROWS_PER_PARTITION, "10"); - - return AlterTableSpecification.alterTable().name(name) - // .with(TableOption.COMPACT_STORAGE) - .with(TableOption.READ_REPAIR_CHANCE, readRepairChance).with(TableOption.COMPACTION, compactionMap) - .with(TableOption.COMPRESSION, compressionMap).with(TableOption.BLOOM_FILTER_FP_CHANCE, bloomFilterFpChance) - .with(TableOption.CACHING, cachingMap).with(TableOption.COMMENT, comment) - .with(TableOption.DCLOCAL_READ_REPAIR_CHANCE, dcLocalReadRepairChance) - .with(TableOption.GC_GRACE_SECONDS, gcGraceSeconds); - } - - @Test - public void test() { - - prepare(); - - log.info(cql); - - assertPreamble(name, cql); - // assertColumns(String.format("%s %s, %s %s, %s %s", partitionKey0, partitionKeyType0, partitionKey1, - // partitionKeyType1, column1, columnType1), cql); - // assertPrimaryKey(String.format("(%s, %s)", partitionKey0, partitionKey1), cql); - // assertNullOption(TableOption.COMPACT_STORAGE.getName(), cql); - // assertDoubleOption(TableOption.READ_REPAIR_CHANCE.getName(), readRepairChance, cql); - // assertDoubleOption(TableOption.DCLOCAL_READ_REPAIR_CHANCE.getName(), dcLocalReadRepairChance, cql); - // assertDoubleOption(TableOption.BLOOM_FILTER_FP_CHANCE.getName(), bloomFilterFpChance, cql); - // assertStringOption(TableOption.CACHING.getName(), CachingOption.KEYS_ONLY.getValue(), cql); - // assertStringOption(TableOption.REPLICATE_ON_WRITE.getName(), replcateOnWrite.toString(), cql); - // assertStringOption(TableOption.COMMENT.getName(), comment, cql); - // assertLongOption(TableOption.GC_GRACE_SECONDS.getName(), gcGraceSeconds, cql); - - } - - /* (non-Javadoc) - * @see org.springframework.cassandra.test.unit.core.cql.generator.TableOperationCqlGeneratorTest#generator() - */ - @Override - public AlterTableCqlGenerator generator() { - return new AlterTableCqlGenerator(specification); - } + private String toCql(AlterTableSpecification spec) { + return new AlterTableCqlGenerator(spec).toCql(); } } diff --git a/spring-cql/src/test/java/org/springframework/cassandra/test/integration/core/cql/generator/TableLifecycleIntegrationTests.java b/spring-cql/src/test/java/org/springframework/cassandra/test/integration/core/cql/generator/TableLifecycleIntegrationTests.java index 4ea472c1d..1c69100c1 100644 --- a/spring-cql/src/test/java/org/springframework/cassandra/test/integration/core/cql/generator/TableLifecycleIntegrationTests.java +++ b/spring-cql/src/test/java/org/springframework/cassandra/test/integration/core/cql/generator/TableLifecycleIntegrationTests.java @@ -67,36 +67,6 @@ public class TableLifecycleIntegrationTests extends AbstractKeyspaceCreatingInte assertNoTable(dropTest.specification, keyspace, session); } - @Test - public void testAlter() { - - createTableTest.prepare(); - - log.info(createTableTest.cql); - - session.execute(createTableTest.cql); - - assertTable(createTableTest.specification, keyspace, session); - - AlterTableCqlGeneratorUnitTests.MultipleOptionsTest alterTest = new AlterTableCqlGeneratorUnitTests.MultipleOptionsTest(); - alterTest.prepare(); - - log.info(alterTest.cql); - - session.execute(alterTest.cql); - - // assertTable(alterTest.specification, keyspace, session); - - DropTableTest dropTest = new DropTableTest(); - dropTest.prepare(); - - log.info(dropTest.cql); - - session.execute(dropTest.cql); - - assertNoTable(dropTest.specification, keyspace, session); - - } public class DropTableTest extends DropTableCqlGeneratorUnitTests.DropTableTest {