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

@@ -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 <stdexcept>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/time.h>
 
 #include <iostream>
+#include <fstream>
 #include <sstream>
 
 #include <boost/foreach.hpp>
 
 #include <cc/cpp/data.h>
+#include <cc/cpp/data_def.h>
 #include <cc/cpp/session.h>
 
 #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<string, string>("unknown", "");
 }
 
+// should be replaced by the general config-getter in cc setup
 std::vector<std::string>
 CommandSession::getZones() {
     ElementPtr cmd, result, env;
     std::vector<std::string> 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()) {

src/bin/parkinglot/ccsession.h

@@ -20,6 +20,8 @@
 #include <string>
 
 #include <cc/cpp/session.h>
+#include <cc/cpp/data_def.h>
+#include <cc/cpp/data.h>
 
 class CommandSession {
 public:
@@ -28,7 +30,11 @@ public:
     std::pair<std::string, std::string> getCommand(int counter);
     std::vector<std::string> 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

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()

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

src/lib/cc/cpp/data_def.cc

@@ -5,14 +5,150 @@
 
 #include <boost/foreach.hpp>
 
+// 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;

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

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": ""
-        }
+        } ]
       }
     ]
   }

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<std::string, ElementPtr>());
 

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 = "*");

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();