[master] Merge branch 'trac2184'

(conflicts purely in generated files, regenerated them)
Conflicts:
	doc/guide/bind10-guide.html
	doc/guide/bind10-guide.txt
	doc/guide/bind10-messages.html

doc/guide/bind10-guide.txt

@@ -968,9 +968,18 @@ Chapter 8. Authoritative Server
  > config set data_sources/classes/IN[1]/params { "example.org": "/path/to/example.org", "example.com": "/path/to/example.com" }
  > config commit
 
-   Unfortunately, due to current technical limitations, the params must be
-   set as one JSON blob, it can't be edited in bindctl. To reload a zone, you
-   the same command as above.
+   Initially, a map value has to be set, but this value may be an empty map.
+   After that, key/value pairs can be added with 'config add' and keys can be
+   removed with 'config remove'. The initial value may be an empty map, but
+   it has to be set before zones are added or removed.
+
+ > config set data_sources/classes/IN[1]/params {}
+ > config add data_sources/classes/IN[1]/params another.example.org /path/to/another.example.org
+ > config add data_sources/classes/IN[1]/params another.example.com /path/to/another.example.com
+ > config remove data_sources/classes/IN[1]/params another.example.org
+
+
+   bindctl. To reload a zone, you the same command as above.
 
   Note
 

doc/guide/bind10-guide.xml

@@ -1611,8 +1611,19 @@ can use various data source backends.
 &gt; <userinput>config set data_sources/classes/IN[1]/params { "example.org": "/path/to/example.org", "example.com": "/path/to/example.com" }</userinput>
 &gt; <userinput>config commit</userinput></screen>
 
-          Unfortunately, due to current technical limitations, the params must
-          be set as one JSON blob, it can't be edited in
+          Initially, a map value has to be set, but this value may be an
+          empty map. After that, key/value pairs can be added with 'config
+          add' and keys can be removed with 'config remove'. The initial
+          value may be an empty map, but it has to be set before zones are
+          added or removed.
+
+          <screen>
+&gt; <userinput>config set data_sources/classes/IN[1]/params {}</userinput>
+&gt; <userinput>config add data_sources/classes/IN[1]/params another.example.org /path/to/another.example.org</userinput>
+&gt; <userinput>config add data_sources/classes/IN[1]/params another.example.com /path/to/another.example.com</userinput>
+&gt; <userinput>config remove data_sources/classes/IN[1]/params another.example.org</userinput>
+          </screen>
+
           <command>bindctl</command>. To reload a zone, you the same command
           as above.
         </para>

src/lib/config/tests/testdata/spec40.spec

@@ -6,6 +6,15 @@
         "item_type": "any",
         "item_optional": false,
         "item_default": "asdf"
+      },
+      { "item_name": "item2",
+        "item_type": "any",
+        "item_optional": true
+      },
+      { "item_name": "item3",
+        "item_type": "any",
+        "item_optional": true,
+        "item_default": null
       }
     ]
   }

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

@@ -144,7 +144,7 @@ class ModuleCCSession(ConfigData):
        module, and one to update the configuration run-time. These
        callbacks are called when 'check_command' is called on the
        ModuleCCSession"""
-       
+
     def __init__(self, spec_file_name, config_handler, command_handler,
                  cc_session=None, handle_logging_config=True,
                  socket_file = None):
@@ -178,9 +178,9 @@ class ModuleCCSession(ConfigData):
         """
         module_spec = isc.config.module_spec_from_file(spec_file_name)
         ConfigData.__init__(self, module_spec)
-        
+
         self._module_name = module_spec.get_module_name()
-        
+
         self.set_config_handler(config_handler)
         self.set_command_handler(command_handler)
 
@@ -248,7 +248,7 @@ class ModuleCCSession(ConfigData):
            returns nothing.
            It calls check_command_without_recvmsg()
            to parse the received message.
-           
+
            If nonblock is True, it just checks if there's a command
            and does nothing if there isn't. If nonblock is False, it
            waits until it arrives. It temporarily sets timeout to infinity,
@@ -265,7 +265,7 @@ class ModuleCCSession(ConfigData):
         """Parse the given message to see if there is a command or a
            configuration update. Calls the corresponding handler
            functions if present. Responds on the channel if the
-           handler returns a message.""" 
+           handler returns a message."""
         # should we default to an answer? success-by-default? unhandled error?
         if msg is not None and not 'result' in msg:
             answer = None
@@ -314,7 +314,7 @@ class ModuleCCSession(ConfigData):
                 answer = create_answer(1, str(exc))
             if answer:
                 self._session.group_reply(env, answer)
-    
+
     def set_config_handler(self, config_handler):
         """Set the config handler for this module. The handler is a
            function that takes the full configuration and handles it.
