diff --git a/frigate/analytics/reporter.py b/frigate/analytics/reporter.py index 0fc5f7e2c1..4539115c7b 100644 --- a/frigate/analytics/reporter.py +++ b/frigate/analytics/reporter.py @@ -127,6 +127,13 @@ class AnalyticsReporter(threading.Thread): if opted_in: resolve_notice(PROMPT_KIND) + + # a new install ID counts notices from the opt-in, so nothing + # raised before consent is sent and nothing raised after is lost + if load_state(self.state_path) is None and save_state( + new_state(), self.state_path + ): + self.notice_registry.mark_reported(self.notice_registry.stats()) else: raise_notice(PROMPT_KIND) delete_state(self.state_path) @@ -151,8 +158,7 @@ class AnalyticsReporter(threading.Thread): if now < self.first_due: return - loaded = load_state(self.state_path) - state = loaded or new_state() + state = load_state(self.state_path) or new_state() if not self._due(state.last_attempt_at, now): return @@ -179,7 +185,7 @@ class AnalyticsReporter(threading.Thread): self.interval = self._next_interval() # the lock stays free during the request, so a save never waits on it - self._send(state.install_id, now, fresh=loaded is None) + self._send(state.install_id, now) def _due(self, last_attempt_at: float, now: float) -> bool: # a last attempt stamped in the future came from a wrong clock @@ -187,12 +193,7 @@ class AnalyticsReporter(threading.Thread): now >= last_attempt_at + self.interval or last_attempt_at > now + INTERVAL_S ) - def _send(self, install_id: str, now: float, fresh: bool) -> None: - # a new install ID counts notices from the opt-in, so nothing raised - # before consent is sent - if fresh: - self.notice_registry.mark_reported(self.notice_registry.stats()) - + def _send(self, install_id: str, now: float) -> None: notice_stats = self.notice_registry.stats() ctx = ReportContext( config=self.config_holder.config, diff --git a/frigate/test/test_analytics_reporter.py b/frigate/test/test_analytics_reporter.py index eed723b862..b03fc7ddf7 100644 --- a/frigate/test/test_analytics_reporter.py +++ b/frigate/test/test_analytics_reporter.py @@ -150,19 +150,26 @@ class TestAnalyticsReporter(unittest.TestCase): self.send.assert_called_once() self.registry.mark_reported.assert_not_called() - def test_a_new_install_id_skips_notices_from_before_the_opt_in(self): + def test_a_new_install_id_counts_notices_from_the_opt_in(self): before = [{**SNAPSHOT[0], "occurrences": 2}] - self.registry.stats.side_effect = [before, SNAPSHOT] + self.registry.stats.return_value = before reporter = self.reporter() - self.advance(FIRST_DELAY_S[0]) + # consent is seen on the first wake, well before the first send + reporter.tick() + self.registry.mark_reported.assert_called_once_with(before) + self.assertIsNotNone(load_state(self.path)) + self.send.assert_not_called() + + # a notice raised during the startup delay goes out with the first report + self.registry.stats.return_value = SNAPSHOT + self.advance(FIRST_DELAY_S[0]) reporter.tick() - # the reset runs before the report's snapshot, which the 204 then marks + self.assertIs(self.build_report.call_args.args[0].notice_stats, SNAPSHOT) self.assertEqual( self.registry.mark_reported.call_args_list, [call(before), call(SNAPSHOT)] ) - self.assertIs(self.build_report.call_args.args[0].notice_stats, SNAPSHOT) def test_an_opt_out_saved_only_to_the_file_stops_the_send(self): self.seed_state()