From df1bfd4177df24e08c2cc38e397b1bab30b23522 Mon Sep 17 00:00:00 2001 From: Colin Powell Date: Tue, 12 Mar 2024 16:57:56 -0400 Subject: [PATCH] [locations] Refactor has moved to not use multiple points --- vrobbler/apps/locations/models.py | 65 ++++++++------------ vrobbler/apps/locations/tests/test_models.py | 6 +- vrobbler/apps/scrobbles/models.py | 16 +---- 3 files changed, 28 insertions(+), 59 deletions(-) diff --git a/vrobbler/apps/locations/models.py b/vrobbler/apps/locations/models.py index 4a9ddea..1aa5e15 100644 --- a/vrobbler/apps/locations/models.py +++ b/vrobbler/apps/locations/models.py @@ -1,4 +1,4 @@ -from decimal import Decimal +from decimal import Decimal, getcontext import logging from typing import Dict from uuid import uuid4 @@ -91,47 +91,37 @@ class GeoLocation(ScrobblableMixin): abs(Decimal(old_lat_lon[1]) - Decimal(self.lon)), ) - def has_moved(self, past_points: list["GeoLocation"]) -> bool: - """GPS jumps from time to time. This function tries to smooth out - when we appear to have flagging our location as having not moved if one of our last - 3""" + def has_moved_for_user(self, user_id: int) -> bool: has_moved = False - has_moved_locs = [] - for point in past_points: - loc_diff = self.loc_diff((point.lat, point.lon)) - loc_has_moved = False - if ( - loc_diff[0] > GEOLOC_PROXIMITY - or loc_diff[1] > GEOLOC_PROXIMITY - ): - loc_has_moved = True - logger.info( - f"[locations] checked whether location has moved against proximity setting", - extra={ - "location": self, - "loc_diff": loc_diff, - "loc_has_moved": loc_has_moved, - "point": point, - "geoloc_proximity": GEOLOC_PROXIMITY, - }, - ) - has_moved_locs.append(loc_has_moved) + user = User.objects.filter(id=user_id).first() + if not user: + return False - # Sum up all True values, if they're more than half of our locations, we've moved - if sum(has_moved_locs) > int(len(past_points) / 2): + last_point = ( + user.scrobble_set.filter(media_type="GeoLocation") + .order_by("-timestamp") + .first() + ).media_obj + + if not last_point: + return True + + loc_diff = self.loc_diff((last_point.lat, last_point.lon)) + if loc_diff[0] > GEOLOC_PROXIMITY or loc_diff[1] > GEOLOC_PROXIMITY: has_moved = True - logger.info( - f"[locations] finished checking for movement", + logger.debug( + f"[locations] checked whether location has moved against proximity setting", extra={ + "location": self, + "loc_diff": loc_diff, "has_moved": has_moved, - "has_moved_locs": has_moved_locs, - "past_points": past_points, + "point": last_point, + "geoloc_proximity": GEOLOC_PROXIMITY, }, ) - return has_moved - def in_proximity(self, named=True) -> models.QuerySet: + def in_proximity(self, named=False) -> models.QuerySet: lat_min = Decimal(self.lat) - GEOLOC_PROXIMITY lat_max = Decimal(self.lat) + GEOLOC_PROXIMITY lon_min = Decimal(self.lon) - GEOLOC_PROXIMITY @@ -143,12 +133,5 @@ class GeoLocation(ScrobblableMixin): lat__gte=lat_min, lon__lte=lon_max, lon__gte=lon_min, - ) - logger.info( - f"[locations] finished looking for proximate locations", - extra={ - "close_locations": close_locations, - "is_title_null": is_title_null, - }, - ) + ).exclude(id=self.id) return close_locations diff --git a/vrobbler/apps/locations/tests/test_models.py b/vrobbler/apps/locations/tests/test_models.py index 983afe6..a89f63e 100644 --- a/vrobbler/apps/locations/tests/test_models.py +++ b/vrobbler/apps/locations/tests/test_models.py @@ -50,8 +50,8 @@ def test_found_in_proximity_location(): close = GeoLocation.objects.create( lat=lat + 0.0001, lon=lon - 0.0001, altitude=60 ) - assert close not in loc.in_proximity() - assert close in loc.in_proximity(named=False) + assert close not in loc.in_proximity(named=True) + assert close in loc.in_proximity() @pytest.mark.django_db @@ -63,7 +63,7 @@ def test_not_found_in_proximity_location(): far = GeoLocation.objects.create( lat=lat + 0.0002, lon=lon - 0.0001, altitude=60 ) - assert far not in loc.in_proximity(named=False) + assert far not in loc.in_proximity() @pytest.mark.django_db diff --git a/vrobbler/apps/scrobbles/models.py b/vrobbler/apps/scrobbles/models.py index 8be5d6c..d1a3a10 100644 --- a/vrobbler/apps/scrobbles/models.py +++ b/vrobbler/apps/scrobbles/models.py @@ -932,10 +932,7 @@ class Scrobble(TimeStampedModel): ) return scrobble - jitter_correction_locations = cls.past_scrobbled_locations( - user_id, POINTS_FOR_MOVEMENT_HISTORY - ) - has_moved = location.has_moved(jitter_correction_locations) + has_moved = location.has_moved(scrobble.user.id) logger.info( f"[scrobbling] checking - has last location has moved?", extra={ @@ -944,7 +941,6 @@ class Scrobble(TimeStampedModel): "media_type": cls.MediaType.GEO_LOCATION, "media_id": location.id, "has_moved": has_moved, - "past_locations_used": jitter_correction_locations, }, ) if not has_moved: @@ -986,16 +982,6 @@ class Scrobble(TimeStampedModel): ) return scrobble - @classmethod - def past_scrobbled_locations( - cls, user_id: int, num: int - ) -> list["Location"]: - past_scrobbles = cls.objects.filter( - media_type="GeoLocation", - user_id=user_id, - ).order_by("-timestamp")[1:num] - return [s.geo_location for s in past_scrobbles] - def update(self, scrobble_data: dict) -> "Scrobble": # Status is a field we get from Mopidy, which refuses to poll us scrobble_status = scrobble_data.pop("mopidy_status", None)