Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Optimize and paginate list_libraries API, add num_blocks and last_published fields

Browse Source
This commit is contained in:
Sid Verma
2020-07-18 14:54:12 +05:30
committed by Kyle McCormick
parent 8d33a5a3e1
commit 1d2fbcc4cc
10 changed files with 191 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
openedx/core/djangoapps/content_libraries/api.py
View File

@@ -36,6 +36,7 @@ for these in most other learning contexts then those methods could be promoted
to the core XBlock API and made generic.
"""
from uuid import UUID
from datetime import datetime
import logging
import attr
@@ -45,6 +46,7 @@ from django.core.exceptions import PermissionDenied
from django.core.validators import validate_unicode_slug
from django.db import IntegrityError
from django.utils.translation import ugettext as _
from elasticsearch.exceptions import ConnectionError as ElasticConnectionError
from lxml import etree
from opaque_keys.edx.keys import LearningContextKey
from opaque_keys.edx.locator import BundleDefinitionLocator, LibraryLocatorV2, LibraryUsageLocatorV2
@@ -56,6 +58,7 @@ from xblock.exceptions import XBlockNotFoundError
from openedx.core.djangoapps.content_libraries import permissions
from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME
from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryNotIndexedException
from openedx.core.djangoapps.content_libraries.models import ContentLibrary, ContentLibraryPermission
from openedx.core.djangoapps.content_libraries.signals import (
CONTENT_LIBRARY_CREATED,
@@ -90,6 +93,12 @@ log = logging.getLogger(__name__)
ContentLibraryNotFound = ContentLibrary.DoesNotExist
class ServerError(APIException):
""" A 500 server error """
status_code = 500
default_detail = "Error occurred in the server. Please contact support with details of this error"
class ContentLibraryBlockNotFound(XBlockNotFoundError):
""" XBlock not found in the content library """
@@ -121,7 +130,9 @@ class ContentLibraryMetadata:
bundle_uuid = attr.ib(type=UUID)
title = attr.ib("")
description = attr.ib("")
num_blocks = attr.ib(0)
version = attr.ib(0)
last_published = attr.ib(default=None, type=datetime)
has_unpublished_changes = attr.ib(False)
# has_unpublished_deletes will be true when the draft version of the library's bundle
# contains deletes of any XBlocks that were in the most recently published version
@@ -214,16 +225,44 @@ class AccessLevel:
NO_ACCESS = None
def list_libraries_for_user(user):
def get_libraries_for_user(user):
"""
Lists up to 50 content libraries that the user has permission to view.
This method makes at least one HTTP call per library so should only be used
for development until we have something more efficient.
Return content libraries that the user has permission to view.
"""
qs = ContentLibrary.objects.all()
filtered_qs = permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs)
return [get_library(ref.library_key) for ref in filtered_qs[:50]]
return permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs)
def get_metadata_from_index(queryset):
"""
Take a list of ContentLibrary objects and return metadata stored in
ContentLibraryIndex.
"""
if not ContentLibraryIndexer.indexing_is_enabled():
raise NotImplementedError("Library indexing needs to be enabled for this API to work")
library_keys = [lib.library_key for lib in queryset]
try:
metadata = ContentLibraryIndexer.get_libraries(library_keys)
libraries = [
ContentLibraryMetadata(
key=key,
bundle_uuid=metadata[i]['uuid'],
title=metadata[i]['title'],
description=metadata[i]['description'],
num_blocks=metadata[i]['num_blocks'],
version=metadata[i]['version'],
last_published=metadata[i]['last_published'],
allow_public_learning=queryset[i].allow_public_learning,
allow_public_read=queryset[i].allow_public_read,
has_unpublished_changes=metadata[i]['has_unpublished_changes'],
has_unpublished_deletes=metadata[i]['has_unpublished_deletes'],
)
for i, key in enumerate(library_keys)
]
return libraries
except (LibraryNotIndexedException, KeyError) as e:
log.exception(e)
raise ServerError("Libraries have missing or invalid indexes, and need to be updated.")
def require_permission_for_library_key(library_key, user, permission):
@@ -253,13 +292,17 @@ def get_library(library_key):
ref = ContentLibrary.objects.get_by_key(library_key)
bundle_metadata = get_bundle(ref.bundle_uuid)
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
num_blocks = len(lib_bundle.get_top_level_usages())
last_published = lib_bundle.get_last_published_time()
(has_unpublished_changes, has_unpublished_deletes) = lib_bundle.has_changes()
return ContentLibraryMetadata(
key=library_key,
bundle_uuid=ref.bundle_uuid,
title=bundle_metadata.title,
description=bundle_metadata.description,
num_blocks=num_blocks,
version=bundle_metadata.latest_version,
last_published=last_published,
allow_public_learning=ref.allow_public_learning,
allow_public_read=ref.allow_public_read,
has_unpublished_changes=has_unpublished_changes,
@@ -313,7 +356,9 @@ def create_library(collection_uuid, org, slug, title, description, allow_public_
bundle_uuid=bundle.uuid,
title=title,
description=description,
num_blocks=0,
version=0,
last_published=None,
allow_public_learning=ref.allow_public_learning,
allow_public_read=ref.allow_public_read,
)

9
openedx/core/djangoapps/content_libraries/libraries_index.py
View File

@@ -51,7 +51,11 @@ class ContentLibraryIndexer:
for library_key in library_keys:
ref = ContentLibrary.objects.get_by_key(library_key)
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
usages = lib_bundle.get_top_level_usages()
num_blocks = len(lib_bundle.get_top_level_usages())
last_published = lib_bundle.get_last_published_time()
last_published_str = None
if last_published:
last_published_str = last_published.strftime('%Y-%m-%dT%H:%M:%SZ')
(has_unpublished_changes, has_unpublished_deletes) = lib_bundle.has_changes()
bundle_metadata = get_bundle(ref.bundle_uuid)
@@ -61,8 +65,9 @@ class ContentLibraryIndexer:
"uuid": str(bundle_metadata.uuid),
"title": bundle_metadata.title,
"description": bundle_metadata.description,
"num_blocks": num_blocks,
"version": bundle_metadata.latest_version,
"num_blocks": len(usages),
"last_published": last_published_str,
"has_unpublished_changes": has_unpublished_changes,
"has_unpublished_deletes": has_unpublished_deletes,
}

18
openedx/core/djangoapps/content_libraries/library_bundle.py
View File

@@ -2,6 +2,7 @@
Helper code for working with Blockstore bundles that contain OLX
"""
import dateutil.parser
import logging
from django.utils.lru_cache import lru_cache
@@ -349,6 +350,23 @@ class LibraryBundle(object):
if f.path.startswith(path_prefix)
]
def get_last_published_time(self):
"""
Return the timestamp when the current library was last published. If the
current draft has never been published, return 0.
"""
cache_key = ("last_published_time", )
usages_found = self.cache.get(cache_key)
if usages_found is not None:
return usages_found
version = get_bundle_version_number(self.bundle_uuid)
if version == 0:
return None
created_at_str = blockstore_api.get_bundle_version(self.bundle_uuid, version)['snapshot']['created_at']
last_published_time = dateutil.parser.parse(created_at_str)
self.cache.set(cache_key, last_published_time)
return last_published_time
@staticmethod
def olx_prefix(definition_key):
"""

4
openedx/core/djangoapps/content_libraries/serializers.py
View File

@@ -8,6 +8,8 @@ from rest_framework import serializers
from openedx.core.djangoapps.content_libraries.models import ContentLibraryPermission
from openedx.core.lib import blockstore_api
DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
class ContentLibraryMetadataSerializer(serializers.Serializer):
"""
@@ -25,7 +27,9 @@ class ContentLibraryMetadataSerializer(serializers.Serializer):
collection_uuid = serializers.UUIDField(format='hex_verbose', write_only=True)
title = serializers.CharField()
description = serializers.CharField(allow_blank=True)
num_blocks = serializers.IntegerField(read_only=True)
version = serializers.IntegerField(read_only=True)
last_published = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True)
allow_public_learning = serializers.BooleanField(default=False)
allow_public_read = serializers.BooleanField(default=False)
has_unpublished_changes = serializers.BooleanField(read_only=True)

