Skip to content

Commit

Permalink
chore: reduce usage of ToUpper (#3874)
Browse files Browse the repository at this point in the history 
We would like to stop passing MutableSlice as arguments and removing ToUpper
is the first step to it.

Signed-off-by: Roman Gershman <[email protected]>
  • Loading branch information
@romange
romange authored Oct 6, 2024
1 parent 129ff0b commit 45aba13
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 82 deletions.
68 changes: 33 additions & 35 deletions src/server/acl/acl_family.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,22 @@
#include "server/server_state.h"
#include "util/proactor_pool.h"

ABSL_FLAG(std::string, aclfile, "", "Path and name to aclfile");
using namespace std;

ABSL_FLAG(string, aclfile, "", "Path and name to aclfile");

namespace dfly::acl {

namespace {

std::string PasswordsToString(const absl::flat_hash_set<std::string>& passwords, bool nopass,
bool full_sha);
using MaterializedContents = std::optional<std::vector<std::vector<std::string_view>>>;
string PasswordsToString(const absl::flat_hash_set<string>& passwords, bool nopass, bool full_sha);
using MaterializedContents = optional<vector<vector<string_view>>>;

MaterializedContents MaterializeFileContents(std::vector<std::string>* usernames,
std::string_view file_contents);
MaterializedContents MaterializeFileContents(vector<string>* usernames, string_view file_contents);

std::string AclKeysToString(const AclKeys& keys);
string AclKeysToString(const AclKeys& keys);

std::string AclPubSubToString(const AclPubSub& pub_sub);
string AclPubSubToString(const AclPubSub& pub_sub);
} // namespace

AclFamily::AclFamily(UserRegistry* registry, util::ProactorPool* pool)
Expand All @@ -74,19 +74,19 @@ void AclFamily::List(CmdArgList args, ConnectionContext* cntx) {
rb->StartArray(registry.size());

for (const auto& [username, user] : registry) {
std::string buffer = "user ";
const std::string password = PasswordsToString(user.Passwords(), user.HasNopass(), false);
string buffer = "user ";
const string password = PasswordsToString(user.Passwords(), user.HasNopass(), false);

const std::string acl_keys = AclKeysToString(user.Keys());
const string acl_keys = AclKeysToString(user.Keys());

const std::string acl_pub_sub = AclPubSubToString(user.PubSub());
const string acl_pub_sub = AclPubSubToString(user.PubSub());

const std::string maybe_space_com = acl_keys.empty() ? "" : " ";
const string maybe_space_com = acl_keys.empty() ? "" : " ";

const std::string acl_cat_and_commands =
const string acl_cat_and_commands =
AclCatAndCommandToString(user.CatChanges(), user.CmdChanges());

using namespace std::string_view_literals;
using namespace string_view_literals;

absl::StrAppend(&buffer, username, " ", user.IsActive() ? "on "sv : "off "sv, password,
acl_keys, maybe_space_com, acl_pub_sub, " ", acl_cat_and_commands);
Expand Down Expand Up @@ -117,7 +117,7 @@ void AclFamily::StreamUpdatesToAllProactorConnections(const std::string& user,
using facade::ErrorReply;

void AclFamily::SetUser(CmdArgList args, ConnectionContext* cntx) {
std::string_view username = facade::ToSV(args[0]);
string_view username = facade::ToSV(args[0]);
auto reg = registry_->GetRegistryWithWriteLock();
const bool exists = reg.registry.contains(username);
const bool has_all_keys = exists ? reg.registry.find(username)->second.Keys().all_keys : false;
Expand All @@ -140,8 +140,8 @@ void AclFamily::SetUser(CmdArgList args, ConnectionContext* cntx) {
cntx->SendOk();
if (exists) {
if (!reset_channels) {
StreamUpdatesToAllProactorConnections(std::string(username), user.AclCommands(),
user.Keys(), user.PubSub());
StreamUpdatesToAllProactorConnections(string(username), user.AclCommands(), user.Keys(),
user.PubSub());
}
// We evict connections that had their channels reseted
else {
Expand All @@ -153,8 +153,7 @@ void AclFamily::SetUser(CmdArgList args, ConnectionContext* cntx) {
std::visit(Overloaded{error_case, update_case}, std::move(req));
}

void AclFamily::EvictOpenConnectionsOnAllProactors(
const absl::flat_hash_set<std::string_view>& users) {
void AclFamily::EvictOpenConnectionsOnAllProactors(const absl::flat_hash_set<string_view>& users) {
auto close_cb = [&users]([[maybe_unused]] size_t id, util::Connection* conn) {
CHECK(conn);
auto connection = static_cast<facade::Connection*>(conn);
Expand Down Expand Up @@ -187,10 +186,10 @@ void AclFamily::EvictOpenConnectionsOnAllProactorsWithRegistry(

void AclFamily::DelUser(CmdArgList args, ConnectionContext* cntx) {
auto& registry = *registry_;
absl::flat_hash_set<std::string_view> users;
absl::flat_hash_set<string_view> users;

for (auto arg : args) {
std::string_view username = facade::ToSV(arg);
string_view username = facade::ToSV(arg);
if (username == "default") {
continue;
}
Expand All @@ -214,24 +213,24 @@ void AclFamily::WhoAmI(CmdArgList args, ConnectionContext* cntx) {
rb->SendBulkString(absl::StrCat("User is ", cntx->authed_username));
}

std::string AclFamily::RegistryToString() const {
string AclFamily::RegistryToString() const {
auto registry_with_read_lock = registry_->GetRegistryWithLock();
auto& registry = registry_with_read_lock.registry;
std::string result;
string result;
for (auto& [username, user] : registry) {
std::string command = "USER ";
const std::string password = PasswordsToString(user.Passwords(), user.HasNopass(), true);
string command = "USER ";
const string password = PasswordsToString(user.Passwords(), user.HasNopass(), true);

const std::string acl_keys = AclKeysToString(user.Keys());
const string acl_keys = AclKeysToString(user.Keys());

const std::string maybe_space = acl_keys.empty() ? "" : " ";
const string maybe_space = acl_keys.empty() ? "" : " ";

const std::string acl_pub_sub = AclPubSubToString(user.PubSub());
const string acl_pub_sub = AclPubSubToString(user.PubSub());

const std::string acl_cat_and_commands =
const string acl_cat_and_commands =
AclCatAndCommandToString(user.CatChanges(), user.CmdChanges());

using namespace std::string_view_literals;
using namespace string_view_literals;

absl::StrAppend(&result, command, username, " ", user.IsActive() ? "ON "sv : "OFF "sv, password,
acl_keys, maybe_space, acl_pub_sub, " ", acl_cat_and_commands, "\n");
Expand Down Expand Up @@ -471,8 +470,8 @@ void AclFamily::Cat(CmdArgList args, ConnectionContext* cntx) {
}

if (args.size() == 1) {
ToUpper(&args[0]);
std::string_view category = facade::ToSV(args[0]);
string category = absl::AsciiStrToUpper(ArgS(args, 0));

if (!cat_table_.contains(category)) {
auto error = absl::StrCat("Unkown category: ", category);
cntx->SendError(error);
Expand Down Expand Up @@ -605,8 +604,7 @@ void AclFamily::DryRun(CmdArgList args, ConnectionContext* cntx) {
return;
}

ToUpper(&args[1]);
auto command = facade::ArgS(args, 1);
string command = absl::AsciiStrToUpper(ArgS(args, 1));
auto* cid = cmd_registry_->Find(command);
if (!cid) {
auto error = absl::StrCat("Command '", command, "' not found");
Expand Down
11 changes: 4 additions & 7 deletions src/server/cluster/cluster_family.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,7 @@ void ClusterFamily::KeySlot(CmdArgList args, ConnectionContext* cntx) {
void ClusterFamily::Cluster(CmdArgList args, ConnectionContext* cntx) {
// In emulated cluster mode, all slots are mapped to the same host, and number of cluster
// instances is thus 1.

ToUpper(&args[0]);
string_view sub_cmd = ArgS(args, 0);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 0));

if (!IsClusterEnabledOrEmulated()) {
return cntx->SendError(kClusterDisabled);
Expand Down Expand Up @@ -445,8 +443,7 @@ void ClusterFamily::DflyCluster(CmdArgList args, ConnectionContext* cntx) {
VLOG(2) << "Got DFLYCLUSTER command (NO_CLIENT_ID): " << args;
}

ToUpper(&args[0]);
string_view sub_cmd = ArgS(args, 0);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 0));
args.remove_prefix(1); // remove subcommand name
if (sub_cmd == "GETSLOTINFO") {
return DflyClusterGetSlotInfo(args, cntx);
Expand Down Expand Up @@ -747,8 +744,8 @@ void ClusterFamily::DflySlotMigrationStatus(CmdArgList args, ConnectionContext*
}

void ClusterFamily::DflyMigrate(CmdArgList args, ConnectionContext* cntx) {
ToUpper(&args[0]);
string_view sub_cmd = ArgS(args, 0);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 0));

args.remove_prefix(1);
if (sub_cmd == "INIT") {
InitMigration(args, cntx);
Expand Down
6 changes: 3 additions & 3 deletions src/server/memory_cmd.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,16 +303,16 @@ void MemoryCmd::ArenaStats(CmdArgList args) {
bool backing = false;
bool show_arenas = false;
if (args.size() >= 2) {
ToUpper(&args[1]);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 1));

if (ArgS(args, 1) == "SHOW") {
if (sub_cmd == "SHOW") {
if (args.size() != 2)
return cntx_->SendError(kSyntaxErr, kSyntaxErrType);
show_arenas = true;
} else {
unsigned tid_indx = 1;

if (ArgS(args, tid_indx) == "BACKING") {
if (sub_cmd == "BACKING") {
++tid_indx;
backing = true;
}
Expand Down
34 changes: 13 additions & 21 deletions src/server/stream_family.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1888,9 +1888,7 @@ optional<pair<AddTrimOpts, unsigned>> ParseAddOrTrimArgsOrReply(CmdArgList args,

unsigned id_indx = 1;
for (; id_indx < args.size(); ++id_indx) {
ToUpper(&args[id_indx]);
string_view arg = ArgS(args, id_indx);

string arg = absl::AsciiStrToUpper(ArgS(args, id_indx));
size_t remaining_args = args.size() - id_indx - 1;

if (is_xadd && arg == "NOMKSTREAM") {
Expand Down Expand Up @@ -2059,8 +2057,7 @@ absl::InlinedVector<streamID, 8> GetXclaimIds(CmdArgList& args) {

void ParseXclaimOptions(CmdArgList& args, ClaimOpts& opts, ConnectionContext* cntx) {
for (size_t i = 0; i < args.size(); ++i) {
ToUpper(&args[i]);
string_view arg = ArgS(args, i);
string arg = absl::AsciiStrToUpper(ArgS(args, i));
bool remaining_args = args.size() - i - 1 > 0;

if (remaining_args) {
Expand Down Expand Up @@ -2186,8 +2183,8 @@ void StreamFamily::XGroup(CmdArgList args, ConnectionContext* cntx) {

void StreamFamily::XInfo(CmdArgList args, ConnectionContext* cntx) {
auto* rb = static_cast<RedisReplyBuilder*>(cntx->reply_builder());
ToUpper(&args[0]);
string_view sub_cmd = ArgS(args, 0);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 0));

if (sub_cmd == "HELP") {
string_view help_arr[] = {"CONSUMERS <key> <groupname>",
" Show consumers of <groupname>.",
Expand Down Expand Up @@ -2254,15 +2251,13 @@ void StreamFamily::XInfo(CmdArgList args, ConnectionContext* cntx) {

if (args.size() >= 3) {
full = 1;
ToUpper(&args[2]);
string_view full_arg = ArgS(args, 2);
string full_arg = absl::AsciiStrToUpper(ArgS(args, 2));
if (full_arg != "FULL") {
return cntx->SendError(
"unknown subcommand or wrong number of arguments for 'STREAM'. Try XINFO HELP.");
}
if (args.size() > 3) {
ToUpper(&args[3]);
string_view count_arg = ArgS(args, 3);
string count_arg = absl::AsciiStrToUpper(ArgS(args, 3));
string_view count_value_arg = ArgS(args, 4);
if (count_arg != "COUNT") {
return cntx->SendError(
Expand Down Expand Up @@ -2447,8 +2442,7 @@ void StreamFamily::XLen(CmdArgList args, ConnectionContext* cntx) {

bool ParseXpendingOptions(CmdArgList& args, PendingOpts& opts, ConnectionContext* cntx) {
size_t id_indx = 0;
ToUpper(&args[id_indx]);
string_view arg = ArgS(args, id_indx);
string arg = absl::AsciiStrToUpper(ArgS(args, id_indx));

if (arg == "IDLE" && args.size() > 4) {
id_indx++;
Expand Down Expand Up @@ -2579,8 +2573,7 @@ std::optional<ReadOpts> ParseReadArgsOrReply(CmdArgList args, bool read_group,
size_t id_indx = 0;

if (opts.read_group) {
ToUpper(&args[id_indx]);
string_view arg = ArgS(args, id_indx);
string arg = absl::AsciiStrToUpper(ArgS(args, id_indx));

if (arg.size() - 1 < 2) {
cntx->SendError(kSyntaxErr);
Expand All @@ -2599,8 +2592,7 @@ std::optional<ReadOpts> ParseReadArgsOrReply(CmdArgList args, bool read_group,
}

for (; id_indx < args.size(); ++id_indx) {
ToUpper(&args[id_indx]);
string_view arg = ArgS(args, id_indx);
string arg = absl::AsciiStrToUpper(ArgS(args, id_indx));

bool remaining_args = args.size() - id_indx - 1 > 0;
if (arg == "BLOCK" && remaining_args) {
Expand Down Expand Up @@ -3078,8 +3070,8 @@ void StreamFamily::XRangeGeneric(CmdArgList args, bool is_rev, ConnectionContext
if (args.size() != 5) {
return cntx->SendError(WrongNumArgsError("XRANGE"), kSyntaxErrType);
}
ToUpper(&args[3]);
string_view opt = ArgS(args, 3);

string opt = absl::AsciiStrToUpper(ArgS(args, 3));
string_view val = ArgS(args, 4);

if (opt != "COUNT" || !absl::SimpleAtoi(val, &range_opts.count)) {
Expand Down Expand Up @@ -3159,8 +3151,8 @@ void StreamFamily::XAutoClaim(CmdArgList args, ConnectionContext* cntx) {
opts.start = rs.parsed_id.val;

for (size_t i = 5; i < args.size(); ++i) {
ToUpper(&args[i]);
string_view arg = ArgS(args, i);
string arg = absl::AsciiStrToUpper(ArgS(args, i));

bool remaining_args = args.size() - i - 1 > 0;

if (remaining_args) {
Expand Down
25 changes: 9 additions & 16 deletions src/server/zset_family.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1106,8 +1106,8 @@ OpResult<unsigned> ParseAggregate(CmdArgList args, bool store, SetOpArgs* op_arg
return OpStatus::SYNTAX_ERR;
}

ToUpper(&args[1]);
auto filled = FillAggType(ArgS(args, 1), op_args);
string agg_type = absl::AsciiStrToUpper(ArgS(args, 1));
auto filled = FillAggType(agg_type, op_args);
if (!filled) {
return filled.status();
}
Expand Down Expand Up @@ -1156,8 +1156,7 @@ OpResult<SetOpArgs> ParseSetOpArgs(CmdArgList args, bool store) {
DCHECK_LE(opt_args_start, args.size()); // Checked inside DetermineKeys

for (size_t i = opt_args_start; i < args.size(); ++i) {
ToUpper(&args[i]);
string_view arg = ArgS(args, i);
string arg = absl::AsciiStrToUpper(ArgS(args, i));
if (arg == "WEIGHTS") {
auto parsed_cnt = ParseWeights(args.subspan(i), &op_args);
if (!parsed_cnt) {
Expand Down Expand Up @@ -1815,9 +1814,7 @@ void ZSetFamily::ZAdd(CmdArgList args, ConnectionContext* cntx) {
ZParams zparams;
size_t i = 1;
for (; i < args.size() - 1; ++i) {
ToUpper(&args[i]);

string_view cur_arg = ArgS(args, i);
string cur_arg = absl::AsciiStrToUpper(ArgS(args, i));

if (cur_arg == "XX") {
zparams.flags |= ZADD_IN_XX; // update only
Expand Down Expand Up @@ -2560,9 +2557,7 @@ void ZSetFamily::GeoAdd(CmdArgList args, ConnectionContext* cntx) {
ZParams zparams;
size_t i = 1;
for (; i < args.size(); ++i) {
ToUpper(&args[i]);

string_view cur_arg = ArgS(args, i);
string cur_arg = absl::AsciiStrToUpper(ArgS(args, i));

if (cur_arg == "XX") {
zparams.flags |= ZADD_IN_XX; // update only
Expand Down Expand Up @@ -2661,8 +2656,8 @@ void ZSetFamily::GeoPos(CmdArgList args, ConnectionContext* cntx) {
void ZSetFamily::GeoDist(CmdArgList args, ConnectionContext* cntx) {
double distance_multiplier = 1;
if (args.size() == 4) {
ToUpper(&args[3]);
string_view unit = ArgS(args, 3);
string unit = absl::AsciiStrToUpper(ArgS(args, 3));

distance_multiplier = ExtractUnit(unit);
args.remove_suffix(1);
if (distance_multiplier < 0) {
Expand Down Expand Up @@ -2933,9 +2928,8 @@ void ZSetFamily::GeoSearch(CmdArgList args, ConnectionContext* cntx) {
bool by_set = false;

for (size_t i = 1; i < args.size(); ++i) {
ToUpper(&args[i]);
string cur_arg = absl::AsciiStrToUpper(ArgS(args, i));

string_view cur_arg = ArgS(args, i);
if (cur_arg == "FROMMEMBER") {
if (from_set) {
return cntx->SendError(kFromMemberLonglatErr);
Expand Down Expand Up @@ -3071,9 +3065,8 @@ void ZSetFamily::GeoRadiusByMember(CmdArgList args, ConnectionContext* cntx) {
shape.type = CIRCULAR_TYPE;

for (size_t i = 4; i < args.size(); ++i) {
ToUpper(&args[i]);
string cur_arg = absl::AsciiStrToUpper(ArgS(args, i));

string_view cur_arg = ArgS(args, i);
if (cur_arg == "ASC") {
if (geo_ops.sorting != Sorting::kUnsorted) {
return cntx->SendError(kAscDescErr);
Expand Down

0 comments on commit 45aba13

Please sign in to comment.