Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a2e7c38

Browse filesBrowse files
JohnVillalovosJohnVillalovos
committed
chore: add EncodedId string class to use to hold URL-encoded paths
Add EncodedId string class. This class returns a URL-encoded string but ensures it will only URL-encode it once even if recursively called. Also added some functional tests of 'lazy' objects to make sure they work.
1 parent 12435d7 commit a2e7c38
Copy full SHA for a2e7c38

File tree

Expand file treeCollapse file tree

9 files changed

+180
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+180
-11
lines changed
Open diff view settings
Collapse file

‎gitlab/mixins.py‎

Copy file name to clipboardExpand all lines: gitlab/mixins.py
+2-4Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,7 @@ def save(self, **kwargs: Any) -> None:
542542
return
543543

544544
# call the manager
545-
# Don't use `self.encoded_id` here as `self.manager.update()` will encode it.
546-
obj_id = self.get_id()
545+
obj_id = self.encoded_id
547546
if TYPE_CHECKING:
548547
assert isinstance(self.manager, UpdateMixin)
549548
server_data = self.manager.update(obj_id, updated_data, **kwargs)
@@ -573,8 +572,7 @@ def delete(self, **kwargs: Any) -> None:
573572
"""
574573
if TYPE_CHECKING:
575574
assert isinstance(self.manager, DeleteMixin)
576-
# Don't use `self.encoded_id` here as `self.manager.delete()` will encode it.
577-
self.manager.delete(self.get_id(), **kwargs)
575+
self.manager.delete(self.encoded_id, **kwargs)
578576

579577

580578
class UserAgentDetailMixin(_RestObjectBase):
Collapse file

‎gitlab/utils.py‎

Copy file name to clipboardExpand all lines: gitlab/utils.py
+64-4Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,77 @@ def copy_dict(dest: Dict[str, Any], src: Dict[str, Any]) -> None:
5656
dest[k] = v
5757

5858

59+
class EncodedId(str):
60+
"""A custom `str` class that will return the URL-encoded value of the string.
61+
62+
* Using it recursively will only url-encode the value once.
63+
* Can accept either `str` or `int` as input value.
64+
* Can be used in an f-string and output the URL-encoded string.
65+
66+
Reference to documentation on why this is necessary.
67+
68+
See::
69+
70+
https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
71+
https://docs.gitlab.com/ee/api/index.html#path-parameters
72+
"""
73+
74+
# `original_str` will contain the original string value that was used to create the
75+
# first instance of EncodedId. We will use this original value to generate the
76+
# URL-encoded value each time.
77+
original_str: str
78+
79+
def __new__(cls, value: Union[str, int, "EncodedId"]) -> "EncodedId":
80+
# __new__() gets called before __init__()
81+
if isinstance(value, int):
82+
value = str(value)
83+
# Make sure isinstance() for `EncodedId` comes before check for `str` as
84+
# `EncodedId` is an instance of `str` and would pass that check.
85+
elif isinstance(value, EncodedId):
86+
# We use the original string value to URL-encode
87+
value = value.original_str
88+
elif isinstance(value, str):
89+
pass
90+
else:
91+
raise ValueError(f"Unsupported type received: {type(value)}")
92+
# Set the value our string will return
93+
value = urllib.parse.quote(value, safe="")
94+
return super().__new__(cls, value)
95+
96+
def __init__(self, value: Union[int, str]) -> None:
97+
# At this point `super().__str__()` returns the URL-encoded value. Which means
98+
# when using this as a `str` it will return the URL-encoded value.
99+
#
100+
# But `value` contains the original value passed in `EncodedId(value)`. We use
101+
# this to always keep the original string that was received so that no matter
102+
# how many times we recurse we only URL-encode our original string once.
103+
if isinstance(value, int):
104+
value = str(value)
105+
# Make sure isinstance() for `EncodedId` comes before check for `str` as
106+
# `EncodedId` is an instance of `str` and would pass that check.
107+
elif isinstance(value, EncodedId):
108+
# This is the key part as we are always keeping the original string even
109+
# through multiple recursions.
110+
value = value.original_str
111+
elif isinstance(value, str):
112+
pass
113+
else:
114+
raise ValueError(f"Unsupported type received: {type(value)}")
115+
self.original_str = value
116+
super().__init__()
117+
118+
59119
@overload
60120
def _url_encode(id: int) -> int:
61121
...
62122

63123

64124
@overload
65-
def _url_encode(id: str) -> str:
125+
def _url_encode(id: Union[str, EncodedId]) -> EncodedId:
66126
...
67127

68128

69-
def _url_encode(id: Union[int, str]) -> Union[int, str]:
129+
def _url_encode(id: Union[int, str, EncodedId]) -> Union[int, EncodedId]:
70130
"""Encode/quote the characters in the string so that they can be used in a path.
71131
72132
Reference to documentation on why this is necessary.
@@ -84,9 +144,9 @@ def _url_encode(id: Union[int, str]) -> Union[int, str]:
84144
parameters.
85145
86146
"""
87-
if isinstance(id, int):
147+
if isinstance(id, (int, EncodedId)):
88148
return id
89-
return urllib.parse.quote(id, safe="")
149+
return EncodedId(id)
90150

91151

92152
def remove_none_from_dict(data: Dict[str, Any]) -> Dict[str, Any]:
Collapse file

‎gitlab/v4/objects/merge_request_approvals.py‎

Copy file name to clipboardExpand all lines: gitlab/v4/objects/merge_request_approvals.py
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def set_approvers(
7575

7676
if TYPE_CHECKING:
7777
assert self._parent is not None
78-
path = f"/projects/{self._parent.get_id()}/approvers"
78+
path = f"/projects/{self._parent.encoded_id}/approvers"
7979
data = {"approver_ids": approver_ids, "approver_group_ids": approver_group_ids}
8080
result = self.gitlab.http_put(path, post_data=data, **kwargs)
8181
if TYPE_CHECKING:
Collapse file

‎tests/functional/api/test_groups.py‎

Copy file name to clipboardExpand all lines: tests/functional/api/test_groups.py
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ def test_groups(gl):
100100
member = group1.members.get(user2.id)
101101
assert member.access_level == gitlab.const.OWNER_ACCESS
102102

103+
gl.auth()
103104
group2.members.delete(gl.user.id)
104105

105106

@@ -198,6 +199,11 @@ def test_group_subgroups_projects(gl, user):
198199
assert gr1_project.namespace["id"] == group1.id
199200
assert gr2_project.namespace["parent_id"] == group1.id
200201

202+
gr1_project.delete()
203+
gr2_project.delete()
204+
group3.delete()
205+
group4.delete()
206+
201207

202208
@pytest.mark.skip
203209
def test_group_wiki(group):
Collapse file

‎tests/functional/api/test_lazy_objects.py‎

Copy file name to clipboard
+39Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import pytest
2+
3+
import gitlab
4+
5+
6+
@pytest.fixture
7+
def lazy_project(gl, project):
8+
assert "/" in project.path_with_namespace
9+
return gl.projects.get(project.path_with_namespace, lazy=True)
10+
11+
12+
def test_lazy_id(project, lazy_project):
13+
assert isinstance(lazy_project.id, str)
14+
assert isinstance(lazy_project.id, gitlab.utils.EncodedId)
15+
assert lazy_project.id == gitlab.utils._url_encode(project.path_with_namespace)
16+
17+
18+
def test_refresh_after_lazy_get_with_path(project, lazy_project):
19+
lazy_project.refresh()
20+
assert lazy_project.id == project.id
21+
22+
23+
def test_save_after_lazy_get_with_path(project, lazy_project):
24+
lazy_project.description = "A new description"
25+
lazy_project.save()
26+
assert lazy_project.id == project.id
27+
assert lazy_project.description == "A new description"
28+
29+
30+
def test_delete_after_lazy_get_with_path(gl, group, wait_for_sidekiq):
31+
project = gl.projects.create({"name": "lazy_project", "namespace_id": group.id})
32+
result = wait_for_sidekiq(timeout=60)
33+
assert result is True, "sidekiq process should have terminated but did not"
34+
lazy_project = gl.projects.get(project.path_with_namespace, lazy=True)
35+
lazy_project.delete()
36+
37+
38+
def test_list_children_after_lazy_get_with_path(gl, lazy_project):
39+
lazy_project.mergerequests.list()
Collapse file

‎tests/functional/api/test_wikis.py‎

Copy file name to clipboardExpand all lines: tests/functional/api/test_wikis.py
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66

77
def test_wikis(project):
8-
98
page = project.wikis.create({"title": "title/subtitle", "content": "test content"})
109
page.content = "update content"
1110
page.title = "subtitle"
Collapse file

‎tests/functional/conftest.py‎

Copy file name to clipboardExpand all lines: tests/functional/conftest.py
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,8 @@ def user(gl):
406406
yield user
407407

408408
try:
409-
user.delete()
409+
# Use `hard_delete=True` or a 'Ghost User' may be created.
410+
user.delete(hard_delete=True)
410411
except gitlab.exceptions.GitlabDeleteError as e:
411412
print(f"User already deleted: {e}")
412413

Collapse file

‎tests/unit/test_base.py‎

Copy file name to clipboardExpand all lines: tests/unit/test_base.py
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ def test_encoded_id(self, fake_manager):
158158
obj.id = "a/path"
159159
assert "a%2Fpath" == obj.encoded_id
160160

161+
# If you assign it again it does not double URL-encode
162+
obj.id = obj.encoded_id
163+
assert "a%2Fpath" == obj.encoded_id
164+
161165
def test_custom_id_attr(self, fake_manager):
162166
class OtherFakeObject(FakeObject):
163167
_id_attr = "foo"
Collapse file

‎tests/unit/test_utils.py‎

Copy file name to clipboardExpand all lines: tests/unit/test_utils.py
+62Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
# You should have received a copy of the GNU Lesser General Public License
1616
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1717

18+
import json
19+
1820
from gitlab import utils
1921

2022

@@ -35,3 +37,63 @@ def test_url_encode():
3537
src = "docs/README.md"
3638
dest = "docs%2FREADME.md"
3739
assert dest == utils._url_encode(src)
40+
41+
42+
class TestEncodedId:
43+
def test_init_str(self):
44+
obj = utils.EncodedId("Hello")
45+
assert "Hello" == str(obj)
46+
assert "Hello" == f"{obj}"
47+
48+
obj = utils.EncodedId("this/is a/path")
49+
assert "this%2Fis%20a%2Fpath" == str(obj)
50+
assert "this%2Fis%20a%2Fpath" == f"{obj}"
51+
52+
def test_init_int(self):
53+
obj = utils.EncodedId(23)
54+
assert "23" == str(obj)
55+
assert "23" == f"{obj}"
56+
57+
def test_init_encodeid_str(self):
58+
value = "Goodbye"
59+
obj_init = utils.EncodedId(value)
60+
obj = utils.EncodedId(obj_init)
61+
assert value == str(obj)
62+
assert value == f"{obj}"
63+
assert value == obj.original_str
64+
65+
value = "we got/a/path"
66+
expected = "we%20got%2Fa%2Fpath"
67+
obj_init = utils.EncodedId(value)
68+
assert value == obj_init.original_str
69+
assert expected == str(obj_init)
70+
assert expected == f"{obj_init}"
71+
# Show that no matter how many times we recursively call it we still only
72+
# URL-encode it once.
73+
obj = utils.EncodedId(
74+
utils.EncodedId(utils.EncodedId(utils.EncodedId(utils.EncodedId(obj_init))))
75+
)
76+
assert expected == str(obj)
77+
assert expected == f"{obj}"
78+
# We have stored a copy of our original string
79+
assert value == obj.original_str
80+
81+
# Show assignments still only encode once
82+
obj2 = obj
83+
assert expected == str(obj2)
84+
assert expected == f"{obj2}"
85+
86+
def test_init_encodeid_int(self):
87+
value = 23
88+
expected = f"{value}"
89+
obj_init = utils.EncodedId(value)
90+
obj = utils.EncodedId(obj_init)
91+
assert expected == str(obj)
92+
assert expected == f"{obj}"
93+
94+
def test_json_serializable(self):
95+
obj = utils.EncodedId("someone")
96+
assert '"someone"' == json.dumps(obj)
97+
98+
obj = utils.EncodedId("we got/a/path")
99+
assert '"we%20got%2Fa%2Fpath"' == json.dumps(obj)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.