Skip to content

Commit

Permalink
feat: API to get taxonomy import plan without running the import (#130)
Browse files Browse the repository at this point in the history
  • Loading branch information
rpenido authored Dec 19, 2023
1 parent 3e2d6a4 commit 1d9c459
Show file tree
Hide file tree
Showing 9 changed files with 310 additions and 59 deletions.
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.4.0"
__version__ = "0.4.1"
22 changes: 14 additions & 8 deletions openedx_tagging/core/tagging/import_export/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def import_tags(
file: BinaryIO,
parser_format: ParserFormat,
replace=False,
) -> bool:
plan_only=False,
) -> tuple[bool, TagImportTask, TagImportPlan | None]:
"""
Execute the necessary actions to import the tags from `file`
Expand All @@ -73,6 +74,8 @@ def import_tags(
Ex. Given a taxonomy with `tag_1`, `tag_2` and `tag_3`. If there is only `tag_1`
in the file (regardless of action), then `tag_2` and `tag_3` will be deleted
if `replace=True`
Set `plan_only` to True to only generate the actions and not execute them.
"""
_import_validations(taxonomy)

Expand All @@ -97,7 +100,7 @@ def import_tags(
# Check if there are errors in the parse
if errors:
task.handle_parser_errors(errors)
return False
return False, task, None

task.log_parser_end()

Expand All @@ -109,16 +112,19 @@ def import_tags(

if tag_import_plan.errors:
task.handle_plan_errors()
return False
return False, task, tag_import_plan

if not plan_only:
task.log_start_execute()
tag_import_plan.execute(task)

task.log_start_execute()
tag_import_plan.execute(task)
task.end_success()
return True
except Exception as exception:

return True, task, tag_import_plan
except Exception as exception: # pylint: disable=broad-exception-caught
# Log any exception
task.log_exception(exception)
return False
return False, task, None


def get_last_import_status(taxonomy: Taxonomy) -> TagImportTaskState:
Expand Down
5 changes: 4 additions & 1 deletion openedx_tagging/core/tagging/import_export/tasks.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
"""
Import and export celery tasks
"""
from __future__ import annotations

from io import BytesIO

from celery import shared_task # type: ignore[import]

import openedx_tagging.core.tagging.import_export.api as import_export_api

from ..models import Taxonomy
from .import_plan import TagImportPlan, TagImportTask
from .parsers import ParserFormat


Expand All @@ -17,7 +20,7 @@ def import_tags_task(
file: BytesIO,
parser_format: ParserFormat,
replace=False,
) -> bool:
) -> tuple[bool, TagImportTask, TagImportPlan | None]:
"""
Runs import on a celery task
"""
Expand Down
41 changes: 40 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from openedx_tagging.core.tagging.data import TagData
from openedx_tagging.core.tagging.import_export.parsers import ParserFormat
from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy
from openedx_tagging.core.tagging.models import ObjectTag, Tag, TagImportTask, Taxonomy


class TaxonomyListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method
Expand Down Expand Up @@ -257,3 +257,42 @@ class TaxonomyImportNewBodySerializer(TaxonomyImportBodySerializer): # pylint:
"""
taxonomy_name = serializers.CharField(required=True)
taxonomy_description = serializers.CharField(default="")


class TagImportTaskSerializer(serializers.ModelSerializer):
"""
Serializer for the TagImportTask model.
"""
class Meta:
model = TagImportTask
fields = [
"id",
"log",
"status",
"creation_date",
]


class TaxonomyImportPlanResponseSerializer(serializers.Serializer):
"""
Serializer for the response of the Taxonomy Import Plan request
"""
task = TagImportTaskSerializer()
plan = serializers.SerializerMethodField()
error = serializers.CharField(required=False, allow_null=True)

def get_plan(self, obj):
"""
Returns the plan of the import
"""
plan = obj.get("plan", None)
if plan:
return plan.plan()

return None

def update(self, instance, validated_data):
raise RuntimeError('`update()` is not supported by the TagImportTask serializer.')

def create(self, validated_data):
raise RuntimeError('`create()` is not supported by the TagImportTask serializer.')
50 changes: 43 additions & 7 deletions openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
update_tag_in_taxonomy,
)
from ...data import TagDataQuerySet
from ...import_export.api import export_tags, get_last_import_log, import_tags
from ...import_export.api import export_tags, import_tags
from ...import_export.parsers import ParserFormat
from ...models import Taxonomy
from ...rules import ObjectTagPermissionItem
Expand All @@ -42,6 +42,7 @@
TaxonomyExportQueryParamsSerializer,
TaxonomyImportBodySerializer,
TaxonomyImportNewBodySerializer,
TaxonomyImportPlanResponseSerializer,
TaxonomyListQueryParamsSerializer,
TaxonomySerializer,
TaxonomyTagCreateBodySerializer,
Expand Down Expand Up @@ -192,6 +193,17 @@ class TaxonomyView(ModelViewSet):
* 200 - Success
* 400 - Bad request
* 403 - Permission denied
**Plan Import/Update Taxonomy Example Requests**
PUT /tagging/rest_api/v1/taxonomy/:pk/tags/import/plan
{
"file": <file>,
}
**Plan Import/Update Taxonomy Query Returns**
* 200 - Success
* 400 - Bad request
* 403 - Permission denied
"""

# System taxonomies use negative numbers for their primary keys
Expand Down Expand Up @@ -279,15 +291,14 @@ def create_import(self, request: Request, **_kwargs) -> Response:

taxonomy = create_taxonomy(taxonomy_name, taxonomy_description)
try:
import_success = import_tags(taxonomy, file, parser_format)
import_success, task, _plan = import_tags(taxonomy, file, parser_format)

if import_success:
serializer = self.get_serializer(taxonomy)
return Response(serializer.data, status=status.HTTP_201_CREATED)
else:
import_error = get_last_import_log(taxonomy)
taxonomy.delete()
return Response(import_error, status=status.HTTP_400_BAD_REQUEST)
return Response(task.log, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
return Response(str(e), status=status.HTTP_400_BAD_REQUEST)

Expand All @@ -305,17 +316,42 @@ def update_import(self, request: Request, **_kwargs) -> Response:

taxonomy = self.get_object()
try:
import_success = import_tags(taxonomy, file, parser_format, replace=True)
import_success, task, _plan = import_tags(taxonomy, file, parser_format, replace=True)

if import_success:
serializer = self.get_serializer(taxonomy)
return Response(serializer.data)
else:
import_error = get_last_import_log(taxonomy)
return Response(import_error, status=status.HTTP_400_BAD_REQUEST)
return Response(task.log, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
return Response(str(e), status=status.HTTP_400_BAD_REQUEST)

@action(detail=True, url_path="tags/import/plan", methods=["put"])
def plan_update_import(self, request: Request, **_kwargs) -> Response:
"""
Plan import tags from the uploaded file to an already created taxonomy,
overwriting any existing tags.
"""
body = TaxonomyImportBodySerializer(data=request.data)
body.is_valid(raise_exception=True)

file = body.validated_data["file"].file
parser_format = body.validated_data["parser_format"]

taxonomy = self.get_object()
try:
import_success, task, plan = import_tags(taxonomy, file, parser_format, replace=True, plan_only=True)

if import_success:
serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan})
return Response(serializer.data)
else:
serializer = TaxonomyImportPlanResponseSerializer({"task": task, "plan": plan, "error": task.log})
return Response(serializer.data, status=status.HTTP_400_BAD_REQUEST)
except ValueError as e:
serializer = TaxonomyImportPlanResponseSerializer({"error": str(e)})
return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST)


