From f6b1a845455e3e739100e0fbcf1cd7b094bc9179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Mon, 22 Dec 2014 18:09:05 +0100 Subject: [PATCH] Cleaned up utils.views to increase performance when fetching single objects from the database for a view. Fixed #1378. --- CHANGELOG | 4 + openslides/agenda/views.py | 98 ++++++------ openslides/assignment/views.py | 52 +++--- openslides/mediafile/views.py | 4 +- openslides/motion/views.py | 150 ++++++++---------- openslides/participant/views.py | 33 ++-- openslides/poll/views.py | 13 +- openslides/utils/views.py | 47 ++++-- tests/mediafile/tests.py | 4 +- tests/settings.py | 1 + tests/utils/models.py | 8 + .../templates/utils/dummymodel_detail.html | 5 + tests/utils/test_views.py | 13 ++ tests/utils/urls.py | 3 + tests/utils/views.py | 14 ++ 15 files changed, 249 insertions(+), 200 deletions(-) create mode 100644 tests/utils/models.py create mode 100644 tests/utils/templates/utils/dummymodel_detail.html diff --git a/CHANGELOG b/CHANGELOG index e09e3d91a..001b7d0db 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,10 @@ Version 1.6.2 (unreleased) ========================== [https://github.com/OpenSlides/OpenSlides/milestones/1.6.2] +Other: +- Cleaned up utils.views to increase performance when fetching single objects + from the database for a view (#1378). + Version 1.6.1 (2014-12-08) ========================== diff --git a/openslides/agenda/views.py b/openslides/agenda/views.py index 5133755d0..bba1582fc 100644 --- a/openslides/agenda/views.py +++ b/openslides/agenda/views.py @@ -183,15 +183,14 @@ class AgendaItemView(SingleObjectMixin, FormView): return check_permission def get_context_data(self, **kwargs): - self.object = self.get_object() - list_of_speakers = self.object.get_list_of_speakers() + list_of_speakers = self.get_object().get_list_of_speakers() active_slide = get_active_slide() active_type = active_slide.get('type', None) kwargs.update({ - 'object': self.object, + 'object': self.get_object(), 'list_of_speakers': list_of_speakers, 'is_on_the_list_of_speakers': Speaker.objects.filter( - item=self.object, begin_time=None, person=self.request.user).exists(), + item=self.get_object(), begin_time=None, person=self.request.user).exists(), 'active_type': active_type, }) return super(AgendaItemView, self).get_context_data(**kwargs) @@ -206,7 +205,7 @@ class AgendaItemView(SingleObjectMixin, FormView): return kwargs -class SetClosed(RedirectView, SingleObjectMixin): +class SetClosed(SingleObjectMixin, RedirectView): """ Close or open an item. """ @@ -218,15 +217,14 @@ class SetClosed(RedirectView, SingleObjectMixin): def get_ajax_context(self, **kwargs): closed = self.kwargs['closed'] if closed: - link = reverse('item_open', args=[self.object.pk]) + link = reverse('item_open', args=[self.get_object().pk]) else: - link = reverse('item_close', args=[self.object.pk]) + link = reverse('item_close', args=[self.get_object().pk]) return super(SetClosed, self).get_ajax_context(closed=closed, link=link) def pre_redirect(self, request, *args, **kwargs): - self.object = self.get_object() closed = kwargs['closed'] - self.object.set_closed(closed) + self.get_object().set_closed(closed) return super(SetClosed, self).pre_redirect(request, *args, **kwargs) def get_url_name_args(self): @@ -245,7 +243,7 @@ class ItemUpdate(UpdateView): url_name_args = [] def get_form_class(self): - if self.object.content_object: + if self.get_object().content_object: form = RelatedItemForm else: form = ItemForm @@ -286,7 +284,7 @@ class ItemDelete(DeleteView): try: options = self.item_delete_answer_options except AttributeError: - if self.object.children.exists(): + if self.get_object().children.exists(): options = [('all', _("Yes, with all child items."))] + self.answer_options else: options = self.answer_options @@ -297,13 +295,13 @@ class ItemDelete(DeleteView): """ Deletes the item but not its children. """ - self.object.delete(with_children=False) + self.get_object().delete(with_children=False) def on_clicked_all(self): """ Deletes the item and its children. """ - self.object.delete(with_children=True) + self.get_object().delete(with_children=True) def get_final_message(self): """ @@ -312,9 +310,9 @@ class ItemDelete(DeleteView): # OpenSlidesError (invalid answer) should never be raised here because # this method should only be called if the answer is 'yes' or 'all'. if self.get_answer() == 'yes': - message = _('Item %s was successfully deleted.') % html_strong(self.object) + message = _('Item %s was successfully deleted.') % html_strong(self.get_object()) else: - message = _('Item %s and its children were successfully deleted.') % html_strong(self.object) + message = _('Item %s and its children were successfully deleted.') % html_strong(self.get_object()) return message @@ -329,18 +327,11 @@ class CreateRelatedAgendaItemView(SingleObjectMixin, RedirectView): url_name = 'item_overview' url_name_args = [] - def get(self, request, *args, **kwargs): - """ - Set self.object to the relevant object. - """ - self.object = self.get_object() - return super(CreateRelatedAgendaItemView, self).get(request, *args, **kwargs) - def pre_redirect(self, request, *args, **kwargs): """ Create the agenda item. """ - self.item = Item.objects.create(content_object=self.object) + self.item = Item.objects.create(content_object=self.get_object()) class AgendaNumberingView(QuestionView): @@ -388,12 +379,11 @@ class SpeakerAppendView(SingleObjectMixin, RedirectView): model = Item def pre_redirect(self, request, *args, **kwargs): - self.object = self.get_object() - if self.object.speaker_list_closed: + if self.get_object().speaker_list_closed: messages.error(request, _('The list of speakers is closed.')) else: try: - Speaker.objects.add(item=self.object, person=request.user) + Speaker.objects.add(item=self.get_object(), person=request.user) except OpenSlidesError, e: messages.error(request, e) else: @@ -418,11 +408,10 @@ class SpeakerDeleteView(DeleteView): return True def get(self, *args, **kwargs): - try: - return super(SpeakerDeleteView, self).get(*args, **kwargs) - except Speaker.DoesNotExist: - messages.error(self.request, _('You are not on the list of speakers.')) + if self.get_object() is None: return super(RedirectView, self).get(*args, **kwargs) + else: + return super(SpeakerDeleteView, self).get(*args, **kwargs) def get_object(self): """ @@ -432,10 +421,22 @@ class SpeakerDeleteView(DeleteView): object with the request.user as speaker. """ try: - return Speaker.objects.get(pk=self.kwargs['speaker']) - except KeyError: - return Speaker.objects.filter( - item=self.kwargs['pk'], person=self.request.user).exclude(weight=None).get() + speaker = self._object + except AttributeError: + speaker_pk = self.kwargs.get('speaker') + if speaker_pk is not None: + queryset = Speaker.objects.filter(pk=speaker_pk) + else: + queryset = Speaker.objects.filter( + item=self.kwargs['pk'], person=self.request.user).exclude(weight=None) + try: + speaker = queryset.get() + except Speaker.DoesNotExist: + speaker = None + if speaker_pk is not None: + messages.error(self.request, _('You are not on the list of speakers.')) + self._object = speaker + return speaker def get_url_name_args(self): return [self.kwargs['pk']] @@ -456,22 +457,21 @@ class SpeakerSpeakView(SingleObjectMixin, RedirectView): model = Item def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() try: speaker = Speaker.objects.filter( person=kwargs['person_id'], - item=self.object, + item=self.get_object(), begin_time=None).get() except Speaker.DoesNotExist: # TODO: Check the MultipleObjectsReturned error here? messages.error( self.request, _('%(person)s is not on the list of %(item)s.') - % {'person': kwargs['person_id'], 'item': self.object}) + % {'person': kwargs['person_id'], 'item': self.get_object()}) else: speaker.begin_speach() def get_url_name_args(self): - return [self.object.pk] + return [self.get_object().pk] class SpeakerEndSpeachView(SingleObjectMixin, RedirectView): @@ -483,21 +483,20 @@ class SpeakerEndSpeachView(SingleObjectMixin, RedirectView): model = Item def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() try: speaker = Speaker.objects.filter( - item=self.object, + item=self.get_object(), end_time=None).exclude(begin_time=None).get() except Speaker.DoesNotExist: messages.error( self.request, _('There is no one speaking at the moment according to %(item)s.') - % {'item': self.object}) + % {'item': self.get_object()}) else: speaker.end_speach() def get_url_name_args(self): - return [self.object.pk] + return [self.get_object().pk] class SpeakerListCloseView(SingleObjectMixin, RedirectView): @@ -510,12 +509,11 @@ class SpeakerListCloseView(SingleObjectMixin, RedirectView): url_name = 'item_view' def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() - self.object.speaker_list_closed = not self.reopen - self.object.save() + self.get_object().speaker_list_closed = not self.reopen + self.get_object().save() def get_url_name_args(self): - return [self.object.pk] + return [self.get_object().pk] class SpeakerChangeOrderView(SingleObjectMixin, RedirectView): @@ -528,9 +526,6 @@ class SpeakerChangeOrderView(SingleObjectMixin, RedirectView): model = Item url_name = 'item_view' - def pre_redirect(self, args, **kwargs): - self.object = self.get_object() - @transaction.commit_manually def pre_post_redirect(self, request, *args, **kwargs): """ @@ -538,7 +533,6 @@ class SpeakerChangeOrderView(SingleObjectMixin, RedirectView): Take the string 'sort_order' from the post-data, and use this order. """ - self.object = self.get_object() transaction.commit() for (counter, speaker) in enumerate(self.request.POST['sort_order'].split(',')): try: @@ -547,7 +541,7 @@ class SpeakerChangeOrderView(SingleObjectMixin, RedirectView): transaction.rollback() break try: - speaker = Speaker.objects.filter(item=self.object).get(pk=speaker_pk) + speaker = Speaker.objects.filter(item=self.get_object()).get(pk=speaker_pk) except: transaction.rollback() break @@ -559,7 +553,7 @@ class SpeakerChangeOrderView(SingleObjectMixin, RedirectView): messages.error(request, _('Could not change order. Invalid data.')) def get_url_name_args(self): - return [self.object.pk] + return [self.get_object().pk] class CurrentListOfSpeakersView(RedirectView): diff --git a/openslides/assignment/views.py b/openslides/assignment/views.py index e2b0b1319..4f0391f56 100644 --- a/openslides/assignment/views.py +++ b/openslides/assignment/views.py @@ -43,31 +43,30 @@ class AssignmentDetail(DetailView): context['form'] = self.form_class(self.request.POST) else: context['form'] = self.form_class() - polls = self.object.poll_set.all() + polls = self.get_object().poll_set.all() if not self.request.user.has_perm('assignment.can_manage_assignment'): - polls = self.object.poll_set.filter(published=True) - vote_results = self.object.vote_results(only_published=True) + polls = self.get_object().poll_set.filter(published=True) + vote_results = self.get_object().vote_results(only_published=True) else: - polls = self.object.poll_set.all() - vote_results = self.object.vote_results(only_published=False) + polls = self.get_object().poll_set.all() + vote_results = self.get_object().vote_results(only_published=False) blocked_candidates = [ candidate.person for candidate in - self.object.assignment_candidates.filter(blocked=True)] + self.get_object().assignment_candidates.filter(blocked=True)] context['polls'] = polls context['vote_results'] = vote_results context['blocked_candidates'] = blocked_candidates - context['user_is_candidate'] = self.object.is_candidate(self.request.user) + context['user_is_candidate'] = self.get_object().is_candidate(self.request.user) return context def post(self, *args, **kwargs): - self.object = self.get_object() if self.request.user.has_perm('assignment.can_nominate_other'): form = self.form_class(self.request.POST) if form.is_valid(): user = form.cleaned_data['candidate'] try: - self.object.run(user, self.request.user) + self.get_object().run(user, self.request.user) except NameError, e: messages.error(self.request, e) else: @@ -101,18 +100,17 @@ class AssignmentSetStatusView(SingleObjectMixin, RedirectView): url_name = 'assignment_detail' def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() status = kwargs.get('status') if status is not None: try: - self.object.set_status(status) + self.get_object().set_status(status) except ValueError, e: messages.error(self.request, e) else: messages.success( self.request, _('Election status was set to: %s.') % - html_strong(self.object.get_status_display()) + html_strong(self.get_object().get_status_display()) ) @@ -137,11 +135,10 @@ class AssignmentRunDeleteView(SingleObjectMixin, RedirectView): url_name = 'assignment_detail' def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() - if self.object.status == 'sea' or self.request.user.has_perm( + if self.get_object().status == 'sea' or self.request.user.has_perm( "assignment.can_manage_assignment"): try: - self.object.delrun(self.request.user, blocked=True) + self.get_object().delrun(self.request.user, blocked=True) except Exception, e: messages.error(self.request, e) else: @@ -167,7 +164,7 @@ class AssignmentRunOtherDeleteView(SingleObjectMixin, QuestionView): def on_clicked_yes(self): self._get_person_information() try: - self.object.delrun(self.person, blocked=False) + self.get_object().delrun(self.person, blocked=False) except Exception, e: self.error = e else: @@ -186,9 +183,8 @@ class AssignmentRunOtherDeleteView(SingleObjectMixin, QuestionView): return message def _get_person_information(self): - self.object = self.get_object() self.person = get_person(self.kwargs.get('user_id')) - self.is_blocked = self.object.is_blocked(self.person) + self.is_blocked = self.get_object().is_blocked(self.person) class PollCreateView(SingleObjectMixin, RedirectView): @@ -197,8 +193,7 @@ class PollCreateView(SingleObjectMixin, RedirectView): url_name = 'assignment_detail' def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() - self.object.gen_poll() + self.get_object().gen_poll() messages.success(self.request, _("New ballot was successfully created.")) @@ -233,15 +228,15 @@ class SetPublishStatusView(SingleObjectMixin, RedirectView): def pre_redirect(self, *args, **kwargs): try: - self.object = self.get_object() + poll = self.get_object() except self.model.DoesNotExist: messages.error(self.request, _('Ballot ID %d does not exist.') % int(kwargs['poll_id'])) else: - if self.object.published: - self.object.set_published(False) + if poll.published: + poll.set_published(False) else: - self.object.set_published(True) + poll.set_published(True) class SetElectedView(SingleObjectMixin, RedirectView): @@ -251,19 +246,18 @@ class SetElectedView(SingleObjectMixin, RedirectView): allow_ajax = True def pre_redirect(self, *args, **kwargs): - self.object = self.get_object() self.person = get_person(kwargs['user_id']) self.elected = kwargs['elected'] - self.object.set_elected(self.person, self.elected) + self.get_object().set_elected(self.person, self.elected) def get_ajax_context(self, **kwargs): if self.elected: link = reverse('assignment_user_not_elected', - args=[self.object.id, self.person.person_id]) + args=[self.get_object().id, self.person.person_id]) text = _('not elected') else: link = reverse('assignment_user_elected', - args=[self.object.id, self.person.person_id]) + args=[self.get_object().id, self.person.person_id]) text = _('elected') return {'elected': self.elected, 'link': link, 'text': text} @@ -284,7 +278,7 @@ class AssignmentPollDeleteView(DeleteView): super(AssignmentPollDeleteView, self).pre_post_redirect(request, *args, **kwargs) def set_assignment(self): - self.assignment = self.object.assignment + self.assignment = self.get_object().assignment def get_redirect_url(self, **kwargs): return reverse('assignment_detail', args=[self.assignment.id]) diff --git a/openslides/mediafile/views.py b/openslides/mediafile/views.py index cfe27bd24..7ee4ce09c 100644 --- a/openslides/mediafile/views.py +++ b/openslides/mediafile/views.py @@ -86,7 +86,7 @@ class MediafileUpdateView(MediafileViewMixin, UpdateView): def get_form_kwargs(self, *args, **kwargs): form_kwargs = super(MediafileUpdateView, self).get_form_kwargs(*args, **kwargs) - form_kwargs['initial'].update({'uploader': self.object.uploader.person_id}) + form_kwargs['initial'].update({'uploader': self.get_object().uploader.person_id}) return form_kwargs @@ -103,7 +103,7 @@ class MediafileDeleteView(DeleteView): def on_clicked_yes(self, *args, **kwargs): """Deletes the file in the filesystem, if user clicks "Yes".""" - self.object.mediafile.delete() + self.get_object().mediafile.delete() return super(MediafileDeleteView, self).on_clicked_yes(*args, **kwargs) diff --git a/openslides/motion/views.py b/openslides/motion/views.py index 97da9976d..9a5a382bc 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -55,14 +55,14 @@ class MotionDetailView(DetailView): version_number = self.kwargs.get('version_number', None) if version_number is not None: try: - version = self.object.versions.get(version_number=int(version_number)) + version = self.get_object().versions.get(version_number=int(version_number)) except MotionVersion.DoesNotExist: raise Http404('Version %s not found' % version_number) else: - version = self.object.get_active_version() + version = self.get_object().get_active_version() kwargs.update({ - 'allowed_actions': self.object.get_allowed_actions(self.request.user), + 'allowed_actions': self.get_object().get_allowed_actions(self.request.user), 'version': version, 'title': version.title, 'text': version.text, @@ -302,21 +302,25 @@ class VersionDeleteView(DeleteView): def get_object(self): try: - motion = Motion.objects.get(pk=int(self.kwargs.get('pk'))) - except Motion.DoesNotExist: - raise Http404('Motion %s not found.' % self.kwargs.get('pk')) - try: - version = MotionVersion.objects.get( - motion=motion, - version_number=int(self.kwargs.get('version_number'))) - except MotionVersion.DoesNotExist: - raise Http404('Version %s not found.' % self.kwargs.get('version_number')) - if version == motion.active_version: - raise Http404('You can not delete the active version of a motion.') + version = self._object + except AttributeError: + try: + motion = Motion.objects.get(pk=int(self.kwargs.get('pk'))) + except Motion.DoesNotExist: + raise Http404('Motion %s not found.' % self.kwargs.get('pk')) + try: + version = MotionVersion.objects.get( + motion=motion, + version_number=int(self.kwargs.get('version_number'))) + except MotionVersion.DoesNotExist: + raise Http404('Version %s not found.' % self.kwargs.get('version_number')) + if version == motion.active_version: + raise Http404('You can not delete the active version of a motion.') + self._object = version return version def get_url_name_args(self): - return (self.object.motion_id, ) + return (self.get_object().motion_id, ) version_delete = VersionDeleteView.as_view() @@ -332,12 +336,11 @@ class VersionPermitView(SingleObjectMixin, QuestionView): def get(self, *args, **kwargs): """ - Set self.object to a motion. + Sets self.version to a motion version. """ - self.object = self.get_object() version_number = self.kwargs.get('version_number', None) try: - self.version = self.object.versions.get(version_number=int(version_number)) + self.version = self.get_object().versions.get(version_number=int(version_number)) except MotionVersion.DoesNotExist: raise Http404('Version %s not found.' % version_number) return super(VersionPermitView, self).get(*args, **kwargs) @@ -346,7 +349,7 @@ class VersionPermitView(SingleObjectMixin, QuestionView): """ Returns a list with arguments to create the success- and question_url. """ - return [self.object.pk, self.version.version_number] + return [self.get_object().pk, self.version.version_number] def get_question_message(self): """ @@ -358,9 +361,9 @@ class VersionPermitView(SingleObjectMixin, QuestionView): """ Activate the version, if the user chooses 'yes'. """ - self.object.active_version = self.version - self.object.save(update_fields=['active_version']) - self.object.write_log( + self.get_object().active_version = self.version + self.get_object().save(update_fields=['active_version']) + self.get_object().write_log( message_list=[ugettext_noop('Version'), ' %d ' % self.version.version_number, ugettext_noop('permitted')], @@ -384,8 +387,8 @@ class VersionDiffView(DetailView): try: rev1 = int(self.request.GET['rev1']) rev2 = int(self.request.GET['rev2']) - version_rev1 = self.object.versions.get(version_number=rev1) - version_rev2 = self.object.versions.get(version_number=rev2) + version_rev1 = self.get_object().versions.get(version_number=rev1) + version_rev2 = self.get_object().versions.get(version_number=rev2) diff_text = htmldiff(version_rev1.text, version_rev2.text) diff_reason = htmldiff(version_rev1.reason, version_rev2.reason) except (KeyError, ValueError, MotionVersion.DoesNotExist): @@ -419,18 +422,11 @@ class SupportView(SingleObjectMixin, QuestionView): model = Motion support = True - def get(self, request, *args, **kwargs): - """ - Set self.object to a motion. - """ - self.object = self.get_object() - return super(SupportView, self).get(request, *args, **kwargs) - def check_permission(self, request): """ Return True if the user can support or unsupport the motion. Else: False. """ - allowed_actions = self.object.get_allowed_actions(request.user) + allowed_actions = self.get_object().get_allowed_actions(request.user) if self.support and not allowed_actions['support']: messages.error(request, _('You can not support this motion.')) return False @@ -459,11 +455,11 @@ class SupportView(SingleObjectMixin, QuestionView): if self.check_permission(self.request): user = self.request.user if self.support: - self.object.support(person=user) - self.object.write_log([ugettext_noop('Motion supported')], user) + self.get_object().support(person=user) + self.get_object().write_log([ugettext_noop('Motion supported')], user) else: - self.object.unsupport(person=user) - self.object.write_log([ugettext_noop('Motion unsupported')], user) + self.get_object().unsupport(person=user) + self.get_object().write_log([ugettext_noop('Motion unsupported')], user) def get_final_message(self): """ @@ -486,26 +482,19 @@ class PollCreateView(SingleObjectMixin, RedirectView): model = Motion url_name = 'motionpoll_detail' - def get(self, request, *args, **kwargs): - """ - Set self.object to a motion. - """ - self.object = self.get_object() - return super(PollCreateView, self).get(request, *args, **kwargs) - def pre_redirect(self, request, *args, **kwargs): """ Create the poll for the motion. """ - self.poll = self.object.create_poll() - self.object.write_log([ugettext_noop("Poll created")], request.user) + self.poll = self.get_object().create_poll() + self.get_object().write_log([ugettext_noop("Poll created")], request.user) messages.success(request, _("New vote was successfully created.")) def get_redirect_url(self, **kwargs): """ Return the URL to the UpdateView of the poll. """ - return reverse('motionpoll_update', args=[self.object.pk, self.poll.poll_number]) + return reverse('motionpoll_update', args=[self.get_object().pk, self.poll.poll_number]) poll_create = PollCreateView.as_view() @@ -525,16 +514,21 @@ class PollMixin(object): Use the motion id and the poll_number from the url kwargs to get the object. """ - queryset = MotionPoll.objects.filter( - motion=self.kwargs['pk'], - poll_number=self.kwargs['poll_number']) - return get_object_or_404(queryset) + try: + obj = self._object + except AttributeError: + queryset = MotionPoll.objects.filter( + motion=self.kwargs['pk'], + poll_number=self.kwargs['poll_number']) + obj = get_object_or_404(queryset) + self._object = obj + return obj def get_url_name_args(self): """ Return the arguments to create the url to the success_url. """ - return [self.object.motion.pk] + return [self.get_object().motion.pk] class PollUpdateView(PollMixin, PollFormView): @@ -566,7 +560,7 @@ class PollUpdateView(PollMixin, PollFormView): Write a log message, if the form is valid. """ value = super(PollUpdateView, self).form_valid(form) - self.object.write_log([ugettext_noop('Poll updated')], self.request.user) + self.get_object().write_log([ugettext_noop('Poll updated')], self.request.user) return value poll_update = PollUpdateView.as_view() @@ -584,13 +578,13 @@ class PollDeleteView(PollMixin, DeleteView): Write a log message, if the form is valid. """ super(PollDeleteView, self).on_clicked_yes() - self.object.motion.write_log([ugettext_noop('Poll deleted')], self.request.user) + self.get_object().motion.write_log([ugettext_noop('Poll deleted')], self.request.user) def get_redirect_url(self, **kwargs): """ Return the URL to the DetailView of the motion. """ - return reverse('motion_detail', args=[self.object.motion.pk]) + return reverse('motion_detail', args=[self.get_object().motion.pk]) poll_delete = PollDeleteView.as_view() @@ -603,15 +597,11 @@ class PollPDFView(PollMixin, PDFView): required_permission = 'motion.can_manage_motion' top_space = 0 - def get(self, *args, **kwargs): - self.object = self.get_object() - return super(PollPDFView, self).get(*args, **kwargs) - def get_filename(self): """ Return the filename for the PDF. """ - return u'%s%s_%s' % (_("Motion"), str(self.object.poll_number), _("Poll")) + return u'%s%s_%s' % (_("Motion"), str(self.get_object().poll_number), _("Poll")) def get_template(self, buffer): return SimpleDocTemplate( @@ -625,7 +615,7 @@ class PollPDFView(PollMixin, PDFView): """ Append PDF objects. """ - motion_poll_to_pdf(pdf, self.object) + motion_poll_to_pdf(pdf, self.get_object()) poll_pdf = PollPDFView.as_view() @@ -646,26 +636,25 @@ class MotionSetStateView(SingleObjectMixin, RedirectView): """ Save the new state and write a log message. """ - self.object = self.get_object() success = False if self.reset: - self.object.reset_state() + self.get_object().reset_state() success = True - elif self.object.state.id == int(kwargs['state']): + elif self.get_object().state.id == int(kwargs['state']): messages.error(request, _('You can not set the state of the motion. It is already done.')) - elif int(kwargs['state']) not in [state.id for state in self.object.state.next_states.all()]: + elif int(kwargs['state']) not in [state.id for state in self.get_object().state.next_states.all()]: messages.error(request, _('You can not set the state of the motion to %s.') % _(State.objects.get(pk=int(kwargs['state'])).name)) else: - self.object.set_state(int(kwargs['state'])) + self.get_object().set_state(int(kwargs['state'])) success = True if success: - self.object.save(update_fields=['state', 'identifier']) - self.object.write_log( - message_list=[ugettext_noop('State changed to'), ' ', self.object.state.name], # TODO: Change string to 'State set to ...' + self.get_object().save(update_fields=['state', 'identifier']) + self.get_object().write_log( + message_list=[ugettext_noop('State changed to'), ' ', self.get_object().state.name], # TODO: Change string to 'State set to ...' person=self.request.user) messages.success(request, _('The state of the motion was set to %s.') - % html_strong(_(self.object.state.name))) + % html_strong(_(self.get_object().state.name))) set_state = MotionSetStateView.as_view() reset_state = MotionSetStateView.as_view(reset=True) @@ -682,7 +671,7 @@ class CreateRelatedAgendaItemView(_CreateRelatedAgendaItemView): Create the agenda item. """ super(CreateRelatedAgendaItemView, self).pre_redirect(request, *args, **kwargs) - self.object.write_log([ugettext_noop('Agenda item created')], self.request.user) + self.get_object().write_log([ugettext_noop('Agenda item created')], self.request.user) create_agenda_item = CreateRelatedAgendaItemView.as_view() @@ -701,13 +690,12 @@ class MotionPDFView(SingleObjectMixin, PDFView): top_space = 0 print_all_motions = False - def get(self, request, *args, **kwargs): - """ - Set self.object to a motion. - """ - if not self.print_all_motions: - self.object = self.get_object() - return super(MotionPDFView, self).get(request, *args, **kwargs) + def get_object(self, *args, **kwargs): + if self.print_all_motions: + obj = None + else: + obj = super(MotionPDFView, self).get_object(*args, **kwargs) + return obj def get_filename(self): """ @@ -716,10 +704,10 @@ class MotionPDFView(SingleObjectMixin, PDFView): if self.print_all_motions: return _("Motions") else: - if self.object.identifier: - suffix = self.object.identifier.replace(' ', '') + if self.get_object().identifier: + suffix = self.get_object().identifier.replace(' ', '') else: - suffix = self.object.title.replace(' ', '_') + suffix = self.get_object().title.replace(' ', '_') suffix = slugify(suffix) return '%s-%s' % (_("Motion"), suffix) @@ -730,7 +718,7 @@ class MotionPDFView(SingleObjectMixin, PDFView): if self.print_all_motions: motions_to_pdf(pdf) else: - motion_to_pdf(pdf, self.object) + motion_to_pdf(pdf, self.get_object()) motion_list_pdf = MotionPDFView.as_view(print_all_motions=True) motion_detail_pdf = MotionPDFView.as_view(print_all_motions=False) diff --git a/openslides/participant/views.py b/openslides/participant/views.py index aa02bbfad..1c7055f0f 100644 --- a/openslides/participant/views.py +++ b/openslides/participant/views.py @@ -162,13 +162,13 @@ class UserDeleteView(DeleteView): url_name_args = [] def pre_redirect(self, request, *args, **kwargs): - if self.object == self.request.user: + if self.get_object() == self.request.user: messages.error(request, _("You can not delete yourself.")) else: super(UserDeleteView, self).pre_redirect(request, *args, **kwargs) def pre_post_redirect(self, request, *args, **kwargs): - if self.object == self.request.user: + if self.get_object() == self.request.user: messages.error(self.request, _("You can not delete yourself.")) else: super(UserDeleteView, self).pre_post_redirect(request, *args, **kwargs) @@ -185,23 +185,22 @@ class SetUserStatusView(SingleObjectMixin, RedirectView): model = User def pre_redirect(self, request, *args, **kwargs): - self.object = self.get_object() action = kwargs['action'] if action == 'activate': - self.object.is_active = True + self.get_object().is_active = True elif action == 'deactivate': - if self.object.user == self.request.user: + if self.get_object().user == self.request.user: messages.error(request, _("You can not deactivate yourself.")) else: - self.object.is_active = False + self.get_object().is_active = False elif action == 'toggle': - self.object.is_active = not self.object.is_active - self.object.save() + self.get_object().is_active = not self.get_object().is_active + self.get_object().save() return super(SetUserStatusView, self).pre_redirect(request, *args, **kwargs) def get_ajax_context(self, **kwargs): context = super(SetUserStatusView, self).get_ajax_context(**kwargs) - context['active'] = self.object.is_active + context['active'] = self.get_object().is_active return context @@ -257,18 +256,14 @@ class ResetPasswordView(SingleObjectMixin, QuestionView): allow_ajax = True question_message = ugettext_lazy('Do you really want to reset the password?') - def get(self, request, *args, **kwargs): - self.object = self.get_object() - return super(ResetPasswordView, self).get(request, *args, **kwargs) - def get_redirect_url(self, **kwargs): - return reverse('user_edit', args=[self.object.id]) + return reverse('user_edit', args=[self.get_object().id]) def on_clicked_yes(self): - self.object.reset_password() + self.get_object().reset_password() def get_final_message(self): - return _('The Password for %s was successfully reset.') % html_strong(self.object) + return _('The Password for %s was successfully reset.') % html_strong(self.get_object()) class GroupOverview(ListView): @@ -361,12 +356,12 @@ class GroupDeleteView(DeleteView): """ Checks whether the group is protected. """ - if self.object.pk in [1, 2]: + if self.get_object().pk in [1, 2]: messages.error(self.request, _('You can not delete this group.')) return True if (not self.request.user.is_superuser and - get_protected_perm() in self.object.permissions.all() and - not Group.objects.exclude(pk=self.object.pk).filter( + get_protected_perm() in self.get_object().permissions.all() and + not Group.objects.exclude(pk=self.get_object().pk).filter( permissions__in=[get_protected_perm()], user__pk=self.request.user.pk).exists()): messages.error( diff --git a/openslides/poll/views.py b/openslides/poll/views.py index 41ee176d5..7bb0bed17 100644 --- a/openslides/poll/views.py +++ b/openslides/poll/views.py @@ -11,11 +11,11 @@ class PollFormView(FormMixin, TemplateView): poll_class = None def get(self, request, *args, **kwargs): - self.poll = self.object = self.get_object() + self.poll = self.get_object() return super(PollFormView, self).get(request, *args, **kwargs) def post(self, request, *args, **kwargs): - self.poll = self.object = self.get_object() + self.poll = self.get_object() option_forms = self.poll.get_vote_forms(data=self.request.POST) FormClass = self.get_modelform_class() @@ -57,8 +57,13 @@ class PollFormView(FormMixin, TemplateView): """ Returns the poll object. Raises Http404 if the poll does not exist. """ - queryset = self.get_poll_class().objects.filter(pk=self.kwargs['poll_id']) - return get_object_or_404(queryset) + try: + obj = self._object + except AttributeError: + queryset = self.get_poll_class().objects.filter(pk=self.kwargs['poll_id']) + obj = get_object_or_404(queryset) + self._object = obj + return obj def get_context_data(self, **kwargs): context = super(PollFormView, self).get_context_data(**kwargs) diff --git a/openslides/utils/views.py b/openslides/utils/views.py index 6bf5b983e..be43de7e0 100644 --- a/openslides/utils/views.py +++ b/openslides/utils/views.py @@ -14,7 +14,6 @@ from django.utils.decorators import method_decorator from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_lazy from django.views import generic as django_views -from django.views.generic.detail import SingleObjectMixin from reportlab.lib.units import cm from reportlab.platypus import SimpleDocTemplate, Spacer @@ -166,6 +165,32 @@ class UrlMixin(object): return value +class SingleObjectMixin(django_views.detail.SingleObjectMixin): + """ + Mixin for single objects from the database. + """ + + def dispatch(self, *args, **kwargs): + if not hasattr(self, 'object'): + # Save the object not only in the cache but in the public + # attribute self.object because Django expects this later. + # Because get_object() has an internal cache this line is not a + # performance problem. + self.object = self.get_object() + return super(SingleObjectMixin, self).dispatch(*args, **kwargs) + + def get_object(self, *args, **kwargs): + """ + Returns the single object from database or cache. + """ + try: + obj = self._object + except AttributeError: + obj = super(SingleObjectMixin, self).get_object(*args, **kwargs) + self._object = obj + return obj + + class FormMixin(UrlMixin): """ Mixin for views with forms. @@ -442,14 +467,14 @@ class FormView(PermissionMixin, ExtraContextMixin, FormMixin, class UpdateView(PermissionMixin, ExtraContextMixin, - ModelFormMixin, django_views.UpdateView): + ModelFormMixin, SingleObjectMixin, django_views.UpdateView): """ View to update an model object. """ def get_success_message(self): if self.success_message is None: - message = _('%s was successfully modified.') % html_strong(self.object) + message = _('%s was successfully modified.') % html_strong(self.get_object()) else: message = self.success_message return message @@ -459,6 +484,10 @@ class CreateView(PermissionMixin, ExtraContextMixin, ModelFormMixin, django_views.CreateView): """ View to create a model object. + + Note: This class has a django method get_object() which is different form + the method in openslides.utils.views.SingleObjectMixin. The result + is not cached. """ def get_success_message(self): @@ -477,10 +506,6 @@ class DeleteView(SingleObjectMixin, QuestionView): success_url = None success_url_name = None - def get(self, request, *args, **kwargs): - self.object = self.get_object() - return super(DeleteView, self).get(request, *args, **kwargs) - def get_redirect_url(self, **kwargs): """ Returns the url on which the delete dialog is shown and the url after @@ -510,22 +535,22 @@ class DeleteView(SingleObjectMixin, QuestionView): """ Returns the question for the delete dialog. """ - return _('Do you really want to delete %s?') % html_strong(self.object) + return _('Do you really want to delete %s?') % html_strong(self.get_object()) def on_clicked_yes(self): """ Deletes the object. """ - self.object.delete() + self.get_object().delete() def get_final_message(self): """ Prints the success message to the user. """ - return _('%s was successfully deleted.') % html_strong(self.object) + return _('%s was successfully deleted.') % html_strong(self.get_object()) -class DetailView(PermissionMixin, ExtraContextMixin, django_views.DetailView): +class DetailView(PermissionMixin, ExtraContextMixin, SingleObjectMixin, django_views.DetailView): """ View to show an model object. """ diff --git a/tests/mediafile/tests.py b/tests/mediafile/tests.py index 507c4691a..315743a2a 100644 --- a/tests/mediafile/tests.py +++ b/tests/mediafile/tests.py @@ -203,8 +203,8 @@ class MediafileTest(TestCase): response = clients['client_normal_user'].get('/mediafile/1/del/') self.assertEqual(response.status_code, 403) bad_client = Client() - response = bad_client.get('/mediafile/2/del/') - self.assertRedirects(response, expected_url='/login/?next=/mediafile/2/del/', status_code=302, target_status_code=200) + response = bad_client.get('/mediafile/1/del/') + self.assertRedirects(response, expected_url='/login/?next=/mediafile/1/del/', status_code=302, target_status_code=200) def test_delete_mediafile_get_request_own_file(self): self.object.uploader = self.vip_user diff --git a/tests/settings.py b/tests/settings.py index f9b58b5ac..d913376e7 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -29,6 +29,7 @@ DATABASES = { # Add OpenSlides plugins to this list INSTALLED_PLUGINS = ( 'tests.person_api', + 'tests.utils', ) INSTALLED_APPS += INSTALLED_PLUGINS diff --git a/tests/utils/models.py b/tests/utils/models.py new file mode 100644 index 000000000..cd5c87cb9 --- /dev/null +++ b/tests/utils/models.py @@ -0,0 +1,8 @@ +from django.db import models + + +class DummyModel(models.Model): + """ + Dummy model to test some model views. + """ + title = models.CharField(max_length=255) diff --git a/tests/utils/templates/utils/dummymodel_detail.html b/tests/utils/templates/utils/dummymodel_detail.html new file mode 100644 index 000000000..1a8a594f6 --- /dev/null +++ b/tests/utils/templates/utils/dummymodel_detail.html @@ -0,0 +1,5 @@ + + + {{ object.title }} + + diff --git a/tests/utils/test_views.py b/tests/utils/test_views.py index fe468538c..8229dbd2b 100644 --- a/tests/utils/test_views.py +++ b/tests/utils/test_views.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import AnonymousUser from django.core.exceptions import ImproperlyConfigured from django.core.urlresolvers import clear_url_caches +from django.db import connection, reset_queries from django.test import RequestFactory from django.test.client import Client from django.test.utils import override_settings @@ -13,6 +14,7 @@ from openslides.utils.signals import template_manipulation from openslides.utils.test import TestCase from . import views as test_views +from .models import DummyModel @override_settings(ROOT_URLCONF='tests.utils.urls') @@ -194,6 +196,17 @@ class QuestionViewTest(ViewTestCase): self.assertIn('the question', question) +class DetailViewTest(ViewTestCase): + def test_get_object_cache(self): + with self.settings(DEBUG=True): + DummyModel.objects.create(title='title_ooth8she7yos1Oi8Boh3') + reset_queries() + client = Client() + response = client.get('/dummy_detail_view/1/') + self.assertContains(response, 'title_ooth8she7yos1Oi8Boh3') + self.assertEqual(len(connection.queries), 3) + + def set_context(sender, request, context, **kwargs): """ receiver for testing the ExtraContextMixin diff --git a/tests/utils/urls.py b/tests/utils/urls.py index c31d30303..181a5db59 100644 --- a/tests/utils/urls.py +++ b/tests/utils/urls.py @@ -28,4 +28,7 @@ urlpatterns += patterns( url(r'^permission_mixin3/$', views.PermissionMixinView.as_view(required_permission='agenda.can_see_agenda')), + + url(r'^dummy_detail_view/(?P\d+)/$', + views.DummyDetailView.as_view()), ) diff --git a/tests/utils/views.py b/tests/utils/views.py index 8af2be1ac..c9e7c4579 100644 --- a/tests/utils/views.py +++ b/tests/utils/views.py @@ -2,6 +2,8 @@ from django.http import HttpResponse from openslides.utils import views +from .models import DummyModel + class GetAbsoluteUrl(object): """ @@ -45,3 +47,15 @@ class UrlMixinView(views.UrlMixin, views.View): class UrlMixinViewWithObject(views.UrlMixin, views.View): object = GetAbsoluteUrl() + + +class DummyDetailView(views.DetailView): + model = DummyModel + + def get_context_data(self, **context): + context = super(DummyDetailView, self).get_context_data(**context) + # Just call get_object() some times to test the cache + self.get_object() + self.get_object() + self.get_object() + return context