Skip to content

Commit

Permalink
Remove Guava usages in main source in favor of JDK built-ins (#76)
Browse files Browse the repository at this point in the history
* Fix documentation methods called on `Duration`

* Use newer support for Guava `CharMatcher`s since the constants were removed in Guava 26.0 and the preferred access pattern is the methods that were added in Guava 19.0

This makes the minimum version of the Cassandra Java Driver go up and supports Guava 19+.
Ideally, these methods should be updated to not require use of `CharMatcher` to not couple with the Guava version that the Cassandra driver uses.
Some hackery could be added to check if `CharMatcher.WHITESPACE` exists for Guava 18 and the current Cassandra version and then use `CharMatcher.whitespace()` on more recent versions, but this would increase complexity.

* Replace all Guava usages with JDK built-ins

Guava used in test library in the same way it exists today.
A test is added to ensure that the hashing output used now is the same as what the Guava hashing output would be.

* Simplify `bytesToHex` implementation
  • Loading branch information
mkobit authored and lw346 committed Apr 5, 2019
1 parent 95d2450 commit 6b3fa2e
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 56 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ import com.datastax.driver.core.*;

// Configure locking for coordination of multiple nodes
CassandraLockConfig lockConfig = CassandraLockConfig.builder()
.withTimeout(Duration.standardSeconds(3))
.withPollingInterval(Duration.millis(500))
.withTimeout(Duration.ofSeconds(3))
.withPollingInterval(Duration.ofMillis(500))
.build();

// Create a Cassandra session
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ dependencies {
api('com.datastax.cassandra:cassandra-driver-core:3.7.1')
implementation('org.slf4j:slf4j-api:1.7.25')

testImplementation("com.google.guava:guava:25.1-jre")
testImplementation('org.apache.logging.log4j:log4j-core:2.11.2')
testImplementation('junit:junit:4.12')
testImplementation('org.assertj:assertj-core:3.12.2')
Expand Down
36 changes: 22 additions & 14 deletions src/main/java/uk/sky/cqlmigrate/CqlFileParser.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package uk.sky.cqlmigrate;

import com.google.common.base.CharMatcher;
import com.google.common.base.Throwables;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static com.google.common.base.Preconditions.checkState;

class CqlFileParser {
private static final Logger LOGGER = LoggerFactory.getLogger(CqlFileParser.class);
private static final Pattern EOL = Pattern.compile(".*\\R|.+\\z");
Expand All @@ -24,14 +21,14 @@ private CqlFileParser() {}
static List<String> getCqlStatementsFrom(Path cqlPath) {
LineProcessor processor = new LineProcessor();

try (Scanner in = new Scanner(cqlPath, "UTF-8")) {
try (Scanner in = new Scanner(cqlPath, StandardCharsets.UTF_8.name())) {
String original;
while ((original = in.findWithinHorizon(EOL, 0)) != null) {
processor.process(original);
}
} catch (IOException | IllegalStateException e ) {
LOGGER.error("Failed to process cql script {}: {}", cqlPath.getFileName(), e.getMessage());
throw Throwables.propagate(e);
throw new RuntimeException(e);
}

processor.check();
Expand All @@ -55,7 +52,7 @@ private enum State {
IS_MULTI_LINE_COMMENT,
IS_OPEN_STMT,
IS_OPEN_VALUE_EXP,
IS_CLOSE_STMT;
IS_CLOSE_STMT
}

private final List<String> statements = new ArrayList<>();
Expand Down Expand Up @@ -99,7 +96,7 @@ private void init(String original) throws IOException {
}

private void findStatement(String original) throws IOException {
String line = CharMatcher.WHITESPACE.trimFrom(original);
String line = original.trim();

if (line.startsWith(CQL_COMMENT_DOUBLE_HYPEN) || line.startsWith(CQL_COMMENT_DOUBLE_SLASH) || line.isEmpty()) {
return;
Expand All @@ -112,17 +109,17 @@ private void findStatement(String original) throws IOException {

if (line.endsWith(CQL_STATEMENT_TERMINATOR)) {
curStmt.append(" ").append(line, 0, line.length() - 1);
statements.add(CharMatcher.WHITESPACE.trimFrom(curStmt.toString()));
statements.add(curStmt.toString().trim());
curState = State.IS_CLOSE_STMT;
process(original);
return;
}

// A semicolon preceded by an odd number of single quotes must be within a string,
// and therefore is not a statement terminator
if (CharMatcher.is(CQL_STATEMENT_STRING_DELIMITER).countIn(line) % 2 != 0) {
if (line.chars().filter(character -> character == CQL_STATEMENT_STRING_DELIMITER).count() % 2 != 0) {
curState = State.IS_OPEN_VALUE_EXP;
curStmt.append(" ").append(CharMatcher.WHITESPACE.trimLeadingFrom(original));
curStmt.append(" ").append(trimLeadingWhitespace(original));
return;
}

Expand All @@ -146,8 +143,16 @@ private void findStatement(String original) throws IOException {
}
}

private static String trimLeadingWhitespace(String original) {
return original.replaceAll("^\\s+", "");
}

private static String trimTrailingWhitespace(String original) {
return original.replaceAll("\\s+$", "");
}

private void findValueExpression(String original) {
if (CharMatcher.is(CQL_STATEMENT_STRING_DELIMITER).countIn(original) % 2 != 0) {
if (original.chars().filter(character -> character == CQL_STATEMENT_STRING_DELIMITER).count() % 2 != 0) {
curStmt.append(original);
curState = State.FIND_EOS;
return;
Expand All @@ -157,8 +162,9 @@ private void findValueExpression(String original) {
}

private void findMultilineComment(String original) {
if (CharMatcher.WHITESPACE.trimTrailingFrom(original).endsWith(CQL_MULTI_LINE_COMMENT_CLOSE))
if (trimTrailingWhitespace(original).endsWith(CQL_MULTI_LINE_COMMENT_CLOSE)) {
curState = State.FIND_EOS;
}
}

private void closedStatement(String original) {
Expand All @@ -167,7 +173,9 @@ private void closedStatement(String original) {
}

private void check() {
checkState(State.IS_CLOSE_STMT.equals(curState) || State.INIT.equals(curState), "File had a non-terminated cql line");
if (!(State.IS_CLOSE_STMT.equals(curState) || State.INIT.equals(curState))) {
throw new IllegalStateException("File had a non-terminated cql line");
}
}

List<String> getResult() {
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/uk/sky/cqlmigrate/CqlMigratorConfig.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package uk.sky.cqlmigrate;

import static java.util.Objects.requireNonNull;

import com.datastax.driver.core.ConsistencyLevel;
import static com.google.common.base.Preconditions.*;

public class CqlMigratorConfig {
private final LockConfig cassandraLockConfig;
private final ConsistencyLevel readConsistencyLevel;
private final ConsistencyLevel writeConsistencyLevel;

private CqlMigratorConfig(LockConfig cassandraLockConfig, ConsistencyLevel readConsistencyLevel, ConsistencyLevel writeConsistencyLevel) {
this.cassandraLockConfig = checkNotNull(cassandraLockConfig);
this.readConsistencyLevel = checkNotNull(readConsistencyLevel);
this.writeConsistencyLevel = checkNotNull(writeConsistencyLevel);
this.cassandraLockConfig = requireNonNull(cassandraLockConfig);
this.readConsistencyLevel = requireNonNull(readConsistencyLevel);
this.writeConsistencyLevel = requireNonNull(writeConsistencyLevel);
}

public static CassandraConfigBuilder builder() {
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/uk/sky/cqlmigrate/CqlMigratorImpl.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package uk.sky.cqlmigrate;

import static java.util.Objects.requireNonNull;

import com.datastax.driver.core.Cluster;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.SimpleStatement;
import com.datastax.driver.core.Statement;
import com.google.common.base.Preconditions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -46,9 +47,9 @@ public static void main(String[] args) {
String username = System.getProperty("username");
String password = System.getProperty("password");

Preconditions.checkNotNull(hosts, "'hosts' property should be provided having value of a comma separated list of cassandra hosts");
Preconditions.checkNotNull(keyspace, "'keyspace' property should be provided having value of the cassandra keyspace");
Preconditions.checkNotNull(directoriesProperty, "'directories' property should be provided having value of the comma separated list of paths to cql files");
requireNonNull(hosts, "'hosts' property should be provided having value of a comma separated list of cassandra hosts");
requireNonNull(keyspace, "'keyspace' property should be provided having value of the cassandra keyspace");
requireNonNull(directoriesProperty, "'directories' property should be provided having value of the comma separated list of paths to cql files");

Collection<Path> directories = Arrays.stream(directoriesProperty.split(","))
.map(Paths::get)
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/uk/sky/cqlmigrate/CqlPaths.java
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
package uk.sky.cqlmigrate;

import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSortedMap;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.stream.StreamSupport;

class CqlPaths {

private static final String BOOTSTRAP_CQL = "bootstrap.cql";
private static final String CQL_FILE_FILTER = "*.cql";

private final ImmutableSortedMap<String, Path> sortedCqlPaths;
private final SortedMap<String, Path> sortedCqlPaths;

public CqlPaths(Map<String, Path> paths) {
this.sortedCqlPaths = ImmutableSortedMap.copyOf(paths);

this.sortedCqlPaths = Collections.unmodifiableSortedMap(new TreeMap<>(paths));
}

static CqlPaths create(Collection<Path> directories) {
Expand All @@ -46,7 +46,7 @@ private static DirectoryStream<Path> directoryStreamFromPath(Path path) {
return Files.newDirectoryStream(path, CQL_FILE_FILTER);
}
catch (IOException e) {
throw Throwables.propagate(e);
throw new RuntimeException(e);
}
}

Expand Down
40 changes: 19 additions & 21 deletions src/main/java/uk/sky/cqlmigrate/SchemaUpdates.java
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
package uk.sky.cqlmigrate;

import static java.util.Objects.requireNonNull;

import com.datastax.driver.core.Row;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.SimpleStatement;
import com.datastax.driver.core.Statement;
import com.datastax.driver.core.TableMetadata;
import com.google.common.base.Throwables;
import com.google.common.hash.Hashing;
import com.google.common.io.ByteSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Collections;

import static com.google.common.base.Preconditions.checkNotNull;

class SchemaUpdates {
private static final Logger LOGGER = LoggerFactory.getLogger(SchemaUpdates.class);
private static final String SCHEMA_UPDATES_TABLE = "schema_updates";
Expand Down Expand Up @@ -53,14 +52,14 @@ private Row getSchemaUpdate(Session session, String filename) {
}

boolean contentsAreDifferent(String filename, Path path) {
Row row = checkNotNull(getSchemaUpdate(sessionContext.getSession(), filename));
Row row = requireNonNull(getSchemaUpdate(sessionContext.getSession(), filename));
String previousSha1 = row.getString(CHECKSUM_COLUMN);

try {
String checksum = calculateChecksum(path);
return !previousSha1.equals(checksum);
} catch (Exception e) {
throw Throwables.propagate(e);
throw new RuntimeException(e);
}
}

Expand All @@ -77,22 +76,21 @@ void add(String filename, Path path) {

private String calculateChecksum(Path path) {
try {
return new PathByteSource(path).hash(Hashing.sha1()).toString();
} catch (IOException e) {
throw Throwables.propagate(e);
final MessageDigest digest = MessageDigest.getInstance("SHA-1");
final byte[] hash = digest.digest(Files.readAllBytes(path));
return bytesToHex(hash);
} catch (IOException | NoSuchAlgorithmException e) {
throw new RuntimeException(e);
}
}

private static class PathByteSource extends ByteSource {
private final Path path;

public PathByteSource(Path path) {
this.path = path;
}

@Override
public InputStream openStream() throws IOException {
return java.nio.file.Files.newInputStream(path);
private static String bytesToHex(byte[] bytes) {
final StringBuilder builder = new StringBuilder(2 * bytes.length);
for (byte b : bytes) {
final int asUnsigned = Byte.toUnsignedInt(b);
builder.append(Character.forDigit(asUnsigned >>> 4, 16))
.append(Character.forDigit(asUnsigned & 0x0F, 16));
}
return builder.toString();
}
}
40 changes: 40 additions & 0 deletions src/test/java/uk/sky/cqlmigrate/SchemaUpdatesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@
import com.datastax.driver.core.Cluster;
import com.datastax.driver.core.ConsistencyLevel;
import com.datastax.driver.core.KeyspaceMetadata;
import com.datastax.driver.core.ResultSet;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.exceptions.AlreadyExistsException;
import com.google.common.hash.Hashing;
import com.google.common.io.ByteSource;
import com.google.common.io.Resources;
import com.google.common.util.concurrent.Uninterruptibles;
import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.thrift.transport.TTransportException;
import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
import org.junit.*;

import java.io.IOException;
import java.net.URL;
import java.nio.file.Paths;
import java.util.concurrent.TimeUnit;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -94,4 +100,38 @@ public void schemaUpdatesTableShouldNotBeCreatedIfExists() throws Exception {
KeyspaceMetadata keyspaceMetadata = cluster.getMetadata().getKeyspace(TEST_KEYSPACE);
assertThat(keyspaceMetadata.getTable(SCHEMA_UPDATES_TABLE)).as("table should have been created").isNotNull();
}

/**
* Make sure that the hashes that are calculated now using JDK builtins to what was previously calculated using
* Guava's com.google.common.hash.Hashing library.
* @see Hashing
*/
@Test
public void rowInsertedWithMessageDigestHashingAlgorithmIsSameAsGuavaSha1HashingAlgorithm() throws Exception {
//given
cluster.connect("system").execute("CREATE KEYSPACE " + TEST_KEYSPACE + " WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 1 };");
Session session = cluster.connect(TEST_KEYSPACE);
SessionContext sessionContext = new SessionContext(session, ConsistencyLevel.ALL, ConsistencyLevel.ALL, clusterHealth);
SchemaUpdates schemaUpdates = new SchemaUpdates(sessionContext, TEST_KEYSPACE);
final String filename = "2018-03-26-18:11-create-some-tables.cql";
final URL cqlResource = Resources.getResource("cql_schema_update_hashing/" + filename);
schemaUpdates.initialise();

//when
schemaUpdates.add(
filename,
Paths.get(cqlResource.toURI())
);

//then
final String guavaSha1Hash = Resources.asByteSource(cqlResource).hash(Hashing.sha1()).toString();
final ResultSet resultSet = session.execute("SELECT * from " + SCHEMA_UPDATES_TABLE);
assertThat(resultSet.all())
.hasOnlyOneElementSatisfying(row -> {
assertThat(row.getString("filename"))
.isEqualTo(filename);
assertThat(row.getString("checksum"))
.isEqualTo(guavaSha1Hash);
});
}
}
Loading

0 comments on commit 6b3fa2e

Please sign in to comment.