From e982de7c41b3c2da9772201032ef6dc8a0fe8a91 Mon Sep 17 00:00:00 2001 From: Calum Mackervoy <c.mackervoy@gmail.com> Date: Fri, 19 Jun 2020 10:44:27 +0000 Subject: [PATCH] bugfix: backlinks fixes --- djangoldp/activities/services.py | 2 +- .../migrations/0012_auto_20200617_1817.py | 30 ++ djangoldp/models.py | 11 +- djangoldp/tests/models.py | 14 + djangoldp/tests/tests_inbox.py | 477 ++++++------------ djangoldp/tests/tests_user_permissions.py | 6 +- djangoldp/views.py | 61 ++- 7 files changed, 261 insertions(+), 340 deletions(-) create mode 100644 djangoldp/migrations/0012_auto_20200617_1817.py diff --git a/djangoldp/activities/services.py b/djangoldp/activities/services.py index 3a828c02..c3379a58 100644 --- a/djangoldp/activities/services.py +++ b/djangoldp/activities/services.py @@ -339,7 +339,7 @@ def check_m2m_for_backlinks(sender, instance, action, *args, **kwargs): for obj in query_set: condition = Model.is_external(obj) and getattr(obj, 'allow_create_backlink', False) if action == "post_add": - condition = condition and not Model.is_external(instance) + condition = condition and not obj.is_backlink if condition: targets.append({ diff --git a/djangoldp/migrations/0012_auto_20200617_1817.py b/djangoldp/migrations/0012_auto_20200617_1817.py new file mode 100644 index 00000000..9cdd1d72 --- /dev/null +++ b/djangoldp/migrations/0012_auto_20200617_1817.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.29 on 2020-06-17 18:17 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('djangoldp', '0011_auto_20200610_1323'), + ] + + operations = [ + migrations.AlterField( + model_name='activity', + name='is_backlink', + field=models.BooleanField(default=False, help_text='set automatically to indicate the Model is a backlink'), + ), + migrations.AlterField( + model_name='follower', + name='is_backlink', + field=models.BooleanField(default=False, help_text='set automatically to indicate the Model is a backlink'), + ), + migrations.AlterField( + model_name='ldpsource', + name='is_backlink', + field=models.BooleanField(default=False, help_text='set automatically to indicate the Model is a backlink'), + ), + ] diff --git a/djangoldp/models.py b/djangoldp/models.py index f5f2d6a3..fb99c638 100644 --- a/djangoldp/models.py +++ b/djangoldp/models.py @@ -31,8 +31,7 @@ class LDPModelManager(models.Manager): class Model(models.Model): urlid = LDPUrlField(blank=True, null=True, unique=True) - is_backlink = models.BooleanField(default=False, - help_text='(DEPRECIATED) set automatically to indicate the Model is a backlink') + is_backlink = models.BooleanField(default=False, help_text='set automatically to indicate the Model is a backlink') allow_create_backlink = models.BooleanField(default=True, help_text='set to False to disable backlink creation after Model save') objects = LDPModelManager() @@ -171,6 +170,14 @@ class Model(models.Model): @classonlymethod def get_or_create(cls, model, urlid, update=False, **field_tuples): + ''' + gets an object with the passed urlid if it exists, creates it if not + :param model: the model class which the object belongs to + :param update: if set to True the object will be updated with the passed field_tuples + :param field_tuples: kwargs for the model creation/updating + :return: the object, fetched or created + :raises Exception: if the object does not exist, but the data passed is invalid + ''' try: logger.debug('[get_or_create] ' + str(model) + ' backlink ' + str(urlid)) rval = model.objects.get(urlid=urlid) diff --git a/djangoldp/tests/models.py b/djangoldp/tests/models.py index 14edc707..aec55aaf 100644 --- a/djangoldp/tests/models.py +++ b/djangoldp/tests/models.py @@ -226,3 +226,17 @@ class Project(Model): authenticated_perms = ["inherit"] rdf_type = 'hd:project' depth = 1 + + +class DateModel(Model): + value = models.DateField() + + class Meta(Model.Meta): + rdf_type = "hd:date" + + +class DateChild(Model): + parent = models.ForeignKey(DateModel, on_delete=models.CASCADE, related_name='children') + + class Meta(Model.Meta): + rdf_type = 'hd:datechild' diff --git a/djangoldp/tests/tests_inbox.py b/djangoldp/tests/tests_inbox.py index 21df3c5e..532fb676 100644 --- a/djangoldp/tests/tests_inbox.py +++ b/djangoldp/tests/tests_inbox.py @@ -1,7 +1,8 @@ import json from django.contrib.auth import get_user_model +from django.db import IntegrityError from rest_framework.test import APIClient, APITestCase -from djangoldp.tests.models import Circle, CircleMember, Project, UserProfile +from djangoldp.tests.models import Circle, CircleMember, Project, UserProfile, DateModel, DateChild from djangoldp.models import Activity, Follower @@ -9,35 +10,60 @@ class TestsInbox(APITestCase): def setUp(self): self.client = APIClient(enforce_csrf_checks=True) + self.user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', + password='glass onion') + self.profile = UserProfile.objects.create(user=self.user) + + def _get_activity_request_template(self, type, obj, target=None, origin=None): + res = { + "@context": [ + "https://www.w3.org/ns/activitystreams", + {"hd": "http://happy-dev.fr/owl/#"} + ], + "summary": "Something happened", + "type": type, + "actor": { + "type": "Service", + "name": "Backlinks Service", + "inbox": "http://127.0.0.1:8000/inbox/" + }, + "object": obj + } + + if target is not None: + res.update({"target": target}) + + if origin is not None: + res.update({"origin": origin}) + + return res + + def _build_target_from_user(self, user): + return { + "@type": "foaf:user", + "name": user.get_full_name(), + "@id": user.urlid + } + + def _assert_activity_created(self, response, activity_len=1): + '''Auxiliary function asserts that the activity was created and returned correctly''' + activities = Activity.objects.all() + self.assertEquals(len(activities), activity_len) + self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) # # CREATE ACTIVITY # def test_create_activity_circle(self): - # a local user has been set as the owner of a distant circle - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": "A circle was created", - "type": "Create", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { + obj = { "@type": "hd:circle", "@id": "https://distant.com/circles/1/", "owner": { "@type": "foaf:user", - "@id": user.urlid + "@id": self.user.urlid } - } } + payload = self._get_activity_request_template("Create", obj) response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json') @@ -45,43 +71,21 @@ class TestsInbox(APITestCase): # assert that the circle was created and the user associated as owner circles = Circle.objects.all() - activities = Activity.objects.all() self.assertEquals(len(circles), 1) - self.assertEquals(len(activities), 1) self.assertIn("https://distant.com/circles/1/", circles.values_list('urlid', flat=True)) - self.assertEqual(circles[0].owner, user) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self.assertEqual(circles[0].owner, self.user) + self._assert_activity_created(response) # # ADD ACTIVITIES # # project model has a direct many-to-many with User def test_add_activity_project(self): - # a local user has joined a distant project - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " was added to Test Project", - "type": "Add", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { - "@type": "hd:project", - "@id": "https://distant.com/projects/1/" - }, - "target": { - "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid - } + obj = { + "@type": "hd:project", + "@id": "https://distant.com/projects/1/" } + payload = self._get_activity_request_template("Add", obj, self._build_target_from_user(self.user)) response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json') @@ -89,50 +93,28 @@ class TestsInbox(APITestCase): # assert that the project backlink(s) & activity were created projects = Project.objects.all() - user_projects = user.projects.all() - activities = Activity.objects.all() + user_projects = self.user.projects.all() self.assertEquals(len(projects), 1) self.assertEquals(len(user_projects), 1) - self.assertEquals(len(activities), 1) self.assertIn("https://distant.com/projects/1/", projects.values_list('urlid', flat=True)) self.assertIn("https://distant.com/projects/1/", user_projects.values_list('urlid', flat=True)) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # circle model has a many-to-many with user, through an intermediate model def test_add_activity_circle(self): - # a local user has joined a distant circle - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " was added to Test Circle", - "type": "Add", - "actor": { - "type": "Service", - "name": "Backlinks Service" + obj = { + "@type": "hd:circlemember", + "@id": "https://distant.com/circle-members/1/", + "user": { + "@type": "foaf:user", + "@id": self.user.urlid }, - "object": { - "@type": "hd:circlemember", - "@id": "https://distant.com/circle-members/1/", - "user": { - "@type": "foaf:user", - "@id": user.urlid - }, - "circle": { - "@type": "hd:circle", - "@id": "https://distant.com/circles/1/" - } - }, - "target": { - "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid + "circle": { + "@type": "hd:circle", + "@id": "https://distant.com/circles/1/" } } + payload = self._get_activity_request_template("Add", obj, 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"') @@ -140,54 +122,31 @@ class TestsInbox(APITestCase): # assert that the circle backlink(s) & activity were created circles = Circle.objects.all() - user_circles = user.circles.all() - activities = Activity.objects.all() + user_circles = self.user.circles.all() self.assertEquals(len(circles), 1) self.assertEquals(len(user_circles), 1) - self.assertEquals(len(activities), 1) 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.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # test sending an add activity when the backlink already exists def test_add_activity_object_already_added(self): - # a local user has joined a distant circle - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - # ..but the receiver already knows about it circle = Circle.objects.create(urlid="https://distant.com/circles/1/") - CircleMember.objects.create(urlid="https://distant.com/circle-members/1/", circle=circle, user=user) + CircleMember.objects.create(urlid="https://distant.com/circle-members/1/", circle=circle, user=self.user) - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " was added to Test Circle", - "type": "Add", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { - "@type": "hd:circlemember", - "@id": "https://distant.com/circle-members/1/", - "user": { - "@type": "foaf:user", - "@id": user.urlid - }, - "circle": { - "@type": "hd:circle", - "@id": "https://distant.com/circles/1/" - } - }, - "target": { + obj = { + "@type": "hd:circlemember", + "@id": "https://distant.com/circle-members/1/", + "user": { "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid + "@id": self.user.urlid + }, + "circle": { + "@type": "hd:circle", + "@id": "https://distant.com/circles/1/" } } + payload = self._get_activity_request_template("Add", obj, self._build_target_from_user(self.user)) response = self.client.post('/inbox/', data=json.dumps(payload), @@ -196,38 +155,17 @@ class TestsInbox(APITestCase): # assert that the circle backlink(s) & activity were created circles = Circle.objects.all() - user_circles = user.circles.all() - activities = Activity.objects.all() + user_circles = self.user.circles.all() self.assertEquals(len(circles), 1) self.assertEquals(len(user_circles), 1) - self.assertEquals(len(activities), 1) 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.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # TODO: https://git.startinblox.com/djangoldp-packages/djangoldp/issues/250 def test_add_activity_str_parameter(self): - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": "Test was added to Test Circle", - "type": "Add", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": "https://distant.com/somethingunknown/1/", - "target": { - "@type": "foaf:user", - "@id": user.urlid - } - } - + payload = self._get_activity_request_template("Add", "https://distant.com/somethingunknown/1/", + 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"') @@ -235,40 +173,16 @@ class TestsInbox(APITestCase): # error behaviour - unknown model def test_add_activity_unknown(self): - # a local user has joined a distant circle - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " was added to Test Circle", - "type": "Add", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { - "@type": "hd:somethingunknown", - "@id": "https://distant.com/somethingunknown/1/" - }, - "target": { - "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid - } + obj = { + "@type": "hd:somethingunknown", + "@id": "https://distant.com/somethingunknown/1/" } - + payload = self._get_activity_request_template("Add", obj, 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, 404) def test_invalid_activity_missing_actor(self): - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - payload = { "@context": [ "https://www.w3.org/ns/activitystreams", @@ -282,7 +196,7 @@ class TestsInbox(APITestCase): }, "target": { "@type": "foaf:user", - "@id": user.urlid + "@id": self.user.urlid } } @@ -291,39 +205,63 @@ class TestsInbox(APITestCase): content_type='application/ld+json;profile="https://www.w3.org/ns/activitystreams"') self.assertEqual(response.status_code, 400) + # test activity setting unsafe fields in object + def test_unsafe_fields_in_activity(self): + obj = { + "@type": "hd:project", + "@id": "https://distant.com/projects/1/", + "pk": 100, + "id": 100 + } + payload = self._get_activity_request_template("Add", obj, self._build_target_from_user(self.user)) + response = self.client.post('/inbox/', + data=json.dumps(payload), content_type='application/ld+json') + self.assertEqual(response.status_code, 201) + + # assert that the project backlink(s) & activity were created + projects = Project.objects.all() + user_projects = self.user.projects.all() + self.assertEquals(len(projects), 1) + self.assertEquals(len(user_projects), 1) + self.assertIn("https://distant.com/projects/1/", projects.values_list('urlid', flat=True)) + self.assertIn("https://distant.com/projects/1/", user_projects.values_list('urlid', flat=True)) + self._assert_activity_created(response) + backlink = Project.objects.get(urlid="https://distant.com/projects/1/") + self.assertNotEqual(backlink.pk, 100) + + def test_missing_not_null_field_activity(self): + # DateChild must not have a null reference to parent + # and parent must not have a null field 'date', which here is missing + obj = { + "@type": "hd:datechild", + "@id": "https://distant.com/datechilds/1/", + "parent": { + "@type": "hd:date", + "@id": "https://distant.com/dates/1/" + } + } + payload = self._get_activity_request_template("Create", obj) + + response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json') + self.assertEqual(response.status_code, 200) + dates = DateModel.objects.all() + date_children = DateChild.objects.all() + self.assertEqual(len(dates), 0) + self.assertEqual(len(date_children), 0) + # # REMOVE & DELETE ACTIVITIES # # project model has a direct many-to-many with User def test_remove_activity_project_using_origin(self): - # a local user has a distant project attached - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) project = Project.objects.create(urlid="https://distant.com/projects/1/") - user.projects.add(project) + self.user.projects.add(project) - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " removed Test Project", - "type": "Remove", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { - "@type": "hd:project", - "@id": "https://distant.com/projects/1/" - }, - "origin": { - "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid - } + 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"') @@ -331,92 +269,47 @@ class TestsInbox(APITestCase): # assert that the circle backlink(s) were removed & activity were created projects = Project.objects.all() - user_projects = user.projects.all() - activities = Activity.objects.all() + user_projects = self.user.projects.all() self.assertEquals(len(projects), 1) self.assertEquals(len(user_projects), 0) - self.assertEquals(len(activities), 1) self.assertIn("https://distant.com/projects/1/", projects.values_list('urlid', flat=True)) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # TODO: test_remove_activity_project_using_target # error behaviour - project does not exist on user def test_remove_activity_nonexistent_project(self): - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) Project.objects.create(urlid="https://distant.com/projects/1/") - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.get_full_name() + " removed Test Project", - "type": "Remove", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { - "@type": "hd:project", - "@id": "https://distant.com/projects/1/" - }, - "origin": { - "@type": "foaf:user", - "name": user.get_full_name(), - "@id": user.urlid - } + 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) - - # assert that the circle backlink(s) were removed & activity were created - projects = Project.objects.all() - user_projects = user.projects.all() - activities = Activity.objects.all() - self.assertEquals(len(projects), 1) - self.assertEquals(len(user_projects), 0) - self.assertEquals(len(activities), 1) - self.assertIn("https://distant.com/projects/1/", projects.values_list('urlid', flat=True)) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # Delete CircleMember def test_delete_activity_circle_using_origin(self): - # a local user has a distant circle attached - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) circle = Circle.objects.create(urlid="https://distant.com/circles/1/", allow_create_backlink=False) - CircleMember.objects.create(urlid="https://distant.com/circle-members/1/",circle=circle, user=user) + CircleMember.objects.create(urlid="https://distant.com/circle-members/1/",circle=circle, user=self.user) - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": "CircleMember was deleted", - "type": "Delete", - "actor": { - "type": "Service", - "name": "Backlinks Service" + obj = { + "@type": "hd:circlemember", + "@id": "https://distant.com/circle-members/1/", + "user": { + "@type": "foaf:user", + "@id": self.user.urlid }, - "object": { - "@type": "hd:circlemember", - "@id": "https://distant.com/circle-members/1/", - "user": { - "@type": "foaf:user", - "@id": user.urlid - }, - "circle": { - "@type": "hd:circle", - "@id": "https://distant.com/circles/1/" - } + "circle": { + "@type": "hd:circle", + "@id": "https://distant.com/circles/1/" } } - + payload = self._get_activity_request_template("Delete", obj) response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json;profile="https://www.w3.org/ns/activitystreams"') @@ -424,13 +317,11 @@ class TestsInbox(APITestCase): # assert that the CircleMember was deleted and activity was created circles = Circle.objects.all() - user_circles = user.circles.all() - activities = Activity.objects.all() + user_circles = self.user.circles.all() self.assertEquals(len(circles), 1) self.assertEquals(len(user_circles), 0) - self.assertEquals(len(activities), 1) self.assertIn("https://distant.com/circles/1/", circles.values_list('urlid', flat=True)) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # TODO: test_delete_activity_circle_using_target @@ -438,77 +329,42 @@ class TestsInbox(APITestCase): # UPDATE Activities # def test_update_activity_circle(self): - # a local user was set as the owner of a distant circle, but the owner has been changed - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - - circle = Circle.objects.create(urlid="https://distant.com/circles/1/", owner=user) - self.assertEqual(circle.owner, user) + circle = Circle.objects.create(urlid="https://distant.com/circles/1/", owner=self.user) + self.assertEqual(circle.owner, self.user) - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": "A circle was updated", - "type": "Update", - "actor": { - "type": "Service", - "name": "Backlinks Service" - }, - "object": { + obj = { "@type": "hd:circle", "@id": "https://distant.com/circles/1/", "owner": { "@type": "foaf:user", "@id": "https://distant.com/users/1/" } - } } - + payload = self._get_activity_request_template("Update", obj) response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json') self.assertEqual(response.status_code, 201) # assert that the circle was created and the user associated as owner circles = Circle.objects.all() - activities = Activity.objects.all() users = get_user_model().objects.all() self.assertEquals(len(circles), 1) - self.assertEquals(len(activities), 1) self.assertEquals(len(users), 2) distant_user = get_user_model().objects.get(urlid="https://distant.com/users/1/") self.assertIn("https://distant.com/circles/1/", circles.values_list('urlid', flat=True)) self.assertEqual(circles[0].owner, distant_user) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) # # FOLLOW activities # def test_follow_activity(self): - # a local user was set as the owner of a distant circle, but the owner has been changed - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - circle = Circle.objects.create(description='Test Description') - - payload = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - {"hd": "http://happy-dev.fr/owl/#"} - ], - "summary": user.urlid + " followed " + circle.urlid, - "type": "Follow", - "actor": { - "type": "Service", - "name": "Backlinks Service", - "inbox": "http://127.0.0.1:8000/inbox/" - }, - "object": { - "@type": "hd:circle", - "@id": circle.urlid - } + obj = { + "@type": "hd:circle", + "@id": circle.urlid } + payload = self._get_activity_request_template("Follow", obj) response = self.client.post('/inbox/', data=json.dumps(payload), content_type='application/ld+json') @@ -516,19 +372,14 @@ class TestsInbox(APITestCase): # assert that Follower was created with correct values followers = Follower.objects.all() - activities = Activity.objects.all() self.assertEquals(len(followers), 1) - self.assertEquals(len(activities), 1) - self.assertIn(response["Location"], activities.values_list('urlid', flat=True)) + self._assert_activity_created(response) follower = followers[0] self.assertEqual("http://127.0.0.1:8000/inbox/", follower.inbox) self.assertEqual(circle.urlid, follower.object) # test Followers are auto-deleted when the object they're following is deleted def test_follower_auto_delete(self): - user = get_user_model().objects.create(username='john', email='jlennon@beatles.com', password='glass onion') - UserProfile.objects.create(user=user) - circle = Circle.objects.create(description='Test Description') Follower.objects.create(object=circle.urlid, inbox="http://127.0.0.1:8000/inbox/") followers = Follower.objects.all() diff --git a/djangoldp/tests/tests_user_permissions.py b/djangoldp/tests/tests_user_permissions.py index f7c17f16..683b8168 100644 --- a/djangoldp/tests/tests_user_permissions.py +++ b/djangoldp/tests/tests_user_permissions.py @@ -1,10 +1,6 @@ from django.contrib.auth import get_user_model from rest_framework.test import APIClient, APITestCase -from guardian.shortcuts import assign_perm - -from djangoldp.permissions import LDPPermissions -from .models import JobOffer, PermissionlessDummy -from djangoldp.views import LDPViewSet +from .models import JobOffer import json diff --git a/djangoldp/views.py b/djangoldp/views.py index ef571f9a..5f056fd5 100644 --- a/djangoldp/views.py +++ b/djangoldp/views.py @@ -6,6 +6,7 @@ from django.conf.urls import url, include from django.contrib.auth import get_user_model from django.core.exceptions import FieldDoesNotExist from django.core.urlresolvers import get_resolver +from django.db import IntegrityError, transaction from django.http import JsonResponse, Http404 from django.shortcuts import get_object_or_404 from django.utils.decorators import classonlymethod @@ -13,6 +14,7 @@ from django.views import View from pyld import jsonld from rest_framework import status from rest_framework.authentication import SessionAuthentication +from rest_framework.exceptions import ValidationError from rest_framework.permissions import AllowAny from rest_framework.parsers import JSONParser from rest_framework.renderers import JSONRenderer @@ -27,8 +29,10 @@ from djangoldp.permissions import LDPPermissions from djangoldp.filters import LocalObjectOnContainerPathBackend from djangoldp.activities import ActivityPubService, as_activitystream from djangoldp.activities.errors import ActivityStreamDecodeError, ActivityStreamValidationError +import logging +logger = logging.getLogger('djangoldp') get_user_model()._meta.rdf_context = {"get_full_name": "rdfs:label"} @@ -85,16 +89,10 @@ class InboxView(APIView): except ActivityStreamValidationError as e: return Response(str(e), status=status.HTTP_400_BAD_REQUEST) - if activity.type == 'Add': - self.handle_add_activity(activity, **kwargs) - elif activity.type == 'Remove': - self.handle_remove_activity(activity, **kwargs) - elif activity.type == 'Delete': - self.handle_delete_activity(activity, **kwargs) - elif activity.type == 'Create' or activity.type == 'Update': - self.handle_create_or_update_activity(activity, **kwargs) - elif activity.type == 'Follow': - self.handle_follow_activity(activity, **kwargs) + try: + self._handle_activity(activity, **kwargs) + except IntegrityError: + return Response({'Unable to save due to an IntegrityError in the receiver model'}, status=status.HTTP_200_OK) # save the activity and return 201 payload = bytes(json.dumps(activity.to_json()), "utf-8") @@ -107,21 +105,46 @@ class InboxView(APIView): return response - def get_or_create_nested_backlinks(self, object, object_model=None, update=False): + def _handle_activity(self, activity, **kwargs): + if activity.type == 'Add': + self.handle_add_activity(activity, **kwargs) + elif activity.type == 'Remove': + self.handle_remove_activity(activity, **kwargs) + elif activity.type == 'Delete': + self.handle_delete_activity(activity, **kwargs) + elif activity.type == 'Create' or activity.type == 'Update': + self.handle_create_or_update_activity(activity, **kwargs) + elif activity.type == 'Follow': + self.handle_follow_activity(activity, **kwargs) + + def atomic_get_or_create_nested_backlinks(self, obj, object_model=None, update=False): + ''' + a version of get_or_create_nested_backlinks in which all nested backlinks are created, or none of them are + ''' + try: + with transaction.atomic(): + return self._get_or_create_nested_backlinks(obj, object_model, update) + except IntegrityError as e: + logger.error(str(e)) + logger.warning('received a backlink which you were not able to save because of a constraint on the model field.') + raise e + + def _get_or_create_nested_backlinks(self, obj, object_model=None, update=False): ''' recursively deconstructs a tree of nested objects, using get_or_create on each leaf/branch - :param object: Dict representation of the object + :param obj: Dict representation of the object :param object_model: The Model class of the object. Will be discovered if set to None :param update: if True will update retrieved objects with new data + :raises Exception: if get_or_create fails on a branch, the creation will be reversed and the Exception re-thrown ''' # store a list of the object's sub-items if object_model is None: - object_model = Model.get_subclass_with_rdf_type(object['@type']) + object_model = Model.get_subclass_with_rdf_type(obj['@type']) if object_model is None: - raise Http404('unable to store type ' + object['@type'] + ', model with this rdf_type not found') + raise Http404('unable to store type ' + obj['@type'] + ', model with this rdf_type not found') branches = {} - for item in object.items(): + for item in obj.items(): # TODO: parse other data types. Match the key to the field_name if isinstance(item[1], dict): item_value = item[1] @@ -130,11 +153,11 @@ class InboxView(APIView): raise Http404('unable to store type ' + item_value['@type'] + ', model with this rdf_type not found') # push nested object tuple as a branch - backlink = self.get_or_create_nested_backlinks(item_value, item_model) + backlink = self._get_or_create_nested_backlinks(item_value, item_model) branches[item[0]] = backlink # get or create the backlink - return Model.get_or_create(object_model, object['@id'], update=update, **branches) + return Model.get_or_create(object_model, obj['@id'], update=update, **branches) # TODO: a fallback here? Saving the backlink as Object or similar def _get_subclass_with_rdf_type_or_404(self, rdf_type): @@ -157,7 +180,7 @@ class InboxView(APIView): return Response({}, status=status.HTTP_404_NOT_FOUND) # store backlink(s) in database - backlink = self.get_or_create_nested_backlinks(activity.object, object_model) + backlink = self.atomic_get_or_create_nested_backlinks(activity.object, object_model) # add object to target target_info = model_meta.get_field_info(target_model) @@ -196,7 +219,7 @@ class InboxView(APIView): handles Create & Update Activities. See https://www.w3.org/ns/activitystreams ''' object_model = self._get_subclass_with_rdf_type_or_404(activity.object['@type']) - self.get_or_create_nested_backlinks(activity.object, object_model, update=True) + self.atomic_get_or_create_nested_backlinks(activity.object, object_model, update=True) def handle_delete_activity(self, activity, **kwargs): ''' -- GitLab