@view_auth_classes
class ObjectTagView(
Expand Down
35 changes: 26 additions & 9 deletions tests/openedx_tagging/core/tagging/import_export/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,53 +91,65 @@ def test_import_export_validations(self) -> None:

def test_with_python_error(self) -> None:
self.file.close()
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "ValueError('I/O operation on closed file.')" in log

def test_with_parser_error(self) -> None:
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_parser_file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "Starting to load data from file" in log
assert "Invalid '.json' format" in log

def test_with_plan_errors(self) -> None:
assert not import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_plan_file,
self.parser_format,
)
assert not result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.ERROR
assert log == task.log
assert "Starting to load data from file" in log
assert "Load data finished" in log
assert "Starting plan actions" in log
assert "Plan finished" in log
assert "Conflict with 'create'" in log

def test_valid(self) -> None:
assert import_export_api.import_tags(
result, task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
replace=True,
)
assert result
status = import_export_api.get_last_import_status(self.taxonomy)
log = import_export_api.get_last_import_log(self.taxonomy)
assert status == TagImportTaskState(task.status)
assert status == TagImportTaskState.SUCCESS
assert log == task.log
assert "Starting to load data from file" in log
assert "Load data finished" in log
assert "Starting plan actions" in log
Expand All @@ -146,33 +158,37 @@ def test_valid(self) -> None:
assert "Execution finished" in log

def test_start_task_after_error(self) -> None:
assert not import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.invalid_parser_file,
self.parser_format,
)
assert import_export_api.import_tags(
assert not result
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

def test_start_task_after_success(self) -> None:
assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

# Opening again the file
json_data = {"tags": self.tags}
self.file = BytesIO(json.dumps(json_data).encode())

assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
self.taxonomy,
self.file,
self.parser_format,
)
assert result

def test_import_with_export_output(self) -> None:
for parser_format in ParserFormat:
Expand All @@ -183,11 +199,12 @@ def test_import_with_export_output(self) -> None:
file = BytesIO(output.encode())
new_taxonomy = Taxonomy(name="New taxonomy")
new_taxonomy.save()
assert import_export_api.import_tags(
result, _task, _plan = import_export_api.import_tags(
new_taxonomy,
file,
parser_format,
)
assert result
old_tags = self.taxonomy.tag_set.all()
assert len(old_tags) == new_taxonomy.tag_set.count()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ def test_import_tags_task(self):
replace = True

with patch('openedx_tagging.core.tagging.import_export.api.import_tags') as mock_import_tags:
mock_import_tags.return_value = True
mock_import_tags.return_value = (True, None, None)

result = import_export_tasks.import_tags_task(self.taxonomy, file, parser_format, replace)
result, _result_task, _result_plan = import_export_tasks.import_tags_task(
self.taxonomy, file, parser_format, replace
)

self.assertTrue(result)
mock_import_tags.assert_called_once_with(self.taxonomy, file, parser_format, replace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,13 @@ def open_template_file(self, template_file):
@ddt.unpack
def test_import_template(self, template_file, parser_format):
with self.open_template_file(template_file) as import_file:
assert import_api.import_tags(
result, _task, _plan = import_api.import_tags(
self.taxonomy,
import_file,
parser_format,
replace=True,
), import_api.get_last_import_log(self.taxonomy)
)
assert result, import_api.get_last_import_log(self.taxonomy)

assert pretty_format_tags(get_tags(self.taxonomy), external_id=True) == [
'Electronic instruments (ELECTRIC) (None) (children: 2)',
Expand Down
Loading

0 comments on commit 1d9c459

Please sign in to comment.