Support HTTP 304 on /client/feature requests (#176)

* Add cache key for etag.

* Parse etag from feature request.

* Add caching for etag header.

* Use cached value to set If-None-Match header if present.

* Add e2e test for caching.

* Don't send metrics if there's nothing to report.

* Fix issue with test cache generation, linting.

UnleashClient/__init__.py

@@ -9,7 +9,7 @@
 from UnleashClient.periodic_tasks import fetch_and_load_features, aggregate_and_send_metrics
 from UnleashClient.strategies import ApplicationHostname, Default, GradualRolloutRandom, \
     GradualRolloutSessionId, GradualRolloutUserId, UserWithId, RemoteAddress, FlexibleRollout
-from UnleashClient.constants import METRIC_LAST_SENT_TIME, DISABLED_VARIATION
+from UnleashClient.constants import METRIC_LAST_SENT_TIME, DISABLED_VARIATION, ETAG
 from .utils import LOGGER
 from .deprecation_warnings import strategy_v2xx_deprecation_check
 
@@ -82,6 +82,7 @@ def __init__(self,
         self.fl_job: Job = None
         self.metric_job: Job = None
         self.cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
+        self.cache[ETAG] = ''
         self.cache.sync()
 
         # Mappings

UnleashClient/api/features.py

@@ -1,3 +1,4 @@
+from typing import Tuple
 import requests
 from UnleashClient.constants import REQUEST_TIMEOUT, FEATURES_URL
 from UnleashClient.utils import LOGGER, log_resp_info
@@ -9,7 +10,8 @@ def get_feature_toggles(url: str,
                         instance_id: str,
                         custom_headers: dict,
                         custom_options: dict,
-                        project: str = None) -> dict:
+                        project: str = None,
+                        cached_etag: str = '') -> Tuple[dict, str]:
     """
     Retrieves feature flags from unleash central server.
 
@@ -23,7 +25,8 @@ def get_feature_toggles(url: str,
     :param custom_headers:
     :param custom_options:
     :param project:
-    :return: Feature flags if successful, empty dict if not.
+    :param cached_etag:
+    :return: (Feature flags, etag) if successful, ({},'') if not
     """
     try:
         LOGGER.info("Getting feature flag.")
@@ -33,6 +36,9 @@ def get_feature_toggles(url: str,
             "UNLEASH-INSTANCEID": instance_id
         }
 
+        if cached_etag:
+            headers['If-None-Match'] = cached_etag
+
         base_url = f"{url}{FEATURES_URL}"
         base_params = {}
 
@@ -44,13 +50,17 @@ def get_feature_toggles(url: str,
                             params=base_params,
                             timeout=REQUEST_TIMEOUT, **custom_options)
 
-        if resp.status_code != 200:
+        if resp.status_code not in [200, 304]:
             log_resp_info(resp)
             LOGGER.warning("Unleash Client feature fetch failed due to unexpected HTTP status code.")
             raise Exception("Unleash Client feature fetch failed!")
 
-        return resp.json()
+        etag = ''
+        if 'etag' in resp.headers.keys():
+            etag = resp.headers['etag']
+
+        return resp.json(), etag
     except Exception as exc:
         LOGGER.exception("Unleash Client feature fetch failed due to exception: %s", exc)
 
-    return {}
+    return {}, ''

UnleashClient/constants.py

@@ -18,3 +18,4 @@
 
 # Cache keys
 FAILED_STRATEGIES = 'failed_strategies'
+ETAG = 'etag'

UnleashClient/periodic_tasks/fetch_and_load.py

@@ -1,7 +1,7 @@
 from fcache.cache import FileCache
 from UnleashClient.api import get_feature_toggles
 from UnleashClient.loader import load_features
-from UnleashClient.constants import FEATURES_URL
+from UnleashClient.constants import FEATURES_URL, ETAG
 from UnleashClient.utils import LOGGER
 
 
@@ -14,12 +14,24 @@ def fetch_and_load_features(url: str,
                             features: dict,
                             strategy_mapping: dict,
                             project: str = None) -> None:
-    feature_provisioning = get_feature_toggles(url, app_name, instance_id, custom_headers, custom_options, project)
+    (feature_provisioning, etag) = get_feature_toggles(
+        url,
+        app_name,
+        instance_id,
+        custom_headers,
+        custom_options,
+        project,
+        cache[ETAG]
+    )
 
     if feature_provisioning:
         cache[FEATURES_URL] = feature_provisioning
         cache.sync()
     else:
         LOGGER.warning("Unable to get feature flag toggles, using cached provisioning.")
 
+    if etag:
+        cache[ETAG] = etag
+        cache.sync()
+
     load_features(cache, features, strategy_mapping)

UnleashClient/periodic_tasks/send_metrics.py

@@ -3,6 +3,7 @@
 from fcache.cache import FileCache
 from UnleashClient.api import send_metrics
 from UnleashClient.constants import METRIC_LAST_SENT_TIME
+from UnleashClient.utils import LOGGER
 
 
 def aggregate_and_send_metrics(url: str,
@@ -39,6 +40,9 @@ def aggregate_and_send_metrics(url: str,
         }
     }
 
-    send_metrics(url, metrics_request, custom_headers, custom_options)
-    ondisk_cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
-    ondisk_cache.sync()
+    if feature_stats_list:
+        send_metrics(url, metrics_request, custom_headers, custom_options)
+        ondisk_cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
+        ondisk_cache.sync()
+    else:
+        LOGGER.debug("No feature flags with metrics, skipping metrics submission.")

tests/unit_tests/api/test_feature.py

@@ -1,7 +1,7 @@
 import responses
 from pytest import mark, param
 from tests.utilities.mocks.mock_features import MOCK_FEATURE_RESPONSE, MOCK_FEATURE_RESPONSE_PROJECT
-from tests.utilities.testing_constants import URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, PROJECT_URL, PROJECT_NAME
+from tests.utilities.testing_constants import URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, PROJECT_URL, PROJECT_NAME, ETAG_VALUE
 from UnleashClient.constants import FEATURES_URL
 from UnleashClient.api import get_feature_toggles
 
@@ -16,28 +16,62 @@
     param({}, 500, lambda result: not result, id="failure"),
 ))
 def test_get_feature_toggle(response, status, expected):
