ref(hc): Deprecate UserLastActive middleware (#53398)

This middleware is effectively redundant with the UserIP logging (which
already occurs in the authentication middleware, which was already a
requirement for this middleware) with small modification --
consolidating the UserIP and User modifications.

This simplifies the code and covers another HC special casing we'd like
to deprecate.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

pyproject.toml

@@ -1127,7 +1127,6 @@ module = [
     "tests.sentry.middleware.test_ratelimit_middleware",
     "tests.sentry.middleware.test_stats",
     "tests.sentry.middleware.test_subdomain",
-    "tests.sentry.middleware.test_useractive",
     "tests.sentry.models.test_notificationaction",
     "tests.sentry.models.test_notificationsetting",
     "tests.sentry.models.test_organization",

src/sentry/conf/server.py

@@ -326,7 +326,6 @@ MIDDLEWARE: tuple[str, ...] = (
     "sentry.middleware.integrations.IntegrationControlMiddleware",
     "sentry.middleware.api_gateway.ApiGatewayMiddleware",
     "sentry.middleware.customer_domain.CustomerDomainMiddleware",
-    "sentry.middleware.user.UserActiveMiddleware",
     "sentry.middleware.sudo.SudoMiddleware",
     "sentry.middleware.superuser.SuperuserMiddleware",
     "sentry.middleware.locale.SentryLocaleMiddleware",

src/sentry/middleware/user.py

@@ -1,56 +0,0 @@
-from __future__ import annotations
-
-from datetime import timedelta
-from typing import Any
-
-from django.utils import timezone
-from django.utils.deprecation import MiddlewareMixin
-from rest_framework.request import Request
-from rest_framework.response import Response
-
-from sentry.models.user import User
-
-from ..silo import SiloMode
-from . import ViewFunc, get_path
-
-
-class UserActiveMiddleware(MiddlewareMixin):
-    disallowed_paths = (
-        "sentry.web.frontend.generic.frontend_app_static_media",
-        "sentry.web.frontend.generic.static_media",
-        "sentry.web.frontend.organization_avatar",
-        "sentry.web.frontend.project_avatar",
-        "sentry.web.frontend.team_avatar",
-        "sentry.web.frontend.user_avatar",
-        "sentry.web.frontend.js_sdk_loader",
-    )
-
-    def process_view(
-        self,
-        request: Request,
-        view_func: ViewFunc,
-        view_args: Any,
-        view_kwargs: Any,
-    ) -> Response | None:
-        path = get_path(view_func)
-        if not path or path.startswith(self.disallowed_paths):
-            return None
-
-        if not request.user.is_authenticated:
-            return None
-
-        now = timezone.now()
-        freq = timedelta(minutes=5)
-        last_active = request.user.last_active
-
-        if last_active and freq > (now - last_active):
-            return None
-
-        request.user.last_active = now
-        # this also seems redundent with UserIP, can we somehow remove it?
-        if SiloMode.get_current_mode() != SiloMode.REGION:
-            user = User.objects.filter(id=request.user.id).first()
-            if user:
-                user.update(last_active=now)
-
-        return None

src/sentry/services/hybrid_cloud/log/impl.py

@@ -1,10 +1,12 @@
 from __future__ import annotations
 
-from django.db import IntegrityError
+import datetime
 
-from sentry.models import AuditLogEntry, OutboxCategory, OutboxScope, RegionOutbox, UserIP
+from django.db import IntegrityError, router
+
+from sentry.models import AuditLogEntry, OutboxCategory, OutboxScope, RegionOutbox, User, UserIP
 from sentry.services.hybrid_cloud.log import AuditLogEvent, LogService, UserIpEvent
-from sentry.utils import metrics
+from sentry.silo import unguarded_write
 
 
 class DatabaseBackedLogService(LogService):
@@ -23,7 +25,7 @@ class DatabaseBackedLogService(LogService):
                 raise
 
     def record_user_ip(self, *, event: UserIpEvent) -> None:
-        updated, created = UserIP.objects.create_or_update(
+        UserIP.objects.create_or_update(
             user_id=event.user_id,
             ip_address=event.ip_address,
             values=dict(
@@ -32,12 +34,13 @@ class DatabaseBackedLogService(LogService):
                 region_code=event.region_code,
             ),
         )
-        if not created and not updated:
-            # This happens when there is an integrity error adding the UserIP -- such as when user is deleted,
-            # or the ip address does not match the db validation.  This is expected and not an error condition
-            # in low quantities.
-            # TODO: Break the foreign key and simply remove this code path.
-            metrics.incr("hybrid_cloud.audit_log.user_ip_event.stale_event")
+        with unguarded_write(router.db_for_write(User)):
+            # It greatly simplifies testing not to be too aggressive on updating the last_active due to many
+            # comparisons with serializers.
+            User.objects.filter(
+                id=event.user_id,
+                last_active__lt=(event.last_seen - datetime.timedelta(minutes=1)),
+            ).update(last_active=event.last_seen)
 
     def find_last_log(
         self, *, organization_id: int | None, target_object_id: int | None, event: int | None

tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py

@@ -67,9 +67,11 @@ class OrganizationIncidentActivityIndexTest(APITestCase):
             resp = self.get_success_response(
                 incident.organization.slug, incident.identifier, desc=0
             )
+
         assert resp.data == expected
 
         expected.reverse()
         with self.feature("organizations:incidents"):
             resp = self.get_success_response(incident.organization.slug, incident.identifier)
+
         assert resp.data == expected

tests/sentry/middleware/test_auth.py

@@ -49,7 +49,9 @@ class AuthenticationMiddlewareTestCase(TestCase):
             self.middleware.process_request(request)
             # Force the user object to materialize
             request.user.id  # noqa
+
         with assume_test_silo_mode(SiloMode.CONTROL):
+            self.user.refresh_from_db()
             assert UserIP.objects.filter(user=self.user, ip_address="127.0.0.1").exists()
 
         assert request.user.is_authenticated

tests/sentry/middleware/test_useractive.py

@@ -1,34 +0,0 @@
-from datetime import timedelta
-from functools import cached_property
-
-from django.test import RequestFactory
-from django.utils import timezone
-
-from sentry.middleware.user import UserActiveMiddleware
-from sentry.testutils import TestCase
-from sentry.testutils.silo import control_silo_test
-
-
-@control_silo_test(stable=True)
-class UserActiveMiddlewareTest(TestCase):
-    middleware = cached_property(UserActiveMiddleware)
-
-    @cached_property
-    def factory(self):
-        return RequestFactory()
-
-    def test_simple(self):
-        self.view = lambda x: None
-
-        user = self.user
-        req = self.factory.get("/")
-        req.user = user
-
-        resp = self.middleware.process_view(req, self.view, [], {})
-        assert resp is None
-        assert timezone.now() - user.last_active < timedelta(minutes=1)
-
-        user.last_active = None
-        resp = self.middleware.process_view(req, self.view, [], {})
-        assert resp is None
-        assert timezone.now() - user.last_active < timedelta(minutes=1)