@@ -521,7 +521,7 @@ class UIModuleCCSession(MultiConfigData):
         if not cur_list:
             cur_list = []
 
-        if value is None:
+        if value is None and "list_item_spec" in module_spec:
             if "item_default" in module_spec["list_item_spec"]:
                 value = module_spec["list_item_spec"]["item_default"]
 
@@ -572,8 +572,14 @@ class UIModuleCCSession(MultiConfigData):
         if module_spec is None:
             raise isc.cc.data.DataNotFoundError("Unknown item " + str(identifier))
 
+        # for type any, we determine the 'type' by what value is set
+        # (which would be either list or dict)
+        cur_value, _ = self.get_value(identifier)
+        type_any = module_spec['item_type'] == 'any'
+
         # the specified element must be a list or a named_set
-        if 'list_item_spec' in module_spec:
+        if 'list_item_spec' in module_spec or\
+           (type_any and type(cur_value) == list):
             value = None
             # in lists, we might get the value with spaces, making it
             # the third argument. In that case we interpret both as
@@ -583,11 +589,12 @@ class UIModuleCCSession(MultiConfigData):
                     value_str += set_value_str
                 value = isc.cc.data.parse_value_str(value_str)
             self._add_value_to_list(identifier, value, module_spec)
-        elif 'named_set_item_spec' in module_spec:
+        elif 'named_set_item_spec' in module_spec or\
+           (type_any and type(cur_value) == dict):
             item_name = None
             item_value = None
             if value_str is not None:
-                item_name =  isc.cc.data.parse_value_str(value_str)
+                item_name = value_str
             if set_value_str is not None:
                 item_value = isc.cc.data.parse_value_str(set_value_str)
             else:
@@ -643,12 +650,23 @@ class UIModuleCCSession(MultiConfigData):
         if value_str is not None:
             value = isc.cc.data.parse_value_str(value_str)
 
-        if 'list_item_spec' in module_spec:
-            if value is not None:
+        # for type any, we determine the 'type' by what value is set
+        # (which would be either list or dict)
+        cur_value, _ = self.get_value(identifier)
+        type_any = module_spec['item_type'] == 'any'
+
+        # there's two forms of 'remove from list'; the remove-value-from-list
+        # form, and the 'remove-by-index' form. We can recognize the second
+        # case by value is None
+        if 'list_item_spec' in module_spec or\
+           (type_any and type(cur_value) == list) or\
+           value is None:
+            if not type_any and value is not None:
                 isc.config.config_data.check_type(module_spec['list_item_spec'], value)
             self._remove_value_from_list(identifier, value)
-        elif 'named_set_item_spec' in module_spec:
-            self._remove_value_from_named_set(identifier, value)
+        elif 'named_set_item_spec' in module_spec or\
+           (type_any and type(cur_value) == dict):
+            self._remove_value_from_named_set(identifier, value_str)
         else:
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named_set")
 

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

@@ -204,6 +204,9 @@ def find_spec_part(element, identifier, strict_identifier = True):
     # always want the 'full' spec of the item
     for id_part in id_parts[:-1]:
         cur_el = _find_spec_part_single(cur_el, id_part)
