diff --git a/google/cloud/spanner/uuid.cc b/google/cloud/spanner/uuid.cc index f14769667bfaa..39261c380b7bf 100644 --- a/google/cloud/spanner/uuid.cc +++ b/google/cloud/spanner/uuid.cc @@ -14,60 +14,18 @@ #include "google/cloud/spanner/uuid.h" #include "google/cloud/internal/make_status.h" +#include "absl/strings/match.h" #include "absl/strings/str_format.h" #include "absl/strings/strip.h" -#include +#include +#include namespace google { namespace cloud { namespace spanner { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN -namespace { -// Helper function to parse a single hexadecimal block of a UUID. -// A hexadecimal block is a 16-digit hexadecimal number, which is represented -// as 8 bytes. -StatusOr ParseHexBlock(absl::string_view& str, - absl::string_view original_str) { - constexpr int kUuidNumberOfHexDigits = 32; - constexpr int kMaxUuidBlockLength = 16; - static auto const* char_to_hex = new std::unordered_map( - {{'0', 0x00}, {'1', 0x01}, {'2', 0x02}, {'3', 0x03}, {'4', 0x04}, - {'5', 0x05}, {'6', 0x06}, {'7', 0x07}, {'8', 0x08}, {'9', 0x09}, - {'a', 0x0a}, {'b', 0x0b}, {'c', 0x0c}, {'d', 0x0d}, {'e', 0x0e}, - {'f', 0x0f}, {'A', 0x0a}, {'B', 0x0b}, {'C', 0x0c}, {'D', 0x0d}, - {'E', 0x0e}, {'F', 0x0f}}); - std::uint64_t block = 0; - for (int j = 0; j < kMaxUuidBlockLength; ++j) { - absl::ConsumePrefix(&str, "-"); - if (str.empty()) { - return internal::InvalidArgumentError( - absl::StrFormat("UUID must contain %d hexadecimal digits: %s", - kUuidNumberOfHexDigits, original_str), - GCP_ERROR_INFO()); - } - auto it = char_to_hex->find(str[0]); - if (it == char_to_hex->end()) { - if (str[0] == '-') { - return internal::InvalidArgumentError( - absl::StrFormat("UUID cannot contain consecutive hyphens: %s", - original_str), - GCP_ERROR_INFO()); - } - - return internal::InvalidArgumentError( - absl::StrFormat("UUID contains invalid character (%c): %s", str[0], - original_str), - GCP_ERROR_INFO()); - } - block = (block << 4) + it->second; - str.remove_prefix(1); - } - return block; -} -} // namespace - -Uuid::Uuid(absl::uint128 value) : uuid_(value) {} +constexpr char kHexDigits[] = "0123456789abcdef"; Uuid::Uuid(std::uint64_t high_bits, std::uint64_t low_bits) : Uuid(absl::MakeUint128(high_bits, low_bits)) {} @@ -76,72 +34,59 @@ std::pair Uuid::As64BitPair() const { return std::make_pair(Uint128High64(uuid_), Uint128Low64(uuid_)); } -// TODO(#15043): Refactor to handle all 128 bits at once instead of splitting -// into a pair of unsigned 64-bit integers. Uuid::operator std::string() const { - constexpr int kUuidStringLen = 36; - constexpr int kChunkLength[] = {8, 4, 4, 4, 12}; - auto to_hex = [](std::uint64_t v, int start_index, int end_index, char* out) { - static constexpr char kHexChar[] = {'0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; - for (int i = start_index; i >= end_index; --i) { - *out++ = kHexChar[(v >> (i * 4)) & 0xf]; - } - return start_index - end_index + 1; - }; - - std::string output; - output.resize(kUuidStringLen); - char* target = const_cast(output.data()); - char* const last = &((output)[output.size()]); - auto bits = Uint128High64(uuid_); - int start = 16; - for (auto length : kChunkLength) { - int end = start - length; - target += to_hex(bits, start - 1, end, target); - // Only hyphens write to valid addresses. - if (target < last) *(target++) = '-'; - if (end == 0) { - start = 16; - bits = Uint128Low64(uuid_); + constexpr char kTemplate[] = "00000000-0000-0000-0000-000000000000"; + char buf[sizeof kTemplate]; + auto uuid = uuid_; + for (auto j = sizeof buf; j-- != 0;) { + if (kTemplate[j] != '0') { + buf[j] = kTemplate[j]; } else { - start = end; + buf[j] = kHexDigits[static_cast(uuid & 0xf)]; + uuid >>= 4; } } - return output; + return buf; } StatusOr MakeUuid(absl::string_view str) { - absl::string_view original_str = str; - // Check and remove optional braces - if (absl::ConsumePrefix(&str, "{")) { - if (!absl::ConsumeSuffix(&str, "}")) { + absl::uint128 uuid = 0; + auto const original_str = str; + if (absl::StartsWith(str, "{") && absl::ConsumeSuffix(&str, "}")) { + str.remove_prefix(1); + } + if (absl::StartsWithIgnoreCase(str, "0x")) { + str.remove_prefix(2); + } + constexpr int kUuidNumberOfHexDigits = 32; + for (int j = 0; j != kUuidNumberOfHexDigits; ++j) { + if (j != 0) absl::ConsumePrefix(&str, "-"); + if (str.empty()) { return internal::InvalidArgumentError( - absl::StrFormat("UUID missing closing '}': %s", original_str), + absl::StrFormat("UUID must contain %d hexadecimal digits: %s", + kUuidNumberOfHexDigits, original_str), GCP_ERROR_INFO()); } + auto const* dp = std::strchr( + kHexDigits, std::tolower(static_cast(str.front()))); + if (dp == nullptr) { + return internal::InvalidArgumentError( + absl::StrFormat( + "UUID contains invalid character '%c' at position %d: %s", + str.front(), str.data() - original_str.data(), original_str), + GCP_ERROR_INFO()); + } + uuid <<= 4; + uuid += dp - kHexDigits; + str.remove_prefix(1); } - - // Check for leading hyphen after stripping any surrounding braces. - if (absl::StartsWith(str, "-")) { - return internal::InvalidArgumentError( - absl::StrFormat("UUID cannot begin with '-': %s", original_str), - GCP_ERROR_INFO()); - } - - // TODO(#15043): Refactor to parse all the bits at once. - auto high_bits = ParseHexBlock(str, original_str); - if (!high_bits.ok()) return std::move(high_bits).status(); - auto low_bits = ParseHexBlock(str, original_str); - if (!low_bits.ok()) return std::move(low_bits).status(); - if (!str.empty()) { return internal::InvalidArgumentError( - absl::StrFormat("Extra characters found after parsing UUID: %s", str), + absl::StrFormat("Extra characters \"%s\" found after parsing UUID: %s", + str, original_str), GCP_ERROR_INFO()); } - - return Uuid(absl::MakeUint128(*high_bits, *low_bits)); + return Uuid{uuid}; } GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/spanner/uuid.h b/google/cloud/spanner/uuid.h index 59d6c0013228b..c445ef2205b70 100644 --- a/google/cloud/spanner/uuid.h +++ b/google/cloud/spanner/uuid.h @@ -20,7 +20,7 @@ #include "absl/numeric/int128.h" #include "absl/strings/string_view.h" #include -#include +#include #include #include @@ -30,7 +30,7 @@ namespace spanner { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN /** - * A representation of the Spanner UUID type: A fixed size 16 byte value + * A representation of the Spanner UUID type: A 16-byte value * that can be represented as a 32-digit hexadecimal string. * * @see https://cloud.google.com/spanner/docs/data-types#uuid_type @@ -41,9 +41,9 @@ class Uuid { Uuid() = default; /// Construct a UUID from one unsigned 128-bit integer. - explicit Uuid(absl::uint128 value); + explicit Uuid(absl::uint128 uuid) : uuid_(uuid) {} - /// Construct a UUID from two unsigned 64-bit pieces. + /// Construct a UUID from two unsigned 64-bit integers. Uuid(std::uint64_t high_bits, std::uint64_t low_bits); /// @name Regular value type, supporting copy, assign, move. @@ -75,15 +75,13 @@ class Uuid { friend bool operator>(Uuid const& lhs, Uuid const& rhs) { return rhs < lhs; } ///@} - /// @name Returns a pair of unsigned 64-bit integers representing the UUID. - std::pair As64BitPair() const; - - /// @name Conversion to unsigned 128-bit integer representation. + /// @name Conversion to one 128-bit unsigned integer. explicit operator absl::uint128() const { return uuid_; } - /// @name Conversion to a lower case string formatted as: - /// [8 hex-digits]-[4 hex-digits]-[4 hex-digits]-[4 hex-digits]-[12 - /// hex-digits] + /// @name Conversion to two unsigned 64-bit integers. + std::pair As64BitPair() const; + + /// @name Conversion to an 8-4-4-4-12 format (lower-case) string. /// Example: 0b6ed04c-a16d-fc46-5281-7f9978c13738 explicit operator std::string() const; diff --git a/google/cloud/spanner/uuid_test.cc b/google/cloud/spanner/uuid_test.cc index d2513134c448e..c1b732e16a2b9 100644 --- a/google/cloud/spanner/uuid_test.cc +++ b/google/cloud/spanner/uuid_test.cc @@ -15,6 +15,7 @@ #include "google/cloud/spanner/uuid.h" #include "google/cloud/testing_util/status_matchers.h" #include +#include namespace google { namespace cloud { @@ -25,7 +26,8 @@ namespace { using ::google::cloud::testing_util::IsOkAndHolds; using ::google::cloud::testing_util::StatusIs; using ::testing::Eq; -using ::testing::HasSubstr; +using ::testing::Pair; +using ::testing::StartsWith; TEST(UuidTest, DefaultConstruction) { Uuid uuid1; @@ -44,10 +46,8 @@ TEST(UuidTest, ConstructedFromUint128) { TEST(UuidTest, ConstructedFromUint64Pieces) { std::uint64_t high = 0x7bf8a7b819171919; std::uint64_t low = 0x2625f208c5824254; - - absl::uint128 expected_value = absl::MakeUint128(high, low); Uuid uuid{high, low}; - EXPECT_THAT(static_cast(uuid), Eq(expected_value)); + EXPECT_THAT(uuid.As64BitPair(), Pair(high, low)); } TEST(UuidTest, RelationalOperations) { @@ -68,123 +68,90 @@ TEST(UuidTest, RelationalOperations) { TEST(UuidTest, ConversionToString) { Uuid uuid0; - Uuid uuid1{absl::MakeUint128(0x0b6ed04ca16dfc46, 0x52817f9978c13738)}; - Uuid uuid2{absl::MakeUint128(0x7bf8a7b819171919, 0x2625f208c5824254)}; - EXPECT_THAT(static_cast(uuid0), Eq("00000000-0000-0000-0000-000000000000")); + + Uuid uuid1{absl::MakeUint128(0x0b6ed04ca16dfc46, 0x52817f9978c13738)}; EXPECT_THAT(static_cast(uuid1), Eq("0b6ed04c-a16d-fc46-5281-7f9978c13738")); + + Uuid uuid2{absl::MakeUint128(0x7bf8a7b819171919, 0x2625f208c5824254)}; EXPECT_THAT(static_cast(uuid2), Eq("7bf8a7b8-1917-1919-2625-f208c5824254")); } +TEST(UuidTest, RoundTripStringRepresentation) { + Uuid expected_uuid{absl::MakeUint128(0x7bf8a7b819171919, 0x2625f208c5824254)}; + auto actual_uuid = MakeUuid(static_cast(expected_uuid)); + EXPECT_THAT(actual_uuid, IsOkAndHolds(expected_uuid)); +} + TEST(UuidTest, ostream) { - Uuid uuid1{absl::MakeUint128(0x0b6ed04ca16dfc46, 0x52817f9978c13738)}; std::stringstream ss; - ss << uuid1; + ss << Uuid{absl::MakeUint128(0x0b6ed04ca16dfc46, 0x52817f9978c13738)}; EXPECT_THAT(ss.str(), Eq("0b6ed04c-a16d-fc46-5281-7f9978c13738")); } -struct MakeUuidTestParam { - std::string name; - std::string input; - uint64_t expected_high_bits; - uint64_t expected_low_bits; - absl::optional error; -}; -class MakeUuidTest : public ::testing::TestWithParam {}; - -TEST_P(MakeUuidTest, MakeUuidFromString) { - Uuid expected_uuid{absl::MakeUint128(GetParam().expected_high_bits, - GetParam().expected_low_bits)}; - StatusOr actual_uuid = MakeUuid(GetParam().input); - if (GetParam().error.has_value()) { - EXPECT_THAT(actual_uuid.status(), - StatusIs(GetParam().error->code(), - HasSubstr(GetParam().error->message()))); - } else { - EXPECT_THAT(actual_uuid, IsOkAndHolds(expected_uuid)); - } -} - -INSTANTIATE_TEST_SUITE_P( - MakeUuid, MakeUuidTest, - testing::Values( - // Error Paths - MakeUuidTestParam{"Empty", "", 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "UUID must contain 32 hexadecimal digits"}}, - MakeUuidTestParam{"EmptyCurlyBraces", "{}", 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "UUID must contain 32 hexadecimal digits"}}, - MakeUuidTestParam{ - "MissingClosingCurlyBrace", "{0b6ed04ca16dfc4652817f9978c13738", - 0x0, 0x0, - Status{ - StatusCode::kInvalidArgument, - "UUID missing closing '}': {0b6ed04ca16dfc4652817f9978c13738"}}, - MakeUuidTestParam{ - "MissingOpeningCurlyBrace", "0b6ed04ca16dfc4652817f9978c13738}", - 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "Extra characters found after parsing UUID: }"}}, - MakeUuidTestParam{"StartsWithInvalidHyphen", - "-0b6ed04ca16dfc4652817f9978c13738", 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "UUID cannot begin with '-':"}}, - MakeUuidTestParam{"ContainsInvalidCharacter", - "0b6ed04ca16dfc4652817f9978c1373g", 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "UUID contains invalid character (g):"}}, - MakeUuidTestParam{"ContainsConsecutiveHyphens", - "0b--6ed04ca16dfc4652817f9978c13738", 0x0, 0x0, - Status{StatusCode::kInvalidArgument, - "UUID cannot contain consecutive hyphens: " - "0b--6ed04ca16dfc4652817f9978c13738"}}, - MakeUuidTestParam{"InsufficientDigits", - "00-00-00-00-00-00-00-00-00-00-00-00-00-00-00", 0x0, - 0x0, - Status{StatusCode::kInvalidArgument, - "UUID must contain 32 hexadecimal digits:"}}, - // Success Paths - MakeUuidTestParam{ - "Zero", "00000000000000000000000000000000", 0x0, 0x0, {}}, - MakeUuidTestParam{"CurlyBraced", - "{0b6ed04ca16dfc4652817f9978c13738}", - 0x0b6ed04ca16dfc46, - 0x52817f9978c13738, - {}}, - MakeUuidTestParam{ - "CurlyBracedFullyHyphenated", - "{0-b-6-e-d-0-4-c-a-1-6-d-f-c-4-6-5-2-8-1-7-f-9-9-7-8-c-1-3-7-3-8}", - 0x0b6ed04ca16dfc46, - 0x52817f9978c13738, - {}}, - MakeUuidTestParam{"CurlyBracedConventionallyHyphenated", - "{0b6ed04c-a16d-fc46-5281-7f9978c13738}", - 0x0b6ed04ca16dfc46, - 0x52817f9978c13738, - {}}, - MakeUuidTestParam{"MixedCase16BitHyphenSeparated", - "7Bf8-A7b8-1917-1919-2625-F208-c582-4254", - 0x7Bf8A7b819171919, - 0x2625F208c5824254, - {}}, - MakeUuidTestParam{"AllUpperCase", - "{DECAFBAD-DEAD-FADE-CAFE-FEEDFACEBEEF}", - 0xdecafbaddeadfade, - 0xcafefeedfacebeef, - {}}), - [](testing::TestParamInfo const& info) { - return info.param.name; - }); - -TEST(UuidTest, RoundTripStringRepresentation) { - Uuid expected_uuid{absl::MakeUint128(0x7bf8a7b819171919, 0x2625f208c5824254)}; - auto str = static_cast(expected_uuid); - auto actual_uuid = MakeUuid(str); - EXPECT_THAT(actual_uuid, IsOkAndHolds(expected_uuid)); +TEST(UuidTest, MakeUuid) { + // Error Paths + EXPECT_THAT( // + MakeUuid(""), + StatusIs( // + StatusCode::kInvalidArgument, + StartsWith("UUID must contain 32 hexadecimal digits:"))); + EXPECT_THAT( + MakeUuid("{0b6ed04ca16dfc4652817f9978c13738"), + StatusIs( + StatusCode::kInvalidArgument, + StartsWith("UUID contains invalid character '{' at position 0:"))); + EXPECT_THAT( + MakeUuid("0b6ed04ca16dfc4652817f9978c13738}"), + StatusIs( // + StatusCode::kInvalidArgument, + StartsWith("Extra characters \"}\" found after parsing UUID:"))); + EXPECT_THAT( + MakeUuid("-0b6ed04ca16dfc4652817f9978c13738"), + StatusIs( + StatusCode::kInvalidArgument, + StartsWith("UUID contains invalid character '-' at position 0:"))); + EXPECT_THAT( + MakeUuid("0b6ed04ca16dfc4652817f9978c1373g"), + StatusIs( + StatusCode::kInvalidArgument, + StartsWith("UUID contains invalid character 'g' at position 31:"))); + EXPECT_THAT( + MakeUuid("0b--6ed04ca16dfc4652817f9978c13738"), + StatusIs( + StatusCode::kInvalidArgument, + StartsWith("UUID contains invalid character '-' at position 3:"))); + EXPECT_THAT( // + MakeUuid("00-00-00-00-00-00-00-00-00-00-00-00-00-00-00"), + StatusIs( // + StatusCode::kInvalidArgument, + StartsWith("UUID must contain 32 hexadecimal digits:"))); + EXPECT_THAT( + MakeUuid("{00000000-0000-0000-0000-000000000000-aa}"), + StatusIs( + StatusCode::kInvalidArgument, + StartsWith("Extra characters \"-aa\" found after parsing UUID:"))); + + // Success Paths + EXPECT_THAT(MakeUuid("00000000000000000000000000000000"), + IsOkAndHolds(Uuid{})); + EXPECT_THAT(MakeUuid("0x0b6ed04ca16dfc4652817f9978c13738"), + IsOkAndHolds(Uuid{0x0b6ed04ca16dfc46, 0x52817f9978c13738})); + EXPECT_THAT(MakeUuid("{0b6ed04ca16dfc4652817f9978c13738}"), + IsOkAndHolds(Uuid{0x0b6ed04ca16dfc46, 0x52817f9978c13738})); + EXPECT_THAT(MakeUuid("{0b6ed04c-a16d-fc46-5281-7f9978c13738}"), + IsOkAndHolds(Uuid{0x0b6ed04ca16dfc46, 0x52817f9978c13738})); + EXPECT_THAT(MakeUuid("7Bf8-A7b8-1917-1919-2625-F208-c582-4254"), + IsOkAndHolds(Uuid{0x7Bf8A7b819171919, 0x2625F208c5824254})); + EXPECT_THAT(MakeUuid("{DECAFBAD-DEAD-FADE-CAFE-FEEDFACEBEEF}"), + IsOkAndHolds(Uuid{0xdecafbaddeadfade, 0xcafefeedfacebeef})); + EXPECT_THAT( + MakeUuid( + "{0-b-6-e-d-0-4-c-a-1-6-d-f-c-4-6-5-2-8-1-7-f-9-9-7-8-c-1-3-7-3-8}"), + IsOkAndHolds(Uuid{0x0b6ed04ca16dfc46, 0x52817f9978c13738})); } } // namespace