Addressing code review comments and suggestions

CHANGELOG.md

@@ -10,7 +10,7 @@
 - MinGW (experimental, source only)
 
 ---
-## Version 3.2.87.1 (03/29/2019)
+## Version 3.2.91.1 (04/01/2019)
 
 ### Design Changes
 

Solutions/MSTelemetrySDK.sln

@@ -121,10 +121,12 @@ Global
 	GlobalSection(SharedMSBuildProjectFiles) = preSolution
 		..\lib\pal\universal\universal.vcxitems*{10e9165b-49d1-4d1c-8248-334b9905b9cc}*SharedItemsImports = 9
 		..\lib\pal\desktop\desktop.vcxitems*{10e9165b-49d1-4d1c-8248-334b9905b9cd}*SharedItemsImports = 9
+		..\lib\modules\exp\exp.vcxitems*{1dc6b38a-b390-34ce-907f-4958807a3d42}*SharedItemsImports = 4
 		..\lib\pal\desktop\desktop.vcxitems*{1dc6b38a-b390-34ce-907f-4958807a3d42}*SharedItemsImports = 4
 		Clienttelemetry\Clienttelemetry.vcxitems*{1dc6b38a-b390-34ce-907f-4958807a3d42}*SharedItemsImports = 4
 		..\lib\pal\desktop\desktop.vcxitems*{1dc6b38a-b390-34ce-907f-4958807a3d43}*SharedItemsImports = 4
 		Clienttelemetry\Clienttelemetry.vcxitems*{1dc6b38a-b390-34ce-907f-4958807a3d43}*SharedItemsImports = 4
+		..\lib\modules\exp\exp.vcxitems*{216a8e97-21f7-4bef-9e52-7f772c177c32}*SharedItemsImports = 4
 		..\lib\pal\desktop\desktop.vcxitems*{216a8e97-21f7-4bef-9e52-7f772c177c32}*SharedItemsImports = 4
 		Clienttelemetry\Clienttelemetry.vcxitems*{216a8e97-21f7-4bef-9e52-7f772c177c32}*SharedItemsImports = 4
 		..\lib\pal\desktop\desktop.vcxitems*{216a8e97-21f7-4bef-9e52-7f772c177c33}*SharedItemsImports = 4
@@ -142,6 +144,7 @@ Global
 		..\lib\pal\universal\universal.vcxitems*{57a81ed9-5603-471f-afee-9656da74178d}*SharedItemsImports = 4
 		Clienttelemetry\Clienttelemetry.vcxitems*{57a81ed9-5603-471f-afee-9656da74178d}*SharedItemsImports = 4
 		..\lib\decoder\decoder.vcxitems*{65b6880e-fc36-443d-a7a5-0e6cda6523ac}*SharedItemsImports = 9
+		..\lib\modules\exp\exp.vcxitems*{7a75748d-5d6b-48a5-83cb-f5f439133d59}*SharedItemsImports = 4
 		..\lib\pal\universal\universal.vcxitems*{7a75748d-5d6b-48a5-83cb-f5f439133d59}*SharedItemsImports = 4
 		Clienttelemetry\Clienttelemetry.vcxitems*{7a75748d-5d6b-48a5-83cb-f5f439133d59}*SharedItemsImports = 4
 		..\lib\shared\Shared.vcxitems*{dc91621e-a203-42df-8e03-3a23dd0602b1}*SharedItemsImports = 4

Solutions/win10-cs/win10-cs.vcxproj

@@ -108,7 +108,7 @@
     <Import Project="..\Clienttelemetry\Clienttelemetry.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\pal\universal\universal.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\shared\Shared.vcxitems" Label="Shared" />
-    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Label="Shared" />
+    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Condition="exists('..\..\lib\modules\exp\exp.vcxitems')" Label="Shared" />
   </ImportGroup>
   <ImportGroup Label="PropertySheets">
     <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />

Solutions/win10-dll/win10-dll.vcxproj

@@ -101,7 +101,7 @@
   <ImportGroup Label="Shared">
     <Import Project="..\Clienttelemetry\Clienttelemetry.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\pal\universal\universal.vcxitems" Label="Shared" />
-    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Label="Shared" />
+    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Condition="exists('..\..\lib\modules\exp\exp.vcxitems')" Label="Shared" />
   </ImportGroup>
   <ImportGroup Label="PropertySheets">
     <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />

Solutions/win10-lib/win10-lib.vcxproj

@@ -100,6 +100,7 @@
   <ImportGroup Label="Shared">
     <Import Project="..\Clienttelemetry\Clienttelemetry.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\pal\universal\universal.vcxitems" Label="Shared" />
+    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Condition="exists('..\..\lib\modules\exp\exp.vcxitems')" Label="Shared" />
   </ImportGroup>
   <ImportGroup Label="PropertySheets">
     <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />

Solutions/win32-dll/win32-dll.vcxproj

