Merge pull request City-of-Helsinki#200 from City-of-Helsinki/only-up…

…date-changed-keywords-and-places

Expand management command to only update changed event numbers

events/api.py

@@ -565,7 +565,7 @@ class KeywordSerializer(LinkedEventsSerializer):
 
     class Meta:
         model = Keyword
-        fields = '__all__'
+        exclude = ('n_events_changed',)
 
 
 class KeywordRetrieveViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
@@ -718,7 +718,7 @@ class PlaceSerializer(LinkedEventsSerializer, GeoModelSerializer):
 
     class Meta:
         model = Place
-        fields = '__all__'
+        exclude = ('n_events_changed',)
 
 
 class PlaceFilter(filters.FilterSet):

events/management/commands/update_n_events.py

@@ -0,0 +1,45 @@
+from django.core.management import BaseCommand, CommandError
+from django.db import transaction
+
+from events.models import Keyword, Place
+from events.utils import recache_n_events_in_locations, recache_n_events
+
+
+class Command(BaseCommand):
+    help = "Update keyword and place event numbers"
+
+    def add_arguments(self, parser):
+        parser.add_argument('model', nargs='?', default=False)
+        parser.add_argument('--all',
+                            default=False,
+                            action='store_true',
+                            dest='update_all',
+                            help='Recalculate everything from scratch')
+
+    def handle_keywords(self, update_all=False):
+        if update_all:
+            keywords = Keyword.objects.all()
+        else:
+            keywords = Keyword.objects.filter(n_events_changed=True)
+        recache_n_events((k.id for k in keywords), all=update_all)
+        print("Updated %s keyword event numbers." % ('all' if update_all else 'changed'))
+        print("A total of %s keywords updated." % (str(keywords.count())))
+
+    def handle_places(self, update_all=False):
+        if update_all:
+            places = Place.objects.all()
+        else:
+            places = Place.objects.filter(n_events_changed=True)
+        recache_n_events_in_locations((k.id for k in places), all=update_all)
+        print("Updated %s place event numbers." % ('all' if update_all else 'changed'))
+        print("A total of %s places updated." % (str(places.count())))
+
+    def handle(self, model=None, update_all=False, **kwargs):
+        if model and model not in ('keyword', 'place'):
+            raise CommandError("Model %s not found. Valid models are 'keyword' and 'place'." % (model, ))
+        if not model or model == 'keyword':
+            self.handle_keywords(update_all=update_all)
+        if not model or model == 'place':
+            self.handle_places(update_all=update_all)
+
+

events/migrations/0037_add_n_events_changed_to_keyword_and_place.py

@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.11 on 2017-03-15 15:54
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.manager
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('events', '0036_add_n_events_to_place'),
+    ]
+
+    operations = [
+        migrations.AlterModelManagers(
+            name='place',
+            managers=[
+                ('geo_objects', django.db.models.manager.Manager()),
+            ],
+        ),
+        migrations.AddField(
+            model_name='keyword',
+            name='n_events_changed',
+            field=models.BooleanField(db_index=True, default=False),
+        ),
+        migrations.AddField(
+            model_name='place',
+            name='n_events_changed',
+            field=models.BooleanField(db_index=True, default=False),
+        ),
+    ]

events/models.py

@@ -31,13 +31,12 @@
 from events import translation_utils
 from django.utils.encoding import python_2_unicode_compatible
 from django.contrib.postgres.fields import HStoreField
-from django.db import transaction, connection
+from django.db import transaction
 from django.db.models.signals import m2m_changed
 from django.dispatch import receiver
 from image_cropping import ImageRatioField
 from munigeo.models import AdministrativeDivision
 
-from events.sql import count_events_for_keywords
 
 User = settings.AUTH_USER_MODEL
 
@@ -230,6 +229,7 @@ class Keyword(BaseModel):
         editable=False,
         db_index=True
     )
+    n_events_changed = models.BooleanField(default=False, db_index=True)
 
     schema_org_type = "Thing/LinkedEventKeyword"
 
@@ -246,23 +246,6 @@ class Meta:
         verbose_name_plural = _('keywords')
 
 
