sync with trunk, resolving a conflict.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac446@3997 e5f2f494-b856-4b98-b285-d166d9295462

ChangeLog

@@ -1,3 +1,20 @@
+  137.	[bug]		jreed
+	Fix run_*.sh scripts that are used for development testing
+	so they use a msgq socket file in the build tree.
+	(Trac #226, svn r3989)
+
+  136.  [bug]       jelte
+  	bindctl (and the configuration manager in general) now no longer
+	accepts 'unknown' data; i.e. data for modules that it does not know
+	about, or configuration items that are not specified in the .spec
+	files.
+	(Trac #202, svn r3967)
+
+  135.  [func]      each
+	Add b10-recurse. This is an example recursive server that
+	currently does forwarding only and no caching.
+	(Trac #327, svn r3903)
+
   134.  [func]      vorner
 	b10-recurse supports timeouts and retries in forwarder mode.
 	(Trac #401, svn r3660)
@@ -8,7 +25,7 @@
 	(Trac #393, r3602)
 
   132.  [func]      vorner
-	The b10-recursive is configured through config manager.
+	The b10-recurse is configured through config manager.
 	It has "listen_on" and "forward_addresses" options.
 	(Trac #389, r3448)
 
@@ -39,7 +56,6 @@
 	for root zone was added.
 	(Trac #85, svn r3836)
 
->>>>>>> .merge-right.r3894
   127.  [bug]       stephen
 	During normal operation process termination and resurrection messages
 	are now output regardless of the state of the verbose flag.

src/bin/auth/tests/query_unittest.cc

@@ -18,6 +18,7 @@
 #include <dns/rrtype.h>
 
 #include <datasrc/zonetable.h>
+#include <datasrc/memory_datasrc.h>
 
 #include <auth/query.h>
 

src/bin/bind10/run_bind10.sh.in

@@ -46,5 +46,5 @@ BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
 export BIND10_MSGQ_SOCKET_FILE
 
 cd ${BIND10_PATH}
-exec ${PYTHON_EXEC} -O bind10 $*
+exec ${PYTHON_EXEC} -O bind10 "$@"
 

src/bin/bindctl/run_bindctl.sh.in

@@ -26,5 +26,8 @@ export PYTHONPATH
 B10_FROM_SOURCE=@abs_top_srcdir@
 export B10_FROM_SOURCE
 
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
 cd ${BINDCTL_PATH}
-exec ${PYTHON_EXEC} -O bindctl $*
+exec ${PYTHON_EXEC} -O bindctl "$@"

src/bin/cmdctl/run_b10-cmdctl.sh.in

@@ -22,6 +22,8 @@ CMD_CTRLD_PATH=@abs_top_builddir@/src/bin/cmdctl
 PYTHONPATH=@abs_top_srcdir@/src/lib/python
 export PYTHONPATH
 
-cd ${CMD_CTRLD_PATH}
-${PYTHON_EXEC} b10-cmdctl
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
 
+cd ${CMD_CTRLD_PATH}
+exec ${PYTHON_EXEC} b10-cmdctl "$@"

src/bin/loadzone/run_loadzone.sh.in

@@ -21,5 +21,8 @@ export PYTHON_EXEC
 PYTHONPATH=@abs_top_builddir@/src/lib/python
 export PYTHONPATH
 
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
 LOADZONE_PATH=@abs_top_builddir@/src/bin/loadzone
-exec ${LOADZONE_PATH}/b10-loadzone $*
+exec ${LOADZONE_PATH}/b10-loadzone "$@"

src/bin/msgq/run_msgq.sh.in

@@ -23,5 +23,8 @@ MYPATH_PATH=@abs_top_builddir@/src/bin/msgq
 PYTHONPATH=@abs_top_srcdir@/src/lib/python
 export PYTHONPATH
 
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
 cd ${MYPATH_PATH}
-exec ${PYTHON_EXEC} -O b10-msgq $*
+exec ${PYTHON_EXEC} -O b10-msgq "$@"

src/bin/stats/run_b10-stats.sh.in

@@ -21,10 +21,13 @@ export PYTHON_EXEC
 PYTHONPATH=@abs_top_builddir@/src/lib/python
 export PYTHONPATH
 
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
 B10_FROM_BUILD=@abs_top_builddir@
 export B10_FROM_BUILD
 
 STATS_PATH=@abs_top_builddir@/src/bin/stats
 
 cd ${STATS_PATH}
-exec ${PYTHON_EXEC} -O b10-stats $*
+exec ${PYTHON_EXEC} -O b10-stats "$@"

src/bin/stats/run_b10-stats_stub.sh.in

@@ -24,7 +24,10 @@ export PYTHONPATH
 B10_FROM_BUILD=@abs_top_srcdir@
 export B10_FROM_BUILD
 
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
 STATS_PATH=@abs_top_builddir@/src/bin/stats
 
 cd ${STATS_PATH}
-exec ${PYTHON_EXEC} -O b10-stats_stub $*
+exec ${PYTHON_EXEC} -O b10-stats_stub "$@"

src/bin/usermgr/run_b10-cmdctl-usermgr.sh.in

@@ -20,6 +20,8 @@ export PYTHON_EXEC
 
 MYPATH_PATH=@abs_top_builddir@/src/bin/usermgr
 
-cd ${MYPATH_PATH}
-${PYTHON_EXEC} b10-cmdctl-usermgr
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
 
+cd ${MYPATH_PATH}
+exec ${PYTHON_EXEC} b10-cmdctl-usermgr "$@"

src/bin/xfrin/run_b10-xfrin.sh.in

@@ -22,6 +22,8 @@ MYPATH_PATH=@abs_top_builddir@/src/bin/xfrin
 PYTHONPATH=@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/.libs
 export PYTHONPATH
 
-cd ${MYPATH_PATH}
-${PYTHON_EXEC} b10-xfrin
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
 
+cd ${MYPATH_PATH}
+exec ${PYTHON_EXEC} b10-xfrin "$@"

src/bin/xfrout/run_b10-xfrout.sh.in

@@ -22,6 +22,8 @@ MYPATH_PATH=@abs_top_builddir@/src/bin/xfrout
 PYTHONPATH=@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/dns/python/.libs
 export PYTHONPATH
 
-cd ${MYPATH_PATH}
-${PYTHON_EXEC} b10-xfrout
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
 
+cd ${MYPATH_PATH}
+exec ${PYTHON_EXEC} b10-xfrout "$@"

src/bin/zonemgr/run_b10-zonemgr.sh.in

@@ -22,6 +22,8 @@ MYPATH_PATH=@abs_top_builddir@/src/bin/zonemgr
 PYTHONPATH=@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/.libs
 export PYTHONPATH
 
-cd ${MYPATH_PATH}
-${PYTHON_EXEC} b10-zonemgr
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
 
+cd ${MYPATH_PATH}
+exec ${PYTHON_EXEC} b10-zonemgr "$@"

src/lib/config/module_spec.cc

@@ -330,13 +330,32 @@ bool
 ModuleSpec::validate_spec_list(ConstElementPtr spec, ConstElementPtr data,
                                const bool full, ElementPtr errors) const
 {
+    bool validated = true;
     std::string cur_item_name;
     BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
         if (!validate_spec(cur_spec_el, data, full, errors)) {
-            return (false);
+            validated = false;
         }
     }
-    return (true);
+
+    typedef std::pair<std::string, ConstElementPtr> maptype;
+    
+    BOOST_FOREACH(maptype m, data->mapValue()) {
+        bool found = false;
+        BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
+            if (cur_spec_el->get("item_name")->stringValue().compare(m.first) == 0) {
+                found = true;
+            }
+        }
+        if (!found) {
+            validated = false;
+            if (errors) {
+                errors->add(Element::create("Unknown item " + m.first));
+            }
+        }
+    }
+
+    return (validated);
 }
 
 }

src/lib/config/tests/module_spec_unittests.cc

@@ -166,8 +166,13 @@ TEST(ModuleSpec, DataValidation) {
     EXPECT_TRUE(data_test(dd, "data22_6.data"));
     EXPECT_TRUE(data_test(dd, "data22_7.data"));
     EXPECT_FALSE(data_test(dd, "data22_8.data"));
+    EXPECT_FALSE(data_test(dd, "data22_9.data"));
 
     ElementPtr errors = Element::createList();
     EXPECT_FALSE(data_test_with_errors(dd, "data22_8.data", errors));
     EXPECT_EQ("[ \"Type mismatch\" ]", errors->str());
+
+    errors = Element::createList();
+    EXPECT_FALSE(data_test_with_errors(dd, "data22_9.data", errors));
+    EXPECT_EQ("[ \"Unknown item value_does_not_exist\" ]", errors->str());
 }

src/lib/config/tests/testdata/Makefile.am

@@ -20,6 +20,7 @@ EXTRA_DIST += data22_5.data
 EXTRA_DIST += data22_6.data
 EXTRA_DIST += data22_7.data
 EXTRA_DIST += data22_8.data
+EXTRA_DIST += data22_9.data
 EXTRA_DIST += spec1.spec
 EXTRA_DIST += spec2.spec
 EXTRA_DIST += spec3.spec

src/lib/config/tests/testdata/data22_8.data

@@ -5,5 +5,6 @@
     "value4": "foo",
     "value5": [ 1, 2, 3 ],
     "value6": { "v61": "bar", "v62": true },
-    "value8": [ { "a": "d" }, { "a": 1 } ]
+    "value8": [ { "a": "d" }, { "a": 1 } ],
+    "value9": { "v91": "hi", "v92": { "v92a": "Hi", "v92b": 3 } }
 }

src/lib/config/tests/testdata/data22_9.data

@@ -0,0 +1,11 @@
+{
+    "value1": 1,
+    "value2": 2.3,
+    "value3": true,
+    "value4": "foo",
+    "value5": [ 1, 2, 3 ],
+    "value6": { "v61": "bar", "v62": true },
+    "value8": [ { "a": "d" }, { "a": "e" } ],
+    "value9": { "v91": "hi", "v92": { "v92a": "Hi", "v92b": 3 } },
+    "value_does_not_exist": 1
+}

src/lib/datasrc/Makefile.am

@@ -15,6 +15,8 @@ libdatasrc_la_SOURCES += static_datasrc.h static_datasrc.cc
 libdatasrc_la_SOURCES += sqlite3_datasrc.h sqlite3_datasrc.cc
 libdatasrc_la_SOURCES += query.h query.cc
 libdatasrc_la_SOURCES += cache.h cache.cc
-libdatasrc_la_SOURCES += rbtree.h 
+libdatasrc_la_SOURCES += rbtree.h
 libdatasrc_la_SOURCES += zonetable.h zonetable.cc
 libdatasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
+libdatasrc_la_SOURCES += zone.h
+libdatasrc_la_SOURCES += result.h

src/lib/datasrc/memory_datasrc.cc

@@ -12,8 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <map>
+#include <boost/shared_ptr.hpp>
+
 #include <dns/name.h>
+#include <dns/rrclass.h>
+
 #include <datasrc/memory_datasrc.h>
+#include <datasrc/rbtree.h>
 
 using namespace std;
 using namespace isc::dns;
@@ -21,6 +27,63 @@ using namespace isc::dns;
 namespace isc {
 namespace datasrc {
 
+// Private data and hidden methods of MemoryZone
+struct MemoryZone::MemoryZoneImpl {
+    // Constructor
+    MemoryZoneImpl(const RRClass& zone_class, const Name& origin) :
+        zone_class_(zone_class), origin_(origin)
+    {}
+
+    // Information about the zone
+    RRClass zone_class_;
+    Name origin_;
+
+    // Some type aliases
+    /*
+     * Each domain consists of some RRsets. They will be looked up by the
+     * RRType.
+     *
+     * The use of map is questionable with regard to performance - there'll
+     * be usually only few RRsets in the domain, so the log n benefit isn't
+     * much and a vector/array might be faster due to its simplicity and
+     * continuous memory location. But this is unlikely to be a performance
+     * critical place and map has better interface for the lookups, so we use
+     * that.
+     */
+    typedef map<RRType, ConstRRsetPtr> Domain;
+    typedef boost::shared_ptr<Domain> DomainPtr;
+    // The tree stores domains
+    typedef RBTree<Domain> DomainTree;
+    typedef RBNode<Domain> DomainNode;
+    // The actual zone data
+    DomainTree domains_;
+};
+
+MemoryZone::MemoryZone(const RRClass& zone_class, const Name& origin) :
+    impl_(new MemoryZoneImpl(zone_class, origin))
+{
+}
+
+MemoryZone::~MemoryZone() {
+    delete impl_;
+}
+
+const Name&
+MemoryZone::getOrigin() const {
+    return (impl_->origin_);
+}
+
+const RRClass&
+MemoryZone::getClass() const {
+    return (impl_->zone_class_);
+}
+
+Zone::FindResult
+MemoryZone::find(const Name&, const RRType&) const {
+    // This is a tentative implementation that always returns NXDOMAIN.
+    return (FindResult(NXDOMAIN, RRsetPtr()));
+}
+
 /// Implementation details for \c MemoryDataSrc hidden from the public
 /// interface.
 ///

src/lib/datasrc/memory_datasrc.h

@@ -24,6 +24,51 @@ class Name;
 
 namespace datasrc {
 
+/// A derived zone class intended to be used with the memory data source.
+class MemoryZone : public Zone {
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    /// \b Note:
+    /// The copy constructor and the assignment operator are intentionally
+    /// defined as private, making this class non copyable.
+    //@{
+private:
+    MemoryZone(const MemoryZone& source);
+    MemoryZone& operator=(const MemoryZone& source);
+public:
+    /// \brief Constructor from zone parameters.
+    ///
+    /// This constructor internally involves resource allocation, and if
+    /// it fails, a corresponding standard exception will be thrown.
+    /// It never throws an exception otherwise.
+    ///
+    /// \param rrclass The RR class of the zone.
+    /// \param origin The origin name of the zone.
+    MemoryZone(const isc::dns::RRClass& rrclass, const isc::dns::Name& origin);
+
+    /// The destructor.
+    virtual ~MemoryZone();
+    //@}
+
+    /// \brief Returns the origin of the zone.
+    virtual const isc::dns::Name& getOrigin() const;
+    /// \brief Returns the class of the zone.
+    virtual const isc::dns::RRClass& getClass() const;
+    /// \brief Looks up an RRset in the zone.
+    ///
+    /// See documentation in \c Zone.
+    virtual FindResult find(const isc::dns::Name& name,
+                            const isc::dns::RRType& type) const;
+
+private:
+    /// \name Hidden private data
+    //@{
+    struct MemoryZoneImpl;
+    MemoryZoneImpl* impl_;
+    //@}
+};
+
 /// \brief A data source that uses in memory dedicated backend.
 ///
 /// The \c MemoryDataSrc class represents a data source and provides a

src/lib/datasrc/result.h

@@ -0,0 +1,40 @@
+// Copyright (C) 2010  CZ NIC
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __DATASRC_RESULT_H
+#define __DATASRC_RESULT_H 1
+
+namespace isc {
+namespace datasrc {
+namespace result {
+/// Result codes of various public methods of in memory data source
+///
+/// The detailed semantics may differ in different methods.
+/// See the description of specific methods for more details.
+///
+/// Note: this is intended to be used from other data sources eventually,
+/// but for now it's specific to in memory data source and its backend.
+enum Result {
+    SUCCESS,  ///< The operation is successful.
+    EXIST,    ///< The search key is already stored.
+    NOTFOUND, ///< The specified object is not found.
+    PARTIALMATCH ///< Only a partial match is found.
+};
+
+}
+}
+}
+
+#endif

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -17,7 +17,6 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
-#include <datasrc/zonetable.h>
 #include <datasrc/memory_datasrc.h>
 
 #include <gtest/gtest.h>
@@ -114,7 +113,6 @@ TEST_F(MemoryDataSrcTest, add_find_Zone) {
               memory_datasrc.findZone(Name("z.i.g.h")).zone->getOrigin());
 }
 
-
 TEST_F(MemoryDataSrcTest, getZoneCount) {
     EXPECT_EQ(0, memory_datasrc.getZoneCount());
     memory_datasrc.addZone(
@@ -131,4 +129,30 @@ TEST_F(MemoryDataSrcTest, getZoneCount) {
                   ZonePtr(new MemoryZone(rrclass, Name("example.org"))));
     EXPECT_EQ(2, memory_datasrc.getZoneCount());
 }
+
+/// \brief Test fixture for the MemoryZone class
+class MemoryZoneTest : public ::testing::Test {
+public:
+    MemoryZoneTest() :
+        class_(RRClass::IN()),
+        origin_("example.org"),
+        zone_(class_, origin_)
+    { }
+    // Some data to test with
+    RRClass class_;
+    Name origin_;
+    // The zone to torture by tests
+    MemoryZone zone_;
+};
+
+/**
+ * \brief Test MemoryZone::MemoryZone constructor.
+ *
+ * Takes the created zone and checks its properties they are the same
+ * as passed parameters.
+ */
+TEST_F(MemoryZoneTest, Constructor) {
+    ASSERT_EQ(class_, zone_.getClass());
+    ASSERT_EQ(origin_, zone_.getOrigin());
+}
 }

src/lib/datasrc/tests/zonetable_unittest.cc

@@ -18,6 +18,8 @@
 #include <dns/rrclass.h>
 
 #include <datasrc/zonetable.h>
+// We use MemoryZone to put something into the table
+#include <datasrc/memory_datasrc.h>
 
 #include <gtest/gtest.h>
 
@@ -73,7 +75,7 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_THROW(zone_table.addZone(ZonePtr()), isc::InvalidParameter);
 }
 
-TEST_F(ZoneTableTest, removeZone) {
+TEST_F(ZoneTableTest, DISABLED_removeZone) {
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));

src/lib/datasrc/zone.h

@@ -0,0 +1,180 @@
+// Copyright (C) 2010  CZ NIC
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __ZONE_H
+#define __ZONE_H 1
+
+#include <datasrc/result.h>
+
+namespace isc {
+namespace datasrc {
+
+/// \brief The base class for a single authoritative zone
+///
+/// The \c Zone class is an abstract base class for representing
+/// a DNS zone as part of data source.
+///
+/// At the moment this is provided mainly for making the \c ZoneTable class
+/// and the authoritative query logic  testable, and only provides a minimal
+/// set of features.
+/// This is why this class is defined in the same header file, but it may
+/// have to move to a separate header file when we understand what is
+/// necessary for this class for actual operation.
+///
+/// The idea is to provide a specific derived zone class for each data
+/// source, beginning with in memory one.  At that point the derived classes
+/// will have more specific features.  For example, they will maintain
+/// information about the location of a zone file, whether it's loaded in
+/// memory, etc.
+///
+/// It's not yet clear how the derived zone classes work with various other
+/// data sources when we integrate these components, but one possibility is
+/// something like this:
+/// - If the underlying database such as some variant of SQL doesn't have an
+///   explicit representation of zones (as part of public interface), we can
+///   probably use a "default" zone class that simply encapsulates the
+///   corresponding data source and calls a common "find" like method.
+/// - Some data source may want to specialize it by inheritance as an
+///   optimization.  For example, in the current schema design of the sqlite3
+///   data source, its (derived) zone class would contain the information of
+///   the "zone ID".
+///
+/// <b>Note:</b> Unlike some other abstract base classes we don't name the
+/// class beginning with "Abstract".  This is because we want to have
+/// commonly used definitions such as \c Result and \c ZonePtr, and we want
+/// to make them look more intuitive.
+class Zone {
+public:
+    /// Result codes of the \c find() method.
+    ///
+    /// Note: the codes are tentative.  We may need more, or we may find
+    /// some of them unnecessary as we implement more details.
+    enum Result {
+        SUCCESS,                ///< An exact match is found.
+        DELEGATION,             ///< The search encounters a zone cut.
+        NXDOMAIN, ///< There is no domain name that matches the search name
+        NXRRSET,  ///< There is a matching name but no RRset of the search type
+        CNAME,    ///< The search encounters and returns a CNAME RR
+        DNAME     ///< The search encounters and returns a DNAME RR
+    };
+
+    /// A helper structure to represent the search result of \c find().
+    ///
+    /// This is a straightforward tuple of the result code and a pointer
+    /// to the found RRset to represent the result of \c find()
+    /// (there will be more members in the future - see the class
+    /// description).
+    /// We use this in order to avoid overloading the return value for both
+    /// the result code ("success" or "not found") and the found object,
+    /// i.e., avoid using \c NULL to mean "not found", etc.
+    ///
+    /// This is a simple value class whose internal state never changes,
+    /// so for convenience we allow the applications to refer to the members
+    /// directly.
+    ///
+    /// Note: we should eventually include a notion of "zone node", which
+    /// corresponds to a particular domain name of the zone, so that we can
+    /// find RRsets of a different RR type for that name (e.g. for type ANY
+    /// query or to include DS RRs with delegation).
+    ///
+    /// Note: we may also want to include the closest enclosure "node" to
+    /// optimize including the NSEC for no-wildcard proof (FWIW NSD does that).
+    struct FindResult {
+        FindResult(Result param_code,
+                   const isc::dns::ConstRRsetPtr param_rrset) :
+            code(param_code), rrset(param_rrset)
+        {}
+        const Result code;
+        const isc::dns::ConstRRsetPtr rrset;
+    };
+
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    //@{
+protected:
+    /// The default constructor.
+    ///
+    /// This is intentionally defined as \c protected as this base class should
+    /// never be instantiated (except as part of a derived class).
+    Zone() {}
+public:
+    /// The destructor.
+    virtual ~Zone() {}
+    //@}
+
+    ///
+    /// \name Getter Methods
+    ///
+    /// These methods should never throw an exception.
+    //@{
+    /// Return the origin name of the zone.
+    virtual const isc::dns::Name& getOrigin() const = 0;
+
+    /// Return the RR class of the zone.
+    virtual const isc::dns::RRClass& getClass() const = 0;
+    //@}
+
+    ///
+    /// \name Search Method
+    ///
+    //@{
+    /// Search the zone for a given pair of domain name and RR type.
+    ///
+    /// Each derived version of this method searches the underlying backend
+    /// for the data that best matches the given name and type.
+    /// This method is expected to be "intelligent", and identifies the
+    /// best possible answer for the search key.  Specifically,
+    /// - If the search name belongs under a zone cut, it returns the code
+    ///   of \c DELEGATION and the NS RRset at the zone cut.
+    /// - If there is no matching name, it returns the code of \c NXDOMAIN,
+    ///   and, if DNSSEC is requested, the NSEC RRset that proves the
+    ///   non-existence.
+    /// - If there is a matching name but no RRset of the search type, it
+    ///   returns the code of \c NXRRSET, and, if DNSSEC is required,
+    ///   the NSEC RRset for that name.
+    /// - If there is a matching name with CNAME, it returns the code of
+    ///   \c CNAME and that CNAME RR.
+    /// - If the search name matches a delegation point of DNAME, it returns
+    ///   the code of \c DNAME and that DNAME RR.
+    ///
+    /// A derived version of this method may involve internal resource
+    /// allocation, especially for constructing the resulting RRset, and may
+    /// throw an exception if it fails.
+    /// It should not throw other types of exceptions.
+    ///
+    /// Note: It's quite likely that we'll need to specify search options.
+    /// For example, we should be able to specify whether to allow returning
+    /// glue records at or under a zone cut.  We leave this interface open
+    /// at this moment.
+    ///
+    /// \param name The domain name to be searched for.
+    /// \param type The RR type to be searched for.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    virtual FindResult find(const isc::dns::Name& name,
+                            const isc::dns::RRType& type) const = 0;
+    //@}
+};
+
+/// \brief A pointer-like type pointing to a \c Zone object.
+typedef boost::shared_ptr<Zone> ZonePtr;
+
+/// \brief A pointer-like type pointing to a \c Zone object.
+typedef boost::shared_ptr<const Zone> ConstZonePtr;
+
+}
+}
+
+#endif

src/lib/datasrc/zonetable.cc

@@ -12,15 +12,12 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-// Note: map and utility (for 'pair') are for temporary workaround.
-// we'll soon replace them with built-in intelligent backend structure. 
-#include <map>
-#include <utility>
+#include <cassert>
 
 #include <dns/name.h>
-#include <dns/rrclass.h>
 
 #include <datasrc/zonetable.h>
+#include <datasrc/rbtree.h>
 
 using namespace std;
 using namespace isc::dns;
@@ -28,46 +25,77 @@ using namespace isc::dns;
 namespace isc {
 namespace datasrc {
 
-struct MemoryZone::MemoryZoneImpl {
-    MemoryZoneImpl(const RRClass& zone_class, const Name& origin) :
-        zone_class_(zone_class), origin_(origin)
-    {}
-    RRClass zone_class_;
-    Name origin_;
-};
-
-MemoryZone::MemoryZone(const RRClass& zone_class, const Name& origin) :
-    impl_(new MemoryZoneImpl(zone_class, origin))
-{
-}
-
-MemoryZone::~MemoryZone() {
-    delete impl_;
-}
-
-const Name&
-MemoryZone::getOrigin() const {
-    return (impl_->origin_);
-}
+/// \short Private data and implementation of ZoneTable
+struct ZoneTable::ZoneTableImpl {
+    // Type aliases to make it shorter
+    typedef RBTree<Zone> ZoneTree;
+    typedef RBNode<Zone> ZoneNode;
+    // The actual storage
+    ZoneTree zones_;
+
+    /*
+     * The implementation methods are here and just wrap-called in the
+     * ZoneTable. We have variables locally (without impl_->), have
+     * type aliases, etc. And they will get inlined anyway.
+     */
+
+    // Implementation of ZoneTable::addZone
+    result::Result addZone(ZonePtr zone) {
+        // Sanity check
+        if (!zone) {
+            isc_throw(InvalidParameter,
+                      "Null pointer is passed to ZoneTable::addZone()");
+        }
 
-const RRClass&
-MemoryZone::getClass() const {
-    return (impl_->zone_class_);
-}
+        // Get the node where we put the zone
+        ZoneNode* node(NULL);
+        switch (zones_.insert(zone->getOrigin(), &node)) {
+            // This is OK
+            case ZoneTree::SUCCEED:
+            case ZoneTree::ALREADYEXIST:
+                break;
+            // Can Not Happen
+            default:
+                assert(0);
+        }
+        // Can Not Happen
+        assert(node);
+
+        // Is it empty? We either just created it or it might be nonterminal
+        if (node->isEmpty()) {
+            node->setData(zone);
+            return (result::SUCCESS);
+        } else { // There's something there already
+            return (result::EXIST);
+        }
+    }
 
-Zone::FindResult
-MemoryZone::find(const Name&, const RRType&) const {
-    // This is a tentative implementation that always returns NXDOMAIN.
-    return (FindResult(NXDOMAIN, RRsetPtr()));
-}
+    // Implementation of ZoneTable::findZone
+    ZoneTable::FindResult findZone(const Name& name) const {
+        ZoneNode *node(NULL);
+        result::Result my_result;
+
+        // Translate the return codes
+        switch (zones_.find(name, &node)) {
+            case ZoneTree::EXACTMATCH:
+                my_result = result::SUCCESS;
+                break;
+            case ZoneTree::PARTIALMATCH:
+                my_result = result::PARTIALMATCH;
+                break;
+            // We have no data there, so translate the pointer to NULL as well
+            case ZoneTree::NOTFOUND:
+                return (FindResult(result::NOTFOUND, ConstZonePtr()));
+            // Can Not Happen
+            default:
+                assert(0);
+        }
 
-// This is a temporary, inefficient implementation using std::map and handmade
-// iteration to realize longest match.
+        // Can Not Happen (remember, NOTFOUND is handled)
+        assert(node);
 
-struct ZoneTable::ZoneTableImpl {
-    typedef map<Name, ZonePtr> ZoneMap;
-    typedef pair<Name, ZonePtr> NameAndZone;
-    ZoneMap zones;
+        return (FindResult(my_result, node->getData()));
+    }
 };
 
 ZoneTable::ZoneTable() : impl_(new ZoneTableImpl)
@@ -79,40 +107,21 @@ ZoneTable::~ZoneTable() {
 
 result::Result
 ZoneTable::addZone(ZonePtr zone) {
-    if (!zone) {
-        isc_throw(InvalidParameter,
-                  "Null pointer is passed to ZoneTable::addZone()");
-    }
-
-    if (impl_->zones.insert(
-            ZoneTableImpl::NameAndZone(zone->getOrigin(), zone)).second
-        == true) {
-        return (result::SUCCESS);
-    } else {
-        return (result::EXIST);
-    }
+    return (impl_->addZone(zone));
 }
 
 result::Result
-ZoneTable::removeZone(const Name& origin) {
-    return (impl_->zones.erase(origin) == 1 ? result::SUCCESS :
-                                              result::NOTFOUND);
+ZoneTable::removeZone(const Name&) {
+    // TODO Implement
+    assert(0);
+    // This should not ever be returned, the assert should kill us by now
+    return (result::SUCCESS);
 }
 
 ZoneTable::FindResult
 ZoneTable::findZone(const Name& name) const {
-    // Inefficient internal loop to find a longest match.
-    // This will be replaced with a single call to more intelligent backend.
-    for (int i = 0; i < name.getLabelCount(); ++i) {
-        Name matchname(name.split(i));
-        ZoneTableImpl::ZoneMap::const_iterator found =
-            impl_->zones.find(matchname);
-        if (found != impl_->zones.end()) {
-            return (FindResult(i == 0 ? result::SUCCESS :
-                               result::PARTIALMATCH, (*found).second));
-        }
-    }
-    return (FindResult(result::NOTFOUND, ConstZonePtr()));
+    return (impl_->findZone(name));
 }
+
 } // end of namespace datasrc
 } // end of namespace isc

src/lib/datasrc/zonetable.h

@@ -19,6 +19,8 @@
 
 #include <dns/rrset.h>
 
+#include <datasrc/zone.h>
+
 namespace isc {
 namespace dns {
 class Name;
@@ -26,215 +28,6 @@ class RRClass;
 };
 
 namespace datasrc {
-namespace result {
-/// Result codes of various public methods of in memory data source
-///
-/// The detailed semantics may differ in different methods.
-/// See the description of specific methods for more details.
-///
-/// Note: this is intended to be used from other data sources eventually,
-/// but for now it's specific to in memory data source and its backend.
-enum Result {
-    SUCCESS,  ///< The operation is successful.
-    EXIST,    ///< The search key is already stored.
-    NOTFOUND, ///< The specified object is not found.
-    PARTIALMATCH ///< \c Only a partial match is found.
-};
-}
-
-/// \brief The base class for a single authoritative zone
-///
-/// The \c Zone class is an abstract base class for representing
-/// a DNS zone as part of data source.
-///
-/// At the moment this is provided mainly for making the \c ZoneTable class
-/// and the authoritative query logic  testable, and only provides a minimal
-/// set of features.
-/// This is why this class is defined in the same header file, but it may
-/// have to move to a separate header file when we understand what is
-/// necessary for this class for actual operation.
-///
-/// The idea is to provide a specific derived zone class for each data
-/// source, beginning with in memory one.  At that point the derived classes
-/// will have more specific features.  For example, they will maintain
-/// information about the location of a zone file, whether it's loaded in
-/// memory, etc.
-///
-/// It's not yet clear how the derived zone classes work with various other
-/// data sources when we integrate these components, but one possibility is
-/// something like this:
-/// - If the underlying database such as some variant of SQL doesn't have an
-///   explicit representation of zones (as part of public interface), we can
-///   probably use a "default" zone class that simply encapsulates the
-///   corresponding data source and calls a common "find" like method.
-/// - Some data source may want to specialize it by inheritance as an
-///   optimization.  For example, in the current schema design of the sqlite3
-///   data source, its (derived) zone class would contain the information of
-///   the "zone ID".
-///
-/// <b>Note:</b> Unlike some other abstract base classes we don't name the
-/// class beginning with "Abstract".  This is because we want to have
-/// commonly used definitions such as \c Result and \c ZonePtr, and we want
-/// to make them look more intuitive.
-class Zone {
-public:
-    /// Result codes of the \c find() method.
-    ///
-    /// Note: the codes are tentative.  We may need more, or we may find
-    /// some of them unnecessary as we implement more details.
-    enum Result {
-        SUCCESS,                ///< An exact match is found.
-        DELEGATION,             ///< The search encounters a zone cut.
-        NXDOMAIN, ///< There is no domain name that matches the search name
-        NXRRSET,  ///< There is a matching name but no RRset of the search type
-        CNAME,    ///< The search encounters and returns a CNAME RR
-        DNAME     ///< The search encounters and returns a DNAME RR
-    };
-
-    /// A helper structure to represent the search result of \c find().
-    ///
-    /// This is a straightforward tuple of the result code and a pointer
-    /// to the found RRset to represent the result of \c find()
-    /// (there will be more members in the future - see the class
-    /// description).
-    /// We use this in order to avoid overloading the return value for both
-    /// the result code ("success" or "not found") and the found object,
-    /// i.e., avoid using \c NULL to mean "not found", etc.
-    ///
-    /// This is a simple value class whose internal state never changes,
-    /// so for convenience we allow the applications to refer to the members
-    /// directly.
-    ///
-    /// Note: we should eventually include a notion of "zone node", which
-    /// corresponds to a particular domain name of the zone, so that we can
-    /// find RRsets of a different RR type for that name (e.g. for type ANY
-    /// query or to include DS RRs with delegation).
-    ///
-    /// Note: we may also want to include the closest enclosure "node" to
-    /// optimize including the NSEC for no-wildcard proof (FWIW NSD does that).
-    struct FindResult {
-        FindResult(Result param_code,
-                   const isc::dns::ConstRRsetPtr param_rrset) :
-            code(param_code), rrset(param_rrset)
-        {}
-        const Result code;
-        const isc::dns::ConstRRsetPtr rrset;
-    };
-
-    ///
-    /// \name Constructors and Destructor.
-    ///
-    //@{
-protected:
-    /// The default constructor.
-    ///
-    /// This is intentionally defined as \c protected as this base class should
-    /// never be instantiated (except as part of a derived class).
-    Zone() {}
-public:
-    /// The destructor.
-    virtual ~Zone() {}
-    //@}
-
-    ///
-    /// \name Getter Methods
-    ///
-    /// These methods should never throw an exception.
-    //@{
-    /// Return the origin name of the zone.
-    virtual const isc::dns::Name& getOrigin() const = 0;
-
-    /// Return the RR class of the zone.
-    virtual const isc::dns::RRClass& getClass() const = 0;
-    //@}
-
-    ///
-    /// \name Search Method
-    ///
-    //@{
-    /// Search the zone for a given pair of domain name and RR type.
-    ///
-    /// Each derived version of this method searches the underlying backend
-    /// for the data that best matches the given name and type.
-    /// This method is expected to be "intelligent", and identifies the
-    /// best possible answer for the search key.  Specifically,
-    /// - If the search name belongs under a zone cut, it returns the code
-    ///   of \c DELEGATION and the NS RRset at the zone cut.
-    /// - If there is no matching name, it returns the code of \c NXDOMAIN,
-    ///   and, if DNSSEC is requested, the NSEC RRset that proves the
-    ///   non-existence.
-    /// - If there is a matching name but no RRset of the search type, it
-    ///   returns the code of \c NXRRSET, and, if DNSSEC is required,
-    ///   the NSEC RRset for that name.
-    /// - If there is a matching name with CNAME, it returns the code of
-    ///   \c CNAME and that CNAME RR.
-    /// - If the search name matches a delegation point of DNAME, it returns
-    ///   the code of \c DNAME and that DNAME RR.
-    ///
-    /// A derived version of this method may involve internal resource
-    /// allocation, especially for constructing the resulting RRset, and may
-    /// throw an exception if it fails.
-    /// It should not throw other types of exceptions.
-    ///
-    /// Note: It's quite likely that we'll need to specify search options.
-    /// For example, we should be able to specify whether to allow returning
-    /// glue records at or under a zone cut.  We leave this interface open
-    /// at this moment.
-    ///
-    /// \param name The domain name to be searched for.
-    /// \param type The RR type to be searched for.
-    /// \return A \c FindResult object enclosing the search result (see above).
-    virtual FindResult find(const isc::dns::Name& name,
-                            const isc::dns::RRType& type) const = 0;
-    //@}
-};
-
-/// \brief A pointer-like type pointing to a \c Zone object.
-typedef boost::shared_ptr<Zone> ZonePtr;
-
-/// \brief A pointer-like type pointing to a \c Zone object.
-typedef boost::shared_ptr<const Zone> ConstZonePtr;
-
-/// A derived zone class intended to be used with the memory data source.
-///
-/// Currently this is almost empty and is only used for testing the
-/// \c ZoneTable class.  It will be substantially expanded, and will probably
-/// moved to a separate header file.
-class MemoryZone : public Zone {
-    ///
-    /// \name Constructors and Destructor.
-    ///
-    /// \b Note:
-    /// The copy constructor and the assignment operator are intentionally
-    /// defined as private, making this class non copyable.
-    //@{
-private:
-    MemoryZone(const MemoryZone& source);
-    MemoryZone& operator=(const MemoryZone& source);
-public:
-    /// \brief Constructor from zone parameters.
-    ///
-    /// This constructor internally involves resource allocation, and if
-    /// it fails, a corresponding standard exception will be thrown.
-    /// It never throws an exception otherwise.
-    ///
-    /// \param rrclass The RR class of the zone.
-    /// \param origin The origin name of the zone.
-    MemoryZone(const isc::dns::RRClass& rrclass, const isc::dns::Name& origin);
-
-    /// The destructor.
-    virtual ~MemoryZone();
-    //@}
-
-    virtual const isc::dns::Name& getOrigin() const;
-    virtual const isc::dns::RRClass& getClass() const;
-    virtual FindResult find(const isc::dns::Name& name,
-                            const isc::dns::RRType& type) const;
-
-private:
-    struct MemoryZoneImpl;
-    MemoryZoneImpl* impl_;
-};
 
 /// \brief A set of authoritative zones.
 ///
@@ -278,8 +71,18 @@ public:
     //@}
 
     /// Add a \c Zone to the \c ZoneTable.
-    /// See the description of <code>MemoryDataSrc::addZone()</code> for more
-    /// details.
+    ///
+    /// \c Zone must not be associated with a NULL pointer; otherwise
+    /// an exception of class \c InvalidParameter will be thrown.
+    /// If internal resource allocation fails, a corresponding standard
+    /// exception will be thrown.
+    /// This method never throws an exception otherwise.
+    ///
+    /// \param zone A \c Zone object to be added.
+    /// \return \c result::SUCCESS If the zone is successfully
+    /// added to the zone table.
+    /// \return \c result::EXIST The zone table already contains
+    /// zone of the same origin.
     result::Result addZone(ZonePtr zone);
 
     /// Remove a \c Zone of the given origin name from the \c ZoneTable.
@@ -294,8 +97,23 @@ public:
     result::Result removeZone(const isc::dns::Name& origin);
 
     /// Find a \c Zone that best matches the given name in the \c ZoneTable.
-    /// See the description of <code>MemoryDataSrc::findZone()</code> for more
-    /// details.
+    ///
+    /// It searches the internal storage for a \c Zone that gives the
+    /// longest match against \c name, and returns the result in the
+    /// form of a \c FindResult object as follows:
+    /// - \c code: The result code of the operation.
+    ///   - \c result::SUCCESS: A zone that gives an exact match
+    ///    is found
+    ///   - \c result::PARTIALMATCH: A zone whose origin is a
+    ///    super domain of \c name is found (but there is no exact match)
+    ///   - \c result::NOTFOUND: For all other cases.
+    /// - \c zone: A <Boost> shared pointer to the found \c Zone object if one
+    ///  is found; otherwise \c NULL.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param name A domain name for which the search is performed.
+    /// \return A \c FindResult object enclosing the search result (see above).
     FindResult findZone(const isc::dns::Name& name) const;
 
 private:

src/lib/python/isc/config/ccsession.py

@@ -224,7 +224,7 @@ class ModuleCCSession(ConfigData):
                     if not self._config_handler:
                         answer = create_answer(2, self._module_name + " has no config handler")
                     elif not self.get_module_spec().validate_config(False, new_config, errors):
-                        answer = create_answer(1, " ".join(errors))
+                        answer = create_answer(1, ", ".join(errors))
                     else:
                         isc.cc.data.remove_identical(new_config, self.get_local_config())
                         answer = self._config_handler(new_config)
@@ -422,7 +422,16 @@ class UIModuleCCSession(MultiConfigData):
         """Commit all local changes, send them through b10-cmdctl to
            the configuration manager"""
         if self.get_local_changes():
-            self._conn.send_POST('/ConfigManager/set_config', [ self.get_local_changes() ])
-            # todo: check result
-            self.request_current_config()
-            self.clear_local_changes()
+            response = self._conn.send_POST('/ConfigManager/set_config',
+                                            [ self.get_local_changes() ])
+            answer = isc.cc.data.parse_value_str(response.read().decode())
+            # answer is either an empty dict (on success), or one
+            # containing errors
+            if answer == {}:
+                self.request_current_config()
+                self.clear_local_changes()
+            elif "error" in answer:
+                print("Error: " + answer["error"])
+                print("Configuration not committed")
+            else:
+                raise ModuleCCSessionError("Unknown format of answer in commit(): " + str(answer))

src/lib/python/isc/config/config_data.py

@@ -464,12 +464,15 @@ class MultiConfigData:
            there is a specification for the given identifier, the type
            is checked."""
         spec_part = self.find_spec_part(identifier)
-        if spec_part is not None and value is not None:
-            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
-            if list_indices is not None \
-               and spec_part['item_type'] == 'list':
-                spec_part = spec_part['list_item_spec']
-            check_type(spec_part, value)
+        if spec_part is not None:
+            if value is not None:
+                id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
+                if list_indices is not None \
+                   and spec_part['item_type'] == 'list':
+                    spec_part = spec_part['list_item_spec']
+                check_type(spec_part, value)
+        else:
+            raise isc.cc.data.DataNotFoundError(identifier)
 
         # Since we do not support list diffs (yet?), we need to
         # copy the currently set list of items to _local_changes

src/lib/python/isc/config/module_spec.py

@@ -328,7 +328,24 @@ def _validate_spec(spec, full, data, errors):
         return True
 
 def _validate_spec_list(module_spec, full, data, errors):
+    # we do not return immediately, there may be more errors
+    # so we keep a boolean to keep track if we found errors
+    validated = True
+
+    # check if the known items are correct
     for spec_item in module_spec:
         if not _validate_spec(spec_item, full, data, errors):
-            return False
-    return True
+            validated = False
+
+    # check if there are items in our data that are not in the
+    # specification
+    for item_name in data:
+        found = False
+        for spec_item in module_spec:
+            if spec_item["item_name"] == item_name:
+                found = True
+        if not found:
+            if errors != None:
+                errors.append("unknown item " + item_name)
+            validated = False
+    return validated

src/lib/python/isc/config/tests/ccsession_test.py

@@ -290,7 +290,7 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_ok)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 2 }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 2 })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
@@ -303,12 +303,12 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_err)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 'aaa' }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 'aaa' })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 1)
-        self.assertEqual({'result': [1, 'just an error']},
+        self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
         
     def test_check_command5(self):
@@ -316,12 +316,12 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_exc)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 'aaa' }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 'aaa' })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 1)
-        self.assertEqual({'result': [1, 'just an exception']},
+        self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
         
     def test_check_command6(self):
@@ -416,7 +416,7 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_ok)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 2 }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 2 })
         self.assertEqual(len(fake_session.message_queue), 0)
         env = { 'group':'Spec2', 'from':None }
         mccs.check_command_without_recvmsg(cmd, env)
@@ -560,6 +560,14 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 0)
         
 
+class fakeData:
+    def decode(self):
+        return "{}";
+
+class fakeAnswer:
+    def read(self):
+        return fakeData();
+
 class fakeUIConn():
     def __init__(self):
         self.get_answers = {}
@@ -581,7 +589,7 @@ class fakeUIConn():
         if name in self.post_answers:
             return self.post_answers[name]
         else:
-            return None
+            return fakeAnswer()
     
 
 class TestUIModuleCCSession(unittest.TestCase):

src/lib/python/isc/config/tests/config_data_test.py

@@ -335,6 +335,8 @@ class TestMultiConfigData(unittest.TestCase):
         pass
 
     def test_get_local_value(self):
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
+        self.mcd.set_specification(module_spec)
         value = self.mcd.get_local_value("Spec2/item1")
         self.assertEqual(None, value)
         self.mcd.set_value("Spec2/item1", 2)
@@ -464,12 +466,11 @@ class TestMultiConfigData(unittest.TestCase):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
-        self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item1", "asdf")
+        self.assertRaises(isc.cc.data.DataTypeError,
+                          self.mcd.set_value, "Spec2/item1", "asdf")
 
-        self.mcd.set_value("Spec2/no_such_item", 4)
-        value, status = self.mcd.get_value("Spec2/no_such_item")
-        self.assertEqual(value, 4)
-        self.assertEqual(MultiConfigData.LOCAL, status)
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          self.mcd.set_value, "Spec2/no_such_item", 4)
 
         self.mcd.set_value("Spec2/item5[0]", "c")
         value, status = self.mcd.get_value("Spec2/item5[0]")

src/lib/python/isc/config/tests/module_spec_test.py

@@ -312,6 +312,18 @@ class TestModuleSpec(unittest.TestCase):
         self.assertEqual(False, isc.config.module_spec._validate_spec(spec, True, {}, None))
         self.assertEqual(False, isc.config.module_spec._validate_spec(spec, True, {}, errors))
         self.assertEqual(['non-optional item an_item missing'], errors)
+
+    def test_validate_unknown_items(self):
+        spec = [{ 'item_name': "an_item",
+                 'item_type': "string",
+                 'item_optional': True,
+                 'item_default': "asdf"
+               }]
+
+        errors = []
+        self.assertEqual(False, isc.config.module_spec._validate_spec_list(spec, True, { 'does_not_exist': 1 }, None))
+        self.assertEqual(False, isc.config.module_spec._validate_spec_list(spec, True, { 'does_not_exist': 1 }, errors))
+        self.assertEqual(['unknown item does_not_exist'], errors)