SDK: nothrow API fixes, tests, noexcept

sdk-cpp/CMakeLists.txt

@@ -18,9 +18,11 @@ if (DO_PLATFORM_LINUX)
     fixup_compile_options_for_arm()
 endif()
 
-# Currently tests run off of the throwing interface, enable them only when building for the throwing interface
-if (DO_BUILD_TESTS AND DO_ENABLE_EXCEPTIONS)
-    add_subdirectory(tests)
+if (DO_BUILD_TESTS)
+    if (DO_ENABLE_EXCEPTIONS)
+        add_subdirectory(tests)
+    endif ()
+    add_subdirectory(tests_nothrow)
 endif()
 
 set(DO_SDK_LIB_NAME "deliveryoptimization")

sdk-cpp/include/do_download.h

@@ -27,7 +27,7 @@ class download
     ~download();
 
 #if (DO_ENABLE_EXCEPTIONS)
-    static download make(const std::string& uri, const std::string& downloadFilePath);
+    static std::unique_ptr<download> make(const std::string& uri, const std::string& downloadFilePath);
 
     void start();
     void pause();
@@ -50,7 +50,7 @@ class download
     download_property_value get_property(download_property key);
 #endif
 
-    static std::error_code make_nothrow(const std::string& uri, const std::string& downloadFilePath, download& out) noexcept;
+    static std::error_code make_nothrow(const std::string& uri, const std::string& downloadFilePath, std::unique_ptr<download>& out) noexcept;
 
     std::error_code start_nothrow() noexcept;
     std::error_code pause_nothrow() noexcept;

sdk-cpp/src/do_download.cpp

@@ -27,10 +27,10 @@ download::download()
 download::~download() = default;
 
 #if defined(DO_ENABLE_EXCEPTIONS)
-download download::make(const std::string& uri, const std::string& downloadFilePath)
+std::unique_ptr<download> download::make(const std::string& uri, const std::string& downloadFilePath)
 {
-    download out;
-    throw_if_fail(out._download->Init(uri, downloadFilePath));
+    std::unique_ptr<download> out;
+    throw_if_fail(make_nothrow(uri, downloadFilePath, out));
     return out;
 }
 
@@ -75,14 +75,14 @@ void download::start_and_wait_until_completion(std::chrono::seconds timeOut)
 
 void download::download_url_to_path(const std::string& uri, const std::string& downloadFilePath, std::chrono::seconds timeOut)
 {
-    download oneShotDownload = download::make(uri, downloadFilePath);
-    oneShotDownload.start_and_wait_until_completion(timeOut);
+    auto oneShotDownload = download::make(uri, downloadFilePath);
+    oneShotDownload->start_and_wait_until_completion(timeOut);
 }
 
 void download::download_url_to_path(const std::string& uri, const std::string& downloadFilePath, const std::atomic_bool& isCancelled, std::chrono::seconds timeOut)
 {
-    download oneShotDownload = download::make(uri, downloadFilePath);
-    oneShotDownload.start_and_wait_until_completion(isCancelled, timeOut);
+    auto oneShotDownload = download::make(uri, downloadFilePath);
+    oneShotDownload->start_and_wait_until_completion(isCancelled, timeOut);
 }
 
 void download::start_and_wait_until_completion(const std::atomic_bool& isCancelled, std::chrono::seconds timeOut)
