Skip to content

Commit b694ca7

Browse files
ggcrggcr
committed
Implementing Delete as PATCH for now yolo
1 parent 5b27d0b commit b694ca7

File tree

4 files changed

+118
-44
lines changed

4 files changed

+118
-44
lines changed

‎delete‎

Whitespace-only changes.

‎src/s3cpp/httpclient.cpp‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ HttpResponse HttpBodyRequest::execute() {
2424
return client_.execute_post(*this);
2525
case HttpMethod::Put:
2626
return client_.execute_post(*this);
27+
case HttpMethod::Delete:
28+
return client_.execute_delete(*this);
2729
default:
2830
throw std::runtime_error(std::format("No matching enum Http Method"));
2931
}
@@ -180,6 +182,63 @@ HttpResponse HttpClient::execute_post(HttpBodyRequest &request) {
180182
std::move(headers_buf));
181183
}
182184

185+
HttpResponse HttpClient::execute_delete(HttpBodyRequest &request) {
186+
if (!curl_handle) {
187+
throw std::runtime_error(
188+
// this can happen both when cURL handle is not initialized or when it
189+
// is invalidated in the HTTPClient copy constructor
190+
"cURL handle is invalid");
191+
}
192+
std::string body_buf;
193+
std::map<std::string, std::string> headers_buf;
194+
std::string error_buf;
195+
196+
// TODO(cristian): from libcurl docs, they state that each curl handle has
197+
// "sticky" params, this is why we are resetting at each get request
198+
// However, I think we should only do this when moving a new handle the only
199+
// thing that will change is the URL from now
200+
//
201+
// curl_easy_reset(curl_handle);
202+
203+
curl_easy_setopt(curl_handle, CURLOPT_URL, request.getURL().c_str());
204+
// body callback
205+
curl_easy_setopt(curl_handle, CURLOPT_WRITEFUNCTION, write_callback);
206+
curl_easy_setopt(curl_handle, CURLOPT_WRITEDATA, &body_buf);
207+
// headers callback
208+
curl_easy_setopt(curl_handle, CURLOPT_HEADERFUNCTION, header_callback);
209+
curl_easy_setopt(curl_handle, CURLOPT_HEADERDATA, &headers_buf);
210+
// delete may have or not have a body
211+
curl_easy_setopt(curl_handle, CURLOPT_CUSTOMREQUEST, "PATCH");
212+
if (request.getBody() != "") // use std::optional
213+
curl_easy_setopt(curl_handle, CURLOPT_POSTFIELDS, request.getBody().c_str());
214+
215+
curl_easy_setopt(curl_handle, CURLOPT_TIMEOUT, request.getTimeout());
216+
217+
// merge client and request headers
218+
// https://stackoverflow.com/questions/34321719
219+
auto headers = request.getHeaders();
220+
headers.insert(this->getHeaders().begin(), this->getHeaders().end());
221+
struct curl_slist *list = NULL;
222+
for (const auto &[k, v] : headers) {
223+
list = curl_slist_append(list, std::format("{}: {}", k, v).c_str());
224+
}
225+
curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, list);
226+
227+
CURLcode code = curl_easy_perform(curl_handle);
228+
curl_slist_free_all(list);
229+
if (code != CURLE_OK) {
230+
throw std::runtime_error(
231+
std::format("libcurl error for request: {}", curl_easy_strerror(code)));
232+
}
233+
234+
// get HTTP code
235+
long response_code = 0;
236+
curl_easy_getinfo(curl_handle, CURLINFO_HTTP_CODE, &response_code);
237+
238+
return HttpResponse(static_cast<int>(response_code), std::move(body_buf),
239+
std::move(headers_buf));
240+
}
241+
183242
size_t HttpClient::write_callback(char *ptr, size_t size, size_t nmemb,
184243
void *userdata) {
185244
std::string *buffer = static_cast<std::string *>(userdata);

‎src/s3cpp/httpclient.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// Forward declaration
1414
class HttpClient;
1515

16-
enum class HttpMethod { Get, Post, Put, Head };
16+
enum class HttpMethod { Get, Post, Put, Head, Delete };
1717

1818
class HttpResponse {
1919
public:
@@ -114,7 +114,7 @@ class HttpBodyRequest : public HttpRequestBase<HttpBodyRequest> {
114114
HttpResponse execute();
115115

116116
private:
117-
std::string body_;
117+
std::string body_ = "";
118118
};
119119

120120
// HttpClient should only focus on handling the cURL handle
@@ -174,6 +174,9 @@ class HttpClient {
174174
[[nodiscard]] HttpBodyRequest put(const std::string &URL) {
175175
return HttpBodyRequest{*this, URL, HttpMethod::Put};
176176
};
177+
[[nodiscard]] HttpBodyRequest del(const std::string &URL) {
178+
return HttpBodyRequest{*this, URL, HttpMethod::Delete};
179+
};
177180

178181
private:
179182
CURL *curl_handle = nullptr;
@@ -190,6 +193,7 @@ class HttpClient {
190193
HttpResponse execute_get(HttpRequest &request);
191194
HttpResponse execute_head(HttpRequest &request);
192195
HttpResponse execute_post(HttpBodyRequest &request);
196+
HttpResponse execute_delete(HttpBodyRequest &request);
193197

194198
const std::unordered_map<std::string, std::string> &getHeaders() const {
195199
return headers_;

‎test/httpclient_test.cpp‎

Lines changed: 53 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,10 @@ TEST(HTTP, HTTPClientMoveOwnership) {
4646

4747
TEST(HTTP, HTTPBodyNonEmpty) {
4848
HttpClient client{};
49-
try {
50-
HttpResponse request =
51-
client.get("https://postman-echo.com/get?foo=bar").execute();
52-
EXPECT_TRUE(request.is_ok());
53-
EXPECT_THAT(request.body(), testing::HasSubstr("\"foo\":\"bar\""));
54-
55-
} catch (const std::exception &e) {
56-
FAIL() << std::format("Request failed with exception: {}", e.what());
57-
}
49+
HttpResponse request =
50+
client.get("https://postman-echo.com/get?foo=bar").execute();
51+
EXPECT_TRUE(request.is_ok());
52+
EXPECT_THAT(request.body(), testing::HasSubstr("\"foo\":\"bar\""));
5853
}
5954

6055
TEST(HTTP, HTTPHandleTimeout) {
@@ -119,17 +114,12 @@ TEST(HTTP, HTTPClientDefaultHeadersOverwrittenByRequestHeaders) {
119114
.timeout(10)
120115
.header("User-Agent", val);
121116

122-
try {
123-
auto resp = request.execute();
124-
125-
// request should work + having the new user-agent header each time
126-
EXPECT_TRUE(resp.is_ok());
127-
EXPECT_THAT(resp.body(), testing::HasSubstr(
128-
std::format("\"user-agent\":\"{}\"", val)));
117+
auto resp = request.execute();
129118

130-
} catch (const std::exception &e) {
131-
FAIL() << std::format("Request failed: {}", e.what());
132-
}
119+
// request should work + having the new user-agent header each time
120+
EXPECT_TRUE(resp.is_ok());
121+
EXPECT_THAT(resp.body(),
122+
testing::HasSubstr(std::format("\"user-agent\":\"{}\"", val)));
133123
}
134124
}
135125

@@ -143,17 +133,13 @@ TEST(HTTP, HTTPRemoveHeader) {
143133
// in which case, when merging request and client headers, if we remove a
144134
// request one, the client one should also be removed!
145135

146-
try {
147-
// Removes the User-Agent header
148-
auto resp = client.get("https://postman-echo.com/get")
149-
.timeout(10)
150-
.header("User-Agent", "")
151-
.execute();
152-
EXPECT_THAT(resp.body(),
153-
testing::Not(testing::HasSubstr("\"user-agent\":\"client\"")));
154-
} catch (const std::exception &e) {
155-
FAIL() << std::format("Request failed: {}", e.what());
156-
}
136+
// Removes the User-Agent header
137+
auto resp = client.get("https://postman-echo.com/get")
138+
.timeout(10)
139+
.header("User-Agent", "")
140+
.execute();
141+
EXPECT_THAT(resp.body(),
142+
testing::Not(testing::HasSubstr("\"user-agent\":\"client\"")));
157143
}
158144

159145
TEST(HTTP, HTTPHead) {
@@ -201,35 +187,60 @@ TEST(HTTP, HTTPHeaderOrder) {
201187

202188
TEST(HTTP, HTTPPost) {
203189
HttpClient client{};
204-
std::string data = "This is expected to be sent back as part of response body";
205-
HttpBodyRequest req = client.post("https://postman-echo.com/post").body(data);
190+
std::string data =
191+
"This is expected to be sent back as part of response body";
192+
HttpBodyRequest req = client.post("https://postman-echo.com/post").body(data);
206193
EXPECT_EQ(req.getBody(), data);
207194
HttpResponse resp = req.execute();
208195
EXPECT_TRUE(resp.is_ok());
209196
EXPECT_EQ(resp.status(), 200);
210-
EXPECT_THAT(resp.body(), testing::HasSubstr(data));
197+
EXPECT_THAT(resp.body(), testing::HasSubstr(data));
211198
}
212199

213200
// NOTE(cristian): This is done at compile time, duh, nothing to check
214201
/*
215202
TEST(HTTP, HTTPGetHeadCRTP) {
216-
// This validates our changes we did with CRTP
217-
// When issuing a GET/HEAD we cannot do a .body()
218-
// this is determined at compile time
219-
HttpClient client{};
220-
HttpRequest req = client.head("https://postman-echo.com/get?foo0=bar1&foo2=bar2");
221-
// Check that we cannot do body
203+
// This validates our changes we did with CRTP
204+
// When issuing a GET/HEAD we cannot do a .body()
205+
// this is determined at compile time
206+
HttpClient client{};
207+
HttpRequest req =
208+
client.head("https://postman-echo.com/get?foo0=bar1&foo2=bar2");
209+
// Check that we cannot do body
222210
}
223211
*/
224212

225213
TEST(HTTP, HTTPPut) {
226214
HttpClient client{};
227-
std::string data = "This is expected to be sent back as part of response body";
228-
HttpBodyRequest req = client.put("https://postman-echo.com/post").body(data);
215+
std::string data =
216+
"This is expected to be sent back as part of response body";
217+
HttpBodyRequest req = client.put("https://postman-echo.com/post").body(data);
229218
EXPECT_EQ(req.getBody(), data);
230219
HttpResponse resp = req.execute();
231220
EXPECT_TRUE(resp.is_ok());
232221
EXPECT_EQ(resp.status(), 200);
233-
EXPECT_THAT(resp.body(), testing::HasSubstr(data));
222+
EXPECT_THAT(resp.body(), testing::HasSubstr(data));
234223
}
235224

225+
TEST(HTTP, HTTPDeleteQueryParamStr) {
226+
HttpClient client{};
227+
std::string query = "deletethis";
228+
HttpBodyRequest req =
229+
client.del(std::format("https://postman-echo.com/patch?{}", query));
230+
HttpResponse resp = req.execute();
231+
EXPECT_TRUE(resp.is_ok());
232+
EXPECT_EQ(resp.status(), 200);
233+
EXPECT_THAT(resp.body(), testing::HasSubstr(query));
234+
}
235+
236+
TEST(HTTP, HTTPDeleteWithBody) {
237+
HttpClient client{};
238+
std::string data = "This is expected to be deleted";
239+
HttpBodyRequest req =
240+
client.del("https://postman-echo.com/patch").body(data);
241+
EXPECT_EQ(req.getBody(), data);
242+
HttpResponse resp = req.execute();
243+
EXPECT_TRUE(resp.is_ok());
244+
EXPECT_EQ(resp.status(), 200);
245+
EXPECT_THAT(resp.body(), testing::HasSubstr(data));
246+
}

0 commit comments

Comments
 (0)