diff --git a/gracedb/events/api/views.py b/gracedb/events/api/views.py index 7df48142241adf405730c6aee3406831ba90c47c..5ebc2ffef82c0b2925a2a9d584b8ace928ab0853 100644 --- a/gracedb/events/api/views.py +++ b/gracedb/events/api/views.py @@ -7,6 +7,7 @@ from django.urls import reverse as django_reverse from django.conf import settings from django.utils.functional import wraps from django.db import IntegrityError +from django.core.exceptions import ValidationError from django.contrib.auth.models import User, Permission from django.contrib.auth.models import Group as AuthGroup @@ -851,15 +852,17 @@ class EventLogList(APIView): retval = tmp.put(request, event.graceid(), n, tagname) # XXX This seems like a bizarre way of getting an error message out. if retval.status_code != 201: - tw_dict = {'tagWarning': 'Error creating tag %s.' % tagname } - #response['tagWarning'] = 'Error creating tag %s.' % tagname + return Response(('Log message created, but error creating ' + 'tag: {0}').format(retval.data), + status=retval.status_code) + #tw_dict = {'tagWarning': 'Error creating tag %s.' % tagname } # Serialize the event log object *after* adding tags! rv = eventLogToDict(logentry, request=request) response = Response(rv, status=status.HTTP_201_CREATED) response['Location'] = rv['self'] - if 'tagWarning' in tw_dict.keys(): - response['tagWarning'] = tw_dict['tagWarning'] + #if 'tagWarning' in tw_dict.keys(): + # response['tagWarning'] = tw_dict['tagWarning'] # Issue alert. description = "LOG: " @@ -1234,7 +1237,12 @@ class EventLogTagDetail(APIView): return HttpResponseForbidden(msg) # Look for the tag. If it doesn't already exist, create it. - tag, created = Tag.objects.get_or_create(name=tagname) + #tag, created = Tag.objects.get_or_create(name=tagname) + try: + tag, created = Tag.objects.get_or_create(name=tagname) + except ValidationError as e: + return Response(e.__str__(), + status=status.HTTP_400_BAD_REQUEST) displayName = request.data.get('displayName') if created: tag.displayName = displayName @@ -1843,21 +1851,23 @@ class VOEventList(APIView): # Create LogEntry to document the new VOEvent. logentry = EventLog(event=event, issuer=request.user, - comment='', + comment='New VOEvent', filename=filename, file_version=file_version) try: logentry.save() - except: + except Exception as e: rv['warnings'] = 'Problem saving log entry for VOEvent %s of %s' % (voevent.N, event.graceid()) - # Tag log entry as 'sky_loc' + # Tag log entry as 'em_follow') tmp = EventLogTagDetail() - retval = tmp.put(request, event.graceid(), logentry.N, 'em_follow') + retval = tmp.put(request, event.graceid(), logentry.N, 'em_follow') # XXX This seems like a bizarre way of getting an error message out. if retval.status_code != 201: - rv['tagWarning'] = 'Error tagging VOEvent log message as em_follow.' + return Response(('VOEvent log message created, but error tagging ' + 'message: {0}').format(retval.data), status=retval.status_code) + #rv['tagWarning'] = 'Error tagging VOEvent log message as em_follow.' # Issue alert. description = "VOEVENT: %s" % filename diff --git a/gracedb/superevents/api/mixins.py b/gracedb/superevents/api/mixins.py index c277ce883faa1cf628d4cfa264ca08f28c860607..e3b5d585f5e520488dacc05a331b6d4f96698d80 100644 --- a/gracedb/superevents/api/mixins.py +++ b/gracedb/superevents/api/mixins.py @@ -1,8 +1,15 @@ from django.shortcuts import get_object_or_404 +from django.core.exceptions import ValidationError as DjangoValidationError +from rest_framework.exceptions import ValidationError as \ + RestFrameworkValidationError +from rest_framework import status, mixins +from rest_framework.response import Response from ..models import Superevent from .settings import SUPEREVENT_LOOKUP_FIELD, SUPEREVENT_LOOKUP_REGEX +import logging +logger = logging.getLogger(__name__) class GetParentMixin(object): parent_lookup_field = None @@ -43,3 +50,47 @@ class BaseGetObjectMixin(object): filter_kwargs = {self.query_field: query_value} obj = get_object_or_404(queryset, **filter_kwargs) return obj + + +class SafeDestroyMixin(mixins.DestroyModelMixin): + """ + Copy of rest_framework's DestroyModelMixin which wraps + the call to perform_destroy() in a try-except block for + proper response handling. + """ + destroy_error_classes = (Exception,) + destroy_error_response_status = status.HTTP_500_INTERNAL_SERVER_ERROR + destroy_error_message = None + + def destroy(self, request, *args, **kwargs): + instance = self.get_object() + try: + self.perform_destroy(instance) + except self.destroy_error_classes as e: + err_msg = self.destroy_error_message or e.__str__() + return Response(err_msg, status=self.destroy_error_response_status) + return Response(status=status.HTTP_204_NO_CONTENT) + + +class SafeCreateMixin(mixins.CreateModelMixin): + """ + Copy of rest_framework's CreateModelMixin which wraps + the call to perform_destroy() in a try-except block for + proper response handling. + """ + create_error_classes = \ + (DjangoValidationError, RestFrameworkValidationError) + create_error_response_status = status.HTTP_400_BAD_REQUEST + create_error_message = None + + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + try: + self.perform_create(serializer) + except self.create_error_classes as e: + err_msg = self.create_error_message or e.__str__() + return Response(err_msg, status=self.create_error_response_status) + headers = self.get_success_headers(serializer.data) + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + diff --git a/gracedb/superevents/api/views.py b/gracedb/superevents/api/views.py index f31d7671e5928c8bcc2df1736819b071e78f3409..3ca2dd54e9c244f65d0c2eb1632b961d6cbd4fae 100644 --- a/gracedb/superevents/api/views.py +++ b/gracedb/superevents/api/views.py @@ -26,7 +26,8 @@ from events.view_utils import reverse as gracedb_reverse from events.api.backends import LigoAuthentication from .filters import SupereventSearchFilter, SupereventOrderingFilter -from .mixins import GetParentSupereventMixin, BaseGetObjectMixin +from .mixins import GetParentSupereventMixin, BaseGetObjectMixin, \ + SafeDestroyMixin, SafeCreateMixin from .paginators import BasePaginationFactory, CustomLabelPagination, \ CustomLogTagPagination, CustomSupereventPagination from .serializers import SupereventSerializer, SupereventUpdateSerializer, \ @@ -102,13 +103,15 @@ class SupereventViewSet(viewsets.ModelViewSet): class SupereventEventViewSet(mixins.ListModelMixin, mixins.CreateModelMixin, mixins.RetrieveModelMixin, - mixins.DestroyModelMixin, + SafeDestroyMixin, GetParentSupereventMixin, viewsets.GenericViewSet): """View for events attached to a superevent""" serializer_class = SupereventEventSerializer pagination_class = BasePaginationFactory(results_name='events') lookup_field = 'graceid' + destroy_error_classes = (Superevent.PreferredEventRemovalError,) + destroy_error_response_status = status.HTTP_400_BAD_REQUEST def get_queryset(self): superevent = self.get_parent() @@ -127,20 +130,11 @@ class SupereventEventViewSet(mixins.ListModelMixin, return obj - def destroy(self, request, *args, **kwargs): - instance = self.get_object() - try: - self.perform_destroy(instance) - except Superevent.PreferredEventRemovalError as e: - return Response(e.__str__(), status=status.HTTP_400_BAD_REQUEST) - - return Response(status=status.HTTP_204_NO_CONTENT) - def perform_destroy(self, instance): - remove_event_from_superevent(instance.superevent, instance, - self.request.user, add_superevent_log=True, - add_event_log=True, issue_superevent_alert=True, - issue_event_alert=True) + remove_event_from_superevent(instance.superevent, instance, + self.request.user, add_superevent_log=True, + add_event_log=True, issue_superevent_alert=True, + issue_event_alert=True) class SupereventLabelViewSet(GetParentSupereventMixin, @@ -158,16 +152,6 @@ class SupereventLabelViewSet(GetParentSupereventMixin, queryset = superevent.labelling_set.all() return queryset - def destroy(self, request, *args, **kwargs): - instance = self.get_object() - try: - self.perform_destroy(instance) - except Exception as e: - return Response(e.__str__(), - status=status.HTTP_500_INTERNAL_SERVER_ERROR) - - return Response(status=status.HTTP_204_NO_CONTENT) - def perform_destroy(self, instance): remove_label_from_superevent(instance, self.request.user, add_log_message=True, issue_alert=True) @@ -175,7 +159,7 @@ class SupereventLabelViewSet(GetParentSupereventMixin, class SupereventLogViewSet(mixins.ListModelMixin, mixins.RetrieveModelMixin, - mixins.CreateModelMixin, + SafeCreateMixin, GetParentSupereventMixin, BaseGetObjectMixin, viewsets.GenericViewSet): @@ -197,7 +181,8 @@ class SupereventLogViewSet(mixins.ListModelMixin, class SupereventLogTagViewSet(GetParentSupereventMixin, BaseGetObjectMixin, - viewsets.ModelViewSet): + viewsets.ModelViewSet, + SafeCreateMixin): """ View for tags attached to a log message which is attached to a superevent. """ @@ -216,16 +201,6 @@ class SupereventLogTagViewSet(GetParentSupereventMixin, parent_log = self.get_parent_log() return parent_log.tags.all() - def destroy(self, request, *args, **kwargs): - instance = self.get_object() - try: - self.perform_destroy(instance) - except Exception as e: - return Response(e.__str__(), - status=status.HTTP_500_INTERNAL_SERVER_ERROR) - - return Response(status=status.HTTP_204_NO_CONTENT) - def perform_destroy(self, instance): parent_log = self.get_parent_log() remove_tag_from_log(parent_log, instance, self.request.user,