chore: replace usage of utils._url_encode() with utils.EncodedId()

JohnVillalovos · JohnVillalovos · commit b07eece0a35d · 2022-01-13T11:17:40.000-08:00
utils.EncodedId() has basically the same functionalityy of using
utils._url_encode(). So remove utils._url_encode() as we don't need
it.
diff --git a/gitlab/base.py b/gitlab/base.py
@@ -223,7 +223,7 @@ def encoded_id(self) -> Any:
         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
@@ -99,7 +99,8 @@ def get(
             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 @@ def update(
         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 @@ def set(self, key: str, value: str, **kwargs: Any) -> base.RESTObject:
         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 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
         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
@@ -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
@@ -75,7 +75,7 @@ def _process_from_parent_attrs(self) -> None:
             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
@@ -52,7 +52,7 @@ def set(
         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
@@ -56,7 +56,7 @@ def save(  # type: ignore
         """
         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 @@ def create(
             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 @@ def update(  # type: ignore
         """
         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 @@ def delete(  # type: ignore
             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 @@ def raw(
         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 @@ def blame(self, file_path: str, ref: str, **kwargs: Any) -> List[Dict[str, Any]]
         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
@@ -39,7 +39,7 @@ def update_submodule(
             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
@@ -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
@@ -20,28 +20,10 @@
 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 @@ def test_init_str(self):
 
     def test_init_int(self):
         obj = utils.EncodedId(23)
+        assert 23 == obj
         assert "23" == str(obj)
         assert "23" == f"{obj}"
 
@@ -60,12 +43,10 @@ def test_init_encodeid_str(self):
         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 @@ def test_init_encodeid_str(self):
         )
         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
