bug fixes for Object.clear and get_or_create

be more careful about clearing and preserving as2/mf2/bsky/our_as1 properties

discovered by @jamietanna, thanks for reporting Jamie!

models.py

@@ -530,7 +530,8 @@ def get_or_create(cls, id, **props):
             obj = Object(id=id)
             obj.new = True
 
-        obj.clear()
+        if set(props.keys()) & set(('our_as1', 'as2', 'mf2', 'bsky')):
+            obj.clear()
         obj.populate(**{
             k: v for k, v in props.items()
             if v and not isinstance(getattr(Object, k), ndb.ComputedProperty)
@@ -543,7 +544,7 @@ def get_or_create(cls, id, **props):
 
     def clear(self):
         """Clears all data properties."""
-        for prop in 'as2', 'bsky', 'mf2':
+        for prop in 'our_as1', 'as2', 'bsky', 'mf2':
             val = getattr(self, prop, None)
             if val:
                 logger.warning(f'Wiping out existing {prop}: {json_dumps(val, indent=2)}')

tests/test_activitypub.py

@@ -1130,7 +1130,7 @@ def test_delete_actor_empty_deleted_object(self, _, mock_get, ___):
         mock_get.assert_not_called()
 
     def test_delete_note(self, _, mock_get, ___):
-        obj = Object(id='http://an/obj', as2={})
+        obj = Object(id='http://an/obj')
         obj.put()
 
         mock_get.side_effect = [

tests/test_models.py

@@ -214,6 +214,12 @@ def check(obj1, obj2):
 
         self.assertEqual(3, Object.query().count())
 
+        # if no data property is set, don't clear existing data properties
+        obj7 = Object.get_or_create('biff', as2={'a': 'b'}, mf2={'c': 'd'})
+        Object.get_or_create('biff', users=[ndb.Key(Web, 'me')])
+        self.assert_object('biff', as2={'a': 'b'}, mf2={'c': 'd'},
+                           users=[ndb.Key(Web, 'me')])
+
     def test_activity_changed(self):
         obj = Object()
         self.assertFalse(obj.activity_changed(None))
@@ -412,6 +418,15 @@ def test_as1_from_mf2_uses_url_as_id(self):
         self.assertNotIn('id', obj.as1['actor'])
         self.assertEqual(['c', 'd'], obj.as1['object'])
 
+    def test_clear(self):
+        ab = {'a': 'b'}
+        obj = Object(our_as1=ab, as2=ab, mf2=ab, bsky=ab)
+        obj.clear()
+        self.assertIsNone(obj.our_as1)
+        self.assertIsNone(obj.as2)
+        self.assertIsNone(obj.mf2)
+        self.assertIsNone(obj.bsky)
+
 
 class FollowerTest(TestCase):
 

tests/testutil.py

@@ -228,12 +228,12 @@ def make_user(self, id, cls=Web, **kwargs):
         """Reuse RSA key across Users because generating it is expensive."""
         obj_key = None
 
-        obj_as2 = kwargs.pop('obj_as2', None) or {}
-        obj_mf2 = kwargs.pop('obj_mf2', None) or {}
+        obj_as2 = kwargs.pop('obj_as2', None)
+        obj_mf2 = kwargs.pop('obj_mf2', None)
         obj_id = kwargs.pop('obj_id', None)
         if not obj_id:
-            obj_id = (obj_as2.get('id')
-                      or util.get_url(obj_mf2, 'properties')
+            obj_id = ((obj_as2 or {}).get('id')
+                      or util.get_url((obj_mf2 or {}), 'properties')
                       or str(self.last_make_user_id))
             self.last_make_user_id += 1
         obj_key = Object(id=obj_id, as2=obj_as2, mf2=obj_mf2).put()