fix #1480

make ac_member_removed and ac_member_added work if the action was triggered remotely.
also pass in the "actor" contact so one can know who did this.

python/examples/group_tracking.py

@@ -32,16 +32,16 @@ class GroupTrackingPlugin:
             print("chat member: {}".format(member.addr))
 
     @account_hookimpl
-    def ac_member_added(self, chat, contact, message):
+    def ac_member_added(self, chat, contact, actor, message):
         print("ac_member_added {} to chat {} from {}".format(
-            contact.addr, chat.id, message.get_sender_contact().addr))
+            contact.addr, chat.id, actor or message.get_sender_contact().addr))
         for member in chat.get_contacts():
             print("chat member: {}".format(member.addr))
 
     @account_hookimpl
-    def ac_member_removed(self, chat, contact, message):
+    def ac_member_removed(self, chat, contact, actor, message):
         print("ac_member_removed {} from chat {} by {}".format(
-            contact.addr, chat.id, message.get_sender_contact().addr))
+            contact.addr, chat.id, actor or message.get_sender_contact().addr))
 
 
 def main(argv=None):

python/examples/test_examples.py

@@ -69,11 +69,11 @@ def test_group_tracking_plugin(acfactory, lp):
 
     lp.sec("now looking at what the bot received")
     botproc.fnmatch_lines("""
-        *ac_member_added {}*
+        *ac_member_added {}*from*{}*
-    """.format(contact3.addr))
+    """.format(contact3.addr, ac1.get_config("addr")))
 
     lp.sec("contact successfully added, now removing")
     ch.remove_contact(contact3)
     botproc.fnmatch_lines("""
-        *ac_member_removed {}*
+        *ac_member_removed {}*from*{}*
-    """.format(contact3.addr))
+    """.format(contact3.addr, ac1.get_config("addr")))

python/src/deltachat/hookspec.py

@@ -16,7 +16,7 @@ class PerAccount:
     """ per-Account-instance hook specifications.
 
     All hooks are executed in a dedicated Event thread.
-    Hooks are not allowed to block/last long as this
+    Hooks are generally not allowed to block/last long as this
     blocks overall event processing on the python side.
     """
     @classmethod
@@ -31,10 +31,6 @@ class PerAccount:
 
         ffi_event has "name", "data1", "data2" values as specified
         with `DC_EVENT_* <https://c.delta.chat/group__DC__EVENT.html>`_.
