Skip to content

Commit 24ea87e

Browse files
committed
fix: improve resource typing and session lifecycle safety
Tighten generics around resource argument translation and session usage, including a typed _session property and clearer JWTAuth handling. Standardize Resource subclasses to call super(), allow None options safely, and guard timetracking parsing against missing sessions. Fixes #2387
1 parent b07a89b commit 24ea87e

File tree

3 files changed

+111
-133
lines changed

3 files changed

+111
-133
lines changed

jira/client.py

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@
3232
Callable,
3333
Generic,
3434
Literal,
35+
ParamSpec,
3536
SupportsIndex,
3637
TypeVar,
38+
cast,
3739
no_type_check,
3840
overload,
3941
)
@@ -100,7 +102,7 @@
100102
try:
101103
from requests_jwt import JWTAuth
102104
except ImportError:
103-
pass
105+
JWTAuth = None
104106

105107

106108
LOG = _logging.getLogger("jira")
@@ -183,25 +185,28 @@ def is_experimental(*args, **kwargs):
183185
return is_experimental
184186

185187

186-
def translate_resource_args(func: Callable):
188+
P = ParamSpec("P")
189+
R = TypeVar("R")
190+
191+
192+
def translate_resource_args(func: Callable[P, R]) -> Callable[P, R]:
187193
"""Decorator that converts Issue and Project resources to their keys when used as arguments.
188194
189195
Args:
190196
func (Callable): the function to decorate
191197
"""
192198

193199
@wraps(func)
194-
def wrapper(*args: Any, **kwargs: Any) -> Any:
195-
arg_list = []
200+
def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
201+
arg_list: list[Any] = []
196202
for arg in args:
197203
if isinstance(arg, Issue | Project):
198204
arg_list.append(arg.key)
199205
elif isinstance(arg, IssueLinkType):
200206
arg_list.append(arg.name)
201207
else:
202208
arg_list.append(arg)
203-
result = func(*arg_list, **kwargs)
204-
return result
209+
return cast(Callable[..., R], func)(*arg_list, **kwargs)
205210

206211
return wrapper
207212

