From 4afd687b3ade9b85e29c5463d32db2f5c6e7c59d Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Tue, 19 Nov 2024 13:12:36 -0500 Subject: [PATCH 1/3] Add API endpoints for listing/clearing active uploads --- dandiapi/api/tests/test_dandiset.py | 78 +++++++++++++++++++++++++++++ dandiapi/api/views/dandiset.py | 54 +++++++++++++++++--- dandiapi/api/views/serializers.py | 13 ++++- 3 files changed, 137 insertions(+), 8 deletions(-) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 570333953..80d985111 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -1116,3 +1116,81 @@ def test_dandiset_contact_person_malformed_contributors(api_client, draft_versio ) assert results.data['draft_version']['dandiset']['contact_person'] == '' + + +@pytest.mark.django_db +@pytest.mark.parametrize('embargoed', [False, True]) +def test_dandiset_rest_list_active_uploads_not_owner(api_client, user, dandiset_factory, embargoed): + ds = dandiset_factory( + embargo_status=Dandiset.EmbargoStatus.EMBARGOED + if embargoed + else Dandiset.EmbargoStatus.OPEN + ) + + # Test unauthenticated + response = api_client.get(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 401 + + # Test unauthorized + api_client.force_authenticate(user=user) + response = api_client.get(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 403 + + +@pytest.mark.django_db +def test_dandiset_rest_list_active_uploads( + authenticated_api_client, user, draft_version, upload_factory +): + ds = draft_version.dandiset + + assign_perm('owner', user, ds) + upload = upload_factory(dandiset=ds) + + response = authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 200 + data = response.json() + assert len(data) == 1 + assert data[0]['upload_id'] == upload.upload_id + + +@pytest.mark.django_db +@pytest.mark.parametrize('embargoed', [False, True]) +def test_dandiset_rest_clear_active_uploads_not_owner( + api_client, user, dandiset_factory, upload_factory, embargoed +): + ds = dandiset_factory( + embargo_status=Dandiset.EmbargoStatus.EMBARGOED + if embargoed + else Dandiset.EmbargoStatus.OPEN + ) + + upload_factory(dandiset=ds) + + # Test unauthenticated + response = api_client.delete(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 401 + + # Test unauthorized + api_client.force_authenticate(user=user) + response = api_client.delete(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 403 + + assert ds.uploads.count() == 1 + + +@pytest.mark.django_db +def test_dandiset_rest_clear_active_uploads( + authenticated_api_client, user, draft_version, upload_factory +): + ds = draft_version.dandiset + + assign_perm('owner', user, ds) + upload_factory(dandiset=ds) + + assert len(authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json()) == 1 + + response = authenticated_api_client.delete(f'/api/dandisets/{ds.identifier}/uploads/') + assert response.status_code == 204 + + assert ds.uploads.count() == 0 + assert len(authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json()) == 0 diff --git a/dandiapi/api/views/dandiset.py b/dandiapi/api/views/dandiset.py index 4dea82aec..839c00c93 100644 --- a/dandiapi/api/views/dandiset.py +++ b/dandiapi/api/views/dandiset.py @@ -1,11 +1,12 @@ from __future__ import annotations +import typing from typing import TYPE_CHECKING from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.db import transaction -from django.db.models import Count, Max, OuterRef, Subquery, Sum +from django.db.models import Count, Max, OuterRef, QuerySet, Subquery, Sum from django.db.models.functions import Coalesce from django.db.models.query_utils import Q from django.http import Http404 @@ -41,6 +42,7 @@ DandisetQueryParameterSerializer, DandisetSearchQueryParameterSerializer, DandisetSearchResultListSerializer, + DandisetUploadSerializer, UserSerializer, VersionMetadataSerializer, ) @@ -49,6 +51,8 @@ if TYPE_CHECKING: from rest_framework.request import Request + from dandiapi.api.models.upload import Upload + class DandisetFilterBackend(filters.OrderingFilter): ordering_fields = ['id', 'name', 'modified', 'size'] @@ -155,6 +159,16 @@ def get_queryset(self): queryset = queryset.filter(embargo_status='OPEN') return queryset + def require_owner_perm(self, dandiset: Dandiset): + # Raise 401 if unauthenticated + if not self.request.user.is_authenticated: + raise NotAuthenticated + + # Raise 403 if unauthorized + self.request.user = typing.cast(User, self.request.user) + if not self.request.user.has_perm('owner', dandiset): + raise PermissionDenied + def get_object(self): # Alternative to path converters, which DRF doesn't support # https://docs.djangoproject.com/en/3.0/topics/http/urls/#registering-custom-path-converters @@ -168,12 +182,8 @@ def get_object(self): dandiset = super().get_object() if dandiset.embargo_status != Dandiset.EmbargoStatus.OPEN: - if not self.request.user.is_authenticated: - # Clients must be authenticated to access it - raise NotAuthenticated - if not self.request.user.has_perm('owner', dandiset): - # The user does not have ownership permission - raise PermissionDenied + self.require_owner_perm(dandiset) + return dandiset @staticmethod @@ -453,3 +463,33 @@ def users(self, request, dandiset__pk): # noqa: C901 ) return Response(owners, status=status.HTTP_200_OK) + + @swagger_auto_schema( + methods=['GET'], + manual_parameters=[DANDISET_PK_PARAM], + request_body=no_body, + operation_summary='List active/incomplete uploads in this dandiset.', + ) + @action(methods=['GET'], detail=True) + def uploads(self, request, dandiset__pk): + dandiset: Dandiset = self.get_object() + + # Special case where a "safe" method is access restricted, due to the nature of uploads + self.require_owner_perm(dandiset) + + uploads: QuerySet[Upload] = dandiset.uploads.all() + serializer = DandisetUploadSerializer(instance=uploads, many=True) + return Response(serializer.data, status=status.HTTP_200_OK) + + @swagger_auto_schema( + manual_parameters=[DANDISET_PK_PARAM], + request_body=no_body, + operation_summary='Delete all active/incomplete uploads in this dandiset.', + ) + @uploads.mapping.delete + def clear_uploads(self, request, dandiset__pk): + dandiset: Dandiset = self.get_object() + self.require_owner_perm(dandiset) + + dandiset.uploads.all().delete() + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/dandiapi/api/views/serializers.py b/dandiapi/api/views/serializers.py index fb9ff03a0..be05589df 100644 --- a/dandiapi/api/views/serializers.py +++ b/dandiapi/api/views/serializers.py @@ -8,7 +8,7 @@ from drf_yasg.utils import swagger_serializer_method from rest_framework import serializers -from dandiapi.api.models import Asset, AssetBlob, AssetPath, Dandiset, Version +from dandiapi.api.models import Asset, AssetBlob, AssetPath, Dandiset, Upload, Version from dandiapi.search.models import AssetSearch if TYPE_CHECKING: @@ -299,6 +299,17 @@ def get_contact_person(self, obj): return extract_contact_person(obj) +class DandisetUploadSerializer(serializers.ModelSerializer): + class Meta: + model = Upload + exclude = [ + 'dandiset', + 'embargoed', + 'id', + 'multipart_upload_id', + ] + + class AssetBlobSerializer(serializers.ModelSerializer): class Meta: model = AssetBlob From e2f5237a7f92723b84855c5fd2c32913797669ca Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Wed, 20 Nov 2024 11:58:15 -0500 Subject: [PATCH 2/3] Add dialog to manage uploads for embargoed dandisets --- web/src/rest.ts | 8 ++ web/src/types/index.ts | 8 ++ .../DandisetLandingView/DandisetUnembargo.vue | 119 +++++++++++++++--- 3 files changed, 121 insertions(+), 14 deletions(-) diff --git a/web/src/rest.ts b/web/src/rest.ts index 69922b1e3..9cde8b971 100644 --- a/web/src/rest.ts +++ b/web/src/rest.ts @@ -12,6 +12,7 @@ import type { AssetPath, Zarr, DandisetSearchResult, + IncompleteUpload, } from '@/types'; import type { Dandiset as DandisetMetadata, @@ -106,6 +107,13 @@ const dandiRest = { throw e; } }, + async uploads(identifier: string): Promise { + const res = await client.get(`dandisets/${identifier}/uploads/`); + return res.data; + }, + async clearUploads(identifier: string) { + await client.delete(`dandisets/${identifier}/uploads/`); + }, async assets( identifier: string, version: string, diff --git a/web/src/types/index.ts b/web/src/types/index.ts index 7e6caaf69..8f040553a 100644 --- a/web/src/types/index.ts +++ b/web/src/types/index.ts @@ -102,3 +102,11 @@ export interface AssetPath { aggregate_size: number; asset: AssetFile | null; } + +export interface IncompleteUpload { + created: string; + blob: string; + upload_id: string; + etag: string; + size: number; +} \ No newline at end of file diff --git a/web/src/views/DandisetLandingView/DandisetUnembargo.vue b/web/src/views/DandisetLandingView/DandisetUnembargo.vue index d7d7cfe42..d862d6333 100644 --- a/web/src/views/DandisetLandingView/DandisetUnembargo.vue +++ b/web/src/views/DandisetLandingView/DandisetUnembargo.vue @@ -4,6 +4,65 @@ outlined class="mt-4 px-3" > + + + + This dandiset has active uploads + + + + This dandiset has {{ incompleteUploads.length }} active uploads. Unembargo may not proceed until these are addressed. You may delete these uploads using the "Clear Uploads" button below. + + + + + + + + + + + Close + + + + + + + + Delete all uploads? Once deleted, any partially uploaded data will be lost. + + + Delete + + + + + + - + - This dandiset is being unembargoed, please wait. - This dandiset has active uploads. Please complete or clear these uploads before proceeding. + This dandiset is being unembargoed, please wait. @@ -119,19 +175,54 @@ import { computed, ref } from 'vue'; import moment from 'moment'; import { dandiRest } from '@/rest'; import { useDandisetStore } from '@/stores/dandiset'; +import type { IncompleteUpload } from '@/types'; +import filesize from 'filesize'; -function formatDate(date: string): string { - return moment(date).format('ll'); +function formatDate(date: string, format: string = 'll'): string { + return moment(date).format(format); } -const store = useDandisetStore(); +const uploadHeaders = [ + { + text: 'Started at', + value: 'created', + }, + { + text: 'Size', + value: 'size', + }, + { + text: 'E-Tag', + value: 'etag', + }, +]; +const store = useDandisetStore(); const currentDandiset = computed(() => store.dandiset); const unembargo_in_progress = computed(() => currentDandiset.value?.dandiset.embargo_status === 'UNEMBARGOING'); -const unembargoDisabled = computed(() => !!(unembargo_in_progress.value || currentDandiset.value === null || currentDandiset.value.active_uploads)); const showWarningDialog = ref(false); const confirmationPhrase = ref(''); +// Upload management +const showUploadManagementDialog = ref(false); +const incompleteUploads = ref([]); +const unembargoDisabled = computed(() => !!(unembargo_in_progress.value || currentDandiset.value === null || currentDandiset.value.active_uploads)); +if (unembargoDisabled.value) { + fetchIncompleteUploads(); +} + +async function fetchIncompleteUploads() { + incompleteUploads.value = await dandiRest.uploads(currentDandiset.value!.dandiset.identifier); +} + +async function clearUploads() { + const identifier = currentDandiset.value!.dandiset.identifier; + await dandiRest.clearUploads(identifier); + showUploadManagementDialog.value = false; + + store.fetchDandiset({ identifier, version: 'draft' }); +} + async function unembargo() { if (currentDandiset.value) { // Display the warning dialog before releasing From f1078bb97c00421a77636dc2a6c296e5122f3d87 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Tue, 26 Nov 2024 17:38:19 -0500 Subject: [PATCH 3/3] Paginate dandiset upload response --- dandiapi/api/tests/test_dandiset.py | 13 +++++++++---- dandiapi/api/views/dandiset.py | 13 +++++++++++-- dandiapi/api/views/serializers.py | 5 +++++ web/src/rest.ts | 18 ++++++++++++++++-- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 80d985111..899c2069d 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -1149,8 +1149,9 @@ def test_dandiset_rest_list_active_uploads( response = authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/') assert response.status_code == 200 data = response.json() - assert len(data) == 1 - assert data[0]['upload_id'] == upload.upload_id + assert data['count'] == 1 + assert len(data['results']) == 1 + assert data['results'][0]['upload_id'] == upload.upload_id @pytest.mark.django_db @@ -1187,10 +1188,14 @@ def test_dandiset_rest_clear_active_uploads( assign_perm('owner', user, ds) upload_factory(dandiset=ds) - assert len(authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json()) == 1 + response = authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json() + assert response['count'] == 1 + assert len(response['results']) == 1 response = authenticated_api_client.delete(f'/api/dandisets/{ds.identifier}/uploads/') assert response.status_code == 204 assert ds.uploads.count() == 0 - assert len(authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json()) == 0 + response = authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json() + assert response['count'] == 0 + assert len(response['results']) == 0 diff --git a/dandiapi/api/views/dandiset.py b/dandiapi/api/views/dandiset.py index 839c00c93..867dd4319 100644 --- a/dandiapi/api/views/dandiset.py +++ b/dandiapi/api/views/dandiset.py @@ -43,6 +43,7 @@ DandisetSearchQueryParameterSerializer, DandisetSearchResultListSerializer, DandisetUploadSerializer, + PaginationQuerySerializer, UserSerializer, VersionMetadataSerializer, ) @@ -467,6 +468,7 @@ def users(self, request, dandiset__pk): # noqa: C901 @swagger_auto_schema( methods=['GET'], manual_parameters=[DANDISET_PK_PARAM], + query_serializer=PaginationQuerySerializer, request_body=no_body, operation_summary='List active/incomplete uploads in this dandiset.', ) @@ -478,8 +480,15 @@ def uploads(self, request, dandiset__pk): self.require_owner_perm(dandiset) uploads: QuerySet[Upload] = dandiset.uploads.all() - serializer = DandisetUploadSerializer(instance=uploads, many=True) - return Response(serializer.data, status=status.HTTP_200_OK) + + # Paginate and return + page = self.paginate_queryset(uploads) + if page is not None: + serializer = DandisetUploadSerializer(page, many=True) + return self.get_paginated_response(serializer.data) + + serializer = DandisetUploadSerializer(uploads, many=True) + return Response(serializer.data) @swagger_auto_schema( manual_parameters=[DANDISET_PK_PARAM], diff --git a/dandiapi/api/views/serializers.py b/dandiapi/api/views/serializers.py index be05589df..f12b5ca2a 100644 --- a/dandiapi/api/views/serializers.py +++ b/dandiapi/api/views/serializers.py @@ -373,6 +373,11 @@ class AssetPathsQueryParameterSerializer(serializers.Serializer): path_prefix = serializers.CharField(default='') +class PaginationQuerySerializer(serializers.Serializer): + page = serializers.IntegerField(default=1) + page_size = serializers.IntegerField(default=100) + + class AssetFileSerializer(AssetSerializer): class Meta(AssetSerializer.Meta): fields = ['asset_id', 'url'] diff --git a/web/src/rest.ts b/web/src/rest.ts index 9cde8b971..233bf73d2 100644 --- a/web/src/rest.ts +++ b/web/src/rest.ts @@ -108,8 +108,22 @@ const dandiRest = { } }, async uploads(identifier: string): Promise { - const res = await client.get(`dandisets/${identifier}/uploads/`); - return res.data; + const uploads = [] + let page = 1; + + // eslint-disable-next-line no-constant-condition + while (true) { + const res = await client.get(`dandisets/${identifier}/uploads/`, {params: { page }}); + + uploads.push(...res.data.results); + if (res.data.next === null) { + break; + } + + page += 1; + } + + return uploads; }, async clearUploads(identifier: string) { await client.delete(`dandisets/${identifier}/uploads/`);