diff options
| -rw-r--r-- | gitlab/base.py | 2 | ||||
| -rw-r--r-- | gitlab/mixins.py | 9 | ||||
| -rw-r--r-- | gitlab/utils.py | 85 | ||||
| -rw-r--r-- | gitlab/v4/cli.py | 2 | ||||
| -rw-r--r-- | gitlab/v4/objects/features.py | 2 | ||||
| -rw-r--r-- | gitlab/v4/objects/files.py | 12 | ||||
| -rw-r--r-- | gitlab/v4/objects/repositories.py | 2 | ||||
| -rw-r--r-- | tests/functional/api/test_lazy_objects.py | 2 | ||||
| -rw-r--r-- | tests/unit/test_utils.py | 25 |
9 files changed, 28 insertions, 113 deletions
diff --git a/gitlab/base.py b/gitlab/base.py index 96e770c..0706ffb 100644 --- a/gitlab/base.py +++ b/gitlab/base.py @@ -223,7 +223,7 @@ class RESTObject(object): path""" obj_id = self.get_id() if isinstance(obj_id, str): - obj_id = gitlab.utils._url_encode(obj_id) + obj_id = gitlab.utils.EncodedId(obj_id) return obj_id @property diff --git a/gitlab/mixins.py b/gitlab/mixins.py index a6794d0..b79c29e 100644 --- a/gitlab/mixins.py +++ b/gitlab/mixins.py @@ -99,7 +99,8 @@ class GetMixin(_RestManagerBase): GitlabAuthenticationError: If authentication is not correct GitlabGetError: If the server cannot perform the request """ - id = utils._url_encode(id) + if isinstance(id, str): + id = utils.EncodedId(id) path = f"{self.path}/{id}" if TYPE_CHECKING: assert self._obj_cls is not None @@ -390,7 +391,7 @@ class UpdateMixin(_RestManagerBase): if id is None: path = self.path else: - path = f"{self.path}/{utils._url_encode(id)}" + path = f"{self.path}/{utils.EncodedId(id)}" self._check_missing_update_attrs(new_data) files = {} @@ -443,7 +444,7 @@ class SetMixin(_RestManagerBase): Returns: The created/updated attribute """ - path = f"{self.path}/{utils._url_encode(key)}" + path = f"{self.path}/{utils.EncodedId(key)}" data = {"value": value} server_data = self.gitlab.http_put(path, post_data=data, **kwargs) if TYPE_CHECKING: @@ -476,7 +477,7 @@ class DeleteMixin(_RestManagerBase): if id is None: path = self.path else: - path = f"{self.path}/{utils._url_encode(id)}" + path = f"{self.path}/{utils.EncodedId(id)}" self.gitlab.http_delete(path, **kwargs) diff --git a/gitlab/utils.py b/gitlab/utils.py index 61e98f3..8b3054c 100644 --- a/gitlab/utils.py +++ b/gitlab/utils.py @@ -16,7 +16,7 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. import urllib.parse -from typing import Any, Callable, Dict, Optional, overload, Union +from typing import Any, Callable, Dict, Optional, Union import requests @@ -71,83 +71,18 @@ class EncodedId(str): https://docs.gitlab.com/ee/api/index.html#path-parameters """ - # `original_str` will contain the original string value that was used to create the - # first instance of EncodedId. We will use this original value to generate the - # URL-encoded value each time. - original_str: str - - def __new__(cls, value: Union[str, int, "EncodedId"]) -> "EncodedId": - # __new__() gets called before __init__() - if isinstance(value, int): - value = str(value) - # Make sure isinstance() for `EncodedId` comes before check for `str` as - # `EncodedId` is an instance of `str` and would pass that check. - elif isinstance(value, EncodedId): - # We use the original string value to URL-encode - value = value.original_str - elif isinstance(value, str): - pass - else: - raise ValueError(f"Unsupported type received: {type(value)}") - # Set the value our string will return + # mypy complains if return type other than the class type. So we ignore issue. + def __new__( # type: ignore + cls, value: Union[str, int, "EncodedId"] + ) -> Union[int, "EncodedId"]: + if isinstance(value, (int, EncodedId)): + return value + + if not isinstance(value, str): + raise TypeError(f"Unsupported type received: {type(value)}") value = urllib.parse.quote(value, safe="") return super().__new__(cls, value) - def __init__(self, value: Union[int, str]) -> None: - # At this point `super().__str__()` returns the URL-encoded value. Which means - # when using this as a `str` it will return the URL-encoded value. - # - # But `value` contains the original value passed in `EncodedId(value)`. We use - # this to always keep the original string that was received so that no matter - # how many times we recurse we only URL-encode our original string once. - if isinstance(value, int): - value = str(value) - # Make sure isinstance() for `EncodedId` comes before check for `str` as - # `EncodedId` is an instance of `str` and would pass that check. - elif isinstance(value, EncodedId): - # This is the key part as we are always keeping the original string even - # through multiple recursions. - value = value.original_str - elif isinstance(value, str): - pass - else: - raise ValueError(f"Unsupported type received: {type(value)}") - self.original_str = value - super().__init__() - - -@overload -def _url_encode(id: int) -> int: - ... - - -@overload -def _url_encode(id: Union[str, EncodedId]) -> EncodedId: - ... - - -def _url_encode(id: Union[int, str, EncodedId]) -> Union[int, EncodedId]: - """Encode/quote the characters in the string so that they can be used in a path. - - Reference to documentation on why this is necessary. - - https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding - - If using namespaced API requests, make sure that the NAMESPACE/PROJECT_PATH is - URL-encoded. For example, / is represented by %2F - - https://docs.gitlab.com/ee/api/index.html#path-parameters - - Path parameters that are required to be URL-encoded must be followed. If not, it - doesn’t match an API endpoint and responds with a 404. If there’s something in front - of the API (for example, Apache), ensure that it doesn’t decode the URL-encoded path - parameters. - - """ - if isinstance(id, (int, EncodedId)): - return id - return EncodedId(id) - def remove_none_from_dict(data: Dict[str, Any]) -> Dict[str, Any]: return {k: v for k, v in data.items() if v is not None} diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py index a76b133..504b7a9 100644 --- a/gitlab/v4/cli.py +++ b/gitlab/v4/cli.py @@ -75,7 +75,7 @@ class GitlabCLI(object): if key not in self.args: continue - self.parent_args[key] = gitlab.utils._url_encode(self.args[key]) + self.parent_args[key] = gitlab.utils.EncodedId(self.args[key]) # If we don't delete it then it will be added to the URL as a query-string del self.args[key] diff --git a/gitlab/v4/objects/features.py b/gitlab/v4/objects/features.py index 69689fa..1631a26 100644 --- a/gitlab/v4/objects/features.py +++ b/gitlab/v4/objects/features.py @@ -52,7 +52,7 @@ class FeatureManager(ListMixin, DeleteMixin, RESTManager): Returns: The created/updated attribute """ - name = utils._url_encode(name) + name = utils.EncodedId(name) path = f"{self.path}/{name}" data = { "value": value, diff --git a/gitlab/v4/objects/files.py b/gitlab/v4/objects/files.py index 644c017..0a56fef 100644 --- a/gitlab/v4/objects/files.py +++ b/gitlab/v4/objects/files.py @@ -56,7 +56,7 @@ class ProjectFile(SaveMixin, ObjectDeleteMixin, RESTObject): """ self.branch = branch self.commit_message = commit_message - self.file_path = utils._url_encode(self.file_path) + self.file_path = utils.EncodedId(self.file_path) super(ProjectFile, self).save(**kwargs) @exc.on_http_error(exc.GitlabDeleteError) @@ -144,7 +144,7 @@ class ProjectFileManager(GetMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa assert data is not None self._check_missing_create_attrs(data) new_data = data.copy() - file_path = utils._url_encode(new_data.pop("file_path")) + file_path = utils.EncodedId(new_data.pop("file_path")) path = f"{self.path}/{file_path}" server_data = self.gitlab.http_post(path, post_data=new_data, **kwargs) if TYPE_CHECKING: @@ -173,7 +173,7 @@ class ProjectFileManager(GetMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa """ new_data = new_data or {} data = new_data.copy() - file_path = utils._url_encode(file_path) + file_path = utils.EncodedId(file_path) data["file_path"] = file_path path = f"{self.path}/{file_path}" self._check_missing_update_attrs(data) @@ -203,7 +203,7 @@ class ProjectFileManager(GetMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa GitlabAuthenticationError: If authentication is not correct GitlabDeleteError: If the server cannot perform the request """ - file_path = utils._url_encode(file_path) + file_path = utils.EncodedId(file_path) path = f"{self.path}/{file_path}" data = {"branch": branch, "commit_message": commit_message} self.gitlab.http_delete(path, query_data=data, **kwargs) @@ -239,7 +239,7 @@ class ProjectFileManager(GetMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa Returns: The file content """ - file_path = utils._url_encode(file_path) + file_path = utils.EncodedId(file_path) path = f"{self.path}/{file_path}/raw" query_data = {"ref": ref} result = self.gitlab.http_get( @@ -266,7 +266,7 @@ class ProjectFileManager(GetMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa Returns: A list of commits/lines matching the file """ - file_path = utils._url_encode(file_path) + file_path = utils.EncodedId(file_path) path = f"{self.path}/{file_path}/blame" query_data = {"ref": ref} result = self.gitlab.http_list(path, query_data, **kwargs) diff --git a/gitlab/v4/objects/repositories.py b/gitlab/v4/objects/repositories.py index ca70b5b..4e8169f 100644 --- a/gitlab/v4/objects/repositories.py +++ b/gitlab/v4/objects/repositories.py @@ -39,7 +39,7 @@ class RepositoryMixin(_RestObjectBase): GitlabPutError: If the submodule could not be updated """ - submodule = utils._url_encode(submodule) + submodule = utils.EncodedId(submodule) path = f"/projects/{self.encoded_id}/repository/submodules/{submodule}" data = {"branch": branch, "commit_sha": commit_sha} if "commit_message" in kwargs: diff --git a/tests/functional/api/test_lazy_objects.py b/tests/functional/api/test_lazy_objects.py index 3db7a60..78ade80 100644 --- a/tests/functional/api/test_lazy_objects.py +++ b/tests/functional/api/test_lazy_objects.py @@ -12,7 +12,7 @@ def lazy_project(gl, project): def test_lazy_id(project, lazy_project): assert isinstance(lazy_project.id, str) assert isinstance(lazy_project.id, gitlab.utils.EncodedId) - assert lazy_project.id == gitlab.utils._url_encode(project.path_with_namespace) + assert lazy_project.id == gitlab.utils.EncodedId(project.path_with_namespace) def test_refresh_after_lazy_get_with_path(project, lazy_project): diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index cccab9d..9f90983 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -20,28 +20,10 @@ import json from gitlab import utils -def test_url_encode(): - src = "nothing_special" - dest = "nothing_special" - assert dest == utils._url_encode(src) - - src = "foo#bar/baz/" - dest = "foo%23bar%2Fbaz%2F" - assert dest == utils._url_encode(src) - - src = "foo%bar/baz/" - dest = "foo%25bar%2Fbaz%2F" - assert dest == utils._url_encode(src) - - # periods/dots should not be modified - src = "docs/README.md" - dest = "docs%2FREADME.md" - assert dest == utils._url_encode(src) - - class TestEncodedId: def test_init_str(self): obj = utils.EncodedId("Hello") + assert "Hello" == obj assert "Hello" == str(obj) assert "Hello" == f"{obj}" @@ -51,6 +33,7 @@ class TestEncodedId: def test_init_int(self): obj = utils.EncodedId(23) + assert 23 == obj assert "23" == str(obj) assert "23" == f"{obj}" @@ -60,12 +43,10 @@ class TestEncodedId: obj = utils.EncodedId(obj_init) assert value == str(obj) assert value == f"{obj}" - assert value == obj.original_str value = "we got/a/path" expected = "we%20got%2Fa%2Fpath" obj_init = utils.EncodedId(value) - assert value == obj_init.original_str assert expected == str(obj_init) assert expected == f"{obj_init}" # Show that no matter how many times we recursively call it we still only @@ -75,8 +56,6 @@ class TestEncodedId: ) assert expected == str(obj) assert expected == f"{obj}" - # We have stored a copy of our original string - assert value == obj.original_str # Show assignments still only encode once obj2 = obj |