From 907b0b6696e99b68bf68afdda2b45fc08fa47ee5 Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Fri, 17 Jul 2020 13:32:30 +0000
Subject: [PATCH] bugfix: infinite loop on remove activity
---
djangoldp/activities/services.py | 29 ++++++++------------
djangoldp/models.py | 4 +--
djangoldp/tests/tests_backlinks_service.py | 1 -
djangoldp/tests/tests_inbox.py | 32 ++++++++++++++++++++++
djangoldp/views.py | 8 ++++--
5 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/djangoldp/activities/services.py b/djangoldp/activities/services.py
index c478eac3..29de522d 100644
--- a/djangoldp/activities/services.py
+++ b/djangoldp/activities/services.py
@@ -15,6 +15,10 @@ import logging
logger = logging.getLogger('djangoldp')
+BACKLINKS_ACTOR = {
+ "type": "Service",
+ "name": "Backlinks Service"
+}
class ActivityPubService(object):
@@ -174,7 +178,7 @@ class ActivityPubService(object):
def _save_sent_activity(cls, activity):
'''Auxiliary function saves a record of parameterised activity'''
payload = bytes(json.dumps(activity), "utf-8")
- local_id = settings.SITE_URL + "outbox/"
+ local_id = settings.SITE_URL + "/outbox/"
obj = ActivityModel.objects.create(local_id=local_id, payload=payload)
obj.aid = Model.absolute_url(obj)
obj.save()
@@ -247,10 +251,7 @@ def check_save_for_backlinks(sender, instance, created, **kwargs):
if len(targets) > 0:
obj = ActivityPubService.build_object_tree(instance)
- actor = {
- "type": "Service",
- "name": "Backlinks Service"
- }
+ actor = BACKLINKS_ACTOR
# Create Activity
if created:
for target in targets:
@@ -276,10 +277,7 @@ def check_delete_for_backlinks(sender, instance, **kwargs):
if len(targets) > 0:
for target in targets:
- ActivityPubService.send_delete_activity({
- "type": "Service",
- "name": "Backlinks Service"
- }, {
+ ActivityPubService.send_delete_activity(BACKLINKS_ACTOR, {
"@id": instance.urlid,
"@type": Model.get_model_rdf_type(sender)
}, target)
@@ -341,10 +339,7 @@ def check_m2m_for_backlinks(sender, instance, action, *args, **kwargs):
}
if action == 'post_add':
for target in targets:
- ActivityPubService.send_add_activity({
- "type": "Service",
- "name": "Backlinks Service"
- }, obj, target)
+ ActivityPubService.send_add_activity(BACKLINKS_ACTOR, obj, target)
inbox = ActivityPubService.discover_inbox(target['@id'])
if not Follower.objects.filter(object=obj['@id'], follower=target['@id']).exists():
Follower.objects.create(object=obj['@id'], inbox=inbox, follower=target['@id'],
@@ -352,10 +347,8 @@ def check_m2m_for_backlinks(sender, instance, action, *args, **kwargs):
elif action == "post_remove" or action == "pre_clear":
for target in targets:
- ActivityPubService.send_remove_activity({
- "type": "Service",
- "name": "Backlinks Service"
- }, obj, target)
+ ActivityPubService.send_remove_activity(BACKLINKS_ACTOR, obj, target)
+ inbox = ActivityPubService.discover_inbox(target['@id'])
for follower in Follower.objects.filter(object=obj['@id'], follower=target['@id'],
- is_backlink=True):
+ inbox=inbox, is_backlink=True):
follower.delete()
diff --git a/djangoldp/models.py b/djangoldp/models.py
index f2907ad0..a8938044 100644
--- a/djangoldp/models.py
+++ b/djangoldp/models.py
@@ -301,9 +301,7 @@ class Activity(Model):
rdf_type = 'as:Activity'
def to_activitystream(self):
- payload = self.payload.decode("utf-8")
- data = json.loads(payload)
- return data
+ return json.loads(self.payload.tobytes())
class Follower(Model):
diff --git a/djangoldp/tests/tests_backlinks_service.py b/djangoldp/tests/tests_backlinks_service.py
index 05dcde6a..d214247b 100644
--- a/djangoldp/tests/tests_backlinks_service.py
+++ b/djangoldp/tests/tests_backlinks_service.py
@@ -81,7 +81,6 @@ class TestsBacklinksService(APITestCase):
@override_settings(SEND_BACKLINKS=True, DISABLE_OUTBOX=True)
def test_local_object_with_external_m2m_delete_parent(self):
- # a local project with three distant users
project = Project.objects.create(description='Test')
external_a = self._get_random_external_user()
project.team.add(external_a)
diff --git a/djangoldp/tests/tests_inbox.py b/djangoldp/tests/tests_inbox.py
index 834228e5..5e937d65 100644
--- a/djangoldp/tests/tests_inbox.py
+++ b/djangoldp/tests/tests_inbox.py
@@ -2,6 +2,7 @@ import json
from django.contrib.auth import get_user_model
from django.conf import settings
from django.db import IntegrityError
+from django.test import override_settings
from rest_framework.test import APIClient, APITestCase
from djangoldp.tests.models import Circle, CircleMember, Project, UserProfile, DateModel, DateChild
from djangoldp.models import Activity, Follower
@@ -153,6 +154,7 @@ class TestsInbox(APITestCase):
self._assert_activity_created(response)
# test sending an add activity when the backlink already exists
+ @override_settings(SEND_BACKLINKS=True, DISABLE_OUTBOX=True)
def test_add_activity_object_already_added(self):
circle = Circle.objects.create(urlid="https://distant.com/circles/1/")
CircleMember.objects.create(urlid="https://distant.com/circle-members/1/", circle=circle, user=self.user)
@@ -170,6 +172,7 @@ class TestsInbox(APITestCase):
}
}
payload = self._get_activity_request_template("Add", obj, self._build_target_from_user(self.user))
+ prior_count = Activity.objects.count()
response = self.client.post('/inbox/',
data=json.dumps(payload),
@@ -184,6 +187,7 @@ class TestsInbox(APITestCase):
self.assertIn("https://distant.com/circles/1/", circles.values_list('urlid', flat=True))
self.assertIn("https://distant.com/circle-members/1/", user_circles.values_list('urlid', flat=True))
self._assert_activity_created(response)
+ self.assertEqual(Activity.objects.count(), prior_count + 1)
# TODO: https://git.startinblox.com/djangoldp-packages/djangoldp/issues/250
def test_add_activity_str_parameter(self):
@@ -315,6 +319,34 @@ class TestsInbox(APITestCase):
self.assertEqual(response.status_code, 201)
self._assert_activity_created(response)
+ @override_settings(SEND_BACKLINKS=True, DISABLE_OUTBOX=True)
+ def test_removing_object_twice(self):
+ project = Project.objects.create(urlid="https://distant.com/projects/1/")
+ self.user.projects.add(project)
+ prior_count = Activity.objects.all().count()
+
+ # remove once via activity
+ obj = {
+ "@type": "hd:project",
+ "@id": "https://distant.com/projects/1/"
+ }
+ payload = self._get_activity_request_template("Remove", obj, origin=self._build_target_from_user(self.user))
+ response = self.client.post('/inbox/',
+ data=json.dumps(payload),
+ content_type='application/ld+json;profile="https://www.w3.org/ns/activitystreams"')
+ self.assertEqual(response.status_code, 201)
+ # received and then sent
+ self.assertEqual(Activity.objects.all().count(), prior_count + 2)
+ prior_count = Activity.objects.all().count()
+
+ # sending remove activity again
+ payload = self._get_activity_request_template("Remove", obj, origin=self._build_target_from_user(self.user))
+ response = self.client.post('/inbox/',
+ data=json.dumps(payload),
+ content_type='application/ld+json;profile="https://www.w3.org/ns/activitystreams"')
+ # just received, did not send
+ self.assertEqual(Activity.objects.all().count(), prior_count + 1)
+
# Delete CircleMember
def test_delete_activity_circle_using_origin(self):
circle = Circle.objects.create(urlid="https://distant.com/circles/1/", allow_create_backlink=False)
diff --git a/djangoldp/views.py b/djangoldp/views.py
index 7c0e7df9..97da3969 100644
--- a/djangoldp/views.py
+++ b/djangoldp/views.py
@@ -190,7 +190,9 @@ class InboxView(APIView):
for field_name, relation_info in target_info.relations.items():
if relation_info.related_model == object_model:
- getattr(target, field_name).add(backlink)
+ attr = getattr(target, field_name)
+ if not attr.filter(urlid=backlink.urlid).exists():
+ attr.add(backlink)
def handle_remove_activity(self, activity, **kwargs):
'''
@@ -215,7 +217,9 @@ class InboxView(APIView):
for field_name, relation_info in origin_info.relations.items():
if relation_info.related_model == object_model:
- getattr(origin, field_name).remove(object_instance)
+ attr = getattr(origin, field_name)
+ if attr.filter(urlid=object_instance.urlid).exists():
+ attr.remove(object_instance)
def handle_create_or_update_activity(self, activity, **kwargs):
'''
--
GitLab