Commit bf0d3aa37e4b5802e26937a628347a79d49e94ba
Committed by
Moritz Wirger
1 parent
0db0a5dd
Implement retry in HueCommandAPI, add tests.
- On errors connection reset or timed out, waits and retries once
Showing
5 changed files
with
274 additions
and
26 deletions
hueplusplus/Hue.cpp
| ... | ... | @@ -79,21 +79,16 @@ Hue HueFinder::GetBridge(const HueIdentification& identification) |
| 79 | 79 | { |
| 80 | 80 | return Hue(identification.ip, pos->second, http_handler); |
| 81 | 81 | } |
| 82 | - else | |
| 82 | + Hue bridge(identification.ip, "", http_handler); | |
| 83 | + bridge.requestUsername(identification.ip); | |
| 84 | + if (bridge.getUsername().empty()) | |
| 83 | 85 | { |
| 84 | - Hue bridge(identification.ip, "", http_handler); | |
| 85 | - bridge.requestUsername(identification.ip); | |
| 86 | - if (bridge.getUsername().empty()) | |
| 87 | - { | |
| 88 | - std::cerr << "Failed to request username for ip " << identification.ip << std::endl; | |
| 89 | - throw std::runtime_error("Failed to request username!"); | |
| 90 | - } | |
| 91 | - else | |
| 92 | - { | |
| 93 | - AddUsername(identification.mac, bridge.getUsername()); | |
| 94 | - } | |
| 95 | - return bridge; | |
| 86 | + std::cerr << "Failed to request username for ip " << identification.ip << std::endl; | |
| 87 | + throw std::runtime_error("Failed to request username!"); | |
| 96 | 88 | } |
| 89 | + AddUsername(identification.mac, bridge.getUsername()); | |
| 90 | + | |
| 91 | + return bridge; | |
| 97 | 92 | } |
| 98 | 93 | |
| 99 | 94 | void HueFinder::AddUsername(const std::string& mac, const std::string& username) | ... | ... |
hueplusplus/HueCommandAPI.cpp
| ... | ... | @@ -27,7 +27,7 @@ HueCommandAPI::HueCommandAPI(const std::string& ip, const std::string& username, |
| 27 | 27 | : ip(ip), |
| 28 | 28 | username(username), |
| 29 | 29 | httpHandler(std::move(httpHandler)), |
| 30 | - timeout(new TimeoutData{std::chrono::steady_clock::now()}) | |
| 30 | + timeout(new TimeoutData{ std::chrono::steady_clock::now() }) | |
| 31 | 31 | {} |
| 32 | 32 | |
| 33 | 33 | Json::Value HueCommandAPI::PUTRequest(const std::string& path, const Json::Value& request) const |
| ... | ... | @@ -38,10 +38,27 @@ Json::Value HueCommandAPI::PUTRequest(const std::string& path, const Json::Value |
| 38 | 38 | { |
| 39 | 39 | std::this_thread::sleep_until(timeout->timeout); |
| 40 | 40 | } |
| 41 | - Json::Value v = httpHandler->PUTJson("/api/" + username + path, request, ip); | |
| 42 | - timeout->timeout = now + minDelay; | |
| 43 | - | |
| 44 | - return v; | |
| 41 | + //If path does not begin with '/', insert it unless path is empty | |
| 42 | + const std::string combinedPath = "/api/" + username + (path.empty() || path.front() == '/' ? "" : "/") + path; | |
| 43 | + try | |
| 44 | + { | |
| 45 | + Json::Value v = httpHandler->PUTJson(combinedPath, request, ip); | |
| 46 | + timeout->timeout = now + minDelay; | |
| 47 | + return v; | |
| 48 | + } | |
| 49 | + catch (const std::system_error& e) | |
| 50 | + { | |
| 51 | + if (e.code() == std::errc::connection_reset || e.code() == std::errc::timed_out) | |
| 52 | + { | |
| 53 | + //Happens when hue is too busy, wait and try again (once) | |
| 54 | + std::this_thread::sleep_for(minDelay); | |
| 55 | + Json::Value v = httpHandler->PUTJson(combinedPath, request, ip); | |
| 56 | + timeout->timeout = std::chrono::steady_clock::now() + minDelay; | |
| 57 | + return v; | |
| 58 | + } | |
| 59 | + //Cannot recover from other types of errors | |
| 60 | + throw; | |
| 61 | + } | |
| 45 | 62 | } |
| 46 | 63 | |
| 47 | 64 | Json::Value HueCommandAPI::GETRequest(const std::string& path, const Json::Value& request) const |
| ... | ... | @@ -52,10 +69,27 @@ Json::Value HueCommandAPI::GETRequest(const std::string& path, const Json::Value |
| 52 | 69 | { |
| 53 | 70 | std::this_thread::sleep_until(timeout->timeout); |
| 54 | 71 | } |
| 55 | - Json::Value v = httpHandler->GETJson("/api/" + username + path, request, ip); | |
| 56 | - timeout->timeout = now + minDelay; | |
| 57 | - | |
| 58 | - return v; | |
| 72 | + //If path does not begin with '/', insert it unless path is empty | |
| 73 | + const std::string combinedPath = "/api/" + username + (path.empty() || path.front() == '/' ? "" : "/") + path; | |
| 74 | + try | |
| 75 | + { | |
| 76 | + Json::Value v = httpHandler->GETJson(combinedPath, request, ip); | |
| 77 | + timeout->timeout = now + minDelay; | |
| 78 | + return v; | |
| 79 | + } | |
| 80 | + catch (const std::system_error& e) | |
| 81 | + { | |
| 82 | + if (e.code() == std::errc::connection_reset || e.code() == std::errc::timed_out) | |
| 83 | + { | |
| 84 | + //Happens when hue is too busy, wait and try again (once) | |
| 85 | + std::this_thread::sleep_for(minDelay); | |
| 86 | + Json::Value v = httpHandler->GETJson(combinedPath, request, ip); | |
| 87 | + timeout->timeout = std::chrono::steady_clock::now() + minDelay; | |
| 88 | + return v; | |
| 89 | + } | |
| 90 | + //Cannot recover from other types of errors | |
| 91 | + throw; | |
| 92 | + } | |
| 59 | 93 | } |
| 60 | 94 | |
| 61 | 95 | Json::Value HueCommandAPI::DELETERequest(const std::string& path, const Json::Value& request) const |
| ... | ... | @@ -66,8 +100,25 @@ Json::Value HueCommandAPI::DELETERequest(const std::string& path, const Json::Va |
| 66 | 100 | { |
| 67 | 101 | std::this_thread::sleep_until(timeout->timeout); |
| 68 | 102 | } |
| 69 | - Json::Value v = httpHandler->DELETEJson("/api/" + username + path, request, ip); | |
| 70 | - timeout->timeout = now + minDelay; | |
| 71 | - | |
| 72 | - return v; | |
| 103 | + //If path does not begin with '/', insert it unless path is empty | |
| 104 | + const std::string combinedPath = "/api/" + username + (path.empty() || path.front() == '/' ? "" : "/") + path; | |
| 105 | + try | |
| 106 | + { | |
| 107 | + Json::Value v = httpHandler->DELETEJson(combinedPath, request, ip); | |
| 108 | + timeout->timeout = now + minDelay; | |
| 109 | + return v; | |
| 110 | + } | |
| 111 | + catch (const std::system_error& e) | |
| 112 | + { | |
| 113 | + if (e.code() == std::errc::connection_reset || e.code() == std::errc::timed_out) | |
| 114 | + { | |
| 115 | + //Happens when hue is too busy, wait and try again (once) | |
| 116 | + std::this_thread::sleep_for(minDelay); | |
| 117 | + Json::Value v = httpHandler->DELETEJson(combinedPath, request, ip); | |
| 118 | + timeout->timeout = std::chrono::steady_clock::now() + minDelay; | |
| 119 | + return v; | |
| 120 | + } | |
| 121 | + //Cannot recover from other types of errors | |
| 122 | + throw; | |
| 123 | + } | |
| 73 | 124 | } | ... | ... |
hueplusplus/test/CMakeLists.txt
| ... | ... | @@ -39,6 +39,7 @@ set(TEST_SOURCES |
| 39 | 39 | ${CMAKE_CURRENT_SOURCE_DIR}/test_ExtendedColorTemperatureStrategy.cpp |
| 40 | 40 | ${CMAKE_CURRENT_SOURCE_DIR}/test_Hue.cpp |
| 41 | 41 | ${CMAKE_CURRENT_SOURCE_DIR}/test_HueLight.cpp |
| 42 | + ${CMAKE_CURRENT_SOURCE_DIR}/test_HueCommandAPI.cpp | |
| 42 | 43 | ${CMAKE_CURRENT_SOURCE_DIR}/test_Main.cpp |
| 43 | 44 | ${CMAKE_CURRENT_SOURCE_DIR}/test_SimpleBrightnessStrategy.cpp |
| 44 | 45 | ${CMAKE_CURRENT_SOURCE_DIR}/test_SimpleColorHueStrategy.cpp | ... | ... |
hueplusplus/test/test_Hue.cpp
| ... | ... | @@ -400,6 +400,7 @@ TEST(Hue, getAllLights) |
| 400 | 400 | Hue test_bridge(bridge_ip, bridge_username, handler); |
| 401 | 401 | |
| 402 | 402 | std::vector<std::reference_wrapper<HueLight>> test_lights = test_bridge.getAllLights(); |
| 403 | + ASSERT_EQ(1, test_lights.size()); | |
| 403 | 404 | EXPECT_EQ(test_lights[0].get().getName(), "Hue ambiance lamp 1"); |
| 404 | 405 | EXPECT_EQ(test_lights[0].get().getColorType(), ColorType::TEMPERATURE); |
| 405 | 406 | } | ... | ... |
hueplusplus/test/test_HueCommandAPI.cpp
0 โ 100644
| 1 | +/** | |
| 2 | + \file test_HueCommandAPI.cpp | |
| 3 | + Copyright Notice\n | |
| 4 | + Copyright (C) 2018 Jan Rogall - developer\n | |
| 5 | + Copyright (C) 2018 Moritz Wirger - developer\n | |
| 6 | + | |
| 7 | + This program is free software; you can redistribute it and/or modify | |
| 8 | + it under the terms of the GNU General Public License as published by | |
| 9 | + the Free Software Foundation; either version 3 of the License, or | |
| 10 | + (at your option) any later version. | |
| 11 | + This program is distributed in the hope that it will be useful, | |
| 12 | + but WITHOUT ANY WARRANTY; without even the implied warranty of | |
| 13 | + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |
| 14 | + GNU General Public License for more details. | |
| 15 | + You should have received a copy of the GNU General Public License | |
| 16 | + along with this program; if not, write to the Free Software Foundation, | |
| 17 | + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA | |
| 18 | +**/ | |
| 19 | + | |
| 20 | +#include <gtest/gtest.h> | |
| 21 | +#include <gmock/gmock.h> | |
| 22 | + | |
| 23 | +#include "../include/Hue.h" | |
| 24 | +#include "../include/json/json.h" | |
| 25 | +#include "mocks/mock_HttpHandler.h" | |
| 26 | +#include "testhelper.h" | |
| 27 | + | |
| 28 | +TEST(HueCommandAPI, PUTRequest) | |
| 29 | +{ | |
| 30 | + using namespace ::testing; | |
| 31 | + std::shared_ptr<MockHttpHandler> httpHandler = std::make_shared<MockHttpHandler>(); | |
| 32 | + | |
| 33 | + HueCommandAPI api(bridge_ip, bridge_username, httpHandler); | |
| 34 | + Json::Value request; | |
| 35 | + Json::Value result = Json::objectValue; | |
| 36 | + result["ok"] = true; | |
| 37 | + | |
| 38 | + //empty path | |
| 39 | + { | |
| 40 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 41 | + EXPECT_EQ(result, api.PUTRequest("", request)); | |
| 42 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 43 | + } | |
| 44 | + //not empty path, starting with slash | |
| 45 | + { | |
| 46 | + const std::string path = "/test"; | |
| 47 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 48 | + EXPECT_EQ(result, api.PUTRequest(path, request)); | |
| 49 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 50 | + } | |
| 51 | + //not empty path, not starting with slash | |
| 52 | + { | |
| 53 | + const std::string path = "test"; | |
| 54 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username + '/' + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 55 | + EXPECT_EQ(result, api.PUTRequest(path, request)); | |
| 56 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 57 | + } | |
| 58 | + //recoverable error | |
| 59 | + { | |
| 60 | + const std::string path = "/test"; | |
| 61 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 62 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 63 | + .WillOnce(Return(result)); | |
| 64 | + EXPECT_EQ(result, api.PUTRequest(path, request)); | |
| 65 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 66 | + } | |
| 67 | + //recoverable error x2 | |
| 68 | + { | |
| 69 | + const std::string path = "/test"; | |
| 70 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 71 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 72 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))); | |
| 73 | + EXPECT_THROW(api.PUTRequest(path, request), std::system_error); | |
| 74 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 75 | + } | |
| 76 | + //unrecoverable error | |
| 77 | + { | |
| 78 | + const std::string path = "/test"; | |
| 79 | + EXPECT_CALL(*httpHandler, PUTJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 80 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::not_enough_memory)))); | |
| 81 | + EXPECT_THROW(api.PUTRequest(path, request), std::system_error); | |
| 82 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 83 | + } | |
| 84 | +} | |
| 85 | + | |
| 86 | +TEST(HueCommandAPI, GETRequest) | |
| 87 | +{ | |
| 88 | + using namespace ::testing; | |
| 89 | + std::shared_ptr<MockHttpHandler> httpHandler = std::make_shared<MockHttpHandler>(); | |
| 90 | + | |
| 91 | + HueCommandAPI api(bridge_ip, bridge_username, httpHandler); | |
| 92 | + Json::Value request; | |
| 93 | + Json::Value result = Json::objectValue; | |
| 94 | + result["ok"] = true; | |
| 95 | + | |
| 96 | + //empty path | |
| 97 | + { | |
| 98 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 99 | + EXPECT_EQ(result, api.GETRequest("", request)); | |
| 100 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 101 | + } | |
| 102 | + //not empty path, starting with slash | |
| 103 | + { | |
| 104 | + const std::string path = "/test"; | |
| 105 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 106 | + EXPECT_EQ(result, api.GETRequest(path, request)); | |
| 107 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 108 | + } | |
| 109 | + //not empty path, not starting with slash | |
| 110 | + { | |
| 111 | + const std::string path = "test"; | |
| 112 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + '/' + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 113 | + EXPECT_EQ(result, api.GETRequest(path, request)); | |
| 114 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 115 | + } | |
| 116 | + //recoverable error | |
| 117 | + { | |
| 118 | + const std::string path = "/test"; | |
| 119 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 120 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 121 | + .WillOnce(Return(result)); | |
| 122 | + EXPECT_EQ(result, api.GETRequest(path, request)); | |
| 123 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 124 | + } | |
| 125 | + //recoverable error x2 | |
| 126 | + { | |
| 127 | + const std::string path = "/test"; | |
| 128 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 129 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 130 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))); | |
| 131 | + EXPECT_THROW(api.GETRequest(path, request), std::system_error); | |
| 132 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 133 | + } | |
| 134 | + //unrecoverable error | |
| 135 | + { | |
| 136 | + const std::string path = "/test"; | |
| 137 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 138 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::not_enough_memory)))); | |
| 139 | + EXPECT_THROW(api.GETRequest(path, request), std::system_error); | |
| 140 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 141 | + } | |
| 142 | +} | |
| 143 | + | |
| 144 | +TEST(HueCommandAPI, DELETERequest) | |
| 145 | +{ | |
| 146 | + using namespace ::testing; | |
| 147 | + std::shared_ptr<MockHttpHandler> httpHandler = std::make_shared<MockHttpHandler>(); | |
| 148 | + | |
| 149 | + HueCommandAPI api(bridge_ip, bridge_username, httpHandler); | |
| 150 | + Json::Value request; | |
| 151 | + Json::Value result = Json::objectValue; | |
| 152 | + result["ok"] = true; | |
| 153 | + | |
| 154 | + //empty path | |
| 155 | + { | |
| 156 | + EXPECT_CALL(*httpHandler, DELETEJson("/api/" + bridge_username, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 157 | + EXPECT_EQ(result, api.DELETERequest("", request)); | |
| 158 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 159 | + } | |
| 160 | + //not empty path, starting with slash | |
| 161 | + { | |
| 162 | + const std::string path = "/test"; | |
| 163 | + EXPECT_CALL(*httpHandler, DELETEJson("/api/" + bridge_username + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 164 | + EXPECT_EQ(result, api.DELETERequest(path, request)); | |
| 165 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 166 | + } | |
| 167 | + //not empty path, not starting with slash | |
| 168 | + { | |
| 169 | + const std::string path = "test"; | |
| 170 | + EXPECT_CALL(*httpHandler, DELETEJson("/api/" + bridge_username + '/' + path, request, bridge_ip, 80)).WillOnce(Return(result)); | |
| 171 | + EXPECT_EQ(result, api.DELETERequest(path, request)); | |
| 172 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 173 | + } | |
| 174 | + //recoverable error | |
| 175 | + { | |
| 176 | + const std::string path = "/test"; | |
| 177 | + EXPECT_CALL(*httpHandler, DELETEJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 178 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 179 | + .WillOnce(Return(result)); | |
| 180 | + EXPECT_EQ(result, api.DELETERequest(path, request)); | |
| 181 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 182 | + } | |
| 183 | + //recoverable error x2 | |
| 184 | + { | |
| 185 | + const std::string path = "/test"; | |
| 186 | + EXPECT_CALL(*httpHandler, DELETEJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 187 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))) | |
| 188 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::connection_reset)))); | |
| 189 | + EXPECT_THROW(api.DELETERequest(path, request), std::system_error); | |
| 190 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 191 | + } | |
| 192 | + //unrecoverable error | |
| 193 | + { | |
| 194 | + const std::string path = "/test"; | |
| 195 | + EXPECT_CALL(*httpHandler, GETJson("/api/" + bridge_username + path, request, bridge_ip, 80)) | |
| 196 | + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::not_enough_memory)))); | |
| 197 | + EXPECT_THROW(api.GETRequest(path, request), std::system_error); | |
| 198 | + Mock::VerifyAndClearExpectations(httpHandler.get()); | |
| 199 | + } | |
| 200 | +} | |
| 0 | 201 | \ No newline at end of file | ... | ... |