Skip to content

Commit ecb7ef6

Browse files
committed
added traffic type validations and unit tests for it
1 parent e4c1104 commit ecb7ef6

File tree

4 files changed

+68
-2
lines changed

4 files changed

+68
-2
lines changed

splitio/client/client.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,14 @@ def track(self, key, traffic_type, event_type, value=None):
337337

338338
key = input_validator.validate_track_key(key)
339339
event_type = input_validator.validate_event_type(event_type)
340-
traffic_type = input_validator.validate_traffic_type(traffic_type)
340+
should_validate_existance = self.ready and self._factory._apikey != 'localhost' #pylint: disable=protected-access
341+
print(self.ready, self._factory._apikey, should_validate_existance)
342+
traffic_type = input_validator.validate_traffic_type(
343+
traffic_type,
344+
should_validate_existance,
345+
self._factory._get_storage('splits'), #pylint: disable=protected-access
346+
)
347+
341348
value = input_validator.validate_value(value)
342349

343350
if key is None or event_type is None or traffic_type is None or value is False:

splitio/client/input_validator.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,12 +288,16 @@ def validate_track_key(key):
288288
return key_str
289289

290290

291-
def validate_traffic_type(traffic_type):
291+
def validate_traffic_type(traffic_type, should_validate_existance, split_storage):
292292
"""
293293
Check if traffic_type is valid for track.
294294
295295
:param traffic_type: traffic_type to be checked
296296
:type traffic_type: str
297+
:param should_validate_existance: Whether to check for existante in the split storage.
298+
:type should_validate_existance: bool
299+
:param split_storage: Split storage.
300+
:param split_storage: splitio.storages.SplitStorage
297301
:return: traffic_type
298302
:rtype: str|None
299303
"""
@@ -305,6 +309,15 @@ def validate_traffic_type(traffic_type):
305309
_LOGGER.warning('track: %s should be all lowercase - converting string to lowercase.',
306310
traffic_type)
307311
traffic_type = traffic_type.lower()
312+
313+
if should_validate_existance and not split_storage.is_valid_traffic_type(traffic_type):
314+
_LOGGER.warning(
315+
'track: Traffic Type %s does not have any corresponding Splits in this environment, '
316+
'make sure you\'re tracking your events to a valid traffic type defined '
317+
'in the Split console.',
318+
traffic_type
319+
)
320+
308321
return traffic_type
309322

310323

tests/client/test_client.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ def _get_storage_mock(name):
318318
destroyed_mock = mocker.PropertyMock()
319319
destroyed_mock.return_value = False
320320
type(factory).destroyed = destroyed_mock
321+
factory._apikey = 'test'
321322
mocker.patch('splitio.client.client.time.time', new=lambda: 1)
322323

323324
client = Client(factory)

tests/client/test_input_validator.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ def test_track(self, mocker):
453453
factory_destroyed = mocker.PropertyMock()
454454
factory_destroyed.return_value = False
455455
type(factory_mock).destroyed = factory_destroyed
456+
factory_mock._apikey = 'some-test'
456457

457458
client = Client(factory_mock)
458459
client._events_storage = mocker.Mock(spec=EventStorage)
@@ -602,6 +603,50 @@ def test_track(self, mocker):
602603
mocker.call("track: value must be a number.")
603604
]
604605

606+
# Test traffic type existance
607+
ready_property = mocker.PropertyMock()
608+
ready_property.return_value = True
609+
type(factory_mock).ready = ready_property
610+
611+
split_storage_mock = mocker.Mock(spec=SplitStorage)
612+
split_storage_mock.is_valid_traffic_type.return_value = True
613+
factory_mock._get_storage.return_value = split_storage_mock
614+
615+
# Test that it doesn't warn if tt is cached, not in localhost mode and sdk is ready
616+
client._logger.reset_mock()
617+
assert client.track("some_key", "traffic_type", "event_type", None) is True
618+
assert client._logger.error.mock_calls == []
619+
assert client._logger.warning.mock_calls == []
620+
621+
# Test that it does warn if tt is cached, not in localhost mode and sdk is ready
622+
split_storage_mock.is_valid_traffic_type.return_value = False
623+
client._logger.reset_mock()
624+
assert client.track("some_key", "traffic_type", "event_type", None) is True
625+
assert client._logger.error.mock_calls == []
626+
assert client._logger.warning.mock_calls == [mocker.call(
627+
'track: Traffic Type %s does not have any corresponding Splits in this environment, '
628+
'make sure you\'re tracking your events to a valid traffic type defined '
629+
'in the Split console.',
630+
'traffic_type'
631+
)]
632+
633+
# Test that it does not warn when in localhost mode.
634+
factory_mock._apikey = 'localhost'
635+
client._logger.reset_mock()
636+
assert client.track("some_key", "traffic_type", "event_type", None) is True
637+
assert client._logger.error.mock_calls == []
638+
assert client._logger.warning.mock_calls == []
639+
640+
# Test that it does not warn when not in localhost mode and not ready
641+
factory_mock._apikey = 'not-localhost'
642+
ready_property.return_value = False
643+
type(factory_mock).ready = ready_property
644+
client._logger.reset_mock()
645+
assert client.track("some_key", "traffic_type", "event_type", None) is True
646+
assert client._logger.error.mock_calls == []
647+
assert client._logger.warning.mock_calls == []
648+
649+
605650
def test_get_treatments(self, mocker):
606651
"""Test getTreatments() method."""
607652
split_mock = mocker.Mock(spec=Split)

0 commit comments

Comments
 (0)