@@ -104,13 +104,15 @@ void download::set_property(download_property prop, const download_property_valu
 
 #endif //DO_ENABLE_EXCEPTIONS
 
-std::error_code download::make_nothrow(const std::string& uri, const std::string& downloadFilePath, download& out) noexcept
+std::error_code download::make_nothrow(const std::string& uri, const std::string& downloadFilePath, std::unique_ptr<download>& out) noexcept
 {
-    download tmp;
-    tmp._download = std::make_shared<msdod::CDownloadImpl>();
-    std::error_code code = tmp._download->Init(uri, downloadFilePath);
+    out.reset();
+    // using 'new' to access non-public constructor
+    std::unique_ptr<download> tmp(new download());
+    tmp->_download = std::make_shared<msdod::CDownloadImpl>();
+    std::error_code code = tmp->_download->Init(uri, downloadFilePath);
     DO_RETURN_IF_FAILED(code);
-    out = tmp;
+    out = std::move(tmp);
     return DO_OK;
 }
 
@@ -210,9 +212,9 @@ std::error_code download::download_url_to_path_nothrow(const std::string& uri, c
 
 std::error_code download::download_url_to_path_nothrow(const std::string& uri, const std::string& downloadFilePath, const std::atomic_bool& isCancelled, std::chrono::seconds timeOut) noexcept
 {
-    download oneShotDownload;
+    std::unique_ptr<download> oneShotDownload;
     DO_RETURN_IF_FAILED(download::make_nothrow(uri, downloadFilePath, oneShotDownload));
-    return oneShotDownload.start_and_wait_until_completion_nothrow(isCancelled, timeOut);
+    return oneShotDownload->start_and_wait_until_completion_nothrow(isCancelled, timeOut);
 }
 
 std::error_code download::set_property_nothrow(download_property prop, const download_property_value& val) noexcept

sdk-cpp/src/internal/com/download_impl.cpp

@@ -201,7 +201,7 @@ class DOStatusCallback :
     }
 
 #if defined(DO_ENABLE_EXCEPTIONS)
-    IFACEMETHODIMP OnStatusChange(IDODownload* download, const DO_DOWNLOAD_STATUS* comStatus)
+    IFACEMETHODIMP OnStatusChange(IDODownload* download, const DO_DOWNLOAD_STATUS* comStatus) noexcept
     {
         try
         {
@@ -225,7 +225,7 @@ class DOStatusCallback :
 #else
     // If an application builds the sdk from source and has toggled DO_ENABLE_EXCEPTIONS, it would be hypocritical for their callback to throw
     // Need to provide this definition because try/catch keywords in the implementation above will fail builds with exceptions disabled
-    IFACEMETHODIMP OnStatusChange(IDODownload* download, const DO_DOWNLOAD_STATUS* comStatus)
+    IFACEMETHODIMP OnStatusChange(IDODownload* download, const DO_DOWNLOAD_STATUS* comStatus) noexcept
     {
         msdo::download_status status = ConvertFromComStatus(*comStatus);
         _callback(*_download, status);

sdk-cpp/tests/com/download_properties_tests.cpp

@@ -62,78 +62,54 @@ class DownloadPropertyTestsDOSVC : public ::testing::Test
 //At the moment, these tests are essentially just verifying that these properties can be set and download succeeds
 TEST_F(DownloadPropertyTestsDOSVC, SmallDownloadSetCallerNameTest)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     std::string strCallerName("dosdkcpp_tests");
     msdo::download_property_value callerName = msdo::download_property_value::make(strCallerName);
-    simpleDownload.set_property(msdo::download_property::caller_name, callerName);
+    simpleDownload->set_property(msdo::download_property::caller_name, callerName);
 
-    simpleDownload.start_and_wait_until_completion();
+    simpleDownload->start_and_wait_until_completion();
     ASSERT_TRUE(boost::filesystem::exists(g_tmpFileName));
 }
 
 TEST_F(DownloadPropertyTestsDOSVC, SmallDownloadWithPhfDigestandCvTest)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     std::vector<int32_t> expectedErrors = { 0, static_cast<int32_t>(msdo::errc::do_e_unknown_property_id) };
 
-    msdo::download_property_value integrityCheckMandatory = msdo::download_property_value::make(true);
+    msdo::download_property_value integrityCheckMandatory = msdo::download_property_value::make(false);
     VerifyCallWithExpectedErrors([&]()
         {
-            simpleDownload.set_property(msdo::download_property::integrity_check_mandatory, integrityCheckMandatory);
+            simpleDownload->set_property(msdo::download_property::integrity_check_mandatory, integrityCheckMandatory);
         }, expectedErrors);
 
     msdo::download_property_value integrityCheckInfo = msdo::download_property_value::make(g_smallFilePhfInfoJson);
     VerifyCallWithExpectedErrors([&]()
         {
-            simpleDownload.set_property(msdo::download_property::integrity_check_info, integrityCheckInfo);
+            simpleDownload->set_property(msdo::download_property::integrity_check_info, integrityCheckInfo);
         }, expectedErrors);
 
     std::string strCorrelationVector("g+Vo71JZwkmJdYfF.0");
     msdo::download_property_value correlationVector = msdo::download_property_value::make(strCorrelationVector);
     VerifyCallWithExpectedErrors([&]()
         {
-            simpleDownload.set_property(msdo::download_property::correlation_vector, correlationVector);
+            simpleDownload->set_property(msdo::download_property::correlation_vector, correlationVector);
         }, expectedErrors);
 
-    simpleDownload.start_and_wait_until_completion();
-
-    ASSERT_TRUE(boost::filesystem::exists(g_tmpFileName));
-}
-
-TEST_F(DownloadPropertyTestsDOSVC, SmallDownloadWithPhfDigestandCvTestNoThrow)
-{
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
-
-    std::vector<int32_t> expectedErrors = { 0, static_cast<int32_t>(msdo::errc::do_e_unknown_property_id) };
-
-    msdo::download_property_value integrityCheckMandatory = msdo::download_property_value::make(true);
-    int32_t code = simpleDownload.set_property_nothrow(msdo::download_property::integrity_check_mandatory, integrityCheckMandatory).value();
-    VerifyError(code, expectedErrors);
-
-    msdo::download_property_value integrityCheckInfo = msdo::download_property_value::make(g_smallFilePhfInfoJson);
-    code = simpleDownload.set_property_nothrow(msdo::download_property::integrity_check_info, integrityCheckInfo).value();
-    VerifyError(code, expectedErrors);
-
-    std::string strCorrelationVector("g+Vo71JZwkmJdYfF.0");
-    msdo::download_property_value correlationVector = msdo::download_property_value::make(strCorrelationVector);
-    code = simpleDownload.set_property_nothrow(msdo::download_property::correlation_vector, correlationVector).value();
-    VerifyError(code, expectedErrors);
-
-    simpleDownload.start_and_wait_until_completion();
+    simpleDownload->start_and_wait_until_completion();
 
     ASSERT_TRUE(boost::filesystem::exists(g_tmpFileName));
 }
 
 TEST_F(DownloadPropertyTestsDOSVC, InvalidPhfDigestTest)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     try
     {
         msdo::download_property_value integrityCheckInfo =  msdo::download_property_value::make("blah");
-        simpleDownload.set_property(msdo::download_property::integrity_check_info, integrityCheckInfo);
+        simpleDownload->set_property(msdo::download_property::integrity_check_info, integrityCheckInfo);
     }
     catch (const msdo::exception& e)
     {
@@ -147,48 +123,48 @@ TEST_F(DownloadPropertyTestsDOSVC, InvalidPhfDigestTest)
 // For some reason, custom headers are getting rejected and returning E_INVALIDARG now, disabling test for now
 TEST_F(DownloadPropertyTestsDOSVC, DISABLED_SmallDownloadWithCustomHeaders)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     std::string strHttpCustomHeaders("XCustom1=someData\nXCustom2=moreData");
     msdo::download_property_value httpCustomHeaders = msdo::download_property_value::make(strHttpCustomHeaders);
-    simpleDownload.set_property(msdo::download_property::http_custom_headers, httpCustomHeaders);
+    simpleDownload->set_property(msdo::download_property::http_custom_headers, httpCustomHeaders);
 
-    simpleDownload.start_and_wait_until_completion();
+    simpleDownload->start_and_wait_until_completion();
 
     ASSERT_TRUE(boost::filesystem::exists(g_tmpFileName));
 }
 
 TEST_F(DownloadPropertyTestsDOSVC, CallbackTestUseDownload)
 {
-    msdo::download simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
     bool hitError = false;
 
     msdo::download_property_value callback = msdo::download_property_value::make([&hitError](msdo::download& download, msdo::download_status& status)
-    {
-        char msgBuf[1024];
-        snprintf(msgBuf, sizeof(msgBuf), "Received status callback: %llu/%llu, 0x%x, 0x%x, %u",
-            status.bytes_transferred(), status.bytes_total(), status.error_code().value(), status.extended_error_code().value(),
-            static_cast<unsigned int>(status.state()));
-        std::cout << msgBuf << std::endl;
-
-        msdo::download_status status2 = download.get_status();
-        if (hitError)
         {
-            download.pause();
-        }
-    });
+            char msgBuf[1024];
+            snprintf(msgBuf, sizeof(msgBuf), "Received status callback: %llu/%llu, 0x%x, 0x%x, %u",
+                status.bytes_transferred(), status.bytes_total(), status.error_code().value(), status.extended_error_code().value(),
+                static_cast<unsigned int>(status.state()));
+            std::cout << msgBuf << std::endl;
+
+            msdo::download_status status2 = download.get_status();
+            if (hitError)
+            {
+                download.pause();
+            }
+        });
 
-    simpleDownload.set_property(msdo::download_property::callback_interface, callback);
-    simpleDownload.start();
+    simpleDownload->set_property(msdo::download_property::callback_interface, callback);
+    simpleDownload->start();
     std::this_thread::sleep_for(5s);
     hitError = true;
 
-    TestHelpers::WaitForState(simpleDownload, msdo::download_state::paused);
+    TestHelpers::WaitForState(*simpleDownload, msdo::download_state::paused);
 }
 
 TEST_F(DownloadPropertyTestsDOSVC, SetCallbackTest)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     int i= 0;
     msdo::download_property_value callback = msdo::download_property_value::make([&i](msdo::download& download, msdo::download_status& status)
@@ -200,30 +176,30 @@ TEST_F(DownloadPropertyTestsDOSVC, SetCallbackTest)
             std::cout << msgBuf << std::endl;
             i += 1;
         });