@@ -214,13 +219,13 @@ def _field_worker(
214219
return {"fields": fieldargs}
215220

216221

217-
ResourceType = TypeVar("ResourceType", contravariant=True, bound=Resource)
222+
ResourceType = TypeVar("ResourceType", bound=Resource)
218223

219224

220-
class ResultList(list, Generic[ResourceType]):
225+
class ResultList(list[ResourceType], Generic[ResourceType]):
221226
def __init__(
222227
self,
223-
iterable: Iterable | None = None,
228+
iterable: Iterable[ResourceType] | None = None,
224229
_startAt: int = 0,
225230
_maxResults: int = 0,
226231
_total: int | None = None,
@@ -306,8 +311,7 @@ def _generate_qsh(self, req):
306311
return qsh
307312

308313
def _sort_and_quote_values(self, values):
309-
ordered_values = sorted(values)
310-
return [quote(value, safe="~") for value in ordered_values]
314+
return [quote(value, safe="~") for value in sorted(values)]
311315

312316

313317
class JiraCookieAuth(AuthBase):
@@ -464,6 +468,8 @@ class JIRA:
464468
JIRA_BASE_URL = Resource.JIRA_BASE_URL
465469
AGILE_BASE_URL = AgileResource.AGILE_BASE_URL
466470

471+
_session_obj: ResilientSession | None
472+
467473
def __init__(
468474
self,
469475
server: str | None = None,
@@ -611,21 +617,21 @@ def __init__(
611617
assert isinstance(self._options["headers"], dict) # for mypy benefit
612618

613619
# Create Session object and update with config options first
614-
self._session = ResilientSession(timeout=timeout)
620+
self._session_obj = ResilientSession(timeout=timeout)
615621
# Add the client authentication certificate to the request if configured
616622
self._add_client_cert_to_session()
617623
# Add the SSL Cert to the request if configured
618624
self._add_ssl_cert_verif_strategy_to_session()
619625

620-
self._session.headers.update(self._options["headers"])
626+
self._session_obj.headers.update(self._options["headers"])
621627

622628
if "cookies" in self._options:
623-
self._session.cookies.update(self._options["cookies"])
629+
self._session_obj.cookies.update(self._options["cookies"])
624630

625-
self._session.max_retries = max_retries
631+
self._session_obj.max_retries = max_retries
626632

627633
if proxies:
628-
self._session.proxies = proxies
634+
self._session_obj.proxies = proxies
629635

630636
# Setup the Auth last,
631637
# so that if any handlers take a copy of the session obj it will be ready
@@ -737,17 +743,15 @@ def __del__(self):
737743
self.close()
738744

739745
def close(self):
740-
session = getattr(self, "_session", None)
741-
if session is not None:
746+
if self._session_obj:
742747
try:
743-
session.close()
748+
self._session_obj.close()
744749
except TypeError:
745750
# TypeError: "'NoneType' object is not callable" could still happen here
746751
# because other references are also in the process to be torn down,
747752
# see warning section in https://docs.python.org/2/reference/datamodel.html#object.__del__
748753
pass
749-
# TODO: https://github.com/pycontribs/jira/issues/1881
750-
self._session = None # type: ignore[arg-type,assignment]
754+
self._session_obj = None
751755

752756
def _check_for_html_error(self, content: str):
753757
# Jira has the bad habit of returning errors in pages with 200 and embedding the
@@ -809,6 +813,7 @@ def json_params() -> dict[str, Any]:
809813

810814
page_params = json_params()
811815

816+
batch_size = None
812817
if startAt:
813818
page_params["startAt"] = startAt
814819
if maxResults:
@@ -909,7 +914,7 @@ def json_params() -> dict[str, Any]:
909914
else: # TODO: unreachable
910915
# it seems that search_users can return a list() containing a single user!
911916
return ResultList(
912-
[item_type(self._options, self._session, resource)], 0, 1, 1, True
917+
[item_type(self._options, self._session_obj, resource)], 0, 1, 1, True
913918
)
914919

915920
@cloud_api
@@ -1060,6 +1065,12 @@ def application_properties(
10601065
params["key"] = key
10611066
return self._get_json("application-properties", params=params)
10621067

1068+
@property
1069+
def _session(self) -> ResilientSession:
1070+
if self._session_obj is None:
1071+
raise JIRAError("JIRA instance has been closed and can no longer be used.")
1072+
return self._session_obj
1073+
10631074
def set_application_property(self, key: str, value: str):
10641075
"""Set the application property.
10651076
@@ -2552,8 +2563,7 @@ def add_remote_link(
25522563
url = self._get_url("issue/" + str(issue) + "/remotelink")
25532564
r = self._session.post(url, data=json.dumps(data))
25542565

2555-
remote_link = RemoteLink(self._options, self._session, raw=json_loads(r))
2556-
return remote_link
2566+
return RemoteLink(self._options, self._session, raw=json_loads(r))
25572567

25582568
def add_simple_link(self, issue: str, object: dict[str, Any]):
25592569
"""Add a simple remote link from an issue to web resource.
@@ -2577,8 +2587,7 @@ def add_simple_link(self, issue: str, object: dict[str, Any]):
25772587
url = self._get_url("issue/" + str(issue) + "/remotelink")
25782588
r = self._session.post(url, data=json.dumps(data))
25792589

2580-
simple_link = RemoteLink(self._options, self._session, raw=json_loads(r))
2581-
return simple_link
2590+
return RemoteLink(self._options, self._session, raw=json_loads(r))
25822591

25832592
# non-resource
25842593
@translate_resource_args
@@ -4006,10 +4015,8 @@ def search_assignable_users_for_issues(
40064015
"Either 'username' or 'query' arguments must be specified."
40074016
)
40084017

4009-
if username is not None:
4010-
params = {"username": username}
4011-
if query is not None:
4012-
params = {"query": query}
4018+
params: dict[str, Any] = {"query": query} if query else {"username": username}
4019+
40134020
if project is not None:
40144021
params["project"] = project
40154022
if issueKey is not None:
@@ -4170,7 +4177,8 @@ def delete_remote_link(
41704177

41714178
if internal_id is not None:
41724179
url = self._get_url(f"issue/{issue}/remotelink/{internal_id}")
4173-
elif global_id is not None:
4180+
else:
4181+
assert global_id is not None
41744182
# stop "&" and other special characters in global_id from messing around with the query
41754183
global_id = urllib.parse.quote(global_id, safe="")
41764184
url = self._get_url(f"issue/{issue}/remotelink?globalId={global_id}")
@@ -4495,11 +4503,10 @@ def _timestamp(dt: datetime.timedelta | None = None):
44954503
return calendar.timegm(t.timetuple())
44964504

44974505
def _create_jwt_session(self, jwt: dict[str, Any]):
4498-
try:
4499-
jwt_auth = JWTAuth(jwt["secret"], alg="HS256")
4500-
except NameError as e:
4501-
self.log.error("JWT authentication requires requests_jwt")
4502-
raise e
4506+
if JWTAuth is None:
4507+
raise JIRAError("JWT authentication requires requests_jwt")
4508+
4509+
jwt_auth = JWTAuth(jwt["secret"], alg="HS256")
45034510
jwt_auth.set_header_format("JWT %s")
45044511

45054512
jwt_auth.add_field("iat", lambda req: JIRA._timestamp())
@@ -5505,7 +5512,7 @@ def boards(
55055512
@translate_resource_args
55065513
def sprints(
55075514
self,
5508-
board_id: int,
5515+
board_id: int | str,
55095516
extended: bool | None = None,
55105517
startAt: int = 0,
55115518
maxResults: int = 50,
@@ -5543,7 +5550,7 @@ def sprints(
55435550

55445551
def sprints_by_name(
55455552
self, id: str | int, extended: bool = False, state: str | None = None
5546-
) -> dict[str, dict[str, Any]]:
5553+
) -> dict[str, dict[str, Any] | None]:
55475554
"""Get a dictionary of sprint Resources where the name of the sprint is the key.
55485555
55495556
Args:
@@ -5651,6 +5658,7 @@ def sprint_info(self, board_id: str, sprint_id: str) -> dict[str, Any]:
56515658
"""
56525659
sprint = Sprint(self._options, self._session)
56535660
sprint.find(sprint_id)
5661+
assert sprint.raw is not None, "sprint.raw is None but should be set by sprint.find()"
56545662
return sprint.raw
56555663

56565664
def sprint(self, id: int) -> Sprint:
@@ -5836,7 +5844,8 @@ def rank(
58365844
if next_issue is not None:
58375845
before_or_after = "Before"
58385846
other_issue = next_issue
5839-
elif prev_issue is not None:
5847+
else:
5848+
assert prev_issue is not None
58405849
before_or_after = "After"
58415850
other_issue = prev_issue
58425851

0 commit comments

Comments
 (0)