You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@crunch.apache.org by jw...@apache.org on 2019/05/02 19:45:14 UTC

[crunch] branch master updated (a6dfa38 -> 7c49b8b)

This is an automated email from the ASF dual-hosted git repository.

jwills pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/crunch.git.


    from a6dfa38  CRUNCH-681: Add and update javadoc
     new 869aac6  CRUNCH-684: Fix .equals and .hashCode for Targets
     new 7c49b8b  CRUNCH-684: Fix NullPointerException

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/crunch/io/FormatBundle.java    |   3 +-
 .../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 ++++++++
 5 files changed, 163 insertions(+), 5 deletions(-)


[crunch] 01/02: CRUNCH-684: Fix .equals and .hashCode for Targets

Posted by jw...@apache.org.
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 <ao...@cerner.com>
AuthorDate: Wed May 1 16:20:17 2019 -0500

    CRUNCH-684: Fix .equals and .hashCode for Targets
    
    Signed-off-by: Josh Wills <jw...@apache.org>
---
 .../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())));
+  }
 }


[crunch] 02/02: CRUNCH-684: Fix NullPointerException

Posted by jw...@apache.org.
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 7c49b8b9f8f85782370e158fc681904c5c34647e
Author: Andrew Olson <ao...@cerner.com>
AuthorDate: Thu May 2 07:36:34 2019 -0500

    CRUNCH-684: Fix NullPointerException
    
    Signed-off-by: Josh Wills <jw...@apache.org>
---
 crunch-core/src/main/java/org/apache/crunch/io/FormatBundle.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crunch-core/src/main/java/org/apache/crunch/io/FormatBundle.java b/crunch-core/src/main/java/org/apache/crunch/io/FormatBundle.java
index bbe7f5c..0b50080 100644
--- a/crunch-core/src/main/java/org/apache/crunch/io/FormatBundle.java
+++ b/crunch-core/src/main/java/org/apache/crunch/io/FormatBundle.java
@@ -31,6 +31,7 @@ import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Map;
 
+import java.util.Objects;
 import java.util.Set;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.builder.HashCodeBuilder;
@@ -147,7 +148,7 @@ public class FormatBundle<K> implements Serializable, Writable, Configurable {
       return false;
     }
     FormatBundle<K> oib = (FormatBundle<K>) other;
-    return formatClass.equals(oib.formatClass) && extraConf.equals(oib.extraConf);
+    return Objects.equals(formatClass, oib.formatClass) && Objects.equals(extraConf, oib.extraConf);
   }
 
   @Override