-    responses.add(responses.GET, FULL_FEATURE_URL, json=response, status=status)
+    responses.add(responses.GET, FULL_FEATURE_URL, json=response, status=status, headers={'etag': ETAG_VALUE})
 
-    result = get_feature_toggles(URL,
-                                 APP_NAME,
-                                 INSTANCE_ID,
-                                 CUSTOM_HEADERS,
-                                 CUSTOM_OPTIONS)
+    (result, etag) = get_feature_toggles(URL,
+                                         APP_NAME,
+                                         INSTANCE_ID,
+                                         CUSTOM_HEADERS,
+                                         CUSTOM_OPTIONS)
 
     assert len(responses.calls) == 1
     assert expected(result)
 
 
 @responses.activate
 def test_get_feature_toggle_project():
-    responses.add(responses.GET, PROJECT_URL, json=MOCK_FEATURE_RESPONSE_PROJECT, status=200)
+    responses.add(responses.GET, PROJECT_URL, json=MOCK_FEATURE_RESPONSE_PROJECT, status=200, headers={'etag': ETAG_VALUE})
 
-    result = get_feature_toggles(URL,
-                                 APP_NAME,
-                                 INSTANCE_ID,
-                                 CUSTOM_HEADERS,
-                                 CUSTOM_OPTIONS,
-                                 PROJECT_NAME)
+    (result, etag) = get_feature_toggles(URL,
+                                         APP_NAME,
+                                         INSTANCE_ID,
+                                         CUSTOM_HEADERS,
+                                         CUSTOM_OPTIONS,
+                                         PROJECT_NAME)
 
     assert len(responses.calls) == 1
     assert len(result["features"]) == 1
