Repository: incubator-beam Updated Branches: refs/heads/master 9b47228c3 -> 3e84a5f3c
[BEAM-929] Fix Findbugs issues in Kinesis * Fix equals and hashcode * Add tests * Remove Serializable from KinesisRecord, as it is in fact coded not serialized Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/367fcac6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/367fcac6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/367fcac6 Branch: refs/heads/master Commit: 367fcac68b38cc613525815181707a071eb8a51c Parents: 9b47228 Author: Dan Halperin <[email protected]> Authored: Mon Nov 7 12:31:19 2016 -0800 Committer: Dan Halperin <[email protected]> Committed: Mon Nov 7 13:33:04 2016 -0800 ---------------------------------------------------------------------- sdks/java/io/kinesis/pom.xml | 21 ++++++-------------- .../beam/sdk/io/kinesis/CustomOptional.java | 21 ++++++++++++++++---- .../beam/sdk/io/kinesis/KinesisRecord.java | 3 +-- .../beam/sdk/io/kinesis/CustomOptionalTest.java | 10 ++++++++++ 4 files changed, 34 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/367fcac6/sdks/java/io/kinesis/pom.xml ---------------------------------------------------------------------- diff --git a/sdks/java/io/kinesis/pom.xml b/sdks/java/io/kinesis/pom.xml index e0b57db..36c7039 100644 --- a/sdks/java/io/kinesis/pom.xml +++ b/sdks/java/io/kinesis/pom.xml @@ -30,19 +30,6 @@ <description>Library to read Kinesis streams.</description> <build> - <pluginManagement> - <plugins> - <!-- BEAM-929 --> - <plugin> - <groupId>org.codehaus.mojo</groupId> - <artifactId>findbugs-maven-plugin</artifactId> - <configuration> - <skip>true</skip> - </configuration> - </plugin> - </plugins> - </pluginManagement> - <plugins> <plugin> <groupId>org.apache.maven.plugins</groupId> @@ -139,7 +126,6 @@ <artifactId>annotations</artifactId> </dependency> - <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> @@ -160,6 +146,12 @@ </dependency> <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava-testlib</artifactId> + <scope>test</scope> + </dependency> + + <dependency> <groupId>org.hamcrest</groupId> <artifactId>hamcrest-all</artifactId> <scope>test</scope> @@ -171,6 +163,5 @@ <version>${project.version}</version> <scope>test</scope> </dependency> - </dependencies> </project> http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/367fcac6/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/CustomOptional.java ---------------------------------------------------------------------- diff --git a/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/CustomOptional.java b/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/CustomOptional.java index 4317a59..4bed0e3 100644 --- a/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/CustomOptional.java +++ b/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/CustomOptional.java @@ -18,6 +18,7 @@ package org.apache.beam.sdk.io.kinesis; import java.util.NoSuchElementException; +import java.util.Objects; /** * Similar to Guava {@code Optional}, but throws {@link NoSuchElementException} for missing element. @@ -53,17 +54,19 @@ abstract class CustomOptional<T> { return value; } - @Override public boolean equals(Object o) { - Present<?> present = (Present<?>) o; + if (!(o instanceof Present)) { + return false; + } - return value != null ? value.equals(present.value) : present.value == null; + Present<?> present = (Present<?>) o; + return Objects.equals(value, present.value); } @Override public int hashCode() { - return value != null ? value.hashCode() : 0; + return Objects.hash(value); } } @@ -82,5 +85,15 @@ abstract class CustomOptional<T> { public T get() { throw new NoSuchElementException(); } + + @Override + public boolean equals(Object o) { + return o instanceof Absent; + } + + @Override + public int hashCode() { + return 0; + } } } http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/367fcac6/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisRecord.java ---------------------------------------------------------------------- diff --git a/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisRecord.java b/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisRecord.java index fe2a33d..02b5370 100644 --- a/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisRecord.java +++ b/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisRecord.java @@ -22,7 +22,6 @@ import static org.apache.commons.lang.builder.HashCodeBuilder.reflectionHashCode import com.amazonaws.services.kinesis.clientlibrary.types.ExtendedSequenceNumber; import com.amazonaws.services.kinesis.clientlibrary.types.UserRecord; import com.google.common.base.Charsets; -import java.io.Serializable; import java.nio.ByteBuffer; import org.apache.commons.lang.builder.EqualsBuilder; import org.joda.time.Instant; @@ -30,7 +29,7 @@ import org.joda.time.Instant; /** * {@link UserRecord} enhanced with utility methods. */ -public class KinesisRecord implements Serializable { +public class KinesisRecord { private Instant readTime; private String streamName; private String shardId; http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/367fcac6/sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/CustomOptionalTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/CustomOptionalTest.java b/sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/CustomOptionalTest.java index 20e8372..00acffe 100644 --- a/sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/CustomOptionalTest.java +++ b/sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/CustomOptionalTest.java @@ -17,6 +17,7 @@ */ package org.apache.beam.sdk.io.kinesis; +import com.google.common.testing.EqualsTester; import java.util.NoSuchElementException; import org.junit.Test; @@ -28,4 +29,13 @@ public class CustomOptionalTest { public void absentThrowsNoSuchElementExceptionOnGet() { CustomOptional.absent().get(); } + + @Test + public void testEqualsAndHashCode() { + new EqualsTester() + .addEqualityGroup(CustomOptional.absent(), CustomOptional.absent()) + .addEqualityGroup(CustomOptional.of(3), CustomOptional.of(3)) + .addEqualityGroup(CustomOptional.of(11)) + .addEqualityGroup(CustomOptional.of("3")).testEquals(); + } }
