minor noop refactoring to web._targets() to make it use Target directly

models.py

@@ -338,6 +338,10 @@ class Target(ndb.Model):
     # so that PROTOCOLS is fully populated
     protocol = ndb.StringProperty(choices=[], required=True)
 
+    def __hash__(self):
+        """Override Model and allow hashing so these can be dict keys."""
+        return hash((self.protocol, self.uri))
+
 
 class Object(StringIdModel):
     """An activity or other object, eg actor.

tests/test_models.py

@@ -9,7 +9,7 @@ from oauth_dropins.webutil.testutil import NOW
 # import first so that Fake is defined before URL routes are registered
 from .testutil import Fake, TestCase
 
-from models import AtpNode, Follower, Object, OBJECT_EXPIRE_AGE
+from models import AtpNode, Follower, Object, OBJECT_EXPIRE_AGE, Target
 import protocol
 from protocol import Protocol
 from web import Web
@@ -116,6 +116,13 @@ class ObjectTest(TestCase):
         super().setUp()
         g.user = None
 
+    def test_target_hashable(self):
+        target = Target(protocol='ui', uri='http://foo')
+
+        # just check that these don't crash
+        assert isinstance(id(target), int)
+        {target: 'foo'}
+
     def test_ndb_in_memory_cache_off(self):
         """It has a weird bug that we want to avoid.
 

web.py

@@ -614,20 +614,21 @@ def webmention_task():
             logger.info(msg)
             return msg, 204
 
+    sorted_targets = sorted(targets.items(), key=lambda t: t[0].uri)
     obj.populate(
         status='in progress',
         labels=['user'],
         delivered=[],
         failed=[],
-        undelivered=[Target(uri=uri, protocol=cls.LABEL)
+        undelivered=[t for t, _ in sorted_targets],
-                     for cls, uri in targets.keys()],
     )
+    logger.info(f'Delivering to: {obj.undelivered}')
 
-    logger.info(f'Delivering to: {sorted(obj.undelivered, key=lambda t: t.uri)}')
+    # make copy of undelivered because we modify it below.
-    # make copy of undelivered because we modify it below
+    # sort targets so order is deterministic for tests.
-    # sort targets so order is deterministic for tests
+    for target, orig_obj in sorted_targets:
-    for (protocol, target), orig_obj in sorted(targets.items()):
+        assert target.uri
-        assert target
+        protocol = PROTOCOLS[target.protocol]
 
         if obj.type == 'follow':
             # should be guaranteed by _targets()
@@ -638,19 +639,17 @@ def webmention_task():
         # this is reused later in ActivityPub.send()
         # TODO: find a better way
         obj.orig_obj = orig_obj
-        target_prop = Target(uri=target, protocol=protocol.LABEL)
-
         try:
-            sent = protocol.send(obj, target, log_data=log_data)
+            sent = protocol.send(obj, target.uri, log_data=log_data)
             if sent:
-                obj.delivered.append(target_prop)
+                obj.delivered.append(target)
-                obj.undelivered.remove(target_prop)
+                obj.undelivered.remove(target)
         except BaseException as e:
             code, body = util.interpret_http_exception(e)
             if not code and not body:
                 raise
-            obj.failed.append(target_prop)
+            obj.failed.append(target)
-            obj.undelivered.remove(target_prop)
+            obj.undelivered.remove(target)
             err = e
         finally:
             log_data = False
@@ -682,7 +681,8 @@ def _targets(obj):
       obj: :class:`models.Object`
 
     Returns: dict: {
-      (:class:`Protocol`:, :str: target URI) tuple: recipient :class:`Object` or None
+      :class:`Target`: original (in response to) :class:`Object`, if any,
+      otherwise None
     }
     """
     logger.info('Finding recipients and their targets')
@@ -719,7 +719,7 @@ def _targets(obj):
 
         target = protocol.target_for(orig_obj)
         if target:
-            targets[protocol, target] = orig_obj
+            targets[Target(protocol=protocol.LABEL, uri=target)] = orig_obj
             logger.info(f'Target for {id} is {target}')
             continue
 
@@ -746,6 +746,6 @@ def _targets(obj):
             # HACK: use last target object from above for reposts, which
             # has its resolved id
             obj = orig_obj if verb == 'share' else None
-            targets[user.__class__, target] = obj
+            targets[Target(protocol=user.LABEL, uri=target)] = obj
 
     return targets