@@ -105,6 +105,7 @@
   <ImportGroup Label="Shared">
     <Import Project="..\Clienttelemetry\Clienttelemetry.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\pal\desktop\desktop.vcxitems" Label="Shared" />
+    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Condition="exists('..\..\lib\modules\exp\exp.vcxitems')" Label="Shared" />
   </ImportGroup>
   <ImportGroup Label="PropertySheets">
     <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />

Solutions/win32-lib/win32-lib.vcxproj

@@ -156,6 +156,7 @@
   <ImportGroup Label="Shared">
     <Import Project="..\Clienttelemetry\Clienttelemetry.vcxitems" Label="Shared" />
     <Import Project="..\..\lib\pal\desktop\desktop.vcxitems" Label="Shared" />
+    <Import Project="..\..\lib\modules\exp\exp.vcxitems" Condition="exists('..\..\lib\modules\exp\exp.vcxitems')" Label="Shared" />
   </ImportGroup>
   <ImportGroup Label="PropertySheets">
     <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />

lib/CMakeLists.txt

@@ -88,10 +88,12 @@ remove_definitions(-D_MBCS)
     pal/desktop/WindowsEnvironmentInfo.cpp
     pal/desktop/NetworkDetector.cpp
   )
-  list(APPEND SRCS
-    modules/utc/desktop/UtcHelpers.cpp
-    modules/utc/UtcTelemetrySystem.cpp
-  )
+  if(EXISTS "modules/utc/")
+    list(APPEND SRCS
+      modules/utc/desktop/UtcHelpers.cpp
+      modules/utc/UtcTelemetrySystem.cpp
+    )
+  endif()
 else()
   message(FATAL_ERROR "No platform abstraction library configured. Set PAL_IMPLEMENTATION.")
 endif()

lib/api/ContextFieldsProvider.cpp