-def recache_n_events(keywords):
-    """
-    Recache the number of events for the given keywords.
-
-    :type keywords: Iterable[Keyword]
-    """
-
-    # The helper function has to exist outside the model, because it is used in migration.
-    # Django apps cannot serialize unbound instance functions when saving model history during migration.
-    count_map = count_events_for_keywords((k.id for k in keywords))
-    for keyword in keywords:
-        n_events = count_map.get(keyword.id, 0)
-        if n_events != keyword.n_events:
-            keyword.n_events = n_events
-            keyword.save(update_fields=("n_events",))
-
-
 class KeywordSet(BaseModel):
     """
     Sets of pre-chosen keywords intended or specific uses and/or organizations,
@@ -316,6 +299,7 @@ class Place(MPTTModel, BaseModel, SchemalessFieldMixin):
         editable=False,
         db_index=True
     )
+    n_events_changed = models.BooleanField(default=False, db_index=True)
 
 
     class Meta:
@@ -343,19 +327,6 @@ def save(self, *args, **kwargs):
 reversion.register(Place)
 
 
-def recache_n_events_in_location(place):
-    """
-    The helper function has to exist outside the model, because it is used in migration.
-    Django apps cannot serialize unbound instance functions when saving model history during migration.
-    :type place: place
-    """
-    n_events = place.events.count()
-    if n_events != place.n_events:
-        place.n_events = n_events
-        # Use the queryset update API to skip the position/divisions save
-        Place.objects.filter(id=place.id).update(n_events=n_events)
-
-
 class OpeningHoursSpecification(models.Model):
     GR_BASE_URL = "http://purl.org/goodrelations/v1#"
     WEEK_DAYS = (
@@ -488,10 +459,10 @@ def save(self, *args, **kwargs):
         super(Event, self).save(*args, **kwargs)
 
         # needed to cache location event numbers
-        if self.location:
-            recache_n_events_in_location(self.location)
+        if not old_location and self.location:
+            Place.objects.filter(id=self.location.id).update(n_events_changed=True)
         if old_location and old_location != self.location:
-            recache_n_events_in_location(old_location)
+            Place.objects.filter(id__in=(old_location.id, self.location.id)).update(n_events_changed=True)
 
     def __str__(self):
         name = ''
@@ -529,9 +500,10 @@ def keyword_added_or_removed(sender, model=None,
     """
     if action in ('post_add', 'post_remove'):
         if model is Keyword:
-            recache_n_events(keywords=Keyword.objects.filter(pk__in=pk_set))
+            Keyword.objects.filter(pk__in=pk_set).update(n_events_changed=True)
         if model is Event:
-            recache_n_events(keywords=[instance])
+            instance.n_events_changed = True
+            instance.save(update_fields=("n_events_changed",))
 
 
 class Offer(models.Model, SimpleValueMixin):

events/sql.py

@@ -1,12 +1,14 @@
 from django.db import connection
 
 
-def count_events_for_keywords(keyword_ids=()):
+def count_events_for_keywords(keyword_ids=(), all=False):
     """
     Get the actual count of events using the given keywords.
 
-    :param keyword_ids: set of keyword ids; pass an empty set to get the data for all keywords
+    :param keyword_ids: set of keyword ids
     :type keyword_ids: Iterable[str]
+    :param all: count all keywords instead
+    :type all: bool
     :return: dict of keyword id to count
     :rtype: dict[str, int]
     """
