CRUNCH-684: Fix .equals and .hashCode for Targets
Signed-off-by: Josh Wills <jwills@apache.org>
diff --git a/crunch-core/src/main/java/org/apache/crunch/io/impl/FileTargetImpl.java b/crunch-core/src/main/java/org/apache/crunch/io/impl/FileTargetImpl.java
index 9c9aaef..d48ac31 100644
--- a/crunch-core/src/main/java/org/apache/crunch/io/impl/FileTargetImpl.java
+++ b/crunch-core/src/main/java/org/apache/crunch/io/impl/FileTargetImpl.java
@@ -23,6 +23,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
@@ -361,12 +362,12 @@
return false;
}
FileTargetImpl o = (FileTargetImpl) other;
- return path.equals(o.path);
+ return Objects.equals(path, o.path) && Objects.equals(formatBundle, o.formatBundle);
}
@Override
public int hashCode() {
- return new HashCodeBuilder().append(path).toHashCode();
+ return new HashCodeBuilder().append(path).append(formatBundle).toHashCode();
}
@Override
diff --git a/crunch-core/src/test/java/org/apache/crunch/io/impl/FileTargetImplTest.java b/crunch-core/src/test/java/org/apache/crunch/io/impl/FileTargetImplTest.java
index 981e20a..3a44703 100644
--- a/crunch-core/src/test/java/org/apache/crunch/io/impl/FileTargetImplTest.java
+++ b/crunch-core/src/test/java/org/apache/crunch/io/impl/FileTargetImplTest.java
@@ -19,8 +19,10 @@
import org.apache.commons.io.FileUtils;
+import org.apache.crunch.Target;
import org.apache.crunch.io.SequentialFileNamingScheme;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.mapreduce.lib.output.SequenceFileOutputFormat;
import org.junit.Rule;
@@ -28,6 +30,7 @@
import org.junit.rules.TemporaryFolder;
import java.io.File;
+import java.net.URI;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
@@ -36,9 +39,12 @@
import java.util.Set;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.matchers.JUnitMatchers.hasItem;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
public class FileTargetImplTest {
@@ -77,4 +83,112 @@
assertThat(fileContents, hasItem(content));
}
}
+
+ @Test
+ public void testEquality() {
+ Target target = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance());
+ Target target2 = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance());
+
+ assertEquals(target, target2);
+ assertEquals(target.hashCode(), target2.hashCode());
+ }
+
+ @Test
+ public void testEqualityWithExtraConf() {
+ Target target = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).outputConf("key", "value");
+ Target target2 = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).outputConf("key", "value");
+
+ assertEquals(target, target2);
+ assertEquals(target.hashCode(), target2.hashCode());
+ }
+
+ @Test
+ public void testEqualityWithFileSystem() {
+ Path path = new Path("/path");
+ Path qualifiedPath = path.makeQualified(URI.create("scheme://cluster"), new Path("/"));
+ FileSystem fs = mock(FileSystem.class);
+ when(fs.makeQualified(path)).thenReturn(qualifiedPath);
+ Configuration conf = new Configuration(false);
+ conf.set("key", "value");
+ when(fs.getConf()).thenReturn(conf);
+
+ Target target = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs);
+ Target target2 = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs);
+
+ assertEquals(target, target2);
+ assertEquals(target.hashCode(), target2.hashCode());
+ }
+
+ @Test
+ public void testInequality() {
+ Target target = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance());
+ Target target2 = new FileTargetImpl(new Path("/path2"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance());
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
+
+ @Test
+ public void testInequalityWithExtraConf() {
+ Target target = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).outputConf("key", "value");
+ Target target2 = new FileTargetImpl(new Path("/path"), SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).outputConf("key", "value2");
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
+
+ @Test
+ public void testInequalityWithFileSystemURI() {
+ Path path = new Path("/path");
+ Path qualifiedPath = path.makeQualified(URI.create("scheme://cluster"), new Path("/"));
+ Path qualifiedPath2 = path.makeQualified(URI.create("scheme://cluster2"), new Path("/"));
+ FileSystem fs = mock(FileSystem.class);
+ FileSystem fs2 = mock(FileSystem.class);
+ when(fs.makeQualified(path)).thenReturn(qualifiedPath);
+ when(fs2.makeQualified(path)).thenReturn(qualifiedPath2);
+ when(fs.getConf()).thenReturn(new Configuration(false));
+ when(fs2.getConf()).thenReturn(new Configuration(false));
+
+ Target target = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs);
+ Target target2 = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs2);
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
+
+ @Test
+ public void testInequalityWithFileSystemConf() {
+ Path path = new Path("/path");
+ Path qualifiedPath = path.makeQualified(URI.create("scheme://cluster"), new Path("/"));
+ FileSystem fs = mock(FileSystem.class);
+ FileSystem fs2 = mock(FileSystem.class);
+ when(fs.makeQualified(path)).thenReturn(qualifiedPath);
+ when(fs2.makeQualified(path)).thenReturn(qualifiedPath);
+ Configuration conf = new Configuration(false);
+ conf.set("key", "value");
+ Configuration conf2 = new Configuration(false);
+ conf2.set("key", "value2");
+ when(fs.getConf()).thenReturn(conf);
+ when(fs2.getConf()).thenReturn(conf2);
+
+ Target target = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs);
+ Target target2 = new FileTargetImpl(path, SequenceFileOutputFormat.class,
+ SequentialFileNamingScheme.getInstance()).fileSystem(fs2);
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
}
\ No newline at end of file
diff --git a/crunch-hbase/src/main/java/org/apache/crunch/io/hbase/HBaseTarget.java b/crunch-hbase/src/main/java/org/apache/crunch/io/hbase/HBaseTarget.java
index d51dee0..87ef45c 100644
--- a/crunch-hbase/src/main/java/org/apache/crunch/io/hbase/HBaseTarget.java
+++ b/crunch-hbase/src/main/java/org/apache/crunch/io/hbase/HBaseTarget.java
@@ -19,6 +19,7 @@
import java.io.IOException;
import java.util.Map;
+import java.util.Objects;
import com.google.common.collect.Maps;
import org.apache.commons.lang.builder.HashCodeBuilder;
@@ -75,13 +76,13 @@
if (!other.getClass().equals(getClass()))
return false;
HBaseTarget o = (HBaseTarget) other;
- return table.equals(o.table);
+ return Objects.equals(table, o.table) && Objects.equals(extraConf, o.extraConf);
}
@Override
public int hashCode() {
HashCodeBuilder hcb = new HashCodeBuilder();
- return hcb.append(table).toHashCode();
+ return hcb.append(table).append(extraConf).toHashCode();
}
@Override
diff --git a/crunch-hbase/src/test/java/org/apache/crunch/io/hbase/HBaseTargetTest.java b/crunch-hbase/src/test/java/org/apache/crunch/io/hbase/HBaseTargetTest.java
index 8faa27d..728732c 100644
--- a/crunch-hbase/src/test/java/org/apache/crunch/io/hbase/HBaseTargetTest.java
+++ b/crunch-hbase/src/test/java/org/apache/crunch/io/hbase/HBaseTargetTest.java
@@ -17,9 +17,14 @@
*/
package org.apache.crunch.io.hbase;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import java.io.IOException;
+
+import org.apache.crunch.Target;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.mapreduce.Job;
import org.junit.Test;
@@ -38,4 +43,40 @@
assertEquals("12345", job.getConfiguration().get("hbase.client.scanner.timeout.period"));
}
+
+ @Test
+ public void testEquality() {
+ Target target = new HBaseTarget("testTable");
+ Target target2 = new HBaseTarget("testTable");
+
+ assertEquals(target, target2);
+ assertEquals(target.hashCode(), target2.hashCode());
+ }
+
+ @Test
+ public void testEqualityWithExtraConf() {
+ Target target = new HBaseTarget("testTable").outputConf("key", "value");
+ Target target2 = new HBaseTarget("testTable").outputConf("key", "value");
+
+ assertEquals(target, target2);
+ assertEquals(target.hashCode(), target2.hashCode());
+ }
+
+ @Test
+ public void testInequality() {
+ Target target = new HBaseTarget("testTable");
+ Target target2 = new HBaseTarget("testTable2");
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
+
+ @Test
+ public void testInequalityWithExtraConf() {
+ Target target = new HBaseTarget("testTable").outputConf("key", "value");
+ Target target2 = new HBaseTarget("testTable").outputConf("key", "value2");
+
+ assertThat(target, is(not(target2)));
+ assertThat(target.hashCode(), is(not(target2.hashCode())));
+ }
}