Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: Move a few internal APIs from std::string to absl::string_view. #2889

Merged
merged 3 commits into from
Mar 25, 2018
Merged

server: Move a few internal APIs from std::string to absl::string_view. #2889

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10f225f
Move a few internal APIs from std::string to absl::string_view.
jmarantz Mar 23, 2018
6e19a68
Make arg-names consistent.
jmarantz Mar 23, 2018
f985810
Remove superfluous std::string conversion.
jmarantz Mar 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions include/envoy/http/header_map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include "envoy/common/pure.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Http {

Expand Down Expand Up @@ -87,9 +89,11 @@ class HeaderString {
const char* c_str() const { return buffer_.ref_; }

/**
* @return a std::string.
* @return an absl::string_view.
*/
std::string getString() const { return std::string(buffer_.ref_, string_length_); }
absl::string_view getStringView() const {
return absl::string_view(buffer_.ref_, string_length_);
}

/**
* Return the string to a default state. Reference strings are not touched. Both inline/dynamic
Expand Down
12 changes: 8 additions & 4 deletions include/envoy/server/admin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "envoy/http/header_map.h"
#include "envoy/network/listen_socket.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Server {

Expand All @@ -19,8 +21,10 @@ namespace Server {
* done in the RouteConfigProviderManagerImpl constructor in source/common/router/rds_impl.cc.
*/
#define MAKE_ADMIN_HANDLER(X) \
[this](const std::string& url, Http::HeaderMap& response_headers, \
Buffer::Instance& data) -> Http::Code { return X(url, response_headers, data); }
[this](absl::string_view path_and_query, Http::HeaderMap& response_headers, \
Buffer::Instance& data) -> Http::Code { \
return X(path_and_query, response_headers, data); \
}

/**
* Global admin HTTP endpoint for the server.
Expand All @@ -37,8 +41,8 @@ class Admin {
* @param response supplies the buffer to fill in with the response body.
* @return Http::Code the response code.
*/
typedef std::function<Http::Code(const std::string& url, Http::HeaderMap& response_headers,
Buffer::Instance& response)>
typedef std::function<Http::Code(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response)>
HandlerCb;

/**
Expand Down
4 changes: 2 additions & 2 deletions source/common/common/utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ std::string StringUtil::join(const std::vector<std::string>& source, const std::
return ret.substr(0, ret.length() - delimiter.length());
}

std::string StringUtil::subspan(const std::string& source, size_t start, size_t end) {
return source.substr(start, end - start);
std::string StringUtil::subspan(absl::string_view source, size_t start, size_t end) {
return std::string(source.data() + start, end - start);
}

std::string StringUtil::escape(const std::string& source) {
Expand Down
3 changes: 2 additions & 1 deletion source/common/common/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,9 @@ class StringUtil {
/**
* Version of substr() that operates on a start and end index instead of a start index and a
* length.
* @return string substring starting at start, and ending right before end.
*/
static std::string subspan(const std::string& source, size_t start, size_t end);
static std::string subspan(absl::string_view source, size_t start, size_t end);

/**
* Escape strings for logging purposes. Returns a copy of the string with
Expand Down
7 changes: 5 additions & 2 deletions source/common/ext_authz/ext_authz_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ void CheckRequestUtils::setAttrContextPeer(envoy::service::auth::v2::AttributeCo

std::string CheckRequestUtils::getHeaderStr(const Envoy::Http::HeaderEntry* entry) {
if (entry) {
return entry->value().getString();
// TODO(jmarantz): plumb absl::string_view further here; there's no need
// to allocate a temp string in the local uses.
return std::string(entry->value().getStringView());
}
return "";
}
Expand Down Expand Up @@ -148,7 +150,8 @@ void CheckRequestUtils::setHttpRequest(
mutable_headers = static_cast<
Envoy::Protobuf::Map<Envoy::ProtobufTypes::String, Envoy::ProtobufTypes::String>*>(
ctx);
(*mutable_headers)[e.key().getString()] = e.value().getString();
(*mutable_headers)[std::string(e.key().getStringView())] =
std::string(e.value().getStringView());
return Envoy::Http::HeaderMap::Iterate::Continue;
},
mutable_headers);
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ std::string Utility::createSslRedirectPath(const HeaderMap& headers) {
headers.Path()->value().c_str());
}

Utility::QueryParams Utility::parseQueryString(const std::string& url) {
Utility::QueryParams Utility::parseQueryString(absl::string_view url) {
QueryParams params;
size_t start = url.find('?');
if (start == std::string::npos) {
Expand All @@ -57,7 +57,7 @@ Utility::QueryParams Utility::parseQueryString(const std::string& url) {
if (end == std::string::npos) {
end = url.size();
}
absl::string_view param(url.c_str() + start, end - start);
absl::string_view param(url.data() + start, end - start);

const size_t equal = param.find('=');
if (equal != std::string::npos) {
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Utility {
* @param url supplies the url to parse.
* @return QueryParams the parsed parameters, if any.
*/
static QueryParams parseQueryString(const std::string& url);
static QueryParams parseQueryString(absl::string_view url);

/**
* Finds the start of the query string in a path
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/rds_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getRouteCon
return new_provider;
};

Http::Code RouteConfigProviderManagerImpl::handlerRoutes(const std::string& url, Http::HeaderMap&,
Http::Code RouteConfigProviderManagerImpl::handlerRoutes(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);
// If there are no query params, print out all the configured route tables.
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/rds_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager,
* @param response supplies the buffer to fill with information.
* @return Http::Code OK if the endpoint can parse and operate on the url, NotFound otherwise.
*/
Http::Code handlerRoutes(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerRoutes(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);

/**
Expand Down
5 changes: 2 additions & 3 deletions source/common/tracing/zipkin/zipkin_tracer_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, Http::HeaderMa
if (request_headers.XB3Sampled()) {
// Checking if sampled flag has been specified. Also checking for 'true' value, as some old
// zipkin tracers may still use that value, although should be 0 or 1.
sampled =
request_headers.XB3Sampled()->value().getString() == ZipkinCoreConstants::get().SAMPLED ||
request_headers.XB3Sampled()->value().getString() == "true";
absl::string_view xb3_sampled = request_headers.XB3Sampled()->value().getStringView();
sampled = xb3_sampled == ZipkinCoreConstants::get().SAMPLED || xb3_sampled == "true";
}

if (request_headers.XB3TraceId() && request_headers.XB3SpanId()) {
Expand Down
40 changes: 20 additions & 20 deletions source/server/http/admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "common/upstream/host_utility.h"

#include "absl/strings/str_replace.h"
#include "absl/strings/string_view.h"

// TODO(mattklein123): Switch to JSON interface methods and remove rapidjson dependency.
#include "rapidjson/document.h"
Expand Down Expand Up @@ -218,7 +219,7 @@ void AdminImpl::addCircuitSettings(const std::string& cluster_name, const std::s
resource_manager.retries().max()));
}

Http::Code AdminImpl::handlerClusters(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerClusters(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
response.add(fmt::format("version_info::{}\n", server_.clusterManager().versionInfo()));

Expand Down Expand Up @@ -275,9 +276,9 @@ Http::Code AdminImpl::handlerClusters(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Http::HeaderMap&,
Http::Code AdminImpl::handlerCpuProfiler(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(std::string(url));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid this conversion/allocation if you push one level deeper into this function. I think it would be straightforward without affecting other functions, at least at first glance. But up to you if you want to do that or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't need that construction. Good catch.

if (query_params.size() != 1 || query_params.begin()->first != "enable" ||
(query_params.begin()->second != "y" && query_params.begin()->second != "n")) {
response.add("?enable=<y|n>\n");
Expand All @@ -299,27 +300,27 @@ Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Http::HeaderMap
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHealthcheckFail(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHealthcheckFail(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.failHealthcheck(true);
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHealthcheckOk(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHealthcheckOk(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.failHealthcheck(false);
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHotRestartVersion(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHotRestartVersion(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
response.add(server_.hotRestart().version());
return Http::Code::OK;
}

Http::Code AdminImpl::handlerLogging(const std::string& url, Http::HeaderMap&,
Http::Code AdminImpl::handlerLogging(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);

Expand All @@ -345,7 +346,7 @@ Http::Code AdminImpl::handlerLogging(const std::string& url, Http::HeaderMap&,
return rc;
}

Http::Code AdminImpl::handlerResetCounters(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerResetCounters(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
for (const Stats::CounterSharedPtr& counter : server_.stats().counters()) {
counter->reset();
Expand All @@ -355,7 +356,7 @@ Http::Code AdminImpl::handlerResetCounters(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerServerInfo(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerServerInfo(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
time_t current_time = time(nullptr);
response.add(fmt::format("envoy {} {} {} {} {}\n", VersionInfo::version(),
Expand All @@ -366,7 +367,7 @@ Http::Code AdminImpl::handlerServerInfo(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerStats(const std::string& url, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerStats(absl::string_view url, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
// We currently don't support timers locally (only via statsd) so just group all the counters
// and gauges together, alpha sort them, and spit them out.
Expand Down Expand Up @@ -477,14 +478,14 @@ std::string AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_st
return strbuf.GetString();
}

Http::Code AdminImpl::handlerQuitQuitQuit(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerQuitQuitQuit(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.shutdown();
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerListenerInfo(const std::string&, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerListenerInfo(absl::string_view, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
response_headers.insertContentType().value().setReference(
Http::Headers::get().ContentTypeValues.Json);
Expand All @@ -496,7 +497,7 @@ Http::Code AdminImpl::handlerListenerInfo(const std::string&, Http::HeaderMap& r
return Http::Code::OK;
}

Http::Code AdminImpl::handlerCerts(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerCerts(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
// This set is used to track distinct certificates. We may have multiple listeners, upstreams, etc
// using the same cert.
Expand All @@ -515,7 +516,7 @@ Http::Code AdminImpl::handlerCerts(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerRuntime(const std::string& url, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerRuntime(absl::string_view url, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
Http::Code rc = Http::Code::OK;
const Http::Utility::QueryParams params = Http::Utility::parseQueryString(url);
Expand Down Expand Up @@ -585,7 +586,7 @@ std::string AdminImpl::runtimeAsJson(
}

void AdminFilter::onComplete() {
std::string path = request_headers_->Path()->value().c_str();
absl::string_view path = request_headers_->Path()->value().getStringView();
ENVOY_STREAM_LOG(debug, "request complete: path: {}", *callbacks_, path);

Buffer::OwnedImpl response;
Expand Down Expand Up @@ -686,7 +687,7 @@ void AdminImpl::createFilterChain(Http::FilterChainFactoryCallbacks& callbacks)
callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{new AdminFilter(*this)});
}

Http::Code AdminImpl::runCallback(const std::string& path_and_query,
Http::Code AdminImpl::runCallback(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response) {
Http::Code code = Http::Code::OK;
bool found_handler = false;
Expand Down Expand Up @@ -722,12 +723,11 @@ std::vector<const AdminImpl::UrlHandler*> AdminImpl::sortedHandlers() const {
}
// Note: it's generally faster to sort a vector with std::vector than to construct a std::map.
std::sort(sorted_handlers.begin(), sorted_handlers.end(),
[&](const UrlHandler* h1, const UrlHandler* h2) { return h1->prefix_ < h2->prefix_; });
[](const UrlHandler* h1, const UrlHandler* h2) { return h1->prefix_ < h2->prefix_; });
return sorted_handlers;
}

Http::Code AdminImpl::handlerHelp(const std::string&, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Code AdminImpl::handlerHelp(absl::string_view, Http::HeaderMap&, Buffer::Instance& response) {
response.add("admin commands are:\n");

// Prefix order is used during searching, but for printing do them in alpha order.
Expand All @@ -737,7 +737,7 @@ Http::Code AdminImpl::handlerHelp(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerAdminHome(const std::string&, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerAdminHome(absl::string_view, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
response_headers.insertContentType().value().setReference(
Http::Headers::get().ContentTypeValues.Html);
Expand Down
36 changes: 19 additions & 17 deletions source/server/http/admin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include "server/config/network/http_connection_manager.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Server {

Expand All @@ -41,7 +43,7 @@ class AdminImpl : public Admin,
const std::string& address_out_path, Network::Address::InstanceConstSharedPtr address,
Server::Instance& server, Stats::ScopePtr&& listener_scope);

Http::Code runCallback(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code runCallback(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
const Network::Socket& socket() override { return *socket_; }
Network::Socket& mutable_socket() { return *socket_; }
Expand Down Expand Up @@ -136,37 +138,37 @@ class AdminImpl : public Admin,
/**
* URL handlers.
*/
Http::Code handlerAdminHome(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerAdminHome(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerCerts(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerCerts(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerClusters(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerClusters(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerCpuProfiler(const std::string& path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHealthcheckFail(const std::string& path_and_query,
Http::Code handlerCpuProfiler(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerHealthcheckFail(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHealthcheckOk(const std::string& path_and_query,
Http::Code handlerHealthcheckOk(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHelp(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerHelp(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerHotRestartVersion(const std::string& path_and_query,
Http::Code handlerHotRestartVersion(absl::string_view path_and_query,
Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerListenerInfo(const std::string& path_and_query,
Http::Code handlerListenerInfo(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerLogging(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerLogging(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerMain(const std::string& path, Buffer::Instance& response);
Http::Code handlerQuitQuitQuit(const std::string& path_and_query,
Http::Code handlerQuitQuitQuit(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerResetCounters(const std::string& path_and_query,
Http::Code handlerResetCounters(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerServerInfo(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerServerInfo(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerStats(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerStats(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerRuntime(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerRuntime(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);

class AdminListener : public Network::ListenerConfig {
Expand Down
Loading