Skip to content

Commit

Permalink
chore: replace usage of utils._url_encode() with utils.EncodedId()
Browse files Browse the repository at this point in the history
utils.EncodedId() has basically the same functionalityy of using
utils._url_encode(). So remove utils._url_encode() as we don't need
it.
  • Loading branch information
JohnVillalovos committed Jan 13, 2022
1 parent a2e7c38 commit b07eece
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 113 deletions.
2 changes: 1 addition & 1 deletion gitlab/base.py
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions gitlab/mixins.py
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)


Expand Down
85 changes: 10 additions & 75 deletions gitlab/utils.py
Expand Up @@ -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

Expand Down Expand Up @@ -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}
2 changes: 1 addition & 1 deletion gitlab/v4/cli.py
Expand Up @@ -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]

Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/objects/features.py
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions gitlab/v4/objects/files.py
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/objects/repositories.py
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/api/test_lazy_objects.py
Expand Up @@ -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):
Expand Down
25 changes: 2 additions & 23 deletions tests/unit/test_utils.py
Expand Up @@ -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}"

Expand All @@ -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}"

Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit b07eece

Please sign in to comment.