Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User per-role cache key for the organization_profile_viewset #2665

Merged
merged 10 commits into from
Aug 21, 2024
5 changes: 5 additions & 0 deletions onadata/apps/api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
safe_delete,
XFORM_REGENERATE_INSTANCE_JSON_TASK,
)
from onadata.libs.utils.logger_tools import invalidate_organization_cache
from onadata.libs.models.share_project import ShareProject
from onadata.libs.utils.email import ProjectInvitationEmail

Expand Down Expand Up @@ -222,6 +223,8 @@ def add_org_user_and_share_projects_async(
else:
tools.add_org_user_and_share_projects(organization, user, role)

invalidate_organization_cache(organization.user.username)

if email_msg and email_subject and user.email:
send_mail(
email_subject,
Expand All @@ -247,6 +250,8 @@ def remove_org_user_async(org_id, user_id):
else:
tools.remove_user_from_organization(organization, user)

invalidate_organization_cache(organization.user.username)


@app.task(base=ShareProjectBaseTask)
def share_project_async(project_id, username, role, remove=False):
Expand Down
22 changes: 22 additions & 0 deletions onadata/apps/api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
from onadata.apps.logger.models import ProjectInvitation, Instance
from onadata.apps.main.tests.test_base import TestBase
from onadata.libs.permissions import ManagerRole
from onadata.libs.serializers.organization_serializer import OrganizationSerializer
from onadata.libs.utils.user_auth import get_user_default_project
from onadata.libs.utils.email import ProjectInvitationEmail
from onadata.libs.utils.cache_tools import ORG_PROFILE_CACHE

User = get_user_model()

Expand Down Expand Up @@ -120,6 +122,12 @@ def mock_get_full_dict(
instance.refresh_from_db()
self.assertFalse(instance.json)

def set_cache_for_org(org, request):
"""Utility to set org cache"""
org_profile_json = OrganizationSerializer(
org, context={"request": request}
).data
cache.set(f"{ORG_PROFILE_CACHE}{org.user.username}-owner", org_profile_json)

@patch("onadata.apps.api.tasks.tools.add_org_user_and_share_projects")
class AddOrgUserAndShareProjectsAsyncTestCase(TestBase):
Expand All @@ -133,13 +141,20 @@ def setUp(self):
self.org = OrganizationProfile.objects.create(
user=self.org_user, name="Ona Org", creator=alice
)
self.extra = {"HTTP_AUTHORIZATION": f"Token {self.user.auth_token}"}

def test_user_added_to_org(self, mock_add):
"""User is added to organization"""
request = self.factory.get("/", **self.extra)
request.user = self.user
set_cache_for_org(self.org, request)
cache_key = f"{ORG_PROFILE_CACHE}{self.org.user.username}-owner"
self.assertIsNotNone(cache.get(cache_key))
add_org_user_and_share_projects_async.delay(
self.org.pk, self.user.pk, "manager"
)
mock_add.assert_called_once_with(self.org, self.user, "manager")
self.assertEqual(cache.get(cache_key), None)

def test_role_optional(self, mock_add):
"""role param is optional"""
Expand Down Expand Up @@ -227,11 +242,18 @@ def setUp(self):
self.org = OrganizationProfile.objects.create(
user=self.org_user, name="Ona Org", creator=alice
)
self.extra = {"HTTP_AUTHORIZATION": f"Token {self.user.auth_token}"}

def test_user_removed_from_org(self, mock_remove):
"""User is removed from organization"""
request = self.factory.get("/", **self.extra)
request.user = self.user
set_cache_for_org(self.org, request)
cache_key = f"{ORG_PROFILE_CACHE}{self.org.user.username}-owner"
self.assertIsNotNone(cache.get(cache_key))
remove_org_user_async.delay(self.org.pk, self.user.pk)
mock_remove.assert_called_once_with(self.org, self.user)
self.assertEqual(cache.get(cache_key), None)

@patch("onadata.apps.api.tasks.logger.exception")
def test_invalid_org_id(self, mock_log, mock_remove):
Expand Down
35 changes: 17 additions & 18 deletions onadata/apps/api/viewsets/organization_profile_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@
OrganizationMemberSerializer,
)
from onadata.libs.serializers.organization_serializer import OrganizationSerializer
from onadata.libs.utils.cache_tools import ORG_PROFILE_CACHE, safe_delete
from onadata.libs.utils.cache_tools import safe_delete
from onadata.libs.utils.common_tools import merge_dicts
from onadata.libs.utils.logger_tools import get_org_profile_cache_key

BaseViewset = get_baseviewset_class()

Expand Down Expand Up @@ -65,8 +66,7 @@ class OrganizationProfileViewSet(

def retrieve(self, request, *args, **kwargs):
"""Get organization from cache or db"""
username = kwargs.get("user")
cache_key = f"{ORG_PROFILE_CACHE}{username}{request.user.username}"
cache_key = get_org_profile_cache_key(request.user, self.get_object())
cached_org = cache.get(cache_key)
if cached_org:
return Response(cached_org)
Expand All @@ -79,20 +79,21 @@ def create(self, request, *args, **kwargs):
response = super().create(request, *args, **kwargs)
organization = response.data
username = organization.get("org")
cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", organization)
organization_profile = OrganizationProfile.objects.get(user__username=username)
cache_key = get_org_profile_cache_key(request.user, organization_profile)
cache.set(cache_key, organization)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use set_cache_for_org here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrankApiyo I've just realized set_cache_for_org is in test_tasks and not in this file

return response

def destroy(self, request, *args, **kwargs):
"""Clear cache and destroy organization"""
username = kwargs.get("user")
safe_delete(f"{ORG_PROFILE_CACHE}{username}{request.user.username}")
cache_key = get_org_profile_cache_key(request.user, self.get_object())
safe_delete(cache_key)
return super().destroy(request, *args, **kwargs)

def update(self, request, *args, **kwargs):
"""Update org in cache and db"""
username = kwargs.get("user")
response = super().update(request, *args, **kwargs)
cache_key = f"{ORG_PROFILE_CACHE}{username}{request.user.username}"
cache_key = get_org_profile_cache_key(request.user, self.get_object())
cache.set(cache_key, response.data)
return response

Expand All @@ -115,21 +116,19 @@ def members(self, request, *args, **kwargs):

serializer = OrganizationMemberSerializer(data=data)

username = kwargs.get("user")
if serializer.is_valid():
serializer.save()
organization = serializer.validated_data.get("organization")
data = OrganizationSerializer(
organization, context={"request": request}
).data
cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", data)
else:
if not serializer.is_valid():
return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST)

serializer.save()

data = self.serializer_class(
organization, context={"request": request}
).data
# pylint: disable=attribute-defined-outside-init
self.etag_data = json.dumps(data)
resp_status = (
status.HTTP_201_CREATED if request.method == "POST" else status.HTTP_200_OK
status.HTTP_201_CREATED if request.method == "POST"
else status.HTTP_200_OK
)

return Response(status=resp_status, data=serializer.data)
19 changes: 19 additions & 0 deletions onadata/libs/utils/logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,32 @@
from onadata.apps.viewer.models.data_dictionary import DataDictionary
from onadata.apps.viewer.models.parsed_instance import ParsedInstance
from onadata.apps.viewer.signals import process_submission
from onadata.libs.permissions import ROLES_ORDERED, get_role_in_org
from onadata.libs.utils.cache_tools import ORG_PROFILE_CACHE, safe_delete
from onadata.libs.utils.analytics import TrackObjectEvent
from onadata.libs.utils.common_tags import METADATA_FIELDS
from onadata.libs.utils.common_tools import get_uuid, report_exception
from onadata.libs.utils.model_tools import set_uuid, queryset_iterator
from onadata.libs.utils.user_auth import get_user_default_project


def get_org_profile_cache_key(user, organization):
"""Return cache key given user and organization profile"""
if user.is_anonymous:
return f"{ORG_PROFILE_CACHE}{organization.user.username}-anon"
user_role = get_role_in_org(user, organization)
org_username = organization.user.username
return f"{ORG_PROFILE_CACHE}{org_username}-{user_role}"


def invalidate_organization_cache(org_username):
"""Set organization cache to none for all roles"""
for role in ROLES_ORDERED:
key = f"{ORG_PROFILE_CACHE}{org_username}-{role.name}"
safe_delete(key)
safe_delete(f"{ORG_PROFILE_CACHE}{org_username}-anon")


OPEN_ROSA_VERSION_HEADER = "X-OpenRosa-Version"
HTTP_OPEN_ROSA_VERSION_HEADER = "HTTP_X_OPENROSA_VERSION"
OPEN_ROSA_VERSION = "1.0"
Expand Down