-    simpleDownload.set_property(msdo::download_property::callback_interface, callback);
+    simpleDownload->set_property(msdo::download_property::callback_interface, callback);
 
-    simpleDownload.start_and_wait_until_completion();
+    simpleDownload->start_and_wait_until_completion();
 
     ASSERT_GE(i, 0);
 }
 
 TEST_F(DownloadPropertyTestsDOSVC, OverrideCallbackTest)
 {
-    msdo::download simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
+    auto simpleDownload = msdo::download::make(g_smallFileUrl, g_tmpFileName);
 
     int i = 0;
     msdo::download_property_value callback = msdo::download_property_value::make([&i](msdo::download&, msdo::download_status&)
         {
             i += 1;
         });
-    simpleDownload.set_property(msdo::download_property::callback_interface, callback);
+    simpleDownload->set_property(msdo::download_property::callback_interface, callback);
 
     msdo::download_property_value::status_callback_t cb2 = [](msdo::download&, msdo::download_status&) {};
 
     callback = msdo::download_property_value::make(cb2);
-    simpleDownload.set_property(msdo::download_property::callback_interface, callback);
+    simpleDownload->set_property(msdo::download_property::callback_interface, callback);
 
-    simpleDownload.start_and_wait_until_completion();
+    simpleDownload->start_and_wait_until_completion();
 
     ASSERT_EQ(i, 0);
 }
