diff options
author | Sybren A. Stüvel <sybren@blender.org> | 2021-09-30 17:29:14 +0300 |
---|---|---|
committer | Sybren A. Stüvel <sybren@blender.org> | 2021-09-30 17:29:14 +0300 |
commit | 628fab696cfaefdd2ac758849c8a1e9a3a0beef0 (patch) | |
tree | d24db3bab2e501ef702dc87cddb732ffd61fb1a4 /source/blender | |
parent | 5d42ea036999a7e82dbc03947968f4ad61093d06 (diff) |
Asset Catalog: introduce `AssetCatalogPath` class
So far we have used `std::string` for asset catalog paths. Some
operations are better described on a dedicated class for this, though.
This commits switches catalog paths from using `std::string` to a
dedicated `blender::bke::AssetCatalogPath` class.
The `using CatalogPath = AssetCatalogPath` alias is still there, and
will be removed in a following cleanup commit.
New `AssetCatalogPath` code reviewed by @severin in D12710.
Diffstat (limited to 'source/blender')
-rw-r--r-- | source/blender/blenkernel/BKE_asset_catalog.hh | 15 | ||||
-rw-r--r-- | source/blender/blenkernel/BKE_asset_catalog_path.hh | 138 | ||||
-rw-r--r-- | source/blender/blenkernel/CMakeLists.txt | 3 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog.cc | 124 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog_path.cc | 220 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog_path_test.cc | 234 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog_test.cc | 39 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_library_test.cc | 2 | ||||
-rw-r--r-- | source/blender/editors/asset/intern/asset_catalog.cc | 12 |
9 files changed, 646 insertions, 141 deletions
diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh index 05db3c808cf..c141ce5fca0 100644 --- a/source/blender/blenkernel/BKE_asset_catalog.hh +++ b/source/blender/blenkernel/BKE_asset_catalog.hh @@ -30,6 +30,8 @@ #include "BLI_uuid.h" #include "BLI_vector.hh" +#include "BKE_asset_catalog_path.hh" + #include <map> #include <memory> #include <set> @@ -38,7 +40,7 @@ namespace blender::bke { using CatalogID = bUUID; -using CatalogPath = std::string; +using CatalogPath = AssetCatalogPath; using CatalogPathComponent = std::string; /* Would be nice to be able to use `std::filesystem::path` for this, but it's currently not * available on the minimum macOS target version. */ @@ -52,7 +54,6 @@ class AssetCatalogTree; * directory hierarchy). */ class AssetCatalogService { public: - static const char PATH_SEPARATOR; static const CatalogFilePath DEFAULT_CATALOG_FILENAME; public: @@ -298,22 +299,12 @@ class AssetCatalog { } flags; /** - * \return true only if this catalog's path is contained within the given path. - * When this catalog's path is equal to the given path, return true as well. - * - * Note that non-normalized paths (so for example starting or ending with a slash) are not - * supported, and result in undefined behavior. - */ - bool is_contained_in(const CatalogPath &other_path) const; - - /** * Create a new Catalog with the given path, auto-generating a sensible catalog simple-name. * * NOTE: the given path will be cleaned up (trailing spaces removed, etc.), so the returned * `AssetCatalog`'s path differ from the given one. */ static std::unique_ptr<AssetCatalog> from_path(const CatalogPath &path); - static CatalogPath cleanup_path(const CatalogPath &path); protected: /** Generate a sensible catalog ID for the given path. */ diff --git a/source/blender/blenkernel/BKE_asset_catalog_path.hh b/source/blender/blenkernel/BKE_asset_catalog_path.hh new file mode 100644 index 00000000000..1e53df553a9 --- /dev/null +++ b/source/blender/blenkernel/BKE_asset_catalog_path.hh @@ -0,0 +1,138 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +/** \file + * \ingroup bke + */ + +#pragma once + +#ifndef __cplusplus +# error This is a C++ header. +#endif + +#include "BLI_function_ref.hh" +#include "BLI_string_ref.hh" +#include "BLI_sys_types.h" + +#include <string> + +namespace blender::bke { + +/** + * Location of an Asset Catalog in the catalog tree, denoted by slash-separated path components. + * + * Each path component is a string that is not allowed to have slashes or colons. The latter is to + * make things easy to save in the colon-delimited Catalog Definition File format. + * + * The path of a catalog determines where in the catalog hierarchy the catalog is shown. Examples + * are "Characters/Ellie/Poses/Hand" or "Kitbash/City/Skyscrapers". The path looks like a + * filesystem path, with a few differences: + * + * - Only slashes are used as path component separators. + * - All paths are absolute, so there is no need for a leading slash. + * + * See https://wiki.blender.org/wiki/Source/Architecture/Asset_System/Catalogs + * + * Paths are stored as byte sequences, and assumed to be UTF-8. + */ +class AssetCatalogPath { + friend std::ostream &operator<<(std::ostream &stream, const AssetCatalogPath &path_to_append); + + private: + /** + * The path itself, such as "Agents/Secret/327". + */ + std::string path_; + + public: + static const char SEPARATOR; + + AssetCatalogPath() = delete; + AssetCatalogPath(StringRef path); + AssetCatalogPath(const std::string &path); + AssetCatalogPath(const char *path); + AssetCatalogPath(const AssetCatalogPath &other_path); + AssetCatalogPath(AssetCatalogPath &&other_path) noexcept; + ~AssetCatalogPath() = default; + + uint64_t hash() const; + uint64_t length() const; /* Length of the path in bytes. */ + + /** C-string representation of the path. */ + const char *c_str() const; + const std::string &str() const; + + /* In-class operators, because of the implicit `AssetCatalogPath(StringRef)` constructor. + * Otherwise `string == string` could cast both sides to `AssetCatalogPath`. */ + bool operator==(const AssetCatalogPath &other_path) const; + bool operator!=(const AssetCatalogPath &other_path) const; + bool operator<(const AssetCatalogPath &other_path) const; + AssetCatalogPath &operator=(const AssetCatalogPath &other_path) = default; + AssetCatalogPath &operator=(AssetCatalogPath &&other_path) = default; + + /** Concatenate two paths, returning the new path. */ + AssetCatalogPath operator/(const AssetCatalogPath &path_to_append) const; + + /* False when the path is empty, true otherwise. */ + operator bool() const; + + /** + * Clean up the path. This ensures: + * - Every path component is stripped of its leading/trailing spaces. + * - Empty components (caused by double slashes or leading/trailing slashes) are removed. + * - Invalid characters are replaced with valid ones. + */ + [[nodiscard]] AssetCatalogPath cleanup() const; + + /** + * \return true only if the given path is a parent of this catalog's path. + * When this catalog's path is equal to the given path, return true as well. + * In other words, this defines a weak subset. + * + * True: "some/path/there" is contained in "some/path" and "some". + * False: "path/there" is not contained in "some/path/there". + * + * Note that non-cleaned-up paths (so for example starting or ending with a + * slash) are not supported, and result in undefined behavior. + */ + bool is_contained_in(const AssetCatalogPath &other_path) const; + + /** + * Change the initial part of the path from `from_path` to `to_path`. + * If this path does not start with `from_path`, return an empty path as result. + * + * Example: + * + * AssetCatalogPath path("some/path/to/some/catalog"); + * path.rebase("some/path", "new/base") -> "new/base/to/some/catalog" + */ + AssetCatalogPath rebase(const AssetCatalogPath &from_path, + const AssetCatalogPath &to_path) const; + + /** Call the callback function for each path component, in left-to-right order. */ + using ComponentIteratorFn = FunctionRef<void(StringRef component_name, bool is_last_component)>; + void iterate_components(ComponentIteratorFn callback) const; + + protected: + /** Strip leading/trailing spaces and replace disallowed characters. */ + static std::string cleanup_component(StringRef component_name); +}; + +/** Output the path as string. */ +std::ostream &operator<<(std::ostream &stream, const AssetCatalogPath &path_to_append); + +} // namespace blender::bke diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt index 24de91959bb..fb7fdd1ac21 100644 --- a/source/blender/blenkernel/CMakeLists.txt +++ b/source/blender/blenkernel/CMakeLists.txt @@ -85,6 +85,7 @@ set(SRC intern/armature_update.c intern/asset.cc intern/asset_catalog.cc + intern/asset_catalog_path.cc intern/asset_library.cc intern/attribute.c intern/attribute_access.cc @@ -306,6 +307,7 @@ set(SRC BKE_armature.hh BKE_asset.h BKE_asset_catalog.hh + BKE_asset_catalog_path.hh BKE_asset_library.h BKE_asset_library.hh BKE_attribute.h @@ -789,6 +791,7 @@ if(WITH_GTESTS) intern/action_test.cc intern/armature_test.cc intern/asset_catalog_test.cc + intern/asset_catalog_path_test.cc intern/asset_library_test.cc intern/asset_test.cc intern/cryptomatte_test.cc diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc index b00f4305aa6..300a15fad6d 100644 --- a/source/blender/blenkernel/intern/asset_catalog.cc +++ b/source/blender/blenkernel/intern/asset_catalog.cc @@ -38,7 +38,6 @@ namespace blender::bke { -const char AssetCatalogService::PATH_SEPARATOR = '/'; const CatalogFilePath AssetCatalogService::DEFAULT_CATALOG_FILENAME = "blender_assets.cats.txt"; /* For now this is the only version of the catalog definition files that is supported. @@ -115,12 +114,12 @@ void AssetCatalogService::update_catalog_path(CatalogID catalog_id, for (auto &catalog_uptr : catalogs_.values()) { AssetCatalog *cat = catalog_uptr.get(); - if (!cat->is_contained_in(old_cat_path)) { + + const CatalogPath new_path = cat->path.rebase(old_cat_path, new_catalog_path); + if (!new_path) { continue; } - - const CatalogPath path_suffix = cat->path.substr(old_cat_path.length()); - cat->path = new_catalog_path + path_suffix; + cat->path = new_path; } this->rebuild_tree(); @@ -319,8 +318,7 @@ CatalogFilePath AssetCatalogService::find_suitable_cdf_path_for_writing( /* - There's no definition file next to the .blend file. * -> Ask the asset library API for an appropriate location. */ char suitable_root_path[PATH_MAX]; - BKE_asset_library_find_suitable_root_path_from_path(blend_file_path.c_str(), - suitable_root_path); + BKE_asset_library_find_suitable_root_path_from_path(blend_file_path.c_str(), suitable_root_path); char asset_lib_cdf_path[PATH_MAX]; BLI_path_join(asset_lib_cdf_path, sizeof(asset_lib_cdf_path), @@ -382,9 +380,9 @@ StringRef AssetCatalogTreeItem::get_name() const CatalogPath AssetCatalogTreeItem::catalog_path() const { - std::string current_path = name_; + CatalogPath current_path = name_; for (const AssetCatalogTreeItem *parent = parent_; parent; parent = parent->parent_) { - current_path = parent->name_ + AssetCatalogService::PATH_SEPARATOR + current_path; + current_path = CatalogPath(parent->name_) / current_path; } return current_path; } @@ -405,32 +403,6 @@ bool AssetCatalogTreeItem::has_children() const /* ---------------------------------------------------------------------- */ -/** - * Iterate over path components, calling \a callback for each component. E.g. "just/some/path" - * iterates over "just", then "some" then "path". - */ -static void iterate_over_catalog_path_components( - const CatalogPath &path, - FunctionRef<void(StringRef component_name, bool is_last_component)> callback) -{ - const char *next_slash_ptr; - - for (const char *path_component = path.data(); path_component && path_component[0]; - /* Jump to one after the next slash if there is any. */ - path_component = next_slash_ptr ? next_slash_ptr + 1 : nullptr) { - next_slash_ptr = BLI_path_slash_find(path_component); - - const bool is_last_component = next_slash_ptr == nullptr; - /* Note that this won't be null terminated. */ - const StringRef component_name = is_last_component ? - path_component : - StringRef(path_component, - next_slash_ptr - path_component); - - callback(component_name, is_last_component); - } -} - void AssetCatalogTree::insert_item(const AssetCatalog &catalog) { const AssetCatalogTreeItem *parent = nullptr; @@ -438,30 +410,29 @@ void AssetCatalogTree::insert_item(const AssetCatalog &catalog) * added to (if not there yet). */ AssetCatalogTreeItem::ChildMap *current_item_children = &root_items_; - BLI_assert_msg(!ELEM(catalog.path[0], '/', '\\'), + BLI_assert_msg(!ELEM(catalog.path.str()[0], '/', '\\'), "Malformed catalog path; should not start with a separator"); const CatalogID nil_id{}; - iterate_over_catalog_path_components( - catalog.path, [&](StringRef component_name, const bool is_last_component) { - /* Insert new tree element - if no matching one is there yet! */ - auto [key_and_item, was_inserted] = current_item_children->emplace( - component_name, - AssetCatalogTreeItem( - component_name, is_last_component ? catalog.catalog_id : nil_id, parent)); - AssetCatalogTreeItem &item = key_and_item->second; - - /* If full path of this catalog already exists as parent path of a previously read catalog, - * we can ensure this tree item's UUID is set here. */ - if (is_last_component && BLI_uuid_is_nil(item.catalog_id_)) { - item.catalog_id_ = catalog.catalog_id; - } + catalog.path.iterate_components([&](StringRef component_name, const bool is_last_component) { + /* Insert new tree element - if no matching one is there yet! */ + auto [key_and_item, was_inserted] = current_item_children->emplace( + component_name, + AssetCatalogTreeItem( + component_name, is_last_component ? catalog.catalog_id : nil_id, parent)); + AssetCatalogTreeItem &item = key_and_item->second; + + /* If full path of this catalog already exists as parent path of a previously read catalog, + * we can ensure this tree item's UUID is set here. */ + if (is_last_component && BLI_uuid_is_nil(item.catalog_id_)) { + item.catalog_id_ = catalog.catalog_id; + } - /* Walk further into the path (no matter if a new item was created or not). */ - parent = &item; - current_item_children = &item.children_; - }); + /* Walk further into the path (no matter if a new item was created or not). */ + parent = &item; + current_item_children = &item.children_; + }); } void AssetCatalogTree::foreach_item(AssetCatalogTreeItem::ItemIterFn callback) @@ -592,7 +563,7 @@ std::unique_ptr<AssetCatalog> AssetCatalogDefinitionFile::parse_catalog_line(con const StringRef path_and_simple_name = line.substr(first_delim + 1); const int64_t second_delim = path_and_simple_name.find_first_of(delim); - CatalogPath catalog_path; + std::string path_in_file; std::string simple_name; if (second_delim == 0) { /* Delimiter as first character means there is no path. These lines are to be ignored. */ @@ -601,16 +572,16 @@ std::unique_ptr<AssetCatalog> AssetCatalogDefinitionFile::parse_catalog_line(con if (second_delim == StringRef::not_found) { /* No delimiter means no simple name, just treat it as all "path". */ - catalog_path = path_and_simple_name; + path_in_file = path_and_simple_name; simple_name = ""; } else { - catalog_path = path_and_simple_name.substr(0, second_delim); + path_in_file = path_and_simple_name.substr(0, second_delim); simple_name = path_and_simple_name.substr(second_delim + 1).trim(); } - catalog_path = AssetCatalog::cleanup_path(catalog_path); - return std::make_unique<AssetCatalog>(catalog_id, catalog_path, simple_name); + CatalogPath catalog_path = path_in_file; + return std::make_unique<AssetCatalog>(catalog_id, catalog_path.cleanup(), simple_name); } bool AssetCatalogDefinitionFile::write_to_disk() const @@ -724,7 +695,7 @@ AssetCatalog::AssetCatalog(const CatalogID catalog_id, std::unique_ptr<AssetCatalog> AssetCatalog::from_path(const CatalogPath &path) { - const CatalogPath clean_path = cleanup_path(path); + const CatalogPath clean_path = path.cleanup(); const CatalogID cat_id = BLI_uuid_generate_random(); const std::string simple_name = sensible_simple_name_for_path(clean_path); auto catalog = std::make_unique<AssetCatalog>(cat_id, clean_path, simple_name); @@ -733,8 +704,8 @@ std::unique_ptr<AssetCatalog> AssetCatalog::from_path(const CatalogPath &path) std::string AssetCatalog::sensible_simple_name_for_path(const CatalogPath &path) { - std::string name = path; - std::replace(name.begin(), name.end(), AssetCatalogService::PATH_SEPARATOR, '-'); + std::string name = path.str(); + std::replace(name.begin(), name.end(), CatalogPath::SEPARATOR, '-'); if (name.length() < MAX_NAME - 1) { return name; } @@ -744,33 +715,4 @@ std::string AssetCatalog::sensible_simple_name_for_path(const CatalogPath &path) return "..." + name.substr(name.length() - 60); } -CatalogPath AssetCatalog::cleanup_path(const CatalogPath &path) -{ - /* TODO(@sybren): maybe go over each element of the path, and trim those? */ - CatalogPath clean_path = StringRef(path).trim().trim(AssetCatalogService::PATH_SEPARATOR).trim(); - return clean_path; -} - -bool AssetCatalog::is_contained_in(const CatalogPath &other_path) const -{ - if (other_path.empty()) { - return true; - } - - if (this->path == other_path) { - return true; - } - - /* To be a child path of 'other_path', our path must be at least a separator and another - * character longer. */ - if (this->path.length() < other_path.length() + 2) { - return false; - } - - const StringRef this_path(this->path); - const bool prefix_ok = this_path.startswith(other_path); - const char next_char = this_path[other_path.length()]; - return prefix_ok && next_char == AssetCatalogService::PATH_SEPARATOR; -} - } // namespace blender::bke diff --git a/source/blender/blenkernel/intern/asset_catalog_path.cc b/source/blender/blenkernel/intern/asset_catalog_path.cc new file mode 100644 index 00000000000..d8af7be4a02 --- /dev/null +++ b/source/blender/blenkernel/intern/asset_catalog_path.cc @@ -0,0 +1,220 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +/** \file + * \ingroup bke + */ + +#include "BKE_asset_catalog_path.hh" + +#include "BLI_path_util.h" + +namespace blender::bke { + +const char AssetCatalogPath::SEPARATOR = '/'; + +AssetCatalogPath::AssetCatalogPath(const std::string &path) : path_(path) +{ +} + +AssetCatalogPath::AssetCatalogPath(StringRef path) : path_(path) +{ +} + +AssetCatalogPath::AssetCatalogPath(const char *path) : path_(path) +{ +} + +AssetCatalogPath::AssetCatalogPath(const AssetCatalogPath &other_path) : path_(other_path.path_) +{ +} + +AssetCatalogPath::AssetCatalogPath(AssetCatalogPath &&other_path) noexcept + : path_(std::move(other_path.path_)) +{ +} + +uint64_t AssetCatalogPath::hash() const +{ + std::hash<std::string> hasher{}; + return hasher(this->path_); +} + +uint64_t AssetCatalogPath::length() const +{ + return this->path_.length(); +} + +const char *AssetCatalogPath::c_str() const +{ + return this->path_.c_str(); +} + +const std::string &AssetCatalogPath::str() const +{ + return this->path_; +} + +/* In-class operators, because of the implicit `AssetCatalogPath(StringRef)` constructor. + * Otherwise `string == string` could cast both sides to `AssetCatalogPath`. */ +bool AssetCatalogPath::operator==(const AssetCatalogPath &other_path) const +{ + return this->path_ == other_path.path_; +} + +bool AssetCatalogPath::operator!=(const AssetCatalogPath &other_path) const +{ + return !(*this == other_path); +} + +bool AssetCatalogPath::operator<(const AssetCatalogPath &other_path) const +{ + return this->path_ < other_path.path_; +} + +AssetCatalogPath AssetCatalogPath::operator/(const AssetCatalogPath &path_to_append) const +{ + /* `"" / "path"` or `"path" / ""` should just result in `"path"` */ + if (!*this) { + return path_to_append; + } + if (!path_to_append) { + return *this; + } + + std::stringstream new_path; + new_path << this->path_ << SEPARATOR << path_to_append.path_; + return AssetCatalogPath(new_path.str()); +} + +AssetCatalogPath::operator bool() const +{ + return !this->path_.empty(); +} + +std::ostream &operator<<(std::ostream &stream, const AssetCatalogPath &path_to_append) +{ + stream << path_to_append.path_; + return stream; +} + +AssetCatalogPath AssetCatalogPath::cleanup() const +{ + std::stringstream clean_components; + bool first_component_seen = false; + + this->iterate_components([&clean_components, &first_component_seen](StringRef component_name, + bool /*is_last_component*/) { + const std::string clean_component = cleanup_component(component_name); + + if (clean_component.empty()) { + /* These are caused by leading, trailing, or double slashes. */ + return; + } + + /* If a previous path component has been streamed already, we need a path separator. This + * cannot use the `is_last_component` boolean, because the last component might be skipped due + * to the condition above. */ + if (first_component_seen) { + clean_components << SEPARATOR; + } + first_component_seen = true; + + clean_components << clean_component; + }); + + return AssetCatalogPath(clean_components.str()); +} + +std::string AssetCatalogPath::cleanup_component(StringRef component) +{ + std::string cleaned = component.trim(); + /* Replace colons with something else, as those are used in the CDF file as delimiter. */ + std::replace(cleaned.begin(), cleaned.end(), ':', '-'); + return cleaned; +} + +bool AssetCatalogPath::is_contained_in(const AssetCatalogPath &other_path) const +{ + if (!other_path) { + /* The empty path contains all other paths. */ + return true; + } + + if (this->path_ == other_path.path_) { + /* Weak is-in relation: equal paths contain each other. */ + return true; + } + + /* To be a child path of 'other_path', our path must be at least a separator and another + * character longer. */ + if (this->length() < other_path.length() + 2) { + return false; + } + + /* Create StringRef to be able to use .startswith(). */ + const StringRef this_path(this->path_); + const bool prefix_ok = this_path.startswith(other_path.path_); + const char next_char = this_path[other_path.length()]; + return prefix_ok && next_char == SEPARATOR; +} + +void AssetCatalogPath::iterate_components(ComponentIteratorFn callback) const +{ + const char *next_slash_ptr; + + for (const char *path_component = this->path_.data(); path_component && path_component[0]; + /* Jump to one after the next slash if there is any. */ + path_component = next_slash_ptr ? next_slash_ptr + 1 : nullptr) { + next_slash_ptr = BLI_path_slash_find(path_component); + + const bool is_last_component = next_slash_ptr == nullptr; + /* Note that this won't be null terminated. */ + const StringRef component_name = is_last_component ? + path_component : + StringRef(path_component, + next_slash_ptr - path_component); + + callback(component_name, is_last_component); + } +} + +AssetCatalogPath AssetCatalogPath::rebase(const AssetCatalogPath &from_path, + const AssetCatalogPath &to_path) const +{ + if (!from_path) { + if (!to_path) { + return AssetCatalogPath(""); + } + return to_path / *this; + } + + if (!this->is_contained_in(from_path)) { + return AssetCatalogPath(""); + } + + if (*this == from_path) { + /* Early return, because otherwise the length+1 below is going to cause problems. */ + return to_path; + } + + /* When from_path = "abcd", we need to skip "abcd/" to get the rest of the path, hence the +1. */ + const StringRef suffix = StringRef(this->path_).substr(from_path.length() + 1); + const AssetCatalogPath path_suffix(suffix); + return to_path / path_suffix; +} + +} // namespace blender::bke diff --git a/source/blender/blenkernel/intern/asset_catalog_path_test.cc b/source/blender/blenkernel/intern/asset_catalog_path_test.cc new file mode 100644 index 00000000000..55919abbb8f --- /dev/null +++ b/source/blender/blenkernel/intern/asset_catalog_path_test.cc @@ -0,0 +1,234 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * The Original Code is Copyright (C) 2020 Blender Foundation + * All rights reserved. + */ + +#include "BKE_asset_catalog_path.hh" + +#include "BLI_set.hh" +#include "BLI_vector.hh" + +#include <set> +#include <sstream> + +#include "testing/testing.h" + +namespace blender::bke::tests { + +TEST(AssetCatalogPathTest, construction) +{ + AssetCatalogPath from_char_literal("the/path"); + + const std::string str_const = "the/path"; + AssetCatalogPath from_string_constant(str_const); + + std::string str_variable = "the/path"; + AssetCatalogPath from_string_variable(str_variable); + + std::string long_string = "this is a long/string/with/a/path in the middle"; + StringRef long_string_ref(long_string); + StringRef middle_bit = long_string_ref.substr(10, 23); + AssetCatalogPath from_string_ref(middle_bit); + EXPECT_EQ(from_string_ref, "long/string/with/a/path"); +} + +TEST(AssetCatalogPathTest, length) +{ + const AssetCatalogPath one("1"); + EXPECT_EQ(1, one.length()); + + const AssetCatalogPath empty(""); + EXPECT_EQ(0, empty.length()); + + const AssetCatalogPath utf8("some/родитель"); + EXPECT_EQ(21, utf8.length()) << "13 characters should be 21 bytes."; +} + +TEST(AssetCatalogPathTest, comparison_operators) +{ + const AssetCatalogPath empty(""); + const AssetCatalogPath the_path("the/path"); + const AssetCatalogPath the_path_child("the/path/child"); + const AssetCatalogPath unrelated_path("unrelated/path"); + const AssetCatalogPath other_instance_same_path("the/path"); + + EXPECT_LT(empty, the_path); + EXPECT_LT(the_path, the_path_child); + EXPECT_LT(the_path, unrelated_path); + + EXPECT_EQ(empty, empty) << "Identical empty instances should compare equal."; + EXPECT_EQ(empty, "") << "Comparison to empty string should be possible."; + EXPECT_EQ(the_path, the_path) << "Identical non-empty instances should compare equal."; + EXPECT_EQ(the_path, "the/path") << "Comparison to string should be possible."; + EXPECT_EQ(the_path, other_instance_same_path) + << "Different instances with equal path should compare equal."; + + EXPECT_NE(the_path, the_path_child); + EXPECT_NE(the_path, unrelated_path); + EXPECT_NE(the_path, empty); + + EXPECT_FALSE(empty); + EXPECT_TRUE(the_path); +} + +TEST(AssetCatalogPathTest, move_semantics) +{ + AssetCatalogPath source_path("source/path"); + EXPECT_TRUE(source_path); + + AssetCatalogPath dest_path = std::move(source_path); + EXPECT_FALSE(source_path); + EXPECT_TRUE(dest_path); +} + +TEST(AssetCatalogPathTest, concatenation) +{ + AssetCatalogPath some_parent("some/родитель"); + AssetCatalogPath child = some_parent / "ребенок"; + + EXPECT_EQ(some_parent, "some/родитель") + << "Appending a child path should not modify the parent."; + EXPECT_EQ(child, "some/родитель/ребенок"); + + AssetCatalogPath appended_compound_path = some_parent / "ребенок/внук"; + EXPECT_EQ(appended_compound_path, "some/родитель/ребенок/внук"); + + AssetCatalogPath empty(""); + AssetCatalogPath child_of_the_void = empty / "child"; + EXPECT_EQ(child_of_the_void, "child") + << "Appending to an empty path should not create an initial slash."; + + AssetCatalogPath parent_of_the_void = some_parent / empty; + EXPECT_EQ(parent_of_the_void, "some/родитель") + << "Prepending to an empty path should not create a trailing slash."; + + std::string subpath = "child"; + AssetCatalogPath concatenated_with_string = some_parent / subpath; + EXPECT_EQ(concatenated_with_string, "some/родитель/child"); +} + +TEST(AssetCatalogPathTest, hashable) +{ + AssetCatalogPath path("heyyyyy"); + + std::set<AssetCatalogPath> path_std_set; + path_std_set.insert(path); + + blender::Set<AssetCatalogPath> path_blender_set; + path_blender_set.add(path); +} + +TEST(AssetCatalogPathTest, stream_operator) +{ + AssetCatalogPath path("путь/в/Пермь"); + std::stringstream sstream; + sstream << path; + EXPECT_EQ("путь/в/Пермь", sstream.str()); +} + +TEST(AssetCatalogPathTest, is_contained_in) +{ + const AssetCatalogPath catpath("simple/path/child"); + EXPECT_FALSE(catpath.is_contained_in("unrelated")); + EXPECT_FALSE(catpath.is_contained_in("sim")); + EXPECT_FALSE(catpath.is_contained_in("simple/pathx")); + EXPECT_FALSE(catpath.is_contained_in("simple/path/c")); + EXPECT_FALSE(catpath.is_contained_in("simple/path/child/grandchild")); + EXPECT_FALSE(catpath.is_contained_in("simple/path/")) + << "Non-normalized paths are not expected to work."; + + EXPECT_TRUE(catpath.is_contained_in("")); + EXPECT_TRUE(catpath.is_contained_in("simple")); + EXPECT_TRUE(catpath.is_contained_in("simple/path")); + + /* Test with some UTF8 non-ASCII characters. */ + AssetCatalogPath some_parent("some/родитель"); + AssetCatalogPath child = some_parent / "ребенок"; + + EXPECT_TRUE(child.is_contained_in(some_parent)); + EXPECT_TRUE(child.is_contained_in("some")); + + AssetCatalogPath appended_compound_path = some_parent / "ребенок/внук"; + EXPECT_TRUE(appended_compound_path.is_contained_in(some_parent)); + EXPECT_TRUE(appended_compound_path.is_contained_in(child)); + + /* Test "going up" directory-style. */ + AssetCatalogPath child_with_dotdot = some_parent / "../../other/hierarchy/part"; + EXPECT_TRUE(child_with_dotdot.is_contained_in(some_parent)) + << "dotdot path components should have no meaning"; +} + +TEST(AssetCatalogPathTest, cleanup) +{ + AssetCatalogPath ugly_path("/ some / родитель / "); + AssetCatalogPath clean_path = ugly_path.cleanup(); + + EXPECT_EQ(AssetCatalogPath("/ some / родитель / "), ugly_path) + << "cleanup should not modify the path instance itself"; + + EXPECT_EQ(AssetCatalogPath("some/родитель"), clean_path); + + AssetCatalogPath double_slashed("some//родитель"); + EXPECT_EQ(AssetCatalogPath("some/родитель"), double_slashed.cleanup()); + + AssetCatalogPath with_colons("some/key:subkey=value/path"); + EXPECT_EQ(AssetCatalogPath("some/key-subkey=value/path"), with_colons.cleanup()); +} + +TEST(AssetCatalogPathTest, iterate_components) +{ + AssetCatalogPath path("путь/в/Пермь"); + Vector<std::pair<std::string, bool>> seen_components; + + path.iterate_components([&seen_components](StringRef component_name, bool is_last_component) { + std::pair<std::string, bool> parameter_pair = std::make_pair<std::string, bool>( + component_name, bool(is_last_component)); + seen_components.append(parameter_pair); + }); + + ASSERT_EQ(3, seen_components.size()); + + EXPECT_EQ("путь", seen_components[0].first); + EXPECT_EQ("в", seen_components[1].first); + EXPECT_EQ("Пермь", seen_components[2].first); + + EXPECT_FALSE(seen_components[0].second); + EXPECT_FALSE(seen_components[1].second); + EXPECT_TRUE(seen_components[2].second); +} + +TEST(AssetCatalogPathTest, rebase) +{ + AssetCatalogPath path("some/path/to/some/catalog"); + EXPECT_EQ(path.rebase("some/path", "new/base"), "new/base/to/some/catalog"); + EXPECT_EQ(path.rebase("", "new/base"), "new/base/some/path/to/some/catalog"); + + EXPECT_EQ(path.rebase("some/path/to/some/catalog", "some/path/to/some/catalog"), + "some/path/to/some/catalog") + << "Rebasing to itself should not change the path."; + + EXPECT_EQ(path.rebase("path/to", "new/base"), "") + << "Non-matching base path should return empty string to indicate 'NO'."; + + /* Empty strings should be handled without crashing or other nasty side-effects. */ + AssetCatalogPath empty(""); + EXPECT_EQ(empty.rebase("path/to", "new/base"), ""); + EXPECT_EQ(empty.rebase("", "new/base"), "new/base"); + EXPECT_EQ(empty.rebase("", ""), ""); +} + +} // namespace blender::bke::tests diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc index 5b94f021797..cde16b97b5d 100644 --- a/source/blender/blenkernel/intern/asset_catalog_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_test.cc @@ -106,7 +106,7 @@ class AssetCatalogTest : public testing::Test { EXPECT_EQ(expected_filename, actual_item.get_name()); /* Does the computed number of parents match? */ EXPECT_EQ(expected_path.parent_count, actual_item.count_parents()); - EXPECT_EQ(expected_path.name, actual_item.catalog_path()); + EXPECT_EQ(expected_path.name, actual_item.catalog_path().str()); } /** @@ -186,21 +186,21 @@ TEST_F(AssetCatalogTest, load_single_file) AssetCatalog *poses_ellie = service.find_catalog(UUID_POSES_ELLIE); ASSERT_NE(nullptr, poses_ellie); EXPECT_EQ(UUID_POSES_ELLIE, poses_ellie->catalog_id); - EXPECT_EQ("character/Ellie/poselib", poses_ellie->path); + EXPECT_EQ("character/Ellie/poselib", poses_ellie->path.str()); EXPECT_EQ("POSES_ELLIE", poses_ellie->simple_name); /* Test white-space stripping and support in the path. */ AssetCatalog *poses_whitespace = service.find_catalog(UUID_POSES_ELLIE_WHITESPACE); ASSERT_NE(nullptr, poses_whitespace); EXPECT_EQ(UUID_POSES_ELLIE_WHITESPACE, poses_whitespace->catalog_id); - EXPECT_EQ("character/Ellie/poselib/white space", poses_whitespace->path); + EXPECT_EQ("character/Ellie/poselib/white space", poses_whitespace->path.str()); EXPECT_EQ("POSES_ELLIE WHITESPACE", poses_whitespace->simple_name); /* Test getting a UTF-8 catalog ID. */ AssetCatalog *poses_ruzena = service.find_catalog(UUID_POSES_RUZENA); ASSERT_NE(nullptr, poses_ruzena); EXPECT_EQ(UUID_POSES_RUZENA, poses_ruzena->catalog_id); - EXPECT_EQ("character/Ružena/poselib", poses_ruzena->path); + EXPECT_EQ("character/Ružena/poselib", poses_ruzena->path.str()); EXPECT_EQ("POSES_RUŽENA", poses_ruzena->simple_name); } @@ -588,7 +588,7 @@ TEST_F(AssetCatalogTest, create_first_catalog_from_scratch) AssetCatalog *written_cat = loaded_service.find_catalog(cat->catalog_id); ASSERT_NE(nullptr, written_cat); EXPECT_EQ(written_cat->catalog_id, cat->catalog_id); - EXPECT_EQ(written_cat->path, cat->path); + EXPECT_EQ(written_cat->path, cat->path.str()); } TEST_F(AssetCatalogTest, create_catalog_after_loading_file) @@ -640,7 +640,7 @@ TEST_F(AssetCatalogTest, create_catalog_path_cleanup) AssetCatalog *cat = service.create_catalog(" /some/path / "); EXPECT_FALSE(BLI_uuid_is_nil(cat->catalog_id)); - EXPECT_EQ("some/path", cat->path); + EXPECT_EQ("some/path", cat->path.str()); EXPECT_EQ("some-path", cat->simple_name); } @@ -652,7 +652,7 @@ TEST_F(AssetCatalogTest, create_catalog_simple_name) EXPECT_FALSE(BLI_uuid_is_nil(cat->catalog_id)); EXPECT_EQ("production/Spite Fright/Characters/Victora/Pose Library/Approved/Body Parts/Hands", - cat->path); + cat->path.str()); EXPECT_EQ("...ht-Characters-Victora-Pose Library-Approved-Body Parts-Hands", cat->simple_name); } @@ -733,12 +733,12 @@ TEST_F(AssetCatalogTest, update_catalog_path) EXPECT_EQ(orig_cat->catalog_id, renamed_cat->catalog_id) << "Changing the path should not change the catalog ID."; - EXPECT_EQ("charlib/Ružena", renamed_cat->path) + EXPECT_EQ("charlib/Ružena", renamed_cat->path.str()) << "Changing the path should change the path. Surprise."; - EXPECT_EQ("charlib/Ružena/hand", service.find_catalog(UUID_POSES_RUZENA_HAND)->path) + EXPECT_EQ("charlib/Ružena/hand", service.find_catalog(UUID_POSES_RUZENA_HAND)->path.str()) << "Changing the path should update children."; - EXPECT_EQ("charlib/Ružena/face", service.find_catalog(UUID_POSES_RUZENA_FACE)->path) + EXPECT_EQ("charlib/Ružena/face", service.find_catalog(UUID_POSES_RUZENA_FACE)->path.str()) << "Changing the path should update children."; } @@ -775,7 +775,7 @@ TEST_F(AssetCatalogTest, merge_catalog_files) /* When there are overlaps, the in-memory (i.e. last-saved) paths should win. */ const AssetCatalog *ruzena_face = loaded_service.find_catalog(UUID_POSES_RUZENA_FACE); - EXPECT_EQ("character/Ružena/poselib/face", ruzena_face->path); + EXPECT_EQ("character/Ružena/poselib/face", ruzena_face->path.str()); } TEST_F(AssetCatalogTest, backups) @@ -846,21 +846,4 @@ TEST_F(AssetCatalogTest, order_by_path) } } -TEST_F(AssetCatalogTest, is_contained_in) -{ - const AssetCatalog cat(BLI_uuid_generate_random(), "simple/path/child", ""); - - EXPECT_FALSE(cat.is_contained_in("unrelated")); - EXPECT_FALSE(cat.is_contained_in("sim")); - EXPECT_FALSE(cat.is_contained_in("simple/pathx")); - EXPECT_FALSE(cat.is_contained_in("simple/path/c")); - EXPECT_FALSE(cat.is_contained_in("simple/path/child/grandchild")); - EXPECT_FALSE(cat.is_contained_in("simple/path/")) - << "Non-normalized paths are not expected to work."; - - EXPECT_TRUE(cat.is_contained_in("")); - EXPECT_TRUE(cat.is_contained_in("simple")); - EXPECT_TRUE(cat.is_contained_in("simple/path")); -} - } // namespace blender::bke::tests diff --git a/source/blender/blenkernel/intern/asset_library_test.cc b/source/blender/blenkernel/intern/asset_library_test.cc index 37686175aed..30ac4dc6ad8 100644 --- a/source/blender/blenkernel/intern/asset_library_test.cc +++ b/source/blender/blenkernel/intern/asset_library_test.cc @@ -49,7 +49,7 @@ TEST(AssetLibraryTest, load_and_free_c_functions) const bUUID uuid_poses_ellie("df60e1f6-2259-475b-93d9-69a1b4a8db78"); AssetCatalog *poses_ellie = service->find_catalog(uuid_poses_ellie); ASSERT_NE(nullptr, poses_ellie) << "unable to find POSES_ELLIE catalog"; - EXPECT_EQ("character/Ellie/poselib", poses_ellie->path); + EXPECT_EQ("character/Ellie/poselib", poses_ellie->path.str()); BKE_asset_library_free(library_c_ptr); } diff --git a/source/blender/editors/asset/intern/asset_catalog.cc b/source/blender/editors/asset/intern/asset_catalog.cc index 68f11d77f44..c3e5a888796 100644 --- a/source/blender/editors/asset/intern/asset_catalog.cc +++ b/source/blender/editors/asset/intern/asset_catalog.cc @@ -19,6 +19,7 @@ */ #include "BKE_asset_catalog.hh" +#include "BKE_asset_catalog_path.hh" #include "BKE_asset_library.hh" #include "BLI_string_utils.h" @@ -33,17 +34,10 @@ struct CatalogUniqueNameFnData { StringRef parent_path; }; -static std::string to_full_path(StringRef parent_path, StringRef name) -{ - return parent_path.is_empty() ? - std::string(name) : - std::string(parent_path) + AssetCatalogService::PATH_SEPARATOR + name; -} - static bool catalog_name_exists_fn(void *arg, const char *name) { CatalogUniqueNameFnData &fn_data = *static_cast<CatalogUniqueNameFnData *>(arg); - std::string fullpath = to_full_path(fn_data.parent_path, name); + CatalogPath fullpath = CatalogPath(fn_data.parent_path) / name; return fn_data.catalog_service.find_catalog_by_path(fullpath); } @@ -70,7 +64,7 @@ AssetCatalog *ED_asset_catalog_add(::AssetLibrary *library, } std::string unique_name = catalog_name_ensure_unique(*catalog_service, name, parent_path); - std::string fullpath = to_full_path(parent_path, unique_name); + CatalogPath fullpath = CatalogPath(parent_path) / unique_name; return catalog_service->create_catalog(fullpath); } |