Merge pull request #1191 from edx/dhm/orphan_finder

Write restful service to find all orphans

CHANGELOG.rst

@@ -9,6 +9,14 @@ Blades: When start time and end time are specified for a video, a visual range
 will be shown on the time slider to highlight the place in the video that will
 be played.
 
+Studio: added restful interface for finding orphans in courses. 
+An orphan is an xblock to which no children relation points and whose type is not 
+in the set contentstore.views.item.DETACHED_CATEGORIES nor 'course'.
+    GET http://host/orphan/org.course returns json array of ids. 
+        Requires course author access.
+    DELETE http://orphan/org.course deletes all the orphans in that course. 
+        Requires is_staff access
+
 Studio: Bug fix for text loss in Course Updates when the text exists
 before the first tag.
 

cms/djangoapps/contentstore/tests/test_orphan.py

+"""
+Test finding orphans via the view and django config
+"""
+import json
+from contentstore.tests.utils import CourseTestCase
+from xmodule.modulestore.django import editable_modulestore, loc_mapper
+from django.core.urlresolvers import reverse
+from student.models import CourseEnrollment
+
+class TestOrphan(CourseTestCase):
+    """
+    Test finding orphans via view and django config
+    """
+    def setUp(self):
+        super(TestOrphan, self).setUp()
+
+        runtime = self.course.runtime
+
+        self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', self.course.location.name, runtime)
+        self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', self.course.location.name, runtime)
+        self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None, runtime)
+        self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime)
+        self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None, runtime)
+        self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime)
+        self._create_item('html', 'OrphanHtml', "<p>Hello</p>", {'display_name': 'Orphan html'}, None, None, runtime)
+        self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None, runtime)
+        self._create_item('about', 'overview', "<p>overview</p>", {}, None, None, runtime)
+        self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None, runtime)
+
+    def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime):
+        location = self.course.location.replace(category=category, name=name)
+        editable_modulestore('direct').create_and_save_xmodule(location, data, metadata, runtime)
+        if parent_name:
+            # add child to parent in mongo
+            parent_location = self.course.location.replace(category=parent_category, name=parent_name)
+            parent = editable_modulestore('direct').get_item(parent_location)
+            parent.children.append(location.url())
+            editable_modulestore('direct').update_children(parent_location, parent.children)
+
+    def test_mongo_orphan(self):
+        """
+        Test that old mongo finds the orphans
+        """
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+
+        orphans = json.loads(
+            self.client.get(
+                orphan_url,
+                HTTP_ACCEPT='application/json'
+            ).content
+        )
+        self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
+        location = self.course.location.replace(category='chapter', name='OrphanChapter')
+        self.assertIn(location.url(), orphans)
+        location = self.course.location.replace(category='vertical', name='OrphanVert')
+        self.assertIn(location.url(), orphans)
+        location = self.course.location.replace(category='html', name='OrphanHtml')
+        self.assertIn(location.url(), orphans)
+
+    def test_mongo_orphan_delete(self):
+        """
+        Test that old mongo deletes the orphans
+        """
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+        self.client.delete(orphan_url)
+        orphans = json.loads(
+            self.client.get(orphan_url, HTTP_ACCEPT='application/json').content
+        )
+        self.assertEqual(len(orphans), 0, "Orphans not deleted {}".format(orphans))
+
+    def test_not_permitted(self):
+        """
+        Test that auth restricts get and delete appropriately
+        """
+        test_user_client, test_user = self.createNonStaffAuthedUserClient()
+        CourseEnrollment.enroll(test_user, self.course.location.course_id)
+        locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True)
+        orphan_url = locator.url_reverse('orphan/', '')
+        response = test_user_client.get(orphan_url)
+        self.assertEqual(response.status_code, 403)
+        response = test_user_client.delete(orphan_url)
+        self.assertEqual(response.status_code, 403)

cms/djangoapps/contentstore/tests/utils.py

@@ -65,7 +65,7 @@ class CourseTestCase(ModuleStoreTestCase):
 
     def createNonStaffAuthedUserClient(self):
         """
-        Create a non-staff user, log them in, and return the client to use for testing.
+        Create a non-staff user, log them in, and return the client, user to use for testing.
         """
         uname = 'teststudent'
         password = 'foo'

cms/djangoapps/contentstore/views/item.py

@@ -7,7 +7,7 @@ from django.core.exceptions import PermissionDenied
 from django.contrib.auth.decorators import login_required
 
 from xmodule.modulestore import Location
-from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.django import modulestore, loc_mapper
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 
@@ -20,8 +20,11 @@ from ..utils import get_modulestore
 from .access import has_access
 from .helpers import _xmodule_recurse
 from xmodule.x_module import XModuleDescriptor
+from django.views.decorators.http import require_http_methods
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
+from student.models import CourseEnrollment
 
-__all__ = ['save_item', 'create_item', 'delete_item']
+__all__ = ['save_item', 'create_item', 'delete_item', 'orphan']
 
 log = logging.getLogger(__name__)
 