@@ -167,7 +167,6 @@ namespace ARIASDK_NS_BEGIN
                 iter = m_commonContextFields.find(COMMONFIELDS_DEVICE_ID);
                 if (iter != m_commonContextFields.end())
                 {
-                    // Ref: https://osgwiki.com/wiki/CommonSchema/device_id
                     // Use "c:" prefix
                     std::string temp("c:");
                     const char *deviceId = iter->second.as_string;

lib/decorators/BaseDecorator.cpp

@@ -33,8 +33,7 @@ namespace ARIASDK_NS_BEGIN {
 
         record.extSdk[0].seq = ++m_sequenceId;
         record.extSdk[0].epoch = m_initId;
-        std::string sdkVersion = PAL::getSdkVersion();
-        record.extSdk[0].libVer = sdkVersion;
+        record.extSdk[0].libVer = PAL::getSdkVersion();
         record.extSdk[0].installId = m_owner.GetLogSessionData()->getSessionSDKUid();
 
         //set Tickets

lib/http/HttpRequestEncoder.cpp

@@ -37,7 +37,6 @@ namespace ARIASDK_NS_BEGIN {
         ctx->httpRequest->SetUrl(m_config.GetCollectorUrl());
 
         ctx->httpRequest->GetHeaders().set("Expect", "100-continue");
-        // ctx->httpRequest->GetHeaders().set("Client-Version", PAL::getSdkVersion());
         ctx->httpRequest->GetHeaders().set("SDK-Version", PAL::getSdkVersion());
         ctx->httpRequest->GetHeaders().set("Client-Id", "NO_AUTH");
         ctx->httpRequest->GetHeaders().set("Content-Type", "application/bond-compact-binary");

lib/include/mat/README

@@ -1 +0,0 @@
-NOTE: this directory is going to be moved to ../mat when we publish code on github.com

lib/include/public/CorrelationVector.hpp

@@ -11,7 +11,6 @@
 namespace ARIASDK_NS_BEGIN
 {
     // Implementation of the Common Schema standard vector clock type.
-    // See https://osgwiki.com/wiki/CorrelationVector for more details.
     // Class methods are thread-safe.
     // Boolean-value methods return false to indicate failures.
 

lib/pal/DebugTrace.hpp

@@ -30,7 +30,7 @@ namespace PAL_NS_BEGIN
 #endif
 
     // Declare/define log component for a namespace
-#define MATSDK_LOG_DECL_COMPONENT_NS()                        extern const char* getMATSDKLogComponent()
+#define MATSDK_LOG_DECL_COMPONENT_NS()                        const char* getMATSDKLogComponent()
 #define MATSDK_LOG_INST_COMPONENT_NS(_name, _desc)            char const* getMATSDKLogComponent() { return _name; }
 
 // Declare/define log component for a class

lib/pal/PAL.cpp

@@ -268,7 +268,7 @@ namespace PAL_NS_BEGIN {
     std::string generateUuidString()
     {
 #ifdef _WIN32
-        GUID uuid;
+        GUID uuid = { 0, 0, 0, { 0, 0, 0, 0, 0, 0, 0, 0 } };
         CoCreateGuid(&uuid);
         return MAT::to_string(uuid);
 #else
@@ -465,14 +465,12 @@ namespace PAL_NS_BEGIN {
     #define OS_NAME     "Unknown"
 #endif
 
-    //---
-    // TODO: [MG] - make it portable...
-    std::string getSdkVersion()
-    {
-        return std::string(EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-") + BUILD_VERSION_STR;
-    }
+    static const std::string sdkVersion(EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-" BUILD_VERSION_STR);
 
-    //---
+    const std::string& getSdkVersion()
+    {
+        return sdkVersion;
+    }
 
     static volatile std::atomic<long> g_palStarted(0);
 

lib/pal/PAL.hpp

@@ -57,7 +57,7 @@
 
 namespace ARIASDK_NS_BEGIN
 {
-    extern void print_backtrace();
+    void print_backtrace();
 } ARIASDK_NS_END
 
 namespace PAL_NS_BEGIN
@@ -75,30 +75,30 @@ namespace PAL_NS_BEGIN
 #endif
     }
 
-    extern const char * getMATSDKLogComponent();
+    const char * getMATSDKLogComponent();
 
     /**
      * Return SDK version in format "<Prefix>-<Platform>-<SKU>-<Projection>-<BuildVersion>".
      */
-    extern std::string getSdkVersion();
+    const std::string& getSdkVersion();
 
     /**
      * Returns a new random UUID in a lowercase hexadecimal format with dashes,
      * but without curly braces, e.g. "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
      */
-    extern std::string generateUuidString();
+    std::string generateUuidString();
 
     /**
      * Return the current system time in milliseconds (since the UNIX epoch - Jan 1, 1970).
      */
-    extern int64_t getUtcSystemTimeMs();
+    int64_t getUtcSystemTimeMs();
 
     /**
      * Return the current system time in .NET ticks
      */
-    extern int64_t getUtcSystemTimeinTicks();
+    int64_t getUtcSystemTimeinTicks();
 
-    extern int64_t getUtcSystemTime();
+    int64_t getUtcSystemTime();
 
     /**
      * Convert given system timestamp in milliseconds to a string in ISO 8601 format
@@ -147,9 +147,9 @@ namespace PAL_NS_BEGIN
     };
 
     /* Optional UTC channel mode for Windows 10 */
-    extern bool IsUtcRegistrationEnabledinWindows();
+    bool IsUtcRegistrationEnabledinWindows();
 
-    extern bool RegisterIkeyWithWindowsTelemetry(std::string const& ikeyin, int storageSize, int uploadQuotaSize);
+    bool RegisterIkeyWithWindowsTelemetry(std::string const& ikeyin, int storageSize, int uploadQuotaSize);
 
 } PAL_NS_END
 

lib/utils/Utils.cpp

@@ -36,7 +36,7 @@ namespace ARIASDK_NS_BEGIN {
 
     void sleep(unsigned delayMs)
     {
-        std::this_thread::sleep_for(ms(delayMs));
+        PAL::sleep(delayMs);
     }
 
     void print_backtrace(void)

lib/utils/Utils.hpp

@@ -40,8 +40,7 @@
 
 namespace ARIASDK_NS_BEGIN {
 
-    // TODO: [MG] - refactor this
-    extern const char* getMATSDKLogComponent();
+    const char* getMATSDKLogComponent();
 
     typedef std::chrono::milliseconds ms;
 

tests/common/Common.hpp

@@ -17,7 +17,7 @@
 
 namespace testing {
 
-    extern const char *getMATSDKLogComponent();
+    const char *getMATSDKLogComponent();
 
     extern MAT::ITelemetrySystem & getSystem();
 

tests/common/MockISemanticContext.hpp

@@ -22,10 +22,9 @@ class MockISemanticContext : public MAT::ContextFieldsProvider {
     MOCK_METHOD1(SetCommercialId,      void(std::string const & commercialId));
     MOCK_METHOD1(SetUserLanguage,      void(std::string const & locale));
     MOCK_METHOD1(SetUserTimeZone,      void(std::string const & timeZone));
-	MOCK_METHOD1(SetAppExperimentETag, void(std::string const& appExperimentETag));
-	MOCK_METHOD1(SetAppExperimentImpressionId, void(std::string const& appExperimentImpressionId));
-	MOCK_METHOD2(SetEventExperimentIds, void(std::string const& eventName, std::string const& experimentIds));
-//    MOCK_METHOD2(SetTicket, void(MAT::TicketType type, std::string const& ticketValue));
+    MOCK_METHOD1(SetAppExperimentETag, void(std::string const& appExperimentETag));
+    MOCK_METHOD1(SetAppExperimentImpressionId, void(std::string const& appExperimentImpressionId));
+    MOCK_METHOD2(SetEventExperimentIds, void(std::string const& eventName, std::string const& experimentIds));
 };