THRIFT-5773 Strong UUID wrapper for C++

Client: cpp/CMakeLists.txt
Patch: Carel Combrink

This closes #2958

LANGUAGES.md

@@ -80,7 +80,7 @@ Thrift's core protocol is TBinary, supported by all languages except for JavaScr
 <!-- Since -----------------><td>0.2.0</td>
 <!-- Build Systems ---------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Language Levels -------><td colspan=2>C++11</td>
-<!-- Field types -----------><td><img src="/doc/images/cred.png" alt=""/></td>
+<!-- Field types -----------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Low-Level Transports --><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Transport Wrappers ----><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Protocols -------------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>

compiler/cpp/src/thrift/generate/t_cpp_generator.cc

@@ -4572,8 +4572,7 @@ string t_cpp_generator::base_type_name(t_base_type::t_base tbase) {
   case t_base_type::TYPE_DOUBLE:
     return "double";
   case t_base_type::TYPE_UUID:
-    // TODO: discuss possibility of a class TUuid;
-    return "std::string";
+    return "apache::thrift::TUuid";
   default:
     throw "compiler error: no C++ base type name for base type " + t_base_type::t_base_name(tbase);
   }
@@ -4614,9 +4613,7 @@ string t_cpp_generator::declare_field(t_field* tfield,
       switch (tbase) {
       case t_base_type::TYPE_VOID:
       case t_base_type::TYPE_STRING:
-        break;
       case t_base_type::TYPE_UUID:
-        result += " = std::string(\"00000000-0000-0000-0000-000000000000\")";
         break;
       case t_base_type::TYPE_BOOL:
         result += " = false";
@@ -4784,10 +4781,10 @@ bool t_cpp_generator::is_struct_storage_not_throwing(t_struct* tstruct) const {
       case t_base_type::TYPE_I32:
       case t_base_type::TYPE_I64:
       case t_base_type::TYPE_DOUBLE:
+      case t_base_type::TYPE_UUID:
         continue;
       case t_base_type::TYPE_VOID:
       case t_base_type::TYPE_STRING:
-      case t_base_type::TYPE_UUID:
       default:
         return false;
     }

lib/c_glib/test/CMakeLists.txt

@@ -231,7 +231,7 @@ add_custom_command(OUTPUT
     gen-cpp/ThriftTest.h
     gen-cpp/ThriftTest_constants.h
     gen-cpp/ThriftTest_types.h
-    COMMAND ${THRIFT_COMPILER} --gen cpp ${PROJECT_SOURCE_DIR}/test/v0.16/ThriftTest.thrift
+    COMMAND ${THRIFT_COMPILER} --gen cpp ${PROJECT_SOURCE_DIR}/test/ThriftTest.thrift
 )
 
 # TODO: Add memory checks using ctest_memcheck or similar

lib/c_glib/test/Makefile.am

@@ -335,7 +335,7 @@ gen-c_glib/t_test_optional_required_test_types.c gen-c_glib/t_test_optional_requ
 gen-c_glib/t_test_second_service.c gen-c_glib/t_test_thrift_test.c gen-c_glib/t_test_thrift_test_types.c gen-c_glib/t_test_second_service.h gen-c_glib/t_test_thrift_test.h gen-c_glib/t_test_thrift_test_types.h: ../../../test/v0.16/ThriftTest.thrift $(THRIFT)
 	$(THRIFT) --gen c_glib $<
 
-gen-cpp/ThriftTest.cpp gen-cpp/ThriftTest_constants.cpp gen-cpp/ThriftTest_types.cpp: ../../../test/v0.16/ThriftTest.thrift $(THRIFT)
+gen-cpp/ThriftTest.cpp gen-cpp/ThriftTest_constants.cpp gen-cpp/ThriftTest_types.cpp: ../../../test/ThriftTest.thrift $(THRIFT)
 	$(THRIFT) --gen cpp $<
 
 TESTS = \

lib/c_glib/test/testthrifttestclient.cpp

@@ -112,6 +112,11 @@ class TestHandler : public ThriftTestIf {
     out = thing;
   }
 
+  std::string testUuid(const std::string thing) override {
+    cout << "[C -> C++] testUuid(\"" << std::hex << thing << "\")" << '\n';
+    return thing;
+  }
+
   void testStruct(Xtruct& out, const Xtruct &thing) override {
     cout << "[C -> C++] testStruct({\"" << thing.string_thing << "\", " << (int)thing.byte_thing << ", " << thing.i32_thing << ", " << thing.i64_thing << "})" << '\n';
     out = thing;

lib/c_glib/test/testthrifttestzlibclient.cpp

@@ -107,6 +107,11 @@ class TestHandler : public ThriftTestIf {
     out = thing;
   }
 
+  std::string testUuid(const std::string thing) override {
+    cout << "[C -> C++] testUuid(\"" << std::hex << thing << "\")" << '\n';
+    return thing;
+  }
+
   void testStruct(Xtruct& out, const Xtruct &thing) override {
     cout << "[C -> C++] testStruct({\"" << thing.string_thing << "\", " << (int)thing.byte_thing << ", " << thing.i32_thing << ", " << thing.i64_thing << "})" << '\n';
     out = thing;
@@ -190,7 +195,8 @@ class TestHandler : public ThriftTestIf {
 
   UserId testTypedef(const UserId thing) override {
     cout << "[C -> C++] testTypedef(" << thing << ")" << '\n';
-    return thing;  }
+    return thing;
+  }
 
   void testMapMap(map<int32_t, map<int32_t,int32_t> > &mapmap, const int32_t hello) override {
     cout << "[C -> C++] testMapMap(" << hello << ")" << '\n';

lib/cpp/CMakeLists.txt

@@ -31,6 +31,7 @@ endif()
 set(thriftcpp_SOURCES
    src/thrift/TApplicationException.cpp
    src/thrift/TOutput.cpp
+   src/thrift/TUuid.cpp
    src/thrift/async/TAsyncChannel.cpp
    src/thrift/async/TAsyncProtocolProcessor.cpp
    src/thrift/async/TConcurrentClientSyncInfo.h
@@ -43,7 +44,6 @@ set(thriftcpp_SOURCES
    src/thrift/protocol/TJSONProtocol.cpp
    src/thrift/protocol/TMultiplexedProtocol.cpp
    src/thrift/protocol/TProtocol.cpp
-   src/thrift/protocol/TUuidUtils.cpp
    src/thrift/transport/TTransportException.cpp
    src/thrift/transport/TFDTransport.cpp
    src/thrift/transport/TSimpleFileTransport.cpp

lib/cpp/Makefile.am

@@ -57,6 +57,7 @@ AM_CPPFLAGS = $(BOOST_CPPFLAGS) $(OPENSSL_INCLUDES) -I$(srcdir)/src -D__STDC_FOR
 
 libthrift_la_SOURCES = src/thrift/TApplicationException.cpp \
                        src/thrift/TOutput.cpp \
+                       src/thrift/TUuid.cpp \
                        src/thrift/VirtualProfiling.cpp \
                        src/thrift/async/TAsyncChannel.cpp \
                        src/thrift/async/TAsyncProtocolProcessor.cpp \
@@ -69,7 +70,6 @@ libthrift_la_SOURCES = src/thrift/TApplicationException.cpp \
                        src/thrift/protocol/TBase64Utils.cpp \
                        src/thrift/protocol/TMultiplexedProtocol.cpp \
                        src/thrift/protocol/TProtocol.cpp \
-                       src/thrift/protocol/TUuidUtils.cpp \
                        src/thrift/transport/TTransportException.cpp \
                        src/thrift/transport/TFDTransport.cpp \
                        src/thrift/transport/TFileTransport.cpp \
@@ -137,6 +137,7 @@ include_thrift_HEADERS = \
                          src/thrift/thrift-config.h \
                          src/thrift/thrift_export.h \
                          src/thrift/TDispatchProcessor.h \
+                         src/thrift/TUuid.h \
                          src/thrift/Thrift.h \
                          src/thrift/TOutput.h \
                          src/thrift/TProcessor.h \

lib/cpp/README.md

@@ -232,6 +232,32 @@ OpenSSL's RAND_poll() when OpenSSL library is first initialized.
 The PRNG seed is key to the application security. This method should be
 overridden if it's not strong enough for you.
 
+# Thrift UUID
+
+The `uuid` `BaseType` is implemented in C++ by the `apache::thrift::TUuid` class. This class
+is a strong wrapper class around an internal buffer of 16 bytes.
+
+The `apache::thrift::TUuid` supports construction from different UUID string representations.
+Some examples of supported string formats are:
+
+* `"hhhhhhhh-hhhh-hhhh-hhhh-hhhhhhhhhhhh"`
+* `"{hhhhhhhh-hhhh-hhhh-hhhh-hhhhhhhhhhhh}"`
+* `"hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh"`
+* `"{hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh}"`
+
+## `TUuid` and `boost::uuids::uuid`
+
+Internally the TUuid class is implemented using the `boost::uuids::uuid` library. As a result the TUuid
+can seamlessly interoperate with the boost UUID type since the underlying data structure is the same.
+
+For convenience, when boost is already used by a project the `THRIFT_TUUID_SUPPORT_BOOST_UUID` preprocessor
+directive can be set when including the thrift library to enable construction of a `TUuid` from a
+`boost::uuids::uuid`. By default this is an implicit constructor that can be changed to be explicit
+by defining the `THRIFT_TUUID_BOOST_CONSTRUCTOR_EXPLICIT` preprocessor directive.
+
+The thrift library does not need to be compiled differently when this constructor is needed. The preprocessor
+directives can be set on the project that uses the thrift library.
+
 # Deprecations
 
 ## 0.12.0

lib/cpp/libthrift.vcxproj

@@ -49,7 +49,6 @@
     <ClCompile Include="src\thrift\protocol\TJSONProtocol.cpp" />
     <ClCompile Include="src\thrift\protocol\TMultiplexedProtocol.cpp" />
     <ClCompile Include="src\thrift\protocol\TProtocol.cpp" />
-    <ClCompile Include="src\thrift\protocol\TUuidUtils.cpp" />
     <ClCompile Include="src\thrift\server\TConnectedClient.cpp" />
     <ClCompile Include="src\thrift\server\TServer.cpp" />
     <ClCompile Include="src\thrift\server\TServerFramework.cpp" />
@@ -58,6 +57,7 @@
     <ClCompile Include="src\thrift\server\TThreadPoolServer.cpp" />
     <ClCompile Include="src\thrift\TApplicationException.cpp" />
     <ClCompile Include="src\thrift\TOutput.cpp" />
+    <ClCompile Include="src\thrift\TUuid.cpp" />
     <ClCompile Include="src\thrift\transport\SocketCommon.cpp" />
     <ClCompile Include="src\thrift\transport\TBufferTransports.cpp" />
     <ClCompile Include="src\thrift\transport\TFDTransport.cpp" />
@@ -97,6 +97,7 @@
     <ClInclude Include="src\thrift\Thrift.h" />
     <ClInclude Include="src\thrift\TOutput.h" />
     <ClInclude Include="src\thrift\TProcessor.h" />
+    <ClInclude Include="src\thrift\TUuid.h" />
     <ClInclude Include="src\thrift\transport\TBufferTransports.h" />
     <ClInclude Include="src\thrift\transport\TFDTransport.h" />
     <ClInclude Include="src\thrift\transport\TFileTransport.h" />

lib/cpp/libthrift.vcxproj.filters

@@ -4,6 +4,7 @@
     <ClCompile Include="src\thrift\transport\TBufferTransports.cpp">
       <Filter>transport</Filter>
     </ClCompile>
+    <ClCompile Include="src\thrift\TUuid.cpp" />
     <ClCompile Include="src\thrift\TOutput.cpp" />
     <ClCompile Include="src\thrift\TApplicationException.cpp" />
     <ClCompile Include="src\thrift\transport\TTransportException.cpp">
@@ -30,9 +31,6 @@
     <ClCompile Include="src\thrift\protocol\TMultiplexedProtocol.cpp">
       <Filter>protocol</Filter>
     </ClCompile>
-    <ClCompile Include="src\thrift\protocol\TUuidUtils.cpp">
-      <Filter>protocol</Filter>
-    </ClCompile>
     <ClCompile Include="src\thrift\transport\TFDTransport.cpp">
       <Filter>transport</Filter>
     </ClCompile>
@@ -109,6 +107,7 @@
     <ClInclude Include="src\thrift\protocol\TBinaryProtocol.h">
       <Filter>protocol</Filter>
     </ClInclude>
+    <ClInclude Include="src\thrift\TUuid.h" />
     <ClInclude Include="src\thrift\Thrift.h" />
     <ClInclude Include="src\thrift\TProcessor.h" />
     <ClInclude Include="src\thrift\TApplicationException.h" />

lib/cpp/src/thrift/protocol/TUuidUtils.cpp → lib/cpp/src/thrift/TUuid.cpp

@@ -17,40 +17,45 @@
  * under the License.
  */
 
-#include <thrift/protocol/TUuidUtils.hpp>
+#include <thrift/TUuid.h>
 
 #include <boost/uuid/string_generator.hpp>
 #include <boost/uuid/uuid.hpp>
 #include <boost/uuid/uuid_io.hpp>
 
 namespace apache {
 namespace thrift {
-namespace protocol {
-
-bool uuid_encode(const std::string& in, std::string& out) {
-  static const boost::uuids::string_generator gen;
-  static const std::string empty_uuid(boost::uuids::uuid::static_size(), '\0');
-  out = empty_uuid;
-  if (in.empty()) {
-    return true;
+
+namespace {
+static const boost::uuids::string_generator gen;
+}
+
+TUuid::TUuid(const std::string& str) noexcept {
+  std::fill(this->begin(), this->end(), 0);
+  if (str.empty()) {
+    return ;
   }
+
   try {
-    const boost::uuids::uuid uuid{gen(in)};
-    std::copy(uuid.begin(), uuid.end(), out.begin());
-    return true;
+    const boost::uuids::uuid uuid{gen(str)};
+    std::copy(uuid.begin(), uuid.end(), this->begin());
   } catch (const std::runtime_error&) {
     // Invalid string most probably
-    return false;
   }
 }
 
-void uuid_decode(const std::string& in, std::string& out) {
-  boost::uuids::uuid uuid{};
-  const size_t to_copy = std::min(in.size(), uuid.size());
-  std::copy(in.begin(), in.begin() + to_copy, uuid.begin());
-  out = boost::uuids::to_string(uuid);
+bool TUuid::is_nil() const noexcept {
+  boost::uuids::uuid uuid_tmp{};
+  std::copy(this->begin(), this->end(), std::begin(uuid_tmp));
+  return uuid_tmp.is_nil();
 }
 
+std::string to_string(const TUuid& in) {
+  boost::uuids::uuid uuid_tmp{};
+  std::copy(std::begin(in), std::end(in), std::begin(uuid_tmp));
+  return boost::uuids::to_string(uuid_tmp);
 }
+
+
 }
-} // apache::thrift::protocol
+} // apache::thrift