+    assert etag == ETAG_VALUE
+
+
+@responses.activate
+def test_get_feature_toggle_failed_etag():
+    responses.add(responses.GET, PROJECT_URL, json={}, status=500, headers={'etag': ETAG_VALUE})
+
+    (result, etag) = get_feature_toggles(URL,
+                                         APP_NAME,
+                                         INSTANCE_ID,
+                                         CUSTOM_HEADERS,
+                                         CUSTOM_OPTIONS,
+                                         PROJECT_NAME)
+
+    assert len(responses.calls) == 1
+    assert etag == ''
+
+
+@responses.activate
+def test_get_feature_toggle_etag_present():
+    responses.add(responses.GET, PROJECT_URL, json={}, status=304, headers={'etag': ETAG_VALUE})
+
+    (result, etag) = get_feature_toggles(URL,
+                                         APP_NAME,
+                                         INSTANCE_ID,
+                                         CUSTOM_HEADERS,
+                                         CUSTOM_OPTIONS,
+                                         PROJECT_NAME,
+                                         ETAG_VALUE)
+
+    assert len(responses.calls) == 1
+    assert not result
+    assert responses.calls[0].request.headers['If-None-Match'] == ETAG_VALUE
+    assert etag == ETAG_VALUE

tests/unit_tests/periodic/test_aggregate_and_send_metrics.py

@@ -45,3 +45,23 @@ def test_aggregate_and_send_metrics():
     assert request['bucket']["toggles"]["My Feature1"]["no"] == 1
     assert "My Feature3" not in request['bucket']["toggles"].keys()
     assert cache[METRIC_LAST_SENT_TIME] > start_time
+
+
+@responses.activate
+def test_no_metrics():
+    responses.add(responses.POST, FULL_METRICS_URL, json={}, status=200)
+
+    start_time = datetime.now(timezone.utc) - timedelta(seconds=60)
+    cache = FileCache("TestCache")
+    cache[METRIC_LAST_SENT_TIME] = start_time
+    strategies = [RemoteAddress(parameters={"IPs": IP_LIST}), Default()]
+
+    my_feature1 = Feature("My Feature1", True, strategies)
+    my_feature1.yes_count = 0
+    my_feature1.no_count = 0
+
+    features = {"My Feature1": my_feature1}
+
+    aggregate_and_send_metrics(URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, features, cache)
+
+    assert len(responses.calls) == 0

tests/unit_tests/periodic/test_fetch_and_load.py

@@ -1,9 +1,9 @@
 import responses
-from UnleashClient.constants import FEATURES_URL
+from UnleashClient.constants import FEATURES_URL, ETAG
 from UnleashClient.periodic_tasks import fetch_and_load_features
 from UnleashClient.features import Feature
 from tests.utilities.mocks.mock_features import MOCK_FEATURE_RESPONSE, MOCK_FEATURE_RESPONSE_PROJECT
-from tests.utilities.testing_constants import URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, DEFAULT_STRATEGY_MAPPING, PROJECT_URL, PROJECT_NAME
+from tests.utilities.testing_constants import URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, DEFAULT_STRATEGY_MAPPING, PROJECT_URL, PROJECT_NAME, ETAG_VALUE
 from tests.utilities.decorators import cache_empty  # noqa: F401
 
 
@@ -14,7 +14,7 @@
 def test_fetch_and_load(cache_empty):  # noqa: F811
     # Set up for tests
     in_memory_features = {}
-    responses.add(responses.GET, FULL_FEATURE_URL, json=MOCK_FEATURE_RESPONSE, status=200)
+    responses.add(responses.GET, FULL_FEATURE_URL, json=MOCK_FEATURE_RESPONSE, status=200, headers={'etag': ETAG_VALUE})
     temp_cache = cache_empty
 
     fetch_and_load_features(URL,
@@ -27,6 +27,7 @@ def test_fetch_and_load(cache_empty):  # noqa: F811
                             DEFAULT_STRATEGY_MAPPING)
 
     assert isinstance(in_memory_features["testFlag"], Feature)
+    assert temp_cache[ETAG] == ETAG_VALUE
 
 
 @responses.activate

tests/unit_tests/test_client.py

@@ -5,7 +5,8 @@
 from UnleashClient import UnleashClient
 from UnleashClient.strategies import Strategy
 from tests.utilities.testing_constants import URL, ENVIRONMENT, APP_NAME, INSTANCE_ID, REFRESH_INTERVAL, REFRESH_JITTER, \
