From 761fef66d65af367734bc3e105c9a46bac1867b0 Mon Sep 17 00:00:00 2001 From: Tobias Kremer Date: Mon, 27 Jul 2026 11:35:27 +0200 Subject: [PATCH] feat(fdb-586): Augment schema parser error messages Now showing in which step of the schema file parsing the error is occurring. Also failing on encountered non-ascii characters in schema files --- src/fdb5/CMakeLists.txt | 2 + src/fdb5/api/exceptions/SchemaError.cc | 18 ++ src/fdb5/api/exceptions/SchemaError.h | 26 +++ src/fdb5/rules/Schema.cc | 23 +-- src/fdb5/rules/SchemaParser.cc | 168 +++++++++++++----- src/fdb5/rules/SchemaParser.h | 22 ++- tests/fdb/CMakeLists.txt | 1 + tests/fdb/parsing/CMakeLists.txt | 13 ++ .../data/broken_schema_comments_no_rule | 88 +++++++++ tests/fdb/parsing/data/broken_schema_no_rule | 6 + .../data/broken_types_missing_semicolon | 6 + tests/fdb/parsing/data/broken_types_no_name | 6 + tests/fdb/parsing/data/broken_types_no_type | 6 + tests/fdb/parsing/data/non_ascii_before_rule | 4 + tests/fdb/parsing/data/non_ascii_chars | 11 ++ tests/fdb/parsing/data/schema | 11 ++ tests/fdb/parsing/data/schema_incomplete_rule | 10 ++ tests/fdb/parsing/test_schema_parsing.cc | 164 +++++++++++++++++ tests/fdb/type/test_toKey.cc | 19 +- 19 files changed, 537 insertions(+), 67 deletions(-) create mode 100644 src/fdb5/api/exceptions/SchemaError.cc create mode 100644 src/fdb5/api/exceptions/SchemaError.h create mode 100644 tests/fdb/parsing/CMakeLists.txt create mode 100644 tests/fdb/parsing/data/broken_schema_comments_no_rule create mode 100644 tests/fdb/parsing/data/broken_schema_no_rule create mode 100644 tests/fdb/parsing/data/broken_types_missing_semicolon create mode 100644 tests/fdb/parsing/data/broken_types_no_name create mode 100644 tests/fdb/parsing/data/broken_types_no_type create mode 100644 tests/fdb/parsing/data/non_ascii_before_rule create mode 100644 tests/fdb/parsing/data/non_ascii_chars create mode 100644 tests/fdb/parsing/data/schema create mode 100644 tests/fdb/parsing/data/schema_incomplete_rule create mode 100644 tests/fdb/parsing/test_schema_parsing.cc diff --git a/src/fdb5/CMakeLists.txt b/src/fdb5/CMakeLists.txt index 9e2a16560..1119305de 100644 --- a/src/fdb5/CMakeLists.txt +++ b/src/fdb5/CMakeLists.txt @@ -23,6 +23,8 @@ list( APPEND fdb5_srcs api/FDBFactory.h api/FDBStats.cc api/FDBStats.h + api/exceptions/SchemaError.cc + api/exceptions/SchemaError.h api/LocalFDB.cc api/LocalFDB.h api/RandomFDB.cc diff --git a/src/fdb5/api/exceptions/SchemaError.cc b/src/fdb5/api/exceptions/SchemaError.cc new file mode 100644 index 000000000..765cfb185 --- /dev/null +++ b/src/fdb5/api/exceptions/SchemaError.cc @@ -0,0 +1,18 @@ +/* + * (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/api/exceptions/SchemaError.h" + +namespace fdb5 { + +SchemaError::SchemaError(const std::string& path, const std::string& what, size_t line) : + eckit::StreamParser::Error(path + ": " + what, line) {} + +} // namespace fdb5 diff --git a/src/fdb5/api/exceptions/SchemaError.h b/src/fdb5/api/exceptions/SchemaError.h new file mode 100644 index 000000000..3d4c3ccf3 --- /dev/null +++ b/src/fdb5/api/exceptions/SchemaError.h @@ -0,0 +1,26 @@ +/* + * (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. + */ + +#pragma once + +#include +#include + +#include "eckit/parser/StreamParser.h" + +namespace fdb5 { + +class SchemaError : public eckit::StreamParser::Error { +public: + + SchemaError(const std::string& path, const std::string& what, size_t line = 0); +}; + +} // namespace fdb5 diff --git a/src/fdb5/rules/Schema.cc b/src/fdb5/rules/Schema.cc index c26fa8d34..19a671bd0 100644 --- a/src/fdb5/rules/Schema.cc +++ b/src/fdb5/rules/Schema.cc @@ -9,12 +9,9 @@ */ #include -#include -#include #include #include #include -#include #include #include #include @@ -27,6 +24,7 @@ #include "eckit/utils/Tokenizer.h" #include "fdb5/LibFdb5.h" +#include "fdb5/api/exceptions/SchemaError.h" #include "fdb5/database/Key.h" #include "fdb5/database/WriteVisitor.h" #include "fdb5/rules/Predicate.h" @@ -190,23 +188,28 @@ void Schema::load(const eckit::PathName& path, const bool replace) { LOG_DEBUG_LIB(LibFdb5) << "Loading FDB rules from " << path << std::endl; - std::ifstream in(path.localPath()); - if (!in) { - auto ex = eckit::CantOpenFile(path); - ex.dumpStackTrace(); - throw ex; + // Constructing the parser opens `path`, so a missing file is reported + // (as eckit::CantOpenFile) before any existing rules are cleared. + SchemaParser parser(path); + + if (replace) { + clear(); } - load(in, replace); + parser.parse(rules_, registry_); + + check(); } void Schema::load(std::istream& s, const bool replace) { + SchemaParser parser(s); + if (replace) { clear(); } - SchemaParser(s).parse(rules_, registry_); + parser.parse(rules_, registry_); check(); } diff --git a/src/fdb5/rules/SchemaParser.cc b/src/fdb5/rules/SchemaParser.cc index ae71d71ed..38f33b027 100644 --- a/src/fdb5/rules/SchemaParser.cc +++ b/src/fdb5/rules/SchemaParser.cc @@ -14,6 +14,9 @@ /// @date April 2016 #include "fdb5/rules/SchemaParser.h" +#include "eckit/exception/Exceptions.h" +#include "eckit/parser/StreamParser.h" +#include "fdb5/api/exceptions/SchemaError.h" #include "fdb5/rules/ExcludeAll.h" #include "fdb5/rules/MatchAlways.h" #include "fdb5/rules/MatchAny.h" @@ -23,13 +26,48 @@ #include "fdb5/rules/Predicate.h" #include "fdb5/rules/Rule.h" +#include #include +#include +#include +#include #include namespace fdb5 { //---------------------------------------------------------------------------------------------------------------------- +SchemaParser::SchemaParser(const eckit::PathName& path) : + path_(std::make_tuple<>(path, std::ifstream(path.localPath()))) { + + if (!std::get<1>(*path_)) { + auto ex = eckit::CantOpenFile(path); + ex.dumpStackTrace(); + throw ex; + } + + parser_ = std::make_unique(std::get<1>(*path_), true); +} + +SchemaParser::SchemaParser(std::istream& in) : + path_(std::nullopt), parser_(std::make_unique(in, true)) {} + +bool SchemaParser::isAscii(char c) { + return static_cast(c) < 128; +} + +char SchemaParser::peek(bool spaces) { + const char peeked = parser_->peek(spaces); + + if (peeked != 0 && !isAscii(peeked)) { + std::stringstream buf; + buf << "Schema file contained non-ASCII character which are not supported." << std::endl; + throw SchemaError(getSchemaPath(), buf.str(), parser_->line() + 1); + } + + return peeked; +} + std::string SchemaParser::parseIdent(bool value, bool emptyOK) { std::string s; for (;;) { @@ -45,19 +83,20 @@ std::string SchemaParser::parseIdent(bool value, bool emptyOK) { case ']': case '?': if (s.empty() && !emptyOK) { - throw StreamParser::Error("Syntax error: found '" + std::to_string(c) + "'", line_ + 1); + throw SchemaError(getSchemaPath(), "Syntax error: found '" + std::string{c} + "'", + parser_->line() + 1); } return s; case '-': if (s.empty() && !emptyOK) { - throw StreamParser::Error("Syntax error: found '-'", line_ + 1); + throw SchemaError(getSchemaPath(), "Syntax error: found '-'", parser_->line() + 1); } if (!value) { return s; } [[fallthrough]]; default: - consume(c); + parser_->consume(c); s += c; break; } @@ -73,19 +112,19 @@ std::unique_ptr SchemaParser::parsePredicate(eckit::StringDict& types char c = peek(); if (c == ':') { - consume(c); + parser_->consume(c); ASSERT(types.find(k) == types.end()); types[k] = parseIdent(false, false); c = peek(); } if (c == '?') { - consume(c); + parser_->consume(c); return std::make_unique(k, new MatchOptional(parseIdent(true, true))); } if (c == '-') { - consume(c); + parser_->consume(c); if (types.find(k) == types.end()) { // Register ignore type types[k] = "Ignore"; @@ -94,7 +133,7 @@ std::unique_ptr SchemaParser::parsePredicate(eckit::StringDict& types } if (c != ',' && c != '[' && c != ']') { - consume("="); + parser_->consume("="); std::string val = parseIdent(true, false); exclude = val[0] == '!'; @@ -107,7 +146,7 @@ std::unique_ptr SchemaParser::parsePredicate(eckit::StringDict& types } while ((c = peek()) == '/') { - consume(c); + parser_->consume(c); values.insert(parseIdent(true, false)); } } @@ -126,16 +165,27 @@ std::unique_ptr SchemaParser::parsePredicate(eckit::StringDict& types } void SchemaParser::parseTypes(eckit::StringDict& types) { - for (;;) { - const auto name = parseIdent(false, true); - if (name.empty()) { - break; + + try { + for (;;) { + const auto name = parseIdent(false, true); + if (name.empty()) { + break; + } + parser_->consume(':'); + const auto type = parseIdent(false, false); + parser_->consume(';'); + ASSERT(types.find(name) == types.end()); + types[name] = type; } - consume(':'); - const auto type = parseIdent(false, false); - consume(';'); - ASSERT(types.find(name) == types.end()); - types[name] = type; + } + catch (eckit::StreamParser::Error& spe) { + std::stringstream buf; + buf << "SchemaParser::parseTypes: Error during parsing of types in schema, check the definitions: ': " + ";'." + << " Underlying issue: " << spe.what(); + + throw SchemaError(getSchemaPath(), buf.str(), parser_->line() + 1); } } @@ -143,13 +193,13 @@ std::unique_ptr SchemaParser::parseDatum() { Rule::Predicates predicates; eckit::StringDict types; - consume('['); + parser_->consume('['); - const std::size_t line = line_ + 1; + const std::size_t line = parser_->line() + 1; char c = peek(); if (c == ']') { - consume(c); + parser_->consume(c); return std::make_unique(line, predicates, types); } @@ -159,13 +209,13 @@ std::unique_ptr SchemaParser::parseDatum() { predicates.emplace_back(parsePredicate(types)); while ((c = peek()) == ',') { - consume(c); + parser_->consume(c); predicates.emplace_back(parsePredicate(types)); } c = peek(); if (c == ']') { - consume(c); + parser_->consume(c); return std::make_unique(line, predicates, types); } } @@ -176,13 +226,13 @@ std::unique_ptr SchemaParser::parseIndex() { eckit::StringDict types; RuleIndex::Child rule; - consume('['); + parser_->consume('['); - const std::size_t line = line_ + 1; + const std::size_t line = parser_->line() + 1; char c = peek(); if (c == ']') { - consume(c); + parser_->consume(c); return std::make_unique(line, predicates, types, std::move(rule)); } @@ -196,14 +246,14 @@ std::unique_ptr SchemaParser::parseIndex() { else { predicates.emplace_back(parsePredicate(types)); while ((c = peek()) == ',') { - consume(c); + parser_->consume(c); predicates.emplace_back(parsePredicate(types)); } } c = peek(); if (c == ']') { - consume(c); + parser_->consume(c); return std::make_unique(line, predicates, types, std::move(rule)); } } @@ -214,37 +264,47 @@ std::unique_ptr SchemaParser::parseDatabase() { eckit::StringDict types; RuleDatabase::Children rules; - consume('['); + try { + parser_->consume('['); - const std::size_t line = line_ + 1; + const std::size_t line = parser_->line() + 1; - char c = peek(); - if (c == ']') { - consume(c); - return std::make_unique(line, predicates, types, rules); - } + char c = peek(); + if (c == ']') { + parser_->consume(c); + return std::make_unique(line, predicates, types, rules); + } - for (;;) { + for (;;) { - c = peek(); + c = peek(); - if (c == '[') { - rules.emplace_back(parseIndex()); - } - else { - predicates.emplace_back(parsePredicate(types)); - while ((c = peek()) == ',') { - consume(c); + if (c == '[') { + rules.emplace_back(parseIndex()); + } + else { predicates.emplace_back(parsePredicate(types)); + while ((c = peek()) == ',') { + parser_->consume(c); + predicates.emplace_back(parsePredicate(types)); + } } - } - c = peek(); - if (c == ']') { - consume(c); - return std::make_unique(line, predicates, types, rules); + c = peek(); + if (c == ']') { + parser_->consume(c); + return std::make_unique(line, predicates, types, rules); + } } } + catch (eckit::StreamParser::Error& spe) { + std::stringstream buf; + buf << "SchemaParser::parseDatabase: Error during parsing of rules in schema, check for closing brackets and " + "definitions." + << " Underlying issue: " << spe.what(); + + throw SchemaError(getSchemaPath(), buf.str(), parser_->line() + 1); + } } void SchemaParser::parse(RuleList& result, TypesRegistry& registry) { @@ -261,10 +321,20 @@ void SchemaParser::parse(RuleList& result, TypesRegistry& registry) { } if (c) { - throw StreamParser::Error(std::string("Error parsing rules: remaining char: ") + c); + throw SchemaError(getSchemaPath(), + std::string("SchemaParser::parse: Error parsing rules: remaining char: ") + c, + parser_->line()); + } + + + if (result.size() == 0) { + std::stringstream buf; + buf << "SchemaParser::parse: Empty rule list. Didn't find any rule in the provided schema file." << std::endl; + throw SchemaError(getSchemaPath(), buf.str(), parser_->line()); } } //---------------------------------------------------------------------------------------------------------------------- + } // namespace fdb5 diff --git a/src/fdb5/rules/SchemaParser.h b/src/fdb5/rules/SchemaParser.h index 034f37dae..edfb3411a 100644 --- a/src/fdb5/rules/SchemaParser.h +++ b/src/fdb5/rules/SchemaParser.h @@ -14,10 +14,13 @@ #ifndef fdb5_SchemaParser_h #define fdb5_SchemaParser_h +#include +#include #include #include #include +#include "eckit/filesystem/PathName.h" #include "eckit/parser/StreamParser.h" #include "eckit/types/Types.h" @@ -27,16 +30,22 @@ namespace fdb5 { //---------------------------------------------------------------------------------------------------------------------- -class SchemaParser : public eckit::StreamParser { +class SchemaParser { public: // methods - SchemaParser(std::istream& in) : StreamParser(in, true) {} + explicit SchemaParser(const eckit::PathName& path); + + explicit SchemaParser(std::istream& in); void parse(RuleList& result, TypesRegistry& registry); private: // methods + bool isAscii(char c); + + char peek(bool spaces = false); + std::string parseIdent(bool value, bool emptyOK); std::unique_ptr parseDatum(); @@ -48,6 +57,15 @@ class SchemaParser : public eckit::StreamParser { std::unique_ptr parsePredicate(eckit::StringDict& types); void parseTypes(eckit::StringDict& types); + +private: // members + + std::string getSchemaPath() const noexcept { + return path_.has_value() ? std::get<0>(*path_).localPath() : "Created from std::istream"; + } + + std::optional> path_; + std::unique_ptr parser_; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/tests/fdb/CMakeLists.txt b/tests/fdb/CMakeLists.txt index e6cecbba1..04055e7ef 100644 --- a/tests/fdb/CMakeLists.txt +++ b/tests/fdb/CMakeLists.txt @@ -72,6 +72,7 @@ add_subdirectory( type ) add_subdirectory( daos ) add_subdirectory( fam ) add_subdirectory( concurrent ) +add_subdirectory( parsing ) if (HAVE_FDB_BUILD_TOOLS) add_subdirectory( timespan ) diff --git a/tests/fdb/parsing/CMakeLists.txt b/tests/fdb/parsing/CMakeLists.txt new file mode 100644 index 000000000..67b8bf13f --- /dev/null +++ b/tests/fdb/parsing/CMakeLists.txt @@ -0,0 +1,13 @@ +set( _test_environment ${test_environment}) + +list( APPEND _test_environment + FDB_HOME=${PROJECT_BINARY_DIR} ) + +file(COPY data DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) + +ecbuild_add_test( TARGET fdb_test_schema_parsing + SOURCES + test_schema_parsing.cc + LIBS + fdb5 + ENVIRONMENT "${_test_environment}") diff --git a/tests/fdb/parsing/data/broken_schema_comments_no_rule b/tests/fdb/parsing/data/broken_schema_comments_no_rule new file mode 100644 index 000000000..7e41a7c18 --- /dev/null +++ b/tests/fdb/parsing/data/broken_schema_comments_no_rule @@ -0,0 +1,88 @@ +# * 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; +offsetdate: Date; +latitude: Double; +longitude: Double; +levelist: Double; +grid: Grid; +expver: Expver; + +time: Time; +offsettime: Time; +fcmonth: Integer; + +number: Integer; +frequency: Integer; +direction: Integer; +channel: Integer; +chem: Integer; +coeffindex: Integer; + +instrument: Integer; +ident: Integer; + +diagnostic: Integer; +iteration: Integer; +system: Integer; +method: Integer; + diff --git a/tests/fdb/parsing/data/broken_schema_no_rule b/tests/fdb/parsing/data/broken_schema_no_rule new file mode 100644 index 000000000..ee9e605b3 --- /dev/null +++ b/tests/fdb/parsing/data/broken_schema_no_rule @@ -0,0 +1,6 @@ +param: Param; +step: Step; +date: Date; +levelist: Double; +expver: Expver; +time: Time; diff --git a/tests/fdb/parsing/data/broken_types_missing_semicolon b/tests/fdb/parsing/data/broken_types_missing_semicolon new file mode 100644 index 000000000..067c64a02 --- /dev/null +++ b/tests/fdb/parsing/data/broken_types_missing_semicolon @@ -0,0 +1,6 @@ +param: Param; +step: Step; +date: Date; +levelist: Double; +expver: Expver +time: Time; diff --git a/tests/fdb/parsing/data/broken_types_no_name b/tests/fdb/parsing/data/broken_types_no_name new file mode 100644 index 000000000..d6c07bdca --- /dev/null +++ b/tests/fdb/parsing/data/broken_types_no_name @@ -0,0 +1,6 @@ +param: Param; +: Step; +date: Date; +levelist: Double; +expver: Expver +time: Time; diff --git a/tests/fdb/parsing/data/broken_types_no_type b/tests/fdb/parsing/data/broken_types_no_type new file mode 100644 index 000000000..836d1972a --- /dev/null +++ b/tests/fdb/parsing/data/broken_types_no_type @@ -0,0 +1,6 @@ +param: Param; +step: ; +date: Date; +levelist: Double; +expver: Expver +time: Time; diff --git a/tests/fdb/parsing/data/non_ascii_before_rule b/tests/fdb/parsing/data/non_ascii_before_rule new file mode 100644 index 000000000..dc11190c7 --- /dev/null +++ b/tests/fdb/parsing/data/non_ascii_before_rule @@ -0,0 +1,4 @@ +param: Param; + [class=od, expver + [type=an + [step, param]]] diff --git a/tests/fdb/parsing/data/non_ascii_chars b/tests/fdb/parsing/data/non_ascii_chars new file mode 100644 index 000000000..6ccef48f3 --- /dev/null +++ b/tests/fdb/parsing/data/non_ascii_chars @@ -0,0 +1,11 @@ +param: Param; +step: Step; +date: Date; +levelist: Double; +expver: Expver; +time: Time; + +[ class=ce, expver, stream=efas/wfas, date, time, model, domain +   [ type, levtype, origin, anoffset? +     [ step, number?, levelist?, param ]] +] diff --git a/tests/fdb/parsing/data/schema b/tests/fdb/parsing/data/schema new file mode 100644 index 000000000..d2dcc812b --- /dev/null +++ b/tests/fdb/parsing/data/schema @@ -0,0 +1,11 @@ +param: Param; +step: Step; +date: Date; +levelist: Double; +expver: Expver; +time: Time; + +[ class, expver=xxxx, stream=oper, date, time, domain? + [ type, levtype + [ step, levelist?, param ]] +] \ No newline at end of file diff --git a/tests/fdb/parsing/data/schema_incomplete_rule b/tests/fdb/parsing/data/schema_incomplete_rule new file mode 100644 index 000000000..087e188b3 --- /dev/null +++ b/tests/fdb/parsing/data/schema_incomplete_rule @@ -0,0 +1,10 @@ +param: Param; +step: Step; +date: Date; +levelist: Double; +expver: Expver; +time: Time; + +[ class, expver=xxxx, stream=oper, date, time, domain? + [ type, levtype + [ step, levelist?, param ]] diff --git a/tests/fdb/parsing/test_schema_parsing.cc b/tests/fdb/parsing/test_schema_parsing.cc new file mode 100644 index 000000000..2b564dbc6 --- /dev/null +++ b/tests/fdb/parsing/test_schema_parsing.cc @@ -0,0 +1,164 @@ + +#include + +#include "eckit/testing/Test.h" +#include "fdb5/api/exceptions/SchemaError.h" +#include "fdb5/rules/Predicate.h" +#include "fdb5/rules/Rule.h" +#include "fdb5/rules/Schema.h" +#include "fdb5/rules/SchemaParser.h" +#include "fdb5/types/TypesRegistry.h" + +namespace { + +// Builds the schema at `path`, expects it to throw ExceptionT, and checks +// that the exception message contains `expectedSubstr`. Replaces the old +// pattern of constructing the schema twice per case (once to check the +// type, once more to check the message). +template +void expectSchemaError(const std::string& path, const std::string& expectedSubstr) { + bool thrown = false; + + try { + fdb5::Schema schema(path); + } + catch (ExceptionT& ex) { + thrown = true; + std::cout << ex.what() << std::endl; + EXPECT(std::string(ex.what()).find(expectedSubstr) != std::string::npos); + } + + EXPECT(thrown); +} + +// Same as expectSchemaError, but drives fdb5::SchemaParser directly off a +// std::istream (rather than fdb5::Schema off a file path), the way callers +// with an in-memory schema do. Used to check that the stream-based +// SchemaParser ctor reports the same errors as the file-based one, just +// without a real path in the message (SchemaParser has no path to report, +// so it falls back to a fixed "Created from std::istream" marker). +template +void expectSchemaErrorFromStream(const std::string& path, const std::string& expectedSubstr) { + std::ifstream in(path); + EXPECT(in); + + bool thrown = false; + fdb5::RuleList rules; + fdb5::TypesRegistry registry; + + try { + fdb5::SchemaParser parser(in); + parser.parse(rules, registry); + } + catch (ExceptionT& ex) { + thrown = true; + std::cout << ex.what() << std::endl; + EXPECT(std::string(ex.what()).find(expectedSubstr) != std::string::npos); + EXPECT(std::string(ex.what()).find("Created from std::istream") != std::string::npos); + } + + EXPECT(thrown); +} + +CASE("Broken schema - Non-existing schema file") { + // No stream counterpart: a std::istream is already open when handed to + // SchemaParser, so there is no "file not found" case on that path. + expectSchemaError("./data/non-existing", "Cannot open"); +} + +CASE("Broken schema - No Rule") { + expectSchemaError("./data/broken_schema_no_rule", "SchemaParser::parse: Empty rule list"); +} + +CASE("Broken schema (stream) - No Rule") { + expectSchemaErrorFromStream("./data/broken_schema_no_rule", + "SchemaParser::parse: Empty rule list"); +} + +CASE("Broken schema - No Rule but comments") { + expectSchemaError("./data/broken_schema_comments_no_rule", + "SchemaParser::parse: Empty rule list"); +} + +CASE("Broken schema (stream) - No Rule but comments") { + expectSchemaErrorFromStream("./data/broken_schema_comments_no_rule", + "SchemaParser::parse: Empty rule list"); +} + +CASE("Broken schema - Missing semicolon types") { + expectSchemaError("./data/broken_types_missing_semicolon", "SchemaParser::parseTypes"); +} + +CASE("Broken schema (stream) - Missing semicolon types") { + expectSchemaErrorFromStream("./data/broken_types_missing_semicolon", "SchemaParser::parseTypes"); +} + +CASE("Broken schema - No type name") { + expectSchemaError("./data/broken_types_no_name", "Error parsing rules"); +} + +CASE("Broken schema (stream) - No type name") { + expectSchemaErrorFromStream("./data/broken_types_no_name", "Error parsing rules"); +} + +CASE("Broken schema - No type type") { + expectSchemaError("./data/broken_types_no_type", "SchemaParser::parseTypes"); +} + +CASE("Broken schema (stream) - No type type") { + expectSchemaErrorFromStream("./data/broken_types_no_type", "SchemaParser::parseTypes"); +} + +CASE("Broken schema - Missing closing bracket") { + expectSchemaError("./data/schema_incomplete_rule", "SchemaParser::parseDatabase"); +} + +CASE("Broken schema (stream) - Missing closing bracket") { + expectSchemaErrorFromStream("./data/schema_incomplete_rule", "SchemaParser::parseDatabase"); +} + +CASE("Broken schema - Non-ASCII chars inside rule") { + expectSchemaError("./data/non_ascii_chars", "non-ASCII"); +} + +CASE("Broken schema (stream) - Non-ASCII chars inside rule") { + expectSchemaErrorFromStream("./data/non_ascii_chars", "non-ASCII"); +} + +CASE("Broken schema - Non-ASCII chars before rule") { + expectSchemaError("./data/non_ascii_before_rule", "non-ASCII"); +} + +CASE("Broken schema (stream) - Non-ASCII chars before rule") { + expectSchemaErrorFromStream("./data/non_ascii_before_rule", "non-ASCII"); +} + + +CASE("Correct schema - Production Schema") { + + EXPECT_NO_THROW(fdb5::Schema("./data/schema")); +} + +CASE("Correct schema (stream) - Production Schema") { + + std::ifstream in("./data/schema"); + EXPECT(in); + + fdb5::RuleList rules; + fdb5::TypesRegistry registry; + + EXPECT_NO_THROW(fdb5::SchemaParser(in).parse(rules, registry)); + EXPECT(!rules.empty()); +} + + +//---------------------------------------------------------------------------------------------------------------------- + +} // anonymous namespace + +int main(int argc, char** argv) { + + eckit::Log::info() << ::getenv("FDB_HOME") << std::endl; + + return ::eckit::testing::run_tests(argc, argv); +} diff --git a/tests/fdb/type/test_toKey.cc b/tests/fdb/type/test_toKey.cc index 54c23da19..60a911700 100644 --- a/tests/fdb/type/test_toKey.cc +++ b/tests/fdb/type/test_toKey.cc @@ -9,8 +9,10 @@ */ #include +#include #include +#include "eckit/filesystem/PathName.h" #include "eckit/testing/Test.h" #include "fdb5/config/Config.h" @@ -360,12 +362,17 @@ CASE("YearMonth - string ctor - expansion") { fdb5::Config yearMonthConfig; { - std::istringstream schemaStream( - "[ class, expver, stream=wamo, domain\n" - " [ type, levtype\n" - " [ date: YearMonth, time, step?, param ]]\n" - "]\n"); - yearMonthConfig.overrideSchema("test_toKey_YearMonth_schema", std::make_unique(schemaStream)); + // Schema construction only reads from a file, so the schema is written out here + // rather than kept as an in-memory stream. + const eckit::PathName schemaPath = eckit::PathName::unique("test_toKey_YearMonth_schema"); + std::ofstream schemaFile(schemaPath.asString()); + schemaFile << "[ class, expver, stream=wamo, domain\n" + " [ type, levtype\n" + " [ date: YearMonth, time, step?, param ]]\n" + "]\n"; + schemaFile.close(); + + yearMonthConfig.overrideSchema(schemaPath, std::make_unique(schemaPath)); }