From a3a08bdc8e8cd0f77eb055ae20dcc1c850ac286d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 8 Jun 2023 11:21:21 +0200 Subject: [PATCH] Avoid passing string parameters by value in KeyPathMapping interface --- src/realm/collection_parent.hpp | 6 +++++- src/realm/obj.cpp | 2 +- src/realm/parser/keypath_mapping.cpp | 22 +++++++++++----------- src/realm/parser/keypath_mapping.hpp | 18 ++++++++++-------- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/realm/collection_parent.hpp b/src/realm/collection_parent.hpp index 8790a2aa750..4ccf855b5cd 100644 --- a/src/realm/collection_parent.hpp +++ b/src/realm/collection_parent.hpp @@ -104,7 +104,6 @@ struct PathElement { , m_type(Type::index) { } - PathElement(StringData str) : string_val(str) , m_type(Type::key) @@ -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) { diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 1bcded7c893..689daf3f345 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -2201,7 +2201,7 @@ CollectionPtr Obj::get_collection_by_stable_path(const StablePath& path) const else { std::string key = mpark::get(index); auto ref = dynamic_cast(collection.get())->get(key); - return {ref, StringData(key)}; + return {ref, key}; } }; auto [ref, path_elem] = get_ref(); diff --git a/src/realm/parser/keypath_mapping.cpp b/src/realm/parser/keypath_mapping.cpp index f6078923af2..870af4db2ec 100644 --- a/src/realm/parser/keypath_mapping.cpp +++ b/src/realm/parser/keypath_mapping.cpp @@ -35,14 +35,14 @@ std::size_t TableAndColHash::operator()(const std::pair& } -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; @@ -65,7 +65,7 @@ util::Optional 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) { @@ -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; } @@ -85,7 +85,7 @@ bool KeyPathMapping::has_table_mapping(const std::string& alias) const return m_table_mappings.count(alias) > 0; } -util::Optional KeyPathMapping::get_table_mapping(const std::string alias) const +util::Optional 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; @@ -95,10 +95,10 @@ util::Optional 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( @@ -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( @@ -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 diff --git a/src/realm/parser/keypath_mapping.hpp b/src/realm/parser/keypath_mapping.hpp index 00492f6c9e6..9464539884e 100644 --- a/src/realm/parser/keypath_mapping.hpp +++ b/src/realm/parser/keypath_mapping.hpp @@ -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 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 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 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::string, TableAndColHash> m_mapping;