8
openedx/core/djangoapps/content_libraries/tests/base.py
View File

@@ -4,6 +4,7 @@ Tests for Blockstore-based Content Libraries
"""
from contextlib import contextmanager
from io import BytesIO
from urllib.parse import urlencode
import unittest
from django.conf import settings
@@ -18,6 +19,7 @@ from openedx.core.lib import blockstore_api
# backwards-incompatible changes like changed URLs.
URL_PREFIX = '/api/libraries/v2/'
URL_LIB_CREATE = URL_PREFIX
URL_LIB_LIST = URL_PREFIX + '?{query_params}'
URL_LIB_DETAIL = URL_PREFIX + '{lib_key}/' # Get data about a library, update or delete library
URL_LIB_BLOCK_TYPES = URL_LIB_DETAIL + 'block_types/' # Get the list of XBlock types that can be added to this library
URL_LIB_LINKS = URL_LIB_DETAIL + 'links/' # Get the list of links in this library, or add a new one
@@ -132,6 +134,12 @@ class ContentLibrariesRestApiTest(APITestCase):
"collection_uuid": str(self.collection.uuid),
}, expect_response)
def _list_libraries(self, query_params_dict=None, expect_response=200):
""" List libraries """
if query_params_dict is None:
query_params_dict = {}
return self._api('get', URL_LIB_LIST.format(query_params=urlencode(query_params_dict)), None, expect_response)
def _get_library(self, lib_key, expect_response=200):
""" Get a library """
return self._api('get', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response)

42
openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
View File

@@ -6,6 +6,7 @@ import unittest
from uuid import UUID
from django.contrib.auth.models import Group
from mock import patch
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
from openedx.core.djangoapps.content_libraries.api import BlockLimitReachedError
@@ -81,6 +82,47 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
@patch("openedx.core.djangoapps.content_libraries.views.LibraryRootPagination.page_size", new=2)
@override_settings(SEARCH_ENGINE="search.tests.mock_search_engine.MockSearchEngine")
def test_list_library(self, is_indexing_enabled):
"""
Test the /libraries API and its pagination
"""
with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}):
lib1 = self._create_library(slug="some-slug-1", title="Existing Library")
lib2 = self._create_library(slug="some-slug-2", title="Existing Library")
if not is_indexing_enabled:
lib1['num_blocks'] = lib2['num_blocks'] = None
lib1['last_published'] = lib2['last_published'] = None
lib1['has_unpublished_changes'] = lib2['has_unpublished_changes'] = None
lib1['has_unpublished_deletes'] = lib2['has_unpublished_deletes'] = None
result = self._list_libraries()
self.assertEqual(len(result), 2)
self.assertEqual(result[0], lib1)
result = self._list_libraries({'pagination': 'true'})
self.assertEqual(len(result['results']), 2)
self.assertEqual(result['next'], None)
# Create another library which causes number of libraries to exceed the page size
self._create_library(slug="some-slug-3", title="Existing Library")
# Verify that if `pagination` param isn't sent, API still honors the max page size.
# This is for maintaining compatibility with older non pagination-aware clients.
result = self._list_libraries()
self.assertEqual(len(result), 2)
# Pagination enabled:
# Verify total elements and valid 'next' in page 1
result = self._list_libraries({'pagination': 'true'})
self.assertEqual(len(result['results']), 2)
self.assertIn('page=2', result['next'])
self.assertIn('pagination=true', result['next'])
# Verify total elements and null 'next' in page 2
result = self._list_libraries({'pagination': 'true', 'page': '2'})
self.assertEqual(len(result['results']), 1)
self.assertEqual(result['next'], None)
# General Content Library XBlock tests:
def test_library_blocks(self):

9
openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py
View File

@@ -41,8 +41,9 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
self.assertEqual(response['title'], result['title'])
self.assertEqual(response['description'], result['description'])
self.assertEqual(response['uuid'], result['bundle_uuid'])
self.assertEqual(response['version'], result['version'])
self.assertEqual(response['num_blocks'], 0)
self.assertEqual(response['version'], result['version'])
self.assertEqual(response['last_published'], None)
self.assertEqual(response['has_unpublished_changes'], False)
self.assertEqual(response['has_unpublished_deletes'], False)
@@ -75,8 +76,9 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
self.assertEqual(response['title'], "New Title")
self.assertEqual(response['description'], "New Title")
self.assertEqual(response['uuid'], lib['bundle_uuid'])
self.assertEqual(response['version'], lib['version'])
self.assertEqual(response['num_blocks'], 0)
self.assertEqual(response['version'], lib['version'])
self.assertEqual(response['last_published'], None)
self.assertEqual(response['has_unpublished_changes'], False)
self.assertEqual(response['has_unpublished_deletes'], False)
@@ -92,10 +94,12 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
"""
Commit library changes, and verify that there are no uncommited changes anymore
"""
last_published = ContentLibraryIndexer.get_libraries([library_key])[0]['last_published']
self._commit_library_changes(str(library_key))
response = ContentLibraryIndexer.get_libraries([library_key])[0]
self.assertEqual(response['has_unpublished_changes'], False)
self.assertEqual(response['has_unpublished_deletes'], False)
self.assertGreaterEqual(response['last_published'], last_published)
return response
def verify_uncommitted_libraries(library_key, has_unpublished_changes, has_unpublished_deletes):
@@ -113,6 +117,7 @@ class ContentLibraryIndexerIndexer(ContentLibrariesRestApiTest):
# Verify uncommitted new blocks
block = self._add_block_to_library(lib['id'], "problem", "problem1")
response = verify_uncommitted_libraries(library_key, True, False)
self.assertEqual(response['last_published'], None)
self.assertEqual(response['num_blocks'], 1)
# Verify committed new blocks
self._commit_library_changes(lib['id'])

45
openedx/core/djangoapps/content_libraries/views.py
View File

@@ -7,10 +7,12 @@ import logging
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Group
from django.shortcuts import get_object_or_404
import edx_api_doc_tools as apidocs
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
from organizations.models import Organization
from rest_framework import status
from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError
from rest_framework.pagination import PageNumberPagination
from rest_framework.parsers import MultiPartParser
from rest_framework.response import Response
from rest_framework.views import APIView
@@ -61,20 +63,53 @@ def convert_exceptions(fn):
return wrapped_fn
class LibraryRootPagination(PageNumberPagination):
"""
Paginates over ContentLibraryMetadata objects.
"""
page_size = 50
page_size_query_param = 'page_size'
@view_auth_classes()
class LibraryRootView(APIView):
"""
Views to list, search for, and create content libraries.
"""
@apidocs.schema(
parameters=[
apidocs.query_parameter(
'pagination',
bool,
description="Enables paginated schema",
),
apidocs.query_parameter(
'page',
int,
description="Page number of result. Defaults to 1",
),
apidocs.query_parameter(
'page_size',
int,
description="Page size of the result. Defaults to 50",
),
],
)
def get(self, request):
"""
Return a list of all content libraries that the user has permission to
view. This is a temporary view for development and returns at most 50
libraries.
Return a list of all content libraries that the user has permission to view.
"""
result = api.list_libraries_for_user(request.user)
return Response(ContentLibraryMetadataSerializer(result, many=True).data)
paginator = LibraryRootPagination()
queryset = api.get_libraries_for_user(request.user)
paginated_qs = paginator.paginate_queryset(queryset, request)
result = api.get_metadata_from_index(paginated_qs)
serializer = ContentLibraryMetadataSerializer(result, many=True)
# Verify `pagination` param to maintain compatibility with older
# non pagination-aware clients
if request.GET.get('pagination', 'false').lower() == 'true':
return paginator.get_paginated_response(serializer.data)
return Response(serializer.data)
def post(self, request):
"""