@@ -232,23 +208,23 @@ TEST_F(DownloadPropertyTestsDOSVC, ForegroundBackgroundRace)
 {
     double backgroundDuration = TimeOperation([&]()
         {
-            msdo::download simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
+            auto simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
 
             msdo::download_property_value foregroundPriority = msdo::download_property_value::make(false);
-            simpleDownload.set_property(msdo::download_property::use_foreground_priority, foregroundPriority);
+            simpleDownload->set_property(msdo::download_property::use_foreground_priority, foregroundPriority);
 
-            simpleDownload.start_and_wait_until_completion();
+            simpleDownload->start_and_wait_until_completion();
         });
 
     printf("Time for background download: %f ms\n", backgroundDuration);
     double foregroundDuration = TimeOperation([&]()
         {
-            msdo::download simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName2);
+            auto simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName2);
 
             msdo::download_property_value foregroundPriority = msdo::download_property_value::make(true);
-            simpleDownload.set_property(msdo::download_property::use_foreground_priority, foregroundPriority);
+            simpleDownload->set_property(msdo::download_property::use_foreground_priority, foregroundPriority);
 
-            simpleDownload.start_and_wait_until_completion();
+            simpleDownload->start_and_wait_until_completion();
         });
     printf("Time for foreground download: %f ms\n", foregroundDuration);
 

sdk-cpp/tests/com/download_tests.cpp

@@ -42,15 +42,15 @@ class DownloadTestsDOSVC : public ::testing::Test
 TEST_F(DownloadTestsDOSVC, Download1PausedDownload2SameDestTest)
 {
     ASSERT_FALSE(boost::filesystem::exists(g_tmpFileName));
-    msdo::download simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
-    msdo::download_status status = simpleDownload.get_status();
+    auto simpleDownload = msdo::download::make(g_largeFileUrl, g_tmpFileName);
+    msdo::download_status status = simpleDownload->get_status();
     ASSERT_EQ(status.state(), msdo::download_state::created);
     ASSERT_EQ(status.bytes_transferred(), 0u);
 
-    simpleDownload.start();
+    simpleDownload->start();
     std::this_thread::sleep_for(1s);
-    simpleDownload.pause();
-    TestHelpers::WaitForState(simpleDownload, msdo::download_state::paused);
+    simpleDownload->pause();
+    TestHelpers::WaitForState(*simpleDownload, msdo::download_state::paused);
 
     msdo::download::download_url_to_path(g_smallFileUrl, g_tmpFileName);
     ASSERT_EQ(boost::filesystem::file_size(boost::filesystem::path(g_tmpFileName)), g_smallFileSizeBytes);

sdk-cpp/tests/dosdkcpp_testrunner.cpp

@@ -34,7 +34,13 @@ int main(int argc, char** argv)
     }
 
 #if defined(DO_INTERFACE_COM)
-    ASSERT_TRUE(SUCCEEDED(CoInitializeEx(nullptr, COINIT_MULTITHREADED))); // SDK leaves com init up to caller, so initialize in test exe here
+    // SDK leaves com init up to caller, so initialize in test exe here
+    const auto hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED);
+    if (FAILED(hr))
+    {
+        std::cout << "CoInitializeEx failed with " << hr << std::endl;
+        return hr;
+    }
 #endif
 
     auto manualStart = vm["manual-start"].as<bool>();
@@ -46,11 +52,11 @@ int main(int argc, char** argv)
         } while (std::cin.get() != '\n');
     }
 
-    int hr = RUN_ALL_TESTS();
+    int testsResult = RUN_ALL_TESTS();
 
 #if defined(DO_INTERFACE_COM)
     CoUninitialize();
 #endif
 
-    return hr;
+    return testsResult;
 }