Skip to content

Commit cb20f0a

Browse files
Shushant SinghPrabhakar Kumar
authored andcommitted
Resets the active session when user closes the browser window.
fixes: #32
1 parent e98e7d0 commit cb20f0a

File tree

5 files changed

+222
-17
lines changed

5 files changed

+222
-17
lines changed

gui/src/components/App/index.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,22 @@ function App () {
175175
}
176176
}
177177

178+
useEffect(() => {
179+
const handlePageHide = (event) => {
180+
// Performs actions before the component unloads
181+
if (isConcurrencyEnabled && !isSessionConcurrent && hasFetchedServerStatus) {
182+
// A POST request to clear the active client when the tab/browser is closed
183+
navigator.sendBeacon('./clear_client_id');
184+
}
185+
event.preventDefault();
186+
event.returnValue = '';
187+
};
188+
window.addEventListener('pagehide', handlePageHide);
189+
return () => {
190+
window.removeEventListener('pagehide', handlePageHide);
191+
};
192+
}, [isConcurrencyEnabled, isSessionConcurrent, hasFetchedServerStatus]);
193+
178194
useEffect(() => {
179195
// Initial fetch environment configuration
180196
if (!hasFetchedEnvConfig) {

matlab_proxy/app.py

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import pkgutil
77
import secrets
88
import sys
9-
import uuid
109

1110
import aiohttp
1211
from aiohttp import client_exceptions, web
@@ -97,16 +96,15 @@ def marshal_error(error):
9796

9897

9998
async def create_status_response(
100-
app, loadUrl=None, client_id=None, transfer_session=False, is_desktop=False
99+
app, loadUrl=None, client_id=None, is_active_client=None
101100
):
102101
"""Send a generic status response about the state of server, MATLAB, MATLAB Licensing and the client session status.
103102
104103
Args:
105104
app (aiohttp.web.Application): Web Server
106105
loadUrl (String, optional): Represents the root URL. Defaults to None.
107-
client_id (String, optional): Represents the unique client_id when concurrency check is enabled. Defaults to None.
108-
transfer_session (Boolean, optional): Represents whether the connection should be transfered or not when concurrency check is enabled. Defaults to False.
109-
is_desktop (Boolean, optional): Represents whether the request is made by the desktop app or some other kernel. Defaults to False.
106+
client_id (String, optional): Represents the generated client_id when concurrency check is enabled and client does not have a client_id. Defaults to None.
107+
is_active_client (Boolean, optional): Represents whether the current client is the active_client when concurrency check is enabled. Defaults to None.
110108
111109
Returns:
112110
JSONResponse: A JSONResponse object containing the generic state of the server, MATLAB, MATLAB Licensing and the client session status.
@@ -124,17 +122,30 @@ async def create_status_response(
124122
"wsEnv": state.settings.get("ws_env", ""),
125123
}
126124

127-
if IS_CONCURRENCY_CHECK_ENABLED and is_desktop:
128-
if not client_id:
129-
client_id = str(uuid.uuid4())
130-
status["clientId"] = client_id
125+
if client_id:
126+
status["clientId"] = client_id
127+
if is_active_client is not None:
128+
status["isActiveClient"] = is_active_client
131129

132-
if not state.active_client or transfer_session:
133-
state.active_client = client_id
130+
return web.json_response(status)
134131

135-
status["isActiveClient"] = True if state.active_client == client_id else False
136132

137-
return web.json_response(status)
133+
@token_auth.authenticate_access_decorator
134+
async def clear_client_id(req):
135+
"""API endpoint to reset the active client
136+
137+
Args:
138+
req (HTTPRequest): HTTPRequest Object
139+
140+
Returns:
141+
Response: an empty response in JSON format
142+
"""
143+
# Sleep for one second prior to clearing the client id to ensure that any remaining get_status responses are fully processed first.
144+
await asyncio.sleep(1)
145+
state = req.app["state"]
146+
state.active_client = None
147+
# This response is of no relevance to the front-end as the client has already exited
148+
return web.json_response({})
138149

139150

140151
@token_auth.authenticate_access_decorator
@@ -201,15 +212,17 @@ async def get_status(req):
201212
JSONResponse: JSONResponse object containing information about the server, MATLAB and MATLAB Licensing.
202213
"""
203214
# The client sends the CLIENT_ID as a query parameter if the concurrency check has been set to true.
215+
state = req.app["state"]
204216
client_id = req.query.get("MWI_CLIENT_ID", None)
205217
transfer_session = json.loads(req.query.get("TRANSFER_SESSION", "false"))
206218
is_desktop = req.query.get("IS_DESKTOP", False)
207219

220+
generated_client_id, is_active_client = state.get_session_status(
221+
is_desktop, client_id, transfer_session
222+
)
223+
208224
return await create_status_response(
209-
req.app,
210-
client_id=client_id,
211-
transfer_session=transfer_session,
212-
is_desktop=is_desktop,
225+
req.app, client_id=generated_client_id, is_active_client=is_active_client
213226
)
214227

215228

@@ -773,6 +786,8 @@ async def cleanup_background_tasks(app):
773786
# Stop any running async tasks
774787
logger = mwi.logger.get()
775788
tasks = state.tasks
789+
if state.task_detect_client_status:
790+
tasks["detect_client_status"] = state.task_detect_client_status
776791
for task_name, task in tasks.items():
777792
if not task.cancelled():
778793
logger.debug(f"Cancelling MWI task: {task_name} : {task} ")
@@ -868,6 +883,7 @@ def create_app(config_name=matlab_proxy.get_default_config_name()):
868883
app.router.add_route("GET", f"{base_url}/get_auth_token", get_auth_token)
869884
app.router.add_route("GET", f"{base_url}/get_env_config", get_env_config)
870885
app.router.add_route("PUT", f"{base_url}/start_matlab", start_matlab)
886+
app.router.add_route("POST", f"{base_url}/clear_client_id", clear_client_id)
871887
app.router.add_route("DELETE", f"{base_url}/stop_matlab", stop_matlab)
872888
app.router.add_route("PUT", f"{base_url}/set_licensing_info", set_licensing_info)
873889
app.router.add_route("PUT", f"{base_url}/update_entitlement", update_entitlement)

matlab_proxy/app_state.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
from collections import deque
1010
from datetime import datetime, timedelta, timezone
1111
from typing import Final, Optional
12+
import uuid
1213

1314
from matlab_proxy import util
1415
from matlab_proxy.constants import (
1516
CONNECTOR_SECUREPORT_FILENAME,
1617
MATLAB_LOGS_FILE_NAME,
18+
IS_CONCURRENCY_CHECK_ENABLED,
1719
)
1820
from matlab_proxy.settings import (
1921
get_process_startup_timeout,
@@ -103,6 +105,12 @@ def __init__(self, settings):
103105
# connected to the backend
104106
self.active_client = None
105107

108+
# Used to detect whether the active client is actively sending out request or is inactive
109+
self.active_client_request_detected = False
110+
111+
# An event loop task to handle the detection of client activity
112+
self.task_detect_client_status = None
113+
106114
def __get_cached_config_file(self):
107115
"""Get the cached config file
108116
@@ -1304,3 +1312,76 @@ def parsed_errs():
13041312
if err is not None:
13051313
self.error = err
13061314
log_error(logger, err)
1315+
1316+
def get_session_status(self, is_desktop, client_id, transfer_session):
1317+
"""
1318+
Determines the session status for a client, potentially generating a new client ID.
1319+
1320+
This function is responsible for managing and tracking the session status of a client.
1321+
It can generate a new client ID if one is not provided and the conditions are met.
1322+
It also manages the active client status within the session, especially in scenarios
1323+
involving desktop clients and when concurrency checks are enabled.
1324+
1325+
Args:
1326+
is_desktop (bool): A flag indicating whether the client is a desktop client.
1327+
client_id (str or None): The client ID. If None, a new client ID may be generated.
1328+
transfer_session (bool): Indicates whether the session should be transferred to this client.
1329+
1330+
Returns:
1331+
tuple:
1332+
- A 2-tuple containing the generated client ID (or None if not generated) and
1333+
a boolean indicating whether the client is considered the active client.
1334+
- If concurrency checks are not enabled or the client is not a desktop client, it returns None for both
1335+
the generated client ID and the active client status.
1336+
"""
1337+
if IS_CONCURRENCY_CHECK_ENABLED and is_desktop:
1338+
generated_client_id = None
1339+
if not client_id:
1340+
generated_client_id = str(uuid.uuid4())
1341+
client_id = generated_client_id
1342+
1343+
if not self.active_client or transfer_session:
1344+
self.active_client = client_id
1345+
1346+
if not self.task_detect_client_status:
1347+
# Create the loop to detect the active status of the client
1348+
loop = util.get_event_loop()
1349+
self.task_detect_client_status = loop.create_task(
1350+
self.detect_active_client_status()
1351+
)
1352+
1353+
if self.active_client == client_id:
1354+
is_active_client = True
1355+
self.active_client_request_detected = True
1356+
else:
1357+
is_active_client = False
1358+
return generated_client_id, is_active_client
1359+
return None, None
1360+
1361+
async def detect_active_client_status(self, sleep_time=1, max_inactive_count=10):
1362+
"""Detects whether the client is online or not by continuously checking if the active client is making requests
1363+
1364+
Args:
1365+
sleep_time (int): The time in seconds for which the process waits before checking for the next get_status request from the active client.
1366+
max_inactive_count (int): The maximum number of times the check for the request from the active_client fails before reseting the active client id.
1367+
"""
1368+
inactive_count = 0
1369+
while self.active_client:
1370+
# Check if the get_status request from the active client is received or not
1371+
await asyncio.sleep(sleep_time)
1372+
if self.active_client_request_detected:
1373+
self.active_client_request_detected = False
1374+
inactive_count = 0
1375+
else:
1376+
inactive_count = inactive_count + 1
1377+
if inactive_count > max_inactive_count:
1378+
# If no request is received from the active_client for more than 10 seconds then clear the active client id
1379+
inactive_count = 0
1380+
self.active_client = None
1381+
if self.task_detect_client_status:
1382+
try:
1383+
# Self cleanup of the task
1384+
self.task_detect_client_status.cancel()
1385+
self.task_detect_client_status = None
1386+
except Exception as e:
1387+
logger.error("Cleaning of task: 'detect_client_status' failed.")

tests/unit/test_app.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,20 @@ async def test_get_status_route(test_server):
284284
assert resp.status == HTTPStatus.OK
285285

286286

287+
async def test_clear_client_id_route(test_server):
288+
"""Test to check endpoint: "/clear_client_id"
289+
290+
Args:
291+
test_server (aiohttp_client): A aiohttp_client server for sending POST request.
292+
"""
293+
294+
state = test_server.server.app["state"]
295+
state.active_client = "mock_client_id"
296+
resp = await test_server.post("/clear_client_id")
297+
assert resp.status == HTTPStatus.OK
298+
assert state.active_client is None
299+
300+
287301
async def test_get_env_config(test_server):
288302
"""Test to check endpoint : "/get_env_config"
289303

tests/unit/test_app_state.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,3 +584,81 @@ async def test_start_matlab_without_xvfb(app_state_fixture, mocker):
584584
assert app_state_fixture.processes["xvfb"] is None
585585
# Check if Matlab started
586586
assert app_state_fixture.processes["matlab"] is mock_matlab
587+
588+
589+
@pytest.mark.parametrize(
590+
"is_desktop, client_id, is_client_id_present, expected_is_active_client",
591+
[
592+
(False, None, False, None),
593+
(False, "mock_id", False, None),
594+
(True, None, True, True),
595+
(True, "mock_id", False, True),
596+
],
597+
ids=[
598+
"request_from_non-desktop_client",
599+
"request_from_non-desktop_client_having_mock_id",
600+
"request_from_desktop_client",
601+
"request_from_desktop_client_having_mock_id",
602+
],
603+
)
604+
async def test_get_session_status(
605+
app_state_fixture,
606+
is_desktop,
607+
client_id,
608+
is_client_id_present,
609+
expected_is_active_client,
610+
):
611+
"""Test to check if correnct session response is returned based on various conditions.
612+
613+
Args:
614+
app_state_fixture (AppState): Object of AppState class with defaults set
615+
is_desktop (bool): A flag indicating whether the client is a desktop client.
616+
client_id (str or None): The client ID. If None, a new client ID may be generated.
617+
is_client_id_present (bool): Indicates whether the expected value of client_id is string or not.
618+
expected_is_active_client (bool): Indicates the expected value of is_active_client
619+
620+
"""
621+
# The value of transfer_session is a Don't Care condition as initially the value of client_id is always None.
622+
output_client_id, output_is_active_client = app_state_fixture.get_session_status(
623+
is_desktop, client_id, transfer_session=False
624+
)
625+
assert isinstance(output_client_id, str) == is_client_id_present, (
626+
"Expected client_id to be a string got None"
627+
if is_client_id_present
628+
else "Expected client_id to be None got a string value"
629+
)
630+
assert (
631+
output_is_active_client == expected_is_active_client
632+
), f"Expected is_active_client to be {expected_is_active_client} got {output_is_active_client}"
633+
# For clean up of task_detect_client_status
634+
app_state_fixture.active_client = None
635+
636+
637+
async def test_get_session_status_can_transfer_session(app_state_fixture):
638+
"""Test to check whether transer session changes client id to the new id
639+
640+
Args:
641+
app_state_fixture (AppState): Object of AppState class with defaults set
642+
"""
643+
app_state_fixture.active_client = "mock_id"
644+
app_state_fixture.get_session_status(
645+
is_desktop=True, client_id="new_id", transfer_session=True
646+
)
647+
assert app_state_fixture.active_client == "new_id"
648+
# For clean up of task_detect_client_status
649+
app_state_fixture.active_client = None
650+
651+
652+
async def test_detect_active_client_status_can_reset_active_client(app_state_fixture):
653+
"""Test to check whether the value of active client is being reset due to the client inactivity.
654+
655+
Args:
656+
app_state_fixture (AppState): Object of AppState class with defaults set
657+
"""
658+
app_state_fixture.active_client = "mock_id"
659+
await app_state_fixture.detect_active_client_status(
660+
sleep_time=0, max_inactive_count=0
661+
)
662+
assert (
663+
app_state_fixture.active_client == None
664+
), f"Expected the active_client to be None"

0 commit comments

Comments
 (0)