This is an automated email from the ASF dual-hosted git repository. jwills pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/crunch.git
commit 869aac60c9d3b5bef10b4e907ec3840be2d8c20e Author: Andrew Olson <[email protected]> AuthorDate: Wed May 1 16:20:17 2019 -0500 CRUNCH-684: Fix .equals and .hashCode for Targets Signed-off-by: Josh Wills <[email protected]> --- .../org/apache/crunch/io/impl/FileTargetImpl.java | 5 +- .../apache/crunch/io/impl/FileTargetImplTest.java | 114 +++++++++++++++++++++ .../org/apache/crunch/io/hbase/HBaseTarget.java | 5 +- .../apache/crunch/io/hbase/HBaseTargetTest.java | 41 ++++++++ 4 files changed, 161 insertions(+), 4 deletions(-) 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.Iterator; 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 @@ public class FileTargetImpl implements PathTarget { 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 @@ package org.apache.crunch.io.impl; 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.Test; 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.Map; 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 @@ public class FileTargetImplTest { 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 @@ package org.apache.crunch.io.hbase; 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 @@ public class HBaseTarget implements MapReduceTarget { 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 @@ public class HBaseTargetTest { 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()))); + } }
