From f39adf2274738f4985a97e1551bc97fae56f2777 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 18 Nov 2009 10:12:32 +0000 Subject: [PATCH] added datadefinition checking to the DataDefinition class, throw DataDefinitionError when that fails first argument of group_sendmsg() is now const parkinglot now reads a parkinglot.spec file and tries to send it to the config manager (which currently fails somewhere in the msgq), file is read from cwd for now (i.e. the bin/bind10/bind10 dir) git-svn-id: svn://bind10.isc.org/svn/bind10/branches/jelte-datadef@296 e5f2f494-b856-4b98-b285-d166d9295462 --- src/bin/parkinglot/ccsession.cc | 48 ++++++++- src/bin/parkinglot/ccsession.h | 6 ++ src/lib/bind-cfgd/python/bind-cfgd.py | 1 - src/lib/cc/cpp/Makefile.am | 2 +- src/lib/cc/cpp/data_def.cc | 145 +++++++++++++++++++++++++- src/lib/cc/cpp/data_def.h | 13 ++- src/lib/cc/cpp/parkinglot.spec | 10 +- src/lib/cc/cpp/session.cc | 3 +- src/lib/cc/cpp/session.h | 2 +- src/lib/cc/cpp/test.cc | 3 + 10 files changed, 217 insertions(+), 16 deletions(-) diff --git a/src/bin/parkinglot/ccsession.cc b/src/bin/parkinglot/ccsession.cc index 29510ae92f..bf605ccdf5 100644 --- a/src/bin/parkinglot/ccsession.cc +++ b/src/bin/parkinglot/ccsession.cc @@ -14,17 +14,26 @@ // $Id$ +// +// todo: generalize this and make it into a specific API for all modules +// to use (i.e. connect to cc, send config and commands, get config, +// react on config change announcements) +// + + #include #include #include #include #include +#include #include #include #include +#include #include #include "common.h" @@ -34,16 +43,53 @@ using namespace std; using ISC::Data::Element; using ISC::Data::ElementPtr; +using ISC::Data::DataDefinition; +using ISC::Data::ParseError; +using ISC::Data::DataDefinitionError; + +void +CommandSession::read_data_definition(const std::string& filename) { + std::ifstream file; + + // this file should be declared in a @something@ directive + file.open("parkinglot.spec"); + if (!file) { + cout << "error opening parkinglot.spec" << endl; + exit(1); + } + + try { + data_definition_ = DataDefinition(file, true); + cout << "Definition: " << endl; + cout << data_definition_.getDefinition() << endl; + } catch (ParseError pe) { + cout << "Error parsing definition file: " << pe.what() << endl; + exit(1); + } catch (DataDefinitionError dde) { + cout << "Error reading definition file: " << dde.what() << endl; + exit(1); + } + file.close(); +} CommandSession::CommandSession() : session_(ISC::CC::Session()) { try { + // todo: workaround, let boss wait until msgq is started + // and remove sleep here + sleep(1); session_.establish(); session_.subscribe("ParkingLot", "*", "meonly"); session_.subscribe("Boss", "*", "meonly"); session_.subscribe("ConfigManager", "*", "meonly"); session_.subscribe("statistics", "*", "meonly"); + read_data_definition("parkinglot.spec"); + sleep(1); + ElementPtr cmd = Element::create_from_string("{ \"config_manager\": 1}"); + // why does the msgq seem to kill this msg? + session_.group_sendmsg(data_definition_.getDefinition(), "ConfigManager"); + cout << "def sent" << endl; } catch (...) { throw std::runtime_error("SessionManager: failed to open sessions"); } @@ -88,12 +134,12 @@ CommandSession::getCommand(int counter) { return std::pair("unknown", ""); } +// should be replaced by the general config-getter in cc setup std::vector CommandSession::getZones() { ElementPtr cmd, result, env; std::vector zone_names; cmd = Element::create_from_string("{ \"command\": [ \"zone\", \"list\" ] }"); - sleep(1); session_.group_sendmsg(cmd, "ConfigManager"); session_.group_recvmsg(env, result, false); BOOST_FOREACH(ElementPtr zone_name, result->get("result")->list_value()) { diff --git a/src/bin/parkinglot/ccsession.h b/src/bin/parkinglot/ccsession.h index 00ed7f18eb..ad395d24cd 100644 --- a/src/bin/parkinglot/ccsession.h +++ b/src/bin/parkinglot/ccsession.h @@ -20,6 +20,8 @@ #include #include +#include +#include class CommandSession { public: @@ -28,7 +30,11 @@ public: std::pair getCommand(int counter); std::vector getZones(); private: + void read_data_definition(const std::string& filename); + ISC::CC::Session session_; + ISC::Data::DataDefinition data_definition_; + ISC::Data::ElementPtr config_; }; #endif // __CCSESSION_H diff --git a/src/lib/bind-cfgd/python/bind-cfgd.py b/src/lib/bind-cfgd/python/bind-cfgd.py index ef2caff25a..a3dc14848e 100644 --- a/src/lib/bind-cfgd/python/bind-cfgd.py +++ b/src/lib/bind-cfgd/python/bind-cfgd.py @@ -112,7 +112,6 @@ if __name__ == "__main__": signal.signal(signal.SIGINT, signal_handler) signal.signal(signal.SIGTERM, signal_handler) cm.read_config() - # do loading here if necessary cm.notify_boss() cm.run() cm.write_config() diff --git a/src/lib/cc/cpp/Makefile.am b/src/lib/cc/cpp/Makefile.am index 6731cfa228..8775fb6c28 100644 --- a/src/lib/cc/cpp/Makefile.am +++ b/src/lib/cc/cpp/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(top_srcdir)/src/lib/cc/cpp -I$(top_srcdir)/ext +AM_CPPFLAGS = -I$(top_srcdir)/src/lib/cc/cpp -I$(top_srcdir)/ext -Wall -Werror bin_PROGRAMS = test test_SOURCES = test.cc diff --git a/src/lib/cc/cpp/data_def.cc b/src/lib/cc/cpp/data_def.cc index bb471b7e6b..ebf194df30 100644 --- a/src/lib/cc/cpp/data_def.cc +++ b/src/lib/cc/cpp/data_def.cc @@ -5,14 +5,150 @@ #include +// todo: add more context to thrown DataDefinitionErrors? using namespace ISC::Data; -DataDefinition::DataDefinition(std::istream& in) throw(ParseError) { +// todo: is there a direct way to get a std::string from an enum label? +static std::string +get_type_string(Element::types type) +{ + switch(type) { + case Element::integer: + return std::string("integer"); + case Element::real: + return std::string("real"); + case Element::boolean: + return std::string("boolean"); + case Element::string: + return std::string("string"); + case Element::list: + return std::string("list"); + case Element::map: + return std::string("map"); + default: + return std::string("unknown"); + } +} + +static Element::types +get_type_value(const std::string& type_name) { + if (type_name == "integer") { + return Element::integer; + } else if (type_name == "real") { + return Element::real; + } else if (type_name == "boolean") { + return Element::boolean; + } else if (type_name == "string") { + return Element::string; + } else if (type_name == "list") { + return Element::list; + } else if (type_name == "map") { + return Element::map; + } else { + throw DataDefinitionError(type_name + " is not a valid type name"); + } +} + +static void +check_leaf_item(const ElementPtr& spec, const std::string& name, Element::types type, bool mandatory) +{ + if (spec->contains(name)) { + if (spec->get(name)->get_type() == type) { + return; + } else { + throw DataDefinitionError(name + " not of type " + get_type_string(type)); + } + } else if (mandatory) { + // todo: want parent item name, and perhaps some info about location + // in list? or just catch and throw new... + // or make this part non-throwing and check return value... + throw DataDefinitionError(name + " missing in " + spec->str()); + } +} + +static void check_config_item_list(const ElementPtr& spec); + +static void +check_config_item(const ElementPtr& spec) { + check_leaf_item(spec, "item_name", Element::string, true); + check_leaf_item(spec, "item_type", Element::string, true); + check_leaf_item(spec, "item_optional", Element::boolean, true); + check_leaf_item(spec, "item_default", + get_type_value(spec->get("item_type")->string_value()), + !spec->get("item_optional")->bool_value() + ); + + // if list, check the list definition + if (get_type_value(spec->get("item_type")->string_value()) == Element::list) { + check_leaf_item(spec, "list_item_spec", Element::map, true); + check_config_item(spec->get("list_item_spec")); + } + // todo: add stuff for type map + if (get_type_value(spec->get("item_type")->string_value()) == Element::map) { + check_leaf_item(spec, "map_item_spec", Element::list, true); + check_config_item_list(spec); + } +} + +static void +check_config_item_list(const ElementPtr& spec) { + if (spec->get_type() != Element::list) { + throw DataDefinitionError("config_data is not a list of elements"); + } + BOOST_FOREACH(ElementPtr item, spec->list_value()) { + check_config_item(item); + } +} + +static void +check_command(const ElementPtr& spec) { + check_leaf_item(spec, "command_name", Element::string, true); + check_leaf_item(spec, "command_args", Element::list, true); + check_config_item_list(spec->get("command_args")); +} + +static void +check_command_list(const ElementPtr& spec) { + if (spec->get_type() != Element::list) { + throw DataDefinitionError("commands is not a list of elements"); + } + BOOST_FOREACH(ElementPtr item, spec->list_value()) { + check_command(item); + } +} + +static void +check_data_specification(const ElementPtr& spec) { + check_leaf_item(spec, "module_name", Element::string, true); + // not mandatory; module could just define commands and have + // no config + if (spec->contains("config_data")) { + check_config_item_list(spec->get("config_data")); + } + if (spec->contains("commands")) { + check_command_list(spec->get("commands")); + } +} + +// checks whether the given element is a valid data definition +// throws a DataDefinitionError if the specification is bad +static void +check_definition(const ElementPtr& def) +{ + if (!def->contains("data_specification")) { + throw DataDefinitionError("Data specification does not contain data_specification element"); + } else { + check_data_specification(def->get("data_specification")); + } +} + +DataDefinition::DataDefinition(std::istream& in, const bool check) + throw(ParseError, DataDefinitionError) { definition = Element::create_from_string(in); - // TODO, make sure the whole structure is complete and valid - if (!definition->contains("data_specification")) { - throw ParseError("Data specification does not contain data_specification element"); + // make sure the whole structure is complete and valid + if (check) { + check_definition(definition); } } @@ -100,7 +236,6 @@ bool DataDefinition::validate_spec_list(const ElementPtr spec, const ElementPtr data) { ElementPtr cur_data_el; std::string cur_item_name; - bool optional; BOOST_FOREACH(ElementPtr cur_spec_el, spec->list_value()) { if (!validate_spec(cur_spec_el, data)) { return false; diff --git a/src/lib/cc/cpp/data_def.h b/src/lib/cc/cpp/data_def.h index 06c0c4f8db..415e7d98e6 100644 --- a/src/lib/cc/cpp/data_def.h +++ b/src/lib/cc/cpp/data_def.h @@ -7,11 +7,22 @@ namespace ISC { namespace Data { + class DataDefinitionError : public std::exception { + public: + DataDefinitionError(std::string m = "Data definition is invalid") : msg(m) {} + ~DataDefinitionError() throw() {} + const char* what() const throw() { return msg.c_str(); } + private: + std::string msg; + }; + class DataDefinition { public: explicit DataDefinition() {}; explicit DataDefinition(ElementPtr e) : definition(e) {}; - explicit DataDefinition(std::istream& in) throw(ParseError); + // todo: make check default false, or leave out option completely and always check? + explicit DataDefinition(std::istream& in, const bool check = true) + throw(ParseError, DataDefinitionError); const ElementPtr getDefinition() { return definition; }; // returns true if the given element conforms to this data diff --git a/src/lib/cc/cpp/parkinglot.spec b/src/lib/cc/cpp/parkinglot.spec index 05ac620304..4b6e98abee 100644 --- a/src/lib/cc/cpp/parkinglot.spec +++ b/src/lib/cc/cpp/parkinglot.spec @@ -1,6 +1,6 @@ { "data_specification": { - "module_name": "parkinglot", + "module_name": "ParkingLot", "config_data": [ { "item_name": "port", @@ -24,21 +24,21 @@ "commands": [ { "command_name": "zone_add", - "command_args": { + "command_args": [ { "item_name": "zone_name", "item_type": "string", "item_optional": false, "item_default": "" - } + } ] }, { "command_name": "zone_delete", - "command_args": { + "command_args": [ { "item_name": "zone_name", "item_type": "string", "item_optional": false, "item_default": "" - } + } ] } ] } diff --git a/src/lib/cc/cpp/session.cc b/src/lib/cc/cpp/session.cc index 8a6305278e..a9b53b0bfa 100644 --- a/src/lib/cc/cpp/session.cc +++ b/src/lib/cc/cpp/session.cc @@ -138,7 +138,8 @@ Session::unsubscribe(std::string group, std::string instance) } unsigned int -Session::group_sendmsg(ElementPtr& msg, std::string group, std::string instance, std::string to) +Session::group_sendmsg(const ElementPtr& msg, std::string group, + std::string instance, std::string to) { ElementPtr env = Element::create(std::map()); diff --git a/src/lib/cc/cpp/session.h b/src/lib/cc/cpp/session.h index bffebba81f..1be1b1ad7a 100644 --- a/src/lib/cc/cpp/session.h +++ b/src/lib/cc/cpp/session.h @@ -43,7 +43,7 @@ namespace ISC { std::string subtype = "normal"); void unsubscribe(std::string group, std::string instance = "*"); - unsigned int group_sendmsg(ISC::Data::ElementPtr& msg, + unsigned int group_sendmsg(const ISC::Data::ElementPtr& msg, std::string group, std::string instance = "*", std::string to = "*"); diff --git a/src/lib/cc/cpp/test.cc b/src/lib/cc/cpp/test.cc index 4ff6a896d8..69d6672670 100644 --- a/src/lib/cc/cpp/test.cc +++ b/src/lib/cc/cpp/test.cc @@ -27,6 +27,9 @@ main(int argc, char **argv) { } catch (ParseError pe) { cout << "Error parsing definition file: " << pe.what() << endl; return 1; + } catch (DataDefinitionError dde) { + cout << "Error reading definition file: " << dde.what() << endl; + return 1; } file.close();