From f44e5816420558f65ba5a36ea975c4d89cbc3fe5 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Thu, 27 Apr 2017 10:25:21 +0300 Subject: [PATCH 1/4] set different forms cache for BaseProjectSerializer --- onadata/apps/api/tools.py | 4 +++- onadata/apps/logger/models/xform.py | 3 +++ onadata/apps/viewer/models/data_dictionary.py | 4 +++- onadata/libs/serializers/project_serializer.py | 6 +++--- onadata/libs/utils/cache_tools.py | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index b1fb38f204..6509785556 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -39,7 +39,8 @@ from onadata.apps.viewer.models.export import Export from onadata.apps.viewer.models.parsed_instance import datetime_from_str from onadata.libs.utils.api_export_tools import custom_response_handler -from onadata.libs.utils.cache_tools import safe_delete, PROJ_FORMS_CACHE +from onadata.libs.utils.cache_tools import ( + safe_delete, PROJ_FORMS_CACHE, PROJ_BASE_FORMS_CACHE) from onadata.libs.utils.logger_tools import publish_form from onadata.libs.utils.logger_tools import response_with_mimetype_and_name from onadata.libs.utils.project_utils import set_project_perms_to_xform_async @@ -338,6 +339,7 @@ def id_string_exists_in_account(): if 'formid' in request.data: xform = get_object_or_404(XForm, pk=request.data.get('formid')) safe_delete('{}{}'.format(PROJ_FORMS_CACHE, xform.project.pk)) + safe_delete('{}{}'.format(PROJ_BASE_FORMS_CACHE, xform.project.pk)) if not ManagerRole.user_has_role(request.user, xform): raise exceptions.PermissionDenied(_( "{} has no manager/owner role to the form {}". format( diff --git a/onadata/apps/logger/models/xform.py b/onadata/apps/logger/models/xform.py index db4f0acc5a..05e58a5d56 100644 --- a/onadata/apps/logger/models/xform.py +++ b/onadata/apps/logger/models/xform.py @@ -29,6 +29,7 @@ from onadata.apps.main.models import MetaData from onadata.libs.models.base_model import BaseModel from onadata.libs.utils.cache_tools import (IS_ORG, PROJ_FORMS_CACHE, + PROJ_BASE_FORMS_CACHE, PROJ_NUM_DATASET_CACHE, PROJ_SUB_DATE_CACHE, safe_delete) from onadata.libs.utils.common_tags import (DURATION, KNOWN_MEDIA_TYPES, NOTES, @@ -914,6 +915,7 @@ def update_profile_num_submissions(sender, instance, **kwargs): def set_object_permissions(sender, instance=None, created=False, **kwargs): # clear cache safe_delete('{}{}'.format(PROJ_FORMS_CACHE, instance.project.pk)) + safe_delete('{}{}'.format(PROJ_BASE_FORMS_CACHE, instance.project.pk)) safe_delete('{}{}'.format(IS_ORG, instance.pk)) if created: @@ -943,6 +945,7 @@ def save_project(sender, instance=None, created=False, **kwargs): def xform_post_delete_callback(sender, instance, **kwargs): if instance.project_id: safe_delete('{}{}'.format(PROJ_FORMS_CACHE, instance.project_id)) + safe_delete('{}{}'.format(PROJ_BASE_FORMS_CACHE, instance.project.pk)) safe_delete('{}{}'.format(PROJ_SUB_DATE_CACHE, instance.project_id)) safe_delete('{}{}'.format(PROJ_NUM_DATASET_CACHE, instance.project_id)) diff --git a/onadata/apps/viewer/models/data_dictionary.py b/onadata/apps/viewer/models/data_dictionary.py index b02ae778aa..243978c965 100644 --- a/onadata/apps/viewer/models/data_dictionary.py +++ b/onadata/apps/viewer/models/data_dictionary.py @@ -15,7 +15,8 @@ from onadata.apps.logger.models.xform import XForm from onadata.apps.logger.xform_instance_parser import XLSFormError from onadata.libs.utils.model_tools import set_uuid -from onadata.libs.utils.cache_tools import PROJ_FORMS_CACHE, safe_delete +from onadata.libs.utils.cache_tools import ( + PROJ_FORMS_CACHE, PROJ_BASE_FORMS_CACHE, safe_delete) from onadata.libs.utils.model_tools import get_columns_with_hxl @@ -141,6 +142,7 @@ def set_object_permissions(sender, instance=None, created=False, **kwargs): if instance.project: # clear cache safe_delete('{}{}'.format(PROJ_FORMS_CACHE, instance.project.pk)) + safe_delete('{}{}'.format(PROJ_BASE_FORMS_CACHE, instance.project.pk)) # seems the super is not called, have to get xform from here xform = XForm.objects.get(pk=instance.pk) diff --git a/onadata/libs/serializers/project_serializer.py b/onadata/libs/serializers/project_serializer.py index 581bcc74aa..fe6289a6c2 100644 --- a/onadata/libs/serializers/project_serializer.py +++ b/onadata/libs/serializers/project_serializer.py @@ -19,7 +19,7 @@ from onadata.libs.utils.cache_tools import ( PROJ_FORMS_CACHE, PROJ_NUM_DATASET_CACHE, PROJ_PERM_CACHE, PROJ_SUB_DATE_CACHE, safe_delete, PROJ_TEAM_USERS_CACHE, - PROJECT_LINKED_DATAVIEWS) + PROJECT_LINKED_DATAVIEWS, PROJ_BASE_FORMS_CACHE) from onadata.apps.api.tools import ( get_organization_members_team, get_organization_owners_team) @@ -229,7 +229,7 @@ def get_users(self, obj): @check_obj def get_forms(self, obj): - forms = cache.get('{}{}'.format(PROJ_FORMS_CACHE, obj.pk)) + forms = cache.get('{}{}'.format(PROJ_BASE_FORMS_CACHE, obj.pk)) if forms: return forms @@ -239,7 +239,7 @@ def get_forms(self, obj): xforms, context={'request': request}, many=True ) forms = list(serializer.data) - cache.set('{}{}'.format(PROJ_FORMS_CACHE, obj.pk), forms) + cache.set('{}{}'.format(PROJ_BASE_FORMS_CACHE, obj.pk), forms) return forms diff --git a/onadata/libs/utils/cache_tools.py b/onadata/libs/utils/cache_tools.py index 58e4b1c92c..8f6409f029 100644 --- a/onadata/libs/utils/cache_tools.py +++ b/onadata/libs/utils/cache_tools.py @@ -10,6 +10,7 @@ def safe_delete(key): PROJ_NUM_DATASET_CACHE = 'ps-num_datasets-' PROJ_SUB_DATE_CACHE = 'ps-last_submission_date-' PROJ_FORMS_CACHE = 'ps-project_forms-' +PROJ_BASE_FORMS_CACHE = 'ps-project_base_forms-' # Cache names used in user_profile_serializer IS_ORG = 'ups-is_org-' From c5eb9c22deeaa11606d6687e008b8747fb768ddc Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Thu, 27 Apr 2017 15:33:38 +0300 Subject: [PATCH 2/4] refactor to use get_xform_users_with_perms which uses a less expensive query --- onadata/apps/main/views.py | 8 +++++--- onadata/libs/utils/user_auth.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/onadata/apps/main/views.py b/onadata/apps/main/views.py index f6ffeafbb9..7f7a845e51 100644 --- a/onadata/apps/main/views.py +++ b/onadata/apps/main/views.py @@ -18,7 +18,7 @@ from django.utils.translation import ugettext as _ from django.views.decorators.http import (require_GET, require_http_methods, require_POST) -from guardian.shortcuts import assign_perm, get_users_with_perms, remove_perm +from guardian.shortcuts import assign_perm, remove_perm from rest_framework.authtoken.models import Token from onadata.apps.logger.models import Instance, XForm @@ -51,7 +51,9 @@ from onadata.libs.utils.user_auth import (add_cors_headers, check_and_set_user, check_and_set_user_and_form, get_user_default_project, - get_xform_and_perms, has_permission, + get_xform_and_perms, + get_xform_users_with_perms, + has_permission, helper_auth_helper, set_profile_data) from onadata.libs.utils.viewer_tools import (EnketoError, enketo_url, get_enketo_preview_url, get_form) @@ -356,7 +358,7 @@ def set_xform_owner_data(data, xform, request, username, id_string): data['external_export_form'] = ExternalExportForm() users_with_perms = [] - for perm in get_users_with_perms(xform, attach_perms=True).items(): + for perm in get_xform_users_with_perms(xform).items(): has_perm = [] if 'change_xform' in perm[1]: has_perm.append(_(u"Can Edit")) diff --git a/onadata/libs/utils/user_auth.py b/onadata/libs/utils/user_auth.py index ccf0b28046..cb48e2e11a 100644 --- a/onadata/libs/utils/user_auth.py +++ b/onadata/libs/utils/user_auth.py @@ -121,6 +121,23 @@ def get_xform_and_perms(username, id_string, request): return [xform, is_owner, can_edit, can_view] +def get_xform_users_with_perms(xform): + """Similar to django-guadian's get_users_with_perms here the query makes + use of the xformuserobjectpermission_set to return a dictionary of users + with a list of permissions to the XForm object. The query in use is not as + expensive as the one in use with get_users_with_perms + """ + user_perms = {} + for p in xform.xformuserobjectpermission_set.all().select_related( + 'user', 'permission').only( + 'user', 'permission__codename', 'content_object_id'): + if p.user.username not in user_perms: + user_perms[p.user] = [] + user_perms[p.user].append(p.permission.codename) + + return user_perms + + def helper_auth_helper(request): if request.user and request.user.is_authenticated(): return None From cdecd29d22944b9732a19c14450859cd6d384971 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Thu, 27 Apr 2017 18:48:52 +0300 Subject: [PATCH 3/4] use num_of_submissions field to prevent a recount --- onadata/apps/logger/models/xform.py | 6 +----- onadata/apps/main/templates/profile.html | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/onadata/apps/logger/models/xform.py b/onadata/apps/logger/models/xform.py index 05e58a5d56..8d147ba203 100644 --- a/onadata/apps/logger/models/xform.py +++ b/onadata/apps/logger/models/xform.py @@ -881,11 +881,7 @@ def hash(self): @property def can_be_replaced(self): - if hasattr(self.submission_count, '__call__'): - num_submissions = self.submission_count() - else: - num_submissions = self.submission_count - return num_submissions == 0 + return self.num_of_submissions == 0 @classmethod def public_forms(cls): diff --git a/onadata/apps/main/templates/profile.html b/onadata/apps/main/templates/profile.html index 1b737f4545..7fb4b565c8 100644 --- a/onadata/apps/main/templates/profile.html +++ b/onadata/apps/main/templates/profile.html @@ -91,7 +91,7 @@

{% blocktrans %}Shared Forms & Public Data ({{ num_forms }}){ {% if form.shared_data %} - {% if form.submission_count %} + {% if form.num_of_submissions %}
From e515af14ec157b16aaee5526cab2ddd85e7a2301 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Thu, 27 Apr 2017 18:50:08 +0300 Subject: [PATCH 4/4] limit to xform fields used in the templates --- onadata/apps/main/views.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/onadata/apps/main/views.py b/onadata/apps/main/views.py index 7f7a845e51..feb1352a23 100644 --- a/onadata/apps/main/views.py +++ b/onadata/apps/main/views.py @@ -214,10 +214,18 @@ def set_form(): "/%s" % request.user.username) url = request_url.replace('http://', 'https://') xforms = XForm.objects.filter(user=content_user)\ - .select_related('user') + .select_related('user').only( + 'id', 'id_string', 'downloadable', 'shared', 'shared_data', + 'user__username', 'num_of_submissions', 'title', + 'last_submission_time', 'instances_with_geopoints', + 'encrypted', 'date_created') user_xforms = xforms # forms shared with user - forms_shared_with = get_forms_shared_with_user(content_user) + forms_shared_with = get_forms_shared_with_user(content_user).only( + 'id', 'id_string', 'downloadable', 'shared', 'shared_data', + 'user__username', 'num_of_submissions', 'title', + 'last_submission_time', 'instances_with_geopoints', 'encrypted', + 'date_created') xforms_list = [ { 'id': 'published',