diff --git a/.github/workflows/sonarqube.yml b/.github/workflows/sonarqube.yml index 029b628..cc040cb 100644 --- a/.github/workflows/sonarqube.yml +++ b/.github/workflows/sonarqube.yml @@ -1,12 +1,7 @@ name: SonarQube analysis -on: - push: - branches: - - main - pull_request: - branches: - - main +on: + workflow_dispatch: {} permissions: pull-requests: read diff --git a/app/config.py b/app/config.py index 5f97eda..9b19f4a 100644 --- a/app/config.py +++ b/app/config.py @@ -1,4 +1,5 @@ '''Shared configuration module''' +from typing import Tuple, Optional import os import redis @@ -8,20 +9,18 @@ REDIS_DB = int(os.environ.get('REDIS_DB', 0)) CACHE_TTL = int(os.environ.get('CACHE_TTL', 300)) -def create_redis_client(): - '''Create and return Redis client with error handling''' +def create_redis_client() -> Tuple[Optional[redis.Redis], bool]: + """Create Redis client and return it with availability status.""" try: - redis_client = redis.StrictRedis( + client = redis.Redis( host=REDIS_HOST, port=REDIS_PORT, db=REDIS_DB, - decode_responses=True, - socket_connect_timeout=240, - socket_timeout=240 + decode_responses=True ) - redis_client.ping() + client.ping() # Test connection print("Connected to Redis successfully!") - return redis_client, True + return client, True except (redis.ConnectionError, redis.TimeoutError) as e: print(f"Could not connect to Redis: {e}") return None, False diff --git a/app/opensense.py b/app/opensense.py index bc928b0..cbecab6 100644 --- a/app/opensense.py +++ b/app/opensense.py @@ -8,7 +8,12 @@ from ijson.common import JSONError as IjsonJSONError from app.config import create_redis_client, CACHE_TTL -redis_client, REDIS_AVAILABLE = create_redis_client() +try: + REDIS_CLIENT, REDIS_AVAILABLE = create_redis_client() +except (redis.ConnectionError, redis.TimeoutError, OSError, ImportError) as e: + print(f"Warning: Redis client creation failed - {e}") + REDIS_CLIENT = None + REDIS_AVAILABLE = False _sensor_stats = {"total_sensors": 0, "null_count": 0} @@ -61,21 +66,25 @@ def _empty_stats() -> Dict[str, int]: return {"total_sensors": 0, "null_count": 0} def _get_cached_temperature() -> Optional[str]: + """Return cached temperature string if present, else None.""" if not REDIS_AVAILABLE: return None try: - cached = redis_client.get("temperature_data") - if cached: - return cached.decode("utf-8") if isinstance(cached, (bytes, bytearray)) else cached + cached_temp = REDIS_CLIENT.get("temperature_data") + if cached_temp: + return cached_temp.decode("utf-8") if isinstance( + cached_temp, (bytes, bytearray) + ) else cached_temp except redis.RedisError: return None return None def _set_cached_temperature(value: str) -> None: + """Store only the temperature string in cache.""" if not REDIS_AVAILABLE: return try: - redis_client.setex("temperature_data", CACHE_TTL, value) + REDIS_CLIENT.setex("temperature_data", CACHE_TTL, value) except redis.RedisError: pass @@ -84,15 +93,19 @@ def _build_params() -> Dict[str, str]: return {"date": time_iso, "format": "json"} def _request_boxes(params: Dict[str, str]): + """Perform a single request and return the response or an error string.""" try: resp = requests.get( "https://api.opensensemap.org/boxes", params=params, stream=True, - timeout=(60, 90) + timeout=(5, 30), + headers={ + 'User-Agent': 'HiveBox-Project/1.0', + 'Accept': 'application/json' + } ) - if hasattr(resp, "raise_for_status"): - resp.raise_for_status() + resp.raise_for_status() if hasattr(resp, "raw") and hasattr(resp.raw, "decode_content"): resp.raw.decode_content = True return resp, None @@ -113,8 +126,9 @@ def _make_sensor_iter(response) -> Tuple[Optional[Iterable[dict]], Optional[str] def get_temperature(): '''Function to get the average temperature from OpenSenseMap API.''' + cached = _get_cached_temperature() - if cached: + if cached is not None: return cached, _empty_stats() params = _build_params() @@ -137,4 +151,4 @@ def get_temperature(): _set_cached_temperature(result) _sensor_stats.update(stats) - return result, _sensor_stats + return result, stats diff --git a/app/readiness.py b/app/readiness.py index d638608..d20f80d 100644 --- a/app/readiness.py +++ b/app/readiness.py @@ -5,7 +5,7 @@ from app.opensense import get_temperature from app.config import create_redis_client -redis_client, REDIS_AVAILABLE = create_redis_client() +REDIS_CLIENT, REDIS_AVAILABLE = create_redis_client() def check_caching(): '''Check if caching content is older than 5 minutes''' @@ -14,7 +14,7 @@ def check_caching(): try: cache_key = "temperature_data" - ttl = redis_client.ttl(cache_key) + ttl = REDIS_CLIENT.ttl(cache_key) if ttl in (-2, -1): return True @@ -64,3 +64,15 @@ def readiness_check(): # If Redis is completely unavailable, still allow the service to be ready print(f"Redis error during readiness check: {e}") return 200 + +def check_redis(): + '''Function to check Redis is Up''' + if REDIS_CLIENT: + try: + if REDIS_CLIENT.ping(): + return '

Redis is available ✔

', True + return '

Redis ping failed ❌

', False + except redis.RedisError as e: + return f'

Redis connection failed ❌: {e}

', False + else: + return '

Redis is not configured ❌

', False diff --git a/tests/test_modules.py b/tests/test_modules.py index 0eaa1e0..509c48d 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -5,6 +5,7 @@ import unittest.mock as mock import io import json +import importlib import requests import redis from minio.error import S3Error, InvalidResponseError @@ -82,6 +83,7 @@ class MockOpenSenseResponse: def __init__(self, temp_value): self.text = "mock response text" self.temp_value = temp_value + self.status_code = 200 # Add this attribute payload = json.dumps([{ 'sensors': [ { @@ -174,7 +176,7 @@ def test_cache_hit(self): mock_redis_client.get.return_value = "cached_result" with mock.patch('app.opensense.REDIS_AVAILABLE', True), \ - mock.patch('app.opensense.redis_client', mock_redis_client), \ + mock.patch('app.opensense.REDIS_CLIENT', mock_redis_client), \ mock.patch('app.opensense.requests.get') as mock_requests: result, _ = opensense.get_temperature() @@ -188,7 +190,7 @@ def test_cache_miss_and_store(self): mock_redis_client.get.return_value = None with mock.patch('app.opensense.REDIS_AVAILABLE', True), \ - mock.patch('app.opensense.redis_client', mock_redis_client), \ + mock.patch('app.opensense.REDIS_CLIENT', mock_redis_client), \ mock.patch('app.opensense.requests.get', return_value=MockOpenSenseResponse(25)): @@ -205,7 +207,7 @@ def test_cache_hit_bytes_decoded(self): mock_redis_client = mock.MagicMock() mock_redis_client.get.return_value = b"cached_result" with mock.patch('app.opensense.REDIS_AVAILABLE', True), \ - mock.patch('app.opensense.redis_client', mock_redis_client), \ + mock.patch('app.opensense.REDIS_CLIENT', mock_redis_client), \ mock.patch('app.opensense.requests.get') as req_get: result, stats = opensense.get_temperature() self.assertEqual(result, "cached_result") @@ -404,7 +406,7 @@ def test_check_caching_key_not_exists(self): mock_redis_client.ttl.return_value = -2 # Key doesn't exist with mock.patch('app.readiness.REDIS_AVAILABLE', True), \ - mock.patch('app.readiness.redis_client', mock_redis_client): + mock.patch('app.readiness.REDIS_CLIENT', mock_redis_client): result = readiness.check_caching() self.assertTrue(result) @@ -414,7 +416,7 @@ def test_check_caching_key_no_expiry(self): mock_redis_client.ttl.return_value = -1 # Key exists but no expiry with mock.patch('app.readiness.REDIS_AVAILABLE', True), \ - mock.patch('app.readiness.redis_client', mock_redis_client): + mock.patch('app.readiness.REDIS_CLIENT', mock_redis_client): result = readiness.check_caching() self.assertTrue(result) @@ -424,7 +426,7 @@ def test_check_caching_fresh_cache(self): mock_redis_client.ttl.return_value = 150 # 2.5 minutes remaining with mock.patch('app.readiness.REDIS_AVAILABLE', True), \ - mock.patch('app.readiness.redis_client', mock_redis_client): + mock.patch('app.readiness.REDIS_CLIENT', mock_redis_client): result = readiness.check_caching() self.assertFalse(result) # Cache is fresh @@ -434,7 +436,7 @@ def test_check_caching_redis_error(self): mock_redis_client.ttl.side_effect = redis.RedisError("boom") with mock.patch('app.readiness.REDIS_AVAILABLE', True), \ - mock.patch('app.readiness.redis_client', mock_redis_client), \ + mock.patch('app.readiness.REDIS_CLIENT', mock_redis_client), \ mock.patch('builtins.print'): result = readiness.check_caching() self.assertTrue(result) @@ -529,5 +531,45 @@ def test_readiness_check_partial_failure(self): self.assertEqual(result, 200) +class TestOpenSenseExceptionBranches(unittest.TestCase): + """Covers exception branches in opensense module.""" + + def test_set_cached_temperature_redis_error_is_swallowed(self): + """_set_cached_temperature should swallow RedisError (pass).""" + with mock.patch('app.opensense.REDIS_AVAILABLE', True), \ + mock.patch('app.opensense.REDIS_CLIENT') as mock_client: + mock_client.setex.side_effect = redis.RedisError("boom") + # Should not raise + opensense._set_cached_temperature("value") + mock_client.setex.assert_called_once() + + def test_get_cached_temperature_redis_error_returns_none(self): + """_get_cached_temperature should return None on RedisError.""" + with mock.patch('app.opensense.REDIS_AVAILABLE', True), \ + mock.patch('app.opensense.REDIS_CLIENT') as mock_client: + mock_client.get.side_effect = redis.RedisError("boom") + result = opensense._get_cached_temperature() + self.assertIsNone(result) + mock_client.get.assert_called_once_with("temperature_data") + + def test_module_level_redis_init_errors_are_caught_all_types(self): + """create_redis_client exceptions at import-time are handled (all types).""" + for exc in ( + redis.ConnectionError("c"), + redis.TimeoutError("t"), + OSError("o"), + ImportError("i")): + with self.subTest(exc=type(exc).__name__), \ + mock.patch('app.config.create_redis_client', side_effect=exc), \ + mock.patch('builtins.print'): + reloaded = importlib.reload(opensense) + self.assertIsNone(reloaded.REDIS_CLIENT) + self.assertFalse(reloaded.REDIS_AVAILABLE) + # Restore module for other tests + with mock.patch('app.config.create_redis_client', return_value=(None, False)), \ + mock.patch('builtins.print'): + importlib.reload(opensense) + + if __name__ == '__main__': unittest.main()