diff --git a/src/ntndarray.cpp b/src/ntndarray.cpp index f8bfc7b..abb55bc 100644 --- a/src/ntndarray.cpp +++ b/src/ntndarray.cpp @@ -175,10 +175,10 @@ bool NTNDArray::is_a(PVStructurePtr const & pvStructure) return is_a(pvStructure->getStructure()); } -static Validator::Definition* definition; -static epicsThreadOnceId definition_once = EPICS_THREAD_ONCE_INIT; +static Validator* validator; +static epicsThreadOnceId validator_once = EPICS_THREAD_ONCE_INIT; -static void definition_init(void *) +static void validator_init(void *) { StructureConstPtr structure( NTNDArray::createBuilder()-> @@ -188,34 +188,37 @@ static void definition_init(void *) addTimeStamp()-> createStructure()); - definition = new Validator::Definition; - definition->structure = std::static_pointer_cast(structure); + std::set optional; // TODO: these should be all getFieldT - definition->optional.insert(structure->getField("descriptor").get()); - definition->optional.insert(structure->getField("alarm").get()); - definition->optional.insert(structure->getField("timeStamp").get()); - definition->optional.insert(structure->getField("display").get()); + optional.insert(structure->getField("descriptor").get()); + optional.insert(structure->getField("alarm").get()); + optional.insert(structure->getField("timeStamp").get()); + optional.insert(structure->getField("display").get()); - // TODO: the following should be a helper in NTNDArrayAttribute + // TODO: the following should come from a helper in NTNDArrayAttribute StructureConstPtr attribute( std::static_pointer_cast( structure->getField("attribute") )->getStructure() ); - definition->optional.insert(attribute->getField("tags").get()); - definition->optional.insert(attribute->getField("alarm").get()); - definition->optional.insert(attribute->getField("timeStamp").get()); + optional.insert(attribute->getField("tags").get()); + optional.insert(attribute->getField("alarm").get()); + optional.insert(attribute->getField("timeStamp").get()); + + validator = new Validator(structure); } +// TODO: I want to deprecate this and replace it with one that accepts +// "Field const &" so nullptr shouldn't have to be checked for here bool NTNDArray::isCompatible(StructureConstPtr const &structure) { if(!structure.get()) return false; - epicsThreadOnce(&definition_once, &definition_init, 0); + epicsThreadOnce(&validator_once, &validator_init, 0); - return Validator::isCompatible(*definition, *structure); + return validator->isCompatible(*structure); } diff --git a/src/validator.cpp b/src/validator.cpp index 84efa80..2a068f4 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -28,33 +28,36 @@ using epics::pvData::FieldConstPtrArray; using epics::pvData::BitSet; using std::string; +using std::set; -namespace { - -struct Helper { - Validator::Definition const & definition; +struct Validator::Helper { + Validator const & validator; std::deque path; Validator::Result result; - Helper(Validator::Definition const & definition); + Helper(Validator const & validator); bool isOptional(Field const & field) const; string getCurrentPath(void) const; + void appendError(Validator::ErrorType type); void appendError(Validator::ErrorType type, string const & field_name); + void appendError(Validator::ErrorType type, string const & ref_field_name, + string const & field_name); + bool validate(Union const & ref, Union const & un); bool validate(Structure const & ref, Structure const & struc); bool validate(Field const & reference, Field const & field); bool validate(Field const & field); }; -Helper::Helper(Validator::Definition const & definition) : definition(definition) {} +Validator::Helper::Helper(Validator const & validator) : validator(validator) {} -bool Helper::isOptional(Field const & field) const +bool Validator::Helper::isOptional(Field const & field) const { - return definition.optional.find(&field) != definition.optional.end(); + return validator.optional.find(&field) != validator.optional.end(); } -string Helper::getCurrentPath(void) const +string Validator::Helper::getCurrentPath(void) const { std::ostringstream os; @@ -69,21 +72,34 @@ string Helper::getCurrentPath(void) const return os.str(); } -void Helper::appendError(Validator::ErrorType type) +void Validator::Helper::appendError(Validator::ErrorType type) { result.errors.push_back(Validator::Error(getCurrentPath(), type)); } -void Helper::appendError(Validator::ErrorType type, string const & field_name) +void Validator::Helper::appendError(Validator::ErrorType type, string const & field_name) { path.push_back(field_name); appendError(type); path.pop_back(); } -bool Helper::validate(Union const & ref, Union const & un) +void Validator::Helper::appendError(Validator::ErrorType type, string const & ref_field_name, + string const & field_name) +{ + path.push_back(ref_field_name); + string ref_field_path = getCurrentPath(); + path.pop_back(); + + path.push_back(field_name); + string field_path(getCurrentPath()); + path.pop_back(); + + result.errors.push_back(Validator::Error(ref_field_path, field_path, type)); +} + +bool Validator::Helper::validate(Union const & ref, Union const & un) { - // TODO: Validator Errors for unions could be more granular if (&un == &ref) return true; @@ -95,35 +111,42 @@ bool Helper::validate(Union const & ref, Union const & un) if (ref.isVariant()) return true; - size_t numfields = ref.getNumberFields(); - - if (un.getNumberFields() != numfields) { - appendError(Validator::ErrorType::INCORRECT_TYPE); - return false; - } - StringArray const & rnames = ref.getFieldNames(); StringArray const & unames = un.getFieldNames(); - if (!std::equal(rnames.cbegin(), rnames.cend(), unames.cend())) { - appendError(Validator::ErrorType::INCORRECT_TYPE); - return false; - } - FieldConstPtrArray const & rfields = ref.getFields(); FieldConstPtrArray const & ufields = un.getFields(); - bool fields_ok = true; - for (size_t i = 0; i < numfields; ++i) { - path.push_back(rnames[i]); - fields_ok = fields_ok && validate(*rfields[i], *ufields[i]); + size_t numRefFields = ref.getNumberFields(); + size_t numUnFields = un.getNumberFields(); + + // Extra fields are OK + bool ok = numRefFields <= numUnFields; + + size_t i = 0; + size_t N = std::min(numRefFields, numUnFields); + for (; i < N; ++i) { + string const & rname(rnames[i]); + string const & uname(unames[i]); + + if (rname != uname) { + appendError(Validator::ErrorType::MISMATCHED_NAME, rname, uname); + ok = false; + continue; + } + + path.push_back(rname); + ok = ok && validate(*rfields[i], *ufields[i]); path.pop_back(); } - return fields_ok; + for (; i < numRefFields; ++i) + appendError(Validator::ErrorType::MISSING_FIELD, rnames[i]); + + return ok; } -bool Helper::validate(Structure const &ref, Structure const &struc) +bool Validator::Helper::validate(Structure const &ref, Structure const &struc) { if (&struc == &ref) return true; @@ -131,9 +154,9 @@ bool Helper::validate(Structure const &ref, Structure const &struc) StringArray const &rnames = ref.getFieldNames(); StringArray const &snames = struc.getFieldNames(); - // TODO: This is naive O(N^2), replace with better algorithm - // TODO: This assumes getField doesn't fail (there's no Field::getFieldT) - bool fields_ok = true; + // TODO: This is naive O(N^2), replace with better algorithm (cache?) + // TODO: This assumes getField doesn't fail (there's no Field::getFieldT (yet?)) + bool ok = true; StringArray::const_iterator ri; for (ri = rnames.cbegin(); ri != rnames.cend(); ++ri) { @@ -146,26 +169,24 @@ bool Helper::validate(Structure const &ref, Structure const &struc) continue; appendError(Validator::ErrorType::MISSING_FIELD, *ri); - fields_ok = false; + ok = false; } else { path.push_back(*ri); - if (!validate(*rfield, *struc.getField(*ri))) { appendError(Validator::ErrorType::INCORRECT_TYPE, *ri); - fields_ok = false; + ok = false; } - path.pop_back(); } } - return fields_ok; + return ok; } -bool Helper::validate(Field const &reference, Field const &field) +bool Validator::Helper::validate(Field const &reference, Field const &field) { Type referenceType = reference.getType(); if (referenceType != field.getType()) @@ -203,45 +224,55 @@ bool Helper::validate(Field const &reference, Field const &field) } } -bool Helper::validate(Field const &field) +bool Validator::Helper::validate(Field const &field) { - return validate(*definition.structure, field); + return validate(*validator.reference, field); } + +std::ostream& Validator::Error::dump(std::ostream& o) const { + switch (type) { + case ErrorType::MISSING_FIELD: + return o << "Missing field '" << ref_path << "'"; + case ErrorType::INCORRECT_TYPE: + return o << "Field '" << ref_path << "' has incorrect type"; + case ErrorType::MISMATCHED_NAME: + return o << "Expected field '" << ref_path + << "' in Union, got '" << path << "'"; + default: + return o << "Unknown error " << type << " in field '" + << ref_path << "'"; + } +} + +Validator::Validator(FieldConstPtr const & reference) +: reference(reference) { + if (!reference) + throw std::logic_error("reference structure must not be NULL"); +} + +Validator::Validator(FieldConstPtr const & reference, + set const & optional) +: reference(reference), optional(optional) { + if (!reference) + throw std::logic_error("reference structure must not be NULL"); } -Validator::Result Validator::validate(Validator::Definition const & definition, - Field const & field) +Validator::Result Validator::validate(Field const & field) const { - Helper helper(definition); + Helper helper(*this); helper.validate(field); return helper.result; } -bool Validator::isCompatible(Validator::Definition const & definition, - Field const & field) +bool Validator::isCompatible(Field const & field) const { - return Helper(definition).validate(field); -} - -bool Validator::isCompatible(Validator::Definition const & definition, - StructureConstPtr const & structure) -{ - FieldConstPtr field(std::static_pointer_cast(structure)); - return Validator::isCompatible(definition, *field); + return Helper(*this).validate(field); } std::ostream& operator<<(std::ostream& o, const Validator::Error& error) { - switch (error.type) { - case Validator::ErrorType::MISSING_FIELD: - return o << "Missing field '" << error.path << "'"; - case Validator::ErrorType::INCORRECT_TYPE: - return o << "Field '" << error.path << "' has incorrect type"; - default: - return o << "Unknown error " << error.type << " in field '" - << error.path << "'"; - } + return error.dump(o); } }} diff --git a/src/validator.h b/src/validator.h index 6cceaf6..d0148c2 100644 --- a/src/validator.h +++ b/src/validator.h @@ -1,15 +1,14 @@ -/* ntvalidator.h */ +/* validator.h */ /* * Copyright information and license terms for this software can be * found in the file LICENSE that is included with the distribution */ -#ifndef NTVALIDATOR_H -#define NTVALIDATOR_H +#ifndef VALIDATOR_H +#define VALIDATOR_H #include #include -#include #include @@ -21,48 +20,61 @@ namespace epics { namespace nt { * @author bsm */ -class epicsShareClass Validator { +class Validator { public: - struct Definition { - epics::pvData::FieldConstPtr structure; - std::set optional; - }; + enum ErrorType { MISSING_FIELD, INCORRECT_TYPE, MISMATCHED_NAME }; - enum ErrorType { MISSING_FIELD, INCORRECT_TYPE }; + struct Helper; struct Error { + std::string ref_path; std::string path; ErrorType type; - Error(std::string const & path, ErrorType type) : path(path), type(type) {}; + Error(std::string const & ref_path, ErrorType type) + : ref_path(ref_path), type(type) {} + + Error(std::string const & ref_path, std::string const & path, ErrorType type) + : ref_path(ref_path), path(path), type(type) {} + + bool operator==(const Error& other) const { + return type == other.type && + ref_path == other.ref_path && + path == other.path; + } + + std::ostream& dump(std::ostream&) const; }; struct Result { std::vector errors; bool valid(void) const { return errors.empty(); } + std::ostream& dump(std::ostream&) const; }; + Validator(epics::pvData::FieldConstPtr const & reference); + + Validator(epics::pvData::FieldConstPtr const & reference, + std::set const & optional); + + Result validate(epics::pvData::Field const & field) const; + /** * Checks whether a Field is compatible with a NT definition. * @param definition the definition of the reference structure * @param field the Field being tested for compatibility * @return true if field is compatible, false otherwise */ - static Result validate(Definition const & definition, - epics::pvData::Field const & field); - - static bool isCompatible(Definition const & definition, - epics::pvData::Field const & field); - - static bool isCompatible(Definition const & definition, - epics::pvData::StructureConstPtr const & structure); + bool isCompatible(epics::pvData::Field const & field) const; private: - Validator(); + friend struct Helper; + epics::pvData::FieldConstPtr const reference; + std::set optional; }; epicsShareExtern std::ostream& operator<<(std::ostream& o, const Validator::Error& error); }} -#endif /* NTVALIDATOR_H */ +#endif /* VALIDATOR_H */ diff --git a/test/validatorTest.cpp b/test/validatorTest.cpp index ce53a64..3d31f67 100644 --- a/test/validatorTest.cpp +++ b/test/validatorTest.cpp @@ -8,28 +8,158 @@ #include "../src/validator.h" #include +#include using namespace epics::nt; using epics::pvData::StructureConstPtr; using epics::pvData::Field; +using epics::pvData::ScalarType; +using epics::pvData::FieldConstPtr; +using epics::pvData::FieldBuilder; +using epics::pvData::FieldBuilderPtr; + +static epics::pvData::FieldCreatePtr FC; + +void test_result() +{ + testDiag("test_result"); + + Validator::Result result; + testOk(result.valid(), "Result with no errors means valid"); + + result.errors.push_back(Validator::Error("a.b.c", Validator::ErrorType::MISSING_FIELD)); + testOk(!result.valid(), "Result with one error means invalid"); +} + +void test_scalar() +{ + testDiag("test_scalar"); + + { + FieldConstPtr def(FC->createScalar(ScalarType::pvBoolean)); + FieldConstPtr field(FC->createScalar(ScalarType::pvBoolean)); + testOk(Validator(def).isCompatible(*field), "isCompatible(Scalar, Scalar)"); + } + + { + FieldConstPtr def(FC->createScalar(ScalarType::pvBoolean)); + FieldConstPtr field(FC->createScalar(ScalarType::pvInt)); + testOk(Validator(def).isCompatible(*field), "isCompatible(Scalar, Scalar)"); + } + + { + FieldConstPtr def(FC->createScalar(ScalarType::pvString)); + FieldConstPtr field(FC->createScalar(ScalarType::pvFloat)); + testOk(Validator(def).isCompatible(*field), "isCompatible(Scalar, Scalar)"); + } + + { + FieldConstPtr def(FC->createScalarArray(ScalarType::pvString)); + FieldConstPtr field(FC->createScalarArray(ScalarType::pvFloat)); + testOk(Validator(def).isCompatible(*field), "isCompatible(ScalarArray, ScalarArray)"); + } + + { + FieldConstPtr def(FC->createScalarArray(ScalarType::pvByte)); + FieldConstPtr field(FC->createScalar(ScalarType::pvByte)); + testOk(!Validator(def).isCompatible(*field), "!isCompatible(ScalarArray, Scalar)"); + } + + { + FieldConstPtr def(FC->createScalar(ScalarType::pvByte)); + FieldConstPtr field(FC->createScalarArray(ScalarType::pvByte)); + testOk(!Validator(def).isCompatible(*field), "!isCompatible(Scalar, ScalarArray)"); + } +} + +void test_union() +{ + testDiag("test_union"); + FieldBuilderPtr FB(FieldBuilder::begin()); + + FieldConstPtr unionVar(FC->createVariantUnion()); + FieldConstPtr unionABC(FB-> + add("A", ScalarType::pvInt)-> + add("B", ScalarType::pvInt)-> + add("C", ScalarType::pvInt)-> + createUnion()); + + FieldConstPtr unionABC2(FB-> + add("A", ScalarType::pvFloat)-> + add("B", ScalarType::pvDouble)-> + add("C", ScalarType::pvString)-> + createUnion()); + + FieldConstPtr unionBAC(FB-> + add("B", ScalarType::pvInt)-> + add("A", ScalarType::pvInt)-> + add("C", ScalarType::pvInt)-> + createUnion()); + + FieldConstPtr unionAB(FB-> + add("A", ScalarType::pvInt)-> + add("B", ScalarType::pvInt)-> + createUnion()); + + FieldConstPtr unionNested1(FB-> + add("A", unionABC)-> + add("B", unionABC)-> + createUnion()); + + FieldConstPtr unionNested2(FB-> + add("A", unionABC)-> + add("B", unionBAC)-> + createUnion()); + + testOk(Validator(unionVar).isCompatible(*unionVar), + "isCompatible(VarUnion, VarUnion)"); + testOk(!Validator(unionVar).isCompatible(*unionABC), + "!isCompatible(VarUnion, Union{A:int, B:int, C:int})"); + testOk(!Validator(unionABC).isCompatible(*unionVar), + "!isCompatible(Union{A:int, B:int, C:int}, VarUnion)"); + testOk(Validator(unionABC).isCompatible(*unionABC), + "isCompatible(Union{A:int, B:int, C:int}, Union{A:int, B:int, C:int})"); + + testOk(Validator(unionABC).isCompatible(*unionABC2), + "isCompatible(Union{A:int, B:int, C:int}, Union{A:float, B:double, C:string})"); + testOk(Validator(unionABC2).isCompatible(*unionABC), + "isCompatible(Union{A:float, B:double, C:string}, Union{A:int, B:int, C:int})"); + + testOk(!Validator(unionABC).isCompatible(*unionBAC), + "!isCompatible(Union{A:int, B:int, C:int}, Union{B:int, A:int, C:int})"); + + testOk(Validator(unionAB).isCompatible(*unionABC), "Extra Union field"); + testOk(!Validator(unionABC).isCompatible(*unionAB), "Missing Union field"); + + { + Validator::Result result = Validator(unionABC).validate(*unionAB); + testOk(result.errors.size() == 1 && + result.errors[0] == Validator::Error("C", Validator::ErrorType::MISSING_FIELD), + "Missing Union field Error"); + } + + testOk(!Validator(unionNested1).isCompatible(*unionNested2), "Nested Unions"); +} void test_isCompatible() { testDiag("test_isCompatible"); + std::set opt; StructureConstPtr ref(NTNDArray::createBuilder()->addAlarm()->createStructure()); - - Validator::Definition def; - def.structure = std::static_pointer_cast(ref); - def.optional.insert(ref->getField("alarm").get()); + opt.insert(ref->getField("alarm").get()); StructureConstPtr struc(NTNDArray::createBuilder()->createStructure()); - testOk1(Validator::isCompatible(def, struc)); + testOk1(Validator(ref, opt).isCompatible(*struc)); } -MAIN(testNTValidator) { - testPlan(1); +MAIN(testValidator) { + testPlan(0); + FC = epics::pvData::getFieldCreate(); + test_result(); + test_scalar(); + test_union(); test_isCompatible(); return testDone(); }