From 485b113ae647cefd1bf54004eb66ea46fef9b0c3 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Tue, 3 Dec 2013 13:34:58 -0500 Subject: [PATCH] Fix AC-660, race condition was frequently causing us to not be able to populate the user. This removes some unnecessary checks that were vestigal from an old implementation. Also adds a unit test for verifying user association with an activity stream object --- awx/main/middleware.py | 53 ++++++++++++++----------------- awx/main/tests/activity_stream.py | 28 +++++++++------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/awx/main/middleware.py b/awx/main/middleware.py index 390dc25e91..15ee181a8a 100644 --- a/awx/main/middleware.py +++ b/awx/main/middleware.py @@ -15,8 +15,6 @@ class ActivityStreamMiddleware(object): def __init__(self): self.disp_uid = None - self.isActivityStreamEvent = False - self.finished = False def process_request(self, request): if hasattr(request, 'user') and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated(): @@ -34,34 +32,31 @@ class ActivityStreamMiddleware(object): drf_user = getattr(drf_request, 'user', None) if self.disp_uid is not None: post_save.disconnect(dispatch_uid=self.disp_uid) - self.finished = True - if self.isActivityStreamEvent: - for instance_id in self.instances: - instance = ActivityStream.objects.filter(id=instance_id) - if instance.exists(): - instance = instance[0] - else: - logger.debug("Failed to look up Activity Stream instance for id : " + str(instance_id)) - continue + for instance_id in self.instances: + instance = ActivityStream.objects.filter(id=instance_id) + if instance.exists(): + instance = instance[0] + else: + logger.debug("Failed to look up Activity Stream instance for id : " + str(instance_id)) + continue - if drf_user is not None and drf_user.__class__ != AnonymousUser: - instance.user = drf_user - try: - instance.save() - except IntegrityError, e: - logger.debug("Integrity Error saving Activity Stream instance for id : " + str(instance_id)) - else: - obj1_type_actual = instance.object1_type.split(".")[-1] - if obj1_type_actual in ("InventoryUpdate", "ProjectUpdate", "Job") and instance.id is not None: - instance.delete() + if drf_user is not None and drf_user.__class__ != AnonymousUser: + instance.user = drf_user + try: + instance.save() + except IntegrityError, e: + logger.debug("Integrity Error saving Activity Stream instance for id : " + str(instance_id)) + else: + obj1_type_actual = instance.object1_type.split(".")[-1] + if obj1_type_actual in ("InventoryUpdate", "ProjectUpdate", "Job") and instance.id is not None: + instance.delete() return response def set_actor(self, user, sender, instance, **kwargs): - if not self.finished: - if sender == ActivityStream: - if isinstance(user, User) and instance.user is None: - instance.user = user - else: - if instance.id not in self.instances: - self.isActivityStreamEvent = True - self.instances.append(instance.id) + if sender == ActivityStream: + if isinstance(user, User) and instance.user is None: + instance.user = user + instance.save() + else: + if instance.id not in self.instances: + self.instances.append(instance.id) diff --git a/awx/main/tests/activity_stream.py b/awx/main/tests/activity_stream.py index 3977b911ae..c66bba4c13 100644 --- a/awx/main/tests/activity_stream.py +++ b/awx/main/tests/activity_stream.py @@ -30,26 +30,23 @@ class ActivityStreamTest(BaseTest): def setUp(self): super(ActivityStreamTest, self).setUp() self.setup_users() - self.organization = self.make_organizations(self.normal_django_user, 1)[0] - self.project = self.make_projects(self.normal_django_user, 1)[0] - self.organization.projects.add(self.project) - self.organization.users.add(self.normal_django_user) + self.org_created = self.post(reverse('api:organization_list'), dict(name='test org', description='test descr'), expect=201, auth=self.get_super_credentials()) # def test_get_activity_stream_list(self): - # url = self.collection() + # url = self.collection() - # with self.current_user(self.normal_django_user): - # self.options(url, expect=200) - # self.head(url, expect=200) - # response = self.get(url, expect=200) - # self.check_pagination_and_size(response, 4, previous=None, next=None) + # with self.current_user(self.super_django_user): + # self.options(url, expect=200) + # self.head(url, expect=200) + # response = self.get(url, expect=200) + # self.check_pagination_and_size(response, 1, previous=None, next=None) def test_basic_fields(self): - org_item = self.item(self.organization.id) + org_item = self.item(1) with self.current_user(self.super_django_user): response = self.get(org_item, expect=200) - self.assertEqual(response['object1_id'], self.organization.id) + self.assertEqual(response['object1_id'], self.org_created['id']) self.assertEqual(response['object1_type'], "awx.main.models.organization.Organization") self.assertEqual(response['object2_id'], None) self.assertEqual(response['object2_type'], None) @@ -59,3 +56,10 @@ class ActivityStreamTest(BaseTest): self.assertTrue("summary_fields" in response) self.assertTrue("object1" in response['summary_fields']) self.assertEquals(response['summary_fields']['object1']['base'], "organization") + + def test_changeby_user(self): + org_item = self.item(1) + + with self.current_user(self.super_django_user): + response = self.get(org_item, expect=200) + self.assertEqual(response['summary_fields']['user']['username'], self.super_django_user.username)