feat: support mutually exclusive attributes and consolidate validation to fix board lists (#2037)

walterrowe · web-flow · commit 3fa330cc341b · 2022-05-31T21:36:14.000+02:00
add exclusive tuple to RequiredOptional data class to support for mutually exclusive attributes consolidate _check_missing_create_attrs and _check_missing_update_attrs from mixins.py into _validate_attrs in utils.py change _create_attrs in board list manager classes from required=('label_ld',) to exclusive=('label_id','asignee_id','milestone_id') closes #1897
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
@@ -256,15 +256,6 @@ class CreateMixin(_RestManagerBase):
     _path: Optional[str]
     gitlab: gitlab.Gitlab
 
-    def _check_missing_create_attrs(self, data: Dict[str, Any]) -> None:
-        missing = []
-        for attr in self._create_attrs.required:
-            if attr not in data:
-                missing.append(attr)
-                continue
-        if missing:
-            raise AttributeError(f"Missing attributes: {', '.join(missing)}")
-
     @exc.on_http_error(exc.GitlabCreateError)
     def create(
         self, data: Optional[Dict[str, Any]] = None, **kwargs: Any
@@ -287,7 +278,7 @@ def create(
         if data is None:
             data = {}
 
-        self._check_missing_create_attrs(data)
+        utils._validate_attrs(data=data, attributes=self._create_attrs)
         data, files = utils._transform_types(data, self._types)
 
         # Handle specific URL for creation
@@ -309,22 +300,6 @@ class UpdateMixin(_RestManagerBase):
     _update_uses_post: bool = False
     gitlab: gitlab.Gitlab
 
-    def _check_missing_update_attrs(self, data: Dict[str, Any]) -> None:
-        if TYPE_CHECKING:
-            assert self._obj_cls is not None
-        # Remove the id field from the required list as it was previously moved
-        # to the http path.
-        required = tuple(
-            [k for k in self._update_attrs.required if k != self._obj_cls._id_attr]
-        )
-        missing = []
-        for attr in required:
-            if attr not in data:
-                missing.append(attr)
-                continue
-        if missing:
-            raise AttributeError(f"Missing attributes: {', '.join(missing)}")
-
     def _get_update_method(
         self,
     ) -> Callable[..., Union[Dict[str, Any], requests.Response]]:
@@ -367,7 +342,12 @@ def update(
         else:
             path = f"{self.path}/{utils.EncodedId(id)}"
 
-        self._check_missing_update_attrs(new_data)
+        excludes = []
+        if self._obj_cls is not None and self._obj_cls._id_attr is not None:
+            excludes = [self._obj_cls._id_attr]
+        utils._validate_attrs(
+            data=new_data, attributes=self._update_attrs, excludes=excludes
+        )
         new_data, files = utils._transform_types(new_data, self._types)
 
         http_method = self._get_update_method()
diff --git a/gitlab/types.py b/gitlab/types.py
@@ -23,6 +23,7 @@
 class RequiredOptional:
     required: Tuple[str, ...] = ()
     optional: Tuple[str, ...] = ()
+    exclusive: Tuple[str, ...] = ()
 
 
 class GitlabAttribute:
diff --git a/gitlab/utils.py b/gitlab/utils.py
@@ -19,7 +19,7 @@
 import traceback
 import urllib.parse
 import warnings
-from typing import Any, Callable, Dict, Optional, Tuple, Type, Union
+from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union
 
 import requests
 
@@ -161,3 +161,30 @@ def warn(
         stacklevel=stacklevel,
         source=source,
     )
+
+
+def _validate_attrs(
+    data: Dict[str, Any],
+    attributes: types.RequiredOptional,
+    excludes: Optional[List[str]] = None,
+) -> None:
+    if excludes is None:
+        excludes = []
+
+    if attributes.required:
+        required = [k for k in attributes.required if k not in excludes]
+        missing = [attr for attr in required if attr not in data]
+        if missing:
+            raise AttributeError(f"Missing attributes: {', '.join(missing)}")
+
+    if attributes.exclusive:
+        exclusives = [attr for attr in data if attr in attributes.exclusive]
+        if len(exclusives) > 1:
+            raise AttributeError(
+                f"Provide only one of these attributes: {', '.join(exclusives)}"
+            )
+        if not exclusives:
+            raise AttributeError(
+                "Must provide one of these attributes: %(attrs)s"
+                % {"attrs": ", ".join(attributes.exclusive)}
+            )
diff --git a/gitlab/v4/objects/boards.py b/gitlab/v4/objects/boards.py
@@ -24,7 +24,9 @@ class GroupBoardListManager(CRUDMixin, RESTManager):
     _path = "/groups/{group_id}/boards/{board_id}/lists"
     _obj_cls = GroupBoardList
     _from_parent_attrs = {"group_id": "group_id", "board_id": "id"}
-    _create_attrs = RequiredOptional(required=("label_id",))
+    _create_attrs = RequiredOptional(
+        exclusive=("label_id", "assignee_id", "milestone_id")
+    )
     _update_attrs = RequiredOptional(required=("position",))
 
     def get(
@@ -55,7 +57,9 @@ class ProjectBoardListManager(CRUDMixin, RESTManager):
     _path = "/projects/{project_id}/boards/{board_id}/lists"
     _obj_cls = ProjectBoardList
     _from_parent_attrs = {"project_id": "project_id", "board_id": "id"}
-    _create_attrs = RequiredOptional(required=("label_id",))
+    _create_attrs = RequiredOptional(
+        exclusive=("label_id", "assignee_id", "milestone_id")
+    )
     _update_attrs = RequiredOptional(required=("position",))
 
     def get(
diff --git a/gitlab/v4/objects/epics.py b/gitlab/v4/objects/epics.py
@@ -1,7 +1,7 @@
 from typing import Any, cast, Dict, Optional, TYPE_CHECKING, Union
 
 from gitlab import exceptions as exc
-from gitlab import types
+from gitlab import types, utils
 from gitlab.base import RESTManager, RESTObject
 from gitlab.mixins import (
     CreateMixin,
@@ -107,7 +107,7 @@ def create(
         """
         if TYPE_CHECKING:
             assert data is not None
-        CreateMixin._check_missing_create_attrs(self, data)
+        utils._validate_attrs(data=data, attributes=self._create_attrs)
         path = f"{self.path}/{data.pop('issue_id')}"
         server_data = self.gitlab.http_post(path, **kwargs)
         if TYPE_CHECKING:
diff --git a/gitlab/v4/objects/files.py b/gitlab/v4/objects/files.py
@@ -145,7 +145,7 @@ def create(
 
         if TYPE_CHECKING:
             assert data is not None
-        self._check_missing_create_attrs(data)
+        utils._validate_attrs(data=data, attributes=self._create_attrs)
         new_data = data.copy()
         file_path = utils.EncodedId(new_data.pop("file_path"))
         path = f"{self.path}/{file_path}"
@@ -179,7 +179,7 @@ def update(  # type: ignore
         file_path = utils.EncodedId(file_path)
         data["file_path"] = file_path
         path = f"{self.path}/{file_path}"
-        self._check_missing_update_attrs(data)
+        utils._validate_attrs(data=data, attributes=self._update_attrs)
         result = self.gitlab.http_put(path, post_data=data, **kwargs)
         if TYPE_CHECKING:
             assert isinstance(result, dict)
diff --git a/gitlab/v4/objects/issues.py b/gitlab/v4/objects/issues.py
@@ -2,7 +2,7 @@
 
 from gitlab import cli
 from gitlab import exceptions as exc
-from gitlab import types
+from gitlab import types, utils
 from gitlab.base import RESTManager, RESTObject
 from gitlab.mixins import (
     CreateMixin,
@@ -272,7 +272,7 @@ def create(  # type: ignore
             GitlabAuthenticationError: If authentication is not correct
             GitlabCreateError: If the server cannot perform the request
         """
-        self._check_missing_create_attrs(data)
+        utils._validate_attrs(data=data, attributes=self._create_attrs)
         if TYPE_CHECKING:
             assert self.path is not None
         server_data = self.gitlab.http_post(self.path, post_data=data, **kwargs)
diff --git a/tests/unit/mixins/test_mixin_methods.py b/tests/unit/mixins/test_mixin_methods.py
@@ -3,6 +3,7 @@
 
 from gitlab import base
 from gitlab import types as gl_types
+from gitlab import utils
 from gitlab.mixins import (
     CreateMixin,
     DeleteMixin,
@@ -173,11 +174,11 @@ class M(CreateMixin, FakeManager):
 
     mgr = M(gl)
     data = {"foo": "bar", "baz": "blah"}
-    mgr._check_missing_create_attrs(data)
+    utils._validate_attrs(data=data, attributes=mgr._create_attrs)
 
     data = {"baz": "blah"}
     with pytest.raises(AttributeError) as error:
-        mgr._check_missing_create_attrs(data)
+        utils._validate_attrs(data=data, attributes=mgr._create_attrs)
     assert "foo" in str(error.value)
 
 
@@ -239,11 +240,11 @@ class M(UpdateMixin, FakeManager):
 
     mgr = M(gl)
     data = {"foo": "bar", "baz": "blah"}
-    mgr._check_missing_update_attrs(data)
+    utils._validate_attrs(data=data, attributes=mgr._update_attrs)
 
     data = {"baz": "blah"}
     with pytest.raises(AttributeError) as error:
-        mgr._check_missing_update_attrs(data)
+        utils._validate_attrs(data=data, attributes=mgr._update_attrs)
     assert "foo" in str(error.value)
 
 
