diff --git a/src/fdb5/CMakeLists.txt b/src/fdb5/CMakeLists.txt index 9e2a16560..61a9ae3c6 100644 --- a/src/fdb5/CMakeLists.txt +++ b/src/fdb5/CMakeLists.txt @@ -295,6 +295,8 @@ if( HAVE_TOCFDB ) toc/FieldRef.h toc/FileSpaceHandler.cc toc/FileSpaceHandler.h + toc/FileSpaceSelector.cc + toc/FileSpaceSelector.h toc/FileSpace.cc toc/FileSpace.h toc/ExpverFileSpaceHandler.cc diff --git a/src/fdb5/toc/FileSpace.cc b/src/fdb5/toc/FileSpace.cc index 8fa31b04c..02173af0f 100644 --- a/src/fdb5/toc/FileSpace.cc +++ b/src/fdb5/toc/FileSpace.cc @@ -14,8 +14,6 @@ #include "eckit/utils/StringTools.h" #include "eckit/exception/Exceptions.h" -#include "eckit/filesystem/FileSpaceStrategies.h" -#include "eckit/os/BackTrace.h" #include "fdb5/LibFdb5.h" #include "fdb5/database/Key.h" @@ -31,13 +29,54 @@ struct VectorPrintSelector { }; } // namespace eckit +//---------------------------------------------------------------------------------------------------------------------- + namespace fdb5 { +namespace { + +metkit::mars::Matcher buildMatcher(const eckit::LocalConfiguration& match) { + std::map regexMap; + for (const auto& keyword : match.keys()) { + std::string pattern; + if (match.isList(keyword)) { + auto values = match.getStringVector(keyword); + if (values.empty()) { + std::ostringstream oss; + oss << "FileSpace match: keyword '" << keyword << "' has no values"; + throw eckit::UserError(oss.str(), Here()); + } + pattern = "^("; + const char* sep = ""; + for (const auto& v : values) { + pattern += sep; + pattern += v; + sep = "|"; + } + pattern += ")$"; + } + else { + pattern = "^(" + match.getString(keyword) + ")$"; + } + regexMap.emplace(keyword, eckit::Regex(pattern)); + } + + return metkit::mars::Matcher(std::move(regexMap), metkit::mars::Matcher::Policy::All); +} + +std::variant makeSelector(const eckit::LocalConfiguration& space) { + if (space.has("match")) { + return MatchSelector{buildMatcher(space.getSubConfiguration("match"))}; + } + return RegexSelector{eckit::Regex{space.getString("regex", ".*")}}; +} + +} // namespace + //---------------------------------------------------------------------------------------------------------------------- -FileSpace::FileSpace(const std::string& name, const std::string& re, const std::string& handler, - const std::vector& roots) : - name_(name), handler_(handler), re_(re), roots_(roots) {} +FileSpace::FileSpace(const std::string& name, const eckit::LocalConfiguration& space, const std::vector& roots) : + name_{name}, handler_{space.getString("handler", "Default")}, selector_{makeSelector(space)}, roots_{roots} {} TocPath FileSpace::filesystem(const Config& config, const Key& key, const eckit::PathName& db) const { // check that the database isn't present already @@ -49,7 +88,6 @@ TocPath FileSpace::filesystem(const Config& config, const Key& key, const eckit: return existingDB; } - LOG_DEBUG_LIB(LibFdb5) << "FDB for key " << key << " not found, selecting a root" << std::endl; LOG_DEBUG_LIB(LibFdb5) << "FDB for key " << key << " not found, selecting a root" << std::endl; return TocPath{FileSpaceHandler::lookup(handler_, config).selectFileSystem(key, *this) / db, ControlIdentifiers{}}; @@ -81,12 +119,11 @@ void FileSpace::enabled(const ControlIdentifier& controlIdentifier, eckit::Strin } } -bool FileSpace::match(const std::string& s) const { - return re_.match(s); +bool FileSpace::match(const Key& key) const { + return std::visit([&](const auto& selector) { return selector.match(key); }, selector_); } eckit::PathName getFullDB(const eckit::PathName& path, const std::string& db) { - static bool searchCaseSensitiveDB = eckit::Resource("fdbSearchCaseSensitiveDB;$FDB_SEARCH_CASESENSITIVE_DB", true); @@ -158,7 +195,9 @@ bool FileSpace::existsDB(const Key& key, const eckit::PathName& db, TocPath& exi void FileSpace::print(std::ostream& out) const { out << "FileSpace(" - << "name=" << name_ << ",handler=" << handler_ << ",regex=" << re_ << ",roots=" << roots_ << ")"; + << "name=" << name_ << ",handler=" << handler_; + std::visit([&out](const auto& s) { out << ",selector=" << s; }, selector_); + out << ",roots=" << roots_ << ")"; } std::vector FileSpace::roots() const { diff --git a/src/fdb5/toc/FileSpace.h b/src/fdb5/toc/FileSpace.h index 654e7f522..dad5f38ff 100644 --- a/src/fdb5/toc/FileSpace.h +++ b/src/fdb5/toc/FileSpace.h @@ -18,18 +18,20 @@ #include #include +#include #include +#include "eckit/config/LocalConfiguration.h" #include "eckit/types/Types.h" -#include "eckit/utils/Regex.h" - #include "fdb5/api/helpers/ControlIterator.h" +#include "fdb5/toc/FileSpaceSelector.h" #include "fdb5/toc/Root.h" namespace fdb5 { class Config; class FileSpaceHandler; +class Key; //---------------------------------------------------------------------------------------------------------------------- @@ -44,8 +46,7 @@ class FileSpace { public: // methods - FileSpace(const std::string& name, const std::string& re, const std::string& handler, - const std::vector& roots); + FileSpace(const std::string& name, const eckit::LocalConfiguration& space, const std::vector& roots); /// Selects the filesystem from where this Key will be inserted /// @note This method must be idempotent -- it returns always the same value after the first call @@ -57,7 +58,7 @@ class FileSpace { void enabled(const ControlIdentifier& controlIdentifier, eckit::StringSet&) const; std::vector enabled(const ControlIdentifier& controlIdentifier) const; - bool match(const std::string& s) const; + bool match(const Key& key) const; friend std::ostream& operator<<(std::ostream& s, const FileSpace& x) { x.print(s); @@ -80,7 +81,7 @@ class FileSpace { std::string handler_; - eckit::Regex re_; + std::variant selector_; RootVec roots_; }; diff --git a/src/fdb5/toc/FileSpaceSelector.cc b/src/fdb5/toc/FileSpaceSelector.cc new file mode 100644 index 000000000..94f30a6e5 --- /dev/null +++ b/src/fdb5/toc/FileSpaceSelector.cc @@ -0,0 +1,73 @@ +/* + * (C) Copyright 2026- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + + +#include "fdb5/toc/FileSpaceSelector.h" + +#include "fdb5/database/Key.h" + +using metkit::mars::Matcher; + +namespace fdb5 { + +//---------------------------------------------------------------------------------------------------------------------- +// Helpers + +namespace { + +/// Adapts fdb5::Key to metkit::mars::RequestLike for use with Matcher. +/// Empty values are treated as absent. +class PartialKeyAdapter : public metkit::mars::RequestLike { +public: + + explicit PartialKeyAdapter(const Key& key) : key_(key) {} + + std::optional get(const std::string& keyword) const override { + const auto [it, found] = key_.find(keyword); + if (!found || it->second.empty()) { + return std::nullopt; + } + return std::cref(it->second); + } + +private: + + const Key& key_; +}; + +} // namespace + +//---------------------------------------------------------------------------------------------------------------------- + +RegexSelector::RegexSelector(eckit::Regex regex) : regex_{std::move(regex)} {} + +bool RegexSelector::match(const Key& key) const { + return regex_.match(key.valuesToString()); +} + +void RegexSelector::print(std::ostream& out) const { + out << regex_; +} + +//---------------------------------------------------------------------------------------------------------------------- + +MatchSelector::MatchSelector(metkit::mars::Matcher matcher) : matcher_{std::move(matcher)} {} + +bool MatchSelector::match(const Key& key) const { + return matcher_.match(PartialKeyAdapter(key), Matcher::MatchOnMissing); +} + +void MatchSelector::print(std::ostream& out) const { + out << matcher_; +} + +//---------------------------------------------------------------------------------------------------------------------- + +} // namespace fdb5 diff --git a/src/fdb5/toc/FileSpaceSelector.h b/src/fdb5/toc/FileSpaceSelector.h new file mode 100644 index 000000000..d84706b83 --- /dev/null +++ b/src/fdb5/toc/FileSpaceSelector.h @@ -0,0 +1,84 @@ +/* + * (C) Copyright 2026- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + + +/// @author Metin Cakircali +/// @date June 2026 + +#pragma once + +#include + +#include "eckit/utils/Regex.h" +#include "metkit/mars/Matcher.h" + +namespace fdb5 { + +class Key; + +//---------------------------------------------------------------------------------------------------------------------- + +class FileSpaceSelector { +public: // methods + + virtual ~FileSpaceSelector() = default; + + virtual bool match(const Key& key) const = 0; + +private: // methods + + virtual void print(std::ostream& out) const = 0; + + friend std::ostream& operator<<(std::ostream& out, const FileSpaceSelector& selector) { + selector.print(out); + return out; + } +}; + +//---------------------------------------------------------------------------------------------------------------------- + +/// uses a regex to match against the serialised key. +class RegexSelector : public FileSpaceSelector { +public: // methods + + RegexSelector(eckit::Regex regex); + + bool match(const Key& key) const override; + +private: // methods + + void print(std::ostream& out) const override; + +private: // members + + eckit::Regex regex_; +}; + +//---------------------------------------------------------------------------------------------------------------------- +// uses a metkit::mars::Matcher to match against the fdb5::Key. +class MatchSelector : public FileSpaceSelector { +public: // methods + + MatchSelector(metkit::mars::Matcher matcher); + + bool match(const Key& key) const override; + +private: // methods + + void print(std::ostream& out) const override; + +private: // members + + metkit::mars::Matcher matcher_; +}; + +//---------------------------------------------------------------------------------------------------------------------- + +} // namespace fdb5 diff --git a/src/fdb5/toc/RootManager.cc b/src/fdb5/toc/RootManager.cc index 47e348914..8a627dd12 100644 --- a/src/fdb5/toc/RootManager.cc +++ b/src/fdb5/toc/RootManager.cc @@ -462,7 +462,10 @@ static FileSpaceTable parseFileSpacesFile(const eckit::PathName& fdbHome) { throw UserError(oss.str(), Here()); } - table.push_back(FileSpace(filespace, regex, handler, roots)); + eckit::LocalConfiguration cfg; + cfg.set("regex", regex); + cfg.set("handler", handler); + table.push_back(FileSpace(filespace, cfg, roots)); break; } @@ -504,7 +507,7 @@ FileSpaceTable RootManager::fileSpaces() { spaceRoots.emplace_back(Root(fdbRootDirectory, "", true, true, true, true)); FileSpaceTable table; - table.emplace_back(FileSpace("", ".*", "Default", spaceRoots)); + table.emplace_back("", eckit::LocalConfiguration{}, spaceRoots); return table; } @@ -532,8 +535,14 @@ FileSpaceTable RootManager::fileSpaces() { } } - table.emplace_back( - FileSpace(name, space.getString("regex", ".*"), space.getString("handler", "Default"), spaceRoots)); + if (space.has("match") && space.has("regex")) { + std::ostringstream oss; + oss << "FDB roots config: file space '" << name + << "' specifies both 'regex' and 'match'; only one is allowed"; + throw eckit::UserError(oss.str(), Here()); + } + + table.emplace_back(name, space, spaceRoots); } return table; } @@ -607,11 +616,10 @@ TocPath RootManager::directory(const Key& key) { } // returns the first filespace that matches - - std::string keystr = key.valuesToString(); + // Write routing uses a full key — require all matcher keywords to be present. for (FileSpaceTable::const_iterator i = spacesTable_.begin(); i != spacesTable_.end(); ++i) { - if (i->match(keystr)) { + if (i->match(key)) { TocPath db = i->filesystem(config_, key, dbpath); LOG_DEBUG_LIB(LibFdb5) << "Database directory " << db.directory_ << std::endl; return db; @@ -619,7 +627,7 @@ TocPath RootManager::directory(const Key& key) { } std::ostringstream oss; - oss << "No FDB file space for " << key << " (" << keystr << ")"; + oss << "No FDB file space for " << key << " (" << key.valuesToString() << ")"; throw eckit::SeriousBug(oss.str()); } @@ -627,18 +635,20 @@ std::vector RootManager::visitableRoots(const std::set& keys) { eckit::StringSet roots; - std::set keystrings; - for (const auto& key : keys) { - keystrings.insert(key.valuesToString()); + if (LibFdb5::instance().debug()) { + std::set keystrings; + for (const auto& key : keys) { + keystrings.insert(key.valuesToString()); + } + eckit::Log::debug() << "RootManager::visitableRoots() trying to match keys " << keystrings + << std::endl; } - LOG_DEBUG_LIB(LibFdb5) << "RootManager::visitableRoots() trying to match keys " << keystrings << std::endl; - for (const auto& space : spacesTable_) { bool matched = false; - for (const std::string& k : keystrings) { - if (space.match(k) || k.empty()) { + for (const Key& key : keys) { + if (space.match(key) || key.empty()) { LOG_DEBUG_LIB(LibFdb5) << "MATCH space " << space << std::endl; space.enabled(ControlIdentifier::List, roots); matched = true; @@ -677,11 +687,9 @@ std::vector RootManager::canArchiveRoots(const Key& key) { eckit::StringSet roots; - std::string k = key.valuesToString(); - + // Archive uses a full key — require all matcher keywords to be present. for (FileSpaceTable::const_iterator i = spacesTable_.begin(); i != spacesTable_.end(); ++i) { - if (i->match(k)) { - + if (i->match(key)) { i->enabled(ControlIdentifier::Archive, roots); } } @@ -695,11 +703,9 @@ std::vector RootManager::canMoveToRoots(const Key& key) { eckit::StringSet roots; - std::string k = key.valuesToString(); - + // Wipe uses a full key — require all matcher keywords to be present. for (FileSpaceTable::const_iterator i = spacesTable_.begin(); i != spacesTable_.end(); ++i) { - if (i->match(k)) { - + if (i->match(key)) { i->enabled(ControlIdentifier::Wipe, roots); } } diff --git a/tests/fdb/CMakeLists.txt b/tests/fdb/CMakeLists.txt index e6cecbba1..5a491e6f3 100644 --- a/tests/fdb/CMakeLists.txt +++ b/tests/fdb/CMakeLists.txt @@ -68,6 +68,7 @@ endforeach() add_subdirectory( api ) add_subdirectory( database ) +add_subdirectory( toc ) add_subdirectory( type ) add_subdirectory( daos ) add_subdirectory( fam ) diff --git a/tests/fdb/toc/CMakeLists.txt b/tests/fdb/toc/CMakeLists.txt new file mode 100644 index 000000000..2d754e5f6 --- /dev/null +++ b/tests/fdb/toc/CMakeLists.txt @@ -0,0 +1,9 @@ +set( _test_environment ${test_environment}) + +list( APPEND _test_environment + FDB_HOME=${PROJECT_BINARY_DIR} ) + +ecbuild_add_test( TARGET fdb_test_toc_filespace_match + SOURCES test_filespace_match.cc + LIBS fdb5 + ENVIRONMENT "${_test_environment}") diff --git a/tests/fdb/toc/test_filespace_match.cc b/tests/fdb/toc/test_filespace_match.cc new file mode 100644 index 000000000..df2199920 --- /dev/null +++ b/tests/fdb/toc/test_filespace_match.cc @@ -0,0 +1,256 @@ +/* + * (C) Copyright 1996- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + +/// Unit tests for FDB-331: keyword-based file-space matching using metkit::mars::Matcher. + +#include "fdb5/config/Config.h" +#include "fdb5/database/Key.h" +#include "fdb5/toc/FileSpace.h" +#include "fdb5/toc/RootManager.h" + +#include "eckit/config/LocalConfiguration.h" +#include "eckit/config/YAMLConfiguration.h" +#include "eckit/exception/Exceptions.h" +#include "eckit/filesystem/LocalPathName.h" +#include "eckit/filesystem/PathName.h" +#include "eckit/filesystem/TmpDir.h" +#include "eckit/testing/Test.h" + +namespace fdb5::test { + +//---------------------------------------------------------------------------------------------------------------------- + +namespace { + +eckit::LocalConfiguration makeMatchConfig() { + // Equivalent YAML: + // match: + // class: od + // expver: '0001' + // stream: [scda, scwv, oper, wave, enfo, waef] + eckit::LocalConfiguration match; + match.set("class", "od"); + match.set("expver", std::string{"0001"}); + match.set("stream", std::vector{"scda", "scwv", "oper", "wave", "enfo", "waef"}); + eckit::LocalConfiguration cfg; + cfg.set("match", match); + return cfg; +} + +fdb5::FileSpace makeMatchSpace() { + return fdb5::FileSpace("kw", makeMatchConfig(), {}); +} + +} // namespace + +//---------------------------------------------------------------------------------------------------------------------- +// Matcher-backed FileSpace: full key with allowed value matches + +CASE("Matcher: full key with allowed value matches") { + auto space = makeMatchSpace(); + + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"stream", "oper"}, {"date", "20241118"}}}; + EXPECT(space.match(key)); +} + +CASE("Matcher: present keyword with disallowed value does not match") { + auto space = makeMatchSpace(); + + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"stream", "eefo"}, {"date", "20241118"}}}; + EXPECT(!space.match(key)); +} + +CASE("Matcher: missing keyword does not disqualify the space (FDB-331, MatchOnMissing)") { + auto space = makeMatchSpace(); + + // No 'stream' keyword at all -- the partial request that broke fdb-list. + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"date", "20241118"}, {"time", "0000"}}}; + EXPECT(space.match(key)); +} + +CASE("Matcher: missing keyword disqualifies with DontMatchOnMissing") { + auto space = makeMatchSpace(); + + // The same partial key, but with DontMatchOnMissing — the absent + // 'stream' keyword now causes the space to be excluded. + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"date", "20241118"}, {"time", "0000"}}}; + EXPECT(!space.match(key)); +} + +CASE("Matcher: empty keyword value is treated as absent (FDB-331)") { + // Schema::matchDatabase() produces partial Keys whose absent dimensions + // are present-but-empty. The KeyAccessor treats such placeholders as + // "absent", so MatchOnMissing lets them through. + auto space = makeMatchSpace(); + + fdb5::Key key; + key.set("class", "od"); + key.set("expver", "0001"); + key.set("stream", ""); // <-- placeholder, not a real value + EXPECT(space.match(key)); +} + +CASE("Matcher: scalar value mismatch fails") { + auto space = makeMatchSpace(); + + fdb5::Key key; + key.set("class", "rd"); + key.set("expver", "0001"); + EXPECT(!space.match(key)); +} + +CASE("Matcher: empty key matches with MatchOnMissing (no constraints can be violated)") { + auto space = makeMatchSpace(); + + fdb5::Key key; + EXPECT(space.match(key)); +} + +//---------------------------------------------------------------------------------------------------------------------- + +CASE("FileSpace: regex backend still works on stringified key") { + eckit::LocalConfiguration cfg; + cfg.set("regex", std::string{"^od:(0001):.*"}); + cfg.set("handler", std::string{"Default"}); + fdb5::FileSpace space("regex_only", cfg, {}); + + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"stream", "oper"}}}; + EXPECT(space.match(key)); + + fdb5::Key kRd{{{"class", "rd"}, {"expver", "0001"}, {"stream", "oper"}}}; + EXPECT(!space.match(kRd)); +} + +CASE("FileSpace: matcher backend recovers partial requests") { + auto space = makeMatchSpace(); + + fdb5::Key kFull{{{"class", "od"}, {"expver", "0001"}, {"stream", "oper"}}}; + EXPECT(space.match(kFull)); + + // The FDB-331 reproducer: request without stream still matches the + // first space, so its roots will be visited. + fdb5::Key kPartial{{{"class", "od"}, {"expver", "0001"}, {"date", "20241118"}, {"time", "0000"}}}; + EXPECT(space.match(kPartial)); +} + +//---------------------------------------------------------------------------------------------------------------------- +// Input validation + +CASE("buildMatcher: keyword with empty value list throws UserError") { + eckit::LocalConfiguration match; + match.set("stream", std::vector{}); + eckit::LocalConfiguration cfg; + cfg.set("match", match); + EXPECT_THROWS_AS(fdb5::FileSpace("test", cfg, {}), eckit::UserError); +} + +//---------------------------------------------------------------------------------------------------------------------- +// Read YAML + +CASE("buildMatcher: builds correctly from YAML (mixed scalar + list)") { + const std::string yaml = + "match:\n" + " class: od\n" + " expver: '0001'\n" + " stream: [scda, scwv, oper]\n"; + eckit::YAMLConfiguration yamlCfg{yaml}; + eckit::LocalConfiguration cfg{yamlCfg}; + + fdb5::FileSpace space("yaml_test", cfg, {}); + + fdb5::Key key{{{"class", "od"}, {"expver", "0001"}, {"stream", "oper"}}}; + EXPECT(space.match(key)); + + fdb5::Key keyOther{{{"class", "od"}, {"expver", "0001"}, {"stream", "eefo"}}}; + EXPECT(!space.match(keyOther)); +} + +//---------------------------------------------------------------------------------------------------------------------- +// RootManager + +CASE("RootManager: visitableRoots() uses match: and handles partial keys (FDB-331)") { + eckit::TmpDir root1{eckit::LocalPathName::cwd().c_str()}; + eckit::TmpDir root2{eckit::LocalPathName::cwd().c_str()}; + + // Mirrors the FDB-331 production layout: a "primary" space narrowed + // by stream, and a "fallback" space matching any od/0001 key. + const std::string yaml = + "type: local\n" + "engine: toc\n" + "spaces:\n" + " - handler: Default\n" + " match:\n" + " class: od\n" + " expver: '0001'\n" + " stream: [scda, scwv, oper, wave, enfo, waef]\n" + " roots:\n" + " - path: " + + root1.asString() + + "\n" + " - handler: Default\n" + " match:\n" + " class: od\n" + " expver: '0001'\n" + " roots:\n" + " - path: " + + root2.asString() + "\n"; + + fdb5::Config config{eckit::YAMLConfiguration{yaml}}; + fdb5::CatalogueRootManager mgr{config}; + + // Full key with an allowed stream: both spaces match (primary by name, + // fallback unconstrained on stream) -- both roots are visited. + fdb5::Key kFull{{{"class", "od"}, {"expver", "0001"}, {"stream", "oper"}, {"date", "20241118"}}}; + auto rootsFull = mgr.visitableRoots(kFull); + EXPECT_EQUAL(rootsFull.size(), 2U); + + // Full key with a NON-allowed stream: only the fallback matches. + fdb5::Key kEefo{{{"class", "od"}, {"expver", "0001"}, {"stream", "eefo"}, {"date", "20241118"}}}; + auto rootsEefo = mgr.visitableRoots(kEefo); + EXPECT_EQUAL(rootsEefo.size(), 1U); + EXPECT_EQUAL(rootsEefo.front(), eckit::PathName{root2.asString()}); + + // FDB-331 reproducer: stream is missing entirely. With the existing + // regex backend the primary space would be silently dropped here + // (regex on the stringified key cannot tell "absent" apart from + // "empty"); with match: both spaces remain visitable. + fdb5::Key kPartial{{{"class", "od"}, {"expver", "0001"}, {"date", "20241118"}}}; + auto rootsPartial = mgr.visitableRoots(kPartial); + EXPECT_EQUAL(rootsPartial.size(), 2U); + + // A different class matches neither space. + fdb5::Key kRd{{{"class", "rd"}, {"expver", "0001"}, {"stream", "oper"}}}; + auto rootsRd = mgr.visitableRoots(kRd); + EXPECT(rootsRd.empty()); +} + +CASE("RootManager: rejects spaces with both regex: and match:") { + const std::string yaml = + "type: local\n" + "engine: toc\n" + "spaces:\n" + " - handler: Default\n" + " regex: ^od:.*\n" + " match:\n" + " class: od\n" + " roots:\n" + " - path: /tmp\n"; + + fdb5::Config config{eckit::YAMLConfiguration{yaml}}; + EXPECT_THROWS_AS(fdb5::CatalogueRootManager{config}, eckit::UserError); +} + +} // namespace fdb5::test + +//---------------------------------------------------------------------------------------------------------------------- + +int main(int argc, char** argv) { + return eckit::testing::run_tests(argc, argv); +} diff --git a/tests/regressions/CMakeLists.txt b/tests/regressions/CMakeLists.txt index 7b53dd7c4..5ca07429c 100644 --- a/tests/regressions/CMakeLists.txt +++ b/tests/regressions/CMakeLists.txt @@ -21,6 +21,7 @@ if (HAVE_FDB_BUILD_TOOLS) # test scripts use the fdb tools add_subdirectory(FDB-303) add_subdirectory(FDB-307) add_subdirectory(FDB-310) + add_subdirectory(FDB-331) add_subdirectory(FDB-332) add_subdirectory(FDB-533) add_subdirectory(FDB-535) diff --git a/tests/regressions/FDB-331/CMakeLists.txt b/tests/regressions/FDB-331/CMakeLists.txt new file mode 100644 index 000000000..00678217d --- /dev/null +++ b/tests/regressions/FDB-331/CMakeLists.txt @@ -0,0 +1,6 @@ +ecbuild_configure_file( FDB-331.sh.in FDB-331.sh @ONLY ) + +ecbuild_add_test( + TYPE SCRIPT + COMMAND FDB-331.sh + ENVIRONMENT "${test_environment}" ) diff --git a/tests/regressions/FDB-331/FDB-331.sh.in b/tests/regressions/FDB-331/FDB-331.sh.in new file mode 100644 index 000000000..912be0f7b --- /dev/null +++ b/tests/regressions/FDB-331/FDB-331.sh.in @@ -0,0 +1,193 @@ +#!/usr/bin/env bash + +# Regression test for FDB-331: +# Root selection in FDB historically uses the ``regex:`` selector, which +# matches against the colon-joined string form of a Key. When a request +# omits a keyword that appears in the regex (e.g. ``stream``), the regex +# fails to match and the corresponding root is silently skipped from +# fdb-list / fdb-where output. +# +# ``regex:`` is widely used in production and its behaviour is preserved +# unchanged. FDB-331 introduces ``match:`` as an additional, opt-in +# selector that operates on the structured Key: missing keywords are +# tolerated, restoring full-coverage visitation for partial requests. +# +# Phase 1 -- ``regex:`` selector: documents the existing production +# behaviour (and exhibits FDB-331 on partial requests). +# Phase 2 -- ``match:`` selector: shows the new keyword-aware behaviour. +# Phase 3 -- a single space cannot specify both ``regex:`` and ``match:``. + +set -eux + +fdbwrite="$" +fdblist="$" +fdbwhere="$" + +gribset="$" + +srcdir=@CMAKE_CURRENT_SOURCE_DIR@ +bindir=@CMAKE_CURRENT_BINARY_DIR@ + +export FDB_HOME=$bindir + +# Keep stdout clean: assertions below grep verbatim tool output +# FDB/eckit debug streams contaminate those outputs +export FDB_DEBUG=0 +export ECKIT_DEBUG=0 + +### cleanup and prepare test + +rm -rf ${bindir}/root_primary ${bindir}/root_fallback +mkdir -p ${bindir}/root_primary ${bindir}/root_fallback + +cp $srcdir/schema $bindir/schema +cp $srcdir/x.grib $bindir/x.grib + +cd $bindir + +### Stage data: one DB per stream value. +### - 'oper' is a "narrow" stream listed in the primary space +### - 'enda' is NOT in the primary space's narrow set +$gribset -s class=od,expver=0001,stream=oper x.grib data.oper.grib +$gribset -s class=od,expver=0001,stream=enda x.grib data.enda.grib + +### Configs ########################################################### + +# Two roots simulating the FDB-331 production layout: a "primary" space +# narrowed by stream and a "fallback" space matching any od/0001 key. + +# regex.yaml: production-style configuration (unchanged semantics). +cat > regex.yaml < match.yaml < invalid.yaml < primary root is visited. +$fdbwhere class=od,expver=0001,stream=oper > where_regex_full.out +grep -q root_primary where_regex_full.out + +# Partial key (no stream): the primary regex requires the stream slot +# to be filled, so it does not match -- only the fallback is visited. +# This is the existing production behaviour and stays unchanged. +$fdbwhere class=od,expver=0001 > where_regex_partial.out +grep -q root_fallback where_regex_partial.out +! grep -q root_primary where_regex_partial.out + +# fdb-list mirrors the behaviour: only entries from the visited root +# are reported on a partial key. +$fdblist class=od,expver=0001,stream=oper --porcelain > list_regex_full.out +test -s list_regex_full.out + +$fdblist class=od,expver=0001 --porcelain > list_regex_partial.out +grep -q stream=enda list_regex_partial.out +! grep -q stream=oper list_regex_partial.out + +echo "Phase 1 OK: regex selector preserves existing production behaviour" + +### Phase 2 -- match selector: keyword-aware addition ################## + +export FDB5_CONFIG_FILE=$bindir/match.yaml + +# Full key still routes the same way as regex. +$fdbwhere class=od,expver=0001,stream=oper > where_match_full.out +grep -q root_primary where_match_full.out + +# Partial key: BOTH roots are visited because the absent ``stream`` +# keyword no longer disqualifies the primary space. +$fdbwhere class=od,expver=0001 > where_match_partial.out +grep -q root_primary where_match_partial.out +grep -q root_fallback where_match_partial.out + +# Stream NOT in the primary's allowed set -- only the fallback is +# visited (the constraint is still enforced when the keyword IS +# present). +$fdbwhere class=od,expver=0001,stream=enda > where_match_enda.out +grep -q root_fallback where_match_enda.out +! grep -q root_primary where_match_enda.out + +# fdb-list partial key now finds BOTH oper and enda entries. +$fdblist class=od,expver=0001 --porcelain > list_match_partial.out +grep -q stream=oper list_match_partial.out +grep -q stream=enda list_match_partial.out + +# A different class matches neither space. +$fdbwhere class=rd,expver=0001 > where_match_rd.out || true +! grep -qE "root_primary|root_fallback" where_match_rd.out + +echo "Phase 2 OK: match selector recovers full-coverage visitation on partial keys" + +### Phase 3 -- regex and match in the same space are mutually exclusive + +export FDB5_CONFIG_FILE=$bindir/invalid.yaml + +if $fdbwhere class=od,expver=0001 >/dev/null 2>&1; then + echo "Invalid config (regex + match) was accepted!" >&2 + exit 1 +fi + +echo "Phase 3 OK: regex + match in the same space is rejected" +echo "FDB-331 reproducer passed." diff --git a/tests/regressions/FDB-331/schema b/tests/regressions/FDB-331/schema new file mode 100644 index 000000000..1a6e9a1ff --- /dev/null +++ b/tests/regressions/FDB-331/schema @@ -0,0 +1,590 @@ + +# * Format of the rules is: + +# [a1, a2, a3 ...[b1, b2, b3... [c1, c2, c3...]]] + +# - The first level (a) defines which attributes are used to name the top level directory +# - The second level (b) defines which attributes are used to name the data files +# - The third level (c) defines which attributes are used as index keys + +# * Rules can be grouped + +# [a1, a2, a3 ... +# [b1, b2, b3... [c1, c2, c3...]] +# [B1, B2, B3... [C1, C2, C3...]] +# ] + +# * A list of values can be given for an attribute +# [ ..., stream=enfo/efov, ... ] +# This will be used when matching rules. + +# * Attributes can be typed +# Globally, at the beginning of this file: + +# refdate: Date; + +# or in the context of a rule: +# [type=cl, ... [date:ClimateMonth, ...]] + +# Typing attributes is done when the user's requests or the GRIB values need to be modified before directories, files and indexes are created. For example, ClimateMonth will transform 2010-04-01 to 'may' internally. + +# * Attributes can be optional +# [ step, levelist?, param ] +# They will be replaced internally by an empty value. It is also possible to provide a default substitution value: e.g. [domain?g] will consider the domain to be 'g' if missing. + +# * Attributes can be removed: +# [grid-] +# This is useful to remove attributes present in the GRIB that should not be ignored + +# * Rules are matched: + +# - If the attributes are present in the GRIB/Request, or marked optional or ignored +# - If a list of possible value is provided, one of them must match, for example +# [ class, expver, stream=enfo/efov, date, time, domain ] +# will match either stream=enfo or stream=efov, all other attributes will be matched if they exist in the GRIB or user's request + +# * On archive: +# - Attributes are extracted from the GRIB (namespace 'mars'), possibly modified by the attribute type +# - Only the first rule is used, so order is important +# - All GRIB attributes must be used by the rules, otherwise an error is raised + +# * On retrieve: +# - Attributes are extracted from the user's request, possibly modified by the attribute type (e.g. for handling of U/V) +# - All the matching rules are considered +# - Only attributes listed in the rules are used to extract values from the user's request + + +# Default types + +param: Param; +step: Step; +date: Date; +hdate: Date; +refdate: Date; +latitude: Double; +longitude: Double; +levelist: Double; +grid: Grid; +expver: Expver; + +time: Time; +fcmonth: Integer; + +number: Integer; +frequency: Integer; +direction: Integer; +channel: Integer; + +instrument: Integer; +ident: Integer; + +diagnostic: Integer; +iteration: Integer; +system: Integer; +method: Integer; + +# ??????? + +# reference: Integer; +# fcperiod: Integer; + +# opttime: Integer; +# leadtime: Integer; + +# quantile: ?????? +# range: ?????? + +# band: Integer; + + +######################################################## +# These rules must be first, otherwise fields of These +# classes will be index with the default rule for oper +[ class=ti/s2, expver, stream, date, time, model + [ origin, type, levtype, hdate? + [ step, number?, levelist?, param ]] +] + +[ class=ms, expver, stream, date, time, country=de + [ domain, type, levtype, dbase, rki, rty, ty + [ step, levelist?, param ]] +] + +[ class=ms, expver, stream, date, time, country=it + [ domain, type, levtype, model, bcmodel, icmodel:First3 + [ step, levelist?, param ] + ] +] + +[ class=el, expver, stream, date, time, domain + [ origin, type, levtype + [ step, levelist?, param ]] +] + +######################################################## +# The are the rules matching most of the fields +# oper/dcda +[ class, expver, stream=oper/dcda/scda, date, time, domain? + + [ type=im/sim + [ step?, ident, instrument, channel ]] + + [ type=ssd + [ step, param, ident, instrument, channel ]] + + [ type=4i, levtype + [ step, iteration, levelist, param ]] + + [ type=me, levtype + [ step, number, levelist?, param ]] + + [ type=ef, levtype + [ step, levelist?, param, channel? ]] + + [ type=ofb/mfb + [ obsgroup, reportype ]] + + [ type, levtype + [ step, levelist?, param ]] + +] + +# dcwv/scwv/wave +[ class, expver, stream=dcwv/scwv/wave, date, time, domain + [ type, levtype + [ step, param, frequency?, direction? ]]] + +# enfo +[ class, expver, stream=enfo/efov, date, time, domain + + [ type, levtype=dp, product?, section? + [ step, number?, levelist?, latitude?, longitude?, range?, param ]] + + [ type=tu, levtype, reference + [ step, number, levelist?, param ]] + + [ type, levtype + [ step, quantile?, number?, levelist?, param ]] + +] + +# waef/weov +[ class, expver, stream=waef/weov, date, time, domain + [ type, levtype + [ step, number?, param, frequency?, direction? ]] +] + +######################################################## +# enda +[ class, expver, stream=enda, date, time, domain + + [ type=ef/em/es/ses, levtype + [ step, number?, levelist?, param, channel? ]] + + [ type=ssd + [ step, number, param, ident, instrument, channel ]] + + + [ type, levtype + [ step, number?, levelist?, param ]] +] + +# ewda +[ class, expver, stream=ewda, date, time, domain + [ type, levtype + [ step, number?, param, frequency?, direction? ]] +] + + +######################################################## +# elda +[ class, expver, stream=elda, date, time, domain? + + [ type=ofb/mfb + [ obsgroup, reportype ]] + + [ type, levtype, anoffset + [ step, number?, levelist?, iteration?, param, channel? ]] +] + +# ewda +[ class, expver, stream=ewla, date, time, domain + [ type, levtype, anoffset + [ step, number?, param, frequency?, direction? ]] +] + +######################################################## +# elda +[ class, expver, stream=lwda, date, time, domain? + + [ type=ssd, anoffset + [ step, param, ident, instrument, channel ]] + + [type=me, levtype, anoffset + [ number, step, levelist?, param]] + + [ type=4i, levtype, anoffset + [ step, iteration, levelist, param ]] + + [ type=ofb/mfb + [ obsgroup, reportype ]] + + [ type, levtype, anoffset + [ step, levelist?, param]] +] + +# ewda +[ class, expver, stream=lwwv, date, time, domain + [ type, levtype, anoffset + [ step, param, frequency?, direction? ]] +] +######################################################## +# amap +[ class, expver, stream=amap, date, time, domain + [ type, levtype, origin + [ step, levelist?, param ]]] + +# maed +[ class, expver, stream=maed, date, time, domain + [ type, levtype, origin + [ step, levelist?, param ]]] + +# mawv +[ class, expver, stream=mawv, date, time, domain + [ type, levtype, origin + [ step, param, frequency?, direction? ]]] + +# cher +[ class, expver, stream=cher, date, time, domain + [ type, levtype + [ step, levelist, param ]]] + + +# efhc +[ class, expver, stream=efhc, refdate, time, domain + [ type, levtype, date + [ step, number?, levelist?, param ]]] + +# efho +[ class, expver, stream=efho, date, time, domain + [ type, levtype, hdate + [ step, number?, levelist?, param ]]] + + +# efhs +[ class, expver, stream=efhs, date, time, domain + [ type, levtype + [ step, quantile?, number?, levelist?, param ]]] + +# wehs +[ class, expver, stream=wehs, date, time, domain + [ type, levtype + [ step, quantile?, number?, levelist?, param ]]] + +# kwbc +[ class, expver, stream=kwbc, date, time, domain + [ type, levtype + [ step, number?, levelist?, param ]]] + +# ehmm +[ class, expver, stream=ehmm, date, time, domain + [ type, levtype, hdate + [ fcmonth, levelist?, param ]]] + + +# ammc/cwao/edzw/egrr/lfpw/rjtd/toga +[ class, expver, stream=ammc/cwao/edzw/egrr/lfpw/rjtd/toga/fgge, date, time, domain + [ type, levtype + [ step, levelist?, param ]]] + +######################################################################## + +# enfh +[ class, expver, stream=enfh, date, time, domain + + [ type, levtype=dp, hdate, product?, section? + [ step, number?, levelist?, latitude?, longitude?, range?, param ]] + + [ type, levtype, hdate + [ step, number?, levelist?, param ]] +] + +# enwh +[ class, expver, stream=enwh, date, time, domain + [ type, levtype, hdate + [ step, number?, param, frequency?, direction? ]] +] + +######################################################################## +# sens +[ class, expver, stream=sens, date, time, domain + [ type, levtype + [ step, diagnostic, iteration, levelist?, param ]]] + +######################################################################## +# esmm +[ class, expver, stream=esmm, date, time, domain + [ type, levtype + [ fcmonth, levelist?, param ]]] +# ewhc +[ class, expver, stream=ewhc, refdate, time, domain + [ type, levtype, date + [ step, number?, param, frequency?, direction? ]]] + +######################################################################## +# ewho +[ class, expver, stream=ewho, date, time, domain + [ type, levtype, hdate + [ step, number?, param, frequency?, direction? ]]] + +# mfam +[ class, expver, stream=mfam, date, time, domain + + [ type=pb/pd, levtype, origin, system?, method + [ fcperiod, quantile, levelist?, param ]] + + [ type, levtype, origin, system?, method + [ fcperiod, number?, levelist?, param ]] + +] + +# mfhm +[ class, expver, stream=mfhm, refdate, time, domain + [ type, levtype, origin, system?, method, date? + [ fcperiod, number?, levelist?, param ]]] +# mfhw +[ class, expver, stream=mfhw, refdate, time, domain + [ type, levtype, origin, system?, method, date + [ step, number?, param ]]] +# mfwm +[ class, expver, stream=mfwm, date, time, domain + [ type, levtype, origin, system?, method + [ fcperiod, number, param ]]] +# mhwm +[ class, expver, stream=mhwm, refdate, time, domain + [ type, levtype, origin, system?, method, date + [ fcperiod, number, param ]]] + +# mmsf +[ class, expver, stream=mmsf, date, time, domain + + [ type, levtype=dp, origin, product, section, system?, method + [ step, number, levelist?, latitude?, longitude?, range?, param ]] + + [ type, levtype, origin, system?, method + [ step, number, levelist?, param ]] +] + +# mnfc +[ class, expver, stream=mnfc, date, time, domain + + [ type, levtype=dp, origin, product, section, system?, method + [ step, number?, levelist?, latitude?, longitude?, range?, param ]] + + [ type, levtype, origin, system?, method + [ step, number?, levelist?, param ]] +] + +# mnfh +[ class, expver, stream=mnfh, refdate, time, domain + [ type, levtype=dp, origin, product, section, system?, method, date + [ step, number?, levelist?, latitude?, longitude?, range?, param ]] + [ type, levtype, origin, system?, method, date? + [ step, number?, levelist?, param ]] +] + +# mnfm +[ class, expver, stream=mnfm, date, time, domain + [ type, levtype, origin, system?, method + [ fcperiod, number?, levelist?, param ]]] + +# mnfw +[ class, expver, stream=mnfw, date, time, domain + [ type, levtype, origin, system?, method + [ step, number?, param ]]] + +# ea/mnth +[ class=ea, expver, stream=mnth, date, domain + [ type, levtype + [ time, step?, levelist?, param ]]] + +# mnth +[ class, expver, stream=mnth, domain + [ type=cl, levtype + [ date: ClimateMonthly, time, levelist?, param ]] + [ type, levtype + [ date , time, step?, levelist?, param ]]] + +# mofc +[ class, expver, stream=mofc, date, time, domain + [ type, levtype=dp, product, section, system?, method + [ step, number?, levelist?, latitude?, longitude?, range?, param ]] + [ type, levtype, system?, method + [ step, number?, levelist?, param ]] +] + +# mofm +[ class, expver, stream=mofm, date, time, domain + [ type, levtype, system?, method + [ fcperiod, number, levelist?, param ]]] + +# mmsa/msmm +[ class, expver, stream=mmsa, date, time, domain + [ type, levtype, origin, system?, method + [ fcmonth, number?, levelist?, param ]]] + +[ class, expver, stream=msmm, date, time, domain + [ type, levtype, origin, system?, method + [ fcmonth, number?, levelist?, param ]]] + +# ocea +[ class, expver, stream=ocea, date, time, domain + [ type, levtype, product, section, system?, method + [ step, number, levelist?, latitude?, longitude?, range?, param ]] +] + +#=# seas +[ class, expver, stream=seas, date, time, domain + + [ type, levtype=dp, product, section, system?, method + [ step, number, levelist?, latitude?, longitude?, range?, param ]] + + [ type, levtype, system?, method + [ step, number, levelist?, param ]] +] + +# sfmm/smma +[ class, expver, stream=sfmm/smma, date, time, domain + [ type, levtype, system?, method + [ fcmonth, number?, levelist?, param ]]] + +# supd +[ class=od, expver, stream=supd, date, time, domain + [ type, levtype, origin?, grid + [ step, levelist?, param ]]] + +# For era +[ class, expver, stream=supd, date, time, domain + [ type, levtype, grid- # The minus sign is here to consume 'grid', but don't index it + [ step, levelist?, param ]]] + +# swmm +[ class, expver, stream=swmm, date, time, domain + [ type, levtype, system?, method + [ fcmonth, number, param ]]] + +# wamf +[ class, expver, stream=wamf, date, time, domain + [ type, levtype, system?, method + [ step, number?, param ]]] + +# ea/wamo +[ class=ea, expver, stream=wamo, date, domain + [ type, levtype + [ time, step?, param ]]] + +# wamo +[ class, expver, stream=wamo, domain + [ type=cl, levtype + [ date: ClimateMonthly, time, param ]] + [ type, levtype + [ date, time, step?, param ]]] + +# wamd +[ class, expver, stream=wamd, date, domain + [ type, levtype + [ param ]]] + +# wasf +[ class, expver, stream=wasf, date, time, domain + [ type, levtype, system?, method + [ step, number, param ]]] +# wmfm +[ class, expver, stream=wmfm, date, time, domain + [ type, levtype, system?, method + [ fcperiod, number, param ]]] + +# moda +[ class, expver, stream=moda, date, domain + [ type, levtype + [ levelist?, param ]]] + +# msdc/mdfa/msda +[ class, expver, stream=msdc/mdfa/msda, domain + [ type, levtype + [ date, time?, step?, levelist?, param ]]] + + + +# seap +[ class, expver, stream=seap, date, time, domain + [ type=sv/svar, levtype, origin, method? + [ step, leadtime, opttime, number, levelist?, param ]] + + [ type=ef, levtype, origin + [ step, levelist?, param, channel? ]] + + [ type, levtype, origin + [ step, levelist?, param ]] + + ] + +[ class, expver, stream=mmaf, date, time, domain + [ type, levtype, origin, system?, method + [ step, number, levelist?, param ]] +] + +[ class, expver, stream=mmam, date, time, domain + [ type, levtype, origin, system?, method + [ fcmonth, number, levelist?, param ]] +] + + +[ class, expver, stream=dacl, domain + [ type=pb, levtype + [ date: ClimateDaily, time, step, quantile, levelist?, param ]] + [ type, levtype + [ date: ClimateDaily, time, step, levelist?, param ]] + +] + +[ class, expver, stream=dacw, domain + [ type=pb, levtype + [ date: ClimateDaily, time, step, quantile, param ]] + [ type, levtype + [ date: ClimateDaily, time, step, param ]] + +] + +[ class, expver, stream=edmm/ewmm, date, time, domain + [ type=ssd + [ step, number, param, ident, instrument, channel ]] + [ type, levtype + [ step, number, levelist?, param ]] +] + +[ class, expver, stream=edmo/ewmo, date, domain + [ type, levtype + [ number, levelist?, param ]] +] + +# stream gfas +[ class=mc/rd, expver, stream=gfas, date, time, domain + [ type=ga, levtype + [ step, param ]] + + [ type=gsd + [ param, ident, instrument ]] + +] + +# class is e2 +[ class, expver, stream=espd, date, time, domain + [ type, levtype, origin, grid + [ step, number, levelist?, param ]]] + +[ class=cs, expver, stream, date:Default, time, domain + [ type, levtype + [ step, levelist?, param ]]] + + diff --git a/tests/regressions/FDB-331/x.grib b/tests/regressions/FDB-331/x.grib new file mode 100644 index 000000000..a70adb6f9 Binary files /dev/null and b/tests/regressions/FDB-331/x.grib differ