-    METRICS_INTERVAL, METRICS_JITTER, DISABLE_METRICS, DISABLE_REGISTRATION, CUSTOM_HEADERS, CUSTOM_OPTIONS, PROJECT_NAME, PROJECT_URL
+    METRICS_INTERVAL, METRICS_JITTER, DISABLE_METRICS, DISABLE_REGISTRATION, CUSTOM_HEADERS, CUSTOM_OPTIONS, PROJECT_NAME, PROJECT_URL, \
+    ETAG_VALUE
 from tests.utilities.mocks.mock_features import MOCK_FEATURE_RESPONSE, MOCK_FEATURE_RESPONSE_PROJECT
 from tests.utilities.mocks.mock_all_features import MOCK_ALL_FEATURES
 from UnleashClient.constants import REGISTER_URL, FEATURES_URL, METRICS_URL
@@ -125,7 +126,7 @@ def test_UC_type_violation():
 def test_uc_lifecycle(unleash_client):
     # Set up API
     responses.add(responses.POST, URL + REGISTER_URL, json={}, status=202)
-    responses.add(responses.GET, URL + FEATURES_URL, json=MOCK_FEATURE_RESPONSE, status=200)
+    responses.add(responses.GET, URL + FEATURES_URL, json=MOCK_FEATURE_RESPONSE, status=200, headers={'etag': ETAG_VALUE})
     responses.add(responses.POST, URL + METRICS_URL, json={}, status=202)
 
     # Create Unleash client and check initial load
@@ -134,8 +135,12 @@ def test_uc_lifecycle(unleash_client):
     assert unleash_client.is_initialized
     assert len(unleash_client.features) >= 4
 
+    # Simulate caching
+    responses.add(responses.GET, URL + FEATURES_URL, json={}, status=304, headers={'etag': ETAG_VALUE})
+    time.sleep(16)
+
     # Simulate server provisioning change
-    responses.add(responses.GET, URL + FEATURES_URL, json=MOCK_ALL_FEATURES, status=200)
+    responses.add(responses.GET, URL + FEATURES_URL, json=MOCK_ALL_FEATURES, status=200, headers={'etag': 'W/somethingelse'})
     time.sleep(30)
     assert len(unleash_client.features) >= 9
 

tests/utilities/decorators.py

@@ -1,14 +1,17 @@
 import pytest
 import uuid
+from datetime import datetime, timezone
 from fcache.cache import FileCache
-from UnleashClient.constants import FEATURES_URL
+from UnleashClient.constants import FEATURES_URL, METRIC_LAST_SENT_TIME, ETAG
 from tests.utilities.mocks import MOCK_ALL_FEATURES, MOCK_CUSTOM_STRATEGY
 
 
 @pytest.fixture()
 def cache_empty():
     cache_name = 'pytest_%s' % uuid.uuid4()
     temporary_cache = FileCache(cache_name)
+    temporary_cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
+    temporary_cache[ETAG] = ''
     yield temporary_cache
     temporary_cache.delete()
 
@@ -18,6 +21,8 @@ def cache_full():
     cache_name = 'pytest_%s' % uuid.uuid4()
     temporary_cache = FileCache(cache_name)
     temporary_cache[FEATURES_URL] = MOCK_ALL_FEATURES
+    temporary_cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
+    temporary_cache[ETAG] = ''
     temporary_cache.sync()
     yield temporary_cache
     temporary_cache.delete()
@@ -28,6 +33,8 @@ def cache_custom():
     cache_name = 'pytest_%s' % uuid.uuid4()
     temporary_cache = FileCache(cache_name)
     temporary_cache[FEATURES_URL] = MOCK_CUSTOM_STRATEGY
+    temporary_cache[METRIC_LAST_SENT_TIME] = datetime.now(timezone.utc)
+    temporary_cache[ETAG] = ''
     temporary_cache.sync()
     yield temporary_cache
     temporary_cache.delete()

tests/utilities/testing_constants.py

@@ -23,6 +23,7 @@
 # Constants
 IP_LIST = "69.208.0.0/29,70.208.1.1,2001:db8:1234::/48,2002:db8:1234:0000:0000:0000:0000:0001"
 PROJECT_NAME = 'ivan'
+ETAG_VALUE = 'W/"730-v0ozrE11zfZK13j7rQ5PxkXfjYQ"'
 
 # Mapping
 DEFAULT_STRATEGY_MAPPING = {