Use Float.compare/Double.compare instead of '==' in geo classes (#13301)

when using == to compare the floats -0.0 and 0.0 they will be considered equal but their hashcode is different.
This commit is contained in:
Tim Grein 2024-04-15 15:54:33 +02:00 committed by GitHub
parent 0345fcabb3
commit 1c3c094227
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 74 additions and 21 deletions

View File

@ -77,7 +77,9 @@ public final class Circle extends LatLonGeometry {
if (this == o) return true;
if (!(o instanceof Circle)) return false;
Circle circle = (Circle) o;
return lat == circle.lat && lon == circle.lon && radiusMeters == circle.radiusMeters;
return Double.compare(lat, circle.lat) == 0
&& Double.compare(lon, circle.lon) == 0
&& Double.compare(radiusMeters, circle.radiusMeters) == 0;
}
@Override

View File

@ -65,7 +65,7 @@ public final class Point extends LatLonGeometry {
if (this == o) return true;
if (!(o instanceof Point)) return false;
Point point = (Point) o;
return point.lat == lat && point.lon == lon;
return Double.compare(point.lat, lat) == 0 && Double.compare(point.lon, lon) == 0;
}
@Override

View File

@ -259,7 +259,10 @@ final class Rectangle2D implements Component2D {
if (this == o) return true;
if (!(o instanceof Rectangle2D)) return false;
Rectangle2D that = (Rectangle2D) o;
return minX == that.minX && maxX == that.maxX && minY == that.minY && maxY == that.maxY;
return Double.compare(minX, that.minX) == 0
&& Double.compare(maxX, that.maxX) == 0
&& Double.compare(minY, that.minY) == 0
&& Double.compare(maxY, that.maxY) == 0;
}
@Override

View File

@ -78,7 +78,9 @@ public final class XYCircle extends XYGeometry {
if (this == o) return true;
if (!(o instanceof XYCircle)) return false;
XYCircle circle = (XYCircle) o;
return x == circle.x && y == circle.y && radius == circle.radius;
return Float.compare(x, circle.x) == 0
&& Float.compare(y, circle.y) == 0
&& Float.compare(radius, circle.radius) == 0;
}
@Override

View File

@ -65,7 +65,7 @@ public final class XYPoint extends XYGeometry {
if (this == o) return true;
if (!(o instanceof XYPoint)) return false;
XYPoint point = (XYPoint) o;
return point.x == x && point.y == y;
return Float.compare(point.x, x) == 0 && Float.compare(point.y, y) == 0;
}
@Override

View File

@ -211,7 +211,7 @@ public abstract class BaseXYShapeTestCase extends BaseSpatialTestCase {
POINT() {
@Override
public XYPoint nextShape() {
return ShapeTestUtil.nextPoint();
return ShapeTestUtil.nextXYPoint();
}
},
LINE() {

View File

@ -39,7 +39,7 @@ public class TestXYMultiPointShapeQueries extends BaseXYShapeTestCase {
int n = random().nextInt(4) + 1;
XYPoint[] points = new XYPoint[n];
for (int i = 0; i < n; i++) {
points[i] = ShapeTestUtil.nextPoint();
points[i] = ShapeTestUtil.nextXYPoint();
}
return points;
}

View File

@ -76,9 +76,9 @@ public class TestCircle extends LuceneTestCase {
assertEquals(circle, copy);
assertEquals(circle.hashCode(), copy.hashCode());
Circle otherCircle = GeoTestUtil.nextCircle();
if (circle.getLon() != otherCircle.getLon()
|| circle.getLat() != otherCircle.getLat()
|| circle.getRadius() != otherCircle.getRadius()) {
if (Double.compare(circle.getLon(), otherCircle.getLon()) != 0
|| Double.compare(circle.getLat(), otherCircle.getLat()) != 0
|| Double.compare(circle.getRadius(), otherCircle.getRadius()) != 0) {
assertNotEquals(circle, otherCircle);
assertNotEquals(circle.hashCode(), otherCircle.hashCode());
} else {

View File

@ -16,6 +16,7 @@
*/
package org.apache.lucene.geo;
import org.apache.lucene.tests.geo.GeoTestUtil;
import org.apache.lucene.tests.util.LuceneTestCase;
public class TestPoint extends LuceneTestCase {
@ -43,4 +44,22 @@ public class TestPoint extends LuceneTestCase {
.getMessage()
.contains("invalid longitude 180.5; must be between -180.0 and 180.0"));
}
public void testEqualsAndHashCode() {
Point point = GeoTestUtil.nextPoint();
Point copy = new Point(point.getLat(), point.getLon());
assertEquals(point, copy);
assertEquals(point.hashCode(), copy.hashCode());
Point otherPoint = GeoTestUtil.nextPoint();
if (Double.compare(point.getLat(), otherPoint.getLat()) != 0
|| Double.compare(point.getLon(), otherPoint.getLon()) != 0) {
assertNotEquals(point, otherPoint);
assertNotEquals(point.hashCode(), otherPoint.hashCode());
} else {
assertEquals(point, otherPoint);
assertEquals(point.hashCode(), otherPoint.hashCode());
}
}
}

View File

@ -121,4 +121,28 @@ public class TestRectangle2D extends LuceneTestCase {
}
}
}
public void testEqualsAndHashCode() {
Random random = random();
XYRectangle xyRectangle = ShapeTestUtil.nextBox(random);
Component2D rectangle2D = Rectangle2D.create(xyRectangle);
Component2D copy = Rectangle2D.create(xyRectangle);
assertEquals(rectangle2D, copy);
assertEquals(rectangle2D.hashCode(), copy.hashCode());
XYRectangle otherXYRectangle = ShapeTestUtil.nextBox(random);
Component2D otherRectangle2D = Rectangle2D.create(otherXYRectangle);
if (Double.compare(rectangle2D.getMinX(), otherRectangle2D.getMinX()) != 0
|| Double.compare(rectangle2D.getMaxX(), otherRectangle2D.getMaxX()) != 0
|| Double.compare(rectangle2D.getMinY(), otherRectangle2D.getMinY()) != 0
|| Double.compare(rectangle2D.getMaxY(), otherRectangle2D.getMaxY()) != 0) {
assertNotEquals(rectangle2D, otherRectangle2D);
assertNotEquals(rectangle2D.hashCode(), otherRectangle2D.hashCode());
} else {
assertEquals(rectangle2D, otherRectangle2D);
assertEquals(rectangle2D.hashCode(), otherRectangle2D.hashCode());
}
}
}

View File

@ -111,9 +111,9 @@ public class TestXYCircle extends LuceneTestCase {
assertEquals(circle, copy);
assertEquals(circle.hashCode(), copy.hashCode());
XYCircle otherCircle = ShapeTestUtil.nextCircle();
if (circle.getX() != otherCircle.getX()
|| circle.getY() != otherCircle.getY()
|| circle.getRadius() != otherCircle.getRadius()) {
if (Float.compare(circle.getX(), otherCircle.getX()) != 0
|| Float.compare(circle.getY(), otherCircle.getY()) != 0
|| Float.compare(circle.getRadius(), otherCircle.getRadius()) != 0) {
assertNotEquals(circle, otherCircle);
assertNotEquals(circle.hashCode(), otherCircle.hashCode());
} else {

View File

@ -87,7 +87,8 @@ public class TestXYPoint extends LuceneTestCase {
assertEquals(point.hashCode(), copy.hashCode());
XYPoint otherPoint =
new XYPoint(ShapeTestUtil.nextFloat(random()), ShapeTestUtil.nextFloat(random()));
if (point.getX() != otherPoint.getX() || point.getY() != otherPoint.getY()) {
if (Float.compare(point.getX(), otherPoint.getX()) != 0
|| Float.compare(point.getY(), otherPoint.getY()) != 0) {
assertNotEquals(point, otherPoint);
// it is possible to have hashcode collisions
} else {

View File

@ -125,10 +125,10 @@ public class TestXYRectangle extends LuceneTestCase {
assertEquals(rectangle, copy);
assertEquals(rectangle.hashCode(), copy.hashCode());
XYRectangle otherRectangle = ShapeTestUtil.nextBox(random());
if (rectangle.minX != otherRectangle.minX
|| rectangle.maxX != otherRectangle.maxX
|| rectangle.minY != otherRectangle.minY
|| rectangle.maxY != otherRectangle.maxY) {
if (Float.compare(rectangle.minX, otherRectangle.minX) != 0
|| Float.compare(rectangle.maxX, otherRectangle.maxX) != 0
|| Float.compare(rectangle.minY, otherRectangle.minY) != 0
|| Float.compare(rectangle.maxY, otherRectangle.maxY) != 0) {
assertNotEquals(rectangle, otherRectangle);
assertNotEquals(rectangle.hashCode(), otherRectangle.hashCode());
} else {

View File

@ -16,6 +16,8 @@
*/
package org.apache.lucene.tests.geo;
import static org.apache.lucene.geo.GeoUtils.*;
import com.carrotsearch.randomizedtesting.RandomizedContext;
import java.io.BufferedReader;
import java.io.FileNotFoundException;
@ -43,12 +45,12 @@ public class GeoTestUtil {
/** returns next pseudorandom latitude (anywhere) */
public static double nextLatitude() {
return nextDoubleInternal(-90, 90);
return nextDoubleInternal(MIN_LAT_INCL, MAX_LAT_INCL);
}
/** returns next pseudorandom longitude (anywhere) */
public static double nextLongitude() {
return nextDoubleInternal(-180, 180);
return nextDoubleInternal(MIN_LON_INCL, MAX_LON_INCL);
}
/**

View File

@ -64,7 +64,7 @@ public class ShapeTestUtil {
}
}
public static XYPoint nextPoint() {
public static XYPoint nextXYPoint() {
Random random = random();
float x = nextFloat(random);
float y = nextFloat(random);