-
-        DANGER: this hook is executed from the callback invoked by core.
-        Hook implementations need to be short running and can typically
-        not call back into core because this would easily cause recursion issues.
         """
 
     @account_hookspec
@@ -55,19 +51,37 @@ class PerAccount:
 
     @account_hookspec
     def ac_message_delivered(self, message):
-        """ Called when an outgoing message has been delivered to SMTP. """
+        """ Called when an outgoing message has been delivered to SMTP.
+
+        :param message: Message that was just delivered.
+        """
 
     @account_hookspec
     def ac_chat_modified(self, chat):
-        """ Chat was created or modified regarding membership, avatar, title. """
+        """ Chat was created or modified regarding membership, avatar, title.
+
+        :param chat: Chat which was modified.
+        """
 
     @account_hookspec
-    def ac_member_added(self, chat, contact, message):
+    def ac_member_added(self, chat, contact, actor, message):
-        """ Called for each contact added to an accepted chat. """
+        """ Called for each contact added to an accepted chat.
+
+        :param chat: Chat where contact was added.
+        :param contact: Contact that was added.
+        :param actor: Who added the contact (None if it was our self-addr)
+        :param message: The original system message that reports the addition.
+        """
 
     @account_hookspec
-    def ac_member_removed(self, chat, contact, message):
+    def ac_member_removed(self, chat, contact, actor, message):
-        """ Called for each contact removed from a chat.  """
+        """ Called for each contact removed from a chat.
+
+        :param chat: Chat where contact was removed.
+        :param contact: Contact that was removed.
+        :param actor: Who removed the contact (None if it was our self-addr)
+        :param message: The original system message that reports the removal.
+        """
 
 
 class Global:

python/src/deltachat/message.py

@@ -1,6 +1,7 @@
 """ The Message object. """
 
 import os
+import re
 from . import props
 from .cutil import from_dc_charpointer, as_dc_charpointer
 from .capi import lib, ffi
@@ -356,20 +357,37 @@ def get_viewtype_code_from_name(view_type_name):
 def map_system_message(msg):
     if msg.is_system_message():
         res = parse_system_add_remove(msg.text)
-        if res:
+        if not res:
-            contact = msg.account.get_contact_by_addr(res[1])
+            return
-            if contact:
+        action, affected, actor = res
-                d = dict(chat=msg.chat, contact=contact, message=msg)
+        affected = msg.account.get_contact_by_addr(affected)
+        if actor == "me":
+            actor = None
+        else:
+            actor = msg.account.get_contact_by_addr(actor)
+        d = dict(chat=msg.chat, contact=affected, actor=actor, message=msg)
         return "ac_member_" + res[0], d
 
 
+def extract_addr(text):
+    m = re.match(r'.*\((.+@.+)\)', text)
+    if m:
+        text = m.group(1)
+    text = text.rstrip(".")
+    return text
+
+
 def parse_system_add_remove(text):
+    """ return add/remove info from parsing the given system message text.
+
+    returns a (action, affected, actor) triple """
+
     # Member Me (x@y) removed by a@b.
-    # Member x@y removed by a@b
+    # Member x@y added by a@b
+    # Member With space (tmp1@x.org) removed by tmp2@x.org.
+    # Member With space (tmp1@x.org) removed by Another member (tmp2@x.org).",
     text = text.lower()
-    parts = text.split()
+    m = re.match(r'member (.+) (removed|added) by (.+)', text)
-    if parts[0] == "member":
+    if m:
-        if parts[2] in ("removed", "added"):
+        affected, action, actor = m.groups()
-            return parts[2], parts[1]
+        return action, extract_addr(affected), extract_addr(actor)
-        if parts[3] in ("removed", "added"):
-            return parts[3], parts[2].strip("()")

python/tests/test_account.py

@@ -11,8 +11,17 @@ from datetime import datetime, timedelta
 
 
 @pytest.mark.parametrize("msgtext,res", [
-    ("Member Me (tmp1@x.org) removed by tmp2@x.org.", ("removed", "tmp1@x.org")),
+    ("Member Me (tmp1@x.org) removed by tmp2@x.org.",
-    ("Member tmp1@x.org added by tmp2@x.org.", ("added", "tmp1@x.org")),
+        ("removed", "tmp1@x.org", "tmp2@x.org")),
+    ("Member With space (tmp1@x.org) removed by tmp2@x.org.",
+        ("removed", "tmp1@x.org", "tmp2@x.org")),
+    ("Member With space (tmp1@x.org) removed by Another member (tmp2@x.org).",
+        ("removed", "tmp1@x.org", "tmp2@x.org")),
+    ("Member With space (tmp1@x.org) removed by me",
+        ("removed", "tmp1@x.org", "me")),
+    ("Member tmp1@x.org added by tmp2@x.org.", ("added", "tmp1@x.org", "tmp2@x.org")),
+    ("Member nothing bla bla", None),
+    ("Another unknown system message", None),
 ])
 def test_parse_system_add_remove(msgtext, res):
     from deltachat.message import parse_system_add_remove
@@ -452,12 +461,12 @@ class TestOfflineChat:
 
         class InPlugin:
             @account_hookimpl
-            def ac_member_added(self, chat, contact):
+            def ac_member_added(self, chat, contact, actor):
-                in_list.append(("added", chat, contact))
+                in_list.append(("added", chat, contact, actor))
 
             @account_hookimpl
-            def ac_member_removed(self, chat, contact):
+            def ac_member_removed(self, chat, contact, actor):
-                in_list.append(("removed", chat, contact))
+                in_list.append(("removed", chat, contact, actor))
 
         ac1.add_account_plugin(InPlugin())
 
@@ -486,10 +495,11 @@ class TestOfflineChat:
 
         assert len(in_list) == 10
         chat_contacts = chat.get_contacts()
-        for in_cmd, in_chat, in_contact in in_list:
+        for in_cmd, in_chat, in_contact, in_actor in in_list:
             assert in_cmd == "added"
             assert in_chat == chat
             assert in_contact in chat_contacts
+            assert in_actor is None
             chat_contacts.remove(in_contact)
 
         assert chat_contacts[0].id == 1  # self contact
@@ -1624,6 +1634,7 @@ class TestOnlineAccount:
 
         lp.sec("ac2: set ephemeral timer to 0")
         chat2.set_ephemeral_timer(0)
+        ac2._evtracker.get_matching("DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED")
 
         lp.sec("ac1: receive system message about ephemeral timer modification")
         ac1._evtracker.get_matching("DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED")