fix: Fixes refresh() for attrib style params

panos/base.py

@@ -686,6 +686,8 @@ def update(self, variable):
         )
         device.set_config_changed()
         path, value, var_path = self._get_param_specific_info(variable)
+        if var_path.vartype == "attrib":
+            raise NotImplementedError("Cannot update 'attrib' style params")
         xpath = "{0}/{1}".format(self.xpath(), path)
 
         if value is None:
@@ -2475,6 +2477,11 @@ def element(self, with_children=True, comparable=False):
         self.xml_merge(ans, itertools.chain(*iterchain))
 
         # Now that the whole element is built, mixin an attrib vartypes.
+        #
+        # We do this here instead of in xml_merge() because attributes are considered
+        # part of the identity in that function, and I'm not sure we want to manage
+        # a list of what attributes are considered part of an element's identity and
+        # what should be mixed in.
         for p in paths:
             if p.vartype != "attrib":
                 continue
@@ -2484,24 +2491,31 @@ def element(self, with_children=True, comparable=False):
             if attrib_value is None or p.exclude:
                 continue
             e = ans
-            find_path = [
-                ".",
-            ]
             for ap in attrib_path:
                 if not ap:
                     continue
+                finder = None
+                tag = None
+                attribs = {}
                 if ap.startswith("entry "):
                     junk, var_to_use = ap.split()
                     sol_value = panos.string_or_list(settings[var_to_use])[0]
-                    find_path.append("entry[@name='{0}']".format(sol_val))
+                    finder = "entry[@name='{0}']".format(sol_value)
+                    tag = "entry"
+                    attribs["name"] = sol_value
                 elif ap == "entry[@name='localhost.localdomain']":
-                    find_path.append(ap)
+                    finder = ap
+                    tag = "entry"
+                    attribs["name"] = "localhost.localdomain"
                 else:
-                    find_path.append(ap.format(**settings))
-            if len(find_path) > 1:
-                e = e.find("/".join(find_path))
-            if e is not None:
-                e.attrib[attrib_name] = attrib_value
+                    finder = ap.format(**settings)
+                    tag = finder
+                e2 = e.find("./{0}".format(finder))
+                if e2 is None:
+                    e = ET.SubElement(e, tag, attribs)
+                else:
+                    e = e2
+            e.attrib[attrib_name] = attrib_value
 
         return ans
 
@@ -2842,6 +2856,8 @@ def _set_inner_xml_tag_text(self, elm, value, comparable=False):
         elif self.vartype == "none":
             # There is no variable, so don't try to populate it
             pass
+        elif self.vartype == "attrib":
+            raise ValueError("attrib not yet supported for classic objects")
         else:
             elm.text = str(value)
 
@@ -2945,10 +2961,13 @@ def element(self, elm, settings, comparable=False):
                 return None
 
         e = elm
+        attr = None
         # Build the element
         tokens = self.path.split("/")
         if self.vartype == "exist":
             del tokens[-1]
+        elif self.vartype == "attrib":
+            attr = tokens.pop()
         for token in tokens:
             if not token:
                 continue
@@ -2965,7 +2984,7 @@ def element(self, elm, settings, comparable=False):
             e.append(child)
             e = child
 
-        self._set_inner_xml_tag_text(e, value, comparable)
+        self._set_inner_xml_tag_text(e, value, comparable, attr)
 
         return elm
 
@@ -3013,10 +3032,13 @@ def parse_xml(self, xml, settings, possibilities):
                 # thus not needed
                 return None
 
+        attr = None
         e = xml
         tokens = self.path.split("/")
         if self.vartype == "exist":
             del tokens[-1]
+        elif self.vartype == "attrib":
+            attr = tokens.pop()
         for p in tokens:
             # Skip this path part if there is no path part
             if not p:
@@ -3063,15 +3085,16 @@ def parse_xml(self, xml, settings, possibilities):
             e = ans
 
         # Pull the value, properly formatted, from this last element
