Merge pull request #78 from GabrielPalmar/test

fix(app): Trying fixes

Author: GabrielPalmar

.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

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

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

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 '<p>Redis is available &#10004;</p>', True
+            return '<p>Redis ping failed &#10060;</p>', False
+        except redis.RedisError as e:
+            return f'<p>Redis connection failed &#10060;: {e}</p>', False
+    else:
+        return '<p>Redis is not configured &#10060;</p>', False

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()