@@ -24,7 +26,7 @@ def count_events_for_keywords(keyword_ids=()):
             ) t
             GROUP BY t.keyword_id;
             ''', [keyword_ids, keyword_ids])
-        else:
+        elif all:
             cursor.execute('''
             SELECT t.keyword_id, COUNT(DISTINCT t.event_id)
             FROM (
@@ -34,4 +36,39 @@ def count_events_for_keywords(keyword_ids=()):
             ) t
             GROUP BY t.keyword_id;
             ''')
+        else:
+            return {}
+        return dict(cursor.fetchall())
+
+
+def count_events_for_places(place_ids=(), all=False):
+    """
+    Get the actual count of events in the given places.
+
+    :param place_ids: set of place ids
+    :type place_ids: Iterable[str]
+    :param all: count all places instead
+    :type all: bool
+    :return: dict of place id to count
+    :rtype: dict[str, int]
+    """
+    # sorry for the non-DRY-ness; would be easier with an SQL generator like SQLAlchemy, but...
+
+    place_ids = tuple(set(place_ids))
+    with connection.cursor() as cursor:
+        if place_ids:
+            cursor.execute('''
+            SELECT e.location_id, COUNT(*)
+            FROM events_event e
+            WHERE location_id IN %s
+            GROUP BY e.location_id;
+            ''', [place_ids])
+        elif all:
+            cursor.execute('''
+            SELECT e.location_id, COUNT(*)
+            FROM events_event e
+            GROUP BY e.location_id;
+            ''')
+        else:
+            return {}
         return dict(cursor.fetchall())

events/tests/test_event_post.py

@@ -6,6 +6,8 @@
 from django.utils import timezone, translation
 from django.utils.encoding import force_text
 from .utils import versioned_reverse as reverse
+from django.core.management import call_command
+
 
 from events.tests.utils import assert_event_data_is_equal
 from .conftest import keyword_id
@@ -182,6 +184,7 @@ def test__keyword_n_events_updated(list_url,
                                                                user):
     api_client.force_authenticate(user=user)
     response = api_client.post(list_url, minimal_event_dict, format='json')
+    call_command('update_n_events')
     assert Keyword.objects.get(id='test').n_events == 1
 
 
@@ -192,6 +195,7 @@ def test__location_n_events_updated(list_url,
                                                                user):
     api_client.force_authenticate(user=user)
     response = api_client.post(list_url, minimal_event_dict, format='json')
+    call_command('update_n_events')
     assert Place.objects.get(id='test location').n_events == 1
 
 

events/tests/test_event_put.py

@@ -2,6 +2,7 @@
 from datetime import timedelta
 from copy import deepcopy
 from django.utils import timezone
+from django.core.management import call_command
 
 import pytest
 from .utils import versioned_reverse as reverse
@@ -55,6 +56,7 @@ def test__keyword_n_events_updated(api_client, minimal_event_dict, user, data_so
     api_client.force_authenticate(user=user)
     response = create_with_post(api_client, minimal_event_dict)
     assert_event_data_is_equal(minimal_event_dict, response.data)
+    call_command('update_n_events')
     assert Keyword.objects.get(id='test').n_events == 1
     data2 = response.data
     print('got the post response')
@@ -67,6 +69,7 @@ def test__keyword_n_events_updated(api_client, minimal_event_dict, user, data_so
     response2 = update_with_put(api_client, event_id, data2)
     print('got the put response')
     print(response2.data)
+    call_command('update_n_events')
     assert Keyword.objects.get(id='test').n_events == 0
     assert Keyword.objects.get(id='test2').n_events == 1
     assert Keyword.objects.get(id='test3').n_events == 1
@@ -79,6 +82,7 @@ def test__location_n_events_updated(api_client, minimal_event_dict, user, place2
     api_client.force_authenticate(user=user)
     response = create_with_post(api_client, minimal_event_dict)
     assert_event_data_is_equal(minimal_event_dict, response.data)
+    call_command('update_n_events')
     assert Place.objects.get(id='test location').n_events == 1
     data2 = response.data
     print('got the post response')
@@ -90,6 +94,7 @@ def test__location_n_events_updated(api_client, minimal_event_dict, user, place2
     response2 = update_with_put(api_client, event_id, data2)
     print('got the put response')
     print(response2.data)
+    call_command('update_n_events')
     assert Place.objects.get(id='test location').n_events == 0
     assert Place.objects.get(id='test location 2').n_events == 1
 

events/utils.py

@@ -1,6 +1,12 @@
 import re
 import collections
 
+from django.db import transaction
+
+from events.models import Keyword, Place
+from events.sql import count_events_for_keywords, count_events_for_places
+
+
 def convert_to_camelcase(s):
     return ''.join(word.title() if i else word for i, word in enumerate(
         s.split('_')))
@@ -38,3 +44,42 @@ def update(d, u):
         else:
             d[k] = u[k]
     return d
+
+
+def recache_n_events(keyword_ids, all=False):
+    """
+    Recache the number of events for the given keywords (by ID).
+
+    :param all: recache all keywords instead
+    :type keyword_ids: Iterable[str]
+    """
+
+    # needed so we don't empty the blasted iterator mid-operation
+    keyword_ids = tuple(set(keyword_ids))
+    with transaction.atomic():
+        if all:
+            Keyword.objects.update(n_events=0)
+        else:
+            Keyword.objects.filter(id__in=keyword_ids).update(n_events=0)
+        for keyword_id, n_events in count_events_for_keywords(keyword_ids, all=all).items():
+            Keyword.objects.filter(id=keyword_id).update(n_events=n_events, n_events_changed=False)
+
+
+def recache_n_events_in_locations(place_ids, all=False):
+    """
+    Recache the number of events for the given locations (by ID).
+
+    :param all: recache all places instead
+    :type place_ids: Iterable[str]
+    """
+
+    # needed so we don't empty the blasted iterator mid-operation
+    place_ids = tuple(set(place_ids))
+    with transaction.atomic():
+        if all:
+            Place.objects.update(n_events=0)
+        else:
+            Place.objects.filter(id__in=place_ids).update(n_events=0)
+        for place_id, n_events in count_events_for_places(place_ids, all=all).items():
+            Place.objects.filter(id=place_id).update(n_events=n_events, n_events_changed=False)
+