diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9805d260..83212255 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,13 @@ Change Log Unreleased ---------- +Fixed +~~~~~ +* Updated time metadata to include UTC timezone. The original implementation used utcnow(), which could give different results if the time were ever interpreted to be local time. See https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow + +Changed +~~~~~~~ +* Updated send_event with an optional time argument to be used as metadata. [4.1.1] - 2023-01-23 --------------------- diff --git a/openedx_events/data.py b/openedx_events/data.py index 8d1f9069..7edd0afc 100644 --- a/openedx_events/data.py +++ b/openedx_events/data.py @@ -5,7 +5,7 @@ pattern. """ import socket -from datetime import datetime +from datetime import datetime, timezone from uuid import UUID, uuid1 import attr @@ -14,6 +14,17 @@ import openedx_events +def _ensure_utc_time(_, attribute, value): + """ + Ensure the value is a UTC datetime. + + Note: Meant to be used along-side an instance_of attr validator. + """ + if value.tzinfo and value.tzinfo == timezone.utc: + return + raise ValueError(f"'{attribute.name}' must have timezone.utc") + + @attr.s(frozen=True) class EventsMetadata: """ @@ -28,8 +39,12 @@ class EventsMetadata: minorversion (int): version of the event type. source (str): logical source of an event. sourcehost (str): physical source of the event. - time (datetime): timestamp when the event was sent. - sourcelib (str): Open edX Events library version. + time (datetime): (optional) timestamp when the event was sent with + UTC timezone. Defaults to current time in UTC. See OEP-41 for + details. + sourcelib (tuple of ints): Open edX Events library version. A tuple was + selected so that version comparisons don't have to worry about + lexical ordering of strings (e.g. '0.9.0' vs. '0.10.0'). """ id = attr.ib(type=UUID, init=False) @@ -37,7 +52,13 @@ class EventsMetadata: minorversion = attr.ib(type=int, converter=attr.converters.default_if_none(0)) source = attr.ib(type=str, init=False) sourcehost = attr.ib(type=str, init=False) - time = attr.ib(type=datetime, init=False) + current_utc_time = datetime.now(timezone.utc) + time = attr.ib( + type=datetime, + default=None, + converter=attr.converters.default_if_none(attr.Factory(lambda: datetime.now(timezone.utc))), + validator=attr.validators.optional([attr.validators.instance_of(datetime), _ensure_utc_time]), + ) sourcelib = attr.ib(type=tuple, init=False) def __attrs_post_init__(self): @@ -56,7 +77,6 @@ def __attrs_post_init__(self): ), ) object.__setattr__(self, "sourcehost", socket.gethostname()) - object.__setattr__(self, "time", datetime.utcnow()) object.__setattr__( self, "sourcelib", tuple(map(int, openedx_events.__version__.split("."))) ) diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index 3f354ecf..9675713e 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -109,5 +109,6 @@ def test_default_does_nothing(self): # Nothing thrown, no warnings. assert producer.send( signal=SESSION_LOGIN_COMPLETED, topic='user-logins', - event_key_field='user.id', event_data={}, event_metadata=EventsMetadata(event_type='eh', minorversion=0) + event_key_field='user.id', event_data={}, + event_metadata=EventsMetadata(event_type='eh', minorversion=0) ) is None diff --git a/openedx_events/tests/test_tooling.py b/openedx_events/tests/test_tooling.py index b64d876a..94f7bde5 100644 --- a/openedx_events/tests/test_tooling.py +++ b/openedx_events/tests/test_tooling.py @@ -4,8 +4,10 @@ Classes: EventsToolingTest: Test events tooling. """ +import datetime from contextlib import contextmanager from unittest.mock import Mock, patch +from uuid import UUID import attr import ddt @@ -85,7 +87,8 @@ def test_get_signal_by_type(self): @override_settings(SERVICE_VARIANT="lms") @patch("openedx_events.data.openedx_events") @patch("openedx_events.data.socket") - def test_get_signal_metadata(self, socket_mock, events_package_mock): + @patch("openedx_events.data.datetime") + def test_generate_signal_metadata(self, datetime_mock, socket_mock, events_package_mock): """ This methods tests getting the generated metadata for an event. @@ -94,23 +97,72 @@ def test_get_signal_metadata(self, socket_mock, events_package_mock): """ events_package_mock.__version__ = "0.1.0" socket_mock.gethostname.return_value = "edx.devstack.lms" + expected_time = datetime.datetime.now(datetime.timezone.utc) + datetime_mock.now.return_value = expected_time expected_metadata = { "event_type": self.event_type, "minorversion": 0, "source": "openedx/lms/web", "sourcehost": "edx.devstack.lms", "sourcelib": [0, 1, 0], + "time": expected_time, } metadata = self.public_signal.generate_signal_metadata() self.assertDictContainsSubset(expected_metadata, attr.asdict(metadata)) + self.assertIsInstance(metadata.id, UUID) + + @override_settings(SERVICE_VARIANT="lms") + @patch("openedx_events.data.openedx_events") + @patch("openedx_events.data.socket") + def test_generate_signal_metadata_with_valid_time(self, socket_mock, events_package_mock): + """ + Tests getting the generated metadata for an event, providing a valid time in UTC. + + Expected behavior: + Returns the metadata containing information about the event. + """ + events_package_mock.__version__ = "0.1.0" + socket_mock.gethostname.return_value = "edx.devstack.lms" + expected_time = datetime.datetime.now(datetime.timezone.utc) + expected_metadata = { + "event_type": self.event_type, + "minorversion": 0, + "source": "openedx/lms/web", + "sourcehost": "edx.devstack.lms", + "sourcelib": [0, 1, 0], + "time": expected_time, + } + + metadata = self.public_signal.generate_signal_metadata(time=expected_time) + + self.assertDictContainsSubset(expected_metadata, attr.asdict(metadata)) + self.assertIsInstance(metadata.id, UUID) + + @ddt.data( + (1, TypeError, "'time' must be : \nfake-output" ) + @patch("openedx_events.tooling.OpenEdxPublicSignal.generate_signal_metadata") + def test_send_event_with_time(self, fake_metadata): + """ + This method tests the process of sending an event with a time argument. + + Expected behavior: + The generate_signal_metadata is called using the passed time. + """ + expected_metadata = Mock(some_data="some_data") + expected_time = datetime.datetime.now(datetime.timezone.utc) + fake_metadata.return_value = expected_metadata + + self.public_signal.send_event(user=self.user_mock, time=expected_time) + + # generate_signal_metadata is fully tested elsewhere + fake_metadata.assert_called_once_with(time=expected_time) + @ddt.data( ( {"student": Mock()}, @@ -190,7 +259,7 @@ def test_send_event_with_django(self): method. Expected behavior: - A warning is showed advicing to use Open edX events custom + A warning is showed advising to use Open edX events custom send_signal method. """ message = "Please, use 'send_event' when triggering an Open edX event." diff --git a/openedx_events/tooling.py b/openedx_events/tooling.py index 1a30598c..cfddc262 100644 --- a/openedx_events/tooling.py +++ b/openedx_events/tooling.py @@ -66,13 +66,18 @@ def get_signal_by_type(cls, event_type): """ return cls._mapping[event_type] - def generate_signal_metadata(self): + def generate_signal_metadata(self, time=None): """ Generate signal metadata when an event is sent. These fields are generated on the fly and are a subset of the Event Message defined in the OEP-41. + Arguments: + time (datetime): (Optional) Timestamp when the event was sent with + UTC timezone. Defaults to current time in UTC. See OEP-41 for + details. + Example usage: >>> metadata = \ STUDENT_REGISTRATION_COMPLETED.generate_signal_metadata() @@ -90,12 +95,22 @@ def generate_signal_metadata(self): return EventsMetadata( event_type=self.event_type, minorversion=self.minor_version, + time=time, ) - def send_event(self, send_robust=True, **kwargs): + def send_event(self, send_robust=True, time=None, **kwargs): """ Send events to all connected receivers. + Arguments: + send_robust (bool): Defaults to True. See Django signal docs. + time (datetime): (Optional - see note) Timestamp when the event was + sent with UTC timezone. For events requiring a DB create or + update, use the timestamp from the DB record. Defaults to + current time in UTC. This argument is optional for backward + compatability, but ideally would be explicitly set. See OEP-41 + for details. + Used to send events just like Django signals are sent. In addition, some validations are executed on the arguments, and then generates relevant metadata that can be used for logging or debugging purposes. Besides this behavior, @@ -158,7 +173,7 @@ def validate_sender(): validate_sender() - kwargs["metadata"] = self.generate_signal_metadata() + kwargs["metadata"] = self.generate_signal_metadata(time=time) if self._allow_send_event_failure or settings.DEBUG or not send_robust: return super().send(sender=None, **kwargs)