+        # As soon as we find 'any', return that
+        if cur_el["item_type"] == "any":
+            return cur_el
         if strict_identifier and spec_part_is_list(cur_el) and\
            not isc.cc.data.identifier_has_list_index(id_part):
             raise isc.cc.data.DataNotFoundError(id_part +
@@ -553,7 +556,6 @@ class MultiConfigData:
             if 'item_default' in spec:
                 # one special case, named_set
                 if spec['item_type'] == 'named_set':
-                    print("is " + id_part + " in named set?")
                     return spec['item_default']
                 else:
                     return spec['item_default']
@@ -582,6 +584,14 @@ class MultiConfigData:
             value = self.get_default_value(identifier)
             if value is not None:
                 return value, self.DEFAULT
+            else:
+                # get_default_value returns None for both
+                # the cases where there is no default, and where
+                # it is set to null, so we need to catch the latter
+                spec_part = self.find_spec_part(identifier)
+                if spec_part and 'item_default' in spec_part and\
+                   spec_part['item_default'] is None:
+                    return None, self.DEFAULT
         return None, self.NONE
 
     def _append_value_item(self, result, spec_part, identifier, all, first = False):
@@ -742,6 +752,8 @@ class MultiConfigData:
                 # list
                 cur_list = cur_value
                 for list_index in list_indices:
+                    if type(cur_list) != list:
+                        raise isc.cc.data.DataTypeError(id + " is not a list")
                     if list_index >= len(cur_list):
                         raise isc.cc.data.DataNotFoundError("No item " +
                                   str(list_index) + " in " + id_part)

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

@@ -33,7 +33,7 @@ class TestHelperFunctions(unittest.TestCase):
         self.assertRaises(ModuleCCSessionError, parse_answer, { 'result': [] })
         self.assertRaises(ModuleCCSessionError, parse_answer, { 'result': [ 'not_an_rcode' ] })
         self.assertRaises(ModuleCCSessionError, parse_answer, { 'result': [ 1, 2 ] })
-        
+
         rcode, val = parse_answer({ 'result': [ 0 ] })
         self.assertEqual(0, rcode)
         self.assertEqual(None, val)
@@ -107,7 +107,7 @@ class TestModuleCCSession(unittest.TestCase):
 
     def spec_file(self, file):
         return self.data_path + os.sep + file
-        
+
     def create_session(self, spec_file_name, config_handler = None,
                        command_handler = None, cc_session = None):
         return ModuleCCSession(self.spec_file(spec_file_name),
@@ -335,7 +335,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [1, 'No config_data specification']},
                          fake_session.get_message('Spec1', None))
-        
+
     def test_check_command3(self):
         fake_session = FakeModuleCCSession()
         mccs = self.create_session("spec2.spec", None, None, fake_session)
@@ -348,7 +348,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [0]},
                          fake_session.get_message('Spec2', None))
-        
+
     def test_check_command4(self):
         fake_session = FakeModuleCCSession()
         mccs = self.create_session("spec2.spec", None, None, fake_session)
@@ -361,7 +361,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
-        
+
     def test_check_command5(self):
         fake_session = FakeModuleCCSession()
         mccs = self.create_session("spec2.spec", None, None, fake_session)
@@ -374,7 +374,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
-        
+
     def test_check_command6(self):
         fake_session = FakeModuleCCSession()
         mccs = self.create_session("spec2.spec", None, None, fake_session)
@@ -460,7 +460,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [1, 'No config_data specification']},
                          fake_session.get_message('Spec1', None))
- 
+
     def test_check_command_without_recvmsg2(self):
         "copied from test_check_command3"
         fake_session = FakeModuleCCSession()
@@ -474,7 +474,7 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         self.assertEqual({'result': [0]},
                           fake_session.get_message('Spec2', None))
- 
+
     def test_check_command_without_recvmsg3(self):
         "copied from test_check_command7"
         fake_session = FakeModuleCCSession()
@@ -487,7 +487,7 @@ class TestModuleCCSession(unittest.TestCase):
         mccs.check_command_without_recvmsg(cmd, env)
         self.assertEqual({'result': [0]},
                          fake_session.get_message('Spec2', None))
- 
+
     def test_check_command_block_timeout(self):
         """Check it works if session has timeout and it sets it back."""
         def cmd_check(mccs, session):
@@ -893,22 +893,22 @@ class fakeUIConn():
 
     def set_get_answer(self, name, answer):
         self.get_answers[name] = answer
-    
+
     def set_post_answer(self, name, answer):
         self.post_answers[name] = answer
-    
+
     def send_GET(self, name, arg = None):
         if name in self.get_answers:
             return self.get_answers[name]
         else:
             return {}
-    
+
     def send_POST(self, name, arg = None):
         if name in self.post_answers:
             return self.post_answers[name]
         else:
             return fakeAnswer()
-    
+
 
 class TestUIModuleCCSession(unittest.TestCase):
     def setUp(self):
@@ -919,9 +919,9 @@ class TestUIModuleCCSession(unittest.TestCase):
 
     def spec_file(self, file):
         return self.data_path + os.sep + file
-        
-    def create_uccs2(self, fake_conn):
-        module_spec = isc.config.module_spec_from_file(self.spec_file("spec2.spec"))
+
+    def create_uccs(self, fake_conn, specfile="spec2.spec"):
+        module_spec = isc.config.module_spec_from_file(self.spec_file(specfile))
         fake_conn.set_get_answer('/module_spec', { module_spec.get_module_name(): module_spec.get_full_spec()})
         fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         return UIModuleCCSession(fake_conn)
