Merge pull request #127 from wiegandm/modify_tag_gmp8

Adapt modify_tag to implementation
greenbone · May 7, 2019 · 181982d · 181982d
2 parents a6e713a + b537a71
commit 181982d
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   versions [PR 113](https://github.com/greenbone/python-gvm/pull/113)
 * Make resource_id optional when creating tags (Gmpv7) [PR 124](https://github.com/greenbone/python-gvm/pull/124)
 * Allow creating tags without resource (Gmpv8) [PR 125](https://github.com/greenbone/python-gvm/pull/125)
+* Adapt modify_tag validation to actual implementation (Gmpv8) [PR 127](https://github.com/greenbone/python-gvm/pull/127)
 
 ### Removed
 * Removed hosts_ordering argument from `modify_target` [PR 88](https://github.com/greenbone/python-gvm/pull/88)

diff --git a/gvm/protocols/gmpv8.py b/gvm/protocols/gmpv8.py
@@ -491,13 +491,11 @@ def modify_tag(
                 'set' or 'remove'.
             resource_type (str, optional): Type of the resources to
                 which to attach the tag. Required if resource_filter
-                or resource_ids is set.
+                is set.
             resource_filter (str, optional) Filter term to select
-                resources the tag is to be attached to. Required if
-                resource_type is set unless resource_ids is set.
+                resources the tag is to be attached to.
             resource_ids (list, optional): IDs of the resources to
-                which to attach the tag. Required if resource_type is
-                set unless resource_filter is set.
+                which to attach the tag.
 
         Returns:
             The response. See :py:meth:`send_command` for details.
@@ -521,17 +519,10 @@ def modify_tag(
             cmd.add_element("active", _to_bool(active))
 
         if resource_action or resource_filter or resource_ids or resource_type:
-            if not resource_filter and not resource_ids:
-                raise RequiredArgument(
-                    "modify_tag requires resource_filter or resource_ids "
-                    "argument when resource_action or resource_type is set"
-                )
-
-            if not resource_type:
+            if resource_filter and not resource_type:
                 raise RequiredArgument(
                     "modify_tag requires resource_type argument when "
-                    "resource_action or resource_filter or resource_ids "
-                    "is set"
+                    "resource_filter is set"
                 )
 
             _xmlresources = cmd.add_element("resources")
@@ -546,6 +537,7 @@ def modify_tag(
                     "resource", attrs={"id": str(resource_id)}
                 )
 
-            _xmlresources.add_element("type", resource_type)
+            if resource_type is not None:
+                _xmlresources.add_element("type", resource_type)
 
         return self._send_xml_command(cmd)
diff --git a/tests/protocols/gmpv8/test_modify_tag.py b/tests/protocols/gmpv8/test_modify_tag.py
@@ -45,44 +45,34 @@ def test_modify_tag_with_comment(self):
         self.gmp.modify_tag(tag_id='t1', comment='foo')
 
         self.connection.send.has_been_called_with(
-            '<modify_tag tag_id="t1">'
-            '<comment>foo</comment>'
-            '</modify_tag>'
+            '<modify_tag tag_id="t1"><comment>foo</comment></modify_tag>'
         )
 
     def test_modify_tag_with_value(self):
         self.gmp.modify_tag(tag_id='t1', value='foo')
 
         self.connection.send.has_been_called_with(
-            '<modify_tag tag_id="t1">'
-            '<value>foo</value>'
-            '</modify_tag>'
+            '<modify_tag tag_id="t1"><value>foo</value></modify_tag>'
         )
 
     def test_modify_tag_with_name(self):
         self.gmp.modify_tag(tag_id='t1', name='foo')
 
         self.connection.send.has_been_called_with(
-            '<modify_tag tag_id="t1">'
-            '<name>foo</name>'
-            '</modify_tag>'
+            '<modify_tag tag_id="t1"><name>foo</name></modify_tag>'
         )
 
     def test_modify_tag_with_active(self):
         self.gmp.modify_tag(tag_id='t1', active=True)
 
         self.connection.send.has_been_called_with(
-            '<modify_tag tag_id="t1">'
-            '<active>1</active>'
-            '</modify_tag>'
+            '<modify_tag tag_id="t1"><active>1</active></modify_tag>'
         )
 
         self.gmp.modify_tag(tag_id='t1', active=False)
 
         self.connection.send.has_been_called_with(
-            '<modify_tag tag_id="t1">'
-            '<active>0</active>'
-            '</modify_tag>'
+            '<modify_tag tag_id="t1"><active>0</active></modify_tag>'
         )
 
     def test_modify_tag_with_resource_filter_and_type(self):
@@ -103,7 +93,7 @@ def test_modify_tag_with_resource_action_filter_and_type(self):
             tag_id='t1',
             resource_action='set',
             resource_filter='name=foo',
-            resource_type='task'
+            resource_type='task',
         )
 
         self.connection.send.has_been_called_with(
@@ -116,9 +106,7 @@ def test_modify_tag_with_resource_action_filter_and_type(self):
 
     def test_modify_tag_with_resource_ids_and_type(self):
         self.gmp.modify_tag(
-            tag_id='t1',
-            resource_ids=['r1'],
-            resource_type='task'
+            tag_id='t1', resource_ids=['r1'], resource_type='task'
         )
 
         self.connection.send.has_been_called_with(
@@ -135,7 +123,7 @@ def test_modify_tag_with_resource_action_ids_and_type(self):
             tag_id='t1',
             resource_action="set",
             resource_ids=['r1'],
-            resource_type='task'
+            resource_type='task',
         )
 
         self.connection.send.has_been_called_with(
@@ -148,19 +136,38 @@ def test_modify_tag_with_resource_action_ids_and_type(self):
         )
 
     def test_modify_tag_with_missing_resource_filter_or_ids_andtype(self):
-        with self.assertRaises(RequiredArgument):
-            self.gmp.modify_tag(tag_id='t1', resource_action='add')
+        self.gmp.modify_tag(tag_id='t1', resource_action='add')
+
+        self.connection.send.has_been_called_with(
+            '<modify_tag tag_id="t1">'
+            '<resources action="add"/>'
+            '</modify_tag>'
+        )
 
     def test_modify_tag_with_missing_resource_type(self):
-        with self.assertRaises(RequiredArgument):
-            self.gmp.modify_tag(tag_id='t1', resource_ids=['r1'])
+        self.gmp.modify_tag(tag_id='t1', resource_ids=['r1'])
+
+        self.connection.send.has_been_called_with(
+            '<modify_tag tag_id="t1">'
+            '<resources>'
+            '<resource id="r1"/>'
+            '</resources>'
+            '</modify_tag>'
+        )
 
         with self.assertRaises(RequiredArgument):
             self.gmp.modify_tag(tag_id='t1', resource_filter='name=foo')
 
     def test_modify_tag_with_missing_resource_filter_and_ids(self):
-        with self.assertRaises(RequiredArgument):
-            self.gmp.modify_tag(tag_id='t1', resource_type='task')
+        self.gmp.modify_tag(tag_id='t1', resource_type='task')
+
+        self.connection.send.has_been_called_with(
+            '<modify_tag tag_id="t1">'
+            '<resources>'
+            '<type>task</type>'
+            '</resources>'
+            '</modify_tag>'
+        )
 
 
 if __name__ == '__main__':