Skip to content

Commit d7f98cc

Browse files
matyhtfmatyhtf
authored
Optimize code. Optimize core tests. Improve code quality. (#5749)
* Optimize core tests [1] --filter=[core] * Optimize core tests [2], Optimize server code * Optimize server code [2] * Fix tests * Fix tests * Added more tests * Fix * Fix 2 * Fix 3 --filter=[core] * Fix 4 --filter=[core] * Fix 5 --filter=[core] * Added System::get_dns_cache_hit_ratio(), fix tests --filter=[core] * Optimize logger impl, add Logger::set_stream() --filter=[core] * Fix tests [6] --filter=[core] * Fix tests [7] --filter=[core] * Fix tests [8] --filter=[core] * Optimize tests [9] --filter=[core]
1 parent 26223cb commit d7f98cc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+849
-429
lines changed

‎core-tests/include/httplib_client.h

Lines changed: 78 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,11 @@ struct Response {
375375
void set_content(const char *s, size_t n, const char *content_type);
376376
void set_content(std::string s, const char *content_type);
377377

378-
void set_content_provider(size_t length, ContentProvider provider, std::function<void()> resource_releaser = [] {});
378+
void set_content_provider(
379+
size_t length, ContentProvider provider, std::function<void()> resource_releaser = [] {});
379380

380-
void set_chunked_content_provider(ChunkedContentProvider provider, std::function<void()> resource_releaser = [] {});
381+
void set_chunked_content_provider(
382+
ChunkedContentProvider provider, std::function<void()> resource_releaser = [] {});
381383

382384
Response() = default;
383385
Response(const Response &) = default;
@@ -417,7 +419,7 @@ class Stream {
417419
virtual void get_remote_ip_and_port(std::string &ip, int &port) const = 0;
418420

419421
template <typename... Args>
420-
ssize_t write_format(const char *fmt, const Args &... args);
422+
ssize_t write_format(const char *fmt, const Args &...args);
421423
ssize_t write(const char *ptr);
422424
ssize_t write(const std::string &s);
423425
};
@@ -2970,16 +2972,17 @@ inline std::string make_multipart_ranges_data(const Request &req,
29702972
const std::string &content_type) {
29712973
std::string data;
29722974

2973-
process_multipart_ranges_data(req,
2974-
res,
2975-
boundary,
2976-
content_type,
2977-
[&](const std::string &token) { data += token; },
2978-
[&](const char *token) { data += token; },
2979-
[&](size_t offset, size_t length) {
2980-
data += res.body.substr(offset, length);
2981-
return true;
2982-
});
2975+
process_multipart_ranges_data(
2976+
req,
2977+
res,
2978+
boundary,
2979+
content_type,
2980+
[&](const std::string &token) { data += token; },
2981+
[&](const char *token) { data += token; },
2982+
[&](size_t offset, size_t length) {
2983+
data += res.body.substr(offset, length);
2984+
return true;
2985+
});
29832986

29842987
return data;
29852988
}
@@ -2990,16 +2993,17 @@ inline size_t get_multipart_ranges_data_length(const Request &req,
29902993
const std::string &content_type) {
29912994
size_t data_length = 0;
29922995

2993-
process_multipart_ranges_data(req,
2994-
res,
2995-
boundary,
2996-
content_type,
2997-
[&](const std::string &token) { data_length += token.size(); },
2998-
[&](const char *token) { data_length += strlen(token); },
2999-
[&](size_t /*offset*/, size_t length) {
3000-
data_length += length;
3001-
return true;
3002-
});
2996+
process_multipart_ranges_data(
2997+
req,
2998+
res,
2999+
boundary,
3000+
content_type,
3001+
[&](const std::string &token) { data_length += token.size(); },
3002+
[&](const char *token) { data_length += strlen(token); },
3003+
[&](size_t /*offset*/, size_t length) {
3004+
data_length += length;
3005+
return true;
3006+
});
30033007

30043008
return data_length;
30053009
}
@@ -3362,8 +3366,8 @@ inline ssize_t Stream::write(const std::string &s) {
33623366
}
33633367

33643368
template <typename... Args>
3365-
inline ssize_t Stream::write_format(const char *fmt, const Args &... args) {
3366-
std::array<char, 2048> buf;
3369+
inline ssize_t Stream::write_format(const char *fmt, const Args &...args) {
3370+
std::array<char, 2048> buf{};
33673371

33683372
#if defined(_MSC_VER) && _MSC_VER < 1900
33693373
auto sn = _snprintf_s(buf, bufsiz, buf.size() - 1, fmt, args...);
@@ -3826,54 +3830,55 @@ inline bool SSLClient::connect_with_proxy(Socket &socket, Response &res, bool &s
38263830
}
38273831

38283832
inline bool SSLClient::initialize_ssl(Socket &socket) {
3829-
auto ssl = detail::ssl_new(socket.sock,
3830-
ctx_,
3831-
ctx_mutex_,
3832-
[&](SSL *ssl) {
3833-
if (ca_cert_file_path_.empty() && ca_cert_store_ == nullptr) {
3834-
SSL_CTX_set_verify(ctx_, SSL_VERIFY_NONE, nullptr);
3835-
} else if (!ca_cert_file_path_.empty()) {
3836-
if (!SSL_CTX_load_verify_locations(ctx_, ca_cert_file_path_.c_str(), nullptr)) {
3837-
return false;
3838-
}
3839-
SSL_CTX_set_verify(ctx_, SSL_VERIFY_PEER, nullptr);
3840-
} else if (ca_cert_store_ != nullptr) {
3841-
if (SSL_CTX_get_cert_store(ctx_) != ca_cert_store_) {
3842-
SSL_CTX_set_cert_store(ctx_, ca_cert_store_);
3843-
}
3844-
SSL_CTX_set_verify(ctx_, SSL_VERIFY_PEER, nullptr);
3845-
}
3846-
3847-
if (SSL_connect(ssl) != 1) {
3848-
return false;
3849-
}
3850-
3851-
if (server_certificate_verification_) {
3852-
verify_result_ = SSL_get_verify_result(ssl);
3853-
3854-
if (verify_result_ != X509_V_OK) {
3855-
return false;
3856-
}
3857-
3858-
auto server_cert = SSL_get_peer_certificate(ssl);
3859-
3860-
if (server_cert == nullptr) {
3861-
return false;
3862-
}
3863-
3864-
if (!verify_host(server_cert)) {
3865-
X509_free(server_cert);
3866-
return false;
3867-
}
3868-
X509_free(server_cert);
3869-
}
3870-
3871-
return true;
3872-
},
3873-
[&](SSL *ssl) {
3874-
SSL_set_tlsext_host_name(ssl, host_.c_str());
3875-
return true;
3876-
});
3833+
auto ssl = detail::ssl_new(
3834+
socket.sock,
3835+
ctx_,
3836+
ctx_mutex_,
3837+
[&](SSL *ssl) {
3838+
if (ca_cert_file_path_.empty() && ca_cert_store_ == nullptr) {
3839+
SSL_CTX_set_verify(ctx_, SSL_VERIFY_NONE, nullptr);
3840+
} else if (!ca_cert_file_path_.empty()) {
3841+
if (!SSL_CTX_load_verify_locations(ctx_, ca_cert_file_path_.c_str(), nullptr)) {
3842+
return false;
3843+
}
3844+
SSL_CTX_set_verify(ctx_, SSL_VERIFY_PEER, nullptr);
3845+
} else if (ca_cert_store_ != nullptr) {
3846+
if (SSL_CTX_get_cert_store(ctx_) != ca_cert_store_) {
3847+
SSL_CTX_set_cert_store(ctx_, ca_cert_store_);
3848+
}
3849+
SSL_CTX_set_verify(ctx_, SSL_VERIFY_PEER, nullptr);
3850+
}
3851+
3852+
if (SSL_connect(ssl) != 1) {
3853+
return false;
3854+
}
3855+
3856+
if (server_certificate_verification_) {
3857+
verify_result_ = SSL_get_verify_result(ssl);
3858+
3859+
if (verify_result_ != X509_V_OK) {
3860+
return false;
3861+
}
3862+
3863+
auto server_cert = SSL_get_peer_certificate(ssl);
3864+
3865+
if (server_cert == nullptr) {
3866+
return false;
3867+
}
3868+
3869+
if (!verify_host(server_cert)) {
3870+
X509_free(server_cert);
3871+
return false;
3872+
}
3873+
X509_free(server_cert);
3874+
}
3875+
3876+
return true;
3877+
},
3878+
[&](SSL *ssl) {
3879+
SSL_set_tlsext_host_name(ssl, host_.c_str());
3880+
return true;
3881+
});
38773882

38783883
if (ssl) {
38793884
socket.ssl = ssl;

‎core-tests/include/test_core.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ int get_random_port();
6262
Socks5Proxy *create_socks5_proxy();
6363
HttpProxy *create_http_proxy();
6464

65-
pid_t child_proc(const std::function<void(void)> &fn);
65+
pid_t spawn_exec(const std::function<void(void)> &fn);
66+
int spawn_exec_and_wait(const std::function<void(void)> &fn);
6667

6768
} // namespace test
6869
}; // namespace swoole

‎core-tests/src/_lib/http.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
namespace websocket = swoole::websocket;
66

7-
using swoole::String;
87
using swoole::Protocol;
8+
using swoole::String;
99

1010
namespace httplib {
1111

@@ -173,7 +173,7 @@ void Client::close_socket(Socket &socket, bool /*process_socket_ret*/) {
173173
}
174174

175175
bool Client::read_response_line(Stream &strm, Response &res) {
176-
std::array<char, 2048> buf;
176+
std::array<char, 2048> buf{};
177177

178178
detail::stream_line_reader line_reader(strm, buf.data(), buf.size());
179179

‎core-tests/src/core/base.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,6 @@ TEST(base, version) {
219219
ASSERT_EQ(swoole_api_version_id(), SWOOLE_API_VERSION_ID);
220220
}
221221

222-
static std::string test_func(std::string test_data_2) {
223-
return test_data + test_data_2;
224-
}
225-
226222
TEST(base, hook) {
227223
int count = 0;
228224
swoole_add_hook(

‎core-tests/src/core/log.cpp

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -117,27 +117,52 @@ TEST(log, rotation) {
117117
}
118118
}
119119

120-
TEST(log, redirect) {
121-
if (test::is_github_ci()) {
122-
return;
123-
}
124-
sw_logger()->reset();
125-
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(1)); // no log file opened
126-
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(0)); // no redirected
120+
TEST(log, redirect_1) {
121+
auto status = test::spawn_exec_and_wait([]() {
122+
sw_logger()->reset();
123+
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(1)); // no log file opened
124+
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(0)); // no redirected
127125

128-
ASSERT_TRUE(sw_logger()->open(file));
129-
ASSERT_TRUE(sw_logger()->redirect_stdout_and_stderr(1));
130-
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(1)); // has been redirected
126+
ASSERT_TRUE(sw_logger()->open(file));
127+
ASSERT_TRUE(sw_logger()->redirect_stdout_and_stderr(1));
128+
ASSERT_FALSE(sw_logger()->redirect_stdout_and_stderr(1)); // has been redirected
131129

132-
printf("hello world\n");
133-
auto content = file_get_contents(file);
134-
ASSERT_NE(content.get(), nullptr);
130+
printf("hello world\n");
131+
auto content = file_get_contents(file);
132+
ASSERT_NE(content.get(), nullptr);
135133

136-
sw_logger()->close();
137-
ASSERT_TRUE(sw_logger()->redirect_stdout_and_stderr(0));
138-
unlink(sw_logger()->get_real_file());
134+
sw_logger()->close();
135+
ASSERT_TRUE(sw_logger()->redirect_stdout_and_stderr(0));
136+
unlink(sw_logger()->get_real_file());
137+
138+
ASSERT_TRUE(content->contains(SW_STRL("hello world\n")));
139+
});
140+
141+
ASSERT_EQ(status, 0);
142+
}
143+
144+
TEST(log, redirect_2) {
145+
auto status = test::spawn_exec_and_wait([]() {
146+
auto file = "/tmp/swoole.log";
147+
auto str = "hello world, hello swoole\n";
148+
149+
sw_logger()->reset();
150+
sw_logger()->open(file);
151+
sw_logger()->redirect_stdout_and_stderr(true);
152+
153+
printf(str);
139154

140-
ASSERT_TRUE(content->contains(SW_STRL("hello world\n")));
155+
File f(file, File::READ);
156+
auto rs = f.read_content();
157+
158+
ASSERT_TRUE(rs->contains(str));
159+
sw_logger()->redirect_stdout_and_stderr(false);
160+
printf(str);
161+
sw_logger()->close();
162+
unlink(sw_logger()->get_real_file());
163+
});
164+
165+
ASSERT_EQ(status, 0);
141166
}
142167

143168
namespace TestA {
@@ -213,3 +238,21 @@ TEST(log, ignore_error) {
213238
ASSERT_FALSE(content->contains(SW_STRL("error 1")));
214239
ASSERT_TRUE(content->contains(SW_STRL("error 2")));
215240
}
241+
242+
TEST(log, open_fail) {
243+
sw_logger()->reset();
244+
sw_logger()->set_level(SW_LOG_NOTICE);
245+
sw_logger()->open("/tmp/not-exists/swoole.log");
246+
sw_logger()->put(SW_LOG_ERROR, SW_STRL("hello world\n"));
247+
}
248+
249+
TEST(log, set_stream) {
250+
sw_logger()->reset();
251+
char *buffer = NULL;
252+
size_t size = 0;
253+
FILE *stream = open_memstream(&buffer, &size);
254+
sw_logger()->set_stream(stream);
255+
sw_logger()->put(SW_LOG_ERROR, SW_STRL("hello world\n"));
256+
257+
ASSERT_NE(strstr(buffer, "ERROR\thello world"), nullptr);
258+
}

‎core-tests/src/coroutine/gethostbyname.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,18 @@ TEST(coroutine_gethostbyname, resolve_cache) {
1313
System::set_dns_cache_capacity(10);
1414
std::string addr1 = System::gethostbyname(domain_baidu, AF_INET);
1515
ASSERT_NE(addr1, "");
16-
int64_t with_cache = Timer::get_absolute_msec();
1716
for (int i = 0; i < 100; ++i) {
1817
std::string addr2 = System::gethostbyname(domain_baidu, AF_INET);
1918
ASSERT_EQ(addr1, addr2);
2019
}
21-
with_cache = Timer::get_absolute_msec() - with_cache;
20+
ASSERT_GT(System::get_dns_cache_hit_ratio(), 0.99);
2221

2322
System::set_dns_cache_capacity(0);
24-
int64_t without_cache = Timer::get_absolute_msec();
2523
for (int i = 0; i < 5; ++i) {
2624
std::string addr2 = System::gethostbyname(domain_baidu, AF_INET);
2725
ASSERT_NE(addr2, "");
2826
}
29-
without_cache = Timer::get_absolute_msec() - without_cache;
30-
31-
ASSERT_GT(without_cache, with_cache);
27+
ASSERT_LT(System::get_dns_cache_hit_ratio(), 0.01);
3228
});
3329
}
3430

0 commit comments

Comments
 (0)