@@ -989,7 +989,7 @@ class TestUIModuleCCSession(unittest.TestCase):
 
     def test_add_remove_value(self):
         fake_conn = fakeUIConn()
-        uccs = self.create_uccs2(fake_conn)
+        uccs = self.create_uccs(fake_conn)
 
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "no_such_item", "a")
@@ -1020,6 +1020,88 @@ class TestUIModuleCCSession(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError,
                           uccs.remove_value, "Spec2/item5", None)
 
+    # Check that the difference between no default and default = null
+    # is recognized
+    def test_default_null(self):
+        fake_conn = fakeUIConn()
+        uccs = self.create_uccs(fake_conn, "spec40.spec")
+        (value, status) = uccs.get_value("/Spec40/item2")
+        self.assertIsNone(value)
+        self.assertEqual(uccs.NONE, status)
+        (value, status) = uccs.get_value("/Spec40/item3")
+        self.assertIsNone(value)
+        self.assertEqual(uccs.DEFAULT, status)
+
+    # Test adding and removing values for type = any
+    def test_add_remove_value_any(self):
+        fake_conn = fakeUIConn()
+        uccs = self.create_uccs(fake_conn, "spec40.spec")
+
+        # Test item set of basic types
+        items = [ 1234, "foo", True, False ]
+        items_as_str = [ '1234', 'foo', 'true', 'false' ]
+
+        def test_fails():
+            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec40/item1", "foo")
+            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec40/item1", "foo", "bar")
+            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "Spec40/item1", "foo")
+            self.assertRaises(isc.cc.data.DataTypeError, uccs.remove_value, "Spec40/item1[0]", None)
+
+        # A few helper functions to perform a number of tests
+        # (to repeat the same test for nested data)
+        def check_list(identifier):
+            for item in items_as_str:
+                uccs.add_value(identifier, item)
+            self.assertEqual((items, 1), uccs.get_value(identifier))
+
+            # Removing from list should work in both ways
+            uccs.remove_value(identifier, "foo")
+            uccs.remove_value(identifier + "[1]", None)
+            self.assertEqual(([1234, False], 1), uccs.get_value(identifier))
+
+            # As should item indexing
+            self.assertEqual((1234, 1), uccs.get_value(identifier + "[0]"))
+            self.assertEqual((False, 1), uccs.get_value(identifier + "[1]"))
+
+        def check_named_set(identifier):
+            for item in items_as_str:
+                # use string version as key as well
+                uccs.add_value(identifier, item, item)
+
+            self.assertEqual((1234, 1), uccs.get_value(identifier + "/1234"))
+            self.assertEqual((True, 1), uccs.get_value(identifier + "/true"))
+
+            for item in items_as_str:
+                # use string version as key as well
+                uccs.remove_value(identifier, item)
+
+
+        # should fail when set to value of primitive type
+        for item in items:
+            uccs.set_value("Spec40/item1", item)
+            test_fails()
+
+        # When set to list, add and remove should work, and its elements
+        # should be considered of type 'any' themselves.
+        uccs.set_value("Spec40/item1", [])
+        check_list("Spec40/item1")
+
+        # When set to dict, it should have the behaviour of a named set
+        uccs.set_value("Spec40/item1", {})
+        check_named_set("Spec40/item1")
+
+        # And, or course, we may need nesting.
+        uccs.set_value("Spec40/item1", { "foo": {}, "bar": [] })
+        check_named_set("Spec40/item1/foo")
+        check_list("Spec40/item1/bar")
+        uccs.set_value("Spec40/item1", [ {}, [] ] )
+        check_named_set("Spec40/item1[0]")
+        check_list("Spec40/item1[1]")
+        uccs.set_value("Spec40/item1", [[[[[[]]]]]] )
+        check_list("Spec40/item1[0][0][0][0][0]")
+        uccs.set_value("Spec40/item1", { 'a': { 'a': { 'a': {} } } } )
+        check_named_set("Spec40/item1/a/a/a")
+
     def test_add_dup_value(self):
         fake_conn = fakeUIConn()
         uccs = self.create_uccs_listtest(fake_conn)
@@ -1101,7 +1183,7 @@ class TestUIModuleCCSession(unittest.TestCase):
 
     def test_commit(self):
         fake_conn = fakeUIConn()
-        uccs = self.create_uccs2(fake_conn)
+        uccs = self.create_uccs(fake_conn)
         uccs.commit()
         uccs._local_changes = {'Spec2': {'item5': [ 'a' ]}}
         uccs.commit()