1
openedx/core/lib/blockstore_api/__init__.py
View File

@@ -38,6 +38,7 @@ from .methods import (
get_bundle_files_dict,
get_bundle_file_metadata,
get_bundle_file_data,
get_bundle_version,
get_bundle_version_files,
# Links:
get_bundle_links,

16
openedx/core/lib/blockstore_api/methods.py
View File

@@ -248,14 +248,23 @@ def delete_draft(draft_uuid):
api_request('delete', api_url('drafts', str(draft_uuid)))
def get_bundle_version(bundle_uuid, version_number):
"""
Get the details of the specified bundle version
"""
if version_number == 0:
return None
version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number))
return api_request('get', version_url)
def get_bundle_version_files(bundle_uuid, version_number):
"""
Get a list of the files in the specified bundle version
"""
if version_number == 0:
return []
version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number))
version_info = api_request('get', version_url)
version_info = get_bundle_version(bundle_uuid, version_number)
return [BundleFile(path=path, **file_metadata) for path, file_metadata in version_info["snapshot"]["files"].items()]
@@ -265,8 +274,7 @@ def get_bundle_version_links(bundle_uuid, version_number):
"""
if version_number == 0:
return {}
version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number))
version_info = api_request('get', version_url)
version_info = get_bundle_version(bundle_uuid, version_number)
return {
name: LinkDetails(
name=name,