Skip to content

Commit

Permalink
feat: batch user is_obsolete updating in auth change command
Browse files Browse the repository at this point in the history
refs KK-1184
  • Loading branch information
karisal-anders committed Jun 14, 2024
1 parent 3595b19 commit 7e89c1b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def add_arguments(self, parser):
"-b",
"--batch_size",
type=int,
help="Batch size for bulk updates and queryset iteration. "
help="Batch size for queryset iteration and data updating. "
"By default %(default)s.",
default=1000,
)
Expand Down
16 changes: 12 additions & 4 deletions users/services.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Optional, Union

from django.contrib.auth import get_user_model
from django.core.paginator import Paginator
from django.db.models import QuerySet
from django.template import Context, Template
from django_ilmoitin.utils import send_notification
Expand Down Expand Up @@ -136,10 +137,10 @@ def send_user_auth_service_is_changing_notifications(
"""
if guardians is None:
guardians = Guardian.objects.prefetch_related(
"children"
"children",
).for_auth_service_is_changing_notification()

handled_user_ids = set()
handled_user_ids = []
_notify_function = (
AuthServiceNotificationService._send_auth_service_is_changing_notification
)
Expand All @@ -148,11 +149,18 @@ def send_user_auth_service_is_changing_notifications(
# Use iterator to reduce memory usage
for guardian in guardians.iterator(chunk_size=batch_size):
_notify_function(guardian, date_of_change_str)
handled_user_ids.add(guardian.user_id)
handled_user_ids.append(guardian.user_id)
finally:
# This will be executed even if an exception is raised
if obsolete_handled_users:
User.objects.filter(id__in=handled_user_ids).update(is_obsolete=True)
# Mark the handled users as obsolete in batches
# to limit SQL update query size
paginator = Paginator(handled_user_ids, batch_size)
for page_num in paginator.page_range:
page_of_user_ids = paginator.page(page_num).object_list
User.objects.filter(id__in=page_of_user_ids).update(
is_obsolete=True
)

@staticmethod
def generate_children_event_history_markdown(guardian: Guardian):
Expand Down
34 changes: 34 additions & 0 deletions users/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,40 @@ def test_command_obsolete_handled_users_true(obsolete_handled_users_argument):
assert Guardian.objects.filter(user__is_obsolete=True).count() == 3


@pytest.mark.parametrize("batch_size_value", [1, 3, 10, 11, 25, 50, 1000, 1000000])
@pytest.mark.django_db
def test_command_obsolete_handled_users_with_batch_size(
obsolete_handled_users_argument, batch_size_argument, batch_size_value
):
"""
Test that "-o" or "--obsolete_handled_users" argument
marks the handled users as obsolete with different batch sizes.
"""
GuardianFactory.create_batch(
size=50,
user__date_joined=timezone.now() - timedelta(days=1),
user__is_obsolete=False,
)
assert Guardian.objects.count() == 50
assert Guardian.objects.filter(user__is_obsolete=False).count() == 50

with mock.patch.object(
AuthServiceNotificationService,
"_send_auth_service_is_changing_notification",
):
call_command(
"send_user_auth_service_is_changing_notifications",
batch_size_argument,
str(batch_size_value),
obsolete_handled_users_argument,
# Needed for including the non-obsoleted users as input:
"--include_non_obsoleted",
)

# Should mark all users as obsolete
assert Guardian.objects.filter(user__is_obsolete=True).count() == 50


@pytest.mark.parametrize("batch_size_value", [0, -1, -9999])
@pytest.mark.django_db
def test_command_invalid_batch_size(batch_size_argument, batch_size_value):
Expand Down

0 comments on commit 7e89c1b

Please sign in to comment.