Merge pull request #1487 from MITx/fix/cdodge/cleanup-deleted-templates

clean up templates that were removed from disk - we need to remove from ...

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -27,10 +27,12 @@ from xmodule.contentstore.django import contentstore
 from xmodule.templates import update_templates
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.templates import update_templates
 
 from xmodule.capa_module import CapaDescriptor
 from xmodule.course_module import CourseDescriptor
 from xmodule.seq_module import SequenceDescriptor
+from xmodule.modulestore.exceptions import ItemNotFoundError
 
 TEST_DATA_MODULESTORE = copy.deepcopy(settings.MODULESTORE)
 TEST_DATA_MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data')
@@ -409,3 +411,32 @@ class ContentStoreTest(ModuleStoreTestCase):
         self.assertIn('markdown', context, "markdown is missing from context")
         self.assertIn('markdown', problem.metadata, "markdown is missing from metadata")
         self.assertNotIn('markdown', problem.editable_metadata_fields, "Markdown slipped into the editable metadata fields")
+
+
+class TemplateTestCase(ModuleStoreTestCase):
+
+    def test_template_cleanup(self):        
+        ms = modulestore('direct')
+
+        # insert a bogus template in the store
+        bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus')
+        source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Empty')
+        
+        ms.clone_item(source_template_location, bogus_template_location)
+
+        verify_create = ms.get_item(bogus_template_location)
+        self.assertIsNotNone(verify_create)
+
+        # now run cleanup
+        update_templates()
+
+        # now try to find dangling template, it should not be in DB any longer
+        asserted = False
+        try:
+            verify_create = ms.get_item(bogus_template_location)
+        except ItemNotFoundError:
+            asserted = True
+
+        self.assertTrue(asserted)     
+
+

cms/djangoapps/contentstore/tests/tests.py

@@ -85,7 +85,6 @@ class ContentStoreTestCase(ModuleStoreTestCase):
         # Now make sure that the user is now actually activated
         self.assertTrue(user(email).is_active)
 
-
 class AuthTestCase(ContentStoreTestCase):
     """Check that various permissions-related things work"""
 

common/lib/xmodule/xmodule/templates.py

@@ -56,6 +56,10 @@ def update_templates():
     available from the installed plugins
     """
 
+    # cdodge: build up a list of all existing templates. This will be used to determine which
+    # templates have been removed from disk - and thus we need to remove from the DB
+    templates_to_delete = modulestore('direct').get_items(['i4x', 'edx', 'templates', None, None, None])
+
     for category, templates in all_templates().items():
         for template in templates:
             if 'display_name' not in template.metadata:
@@ -85,3 +89,12 @@ def update_templates():
             modulestore('direct').update_item(template_location, template.data)
             modulestore('direct').update_children(template_location, template.children)
             modulestore('direct').update_metadata(template_location, template.metadata)
+
+            # remove template from list of templates to delete
+            templates_to_delete = [t for t in templates_to_delete if t.location != template_location]
+
+    # now remove all templates which appear to have removed from disk
+    if len(templates_to_delete) > 0:
+        logging.debug('deleting dangling templates = {0}'.format(templates_to_delete))
+        for template in templates_to_delete:
+            modulestore('direct').delete_item(template.location)