From 7bd1485ab6d17b0f9b7a3e4ffc04f900af845fff Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 30 Mar 2019 17:46:27 +0100 Subject: [PATCH 01/11] optimize parsing of dates and timestamps Stress testing the Spanner client library showed that more than 50% of the total cpu time consumed by the client library while executing queries and consuming result sets is spent on parsing dates and timestamps. This stress test uses a result set containing one column of each possible data type of Spanner (both single value column and array column). Spanner automatically translates the string values that are sent by the server into Date and Timestamp objects before the user requests these from the ResultSet. In a real-life scenario, the total percentage of time spent on parsing these values will however be a lot lower, as most of the time the client library will be waiting on network traffic. --- .../src/main/java/com/google/cloud/Date.java | 23 +++--- .../main/java/com/google/cloud/IntParser.java | 72 +++++++++++++++++++ .../main/java/com/google/cloud/Timestamp.java | 56 ++++++++++----- .../test/java/com/google/cloud/DateTest.java | 16 ++++- .../java/com/google/cloud/IntParserTest.java | 63 ++++++++++++++++ .../java/com/google/cloud/TimestampTest.java | 27 ++++++- 6 files changed, 225 insertions(+), 32 deletions(-) create mode 100644 google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java create mode 100644 google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java index 442e7dca0eef..6399626afc36 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java @@ -16,20 +16,16 @@ package com.google.cloud; -import com.google.api.core.BetaApi; -import com.google.common.base.Preconditions; import java.io.Serializable; import java.util.Calendar; import java.util.Objects; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import com.google.api.core.BetaApi; +import com.google.common.base.Preconditions; /** Represents a Date without time, such as 2017-03-17. Date is timezone independent. */ @BetaApi("This is going to be replaced with LocalDate from threetenbp") public final class Date implements Comparable, Serializable { - // Date format "yyyy-mm-dd" - private static final Pattern FORMAT_REGEXP = Pattern.compile("(\\d\\d\\d\\d)-(\\d\\d)-(\\d\\d)"); private static final long serialVersionUID = 8067099123096783929L; private final int year; private final int month; @@ -57,13 +53,14 @@ public static Date fromYearMonthDay(int year, int month, int dayOfMonth) { /** @param date Data in RFC 3339 date format (yyyy-mm-dd). */ public static Date parseDate(String date) { - Matcher matcher = FORMAT_REGEXP.matcher(date); - if (!matcher.matches()) { - throw new IllegalArgumentException("Invalid date: " + date); - } - int year = Integer.parseInt(matcher.group(1)); - int month = Integer.parseInt(matcher.group(2)); - int dayOfMonth = Integer.parseInt(matcher.group(3)); + Preconditions.checkNotNull(date); + final String invalidDate = "Invalid date: " + date; + Preconditions.checkArgument(date.length() == 10, invalidDate); + Preconditions.checkArgument(date.charAt(4) == '-', invalidDate); + Preconditions.checkArgument(date.charAt(7) == '-', invalidDate); + int year = IntParser.parseInt(date, 0, 4); + int month = IntParser.parseInt(date, 5, 7); + int dayOfMonth = IntParser.parseInt(date, 8, 10); return new Date(year, month, dayOfMonth); } diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java new file mode 100644 index 000000000000..700ae6e0ef93 --- /dev/null +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java @@ -0,0 +1,72 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud; + +import com.google.common.base.Preconditions; + +/** + * Util class for fast parsing of integer values. This is used by {@link Date#parseDate(String)} and + * {@link Timestamp#parseTimestamp(String)}. These parse methods are used internally by Google + * client libraries to parse text values returned by services, and these parse methods should be as + * efficient as possible. + */ +class IntParser { + + private static final int[] POWERS_OF_10 = + {1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000}; + + /** Parses an int from the given input between the specified begin and end. */ + static int parseInt(String input, int begin, int end) { + Preconditions.checkNotNull(input); + Preconditions.checkArgument(end - begin <= 10, "Max input length is 10"); + Preconditions.checkArgument(end >= begin, "End must be greater or equal to begin"); + Preconditions.checkArgument(begin >= 0, "Begin must be >= 0"); + Preconditions.checkArgument(end <= input.length(), "End must be <= input.length()"); + int res = 0; + for (int index = begin; index < end; index++) { + res += parseDigit(input.charAt(index), input) * POWERS_OF_10[end - index - 1]; + } + return res; + } + + private static int parseDigit(char c, String input) { + if (c == '0') { + return 0; + } else if (c == '1') { + return 1; + } else if (c == '2') { + return 2; + } else if (c == '3') { + return 3; + } else if (c == '4') { + return 4; + } else if (c == '5') { + return 5; + } else if (c == '6') { + return 6; + } else if (c == '7') { + return 7; + } else if (c == '8') { + return 8; + } else if (c == '9') { + return 9; + } else { + throw new NumberFormatException("Not a digit: " + c); + } + } + +} diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 618ef9c7211e..453c1a2b4e64 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -17,18 +17,15 @@ package com.google.cloud; import static com.google.common.base.Preconditions.checkArgument; - -import com.google.protobuf.util.Timestamps; import java.io.Serializable; import java.util.Date; import java.util.Objects; import java.util.concurrent.TimeUnit; -import org.threeten.bp.Instant; import org.threeten.bp.LocalDateTime; import org.threeten.bp.ZoneOffset; import org.threeten.bp.format.DateTimeFormatter; -import org.threeten.bp.format.DateTimeFormatterBuilder; -import org.threeten.bp.temporal.TemporalAccessor; +import com.google.api.client.util.Preconditions; +import com.google.protobuf.util.Timestamps; /** * Represents a timestamp with nanosecond precision. Timestamps cover the range [0001-01-01, @@ -49,15 +46,6 @@ public final class Timestamp implements Comparable, Serializable { private static final DateTimeFormatter format = DateTimeFormatter.ISO_LOCAL_DATE_TIME; - private static final DateTimeFormatter timestampParser = - new DateTimeFormatterBuilder() - .appendOptional(DateTimeFormatter.ISO_LOCAL_DATE_TIME) - .optionalStart() - .appendOffsetId() - .optionalEnd() - .toFormatter() - .withZone(ZoneOffset.UTC); - private final long seconds; private final int nanos; @@ -181,9 +169,43 @@ public com.google.protobuf.Timestamp toProto() { * the timezone offset (always ends in "Z"). */ public static Timestamp parseTimestamp(String timestamp) { - TemporalAccessor temporalAccessor = timestampParser.parse(timestamp); - Instant instant = Instant.from(temporalAccessor); - return ofTimeSecondsAndNanos(instant.getEpochSecond(), instant.getNano()); + Preconditions.checkNotNull(timestamp); + final String invalidTimestamp = "Invalid timestamp: " + timestamp; + Preconditions.checkArgument(timestamp.length() >= 19 && timestamp.length() <= 30, invalidTimestamp); + Preconditions.checkArgument(timestamp.charAt(4) == '-', invalidTimestamp); + Preconditions.checkArgument(timestamp.charAt(7) == '-', invalidTimestamp); + Preconditions.checkArgument(timestamp.charAt(10) == 'T', invalidTimestamp); + Preconditions.checkArgument( + (timestamp.length() == 19 && timestamp.charAt(18) != 'Z') + || + (timestamp.length() == 20 && timestamp.charAt(19) == 'Z') + || + (timestamp.charAt(19) == '.' && timestamp.length() > 20 && timestamp.charAt(20) != 'Z'), + invalidTimestamp); + int year = IntParser.parseInt(timestamp, 0, 4); + int month = IntParser.parseInt(timestamp, 5, 7); + int day = IntParser.parseInt(timestamp, 8, 10); + int hour = IntParser.parseInt(timestamp, 11, 13); + int minute = IntParser.parseInt(timestamp, 14, 16); + int second = IntParser.parseInt(timestamp, 17, 19); + int fraction = 0; + if(timestamp.length() > 20) { + if(timestamp.charAt(19) == '.') { + int endIndex; + if(timestamp.charAt(timestamp.length() - 1) == 'Z') { + endIndex = timestamp.length() - 1; + } else { + endIndex = timestamp.length(); + } + if(endIndex - 20 > 9) { + throw new IllegalArgumentException(invalidTimestamp); + } + fraction = IntParser.parseInt(timestamp, 20, endIndex); + } else { + } + } + LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction); + return ofTimeSecondsAndNanos(ldt.toEpochSecond(ZoneOffset.UTC), ldt.getNano()); } private StringBuilder toString(StringBuilder b) { diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java index 5892cc718b01..ff53e6e8fbce 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java @@ -18,7 +18,7 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; - +import static org.junit.Assert.fail; import com.google.common.testing.EqualsTester; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -38,6 +38,20 @@ public void parseDate() { assertThat(date.getYear()).isEqualTo(2016); assertThat(date.getMonth()).isEqualTo(9); assertThat(date.getDayOfMonth()).isEqualTo(18); + parseInvalidDate("2016/09/18"); + parseInvalidDate("2016 09 18"); + parseInvalidDate("2016-9-18"); + parseInvalidDate("2016-09-18T10:00"); + parseInvalidDate(""); + } + + private void parseInvalidDate(String input) { + try { + Date.parseDate(input); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage()).contains("Invalid date"); + } } @Test diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java new file mode 100644 index 000000000000..0dc6d6888fbe --- /dev/null +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java @@ -0,0 +1,63 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class IntParserTest { + + @Test + public void testParse() { + assertThat(IntParser.parseInt("1", 0, 1)).isEqualTo(1); + assertThat(IntParser.parseInt("1234", 0, 4)).isEqualTo(1234); + assertThat(IntParser.parseInt("1234", 2, 4)).isEqualTo(34); + assertThat(IntParser.parseInt("1234", 0, 2)).isEqualTo(12); + assertThat(IntParser.parseInt("1234567890", 0, 10)).isEqualTo(1234567890); + assertThat(IntParser.parseInt("1234567890", 1, 10)).isEqualTo(234567890); + assertThat(IntParser.parseInt("0123456789", 0, 10)).isEqualTo(123456789); + assertThat(IntParser.parseInt("00001234", 0, 8)).isEqualTo(1234); + assertThat(IntParser.parseInt("", 0, 0)).isEqualTo(0); + parseInvalidNumber("test", 0, 4); + parseInvalidNumber("123T456", 0, 4); + parseInvalidArgument("", 0, 1); + parseInvalidArgument("1234", 0, 5); + parseInvalidArgument("1234", -1, 4); + parseInvalidArgument("1234", 3, 2); + } + + private void parseInvalidNumber(String input, int begin, int end) { + try { + IntParser.parseInt(input, begin, end); + fail("Expected exception"); + } catch (NumberFormatException e) { + } + } + + private void parseInvalidArgument(String input, int begin, int end) { + try { + IntParser.parseInt(input, begin, end); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + } + } + +} diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index db9ede84dce1..21348fc6a93a 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -18,7 +18,7 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; - +import static org.junit.Assert.fail; import com.google.common.testing.EqualsTester; import java.util.Calendar; import java.util.Date; @@ -183,6 +183,23 @@ public void parseTimestamp() { .isEqualTo(Timestamp.MAX_VALUE); assertThat(Timestamp.parseTimestamp(TEST_TIME_ISO)) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + parseInvalidTimestamp(""); + parseInvalidTimestamp("0001-01-01 00:00:00Z"); + parseInvalidTimestamp("0001-1-1 00:00:00Z"); + parseInvalidTimestamp("0001-01-01T00:00:00.Z"); + parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); + parseInvalidTimestamp("0001-01-01T00:00Z"); + } + + private void parseInvalidTimestamp(String input) { + try { + Timestamp.parseTimestamp(input); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage()).contains("Invalid timestamp"); + } } @Test @@ -192,6 +209,14 @@ public void parseTimestampWithoutTimeZoneOffset() { .isEqualTo(Timestamp.MAX_VALUE); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.123456789").getNanos()).isEqualTo(123456789); + parseInvalidTimestamp("0001-01-01 00:00:00"); + parseInvalidTimestamp("0001-1-1 00:00:00"); + parseInvalidTimestamp("0001-01-01T00:00:00."); + parseInvalidTimestamp("0001-01-01T00:00:00.1234567890"); + parseInvalidTimestamp("0001-01-01T00:00"); } @Test From fee061b8e6399722c3d47aa6023833c2bac54fae Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 30 Mar 2019 20:22:03 +0100 Subject: [PATCH 02/11] fixed formatting --- .../src/main/java/com/google/cloud/Date.java | 4 ++-- .../main/java/com/google/cloud/IntParser.java | 6 ++--- .../main/java/com/google/cloud/Timestamp.java | 24 ++++++++++--------- .../test/java/com/google/cloud/DateTest.java | 1 + .../java/com/google/cloud/IntParserTest.java | 2 +- .../java/com/google/cloud/TimestampTest.java | 6 +++-- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java index 6399626afc36..52a2d5669924 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java @@ -16,11 +16,11 @@ package com.google.cloud; +import com.google.api.core.BetaApi; +import com.google.common.base.Preconditions; import java.io.Serializable; import java.util.Calendar; import java.util.Objects; -import com.google.api.core.BetaApi; -import com.google.common.base.Preconditions; /** Represents a Date without time, such as 2017-03-17. Date is timezone independent. */ @BetaApi("This is going to be replaced with LocalDate from threetenbp") diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java index 700ae6e0ef93..878a28b4c4d7 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java @@ -26,8 +26,9 @@ */ class IntParser { - private static final int[] POWERS_OF_10 = - {1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000}; + private static final int[] POWERS_OF_10 = { + 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000 + }; /** Parses an int from the given input between the specified begin and end. */ static int parseInt(String input, int begin, int end) { @@ -68,5 +69,4 @@ private static int parseDigit(char c, String input) { throw new NumberFormatException("Not a digit: " + c); } } - } diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 453c1a2b4e64..5b2b91157391 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -17,6 +17,9 @@ package com.google.cloud; import static com.google.common.base.Preconditions.checkArgument; + +import com.google.api.client.util.Preconditions; +import com.google.protobuf.util.Timestamps; import java.io.Serializable; import java.util.Date; import java.util.Objects; @@ -24,8 +27,6 @@ import org.threeten.bp.LocalDateTime; import org.threeten.bp.ZoneOffset; import org.threeten.bp.format.DateTimeFormatter; -import com.google.api.client.util.Preconditions; -import com.google.protobuf.util.Timestamps; /** * Represents a timestamp with nanosecond precision. Timestamps cover the range [0001-01-01, @@ -171,16 +172,17 @@ public com.google.protobuf.Timestamp toProto() { public static Timestamp parseTimestamp(String timestamp) { Preconditions.checkNotNull(timestamp); final String invalidTimestamp = "Invalid timestamp: " + timestamp; - Preconditions.checkArgument(timestamp.length() >= 19 && timestamp.length() <= 30, invalidTimestamp); + Preconditions.checkArgument( + timestamp.length() >= 19 && timestamp.length() <= 30, invalidTimestamp); Preconditions.checkArgument(timestamp.charAt(4) == '-', invalidTimestamp); Preconditions.checkArgument(timestamp.charAt(7) == '-', invalidTimestamp); Preconditions.checkArgument(timestamp.charAt(10) == 'T', invalidTimestamp); Preconditions.checkArgument( (timestamp.length() == 19 && timestamp.charAt(18) != 'Z') - || - (timestamp.length() == 20 && timestamp.charAt(19) == 'Z') - || - (timestamp.charAt(19) == '.' && timestamp.length() > 20 && timestamp.charAt(20) != 'Z'), + || (timestamp.length() == 20 && timestamp.charAt(19) == 'Z') + || (timestamp.charAt(19) == '.' + && timestamp.length() > 20 + && timestamp.charAt(20) != 'Z'), invalidTimestamp); int year = IntParser.parseInt(timestamp, 0, 4); int month = IntParser.parseInt(timestamp, 5, 7); @@ -189,15 +191,15 @@ public static Timestamp parseTimestamp(String timestamp) { int minute = IntParser.parseInt(timestamp, 14, 16); int second = IntParser.parseInt(timestamp, 17, 19); int fraction = 0; - if(timestamp.length() > 20) { - if(timestamp.charAt(19) == '.') { + if (timestamp.length() > 20) { + if (timestamp.charAt(19) == '.') { int endIndex; - if(timestamp.charAt(timestamp.length() - 1) == 'Z') { + if (timestamp.charAt(timestamp.length() - 1) == 'Z') { endIndex = timestamp.length() - 1; } else { endIndex = timestamp.length(); } - if(endIndex - 20 > 9) { + if (endIndex - 20 > 9) { throw new IllegalArgumentException(invalidTimestamp); } fraction = IntParser.parseInt(timestamp, 20, endIndex); diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java index ff53e6e8fbce..f9f44bc007e7 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java @@ -19,6 +19,7 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; + import com.google.common.testing.EqualsTester; import java.text.ParseException; import java.text.SimpleDateFormat; diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java index 0dc6d6888fbe..e221b601a5b7 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -59,5 +60,4 @@ private void parseInvalidArgument(String input, int begin, int end) { } catch (IllegalArgumentException e) { } } - } diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 21348fc6a93a..ebf7db06afed 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -19,6 +19,7 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; + import com.google.common.testing.EqualsTester; import java.util.Calendar; import java.util.Date; @@ -184,7 +185,7 @@ public void parseTimestamp() { assertThat(Timestamp.parseTimestamp(TEST_TIME_ISO)) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0Z")) - .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); parseInvalidTimestamp(""); parseInvalidTimestamp("0001-01-01 00:00:00Z"); parseInvalidTimestamp("0001-1-1 00:00:00Z"); @@ -211,7 +212,8 @@ public void parseTimestampWithoutTimeZoneOffset() { .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); - assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.123456789").getNanos()).isEqualTo(123456789); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.123456789").getNanos()) + .isEqualTo(123456789); parseInvalidTimestamp("0001-01-01 00:00:00"); parseInvalidTimestamp("0001-1-1 00:00:00"); parseInvalidTimestamp("0001-01-01T00:00:00."); From 16657ee763082b1a615f9942373b16cfcc43f137 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sun, 31 Mar 2019 13:23:44 +0200 Subject: [PATCH 03/11] fixed parsing nanoseconds from less than 9 digits --- .../main/java/com/google/cloud/IntParser.java | 12 ++++- .../main/java/com/google/cloud/Timestamp.java | 4 +- .../java/com/google/cloud/TimestampTest.java | 53 ++++++++++++++++++- 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java index 878a28b4c4d7..01c2a27c4b77 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java @@ -32,7 +32,17 @@ class IntParser { /** Parses an int from the given input between the specified begin and end. */ static int parseInt(String input, int begin, int end) { + return parseInt(input, begin, end, 0); + } + + /** + * Parses an int from the given input between the specified begin and end. The value is multiplied + * by 10^exponent. The exponent must be between 0 and 10 inclusive. + */ + static int parseInt(String input, int begin, int end, int exponent) { Preconditions.checkNotNull(input); + Preconditions.checkArgument( + exponent >= 0 && exponent < POWERS_OF_10.length, "Exponent out of range"); Preconditions.checkArgument(end - begin <= 10, "Max input length is 10"); Preconditions.checkArgument(end >= begin, "End must be greater or equal to begin"); Preconditions.checkArgument(begin >= 0, "Begin must be >= 0"); @@ -41,7 +51,7 @@ static int parseInt(String input, int begin, int end) { for (int index = begin; index < end; index++) { res += parseDigit(input.charAt(index), input) * POWERS_OF_10[end - index - 1]; } - return res; + return res * POWERS_OF_10[exponent]; } private static int parseDigit(char c, String input) { diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 5b2b91157391..6615ffbf2258 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -202,7 +202,9 @@ public static Timestamp parseTimestamp(String timestamp) { if (endIndex - 20 > 9) { throw new IllegalArgumentException(invalidTimestamp); } - fraction = IntParser.parseInt(timestamp, 20, endIndex); + // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - (endIndex + // - 20)). + fraction = IntParser.parseInt(timestamp, 20, endIndex, 9 - (endIndex - 20)); } else { } } diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index ebf7db06afed..cde3b4151fdd 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -43,6 +43,8 @@ public class TimestampTest { private static final long TEST_TIME_MILLISECONDS_NEGATIVE = -1000L; private static final Date TEST_DATE = new Date(TEST_TIME_MILLISECONDS); private static final Date TEST_DATE_PRE_EPOCH = new Date(TEST_TIME_MILLISECONDS_NEGATIVE); + private static final long TIMESTAMP_SECONDS_MAX = 253402300799L; + private static final long TIMESTAMP_SECONDS_MIN = -62135596800L; @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -180,17 +182,42 @@ public void testToString() { @Test public void parseTimestamp() { assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00Z")).isEqualTo(Timestamp.MIN_VALUE); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.1Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.01Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.100000000Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.010000000Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.000000001Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1)); assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999Z")) .isEqualTo(Timestamp.MAX_VALUE); + assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.99999999Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999_0)); + assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.099999999Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999)); assertThat(Timestamp.parseTimestamp(TEST_TIME_ISO)) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0Z")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); + assertThat(Timestamp.parseTimestamp(Timestamp.ofTimeMicroseconds(20L).toString())) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020000Z")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00002Z")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020Z")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00012340Z")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(0, 123400)); + parseInvalidTimestamp(""); parseInvalidTimestamp("0001-01-01 00:00:00Z"); parseInvalidTimestamp("0001-1-1 00:00:00Z"); parseInvalidTimestamp("0001-01-01T00:00:00.Z"); - parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); + parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); // too long parseInvalidTimestamp("0001-01-01T00:00Z"); } @@ -206,18 +233,40 @@ private void parseInvalidTimestamp(String input) { @Test public void parseTimestampWithoutTimeZoneOffset() { assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00")).isEqualTo(Timestamp.MIN_VALUE); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.1")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.01")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.000000001")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.100000000")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_0000_0000)); + assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.010000000")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MIN, 1_000_0000)); assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.999999999")) .isEqualTo(Timestamp.MAX_VALUE); + assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.99999999")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999_0)); + assertThat(Timestamp.parseTimestamp("9999-12-31T23:59:59.099999999")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TIMESTAMP_SECONDS_MAX, 9999_9999)); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); assertThat(Timestamp.parseTimestamp("2015-10-12T15:14:54.0")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(TEST_TIME_SECONDS, 0)); assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00.123456789").getNanos()) .isEqualTo(123456789); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020000")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00002")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.000020")) + .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); + assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00012340")) + .isEqualTo(Timestamp.ofTimeSecondsAndNanos(0, 123400)); parseInvalidTimestamp("0001-01-01 00:00:00"); parseInvalidTimestamp("0001-1-1 00:00:00"); parseInvalidTimestamp("0001-01-01T00:00:00."); - parseInvalidTimestamp("0001-01-01T00:00:00.1234567890"); + parseInvalidTimestamp("0001-01-01T00:00:00.1234567890"); // too long parseInvalidTimestamp("0001-01-01T00:00"); } From 85aee11619ee10630f74b8a0b8577b8d7af62b0a Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 3 Apr 2019 09:12:08 +0200 Subject: [PATCH 04/11] switch statement is faster than if-statement Parsing the decimal is digit is faster using a switch statement than a number of if statements. The switch statement is also faster than Character.getNumericValue(char), as the latter also takes additional unicode numerical values into account. --- .../main/java/com/google/cloud/IntParser.java | 44 +++++++++---------- .../java/com/google/cloud/IntParserTest.java | 5 +++ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java index 01c2a27c4b77..ac994bed2e81 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java @@ -55,28 +55,28 @@ static int parseInt(String input, int begin, int end, int exponent) { } private static int parseDigit(char c, String input) { - if (c == '0') { - return 0; - } else if (c == '1') { - return 1; - } else if (c == '2') { - return 2; - } else if (c == '3') { - return 3; - } else if (c == '4') { - return 4; - } else if (c == '5') { - return 5; - } else if (c == '6') { - return 6; - } else if (c == '7') { - return 7; - } else if (c == '8') { - return 8; - } else if (c == '9') { - return 9; - } else { - throw new NumberFormatException("Not a digit: " + c); + switch(c) { + case '0': + return 0; + case '1': + return 1; + case '2': + return 2; + case '3': + return 3; + case '4': + return 4; + case '5': + return 5; + case '6': + return 6; + case '7': + return 7; + case '8': + return 8; + case '9': + return 9; } + throw new NumberFormatException("Not a decimal digit: " + c); } } diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java index e221b601a5b7..34cefd26dd32 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java @@ -43,6 +43,11 @@ public void testParse() { parseInvalidArgument("1234", 0, 5); parseInvalidArgument("1234", -1, 4); parseInvalidArgument("1234", 3, 2); + // Roman literal 50 is a valid numeric character, but not a decimal digit. + char c = '\u216C'; + int val = Character.getNumericValue(c); + assertThat(val).isEqualTo(50); + parseInvalidArgument(String.valueOf(c), 0, 1); } private void parseInvalidNumber(String input, int begin, int end) { From acf3c11532342424ae1456ae247beea5fe8d4329 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 3 Apr 2019 16:58:52 +0200 Subject: [PATCH 05/11] fixed formatting --- .../src/main/java/com/google/cloud/IntParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java index ac994bed2e81..e9ecfa8eea7e 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java @@ -55,7 +55,7 @@ static int parseInt(String input, int begin, int end, int exponent) { } private static int parseDigit(char c, String input) { - switch(c) { + switch (c) { case '0': return 0; case '1': From b1feca443384f2323d9ff5cde2274c1b366345b2 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 4 Apr 2019 14:26:06 +0200 Subject: [PATCH 06/11] added additional tests --- .../java/com/google/cloud/TimestampTest.java | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index cde3b4151fdd..76e8f32c47fa 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -31,6 +31,7 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.threeten.bp.DateTimeException; /** Unit tests for {@link com.google.cloud.Timestamp}. */ @RunWith(JUnit4.class) @@ -212,24 +213,52 @@ public void parseTimestamp() { .isEqualTo(Timestamp.ofTimeMicroseconds(20L)); assertThat(Timestamp.parseTimestamp("1970-01-01T00:00:00.00012340Z")) .isEqualTo(Timestamp.ofTimeSecondsAndNanos(0, 123400)); + assertThat(Timestamp.parseTimestamp("2004-02-29T00:00:00Z")).isNotNull(); // normal leap year + assertThat(Timestamp.parseTimestamp("2000-02-29T00:00:00Z")).isNotNull(); // special leap year parseInvalidTimestamp(""); - parseInvalidTimestamp("0001-01-01 00:00:00Z"); - parseInvalidTimestamp("0001-1-1 00:00:00Z"); - parseInvalidTimestamp("0001-01-01T00:00:00.Z"); + parseInvalidTimestamp("TEST"); + parseInvalidTimestamp("0001-01-01 00:00:00Z"); // missing 'T' + parseInvalidTimestamp("0001-1-1 00:00:00Z"); // missing 0 + parseInvalidTimestamp("0001-01-01T00:00:00.Z"); // missing digits after .s parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); // too long - parseInvalidTimestamp("0001-01-01T00:00Z"); + parseInvalidTimestamp("0001-01-01T00:00Z"); // missing seconds + parseInvalidTimestamp("10000-01-01T00:00Z"); // year too long + parseTimestampOutOfRange("0000-01-01T00:00:00.1Z"); + parseValidFormatInvalidDate("0001-00-01T00:00:00.1Z"); // month is zero + parseValidFormatInvalidDate("0001-01-00T00:00:00.1Z"); // day is zero + parseValidFormatInvalidDate("0001-13-01T00:00:00.1Z"); // invalid month + parseValidFormatInvalidDate("0001-01-32T00:00:00.1Z"); // invalid day + parseValidFormatInvalidLeapYear("0001-02-29T00:00:00.1Z"); // not a leap year + parseValidFormatInvalidLeapYear("1900-02-29T00:00:00.1Z"); // not a leap year } - private void parseInvalidTimestamp(String input) { + private void parseInvalid(String input, Class exception, String msg) { try { Timestamp.parseTimestamp(input); fail("Expected exception"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage()).contains("Invalid timestamp"); + } catch (Exception e) { + assertThat(e.getClass()).isEqualTo(exception); + assertThat(e.getMessage()).contains(msg); } } + private void parseInvalidTimestamp(String input) { + parseInvalid(input, IllegalArgumentException.class, "Invalid timestamp"); + } + + private void parseTimestampOutOfRange(String input) { + parseInvalid(input, IllegalArgumentException.class, "timestamp out of range"); + } + + private void parseValidFormatInvalidDate(String input) { + parseInvalid(input, DateTimeException.class, "Invalid value"); + } + + private void parseValidFormatInvalidLeapYear(String input) { + parseInvalid(input, DateTimeException.class, "not a leap year"); + } + @Test public void parseTimestampWithoutTimeZoneOffset() { assertThat(Timestamp.parseTimestamp("0001-01-01T00:00:00")).isEqualTo(Timestamp.MIN_VALUE); From be151f8a7ca73366f6c6dfd80131d55cbef425e1 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 5 Apr 2019 08:15:34 +0200 Subject: [PATCH 07/11] T and Z should be case insensitive --- .../src/main/java/com/google/cloud/Timestamp.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 6615ffbf2258..5693815a7705 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -176,13 +176,15 @@ public static Timestamp parseTimestamp(String timestamp) { timestamp.length() >= 19 && timestamp.length() <= 30, invalidTimestamp); Preconditions.checkArgument(timestamp.charAt(4) == '-', invalidTimestamp); Preconditions.checkArgument(timestamp.charAt(7) == '-', invalidTimestamp); - Preconditions.checkArgument(timestamp.charAt(10) == 'T', invalidTimestamp); Preconditions.checkArgument( - (timestamp.length() == 19 && timestamp.charAt(18) != 'Z') - || (timestamp.length() == 20 && timestamp.charAt(19) == 'Z') + timestamp.charAt(10) == 'T' || timestamp.charAt(10) == 't', invalidTimestamp); + Preconditions.checkArgument( + (timestamp.length() == 19 && (timestamp.charAt(18) != 'Z' || timestamp.charAt(18) == 'z')) + || (timestamp.length() == 20 + && (timestamp.charAt(19) == 'Z' || timestamp.charAt(19) == 'z')) || (timestamp.charAt(19) == '.' && timestamp.length() > 20 - && timestamp.charAt(20) != 'Z'), + && (timestamp.charAt(20) != 'Z' || timestamp.charAt(20) == 'z')), invalidTimestamp); int year = IntParser.parseInt(timestamp, 0, 4); int month = IntParser.parseInt(timestamp, 5, 7); @@ -194,7 +196,8 @@ public static Timestamp parseTimestamp(String timestamp) { if (timestamp.length() > 20) { if (timestamp.charAt(19) == '.') { int endIndex; - if (timestamp.charAt(timestamp.length() - 1) == 'Z') { + if (timestamp.charAt(timestamp.length() - 1) == 'Z' + || timestamp.charAt(timestamp.length() - 1) == 'z') { endIndex = timestamp.length() - 1; } else { endIndex = timestamp.length(); From 2cd776dd8a2dd0fa59b58dc526e40c68602513df Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 6 Apr 2019 08:37:26 +0200 Subject: [PATCH 08/11] removed date parsing (moved to separate PR) --- .../src/main/java/com/google/cloud/Date.java | 19 +++++++++++-------- .../test/java/com/google/cloud/DateTest.java | 15 --------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java index 52a2d5669924..442e7dca0eef 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Date.java @@ -21,11 +21,15 @@ import java.io.Serializable; import java.util.Calendar; import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** Represents a Date without time, such as 2017-03-17. Date is timezone independent. */ @BetaApi("This is going to be replaced with LocalDate from threetenbp") public final class Date implements Comparable, Serializable { + // Date format "yyyy-mm-dd" + private static final Pattern FORMAT_REGEXP = Pattern.compile("(\\d\\d\\d\\d)-(\\d\\d)-(\\d\\d)"); private static final long serialVersionUID = 8067099123096783929L; private final int year; private final int month; @@ -53,14 +57,13 @@ public static Date fromYearMonthDay(int year, int month, int dayOfMonth) { /** @param date Data in RFC 3339 date format (yyyy-mm-dd). */ public static Date parseDate(String date) { - Preconditions.checkNotNull(date); - final String invalidDate = "Invalid date: " + date; - Preconditions.checkArgument(date.length() == 10, invalidDate); - Preconditions.checkArgument(date.charAt(4) == '-', invalidDate); - Preconditions.checkArgument(date.charAt(7) == '-', invalidDate); - int year = IntParser.parseInt(date, 0, 4); - int month = IntParser.parseInt(date, 5, 7); - int dayOfMonth = IntParser.parseInt(date, 8, 10); + Matcher matcher = FORMAT_REGEXP.matcher(date); + if (!matcher.matches()) { + throw new IllegalArgumentException("Invalid date: " + date); + } + int year = Integer.parseInt(matcher.group(1)); + int month = Integer.parseInt(matcher.group(2)); + int dayOfMonth = Integer.parseInt(matcher.group(3)); return new Date(year, month, dayOfMonth); } diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java index f9f44bc007e7..5892cc718b01 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/DateTest.java @@ -18,7 +18,6 @@ import static com.google.common.testing.SerializableTester.reserializeAndAssert; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; import com.google.common.testing.EqualsTester; import java.text.ParseException; @@ -39,20 +38,6 @@ public void parseDate() { assertThat(date.getYear()).isEqualTo(2016); assertThat(date.getMonth()).isEqualTo(9); assertThat(date.getDayOfMonth()).isEqualTo(18); - parseInvalidDate("2016/09/18"); - parseInvalidDate("2016 09 18"); - parseInvalidDate("2016-9-18"); - parseInvalidDate("2016-09-18T10:00"); - parseInvalidDate(""); - } - - private void parseInvalidDate(String input) { - try { - Date.parseDate(input); - fail("Expected exception"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage()).contains("Invalid date"); - } } @Test From 03384a0f9a9cd6634ab6d1d7b5e48622571afeda Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sun, 7 Apr 2019 09:52:17 +0200 Subject: [PATCH 09/11] Integer.parseInt is just as efficient --- .../main/java/com/google/cloud/IntParser.java | 82 ------------------- .../main/java/com/google/cloud/Timestamp.java | 19 +++-- .../java/com/google/cloud/IntParserTest.java | 68 --------------- 3 files changed, 12 insertions(+), 157 deletions(-) delete mode 100644 google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java delete mode 100644 google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java deleted file mode 100644 index e9ecfa8eea7e..000000000000 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/IntParser.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright 2019 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.cloud; - -import com.google.common.base.Preconditions; - -/** - * Util class for fast parsing of integer values. This is used by {@link Date#parseDate(String)} and - * {@link Timestamp#parseTimestamp(String)}. These parse methods are used internally by Google - * client libraries to parse text values returned by services, and these parse methods should be as - * efficient as possible. - */ -class IntParser { - - private static final int[] POWERS_OF_10 = { - 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000 - }; - - /** Parses an int from the given input between the specified begin and end. */ - static int parseInt(String input, int begin, int end) { - return parseInt(input, begin, end, 0); - } - - /** - * Parses an int from the given input between the specified begin and end. The value is multiplied - * by 10^exponent. The exponent must be between 0 and 10 inclusive. - */ - static int parseInt(String input, int begin, int end, int exponent) { - Preconditions.checkNotNull(input); - Preconditions.checkArgument( - exponent >= 0 && exponent < POWERS_OF_10.length, "Exponent out of range"); - Preconditions.checkArgument(end - begin <= 10, "Max input length is 10"); - Preconditions.checkArgument(end >= begin, "End must be greater or equal to begin"); - Preconditions.checkArgument(begin >= 0, "Begin must be >= 0"); - Preconditions.checkArgument(end <= input.length(), "End must be <= input.length()"); - int res = 0; - for (int index = begin; index < end; index++) { - res += parseDigit(input.charAt(index), input) * POWERS_OF_10[end - index - 1]; - } - return res * POWERS_OF_10[exponent]; - } - - private static int parseDigit(char c, String input) { - switch (c) { - case '0': - return 0; - case '1': - return 1; - case '2': - return 2; - case '3': - return 3; - case '4': - return 4; - case '5': - return 5; - case '6': - return 6; - case '7': - return 7; - case '8': - return 8; - case '9': - return 9; - } - throw new NumberFormatException("Not a decimal digit: " + c); - } -} diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 5693815a7705..156543e5a0dc 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -165,6 +165,10 @@ public com.google.protobuf.Timestamp toProto() { return com.google.protobuf.Timestamp.newBuilder().setSeconds(seconds).setNanos(nanos).build(); } + private static final int[] POWERS_OF_10 = { + 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000 + }; + /** * Creates a Timestamp instance from the given string. String is in the RFC 3339 format without * the timezone offset (always ends in "Z"). @@ -186,12 +190,12 @@ public static Timestamp parseTimestamp(String timestamp) { && timestamp.length() > 20 && (timestamp.charAt(20) != 'Z' || timestamp.charAt(20) == 'z')), invalidTimestamp); - int year = IntParser.parseInt(timestamp, 0, 4); - int month = IntParser.parseInt(timestamp, 5, 7); - int day = IntParser.parseInt(timestamp, 8, 10); - int hour = IntParser.parseInt(timestamp, 11, 13); - int minute = IntParser.parseInt(timestamp, 14, 16); - int second = IntParser.parseInt(timestamp, 17, 19); + int year = Integer.parseInt(timestamp.substring(0, 4)); + int month = Integer.parseInt(timestamp.substring(5, 7)); + int day = Integer.parseInt(timestamp.substring(8, 10)); + int hour = Integer.parseInt(timestamp.substring(11, 13)); + int minute = Integer.parseInt(timestamp.substring(14, 16)); + int second = Integer.parseInt(timestamp.substring(17, 19)); int fraction = 0; if (timestamp.length() > 20) { if (timestamp.charAt(19) == '.') { @@ -205,9 +209,10 @@ public static Timestamp parseTimestamp(String timestamp) { if (endIndex - 20 > 9) { throw new IllegalArgumentException(invalidTimestamp); } + fraction = Integer.parseInt(timestamp.substring(20, endIndex)); // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - (endIndex // - 20)). - fraction = IntParser.parseInt(timestamp, 20, endIndex, 9 - (endIndex - 20)); + fraction *= POWERS_OF_10[9 - (endIndex - 20)]; } else { } } diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java deleted file mode 100644 index 34cefd26dd32..000000000000 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/IntParserTest.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright 2019 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.cloud; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class IntParserTest { - - @Test - public void testParse() { - assertThat(IntParser.parseInt("1", 0, 1)).isEqualTo(1); - assertThat(IntParser.parseInt("1234", 0, 4)).isEqualTo(1234); - assertThat(IntParser.parseInt("1234", 2, 4)).isEqualTo(34); - assertThat(IntParser.parseInt("1234", 0, 2)).isEqualTo(12); - assertThat(IntParser.parseInt("1234567890", 0, 10)).isEqualTo(1234567890); - assertThat(IntParser.parseInt("1234567890", 1, 10)).isEqualTo(234567890); - assertThat(IntParser.parseInt("0123456789", 0, 10)).isEqualTo(123456789); - assertThat(IntParser.parseInt("00001234", 0, 8)).isEqualTo(1234); - assertThat(IntParser.parseInt("", 0, 0)).isEqualTo(0); - parseInvalidNumber("test", 0, 4); - parseInvalidNumber("123T456", 0, 4); - parseInvalidArgument("", 0, 1); - parseInvalidArgument("1234", 0, 5); - parseInvalidArgument("1234", -1, 4); - parseInvalidArgument("1234", 3, 2); - // Roman literal 50 is a valid numeric character, but not a decimal digit. - char c = '\u216C'; - int val = Character.getNumericValue(c); - assertThat(val).isEqualTo(50); - parseInvalidArgument(String.valueOf(c), 0, 1); - } - - private void parseInvalidNumber(String input, int begin, int end) { - try { - IntParser.parseInt(input, begin, end); - fail("Expected exception"); - } catch (NumberFormatException e) { - } - } - - private void parseInvalidArgument(String input, int begin, int end) { - try { - IntParser.parseInt(input, begin, end); - fail("Expected exception"); - } catch (IllegalArgumentException e) { - } - } -} From 82a825701bea01fcb45a8f15a9bdce52c84a675e Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 9 Apr 2019 11:41:26 +0200 Subject: [PATCH 10/11] added try-catch for NumberFormatException --- .../main/java/com/google/cloud/Timestamp.java | 53 ++++++++++--------- .../java/com/google/cloud/TimestampTest.java | 9 +++- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 156543e5a0dc..4a61dfc6d084 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -190,34 +190,39 @@ public static Timestamp parseTimestamp(String timestamp) { && timestamp.length() > 20 && (timestamp.charAt(20) != 'Z' || timestamp.charAt(20) == 'z')), invalidTimestamp); - int year = Integer.parseInt(timestamp.substring(0, 4)); - int month = Integer.parseInt(timestamp.substring(5, 7)); - int day = Integer.parseInt(timestamp.substring(8, 10)); - int hour = Integer.parseInt(timestamp.substring(11, 13)); - int minute = Integer.parseInt(timestamp.substring(14, 16)); - int second = Integer.parseInt(timestamp.substring(17, 19)); - int fraction = 0; - if (timestamp.length() > 20) { - if (timestamp.charAt(19) == '.') { - int endIndex; - if (timestamp.charAt(timestamp.length() - 1) == 'Z' - || timestamp.charAt(timestamp.length() - 1) == 'z') { - endIndex = timestamp.length() - 1; + try { + int year = Integer.parseInt(timestamp.substring(0, 4)); + int month = Integer.parseInt(timestamp.substring(5, 7)); + int day = Integer.parseInt(timestamp.substring(8, 10)); + int hour = Integer.parseInt(timestamp.substring(11, 13)); + int minute = Integer.parseInt(timestamp.substring(14, 16)); + int second = Integer.parseInt(timestamp.substring(17, 19)); + int fraction = 0; + if (timestamp.length() > 20) { + if (timestamp.charAt(19) == '.') { + int endIndex; + if (timestamp.charAt(timestamp.length() - 1) == 'Z' + || timestamp.charAt(timestamp.length() - 1) == 'z') { + endIndex = timestamp.length() - 1; + } else { + endIndex = timestamp.length(); + } + if (endIndex - 20 > 9) { + throw new IllegalArgumentException(invalidTimestamp); + } + fraction = Integer.parseInt(timestamp.substring(20, endIndex)); + // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - + // (endIndex + // - 20)). + fraction *= POWERS_OF_10[9 - (endIndex - 20)]; } else { - endIndex = timestamp.length(); } - if (endIndex - 20 > 9) { - throw new IllegalArgumentException(invalidTimestamp); - } - fraction = Integer.parseInt(timestamp.substring(20, endIndex)); - // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - (endIndex - // - 20)). - fraction *= POWERS_OF_10[9 - (endIndex - 20)]; - } else { } + LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction); + return ofTimeSecondsAndNanos(ldt.toEpochSecond(ZoneOffset.UTC), ldt.getNano()); + } catch (NumberFormatException e) { + throw new IllegalArgumentException(invalidTimestamp, e); } - LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction); - return ofTimeSecondsAndNanos(ldt.toEpochSecond(ZoneOffset.UTC), ldt.getNano()); } private StringBuilder toString(StringBuilder b) { diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java index 76e8f32c47fa..37f9fe3fbaf9 100644 --- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java +++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/TimestampTest.java @@ -218,7 +218,14 @@ public void parseTimestamp() { parseInvalidTimestamp(""); parseInvalidTimestamp("TEST"); - parseInvalidTimestamp("0001-01-01 00:00:00Z"); // missing 'T' + parseInvalidTimestamp("aaaa-01-01T00:00:00Z"); + parseInvalidTimestamp("0001-bb-01T00:00:00Z"); + parseInvalidTimestamp("0001-01-ccT00:00:00Z"); + parseInvalidTimestamp("0001-01-01Tdd:00:00Z"); + parseInvalidTimestamp("0001-01-01T00:ee:00Z"); + parseInvalidTimestamp("0001-01-01T00:ee:00Z"); + parseInvalidTimestamp("0001-01-01T00:00:ffZ"); + parseInvalidTimestamp("0001-01-01T00:00:00.123aZ"); parseInvalidTimestamp("0001-1-1 00:00:00Z"); // missing 0 parseInvalidTimestamp("0001-01-01T00:00:00.Z"); // missing digits after .s parseInvalidTimestamp("0001-01-01T00:00:00.1234567890Z"); // too long From 63001ff894aa9f1356cc2abf5316b2f0964b093b Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 12 Apr 2019 20:38:22 +0200 Subject: [PATCH 11/11] removed unnecessary else and put calculation on one line --- .../src/main/java/com/google/cloud/Timestamp.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java index 4a61dfc6d084..06f2c1c5bf33 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/Timestamp.java @@ -200,6 +200,7 @@ public static Timestamp parseTimestamp(String timestamp) { int fraction = 0; if (timestamp.length() > 20) { if (timestamp.charAt(19) == '.') { + // The timestamp contains a fraction. int endIndex; if (timestamp.charAt(timestamp.length() - 1) == 'Z' || timestamp.charAt(timestamp.length() - 1) == 'z') { @@ -211,11 +212,9 @@ public static Timestamp parseTimestamp(String timestamp) { throw new IllegalArgumentException(invalidTimestamp); } fraction = Integer.parseInt(timestamp.substring(20, endIndex)); - // Adjust the result to nanoseconds if the input length is less than 9 digits (9 - - // (endIndex - // - 20)). + // Adjust the result to nanoseconds if the input length is less than 9 digits + // (9 - (endIndex - 20)). fraction *= POWERS_OF_10[9 - (endIndex - 20)]; - } else { } } LocalDateTime ldt = LocalDateTime.of(year, month, day, hour, minute, second, fraction);