From 2ba4a380f28a18414affaf077e8472ec7e83b5e1 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Wed, 3 Jul 2024 17:16:48 +0530 Subject: [PATCH 1/3] delete unwanted custom metadata objects --- contentcuration/contentcuration/models.py | 6 ++---- contentcuration/contentcuration/viewsets/base.py | 9 ++++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 9e0911b619..0fb1b09a7c 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -2565,14 +2565,13 @@ def serialize_to_change_dict(self): class CustomTaskMetadata(models.Model): # Task_id for reference task_id = models.CharField( - max_length=255, # Adjust the max_length as needed + max_length=255, unique=True, ) - # user shouldn't be null, but in order to append the field, this needs to be allowed user = models.ForeignKey(settings.AUTH_USER_MODEL, related_name="tasks", on_delete=models.CASCADE, null=True) channel_id = DjangoUUIDField(db_index=True, null=True, blank=True) progress = models.IntegerField(null=True, blank=True, validators=[MinValueValidator(0), MaxValueValidator(100)]) - # a hash of the task name and kwargs for identifying repeat tasks + # A hash of the task name and kwargs for identifying repeat tasks signature = models.CharField(null=True, blank=False, max_length=32) date_created = models.DateTimeField( auto_now_add=True, @@ -2582,7 +2581,6 @@ class CustomTaskMetadata(models.Model): class Meta: indexes = [ - # add index that matches query usage for signature models.Index( fields=['signature'], name='task_result_signature', diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index bd958b659d..5b964ac7de 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -936,9 +936,12 @@ def create_change_tracker(pk, table, channel_id, user, task_name): # Clean up any previous tasks specific to this in case there were failures. signature = generate_task_signature(task_name, task_kwargs=task_kwargs, channel_id=channel_id) - task_id_to_delete = CustomTaskMetadata.objects.filter(channel_id=channel_id, signature=signature) - if task_id_to_delete: - TaskResult.objects.filter(task_id=task_id_to_delete, task_name=task_name).delete() + custom_task_metadata_qs = CustomTaskMetadata.objects.filter(channel_id=channel_id, signature=signature) + if custom_task_metadata_qs.exists(): + task_result_qs = TaskResult.objects.filter(task_id=custom_task_metadata_qs[0].task_id, task_name=task_name) + if task_result_qs.exists(): + task_result_qs[0].delete() + custom_task_metadata_qs[0].delete() task_id = uuid.uuid4().hex From 8b3cbc4d3e71a2e7d9a02c570745e2e644091aa7 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Fri, 5 Jul 2024 17:22:56 +0530 Subject: [PATCH 2/3] add tests --- .../tests/test_exportchannel.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 36e331c713..cb2a3292cf 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -1,13 +1,17 @@ from __future__ import absolute_import +import json import os import random import string import tempfile +import uuid import pytest +from celery import states from django.core.management import call_command from django.db import connections +from django_celery_results.models import TaskResult from kolibri_content import models as kolibri_models from kolibri_content.router import cleanup_content_database_connection from kolibri_content.router import get_active_content_database @@ -29,6 +33,8 @@ from .testdata import slideshow from .testdata import thumbnail_bytes from contentcuration import models as cc +from contentcuration.models import CustomTaskMetadata +from contentcuration.utils.celery.tasks import generate_task_signature from contentcuration.utils.publish import ChannelIncompleteError from contentcuration.utils.publish import convert_channel_thumbnail from contentcuration.utils.publish import create_content_database @@ -37,6 +43,7 @@ from contentcuration.utils.publish import map_prerequisites from contentcuration.utils.publish import MIN_SCHEMA_VERSION from contentcuration.utils.publish import set_channel_icon_encoding +from contentcuration.viewsets.base import create_change_tracker pytestmark = pytest.mark.django_db @@ -553,3 +560,43 @@ def test_fill_published_fields(self): self.assertTrue(channel.published_data) self.assertIsNotNone(channel.published_data.get(0)) self.assertEqual(channel.published_data[0]['version_notes'], version_notes) + + +class PublishFailCleansUpTaskObjects(StudioTestCase): + def setUp(self): + super(PublishFailCleansUpTaskObjects, self).setUpBase() + + def test_failed_task_objects_cleaned_up_when_publishing(self): + channel_id = self.channel.id + task_name = 'export-channel' + task_id = uuid.uuid4().hex + pk = 'ab684452f2ad4ba6a1426d6410139f60' + table = 'channel' + task_kwargs = json.dumps({'pk': pk, 'table': table}) + signature = generate_task_signature(task_name, task_kwargs=task_kwargs, channel_id=channel_id) + + TaskResult.objects.create( + task_id=task_id, + status=states.FAILURE, + task_name=task_name, + ) + + CustomTaskMetadata.objects.create( + task_id=task_id, + channel_id=channel_id, + user=self.user, + signature=signature + ) + + assert TaskResult.objects.filter(task_id=task_id).exists() + assert CustomTaskMetadata.objects.filter(task_id=task_id).exists() + + with create_change_tracker(pk, table, channel_id, self.user, task_name): + pass + + assert not TaskResult.objects.filter(task_id=task_id).exists() + assert not CustomTaskMetadata.objects.filter(task_id=task_id).exists() + + new_task_result = TaskResult.objects.get(task_name=task_name, status=states.STARTED) + new_custom_task_metadata = CustomTaskMetadata.objects.get(channel_id=channel_id, user=self.user, signature=signature) + assert new_custom_task_metadata.task_id == new_task_result.task_id From 909e3f28e3e2cdd21f7595fbe850a58938c7fd99 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Sun, 7 Jul 2024 23:04:40 +0530 Subject: [PATCH 3/3] fix written test --- .../contentcuration/tests/test_exportchannel.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index cb2a3292cf..90a8064f41 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -592,11 +592,8 @@ def test_failed_task_objects_cleaned_up_when_publishing(self): assert CustomTaskMetadata.objects.filter(task_id=task_id).exists() with create_change_tracker(pk, table, channel_id, self.user, task_name): - pass - - assert not TaskResult.objects.filter(task_id=task_id).exists() - assert not CustomTaskMetadata.objects.filter(task_id=task_id).exists() - - new_task_result = TaskResult.objects.get(task_name=task_name, status=states.STARTED) - new_custom_task_metadata = CustomTaskMetadata.objects.get(channel_id=channel_id, user=self.user, signature=signature) - assert new_custom_task_metadata.task_id == new_task_result.task_id + assert not TaskResult.objects.filter(task_id=task_id).exists() + assert not CustomTaskMetadata.objects.filter(task_id=task_id).exists() + new_task_result = TaskResult.objects.filter(task_name=task_name, status=states.STARTED).first() + new_custom_task_metadata = CustomTaskMetadata.objects.get(channel_id=channel_id, user=self.user, signature=signature) + assert new_custom_task_metadata.task_id == new_task_result.task_id