Skip to content

Commit

Permalink
Add REST API tests for /requests API && test both versions of the exp…
Browse files Browse the repository at this point in the history
…ort API (#8216)

This PR fixes the following issues:
- [export API v1] do not reinitialize dataset export process when
downloading a result file if a resource (project|task|job) has been
updated since the first initialized export request
- [export API v1] return `rq_id` for all requests with 202 status code
(not only for initialization requests)
- [requests API] Fixed filtering by format && added resource to allowed
filters

REST API tests updates:
- Added tests to check requests filtration using simple filters
- Added tests to check specific requests retrieving
- Updated all tests that export project|task|job
datasets|annotations|backups:
  - to test both API versions (including API mixing)
- to use only appropriate resources by checking the default export
location
- Added fixtures to filter projects/tasks assets
- Updated default target|source buckets to `import/export` bucket to
exclude the same bucket usage as a data source in several tests (when
bucket content is used as task data) and as a bucket for results

## Summary by CodeRabbit

- **New Features**
- Enhanced job handling for exports, improving error management and job
state tracking.
- Introduced a new `resource` field in the request handling system to
improve data categorization.
- Added new filtering capabilities for API queries, allowing users to
filter by the `resource` field.

- **Bug Fixes**
	- Improved status checks and handling for job requests.
- Introduced exception handling for forbidden access during project
backup attempts.

- **Tests**
- Refactored test suites to improve coverage and ensure compatibility
across versions with new methods and exception handling.
	- New tests added to validate request handling functionality.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Maxim Zhiltsov <zhiltsov.max35@gmail.com>
  • Loading branch information
Marishka17 and zhiltsov-max authored Aug 21, 2024
1 parent 3cd4c39 commit 3fdb032
Show file tree
Hide file tree
Showing 27 changed files with 1,568 additions and 426 deletions.
9 changes: 9 additions & 0 deletions changelog.d/20240820_134050_maria_rest_api_tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
### Fixed

- Prevent export process from restarting when downloading a result file,
that resulted in downloading a file with new request ID
(<https://github.com/cvat-ai/cvat/pull/8216>)
- Race condition occurred while handling parallel export requests
(<https://github.com/cvat-ai/cvat/pull/8216>)
- Requests filtering using format and target filters
(<https://github.com/cvat-ai/cvat/pull/8216>)

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions cvat/apps/engine/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from django.db.models.query import QuerySet
from django.utils.translation import gettext_lazy as _
from django.utils.encoding import force_str
from django.http import HttpRequest
from rest_framework.request import Request
from rest_framework import filters
from rest_framework.compat import coreapi, coreschema
from rest_framework.exceptions import ValidationError
Expand Down Expand Up @@ -412,11 +412,11 @@ def get_schema_operation_parameters(self, view):
parameters.append(parameter)
return parameters

def filter_queryset(self, request: HttpRequest, queryset: Iterable, view):
def filter_queryset(self, request: Request, queryset: Iterable, view):
filtered_queryset = queryset

query_params = request.query_params
filters_to_use = set(query_params)
filters_to_use = set(query_params.keys())

simple_filters = getattr(view, self.filter_fields_attr, None)
lookup_fields = self.get_lookup_fields(view)
Expand Down Expand Up @@ -465,7 +465,7 @@ def get_ordering(self, request, queryset, view) -> Tuple[List[str], bool]:

return result, reverse

def filter_queryset(self, request: HttpRequest, queryset: Iterable, view) -> Iterable:
def filter_queryset(self, request: Request, queryset: Iterable, view) -> Iterable:
ordering, reverse = self.get_ordering(request, queryset, view)

if ordering:
Expand Down Expand Up @@ -520,7 +520,7 @@ def _apply_filter(self, rules, lookup_fields, obj):
else:
raise ValidationError(f'filter: {op} operation with {args} arguments is not implemented')

def filter_queryset(self, request: HttpRequest, queryset: Iterable, view) -> Iterable:
def filter_queryset(self, request: Request, queryset: Iterable, view) -> Iterable:
filtered_queryset = queryset
json_rules = request.query_params.get(self.filter_param)
if json_rules:
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from rest_framework.response import Response
from rest_framework.views import APIView

from cvat.apps.engine.background_operations import (BackupExportManager,
from cvat.apps.engine.background import (BackupExportManager,
DatasetExportManager)
from cvat.apps.engine.handlers import clear_import_cache
from cvat.apps.engine.location import StorageType, get_location_configuration
Expand Down
4 changes: 4 additions & 0 deletions cvat/apps/engine/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ def get_rq_lock_by_user(queue: DjangoRQ, user_id: int) -> Union[Lock, nullcontex
return queue.connection.lock(f'{queue.name}-lock-{user_id}', timeout=30)
return nullcontext()

def get_rq_lock_for_job(queue: DjangoRQ, rq_id: str) -> Lock:
# lock timeout corresponds to the nginx request timeout (proxy_read_timeout)
return queue.connection.lock(f'lock-for-job-{rq_id}'.lower(), timeout=60)

def get_rq_job_meta(
request: HttpRequest,
db_obj: Any,
Expand Down
12 changes: 6 additions & 6 deletions cvat/apps/engine/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ def upload_finished(self, request):
return Response(data='Unknown upload was finished',
status=status.HTTP_400_BAD_REQUEST)

@extend_schema(summary='Get project annotations or export them as a dataset',
@extend_schema(summary='Export project annotations as a dataset',
description=textwrap.dedent("""\
Deprecation warning:
Expand Down Expand Up @@ -535,11 +535,7 @@ def upload_finished(self, request):
def annotations(self, request, pk):
# FUTURE-TODO: mark exporting dataset using this endpoint as deprecated when new API for result file downloading will be implemented
self._object = self.get_object() # force call of check_object_permissions()
return self.export_dataset_v1(
request=request,
save_images=False,
get_data=dm.task.get_job_data,
)
return self.export_dataset_v1(request=request, save_images=False)

@extend_schema(summary='Back up a project',
description=textwrap.dedent("""\
Expand Down Expand Up @@ -3275,6 +3271,7 @@ class RequestViewSet(viewsets.GenericViewSet):
'job_id',
# derivatives fields (from parsed rq_id)
'action',
'target',
'subresource',
'format',
]
Expand All @@ -3284,7 +3281,9 @@ class RequestViewSet(viewsets.GenericViewSet):
lookup_fields = {
'created_date': 'created_at',
'action': 'parsed_rq_id.action',
'target': 'parsed_rq_id.target',
'subresource': 'parsed_rq_id.subresource',
'format': 'parsed_rq_id.format',
'status': 'get_status',
'project_id': 'meta.project_id',
'task_id': 'meta.task_id',
Expand All @@ -3300,6 +3299,7 @@ class RequestViewSet(viewsets.GenericViewSet):
'task_id': SchemaField('integer'),
'job_id': SchemaField('integer'),
'action': SchemaField('string', RequestAction.choices),
'target': SchemaField('string', RequestTarget.choices),
'subresource': SchemaField('string', RequestSubresource.choices),
'format': SchemaField('string'),
'org': SchemaField('string'),
Expand Down
13 changes: 11 additions & 2 deletions cvat/schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3552,7 +3552,7 @@ paths:
- POST /api/projects/<project_id>/dataset/export?save_images=False to initiate exporting process
- GET /api/requests/<rq_id> to check export status,
where rq_id is request id returned on initializing request'
summary: Get project annotations or export them as a dataset
summary: Export project annotations as a dataset
parameters:
- in: query
name: action
Expand Down Expand Up @@ -4541,7 +4541,7 @@ paths:
Details about the syntax used can be found at the link: https://jsonlogic.com/
Available filter_fields: ['status', 'project_id', 'task_id', 'job_id', 'action', 'subresource', 'format'].
Available filter_fields: ['status', 'project_id', 'task_id', 'job_id', 'action', 'target', 'subresource', 'format'].
schema:
type: string
- name: format
Expand Down Expand Up @@ -4602,6 +4602,15 @@ paths:
- annotations
- dataset
- backup
- name: target
in: query
description: A simple equality filter for the target field
schema:
type: string
enum:
- project
- task
- job
- name: task_id
in: query
description: A simple equality filter for the task_id field
Expand Down
1 change: 1 addition & 0 deletions dev/format_python_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ for paths in \
"cvat/apps/quality_control" \
"cvat/apps/analytics_report" \
"cvat/apps/engine/lazy_list.py" \
"cvat/apps/engine/background.py" \
; do
${BLACK} -- ${paths}
${ISORT} -- ${paths}
Expand Down
2 changes: 1 addition & 1 deletion tests/python/rest_api/test_cloud_storages.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def _get_endpoint(self, api_client: ApiClient) -> Endpoint:
("provider_type", "name", "resource", "credentials_type", "owner"),
)
def test_can_use_simple_filter_for_object_list(self, field):
return super().test_can_use_simple_filter_for_object_list(field)
return super()._test_can_use_simple_filter_for_object_list(field)


@pytest.mark.usefixtures("restore_db_per_function")
Expand Down
2 changes: 1 addition & 1 deletion tests/python/rest_api/test_invitations.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def _get_endpoint(self, api_client: ApiClient) -> Endpoint:
("owner",),
)
def test_can_use_simple_filter_for_object_list(self, field):
return super().test_can_use_simple_filter_for_object_list(field)
return super()._test_can_use_simple_filter_for_object_list(field)


@pytest.mark.usefixtures("restore_db_per_class")
Expand Down
4 changes: 2 additions & 2 deletions tests/python/rest_api/test_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def _get_endpoint(self, api_client: ApiClient) -> Endpoint:
("owner", "assignee", "job_id", "resolved", "frame_id"),
)
def test_can_use_simple_filter_for_object_list(self, field):
return super().test_can_use_simple_filter_for_object_list(field)
return super()._test_can_use_simple_filter_for_object_list(field)


class TestCommentsListFilters(CollectionSimpleFilterTestBase):
Expand Down Expand Up @@ -393,7 +393,7 @@ def _get_field_samples(self, field: str) -> Tuple[Any, List[Dict[str, Any]]]:
("owner", "issue_id", "job_id", "frame_id"),
)
def test_can_use_simple_filter_for_object_list(self, field):
return super().test_can_use_simple_filter_for_object_list(field)
return super()._test_can_use_simple_filter_for_object_list(field)


@pytest.mark.usefixtures("restore_db_per_class")
Expand Down
Loading

0 comments on commit 3fdb032

Please sign in to comment.