From c5f92d257998a80041ab41f3dde543a0812177ff Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 2 Sep 2021 14:48:20 -0400 Subject: [PATCH 01/14] Create model for async AQL tasks --- multinet/api/models/__init__.py | 3 ++- multinet/api/models/tasks.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/multinet/api/models/__init__.py b/multinet/api/models/__init__.py index 0be2957..8b66646 100644 --- a/multinet/api/models/__init__.py +++ b/multinet/api/models/__init__.py @@ -1,9 +1,10 @@ from .network import Network from .table import Table -from .tasks import Task, Upload +from .tasks import Task, Upload, AqlQuery from .workspace import Workspace, WorkspaceRole, WorkspaceRoleChoice __all__ = [ + 'AqlQuery', 'Network', 'Table', 'Task', diff --git a/multinet/api/models/tasks.py b/multinet/api/models/tasks.py index 0ac52d0..372cd8c 100644 --- a/multinet/api/models/tasks.py +++ b/multinet/api/models/tasks.py @@ -38,3 +38,10 @@ class DataType(models.TextChoices): blob = S3FileField() data_type = models.CharField(max_length=20, choices=DataType.choices) + + +class AqlQuery(Task): + """An obhect to track AQL queries.""" + + query = models.CharField() + query_results = models.JSONField() From 4757b6b6db8a94afd2c89ceb134835f2eccf561a Mon Sep 17 00:00:00 2001 From: naglepuff Date: Thu, 2 Sep 2021 14:50:28 -0400 Subject: [PATCH 02/14] Create celery task for AQL queries --- multinet/api/tasks/aql/__init__.py | 0 multinet/api/tasks/aql/aql_query.py | 33 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 multinet/api/tasks/aql/__init__.py create mode 100644 multinet/api/tasks/aql/aql_query.py diff --git a/multinet/api/tasks/aql/__init__.py b/multinet/api/tasks/aql/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py new file mode 100644 index 0000000..8a94c60 --- /dev/null +++ b/multinet/api/tasks/aql/aql_query.py @@ -0,0 +1,33 @@ +import json + +from arango.exceptions import AQLQueryExecuteError, ArangoServerError +from multinet.api.utils.arango import ArangoQuery +from arango.cursor import Cursor +from celery import shared_task + +from multinet.api.models import AqlQuery, Workspace +from multinet.api.tasks import MultinetCeleryTask + + +class ExecuteAqlQueryTask(MultinetCeleryTask): + task_model = AqlQuery + + +@shared_task(base=ExecuteAqlQueryTask) +def execute_query(task_id: int) -> None: + query_task: AqlQuery = AqlQuery.objects.select_related('workspace').get(id=task_id) + workspace: Workspace = query_task.workspace + query_str = query_task.query + + try: + # Run the query on Arango DB + database = workspace.get_arango_db() + query = ArangoQuery(database, query_str) + cursor: Cursor = query.execute() + + # Store the results on the task object + jsonResults = json.dumps(list(cursor)) + query_task.query_results = jsonResults + query_task.save() + except (AQLQueryExecuteError, ArangoServerError) as err: + ExecuteAqlQueryTask.fail_task_with_message(query_task, err.error_message) From e4a0229097e2a61d37ae860e3e744b789798c6ae Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 6 Sep 2021 15:52:46 -0400 Subject: [PATCH 03/14] Use TextField for AQL queries Also create migration for the new AqlQuery model. --- multinet/api/migrations/0010_aqlquery.py | 85 ++++++++++++++++++++++++ multinet/api/models/tasks.py | 2 +- 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 multinet/api/migrations/0010_aqlquery.py diff --git a/multinet/api/migrations/0010_aqlquery.py b/multinet/api/migrations/0010_aqlquery.py new file mode 100644 index 0000000..9e2cf6c --- /dev/null +++ b/multinet/api/migrations/0010_aqlquery.py @@ -0,0 +1,85 @@ +# Generated by Django 3.2.6 on 2021-09-06 19:46 + +from django.conf import settings +import django.contrib.postgres.fields +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('api', '0009_alter_upload_options'), + ] + + operations = [ + migrations.CreateModel( + name='AqlQuery', + fields=[ + ( + 'id', + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name='ID' + ), + ), + ( + 'created', + django_extensions.db.fields.CreationDateTimeField( + auto_now_add=True, verbose_name='created' + ), + ), + ( + 'modified', + django_extensions.db.fields.ModificationDateTimeField( + auto_now=True, verbose_name='modified' + ), + ), + ( + 'error_messages', + django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=500), + blank=True, + null=True, + size=None, + ), + ), + ( + 'status', + models.CharField( + choices=[ + ('PENDING', 'Pending'), + ('STARTED', 'Started'), + ('FAILED', 'Failed'), + ('FINISHED', 'Finished'), + ], + default='PENDING', + max_length=10, + ), + ), + ('query', models.TextField()), + ('query_results', models.JSONField()), + ( + 'user', + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='aqlquerys', + to=settings.AUTH_USER_MODEL, + ), + ), + ( + 'workspace', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='aqlquerys', + to='api.workspace', + ), + ), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/multinet/api/models/tasks.py b/multinet/api/models/tasks.py index 372cd8c..da9a84b 100644 --- a/multinet/api/models/tasks.py +++ b/multinet/api/models/tasks.py @@ -43,5 +43,5 @@ class DataType(models.TextChoices): class AqlQuery(Task): """An obhect to track AQL queries.""" - query = models.CharField() + query = models.TextField() query_results = models.JSONField() From 8237df6585fd8819a3969935e70d63a4b3cd867b Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 6 Sep 2021 16:44:11 -0400 Subject: [PATCH 04/14] Create ViewSet for AQL Queries Also fix some linting issues. --- multinet/api/models/__init__.py | 2 +- multinet/api/tasks/aql/__init__.py | 3 +++ multinet/api/tasks/aql/aql_query.py | 8 +++---- multinet/api/views/__init__.py | 2 ++ multinet/api/views/query.py | 36 +++++++++++++++++++++++++++++ multinet/api/views/serializers.py | 10 +++++++- multinet/urls.py | 7 ++++++ 7 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 multinet/api/views/query.py diff --git a/multinet/api/models/__init__.py b/multinet/api/models/__init__.py index 8b66646..41843be 100644 --- a/multinet/api/models/__init__.py +++ b/multinet/api/models/__init__.py @@ -1,6 +1,6 @@ from .network import Network from .table import Table -from .tasks import Task, Upload, AqlQuery +from .tasks import AqlQuery, Task, Upload from .workspace import Workspace, WorkspaceRole, WorkspaceRoleChoice __all__ = [ diff --git a/multinet/api/tasks/aql/__init__.py b/multinet/api/tasks/aql/__init__.py index e69de29..79fd0ba 100644 --- a/multinet/api/tasks/aql/__init__.py +++ b/multinet/api/tasks/aql/__init__.py @@ -0,0 +1,3 @@ +from .aql_query import ExecuteAqlQueryTask, execute_query + +__all__ = ['ExecuteAqlQueryTask', 'execute_query'] diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py index 8a94c60..ca88b5e 100644 --- a/multinet/api/tasks/aql/aql_query.py +++ b/multinet/api/tasks/aql/aql_query.py @@ -1,12 +1,12 @@ import json -from arango.exceptions import AQLQueryExecuteError, ArangoServerError -from multinet.api.utils.arango import ArangoQuery from arango.cursor import Cursor +from arango.exceptions import AQLQueryExecuteError, ArangoServerError from celery import shared_task from multinet.api.models import AqlQuery, Workspace from multinet.api.tasks import MultinetCeleryTask +from multinet.api.utils.arango import ArangoQuery class ExecuteAqlQueryTask(MultinetCeleryTask): @@ -26,8 +26,8 @@ def execute_query(task_id: int) -> None: cursor: Cursor = query.execute() # Store the results on the task object - jsonResults = json.dumps(list(cursor)) - query_task.query_results = jsonResults + json_results = json.dumps(list(cursor)) + query_task.query_results = json_results query_task.save() except (AQLQueryExecuteError, ArangoServerError) as err: ExecuteAqlQueryTask.fail_task_with_message(query_task, err.error_message) diff --git a/multinet/api/views/__init__.py b/multinet/api/views/__init__.py index 88063a2..baefc05 100644 --- a/multinet/api/views/__init__.py +++ b/multinet/api/views/__init__.py @@ -1,4 +1,5 @@ from .network import NetworkViewSet +from .query import AqlQueryViewSet from .table import TableViewSet from .upload import UploadViewSet from .users import users_me_view, users_search_view @@ -11,4 +12,5 @@ 'TableViewSet', 'UploadViewSet', 'WorkspaceViewSet', + 'AqlQueryViewSet', ] diff --git a/multinet/api/views/query.py b/multinet/api/views/query.py new file mode 100644 index 0000000..050f61f --- /dev/null +++ b/multinet/api/views/query.py @@ -0,0 +1,36 @@ +from django.shortcuts import get_object_or_404 +from drf_yasg.utils import swagger_auto_schema +from rest_framework import status +from rest_framework.permissions import IsAuthenticatedOrReadOnly +from rest_framework.response import Response +from rest_framework.viewsets import ReadOnlyModelViewSet + +from multinet.api.auth.decorators import require_workspace_permission +from multinet.api.models import AqlQuery, Workspace, WorkspaceRoleChoice +from multinet.api.tasks.aql import execute_query + +from .common import WorkspaceChildMixin +from .serializers import AqlQuerySerializer, AqlQueryTaskSerializer + + +class AqlQueryViewSet(WorkspaceChildMixin, ReadOnlyModelViewSet): + permission_classes = [IsAuthenticatedOrReadOnly] + serializer_class = AqlQueryTaskSerializer + swagger_tags = ['queries'] + + @swagger_auto_schema(request_body=AqlQuerySerializer()) + @require_workspace_permission(WorkspaceRoleChoice.READER) + def create(self, request, parent_lookup_workspace__name: str): + """Create an AQL query task.""" + workspace: Workspace = get_object_or_404(Workspace, name=parent_lookup_workspace__name) + serializer = AqlQuerySerializer(data=request.data) + serializer.is_valid(raise_exception=True) + query_str = serializer.validated_data['query'] + + query: AqlQuery = AqlQuery.objects.create( + workspace=workspace, user=request.user, query=query_str + ) + + execute_query.delay(task_id=query.pk) + + return Response(AqlQueryTaskSerializer(query).data, status=status.HTTP_200_OK) diff --git a/multinet/api/views/serializers.py b/multinet/api/views/serializers.py index e6cfdc1..e323710 100644 --- a/multinet/api/views/serializers.py +++ b/multinet/api/views/serializers.py @@ -2,7 +2,7 @@ from django.contrib.auth.validators import UnicodeUsernameValidator from rest_framework import serializers -from multinet.api.models import Network, Table, Upload, Workspace +from multinet.api.models import AqlQuery, Network, Table, Upload, Workspace from multinet.api.tasks.upload.utils import ColumnTypeEnum @@ -92,6 +92,14 @@ class AqlQuerySerializer(serializers.Serializer): query = serializers.CharField() +class AqlQueryTaskSerializer(serializers.ModelSerializer): + class Meta: + model = AqlQuery + fields = '__all__' + + workspace = WorkspaceSerializer() + + class LimitOffsetSerializer(serializers.Serializer): limit = serializers.IntegerField(required=False) offset = serializers.IntegerField(required=False) diff --git a/multinet/urls.py b/multinet/urls.py index 5de5601..36cf334 100644 --- a/multinet/urls.py +++ b/multinet/urls.py @@ -7,6 +7,7 @@ from rest_framework_extensions.routers import ExtendedSimpleRouter from multinet.api.views import ( + AqlQueryViewSet, NetworkViewSet, TableViewSet, UploadViewSet, @@ -35,6 +36,12 @@ basename='upload', parents_query_lookups=[f'workspace__{WorkspaceViewSet.lookup_field}'], ) +workspaces_routes.register( + 'queries', + AqlQueryViewSet, + basename='query', + parents_query_lookups=[f'workspace__{WorkspaceViewSet.lookup_field}'], +) # OpenAPI generation From b2598297da1d7bd61e777327f3ede84f19cb8234 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 7 Sep 2021 16:54:35 -0400 Subject: [PATCH 05/14] Change query_results to allow Null When a query task is first created, it won't have any results. The best representation for this is Null, because a placeholder or default values like an empty dictionary could imply that a query returned no results. --- .../0011_alter_aqlquery_query_results.py | 18 ++++++++++++++++++ multinet/api/models/tasks.py | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 multinet/api/migrations/0011_alter_aqlquery_query_results.py diff --git a/multinet/api/migrations/0011_alter_aqlquery_query_results.py b/multinet/api/migrations/0011_alter_aqlquery_query_results.py new file mode 100644 index 0000000..7a4ff72 --- /dev/null +++ b/multinet/api/migrations/0011_alter_aqlquery_query_results.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.6 on 2021-09-07 19:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0010_aqlquery'), + ] + + operations = [ + migrations.AlterField( + model_name='aqlquery', + name='query_results', + field=models.JSONField(blank=True, null=True), + ), + ] diff --git a/multinet/api/models/tasks.py b/multinet/api/models/tasks.py index da9a84b..e5b6f0a 100644 --- a/multinet/api/models/tasks.py +++ b/multinet/api/models/tasks.py @@ -44,4 +44,4 @@ class AqlQuery(Task): """An obhect to track AQL queries.""" query = models.TextField() - query_results = models.JSONField() + query_results = models.JSONField(blank=True, null=True) From 0cd120c53719ef615cc56e92a7c6dd1c693eb04b Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 7 Sep 2021 16:56:33 -0400 Subject: [PATCH 06/14] Specify CharField for user in aql serializer --- multinet/api/views/serializers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/multinet/api/views/serializers.py b/multinet/api/views/serializers.py index e323710..a706e86 100644 --- a/multinet/api/views/serializers.py +++ b/multinet/api/views/serializers.py @@ -99,6 +99,9 @@ class Meta: workspace = WorkspaceSerializer() + # Specify user as a CharField to return username + user = serializers.CharField() + class LimitOffsetSerializer(serializers.Serializer): limit = serializers.IntegerField(required=False) From 070c8382dc0810f3e1b68bb8078fc9b958174072 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 7 Sep 2021 17:02:36 -0400 Subject: [PATCH 07/14] Add queryset to AqlQueryViewSet --- multinet/api/views/query.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/multinet/api/views/query.py b/multinet/api/views/query.py index 050f61f..1f7a0ce 100644 --- a/multinet/api/views/query.py +++ b/multinet/api/views/query.py @@ -14,11 +14,14 @@ class AqlQueryViewSet(WorkspaceChildMixin, ReadOnlyModelViewSet): + queryset = AqlQuery.objects.all().select_related('workspace') permission_classes = [IsAuthenticatedOrReadOnly] serializer_class = AqlQueryTaskSerializer swagger_tags = ['queries'] - @swagger_auto_schema(request_body=AqlQuerySerializer()) + @swagger_auto_schema( + request_body=AqlQuerySerializer(), responses={200: AqlQueryTaskSerializer()} + ) @require_workspace_permission(WorkspaceRoleChoice.READER) def create(self, request, parent_lookup_workspace__name: str): """Create an AQL query task.""" From 229a3154c3dc4de6ad3a429adfe6897ceb109bf4 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 7 Sep 2021 17:05:01 -0400 Subject: [PATCH 08/14] Set the query_results without using json --- multinet/api/tasks/aql/aql_query.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py index ca88b5e..04226da 100644 --- a/multinet/api/tasks/aql/aql_query.py +++ b/multinet/api/tasks/aql/aql_query.py @@ -1,5 +1,3 @@ -import json - from arango.cursor import Cursor from arango.exceptions import AQLQueryExecuteError, ArangoServerError from celery import shared_task @@ -26,8 +24,7 @@ def execute_query(task_id: int) -> None: cursor: Cursor = query.execute() # Store the results on the task object - json_results = json.dumps(list(cursor)) - query_task.query_results = json_results + query_task.query_results = list(cursor) query_task.save() except (AQLQueryExecuteError, ArangoServerError) as err: ExecuteAqlQueryTask.fail_task_with_message(query_task, err.error_message) From b98c23cfb94de0a2a258b5b4814cbfe8d8c42f35 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Tue, 7 Sep 2021 17:08:03 -0400 Subject: [PATCH 09/14] Initial tests for async aql --- multinet/api/tests/test_query.py | 58 ++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 multinet/api/tests/test_query.py diff --git a/multinet/api/tests/test_query.py b/multinet/api/tests/test_query.py new file mode 100644 index 0000000..9a2301f --- /dev/null +++ b/multinet/api/tests/test_query.py @@ -0,0 +1,58 @@ +from django.contrib.auth.models import User +import pytest +from rest_framework.response import Response +from rest_framework.test import APIClient + +from multinet.api.models.tasks import AqlQuery +from multinet.api.models.workspace import Workspace, WorkspaceRole, WorkspaceRoleChoice +from multinet.api.tests.conftest import populated_table +from multinet.api.tests.fuzzy import INTEGER_ID_RE, TIMESTAMP_RE, workspace_re + + +@pytest.fixture +def simple_query(workspace: Workspace, user: User, authenticated_api_client: APIClient): + """Create a fixture representing the response of a POST request for AQL queries.""" + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + node_table = populated_table(workspace, False) + query_str = f'FOR document IN {node_table.name} RETURN document' + r: Response = authenticated_api_client.post( + f'/api/workspaces/{workspace.name}/queries/', {'query': query_str}, format='json' + ) + WorkspaceRole.objects.filter(workspace=workspace, user=user).delete() + return {'response': r, 'query': query_str, 'nodes': list(node_table.get_rows())} + + +@pytest.mark.django_db +def test_query_rest_create(workspace: Workspace, user: User, simple_query): + r = simple_query['response'] + assert r.status_code == 200 + assert r.json() == { + 'id': INTEGER_ID_RE, + 'workspace': workspace_re(workspace), + 'query': simple_query['query'], + 'user': user.username, + 'error_messages': None, + 'status': AqlQuery.Status.PENDING, + 'created': TIMESTAMP_RE, + 'modified': TIMESTAMP_RE, + 'query_results': None, + } + + +@pytest.mark.django_db +def test_query_rest_retrieve( + workspace: Workspace, user: User, authenticated_api_client: APIClient, simple_query +): + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + query_info = simple_query['response'].json() + query_id = query_info['id'] + r: Response = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/queries/{query_id}/' + ) + assert r.status_code == 200 + r_json = r.json() + results = r_json['query_results'] + expected_results = simple_query['nodes'] + assert len(results) == len(expected_results) + for row in results: + assert row in expected_results From 02e96b4e180ccd068ad591045cbc44b5a579071a Mon Sep 17 00:00:00 2001 From: naglepuff Date: Fri, 10 Sep 2021 12:37:54 -0400 Subject: [PATCH 10/14] Add tests with mutating queries Creation of tasks representing mutating queries are allowed, but no results will be stored in them. They will finish with an error message from Arango. --- multinet/api/tests/test_query.py | 97 ++++++++++++++++++++++++++++---- 1 file changed, 86 insertions(+), 11 deletions(-) diff --git a/multinet/api/tests/test_query.py b/multinet/api/tests/test_query.py index 9a2301f..eccc7bc 100644 --- a/multinet/api/tests/test_query.py +++ b/multinet/api/tests/test_query.py @@ -1,4 +1,5 @@ from django.contrib.auth.models import User +from faker import Faker import pytest from rest_framework.response import Response from rest_framework.test import APIClient @@ -10,7 +11,7 @@ @pytest.fixture -def simple_query(workspace: Workspace, user: User, authenticated_api_client: APIClient): +def valid_query(workspace: Workspace, user: User, authenticated_api_client: APIClient): """Create a fixture representing the response of a POST request for AQL queries.""" workspace.set_user_permission(user, WorkspaceRoleChoice.READER) node_table = populated_table(workspace, False) @@ -22,14 +23,28 @@ def simple_query(workspace: Workspace, user: User, authenticated_api_client: API return {'response': r, 'query': query_str, 'nodes': list(node_table.get_rows())} +@pytest.fixture +def mutating_query(workspace: Workspace, user: User, authenticated_api_client: APIClient): + """Create a fixture for a mutating AQL query that will have an error message post processing.""" + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + node_table = populated_table(workspace, False) + fake = Faker() + query_str = f"INSERT {{ 'name': {fake.pystr()} }} INTO {node_table.name}" + r: Response = authenticated_api_client.post( + f'/api/workspaces/{workspace.name}/queries/', {'query': query_str}, format='json' + ) + WorkspaceRole.objects.filter(workspace=workspace, user=user).delete() + return {'response': r, 'query': query_str, 'nodes': list(node_table.get_rows())} + + @pytest.mark.django_db -def test_query_rest_create(workspace: Workspace, user: User, simple_query): - r = simple_query['response'] +def test_query_rest_create(workspace: Workspace, user: User, valid_query): + r = valid_query['response'] assert r.status_code == 200 assert r.json() == { 'id': INTEGER_ID_RE, 'workspace': workspace_re(workspace), - 'query': simple_query['query'], + 'query': valid_query['query'], 'user': user.username, 'error_messages': None, 'status': AqlQuery.Status.PENDING, @@ -40,19 +55,79 @@ def test_query_rest_create(workspace: Workspace, user: User, simple_query): @pytest.mark.django_db +@pytest.mark.parametrize( + 'permission,is_owner,status_code,success', + [ + (None, False, 404, False), + (WorkspaceRoleChoice.READER, False, 200, True), + (WorkspaceRoleChoice.WRITER, False, 200, True), + (WorkspaceRoleChoice.MAINTAINER, False, 200, True), + (None, True, 200, True), + ], +) def test_query_rest_retrieve( - workspace: Workspace, user: User, authenticated_api_client: APIClient, simple_query + workspace: Workspace, + user: User, + authenticated_api_client: APIClient, + valid_query, + permission: WorkspaceRoleChoice, + is_owner: bool, + status_code: int, + success: bool, +): + if permission is not None: + workspace.set_user_permission(user, permission) + elif is_owner: + workspace.set_owner(user) + + query_info = valid_query['response'].json() + query_id = query_info['id'] + r: Response = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/queries/{query_id}/' + ) + assert r.status_code == status_code + if success: + r_json = r.json() + results = r_json['query_results'] + expected_results = valid_query['nodes'] + assert len(results) == len(expected_results) + assert r_json['status'] == AqlQuery.Status.FINISHED + for row in results: + assert row in expected_results + + +@pytest.mark.django_db +def test_query_rest_create_mutating(workspace: Workspace, user: User, mutating_query): + r = mutating_query['response'] + + # even though the query is not read-only, the task object should be created + assert r.status_code == 200 + assert r.json() == { + 'id': INTEGER_ID_RE, + 'workspace': workspace_re(workspace), + 'query': mutating_query['query'], + 'user': user.username, + 'error_messages': None, + 'status': AqlQuery.Status.PENDING, + 'created': TIMESTAMP_RE, + 'modified': TIMESTAMP_RE, + 'query_results': None, + } + + +@pytest.mark.django_db +def test_query_rest_retrieve_mutating( + workspace: Workspace, user: User, authenticated_api_client: APIClient, mutating_query ): workspace.set_user_permission(user, WorkspaceRoleChoice.READER) - query_info = simple_query['response'].json() + + query_info = mutating_query['response'].json() query_id = query_info['id'] r: Response = authenticated_api_client.get( f'/api/workspaces/{workspace.name}/queries/{query_id}/' ) assert r.status_code == 200 r_json = r.json() - results = r_json['query_results'] - expected_results = simple_query['nodes'] - assert len(results) == len(expected_results) - for row in results: - assert row in expected_results + assert r_json['query_results'] is None + assert len(r_json['error_messages']) > 0 + assert r_json['status'] == AqlQuery.Status.FINISHED From 65209c25ae892c32450b4894d9aa08a53dd5198a Mon Sep 17 00:00:00 2001 From: naglepuff Date: Fri, 10 Sep 2021 12:45:15 -0400 Subject: [PATCH 11/14] Increase time limit for async AQL queries --- multinet/api/tasks/aql/aql_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py index 04226da..df5d404 100644 --- a/multinet/api/tasks/aql/aql_query.py +++ b/multinet/api/tasks/aql/aql_query.py @@ -20,7 +20,7 @@ def execute_query(task_id: int) -> None: try: # Run the query on Arango DB database = workspace.get_arango_db() - query = ArangoQuery(database, query_str) + query = ArangoQuery(database, query_str, time_limit_secs=60) cursor: Cursor = query.execute() # Store the results on the task object From 110f2b2c29f7cc45ba97170d5e4f6545483d7ea5 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 13 Sep 2021 09:45:49 -0400 Subject: [PATCH 12/14] Rename query_results and squash migrations --- multinet/api/migrations/0010_aqlquery.py | 4 ++-- .../0011_alter_aqlquery_query_results.py | 18 ------------------ multinet/api/models/tasks.py | 2 +- multinet/api/tasks/aql/aql_query.py | 2 +- multinet/api/tests/test_query.py | 8 ++++---- 5 files changed, 8 insertions(+), 26 deletions(-) delete mode 100644 multinet/api/migrations/0011_alter_aqlquery_query_results.py diff --git a/multinet/api/migrations/0010_aqlquery.py b/multinet/api/migrations/0010_aqlquery.py index 9e2cf6c..220eee2 100644 --- a/multinet/api/migrations/0010_aqlquery.py +++ b/multinet/api/migrations/0010_aqlquery.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.6 on 2021-09-06 19:46 +# Generated by Django 3.2.6 on 2021-09-13 13:31 from django.conf import settings import django.contrib.postgres.fields @@ -59,7 +59,7 @@ class Migration(migrations.Migration): ), ), ('query', models.TextField()), - ('query_results', models.JSONField()), + ('results', models.JSONField(blank=True, null=True)), ( 'user', models.ForeignKey( diff --git a/multinet/api/migrations/0011_alter_aqlquery_query_results.py b/multinet/api/migrations/0011_alter_aqlquery_query_results.py deleted file mode 100644 index 7a4ff72..0000000 --- a/multinet/api/migrations/0011_alter_aqlquery_query_results.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 3.2.6 on 2021-09-07 19:29 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0010_aqlquery'), - ] - - operations = [ - migrations.AlterField( - model_name='aqlquery', - name='query_results', - field=models.JSONField(blank=True, null=True), - ), - ] diff --git a/multinet/api/models/tasks.py b/multinet/api/models/tasks.py index e5b6f0a..886ef8b 100644 --- a/multinet/api/models/tasks.py +++ b/multinet/api/models/tasks.py @@ -44,4 +44,4 @@ class AqlQuery(Task): """An obhect to track AQL queries.""" query = models.TextField() - query_results = models.JSONField(blank=True, null=True) + results = models.JSONField(blank=True, null=True) diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py index df5d404..3469256 100644 --- a/multinet/api/tasks/aql/aql_query.py +++ b/multinet/api/tasks/aql/aql_query.py @@ -24,7 +24,7 @@ def execute_query(task_id: int) -> None: cursor: Cursor = query.execute() # Store the results on the task object - query_task.query_results = list(cursor) + query_task.results = list(cursor) query_task.save() except (AQLQueryExecuteError, ArangoServerError) as err: ExecuteAqlQueryTask.fail_task_with_message(query_task, err.error_message) diff --git a/multinet/api/tests/test_query.py b/multinet/api/tests/test_query.py index eccc7bc..9d200ef 100644 --- a/multinet/api/tests/test_query.py +++ b/multinet/api/tests/test_query.py @@ -50,7 +50,7 @@ def test_query_rest_create(workspace: Workspace, user: User, valid_query): 'status': AqlQuery.Status.PENDING, 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, - 'query_results': None, + 'results': None, } @@ -88,7 +88,7 @@ def test_query_rest_retrieve( assert r.status_code == status_code if success: r_json = r.json() - results = r_json['query_results'] + results = r_json['results'] expected_results = valid_query['nodes'] assert len(results) == len(expected_results) assert r_json['status'] == AqlQuery.Status.FINISHED @@ -111,7 +111,7 @@ def test_query_rest_create_mutating(workspace: Workspace, user: User, mutating_q 'status': AqlQuery.Status.PENDING, 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, - 'query_results': None, + 'results': None, } @@ -128,6 +128,6 @@ def test_query_rest_retrieve_mutating( ) assert r.status_code == 200 r_json = r.json() - assert r_json['query_results'] is None + assert r_json['results'] is None assert len(r_json['error_messages']) > 0 assert r_json['status'] == AqlQuery.Status.FINISHED From 0d40fd19640c66f241f252cde47ef3d7b1628d17 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 13 Sep 2021 10:27:32 -0400 Subject: [PATCH 13/14] Rename ExecuteAqlQueryTask Also remove custom exception handling from that task, and instead leverage MultinetCeleryTask to handle any exceptions during task execution. --- multinet/api/models/tasks.py | 2 +- multinet/api/tasks/aql/__init__.py | 4 ++-- multinet/api/tasks/aql/aql_query.py | 25 +++++++++++-------------- multinet/api/tests/test_query.py | 2 +- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/multinet/api/models/tasks.py b/multinet/api/models/tasks.py index 886ef8b..7ed6f6f 100644 --- a/multinet/api/models/tasks.py +++ b/multinet/api/models/tasks.py @@ -41,7 +41,7 @@ class DataType(models.TextChoices): class AqlQuery(Task): - """An obhect to track AQL queries.""" + """An object to track AQL queries.""" query = models.TextField() results = models.JSONField(blank=True, null=True) diff --git a/multinet/api/tasks/aql/__init__.py b/multinet/api/tasks/aql/__init__.py index 79fd0ba..2fd9c7c 100644 --- a/multinet/api/tasks/aql/__init__.py +++ b/multinet/api/tasks/aql/__init__.py @@ -1,3 +1,3 @@ -from .aql_query import ExecuteAqlQueryTask, execute_query +from .aql_query import AqlQueryTask, execute_query -__all__ = ['ExecuteAqlQueryTask', 'execute_query'] +__all__ = ['AqlQueryTask', 'execute_query'] diff --git a/multinet/api/tasks/aql/aql_query.py b/multinet/api/tasks/aql/aql_query.py index 3469256..9c12cbe 100644 --- a/multinet/api/tasks/aql/aql_query.py +++ b/multinet/api/tasks/aql/aql_query.py @@ -1,5 +1,6 @@ from arango.cursor import Cursor -from arango.exceptions import AQLQueryExecuteError, ArangoServerError + +# from arango.exceptions import AQLQueryExecuteError, ArangoServerError from celery import shared_task from multinet.api.models import AqlQuery, Workspace @@ -7,24 +8,20 @@ from multinet.api.utils.arango import ArangoQuery -class ExecuteAqlQueryTask(MultinetCeleryTask): +class AqlQueryTask(MultinetCeleryTask): task_model = AqlQuery -@shared_task(base=ExecuteAqlQueryTask) +@shared_task(base=AqlQueryTask) def execute_query(task_id: int) -> None: query_task: AqlQuery = AqlQuery.objects.select_related('workspace').get(id=task_id) workspace: Workspace = query_task.workspace query_str = query_task.query + # Run the query on Arango DB + database = workspace.get_arango_db() + query = ArangoQuery(database, query_str, time_limit_secs=60) + cursor: Cursor = query.execute() - try: - # Run the query on Arango DB - database = workspace.get_arango_db() - query = ArangoQuery(database, query_str, time_limit_secs=60) - cursor: Cursor = query.execute() - - # Store the results on the task object - query_task.results = list(cursor) - query_task.save() - except (AQLQueryExecuteError, ArangoServerError) as err: - ExecuteAqlQueryTask.fail_task_with_message(query_task, err.error_message) + # Store the results on the task object + query_task.results = list(cursor) + query_task.save() diff --git a/multinet/api/tests/test_query.py b/multinet/api/tests/test_query.py index 9d200ef..bb240a9 100644 --- a/multinet/api/tests/test_query.py +++ b/multinet/api/tests/test_query.py @@ -130,4 +130,4 @@ def test_query_rest_retrieve_mutating( r_json = r.json() assert r_json['results'] is None assert len(r_json['error_messages']) > 0 - assert r_json['status'] == AqlQuery.Status.FINISHED + assert r_json['status'] == AqlQuery.Status.FAILED From 1b61e620421d2dd9b24d8ba88ae116ce231fd524 Mon Sep 17 00:00:00 2001 From: naglepuff Date: Mon, 13 Sep 2021 12:16:09 -0400 Subject: [PATCH 14/14] Create /results endpoint for AQL queries --- multinet/api/tests/test_query.py | 103 ++++++++++++++++++++++-------- multinet/api/views/query.py | 21 +++++- multinet/api/views/serializers.py | 11 +++- 3 files changed, 107 insertions(+), 28 deletions(-) diff --git a/multinet/api/tests/test_query.py b/multinet/api/tests/test_query.py index bb240a9..c5f4995 100644 --- a/multinet/api/tests/test_query.py +++ b/multinet/api/tests/test_query.py @@ -50,7 +50,24 @@ def test_query_rest_create(workspace: Workspace, user: User, valid_query): 'status': AqlQuery.Status.PENDING, 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, - 'results': None, + } + + +@pytest.mark.django_db +def test_query_rest_create_mutating(workspace: Workspace, user: User, mutating_query): + r = mutating_query['response'] + + # even though the query is not read-only, the task object should be created + assert r.status_code == 200 + assert r.json() == { + 'id': INTEGER_ID_RE, + 'workspace': workspace_re(workspace), + 'query': mutating_query['query'], + 'user': user.username, + 'error_messages': None, + 'status': AqlQuery.Status.PENDING, + 'created': TIMESTAMP_RE, + 'modified': TIMESTAMP_RE, } @@ -88,31 +105,7 @@ def test_query_rest_retrieve( assert r.status_code == status_code if success: r_json = r.json() - results = r_json['results'] - expected_results = valid_query['nodes'] - assert len(results) == len(expected_results) assert r_json['status'] == AqlQuery.Status.FINISHED - for row in results: - assert row in expected_results - - -@pytest.mark.django_db -def test_query_rest_create_mutating(workspace: Workspace, user: User, mutating_query): - r = mutating_query['response'] - - # even though the query is not read-only, the task object should be created - assert r.status_code == 200 - assert r.json() == { - 'id': INTEGER_ID_RE, - 'workspace': workspace_re(workspace), - 'query': mutating_query['query'], - 'user': user.username, - 'error_messages': None, - 'status': AqlQuery.Status.PENDING, - 'created': TIMESTAMP_RE, - 'modified': TIMESTAMP_RE, - 'results': None, - } @pytest.mark.django_db @@ -128,6 +121,64 @@ def test_query_rest_retrieve_mutating( ) assert r.status_code == 200 r_json = r.json() - assert r_json['results'] is None assert len(r_json['error_messages']) > 0 assert r_json['status'] == AqlQuery.Status.FAILED + + +@pytest.mark.django_db +@pytest.mark.parametrize( + 'permission,is_owner,status_code,success', + [ + (None, False, 404, False), + (WorkspaceRoleChoice.READER, False, 200, True), + (WorkspaceRoleChoice.WRITER, False, 200, True), + (WorkspaceRoleChoice.MAINTAINER, False, 200, True), + (None, True, 200, True), + ], +) +def test_query_rest_retrieve_results( + workspace: Workspace, + user: User, + authenticated_api_client: APIClient, + valid_query, + permission: WorkspaceRoleChoice, + is_owner: bool, + status_code: int, + success: bool, +): + if permission is not None: + workspace.set_user_permission(user, permission) + elif is_owner: + workspace.set_owner(user) + + query_info = valid_query['response'].json() + query_id = query_info['id'] + r: Response = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/queries/{query_id}/results/' + ) + assert r.status_code == status_code + if success: + r_json = r.json() + assert r_json['id'] == query_id + assert r_json['workspace'] == str(workspace) + assert r_json['user'] == str(user) + + results = r_json['results'] + expected_results = valid_query['nodes'] + assert len(results) == len(expected_results) + for row in results: + assert row in expected_results + + +@pytest.mark.django_db +def test_query_rest_retrieve_results_mutating( + workspace: Workspace, user: User, authenticated_api_client: APIClient, mutating_query +): + workspace.set_user_permission(user, WorkspaceRoleChoice.READER) + query_info = mutating_query['response'].json() + query_id = query_info['id'] + r: Response = authenticated_api_client.get( + f'/api/workspaces/{workspace.name}/queries/{query_id}/results/' + ) + assert r.status_code == 400 + assert r.data == 'The given query could not be executed, and has no results' diff --git a/multinet/api/views/query.py b/multinet/api/views/query.py index 1f7a0ce..37264fb 100644 --- a/multinet/api/views/query.py +++ b/multinet/api/views/query.py @@ -1,6 +1,7 @@ from django.shortcuts import get_object_or_404 from drf_yasg.utils import swagger_auto_schema from rest_framework import status +from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.response import Response from rest_framework.viewsets import ReadOnlyModelViewSet @@ -10,7 +11,7 @@ from multinet.api.tasks.aql import execute_query from .common import WorkspaceChildMixin -from .serializers import AqlQuerySerializer, AqlQueryTaskSerializer +from .serializers import AqlQueryResultsSerializer, AqlQuerySerializer, AqlQueryTaskSerializer class AqlQueryViewSet(WorkspaceChildMixin, ReadOnlyModelViewSet): @@ -37,3 +38,21 @@ def create(self, request, parent_lookup_workspace__name: str): execute_query.delay(task_id=query.pk) return Response(AqlQueryTaskSerializer(query).data, status=status.HTTP_200_OK) + + @swagger_auto_schema(responses={200: AqlQueryResultsSerializer()}) + @action(detail=True, url_path='results') + @require_workspace_permission(WorkspaceRoleChoice.READER) + def results(self, request, parent_lookup_workspace__name: str, pk): + workspace: Workspace = get_object_or_404(Workspace, name=parent_lookup_workspace__name) + aql_task: AqlQuery = get_object_or_404(AqlQuery, workspace=workspace, pk=pk) + if aql_task.status == AqlQuery.Status.FINISHED: + return Response(AqlQueryResultsSerializer(aql_task).data, status=status.HTTP_200_OK) + elif aql_task.status in [AqlQuery.Status.STARTED, AqlQuery.Status.PENDING]: + return Response( + 'The given query has not finished executing', status=status.HTTP_400_BAD_REQUEST + ) + elif aql_task.status == AqlQuery.Status.FAILED: + return Response( + 'The given query could not be executed, and has no results', + status=status.HTTP_400_BAD_REQUEST, + ) diff --git a/multinet/api/views/serializers.py b/multinet/api/views/serializers.py index a706e86..2fd1610 100644 --- a/multinet/api/views/serializers.py +++ b/multinet/api/views/serializers.py @@ -95,7 +95,7 @@ class AqlQuerySerializer(serializers.Serializer): class AqlQueryTaskSerializer(serializers.ModelSerializer): class Meta: model = AqlQuery - fields = '__all__' + exclude = ['results'] workspace = WorkspaceSerializer() @@ -103,6 +103,15 @@ class Meta: user = serializers.CharField() +class AqlQueryResultsSerializer(serializers.ModelSerializer): + class Meta: + model = AqlQuery + fields = ['id', 'workspace', 'user', 'results'] + + workspace = serializers.CharField() + user = serializers.CharField() + + class LimitOffsetSerializer(serializers.Serializer): limit = serializers.IntegerField(required=False) offset = serializers.IntegerField(required=False)