Skip to content

Commit f331758

Browse files
proscar87claude
andauthored
Allow injecting a requests.Session to keep logins from piling up (#162)
* Allow injecting a requests.Session to keep logins from piling up Requested by @johanzander in #160: every GrowattApi instance builds its own fresh Session, so nothing is shared or persisted. In the Home Assistant integration login() is called from the config flow, from setup, and from the coordinator on every update cycle -- which is the login frequency that drives accounts into the ~24h 507 lockout that #160 only reports. A constructor-injected `session` lets a consumer share one Session across API instances *and* restore persisted cookies at startup. The library takes on no storage responsibility: where the cookies live stays the caller's business. The raise_for_status hook is now appended rather than assigned. A shared Session may already carry hooks and replacing `session.hooks` outright would drop them silently; a bare callable is normalised to a list, which requests permits. Six of the seven tests fail without the parameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Install the response hook once per shared session Every GrowattApi appended its own raise_for_status hook, so N instances sharing one injected session left N hooks on it and each response was checked N times. Skip the install when an earlier instance already put the hook there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Leave a supplied session's User-Agent alone, and match the hook by identity Review feedback on #162: - The User-Agent of an injected session is only changed when the caller passes agent_identifier or add_random_user_id. Before, every instance overwrote it, and with several instances on one session the last won. - The response hook is a module-level function and is matched by identity, so an unrelated hook with the same name is not mistaken for ours. - The docstring says that skipping login() while restored cookies are valid is up to the caller, that the session now raises on HTTP errors for every request, and that the 507 lockout (HTTP 200, msg 507) is not caught by that hook. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: proscar87 <proscar87@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 66614e0 commit f331758

2 files changed

Lines changed: 209 additions & 13 deletions

File tree

‎growattServer/base_api.py‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ def _raise_if_rate_limited(response: requests.Response) -> None:
6060
raise GrowattRateLimitError(error_code=RATE_LIMITED_CODE)
6161

6262

63+
def _raise_for_status(response: requests.Response, *args: object, **kwargs: object) -> None:
64+
"""Raise on Growatt's 507 refusal and on HTTP errors; the session's response hook."""
65+
_ = args
66+
_ = kwargs
67+
_raise_if_rate_limited(response)
68+
response.raise_for_status()
69+
70+
6371
def hash_password(password: str) -> str:
6472
"""
6573
Return a modified MD5-like hex digest with 'c' substitutions.
@@ -88,13 +96,31 @@ class GrowattApi:
8896
server_url = "https://openapi.growatt.com/"
8997
agent_identifier = "Dalvik/2.1.0 (Linux; U; Android 12; https://github.com/indykoning/PyPi_GrowattServer)"
9098

91-
def __init__(self, add_random_user_id: bool = False, agent_identifier: str | None = None) -> None:
99+
def __init__(
100+
self,
101+
add_random_user_id: bool = False,
102+
agent_identifier: str | None = None,
103+
session: requests.Session | None = None,
104+
) -> None:
92105
"""
93106
Initialize the Growatt API client.
94107
95108
Args:
96109
add_random_user_id: Append a short random suffix to the user-agent.
97110
agent_identifier: Optional override for the user-agent string.
111+
session: An existing :class:`requests.Session` to use instead of
112+
creating a fresh one. Passing one lets a consumer share a
113+
single session across API instances and restore persisted
114+
cookies at startup, so a login is not needed on every call --
115+
which is what drives accounts into the 507 lockout. The library
116+
stores nothing itself: persistence stays the caller's business,
117+
and so is skipping ``login()`` while the restored cookies are
118+
still valid. The session gets a response hook that, for every
119+
request made through it, raises on HTTP errors and raises
120+
:class:`GrowattRateLimitError` when the body carries the 507
121+
lockout (which arrives as HTTP 200).
122+
The User-Agent of a supplied session is only changed when
123+
``agent_identifier`` or ``add_random_user_id`` is given.
98124
99125
"""
100126
if agent_identifier is not None:
@@ -105,18 +131,24 @@ def __init__(self, add_random_user_id: bool = False, agent_identifier: str | Non
105131
random_number = "".join(str(secrets.randbelow(10)) for _ in range(5))
106132
self.agent_identifier += " - " + random_number
107133

108-
self.session = requests.Session()
109-
110-
def _raise_for_status(response, *args: object, **kwargs: object) -> None:
111-
_ = args
112-
_ = kwargs
113-
_raise_if_rate_limited(response)
114-
response.raise_for_status()
115-
116-
self.session.hooks = {"response": [_raise_for_status]}
117-
118-
headers = {"User-Agent": self.agent_identifier}
119-
self.session.headers.update(headers)
134+
self.session = session if session is not None else requests.Session()
135+
136+
# Append rather than assign: a caller-supplied session may already carry
137+
# hooks of its own, and replacing the dict would silently drop them.
138+
hooks = self.session.hooks.setdefault("response", [])
139+
if callable(hooks): # requests allows a bare callable here
140+
hooks = [hooks]
141+
# Several instances sharing one session must not stack one copy each,
142+
# or every response would be checked once per instance. Compared by
143+
# identity, so an unrelated hook that happens to share the name is kept.
144+
already_installed = any(hook is _raise_for_status for hook in hooks)
145+
self.session.hooks["response"] = hooks if already_installed else [*hooks, _raise_for_status]
146+
147+
# A supplied session keeps its own User-Agent unless the caller asked
148+
# for one; otherwise several instances on one session would overwrite
149+
# each other and the caller's header.
150+
if session is None or agent_identifier is not None or add_random_user_id:
151+
self.session.headers.update({"User-Agent": self.agent_identifier})
120152

121153
def __get_date_string(self, timespan: Timespan | None = None, date: datetime.datetime | None = None) -> str:
122154
if timespan is not None and not isinstance(timespan, Timespan):

‎tests/test_session_injection.py‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
"""A caller-supplied session lets cookies survive across API instances."""
2+
import json
3+
4+
import pytest
5+
import requests
6+
7+
from growattServer import GrowattApi, GrowattRateLimitError
8+
9+
10+
def test_default_still_creates_its_own_session():
11+
api = GrowattApi()
12+
13+
assert isinstance(api.session, requests.Session)
14+
15+
16+
def test_injected_session_is_used_as_is():
17+
shared = requests.Session()
18+
19+
api = GrowattApi(session=shared)
20+
21+
assert api.session is shared
22+
23+
24+
def test_two_instances_share_one_session():
25+
"""The point of the parameter: one login, many API objects."""
26+
shared = requests.Session()
27+
28+
first = GrowattApi(session=shared)
29+
second = GrowattApi(session=shared)
30+
31+
assert first.session is second.session
32+
33+
34+
def test_cookies_on_the_injected_session_survive():
35+
"""Restoring persisted cookies at startup is the reason this exists."""
36+
shared = requests.Session()
37+
shared.cookies.set("JSESSIONID", "restored-from-storage")
38+
39+
api = GrowattApi(session=shared)
40+
41+
assert api.session.cookies.get("JSESSIONID") == "restored-from-storage"
42+
43+
44+
def test_existing_hooks_on_the_injected_session_are_kept():
45+
"""A shared session may already carry hooks; they must not be dropped."""
46+
called = []
47+
48+
def _mine(response, *args, **kwargs):
49+
called.append(response)
50+
51+
shared = requests.Session()
52+
shared.hooks["response"] = [_mine]
53+
54+
api = GrowattApi(session=shared)
55+
56+
assert _mine in api.session.hooks["response"]
57+
assert len(api.session.hooks["response"]) == 2
58+
59+
60+
def test_sharing_a_session_installs_the_hook_once():
61+
"""N instances on one session must not stack N copies of the hook."""
62+
def _mine(response, *args, **kwargs):
63+
pass
64+
65+
shared = requests.Session()
66+
shared.hooks["response"] = [_mine]
67+
68+
for _ in range(3):
69+
GrowattApi(session=shared)
70+
71+
assert len(shared.hooks["response"]) == 2
72+
assert shared.hooks["response"][0] is _mine
73+
74+
75+
def test_shared_session_checks_each_response_once():
76+
"""Each response goes through raise_for_status once, however many instances."""
77+
shared = requests.Session()
78+
for _ in range(3):
79+
GrowattApi(session=shared)
80+
81+
response = requests.Response()
82+
response.status_code = 200
83+
calls = []
84+
response.raise_for_status = lambda: calls.append(response)
85+
86+
requests.hooks.dispatch_hook("response", shared.hooks, response)
87+
88+
assert len(calls) == 1
89+
90+
91+
def test_bare_callable_hook_is_normalised():
92+
"""requests permits a single callable instead of a list."""
93+
def _mine(response, *args, **kwargs):
94+
pass
95+
96+
shared = requests.Session()
97+
shared.hooks["response"] = _mine
98+
99+
api = GrowattApi(session=shared)
100+
101+
assert _mine in api.session.hooks["response"]
102+
103+
104+
def test_user_agent_is_applied_to_the_injected_session():
105+
shared = requests.Session()
106+
107+
api = GrowattApi(session=shared, agent_identifier="my-agent")
108+
109+
assert shared.headers["User-Agent"] == "my-agent"
110+
111+
112+
def test_injected_session_keeps_its_user_agent_by_default():
113+
"""Without agent_identifier, a caller's own User-Agent must survive."""
114+
shared = requests.Session()
115+
shared.headers["User-Agent"] = "callers-agent"
116+
117+
GrowattApi(session=shared)
118+
119+
assert shared.headers["User-Agent"] == "callers-agent"
120+
121+
122+
def test_own_session_still_gets_the_library_user_agent():
123+
api = GrowattApi()
124+
125+
assert api.session.headers["User-Agent"] == GrowattApi.agent_identifier
126+
127+
128+
def test_random_user_id_is_applied_to_the_injected_session():
129+
"""Asking for a random suffix is asking for the header to change."""
130+
shared = requests.Session()
131+
shared.headers["User-Agent"] = "callers-agent"
132+
133+
GrowattApi(session=shared, add_random_user_id=True)
134+
135+
assert shared.headers["User-Agent"].startswith(GrowattApi.agent_identifier + " - ")
136+
137+
138+
def test_an_unrelated_hook_with_the_same_name_is_not_mistaken_for_ours():
139+
"""De-duplication is by identity, not by name."""
140+
def _raise_for_status(response, *args, **kwargs):
141+
pass
142+
143+
# As if it were a top-level function of the same name in another module.
144+
_raise_for_status.__qualname__ = "_raise_for_status"
145+
shared = requests.Session()
146+
shared.hooks["response"] = [_raise_for_status]
147+
148+
GrowattApi(session=shared)
149+
150+
assert len(shared.hooks["response"]) == 2
151+
152+
153+
def test_an_injected_session_still_raises_on_the_507_lockout():
154+
"""The 507 check lives in the hook, so a shared session must carry it too."""
155+
shared = requests.Session()
156+
GrowattApi(session=shared)
157+
158+
response = requests.Response()
159+
response.status_code = 200
160+
response._content = json.dumps({"success": False, "msg": "507"}).encode()
161+
response.headers["Content-Type"] = "application/json"
162+
163+
with pytest.raises(GrowattRateLimitError):
164+
requests.hooks.dispatch_hook("response", shared.hooks, response)

0 commit comments

Comments
 (0)