Repository: redpen-cc/redpen
Pull request #549 (2016-02-16 10:45:32)

Implement consistent clone(), equals(), hashCode() and Serializable for all config classes


Patch:
diff --git a/redpen-core/src/main/java/cc/redpen/config/Configuration.java b/redpen-core/src/main/java/cc/redpen/config/Configuration.java
index e6af6b43..4f8cd6ec 100644
--- a/redpen-core/src/main/java/cc/redpen/config/Configuration.java
+++ b/redpen-core/src/main/java/cc/redpen/config/Configuration.java
@@ -21,8 +21,12 @@ import cc.redpen.tokenizer.JapaneseTokenizer;
 import cc.redpen.tokenizer.RedPenTokenizer;
 import cc.redpen.tokenizer.WhiteSpaceTokenizer;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 
 import static java.util.stream.Collectors.toList;
@@ -31,11 +35,11 @@ import static org.apache.commons.lang3.StringUtils.isEmpty;
 /**
  * Contains Settings used throughout {@link cc.redpen.RedPen}.
  */
-public class Configuration implements Cloneable {
+public class Configuration implements Serializable, Cloneable {
     private SymbolTable symbolTable;
     private List<ValidatorConfiguration> validatorConfigs = new ArrayList<>();
-    private String lang;
-    private RedPenTokenizer tokenizer;
+    private final String lang;
+    private transient RedPenTokenizer tokenizer;
 
     /**
      * Constructor.
@@ -45,13 +49,11 @@ public class Configuration implements Cloneable {
 
         this.validatorConfigs.addAll(validatorConfigs);
         this.lang = lang;
-        switch (lang) {
-            case "ja":
-                this.tokenizer = new JapaneseTokenizer();
-                break;
-            default:
-                this.tokenizer = new WhiteSpaceTokenizer();
-        }
+        initTokenizer();
+    }
+
+    private void initTokenizer() {
+        this.tokenizer = lang.equals("ja") ? new JapaneseTokenizer() : new WhiteSpaceTokenizer();
     }
 
     /**
@@ -123,6 +125,24 @@ public class Configuration implements Cloneable {
         }
     }
 
+    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+        in.defaultReadObject();
+        initTokenizer();
+    }
+
+    @Override public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof Configuration)) return false;
+        Configuration that = (Configuration)o;
+        return Objects.equals(lang, that.lang) &&
+          Objects.equals(symbolTable, that.symbolTable) &&
+          Objects.equals(validatorConfigs, that.validatorConfigs);
+    }
+
+    @Override public int hashCode() {
+        return getKey().hashCode();
+    }
+
     /**
      * Builder class of Configuration.
      */
diff --git a/redpen-core/src/main/java/cc/redpen/config/Symbol.java b/redpen-core/src/main/java/cc/redpen/config/Symbol.java
index 614fb825..f992dbcd 100644
--- a/redpen-core/src/main/java/cc/redpen/config/Symbol.java
+++ b/redpen-core/src/main/java/cc/redpen/config/Symbol.java
@@ -19,6 +19,7 @@ package cc.redpen.config;
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * Represent a character settings.
@@ -120,25 +121,17 @@ public class Symbol implements Serializable {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
 
-        Symbol symbol = (Symbol) o;
-
-        if (needAfterSpace != symbol.needAfterSpace) return false;
-        if (needBeforeSpace != symbol.needBeforeSpace) return false;
-        if (value != symbol.value) return false;
-        if (!Arrays.equals(invalidChars, symbol.invalidChars)) return false;
-        if (name != symbol.name) return false;
-
-        return true;
+        Symbol that = (Symbol) o;
+        return Objects.equals(name, that.name) &&
+               Objects.equals(value, that.value) &&
+               Arrays.equals(invalidChars, that.invalidChars) &&
+               Objects.equals(needBeforeSpace, that.needBeforeSpace) &&
+               Objects.equals(needAfterSpace, that.needAfterSpace);
     }
 
     @Override
     public int hashCode() {
-        int result = name != null ? name.hashCode() : 0;
-        result = 31 * result + (int) value;
-        result = 31 * result + (invalidChars != null ? Arrays.hashCode(invalidChars) : 0);
-        result = 31 * result + (needBeforeSpace ? 1 : 0);
-        result = 31 * result + (needAfterSpace ? 1 : 0);
-        return result;
+        return Objects.hashCode(name);
     }
 
     @Override
diff --git a/redpen-core/src/main/java/cc/redpen/config/SymbolTable.java b/redpen-core/src/main/java/cc/redpen/config/SymbolTable.java
index ec6e682e..5a4c0fe6 100644
--- a/redpen-core/src/main/java/cc/redpen/config/SymbolTable.java
+++ b/redpen-core/src/main/java/cc/redpen/config/SymbolTable.java
@@ -31,8 +31,8 @@ import static cc.redpen.config.SymbolType.*;
  */
 public class SymbolTable implements Serializable, Cloneable {
     private static final long serialVersionUID = 1612920745151501631L;
-    private final Map<SymbolType, Symbol> symbolDictionary = new LinkedHashMap<>();
-    private final Map<Character, Symbol> valueDictionary = new LinkedHashMap<>();
+    private Map<SymbolType, Symbol> symbolDictionary = new LinkedHashMap<>();
+    private Map<Character, Symbol> valueDictionary = new LinkedHashMap<>();
     private String variant;
     private String lang;
     private static final Logger LOG = LoggerFactory.getLogger(SymbolTable.class);
@@ -144,29 +144,23 @@ public class SymbolTable implements Serializable, Cloneable {
         if (o == null || getClass() != o.getClass()) return false;
 
         SymbolTable that = (SymbolTable) o;
-
-        if (lang != null ? !lang.equals(that.lang) : that.lang != null) return false;
-        if (symbolDictionary != null ? !symbolDictionary.equals(that.symbolDictionary) : that.symbolDictionary != null)
-            return false;
-        if (variant != null ? !variant.equals(that.variant) : that.variant != null) return false;
-        if (valueDictionary != null ? !valueDictionary.equals(that.valueDictionary) : that.valueDictionary != null)
-            return false;
-
-        return true;
+        return Objects.equals(lang, that.lang) &&
+               Objects.equals(variant, that.variant) &&
+               Objects.equals(symbolDictionary, that.symbolDictionary);
     }
 
     @Override
     public int hashCode() {
-        int result = symbolDictionary != null ? symbolDictionary.hashCode() : 0;
-        result = 31 * result + (valueDictionary != null ? valueDictionary.hashCode() : 0);
-        result = 31 * result + (variant != null ? variant.hashCode() : 0);
-        result = 31 * result + (lang != null ? lang.hashCode() : 0);
-        return result;
+        return Objects.hash(lang, variant, symbolDictionary);
     }
 
-    @Override public SymbolTable clone() {
+    @Override
+    public SymbolTable clone() {
         try {
-            return (SymbolTable)super.clone();
+            SymbolTable clone = (SymbolTable)super.clone();
+            clone.symbolDictionary = new LinkedHashMap<>(symbolDictionary);
+            clone.valueDictionary = new LinkedHashMap<>(valueDictionary);
+            return clone;
         }
         catch (CloneNotSupportedException e) {
             throw new RuntimeException(e);
diff --git a/redpen-core/src/main/java/cc/redpen/config/ValidatorConfiguration.java b/redpen-core/src/main/java/cc/redpen/config/ValidatorConfiguration.java
index 4f1a99d8..180f2f3c 100644
--- a/redpen-core/src/main/java/cc/redpen/config/ValidatorConfiguration.java
+++ b/redpen-core/src/main/java/cc/redpen/config/ValidatorConfiguration.java
@@ -17,6 +17,7 @@
  */
 package cc.redpen.config;
 
+import java.io.Serializable;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -24,9 +25,9 @@ import java.util.Objects;
 /**
  * Configuration for Validators.
  */
-public class ValidatorConfiguration implements Cloneable {
+public class ValidatorConfiguration implements Serializable, Cloneable {
     private final String configurationName;
-    private final Map<String, String> attributes;
+    private Map<String, String> attributes;
 
     /**
      * Constructor.
@@ -119,7 +120,9 @@ public class ValidatorConfiguration implements Cloneable {
      */
     @Override public ValidatorConfiguration clone() {
         try {
-            return (ValidatorConfiguration)super.clone();
+            ValidatorConfiguration clone = (ValidatorConfiguration)super.clone();
+            clone.attributes = new HashMap<>(attributes);
+            return clone;
         }
         catch (CloneNotSupportedException e) {
             throw new RuntimeException(e);


Test patch:
diff --git a/redpen-core/src/test/java/cc/redpen/config/ConfigurationTest.java b/redpen-core/src/test/java/cc/redpen/config/ConfigurationTest.java
index a271f0b3..ec97b6dc 100644
--- a/redpen-core/src/test/java/cc/redpen/config/ConfigurationTest.java
+++ b/redpen-core/src/test/java/cc/redpen/config/ConfigurationTest.java
@@ -19,8 +19,13 @@ package cc.redpen.config;
 
 import org.junit.Test;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.util.Optional;
 
+import static cc.redpen.config.SymbolType.AMPERSAND;
 import static java.util.Collections.emptyList;
 import static org.junit.Assert.*;
 
@@ -93,21 +98,59 @@ public class ConfigurationTest {
     }
 
     @Test
-    public void configurationCanBeCloned() throws Exception {
+    public void canBeCloned() throws Exception {
         Configuration conf = new Configuration.ConfigurationBuilder()
           .setLanguage("ja")
           .setVariant("hankaku")
           .addValidatorConfig(new ValidatorConfiguration("SentenceLength")).build();
 
-        Configuration conf2 = conf.clone();
-        assertNotSame(conf, conf2);
-        assertEquals("ja", conf2.getLang());
-        assertEquals("hankaku", conf2.getVariant());
+        Configuration clone = conf.clone();
+        assertNotSame(conf, clone);
+        assertEquals(conf.getLang(), clone.getLang());
+        assertEquals(conf.getVariant(), clone.getVariant());
 
-        assertNotSame(conf.getValidatorConfigs().get(0), conf2.getValidatorConfigs().get(0));
-        assertEquals(conf.getValidatorConfigs(), conf2.getValidatorConfigs());
+        assertNotSame(conf.getValidatorConfigs(), clone.getValidatorConfigs());
+        assertNotSame(conf.getValidatorConfigs().get(0), clone.getValidatorConfigs().get(0));
+        assertEquals(conf.getValidatorConfigs(), clone.getValidatorConfigs());
 
-        assertNotSame(conf.getSymbolTable(), conf2.getSymbolTable());
-        assertEquals(conf.getSymbolTable(), conf2.getSymbolTable());
+        assertNotSame(conf.getSymbolTable(), clone.getSymbolTable());
+        assertEquals(conf.getSymbolTable(), clone.getSymbolTable());
+    }
+
+    @Test
+    public void equals() throws Exception {
+        Configuration conf = new Configuration.ConfigurationBuilder()
+          .setLanguage("ja")
+          .setVariant("hankaku")
+          .addValidatorConfig(new ValidatorConfiguration("SentenceLength")).build();
+
+        Configuration clone = conf.clone();
+        assertEquals(conf, clone);
+        assertEquals(conf.hashCode(), clone.hashCode());
+
+        clone.getValidatorConfigs().remove(0);
+        assertFalse(conf.equals(clone));
+
+        clone = conf.clone();
+        clone.getSymbolTable().overrideSymbol(new Symbol(AMPERSAND, '^'));
+        assertFalse(conf.equals(clone));
+    }
+
+    @Test
+    public void serializable() throws Exception {
+        Configuration conf = new Configuration.ConfigurationBuilder()
+          .setLanguage("ja")
+          .setVariant("hankaku")
+          .addValidatorConfig(new ValidatorConfiguration("SentenceLength")).build();
+
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        ObjectOutputStream out = new ObjectOutputStream(bytes);
+        out.writeObject(conf);
+
+        ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()));
+        Configuration conf2 = (Configuration)in.readObject();
+
+        assertEquals(conf, conf2);
+        assertEquals(conf.getTokenizer().getClass(), conf2.getTokenizer().getClass());
     }
 }
diff --git a/redpen-core/src/test/java/cc/redpen/config/SymbolTableTest.java b/redpen-core/src/test/java/cc/redpen/config/SymbolTableTest.java
index 18d3c0fe..33b98fb0 100644
--- a/redpen-core/src/test/java/cc/redpen/config/SymbolTableTest.java
+++ b/redpen-core/src/test/java/cc/redpen/config/SymbolTableTest.java
@@ -4,10 +4,11 @@ import org.junit.Test;
 
 import java.util.Optional;
 
-import static cc.redpen.config.SymbolType.COMMA;
-import static cc.redpen.config.SymbolType.FULL_STOP;
+import static cc.redpen.config.SymbolType.*;
 import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
 
 public class SymbolTableTest {
   @Test
@@ -39,4 +40,19 @@ public class SymbolTableTest {
     assertEquals('.', table.getSymbol(FULL_STOP).getValue());
     assertEquals(',', table.getSymbol(COMMA).getValue());
   }
+
+  @Test
+  public void canBeCloned() throws Exception {
+    SymbolTable symbolTable = new SymbolTable("ja", Optional.of("hankaku"), singletonList(new Symbol(BACKSLASH, '+')));
+    SymbolTable clone = symbolTable.clone();
+
+    assertNotSame(symbolTable, clone);
+    assertEquals(symbolTable, clone);
+    assertEquals(symbolTable.getLang(), clone.getLang());
+    assertEquals(symbolTable.getVariant(), clone.getVariant());
+
+    symbolTable.overrideSymbol(new Symbol(AMPERSAND, ','));
+    assertEquals('&', clone.getSymbol(AMPERSAND).getValue());
+    assertEquals(COMMA, clone.getSymbolByValue(',').getType());
+  }
 }
\ No newline at end of file
diff --git a/redpen-core/src/test/java/cc/redpen/config/ValidatorConfigurationTest.java b/redpen-core/src/test/java/cc/redpen/config/ValidatorConfigurationTest.java
new file mode 100644
index 00000000..f5ff2d4e
--- /dev/null
+++ b/redpen-core/src/test/java/cc/redpen/config/ValidatorConfigurationTest.java
@@ -0,0 +1,21 @@
+package cc.redpen.config;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+
+public class ValidatorConfigurationTest {
+
+  @Test
+  public void canBeCloned() throws Exception {
+    ValidatorConfiguration conf = new ValidatorConfiguration("test").addAttribute("foo", "bar");
+    ValidatorConfiguration clone = conf.clone();
+
+    assertNotSame(conf, clone);
+    assertEquals(conf, clone);
+
+    assertNotSame(conf.getAttributes(), clone.getAttributes());
+    assertEquals(conf.getAttributes(), clone.getAttributes());
+  }
+}
\ No newline at end of file