Skip to content

Commit

Permalink
Avoid passing string parameters by value in KeyPathMapping interface
Browse files Browse the repository at this point in the history
  • Loading branch information
jedelbo committed Jun 8, 2023
1 parent e13650b commit a3a08bd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 21 deletions.
6 changes: 5 additions & 1 deletion src/realm/collection_parent.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ struct PathElement {
, m_type(Type::index)
{
}

PathElement(StringData str)
: string_val(str)
, m_type(Type::key)
Expand All @@ -115,6 +114,11 @@ struct PathElement {
, m_type(Type::key)
{
}
PathElement(const std::string& str)
: string_val(str)
, m_type(Type::key)
{
}
PathElement(const PathElement& other)
: m_type(other.m_type)
{
Expand Down
2 changes: 1 addition & 1 deletion src/realm/obj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2201,7 +2201,7 @@ CollectionPtr Obj::get_collection_by_stable_path(const StablePath& path) const
else {
std::string key = mpark::get<std::string>(index);
auto ref = dynamic_cast<Dictionary*>(collection.get())->get(key);
return {ref, StringData(key)};
return {ref, key};
}
};
auto [ref, path_elem] = get_ref();
Expand Down
22 changes: 11 additions & 11 deletions src/realm/parser/keypath_mapping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ std::size_t TableAndColHash::operator()(const std::pair<TableKey, std::string>&
}


bool KeyPathMapping::add_mapping(ConstTableRef table, std::string name, std::string alias)
bool KeyPathMapping::add_mapping(ConstTableRef table, const std::string& name, std::string alias)
{
auto table_key = table->get_key();
auto it_and_success = m_mapping.insert({{table_key, name}, alias});
return it_and_success.second;
}

bool KeyPathMapping::remove_mapping(ConstTableRef table, std::string name)
bool KeyPathMapping::remove_mapping(ConstTableRef table, const std::string& name)
{
auto table_key = table->get_key();
return m_mapping.erase({table_key, name}) > 0;
Expand All @@ -65,7 +65,7 @@ util::Optional<std::string> KeyPathMapping::get_mapping(TableKey table_key, cons
return ret;
}

bool KeyPathMapping::add_table_mapping(ConstTableRef table, std::string alias)
bool KeyPathMapping::add_table_mapping(ConstTableRef table, const std::string& alias)
{
std::string real_table_name = table->get_name();
if (alias == real_table_name) {
Expand All @@ -75,7 +75,7 @@ bool KeyPathMapping::add_table_mapping(ConstTableRef table, std::string alias)
return it_and_success.second;
}

bool KeyPathMapping::remove_table_mapping(std::string alias_to_remove)
bool KeyPathMapping::remove_table_mapping(const std::string& alias_to_remove)
{
return m_table_mappings.erase(alias_to_remove) > 0;
}
Expand All @@ -85,7 +85,7 @@ bool KeyPathMapping::has_table_mapping(const std::string& alias) const
return m_table_mappings.count(alias) > 0;
}

util::Optional<std::string> KeyPathMapping::get_table_mapping(const std::string alias) const
util::Optional<std::string> KeyPathMapping::get_table_mapping(const std::string& alias) const
{
if (auto it = m_table_mappings.find(alias); it != m_table_mappings.end()) {
return it->second;
Expand All @@ -95,10 +95,10 @@ util::Optional<std::string> KeyPathMapping::get_table_mapping(const std::string

constexpr static size_t max_substitutions_allowed = 50;

std::string KeyPathMapping::translate_table_name(const std::string& identifier)
std::string KeyPathMapping::translate_table_name(std::string_view identifier)
{
size_t substitutions = 0;
std::string alias = identifier;
std::string alias{identifier};
while (auto mapped = get_table_mapping(alias)) {
if (substitutions > max_substitutions_allowed) {
throw MappingError(
Expand All @@ -115,11 +115,11 @@ std::string KeyPathMapping::translate_table_name(const std::string& identifier)
return alias;
}

std::string KeyPathMapping::translate(ConstTableRef table, const std::string& identifier)
std::string KeyPathMapping::translate(ConstTableRef table, std::string_view identifier)
{
size_t substitutions = 0;
auto tk = table->get_key();
std::string alias = identifier;
std::string alias{identifier};
while (auto mapped = get_mapping(tk, alias)) {
if (substitutions > max_substitutions_allowed) {
throw MappingError(
Expand All @@ -132,10 +132,10 @@ std::string KeyPathMapping::translate(ConstTableRef table, const std::string& id
return alias;
}

std::string KeyPathMapping::translate(const LinkChain& link_chain, const std::string& identifier)
std::string KeyPathMapping::translate(const LinkChain& link_chain, std::string_view identifier)
{
auto table = link_chain.get_current_table();
return translate(table, identifier);
return translate(table, std::string{identifier});
}

} // namespace query_parser
Expand Down
18 changes: 10 additions & 8 deletions src/realm/parser/keypath_mapping.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,20 @@ class KeyPathMapping {
public:
KeyPathMapping() = default;
// returns true if added, false if duplicate key already exists
bool add_mapping(ConstTableRef table, std::string name, std::string alias);
bool remove_mapping(ConstTableRef table, std::string name);
bool add_mapping(ConstTableRef table, const std::string&, std::string alias);
bool remove_mapping(ConstTableRef table, const std::string&);
bool has_mapping(ConstTableRef table, const std::string& name) const;
util::Optional<std::string> get_mapping(TableKey table_key, const std::string& name) const;
// table names are only used in backlink queries with the syntax '@links.TableName.property'
bool add_table_mapping(ConstTableRef table, std::string alias);
bool remove_table_mapping(std::string alias_to_remove);

bool add_table_mapping(ConstTableRef table, const std::string& alias);
bool remove_table_mapping(const std::string& alias_to_remove);
bool has_table_mapping(const std::string& alias) const;
util::Optional<std::string> get_table_mapping(const std::string name) const;
std::string translate(const LinkChain&, const std::string& identifier);
std::string translate(ConstTableRef table, const std::string& identifier);
std::string translate_table_name(const std::string& identifier);
util::Optional<std::string> get_table_mapping(const std::string& name) const;

std::string translate(const LinkChain&, std::string_view identifier);
std::string translate(ConstTableRef table, std::string_view identifier);
std::string translate_table_name(std::string_view identifier);

protected:
std::unordered_map<std::pair<TableKey, std::string>, std::string, TableAndColHash> m_mapping;
Expand Down

0 comments on commit a3a08bd

Please sign in to comment.