From 4a340568b4b759439c2294bdea63b533070dd2fe Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 15 Mar 2018 11:53:47 +0100 Subject: [PATCH] Fix DateTimeValue equals and compareTo; move TimeZones to values module DateTimeValue now correctly marks renamed time zones as being equal. --- .../bolt/v2/messaging/Neo4jPackV2Test.java | 2 +- community/kernel/pom.xml | 6 --- .../index/schema/ZonedDateTimeSchemaKey.java | 2 +- .../impl/index/schema/ZonedTimeSchemaKey.java | 2 +- .../neo4j/kernel/impl/store/TemporalType.java | 1 + .../IndexProviderCompatibilityTestSuite.java | 1 + community/values/pom.xml | 6 +++ .../neo4j/values/storable/DateTimeValue.java | 45 +++++++++++++++++-- .../org/neo4j/values/storable}/TimeZones.java | 16 +++++-- .../src/main/resources/TZIDS | 0 .../values/storable/DateTimeValueTest.java | 7 +++ .../neo4j/values/storable}/TimeZonesTest.java | 2 +- .../values/storable/ValueComparisonTest.java | 12 +++++ 13 files changed, 85 insertions(+), 17 deletions(-) rename community/{kernel/src/main/java/org/neo4j/kernel/impl/store => values/src/main/java/org/neo4j/values/storable}/TimeZones.java (85%) rename community/{kernel => values}/src/main/resources/TZIDS (100%) rename community/{kernel/src/test/java/org/neo4j/kernel/impl/store => values/src/test/java/org/neo4j/values/storable}/TimeZonesTest.java (99%) diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v2/messaging/Neo4jPackV2Test.java b/community/bolt/src/test/java/org/neo4j/bolt/v2/messaging/Neo4jPackV2Test.java index d7f5241549e72..b70199021fc85 100644 --- a/community/bolt/src/test/java/org/neo4j/bolt/v2/messaging/Neo4jPackV2Test.java +++ b/community/bolt/src/test/java/org/neo4j/bolt/v2/messaging/Neo4jPackV2Test.java @@ -35,7 +35,7 @@ import org.neo4j.bolt.v1.messaging.Neo4jPack; import org.neo4j.bolt.v1.packstream.PackedInputArray; import org.neo4j.bolt.v1.packstream.PackedOutputArray; -import org.neo4j.kernel.impl.store.TimeZones; +import org.neo4j.values.storable.TimeZones; import org.neo4j.values.AnyValue; import org.neo4j.values.storable.CoordinateReferenceSystem; import org.neo4j.values.storable.DateTimeValue; diff --git a/community/kernel/pom.xml b/community/kernel/pom.xml index 84a5e9b1de91e..4d8adfffacd9d 100644 --- a/community/kernel/pom.xml +++ b/community/kernel/pom.xml @@ -267,11 +267,5 @@ the relevant Commercial Agreement. test-jar test - - org.apache.commons - commons-compress - 1.16.1 - test - diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java index 3f616c44c9441..41619f445cf95 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java @@ -22,7 +22,7 @@ import java.time.ZoneId; import java.time.ZoneOffset; -import org.neo4j.kernel.impl.store.TimeZones; +import org.neo4j.values.storable.TimeZones; import org.neo4j.values.storable.DateTimeValue; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Values; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedTimeSchemaKey.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedTimeSchemaKey.java index 651bddbc1a536..681eb5c259da4 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedTimeSchemaKey.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedTimeSchemaKey.java @@ -21,7 +21,7 @@ import java.time.ZoneOffset; -import org.neo4j.kernel.impl.store.TimeZones; +import org.neo4j.values.storable.TimeZones; import org.neo4j.values.storable.TimeValue; import org.neo4j.values.storable.Value; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TemporalType.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TemporalType.java index ca9195259bb76..78bf9e0cfdb9f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TemporalType.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TemporalType.java @@ -43,6 +43,7 @@ import org.neo4j.values.storable.LocalTimeValue; import org.neo4j.values.storable.LongArray; import org.neo4j.values.storable.TimeValue; +import org.neo4j.values.storable.TimeZones; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Values; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java b/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java index ed3a545be9173..9e6a3ebce3829 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java @@ -115,6 +115,7 @@ public Compatibility( IndexProviderCompatibilityTestSuite testSuite, SchemaIndex DateTimeValue.datetime( 2014, 3, 25, 12, 46, 13, 7474, "+05:00" ), DateTimeValue.datetime( 2014, 3, 25, 12, 45, 14, 7474, "+05:00" ), DateTimeValue.datetime( 2014, 3, 25, 12, 45, 14, 7475, "+05:00" ), + DateTimeValue.datetime( 2001, 1, 25, 11, 11, 30, 0, "Canada/East-Saskatchewan" ), DateTimeValue.datetime( 10000, 100, ZoneOffset.ofTotalSeconds( 3 ) ), DateTimeValue.datetime( 10000, 101, ZoneOffset.ofTotalSeconds( -3 ) ), DurationValue.duration( 10, 20, 30, 40 ), diff --git a/community/values/pom.xml b/community/values/pom.xml index 10700baf2c91d..7ece5f128b34a 100644 --- a/community/values/pom.xml +++ b/community/values/pom.xml @@ -71,5 +71,11 @@ org.mockito mockito-core + + org.apache.commons + commons-compress + 1.16.1 + test + diff --git a/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java b/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java index 4637679f0c6f6..4c01e251b1cd5 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java +++ b/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java @@ -407,7 +407,15 @@ boolean hasTime() @Override public boolean equals( Value other ) { - return other instanceof DateTimeValue && value.equals( ((DateTimeValue) other).value ); + if ( other instanceof DateTimeValue ) + { + ZonedDateTime that = ((DateTimeValue) other).value; + return value.toLocalDateTime().equals( that.toLocalDateTime() ) && + value.getOffset().equals( that.getOffset() ) && + !areDifferentZones( value.getZone(), that.getZone() ); + + } + return false; } @Override @@ -427,10 +435,39 @@ public void writeTo( ValueWriter writer ) throws E } @Override - int unsafeCompareTo( Value otherValue ) + int unsafeCompareTo( Value other ) + { + ZonedDateTime that = ((DateTimeValue) other).value; + int cmp = Long.compare( value.toEpochSecond(), that.toEpochSecond() ); + if ( cmp == 0 ) + { + cmp = value.toLocalTime().getNano() - that.toLocalTime().getNano(); + if ( cmp == 0 ) + { + cmp = value.toLocalDateTime().compareTo( that.toLocalDateTime() ); + if ( cmp == 0 ) + { + ZoneId thisZone = value.getZone(); + ZoneId thatZone = that.getZone(); + if ( areDifferentZones( thisZone, thatZone ) ) + { + cmp = thisZone.getId().compareTo( thatZone.getId() ); + } + if ( cmp == 0 ) + { + cmp = value.getChronology().compareTo( that.getChronology() ); + } + } + } + } + return cmp; + } + + private boolean areDifferentZones( ZoneId thisZone, ZoneId thatZone ) { - DateTimeValue other = (DateTimeValue) otherValue; - return value.compareTo( other.value ); + return !(thisZone instanceof ZoneOffset) && + !(thatZone instanceof ZoneOffset) && + TimeZones.map( thisZone.getId() ) != TimeZones.map( thatZone.getId() ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TimeZones.java b/community/values/src/main/java/org/neo4j/values/storable/TimeZones.java similarity index 85% rename from community/kernel/src/main/java/org/neo4j/kernel/impl/store/TimeZones.java rename to community/values/src/main/java/org/neo4j/values/storable/TimeZones.java index a1ed15f0d7ef6..8856e19e4bcb5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/TimeZones.java +++ b/community/values/src/main/java/org/neo4j/values/storable/TimeZones.java @@ -17,7 +17,7 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ -package org.neo4j.kernel.impl.store; +package org.neo4j.values.storable; import java.io.BufferedReader; import java.io.IOException; @@ -85,6 +85,8 @@ public static Set supportedTimeZones() { String latestVersion = ""; Pattern version = Pattern.compile( "# tzdata([0-9]{4}[a-z])" ); + Map tempStringToShort = new HashMap<>( 1024 ); + try ( BufferedReader reader = new BufferedReader( new InputStreamReader( TimeZones.class.getResourceAsStream( "/TZIDS" ) ) ) ) { for ( String line; (line = reader.readLine()) != null; ) @@ -108,12 +110,12 @@ else if ( line.startsWith( "#" ) ) String oldName = line.substring( 0, sep ); String newName = line.substring( sep + 1 ); TIME_ZONE_SHORT_TO_STRING.add( newName ); - TIME_ZONE_STRING_TO_SHORT.put( oldName, (short) (TIME_ZONE_SHORT_TO_STRING.size() - 1) ); + tempStringToShort.put( oldName, (short) (TIME_ZONE_SHORT_TO_STRING.size() - 1) ); } else { TIME_ZONE_SHORT_TO_STRING.add( line ); - TIME_ZONE_STRING_TO_SHORT.put( line, (short) (TIME_ZONE_SHORT_TO_STRING.size() - 1) ); + tempStringToShort.put( line, (short) (TIME_ZONE_SHORT_TO_STRING.size() - 1) ); } } LATEST_SUPPORTED_IANA_VERSION = latestVersion; @@ -122,5 +124,13 @@ else if ( line.startsWith( "#" ) ) { throw new RuntimeException( "Failed to read time zone id file." ); } + + for ( Map.Entry entry : tempStringToShort.entrySet() ) + { + String oldName = entry.getKey(); + String newName = TIME_ZONE_SHORT_TO_STRING.get( entry.getValue() ); + Short newNameId = tempStringToShort.get( newName ); + TIME_ZONE_STRING_TO_SHORT.put( oldName, newNameId ); + } } } diff --git a/community/kernel/src/main/resources/TZIDS b/community/values/src/main/resources/TZIDS similarity index 100% rename from community/kernel/src/main/resources/TZIDS rename to community/values/src/main/resources/TZIDS diff --git a/community/values/src/test/java/org/neo4j/values/storable/DateTimeValueTest.java b/community/values/src/test/java/org/neo4j/values/storable/DateTimeValueTest.java index 636dd68d0b895..8ff736bcacafe 100644 --- a/community/values/src/test/java/org/neo4j/values/storable/DateTimeValueTest.java +++ b/community/values/src/test/java/org/neo4j/values/storable/DateTimeValueTest.java @@ -454,6 +454,13 @@ public void shouldEqualItself() assertEqual( datetime( 10000, 100, UTC ), datetime( 10000, 100, UTC ) ); } + @Test + public void shouldEqualRenamedTimeZone() + { + assertEqual( datetime( 10000, 100, ZoneId.of( "Canada/Saskatchewan" ) ), + datetime( 10000, 100, ZoneId.of( "Canada/East-Saskatchewan" ) ) ); + } + @Test public void shouldNotEqualSameInstantButDifferentTimezone() { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TimeZonesTest.java b/community/values/src/test/java/org/neo4j/values/storable/TimeZonesTest.java similarity index 99% rename from community/kernel/src/test/java/org/neo4j/kernel/impl/store/TimeZonesTest.java rename to community/values/src/test/java/org/neo4j/values/storable/TimeZonesTest.java index 02e403af65f71..511c02cc80a42 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TimeZonesTest.java +++ b/community/values/src/test/java/org/neo4j/values/storable/TimeZonesTest.java @@ -17,7 +17,7 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ -package org.neo4j.kernel.impl.store; +package org.neo4j.values.storable; import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; diff --git a/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java b/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java index 0bfa89267542b..0ce8bd77205db 100644 --- a/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java +++ b/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java @@ -27,6 +27,7 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.time.OffsetTime; +import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.Arrays; import java.util.Comparator; @@ -34,6 +35,7 @@ import java.util.stream.Collectors; import static java.lang.String.format; +import static org.junit.Assert.assertEquals; import static org.neo4j.values.storable.CoordinateReferenceSystem.Cartesian; import static org.neo4j.values.storable.CoordinateReferenceSystem.WGS84; import static org.neo4j.values.storable.DateTimeValue.datetime; @@ -103,6 +105,8 @@ public class ValueComparisonTest datetime(2018, 2, 2, 0, 0, 0, 0, "+00:00"), datetime(2018, 2, 2, 1, 30, 0, 0, "+01:00"), datetime(2018, 2, 2, 1, 0, 0, 0, "+00:00"), + datetime(2018, 3, 2, 1, 0, 0, 0, "Europe/Berlin"), + datetime(2018, 3, 2, 1, 0, 0, 0, "Europe/Stockholm"), // same offset as Europe/Berlin, so compared by zone name localDateTime(2018, 2, 2, 0, 0, 0, 0), localDateTime(2018, 2, 2, 0, 0, 0, 1), localDateTime(2018, 2, 2, 0, 0, 1, 0), @@ -259,6 +263,14 @@ public void shouldOrderValuesCorrectly() } } + @Test + public void shouldCompareRenamedTimeZonesByZoneNumber() + { + int cmp = Values.COMPARATOR.compare( datetime( 10000, 100, ZoneId.of( "Canada/Saskatchewan" ) ), + datetime( 10000, 100, ZoneId.of( "Canada/East-Saskatchewan" ) ) ); + assertEquals( "East-Saskatchewan and Saskatchewan are the same place", 0, cmp ); + } + private int compare( Comparator comparator, T left, T right ) { int cmp1 = comparator.compare( left, right );