Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [AXM-749] Implement s3 storage supporting #2581

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lms/djangoapps/mobile_api/course_info/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ def _extend_block_info_with_offline_data(blocks_info_data: Dict[str, Dict]) -> N
block_info.update({
'offline_download': {
'file_url': file_url,
'last_modified': default_storage.get_created_time(offline_content_path),
'last_modified': default_storage.get_modified_time(offline_content_path),
'file_size': default_storage.size(offline_content_path)
}
})
28 changes: 8 additions & 20 deletions openedx/features/offline_mode/assets_management.py

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually asking to make Assets manager as class, and not just utils.py
Maybe it's worth to combine the class you've created and some functions from this file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to push the latest changes. Based on this, it will be possible to decide what can be refactored

Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
"""
This module contains utility functions for managing assets and files.
"""
import shutil

import logging
import os
import requests

from botocore.exceptions import ClientError
from django.conf import settings
from django.core.files.storage import default_storage

Expand Down Expand Up @@ -78,35 +79,22 @@ def create_subdirectories_for_asset(file_path):
os.mkdir(out_dir_name)


def remove_old_files(xblock):
def clean_outdated_xblock_files(xblock):
"""
Removes the 'assets' directory, 'index.html', and 'offline_content.zip' files
in the specified base path directory.
Removes the old zip file with Offline Content from media storage.
Args:
(XBlock): The XBlock instance
"""
try:
base_path = block_storage_path(xblock)
assets_path = os.path.join(base_path, 'assets')
index_file_path = os.path.join(base_path, 'index.html')
offline_zip_path = os.path.join(base_path, f'{xblock.location.block_id}.zip')

# Delete the 'assets' directory if it exists
if os.path.isdir(assets_path):
shutil.rmtree(assets_path)
log.info(f"Successfully deleted the directory: {assets_path}")

# Delete the 'index.html' file if it exists
if default_storage.exists(index_file_path):
os.remove(index_file_path)
log.info(f"Successfully deleted the file: {index_file_path}")

# Delete the 'offline_content.zip' file if it exists
if default_storage.exists(offline_zip_path):
os.remove(offline_zip_path)
default_storage.delete(offline_zip_path)
log.info(f"Successfully deleted the file: {offline_zip_path}")

except OSError as e:
except ClientError as e:
log.error(f"Error occurred while deleting the files or directory: {e}")


Expand Down Expand Up @@ -152,8 +140,8 @@ def is_modified(xblock):
file_path = os.path.join(block_storage_path(xblock), f'{xblock.location.block_id}.zip')

try:
last_modified = default_storage.get_created_time(file_path)
except OSError:
last_modified = default_storage.get_modified_time(file_path)
except (OSError, ClientError):
return True

return xblock.published_on > last_modified
Expand Down
125 changes: 125 additions & 0 deletions openedx/features/offline_mode/storage_management.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"""
Utility functions and classes for offline mode.
"""
import os
import logging
import shutil
from tempfile import mkdtemp

from django.contrib.auth import get_user_model
from django.core.files.base import ContentFile
from django.http.response import Http404

from openedx.core.storage import get_storage
from zipfile import ZipFile

from .assets_management import block_storage_path, clean_outdated_xblock_files, is_modified
from .html_manipulator import HtmlManipulator

User = get_user_model()
log = logging.getLogger(__name__)


class OfflineContentGenerator:
"""
Creates zip file with Offline Content in the media storage.
"""

def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None):
"""
Creates `SaveOfflineContentToStorage` object.

Args:
xblock (XBlock): The XBlock instance
html_data (str): The rendered HTML representation of the XBlock
storage_class: Used media storage class.
storage_kwargs (dict): Additional storage attributes.
"""
if storage_kwargs is None:
storage_kwargs = {}

self.xblock = xblock
self.html_data = html_data
self.storage = get_storage(storage_class, **storage_kwargs)

def generate_offline_content(self):
"""
Generates archive with XBlock content for offline mode.
"""
if not is_modified(self.xblock):
return

base_path = block_storage_path(self.xblock)
clean_outdated_xblock_files(self.xblock)
tmp_dir = mkdtemp()

try:
self.save_xblock_html(tmp_dir)
self.create_zip_file(tmp_dir, base_path, f'{self.xblock.location.block_id}.zip')
except Http404:
log.error(
f'Block {self.xblock.location.block_id} cannot be fetched from course'
f' {self.xblock.location.course_key} during offline content generation.'
)
finally:
shutil.rmtree(tmp_dir, ignore_errors=True)

def save_xblock_html(self, tmp_dir):
"""
Saves the XBlock HTML content to a file.

Generates the 'index.html' file with the HTML added to use it locally.

Args:
tmp_dir (str): The temporary directory path to save the xblock content
"""
html_manipulator = HtmlManipulator(self.xblock, self.html_data, tmp_dir)
updated_html = html_manipulator.process_html()

with open(os.path.join(tmp_dir, 'index.html'), 'w') as file:
file.write(updated_html)

def create_zip_file(self, temp_dir, base_path, file_name):
"""
Creates a zip file with the Offline Content in the media storage.

Args:
temp_dir (str): The temporary directory path where the content is stored
base_path (str): The base path directory to save the zip file
file_name (str): The name of the zip file
"""
file_path = os.path.join(temp_dir, file_name)

with ZipFile(file_path, 'w') as zip_file:
zip_file.write(os.path.join(temp_dir, 'index.html'), 'index.html')
self.add_files_to_zip_recursively(
zip_file,
current_base_path=os.path.join(temp_dir, 'assets'),
current_path_in_zip='assets',
)
with open(file_path, 'rb') as buffered_zip:
content_file = ContentFile(buffered_zip.read())
self.storage.save(base_path + file_name, content_file)

log.info(f'Offline content for {file_name} has been generated.')

def add_files_to_zip_recursively(self, zip_file, current_base_path, current_path_in_zip):
"""
Recursively adds files to the zip file.

Args:
zip_file (ZipFile): The zip file object
current_base_path (str): The current base path directory
current_path_in_zip (str): The current path in the zip file
"""
try:
for resource_path in os.listdir(current_base_path):
full_path = os.path.join(current_base_path, resource_path)
full_path_in_zip = os.path.join(current_path_in_zip, resource_path)
if os.path.isfile(full_path):
zip_file.write(full_path, full_path_in_zip)
else:
self.add_files_to_zip_recursively(zip_file, full_path, full_path_in_zip)
except OSError:
log.error(f'Error while reading the directory: {current_base_path}')
return
4 changes: 2 additions & 2 deletions openedx/features/offline_mode/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT
from .renderer import XBlockRenderer
from .utils import generate_offline_content
from .storage_management import OfflineContentGenerator


@shared_task
Expand Down Expand Up @@ -42,4 +42,4 @@ def generate_offline_content_for_block(block_id, html_data):
"""
block_usage_key = UsageKey.from_string(block_id)
xblock = modulestore().get_item(block_usage_key)
generate_offline_content(xblock, html_data)
OfflineContentGenerator(xblock, html_data).generate_offline_content()
108 changes: 0 additions & 108 deletions openedx/features/offline_mode/utils.py

This file was deleted.

Loading