-        self.parse_value_from_xml_last_tag(e, settings)
+        self.parse_value_from_xml_last_tag(e, settings, attr)
 
-    def _set_inner_xml_tag_text(self, elm, value, comparable=False):
+    def _set_inner_xml_tag_text(self, elm, value, comparable=False, attr=None):
         """Sets the final elm's .text as appropriate given the vartype.
 
         Args:
             elm (xml.etree.ElementTree.Element): The element whose .text to set.
             value (various): The value to put in the .text, conforming to the vartype of this parameter.
             comparable (bool): Make updates for element string comparisons.  For encrypted fields, if the text should be set to a password hash (True) or left as a basestring (False).  For entry and member vartypes, sort the entries (True) or leave them as-is (False).
+            attr (str): For `vartype="attrib"`, the attribute name.
 
         """
         # Format the element text appropriately
@@ -3102,10 +3125,12 @@ def _set_inner_xml_tag_text(self, elm, value, comparable=False):
             elm.text = str(int(value))
         elif self.vartype == "encrypted" and comparable:
             elm.text = self._sha1_hash(str(value))
+        elif self.vartype == "attrib":
+            elm.attrib[attr] = value
         else:
             elm.text = str(value)
 
-    def parse_value_from_xml_last_tag(self, elm, settings):
+    def parse_value_from_xml_last_tag(self, elm, settings, attr):
         """Actually do the parsing for this parameter.
 
         The value parsed is saved into the ``settings`` dict.
@@ -3115,6 +3140,7 @@ def parse_value_from_xml_last_tag(self, elm, settings):
                 document passed in to ``parse_xml()`` that contains the actual
                 value to parse out for this parameter.
             settings (dict): The dict where the parsed value will be saved.
+            attr (str): For `vartype="attrib"`, the attribute name.
 
         Raises:
             ValueError: If a param is in an incorrect format.
@@ -3143,6 +3169,8 @@ def parse_value_from_xml_last_tag(self, elm, settings):
             pass
         elif self.vartype == "int":
             settings[self.param] = int(elm.text)
+        elif self.vartype == "attrib":
+            settings[self.param] = elm.attrib.get(attr, None)
         else:
             settings[self.param] = elm.text
 

tests/test_params.py

@@ -0,0 +1,212 @@
+import pytest
+import xml.etree.ElementTree as ET
+
+from panos.base import ENTRY, Root
+from panos.base import VersionedPanObject, VersionedParamPath
+from panos.firewall import Firewall
+
+
+class FakeObject(VersionedPanObject):
+    """Fake object for testing."""
+
+    SUFFIX = ENTRY
+    ROOT = Root.VSYS
+
+    def _setup(self):
+        self._xpaths.add_profile(value="/fake")
+
+        params = []
+
+        params.append(VersionedParamPath("uuid", vartype="attrib", path="uuid",),)
+        params.append(VersionedParamPath("size", vartype="int", path="size",),)
+        params.append(VersionedParamPath("listing", vartype="member", path="listing",),)
+        params.append(VersionedParamPath("pb1", vartype="exist", path="pb1",),)
+        params.append(VersionedParamPath("pb2", vartype="exist", path="pb2",),)
+        params.append(VersionedParamPath("live", vartype="yesno", path="live",),)
+        params.append(
+            VersionedParamPath("disabled", vartype="yesno", path="disabled",),
+        )
+        params.append(
+            VersionedParamPath("uuid2", vartype="attrib", path="level-2/uuid",),
+        )
+        params.append(VersionedParamPath("age", vartype="int", path="level-2/age",),)
+        params.append(
+            VersionedParamPath(
+                "interfaces", vartype="member", path="level-2/interface",
+            ),
+        )
+
+        self._params = tuple(params)
+
+
+def _verify_render(o, expected):
+    ans = o.element_str().decode("utf-8")
+
+    assert ans == expected
+
+
+def _refreshed_object():
+    fw = Firewall("127.0.0.1", "admin", "admin", "secret")
+    fw._version_info = (9999, 0, 0)
+
+    o = FakeObject()
+    fw.add(o)
+
+    o = o.refreshall_from_xml(_refresh_xml())[0]
+
+    return o
+
+
+def _refresh_xml():
+    return ET.fromstring(
+        """
+<result>
+    <entry name="test" uuid="123-456">
+        <size>5</size>
+        <listing>
+            <member>first</member>
+            <member>second</member>
+        </listing>
+        <pb1 />
+        <disabled>yes</disabled>
+        <level-2 uuid="456-789">
+            <age>12</age>
+            <interface>
+                <member>third</member>
+                <member>fourth</member>
+            </interface>
+        </level-2>
+    </entry>
+</result>"""
+    )
+
+
+# int at base level
+def test_render_int():
+    _verify_render(
+        FakeObject("test", size=5), '<entry name="test"><size>5</size></entry>',
+    )
+
+
+def test_parse_int():
+    o = _refreshed_object()
+
+    assert o.size == 5
+
+
+# member list at base level
+def test_render_member():
+    _verify_render(
+        FakeObject("test", listing=["one", "two"]),
+        '<entry name="test"><listing><member>one</member><member>two</member></listing></entry>',
+    )
+
+
+def test_parse_member():
+    o = _refreshed_object()
+
+    assert o.listing == ["first", "second"]
+
+
+# exist at base level
+def test_render_exist():
+    _verify_render(
+        FakeObject("test", pb1=True), '<entry name="test"><pb1 /></entry>',
+    )
+
+
+def test_parse_exists():
+    o = _refreshed_object()
+
+    assert o.pb1
+    assert not o.pb2
+
+
+# yesno at base level
+def test_render_yesno():
+    _verify_render(
+        FakeObject("test", disabled=True),
+        '<entry name="test"><disabled>yes</disabled></entry>',
+    )
+
+
+def test_parse_yesno():
+    o = _refreshed_object()
+
+    assert o.disabled
+
+
+# attrib
+def test_render_attrib():
+    _verify_render(
+        FakeObject("test", uuid="123-456"), '<entry name="test" uuid="123-456" />',
+    )
+
+
+def test_parse_attrib():
+    o = _refreshed_object()
+
+    assert o.uuid == "123-456"
+
+
+# int at depth 1
+def test_render_d1_int():
+    _verify_render(
+        FakeObject("test", age=12),
+        '<entry name="test"><level-2><age>12</age></level-2></entry>',
+    )
+
+
+def test_parse_d1_int():
+    o = _refreshed_object()
+
+    assert o.age == 12
+
+
+# member list at depth 1
+def test_render_d1_member():
+    _verify_render(
+        FakeObject("test", interfaces=["third", "fourth"]),
+        "".join(
+            [
+                '<entry name="test"><level-2>',
+                "<interface><member>third</member><member>fourth</member></interface>",
+                "</level-2></entry>",
+            ]
+        ),
+    )
+
+
+def test_parse_d1_member():
+    o = _refreshed_object()
+
+    assert o.interfaces == ["third", "fourth"]
+
+
+# uuid at depth 1
+def test_render_d1_attrib_standalone():
+    _verify_render(
+        FakeObject("test", uuid2="456-789"),
+        '<entry name="test"><level-2 uuid="456-789" /></entry>',
+    )
+
+
+def test_render_d1_attrib_mixed():
+    _verify_render(
+        FakeObject("test", uuid2="456-789", age=12),
+        '<entry name="test"><level-2 uuid="456-789"><age>12</age></level-2></entry>',
+    )
+
+
+def test_parse_d1_attrib():
+    o = _refreshed_object()
+
+    assert o.uuid2 == "456-789"
+
+
+# should raise an exception
+def test_update_attrib_raises_not_implemented_exception():
+    o = _refreshed_object()
+
+    with pytest.raises(NotImplementedError):
+        o.update("uuid")