@@ -200,3 +203,34 @@ def delete_item(request):
                 modulestore('direct').update_children(parent.location, parent.children)
 
     return JsonResponse()
+
+# pylint: disable=W0613
+@login_required
+@require_http_methods(("GET", "DELETE"))
+def orphan(request, tag=None, course_id=None, branch=None, version_guid=None, block=None):
+    """
+    View for handling orphan related requests. GET gets all of the current orphans.
+    DELETE removes all orphans (requires is_staff access)
+
+    An orphan is a block whose category is not in the DETACHED_CATEGORY list, is not the root, and is not reachable
+    from the root via children
+
+    :param request:
+    :param course_id: Locator syntax course_id
+    """
+    location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+    # DHM: when split becomes back-end, move or conditionalize this conversion
+    old_location = loc_mapper().translate_locator_to_location(location)
+    if request.method == 'GET':
+        if has_access(request.user, old_location):
+            return JsonResponse(modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft'))
+        else:
+            raise PermissionDenied()
+    if request.method == 'DELETE':
+        if request.user.is_staff:
+            items = modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft')
+            for item in items:
+                modulestore('draft').delete_item(item, True)
+            return JsonResponse({'deleted': items})
+        else:
+            raise PermissionDenied()

cms/urls.py

@@ -130,6 +130,7 @@ urlpatterns += patterns(
     url(r'^login_post$', 'student.views.login_user', name='login_post'),
 
     url(r'^logout$', 'student.views.logout_user', name='logout'),
+
 )
 
 # restful api
@@ -140,6 +141,7 @@ urlpatterns += patterns(
     # (?ix) == ignore case and verbose (multiline regex)
     url(r'(?ix)^course/{}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
     url(r'(?ix)^checklists/{}(/)?(?P<checklist_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'),
+    url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan')
 )
 
 js_info_dict = {

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -34,6 +34,7 @@ from xblock.fields import Scope, ScopeIds
 from xmodule.modulestore import ModuleStoreBase, Location, MONGO_MODULESTORE_TYPE
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
+import re
 
 log = logging.getLogger(__name__)
 
@@ -697,7 +698,7 @@ class MongoModuleStore(ModuleStoreBase):
             course.tabs = existing_tabs
             # Save any changes to the course to the MongoKeyValueStore
             course.save()
-            self.update_metadata(course.location, course.xblock_kvs._metadata)
+            self.update_metadata(course.location, course.get_explicitly_set_fields_by_scope(Scope.settings))
 
     def fire_updated_modulestore_signal(self, course_id, location):
         """
@@ -854,6 +855,24 @@ class MongoModuleStore(ModuleStoreBase):
         """
         return MONGO_MODULESTORE_TYPE
 
+    def get_orphans(self, course_location, detached_categories, _branch):
+        """
+        Return an array all of the locations for orphans in the course.
+        """
+        all_items = self.collection.find({
+            '_id.org': course_location.org,
+            '_id.course': course_location.course,
+            '_id.category': {'$nin': detached_categories}
+        })
+        all_reachable = set()
+        item_locs = set()
+        for item in all_items:
+            if item['_id']['category'] != 'course':
+                item_locs.add(Location(item['_id']).replace(revision=None).url())
+            all_reachable = all_reachable.union(item.get('definition', {}).get('children', []))
+        item_locs -= all_reachable
+        return list(item_locs)
+
     def _create_new_field_data(self, category, location, definition_data, metadata):
         """
         To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -421,6 +421,24 @@ class SplitMongoModuleStore(ModuleStoreBase):
                     items.append(BlockUsageLocator(url=locator.as_course_locator(), usage_id=parent_id))
         return items
 
+    def get_orphans(self, course_id, detached_categories, branch):
+        """
+        Return a dict of all of the orphans in the course.
+
+        :param course_id:
+        """
+        course = self._lookup_course(CourseLocator(course_id=course_id, branch=branch))
+        items = set(course['structure']['blocks'].keys())
+        items.remove(course['structure']['root'])
+        for block_id, block_data in course['structure']['blocks'].iteritems():
+            items.difference_update(block_data.get('fields', {}).get('children', []))
+            if block_data['category'] in detached_categories:
+                items.discard(block_id)
+        return [
+            BlockUsageLocator(course_id=course_id, branch=branch, usage_id=block_id)
+            for block_id in items
+        ]
+
     def get_course_index_info(self, course_locator):
         """
         The index records the initial creation of the indexed course and tracks the current version

common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py

+import uuid
+import mock
+import unittest
+import random
+import datetime
+
+from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.mongo import MongoModuleStore
+from xmodule.modulestore.split_mongo import SplitMongoModuleStore
+from xmodule.modulestore import Location
+from xmodule.fields import Date
+from xmodule.modulestore.locator import BlockUsageLocator, CourseLocator
+
+
+class TestOrphan(unittest.TestCase):
+    """
+    Test the orphan finding code
+    """
+
+    # Snippet of what would be in the django settings envs file
+    db_config = {
+        'host': 'localhost',
+        'db': 'test_xmodule',
+    }
+
+    modulestore_options = {
+        'default_class': 'xmodule.raw_module.RawDescriptor',
+        'fs_root': '',
+        'render_template': mock.Mock(return_value=""),
+        'xblock_mixins': (InheritanceMixin,)
+    }
+
+    split_course_id = 'test_org.test_course.runid'
+
+    def setUp(self):
+        self.db_config['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex)
+
+        self.userid = random.getrandbits(32)
+        super(TestOrphan, self).setUp()
+        self.split_mongo = SplitMongoModuleStore(
+            self.db_config,
+            **self.modulestore_options
+        )
+        self.addCleanup(self.tear_down_split)
+        self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
+        self.addCleanup(self.tear_down_mongo)
+        self.course_location = None
+        self._create_course()
+
+    def tear_down_split(self):
+        """
+        Remove the test collections, close the db connection
+        """
+        split_db = self.split_mongo.db
+        split_db.drop_collection(split_db.course_index)
+        split_db.drop_collection(split_db.structures)
+        split_db.drop_collection(split_db.definitions)
+        split_db.connection.close()
+
+    def tear_down_mongo(self):
+        """
+        Remove the test collections, close the db connection
+        """
+        split_db = self.split_mongo.db
+        # old_mongo doesn't give a db attr, but all of the dbs are the same
+        split_db.drop_collection(self.old_mongo.collection)
+
+    def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime):
+        """
+        Create the item of the given category and block id in split and old mongo, add it to the optional
+        parent. The parent category is only needed because old mongo requires it for the id.
+        """
+        location = Location('i4x', 'test_org', 'test_course', category, name)
+        self.old_mongo.create_and_save_xmodule(location, data, metadata, runtime)
+        if isinstance(data, basestring):
+            fields = {'data': data}
+        else:
+            fields = data.copy()
+        fields.update(metadata)
+        if parent_name:
+            # add child to parent in mongo
+            parent_location = Location('i4x', 'test_org', 'test_course', parent_category, parent_name)
+            parent = self.old_mongo.get_item(parent_location)
+            parent.children.append(location.url())
+            self.old_mongo.update_children(parent_location, parent.children)
+            # create pointer for split
+            course_or_parent_locator = BlockUsageLocator(
+                course_id=self.split_course_id,
+                branch='draft',
+                usage_id=parent_name
+            )
+        else:
+            course_or_parent_locator = CourseLocator(
+                course_id='test_org.test_course.runid',
+                branch='draft',
+            )
+        self.split_mongo.create_item(course_or_parent_locator, category, self.userid, usage_id=name, fields=fields)
+
+    def _create_course(self):
+        """
+        * some detached items
+        * some attached children
+        * some orphans
+        """
+        date_proxy = Date()
+        metadata = {
+            'start': date_proxy.to_json(datetime.datetime(2000, 3, 13, 4)),
+            'display_name': 'Migration test course',
+        }
+        data = {
+            'wiki_slug': 'test_course_slug'
+        }
+        fields = metadata.copy()
+        fields.update(data)
+        # split requires the course to be created separately from creating items
+        self.split_mongo.create_course(
+            'test_org', 'my course', self.userid, self.split_course_id, fields=fields, root_usage_id='runid'
+        )
+        self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid')
+        self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata)
+        runtime = self.old_mongo.get_item(self.course_location).runtime
+
+        self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', runtime)
+        self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', runtime)
+        self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None, runtime)
+        self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime)
+        self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None, runtime)
+        self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime)
+        self._create_item('html', 'OrphanHtml', "<p>Hello</p>", {'display_name': 'Orphan html'}, None, None, runtime)
+        self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None, runtime)
+        self._create_item('about', 'overview', "<p>overview</p>", {}, None, None, runtime)
+        self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None, runtime)
+
+    def test_mongo_orphan(self):
+        """
+        Test that old mongo finds the orphans
+        """
+        orphans = self.old_mongo.get_orphans(self.course_location, ['static_tab', 'about', 'course_info'], None)
+        self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
+        location = self.course_location.replace(category='chapter', name='OrphanChapter')
+        self.assertIn(location.url(), orphans)
+        location = self.course_location.replace(category='vertical', name='OrphanVert')
+        self.assertIn(location.url(), orphans)
+        location = self.course_location.replace(category='html', name='OrphanHtml')
+        self.assertIn(location.url(), orphans)
+
+    def test_split_orphan(self):
+        """
+        Test that old mongo finds the orphans
+        """
+        orphans = self.split_mongo.get_orphans(self.split_course_id, ['static_tab', 'about', 'course_info'], 'draft')
+        self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
+        location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanChapter')
+        self.assertIn(location, orphans)
+        location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanVert')
+        self.assertIn(location, orphans)
+        location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanHtml')
+        self.assertIn(location, orphans)