Address PR comments

elastic · Jun 15, 2015 · a08f51e · a08f51e
1 parent 0ef5a27
commit a08f51e
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 32 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
@@ -203,12 +203,10 @@ public MappedFieldType clone() {
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (!(o instanceof MappedFieldType)) return false;
         if (!super.equals(o)) return false;
         MappedFieldType fieldType = (MappedFieldType) o;
-        return Objects.equals(boost, fieldType.boost) &&
-            Objects.equals(docValues, fieldType.docValues) &&
+        return boost == fieldType.boost &&
+            docValues == fieldType.docValues &&
             Objects.equals(names, fieldType.names) &&
             Objects.equals(indexAnalyzer, fieldType.indexAnalyzer) &&
             Objects.equals(searchAnalyzer, fieldType.searchAnalyzer) &&

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java
@@ -125,8 +125,6 @@ public MappedFieldType clone() {
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (!(o instanceof BinaryFieldType)) return false;
             if (!super.equals(o)) return false;
             BinaryFieldType that = (BinaryFieldType) o;
             return Objects.equals(tryUncompressing, that.tryUncompressing);

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
@@ -237,17 +237,15 @@ public DateFieldType clone() {
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (!(o instanceof DateFieldType)) return false;
             if (!super.equals(o)) return false;
             DateFieldType that = (DateFieldType) o;
             return Objects.equals(dateTimeFormatter.format(), that.dateTimeFormatter.format()) &&
-                Objects.equals(timeUnit, that.timeUnit);
+                   Objects.equals(timeUnit, that.timeUnit);
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(super.hashCode(), dateTimeFormatter, timeUnit);
+            return Objects.hash(super.hashCode(), dateTimeFormatter.format(), timeUnit);
         }
 
         public FormatDateTimeFormatter dateTimeFormatter() {

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java
@@ -314,16 +314,14 @@ public MappedFieldType clone() {
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (!(o instanceof GeoPointFieldType)) return false;
             if (!super.equals(o)) return false;
             GeoPointFieldType that = (GeoPointFieldType) o;
-            return java.util.Objects.equals(geohashPrecision, that.geohashPrecision) &&
-                java.util.Objects.equals(geohashPrefixEnabled, that.geohashPrefixEnabled) &&
-                java.util.Objects.equals(validateLon, that.validateLon) &&
-                java.util.Objects.equals(validateLat, that.validateLat) &&
-                java.util.Objects.equals(normalizeLon, that.normalizeLon) &&
-                java.util.Objects.equals(normalizeLat, that.normalizeLat) &&
+            return geohashPrecision == that.geohashPrecision &&
+                geohashPrefixEnabled == that.geohashPrefixEnabled &&
+                validateLon == that.validateLon &&
+                validateLat == that.validateLat &&
+                normalizeLon == that.normalizeLon &&
+                normalizeLat == that.normalizeLat &&
                 java.util.Objects.equals(geohashFieldType, that.geohashFieldType) &&
                 java.util.Objects.equals(latFieldType, that.latFieldType) &&
                 java.util.Objects.equals(lonFieldType, that.lonFieldType);

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java
@@ -208,17 +208,15 @@ public MappedFieldType clone() {
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (!(o instanceof GeoShapeFieldType)) return false;
             if (!super.equals(o)) return false;
             GeoShapeFieldType that = (GeoShapeFieldType) o;
-            return Objects.equals(treeLevels, that.treeLevels) &&
-                Objects.equals(precisionInMeters, that.precisionInMeters) &&
-                Objects.equals(defaultDistanceErrorPct, that.defaultDistanceErrorPct) &&
+            return treeLevels == that.treeLevels &&
+                precisionInMeters == that.precisionInMeters &&
+                defaultDistanceErrorPct == that.defaultDistanceErrorPct &&
                 Objects.equals(tree, that.tree) &&
                 Objects.equals(strategyName, that.strategyName) &&
                 Objects.equals(distanceErrorPct, that.distanceErrorPct) &&
-                Objects.equals(orientation, that.orientation);
+                orientation == that.orientation;
         }
 
         @Override
@@ -229,6 +227,8 @@ public int hashCode() {
         @Override
         public void freeze() {
             super.freeze();
+            // This is a bit hackish: we need to setup the spatial tree and strategies once the field name is set, which
+            // must be by the time freeze is called.
             SpatialPrefixTree prefixTree;
             if ("geohash".equals(tree)) {
                 prefixTree = new GeohashPrefixTree(ShapeBuilder.SPATIAL_CONTEXT, getLevels(treeLevels, precisionInMeters, Defaults.GEOHASH_LEVELS, true));

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java
@@ -151,11 +151,9 @@ protected FieldNamesFieldType(FieldNamesFieldType ref) {
 
         @Override
         public boolean equals(Object o) {
-            if (this == o) return true;
-            if (!(o instanceof FieldNamesFieldType)) return false;
             if (!super.equals(o)) return false;
             FieldNamesFieldType that = (FieldNamesFieldType) o;
-            return Objects.equals(enabled, that.enabled);
+            return enabled == that.enabled;
         }
 
         @Override

diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java
@@ -38,7 +38,7 @@ protected void modifyProperty(MappedFieldType ft, int propNum) {
         BinaryFieldMapper.BinaryFieldType bft = (BinaryFieldMapper.BinaryFieldType)ft;
         switch (propNum) {
             case 0: bft.setTryUncompressing(!bft.tryUncompressing()); break;
-            default: super.modifyProperty(ft, numProperties() - propNum - 1);
+            default: super.modifyProperty(ft, 1 + propNum);
         }
     }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java
@@ -47,7 +47,7 @@ protected void modifyProperty(MappedFieldType ft, int propNum) {
         switch (propNum) {
             case 0: dft.setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT)); break;
             case 1: dft.setTimeUnit(TimeUnit.HOURS); break;
-            default: super.modifyProperty(ft, numProperties() - propNum - 1);
+            default: super.modifyProperty(ft, 2 + propNum);
         }
     }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java
@@ -42,7 +42,7 @@ protected void modifyProperty(MappedFieldType ft, int propNum) {
             case 3: gft.setValidateLat(!gft.validateLat()); break;
             case 4: gft.setNormalizeLon(!gft.normalizeLon()); break;
             case 5: gft.setNormalizeLat(!gft.normalizeLat()); break;
-            default: super.modifyProperty(ft, numProperties() - propNum - 1);
+            default: super.modifyProperty(ft, 6 + propNum);
         }
     }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java
@@ -45,7 +45,7 @@ protected void modifyProperty(MappedFieldType ft, int propNum) {
             case 3: gft.setPrecisionInMeters(20); break;
             case 4: gft.setDefaultDistanceErrorPct(0.5); break;
             case 5: gft.setOrientation(ShapeBuilder.Orientation.LEFT); break;
-            default: super.modifyProperty(ft, numProperties() - propNum - 1);
+            default: super.modifyProperty(ft, 6 + propNum);
         }
     }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java
@@ -37,7 +37,7 @@ protected void modifyProperty(MappedFieldType ft, int propNum) {
         FieldNamesFieldMapper.FieldNamesFieldType fnft = (FieldNamesFieldMapper.FieldNamesFieldType)ft;
         switch (propNum) {
             case 0: fnft.setEnabled(!fnft.isEnabled()); break;
-            default: super.modifyProperty(ft, numProperties() - propNum - 1);
+            default: super.modifyProperty(ft, 1 + propNum);
         }
     }
 }