From 55e5c4088b4664e2812aab3ab9e168efced1e36a Mon Sep 17 00:00:00 2001 From: taoerman Date: Mon, 31 Mar 2025 00:09:12 -0700 Subject: [PATCH 1/3] Add use_staging_tree to publish_channel --- .../tests/test_exportchannel.py | 129 +++++++++++++++++- .../contentcuration/tests/test_sync.py | 4 +- .../contentcuration/utils/publish.py | 98 ++++++++----- 3 files changed, 194 insertions(+), 37 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index a235fafe86..753b6f285f 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -7,9 +7,12 @@ import pytest from celery import states +from django.conf import settings from django.core.management import call_command from django.db import connections from django_celery_results.models import TaskResult +from django.core.files.storage import FileSystemStorage +from django.test.utils import override_settings 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 @@ -25,7 +28,7 @@ from .base import StudioTestCase from .helpers import clear_tasks -from .testdata import channel +from .testdata import channel, tree from .testdata import create_studio_file from .testdata import node as create_node from .testdata import slideshow @@ -40,6 +43,8 @@ from contentcuration.utils.publish import fill_published_fields from contentcuration.utils.publish import map_prerequisites from contentcuration.utils.publish import MIN_SCHEMA_VERSION +from contentcuration.utils.publish import NoneContentNodeTreeError +from contentcuration.utils.publish import publish_channel from contentcuration.utils.publish import set_channel_icon_encoding from contentcuration.viewsets.base import create_change_tracker @@ -600,3 +605,125 @@ def test_failed_task_objects_cleaned_up_when_publishing(self): 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 + +class PublishStagingTreeTestCase(StudioTestCase): + @classmethod + def setUpClass(cls): + super(PublishStagingTreeTestCase, cls).setUpClass() + cls.patch_copy_db = patch('contentcuration.utils.publish.save_export_database') + cls.mock_save_export = cls.patch_copy_db.start() + + @classmethod + def tearDownClass(cls): + super(PublishStagingTreeTestCase, cls).tearDownClass() + cls.patch_copy_db.stop() + + def setUp(self): + super(PublishStagingTreeTestCase, self).setUp() + + self.channel_version = 3 + self.incomplete_video_in_staging = 'Incomplete video in staging tree' + self.complete_video_in_staging = 'Complete video in staging tree' + self.incomplete_video_in_main = 'Incomplete video in main tree' + self.complete_video_in_main = 'Complete video in main tree' + + self.content_channel = channel() + self.content_channel.staging_tree = tree() + self.content_channel.version = self.channel_version + self.content_channel.save() + + # Incomplete node should be excluded. + new_node = create_node({'kind_id': 'video', 'title': self.incomplete_video_in_staging, 'children': []}) + new_node.complete = False + new_node.parent = self.content_channel.staging_tree + new_node.published = False + new_node.save() + + # Complete node should be included. + new_video = create_node({'kind_id': 'video', 'title': self.complete_video_in_staging, 'children': []}) + new_video.complete = True + new_video.parent = self.content_channel.staging_tree + new_node.published = False + new_video.save() + + # Incomplete node in main_tree. + new_node = create_node({'kind_id': 'video', 'title': self.incomplete_video_in_main, 'children': []}) + new_node.complete = False + new_node.parent = self.content_channel.main_tree + new_node.published = False + new_node.save() + + # Complete node in main_tree. + new_node = create_node({'kind_id': 'video', 'title': self.complete_video_in_main, 'children': []}) + new_node.complete = True + new_node.parent = self.content_channel.main_tree + new_node.published = False + new_node.save() + + def run_publish_channel(self): + publish_channel( + self.admin_user.id, + self.content_channel.id, + version_notes="", + force=False, + force_exercises=False, + send_email=False, + progress_tracker=None, + language="fr", + use_staging_tree=True + ) + + def test_none_staging_tree(self): + self.content_channel.staging_tree = None + self.content_channel.save() + with self.assertRaises(NoneContentNodeTreeError): + self.run_publish_channel() + + def test_staging_tree_published(self): + self.assertFalse(self.content_channel.staging_tree.published) + self.run_publish_channel() + self.content_channel.refresh_from_db() + self.assertTrue(self.content_channel.staging_tree.published) + + def test_next_version_exported(self): + self.run_publish_channel() + self.mock_save_export.assert_called_with( + self.content_channel.id, + "next", + True, + ) + + def test_main_tree_not_impacted(self): + self.assertFalse(self.content_channel.main_tree.published) + self.run_publish_channel() + self.content_channel.refresh_from_db() + self.assertFalse(self.content_channel.main_tree.published) + + def test_channel_version_not_incremented(self): + self.assertEqual(self.content_channel.version, self.channel_version) + self.run_publish_channel() + self.content_channel.refresh_from_db() + self.assertEqual(self.content_channel.version, self.channel_version) + + def test_staging_tree_used_for_publish(self): + set_channel_icon_encoding(self.content_channel) + self.tempdb = create_content_database( + self.content_channel, + True, + self.admin_user.id, + True, + progress_tracker=None, + use_staging_tree=True, + ) + set_active_content_database(self.tempdb) + + nodes = kolibri_models.ContentNode.objects.all() + self.assertEqual(nodes.filter(title=self.incomplete_video_in_staging).count(), 0) + self.assertEqual(nodes.filter(title=self.complete_video_in_staging).count(), 1) + self.assertEqual(nodes.filter(title=self.incomplete_video_in_main).count(), 0) + self.assertEqual(nodes.filter(title=self.complete_video_in_main).count(), 0) + + cleanup_content_database_connection(self.tempdb) + set_active_content_database(None) + if os.path.exists(self.tempdb): + os.remove(self.tempdb) diff --git a/contentcuration/contentcuration/tests/test_sync.py b/contentcuration/contentcuration/tests/test_sync.py index a8e3ef6325..57a1a746ac 100644 --- a/contentcuration/contentcuration/tests/test_sync.py +++ b/contentcuration/contentcuration/tests/test_sync.py @@ -47,8 +47,8 @@ def setUp(self): # Put all nodes into a clean state so we can track when syncing # causes changes in the tree. - mark_all_nodes_as_published(self.channel) - mark_all_nodes_as_published(self.derivative_channel) + mark_all_nodes_as_published(self.channel.main_tree) + mark_all_nodes_as_published(self.derivative_channel.main_tree) def _add_temp_file_to_content_node(self, node): new_file = create_temp_file("mybytes") diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 210eaf5efb..b4ec4acc77 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -72,6 +72,10 @@ class ChannelIncompleteError(Exception): pass +class NoneContentNodeTreeError(Exception): + pass + + class SlowPublishError(Exception): """ Used to track slow Publishing operations. We don't raise this error, @@ -111,17 +115,17 @@ def send_emails(channel, user_id, version_notes=''): user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL, html_message=message) -def create_content_database(channel, force, user_id, force_exercises, progress_tracker=None): +def create_content_database(channel, force, user_id, force_exercises, progress_tracker=None, use_staging_tree=False): """ :type progress_tracker: contentcuration.utils.celery.ProgressTracker|None """ # increment the channel version - if not force: + if not use_staging_tree and not force: raise_if_nodes_are_all_unchanged(channel) fh, tempdb = tempfile.mkstemp(suffix=".sqlite3") with using_content_database(tempdb): - if not channel.main_tree.publishing: + if not use_staging_tree and not channel.main_tree.publishing: channel.mark_publishing(user_id) call_command("migrate", @@ -130,8 +134,9 @@ def create_content_database(channel, force, user_id, force_exercises, progress_t no_input=True) if progress_tracker: progress_tracker.track(10) + base_tree = channel.staging_tree if use_staging_tree else channel.main_tree tree_mapper = TreeMapper( - channel.main_tree, + base_tree, channel.language, channel.id, channel.name, @@ -141,14 +146,16 @@ def create_content_database(channel, force, user_id, force_exercises, progress_t inherit_metadata=bool(channel.ricecooker_version), ) tree_mapper.map_nodes() - kolibri_channel = map_channel_to_kolibri_channel(channel) + kolibri_channel = map_channel_to_kolibri_channel(channel, use_staging_tree) # It should be at this percent already, but just in case. if progress_tracker: progress_tracker.track(90) - map_prerequisites(channel.main_tree) + map_prerequisites(base_tree) + # Need to save as version being published, not current version + version = "next" if use_staging_tree else channel.version + 1 save_export_database( - channel.pk, channel.version + 1 - ) # Need to save as version being published, not current version + channel.pk, version, use_staging_tree, + ) if channel.public: mapper = ChannelMapper(kolibri_channel) mapper.run() @@ -732,8 +739,9 @@ def map_prerequisites(root_node): logging.error('Unable to find source node for prerequisite relationship {}'.format(str(e))) -def map_channel_to_kolibri_channel(channel): +def map_channel_to_kolibri_channel(channel, use_staging_tree=False): logging.debug("Generating the channel metadata.") + base_tree = channel.staging_tree if use_staging_tree else channel.main_tree kolibri_channel = kolibrimodels.ChannelMetadata.objects.create( id=channel.id, name=channel.name, @@ -741,8 +749,8 @@ def map_channel_to_kolibri_channel(channel): tagline=channel.tagline, version=channel.version + 1, # Need to save as version being published, not current version thumbnail=channel.icon_encoding, - root_pk=channel.main_tree.node_id, - root_id=channel.main_tree.node_id, + root_pk=base_tree.node_id, + root_id=base_tree.node_id, min_schema_version=MIN_SCHEMA_VERSION, # Need to modify Kolibri so we can import this without importing models ) logging.info("Generated the channel metadata.") @@ -805,25 +813,33 @@ def raise_if_nodes_are_all_unchanged(channel): logging.info("Some nodes are changed.") -def mark_all_nodes_as_published(channel): +def mark_all_nodes_as_published(tree): logging.debug("Marking all nodes as published.") - channel.main_tree.get_family().update(changed=False, published=True) + tree.get_family().update(changed=False, published=True) logging.info("Marked all nodes as published.") -def save_export_database(channel_id, version): +def save_export_database(channel_id, version, use_staging_tree=False): logging.debug("Saving export database") current_export_db_location = get_active_content_database() - target_paths = [ - os.path.join( - settings.DB_ROOT, "{id}.sqlite3".format(id=channel_id) - ), - os.path.join( - settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) - ), - ] + # Only create "-next.sqlite3" if staging_tree is used + if use_staging_tree: + target_paths = [ + os.path.join( + settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) + ) + ] + else: + target_paths = [ + os.path.join( + settings.DB_ROOT, "{id}.sqlite3".format(id=channel_id) + ), + os.path.join( + settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) + ), + ] for target_export_db_location in target_paths: with open(current_export_db_location, 'rb') as currentf: @@ -919,30 +935,44 @@ def publish_channel( send_email=False, progress_tracker=None, language=settings.LANGUAGE_CODE, + use_staging_tree=False, ): """ :type progress_tracker: contentcuration.utils.celery.ProgressTracker|None """ channel = ccmodels.Channel.objects.get(pk=channel_id) + base_tree = channel.staging_tree if use_staging_tree else channel.main_tree + if base_tree is None: + tree_name = "staging_tree" if use_staging_tree else "main_tree" + raise NoneContentNodeTreeError(f"{tree_name} is None!") kolibri_temp_db = None start = time.time() try: set_channel_icon_encoding(channel) - kolibri_temp_db = create_content_database(channel, force, user_id, force_exercises, progress_tracker=progress_tracker) - increment_channel_version(channel) + kolibri_temp_db = create_content_database( + channel, + force, + user_id, + force_exercises, + progress_tracker=progress_tracker, + use_staging_tree=use_staging_tree, + ) add_tokens_to_channel(channel) - sync_contentnode_and_channel_tsvectors(channel_id=channel.id) - mark_all_nodes_as_published(channel) - fill_published_fields(channel, version_notes) + if not use_staging_tree: + increment_channel_version(channel) + sync_contentnode_and_channel_tsvectors(channel_id=channel.id) + mark_all_nodes_as_published(base_tree) + if not use_staging_tree: + fill_published_fields(channel, version_notes) # Attributes not getting set for some reason, so just save it here - channel.main_tree.publishing = False - channel.main_tree.changed = False - channel.main_tree.published = True - channel.main_tree.save() + base_tree.publishing = False + base_tree.changed = False + base_tree.published = True + base_tree.save() # Delete public channel cache. - if channel.public: + if not use_staging_tree and channel.public: delete_public_channel_cache_keys() if send_email: @@ -960,8 +990,8 @@ def publish_channel( finally: if kolibri_temp_db and os.path.exists(kolibri_temp_db): os.remove(kolibri_temp_db) - channel.main_tree.publishing = False - channel.main_tree.save() + base_tree.publishing = False + base_tree.save() elapsed = time.time() - start From 9bb12df2d815d047c811e8d65933de020757c5dc Mon Sep 17 00:00:00 2001 From: taoerman Date: Mon, 31 Mar 2025 00:14:34 -0700 Subject: [PATCH 2/3] Remove unused imports --- contentcuration/contentcuration/tests/test_exportchannel.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 753b6f285f..f4143b53b1 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -7,12 +7,9 @@ import pytest from celery import states -from django.conf import settings from django.core.management import call_command from django.db import connections from django_celery_results.models import TaskResult -from django.core.files.storage import FileSystemStorage -from django.test.utils import override_settings 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 From 486e0667bfb89ee391635da526c550a8b5d2c20d Mon Sep 17 00:00:00 2001 From: taoerman Date: Wed, 2 Apr 2025 20:46:58 -0700 Subject: [PATCH 3/3] Small updates --- .../contentcuration/utils/publish.py | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index b4ec4acc77..d50934af79 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -824,22 +824,16 @@ def mark_all_nodes_as_published(tree): def save_export_database(channel_id, version, use_staging_tree=False): logging.debug("Saving export database") current_export_db_location = get_active_content_database() - # Only create "-next.sqlite3" if staging_tree is used - if use_staging_tree: - target_paths = [ - os.path.join( - settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) - ) - ] - else: - target_paths = [ - os.path.join( - settings.DB_ROOT, "{id}.sqlite3".format(id=channel_id) - ), - os.path.join( - settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) - ), - ] + target_paths = [ + os.path.join( + settings.DB_ROOT, "{}-{}.sqlite3".format(channel_id, version) + ) + ] + # Only create non-version path if not using the staging tree + if not use_staging_tree: + target_paths.append( + os.path.join(settings.DB_ROOT, "{id}.sqlite3".format(id=channel_id) + )) for target_export_db_location in target_paths: with open(current_export_db_location, 'rb') as currentf: @@ -961,8 +955,7 @@ def publish_channel( if not use_staging_tree: increment_channel_version(channel) sync_contentnode_and_channel_tsvectors(channel_id=channel.id) - mark_all_nodes_as_published(base_tree) - if not use_staging_tree: + mark_all_nodes_as_published(base_tree) fill_published_fields(channel, version_notes) # Attributes not getting set for some reason, so just save it here