tests/templates: Rework ImplRequest

The portal backend mocks would all implement the same pattern by copy
and pasting a bunch of code which is not that easy to understand and
also subtly broken because they would often not unexport the request
object.

This changes the ImplRequest to own the control flow and do all the
right things in a single place and provides callbacks for more advanced
use cases.

tests/templates/__init__.py

@@ -3,6 +3,7 @@
 # This file is formatted with Python Black
 
 from typing import Callable, Dict, Optional, NamedTuple
+from gi.repository import GLib
 import dbus
 import dbusmock
 import logging
@@ -35,62 +36,113 @@ class Response(NamedTuple):
 
 class ImplRequest:
     """
-    Implementation of a org.freedesktop.impl.portal.Request object. Typically
-    this object needs to be merely exported:
+    Implementation of an ``org.freedesktop.impl.portal.Request`` object exposed
+    on the object path ``handle``.
 
-        >>> r = ImplRequest(mock, "org.freedesktop.impl.portal.Test", handle)
-        >>> r.export()
-
-    Where the test or the backend implementation relies on the Closed() method
-    of the ImplRequest, provide a callback to be invoked.
-
-        >>> r.export(close_callback=my_callback)
-
-    Note that the latter only works if the test invokes methods
-    asynchronously.
-
-    .. attribute:: closed
-
-        Set to True if the Close() method on the Request was invoked
+    The dbus method implementations need to be invoked asynchronously and the
+    async callbacks must be passed in ``cb_success`` and ``cb_error``.
 
+    The request either waits until it is closed by x-d-p (``wait_for_close``) or
+    responds to the request (``respond``).
     """
 
-    def __init__(self, mock: "dbusmock.DBusMockObject", busname: str, handle: str):
+    def __init__(
+        self,
+        mock: dbusmock.DBusMockObject,
+        busname: str,
+        handle: str,
+        logger: logging.Logger,
+        cb_success: Callable,
+        cb_error: Callable,
+    ):
         self.mock = mock
-        self.handle = handle
-        self.closed = False
-        self._close_callback: Optional[Callable] = None
-
         bus = mock.connection
         proxy = bus.get_object(busname, handle)
-        mock_interface = dbus.Interface(proxy, dbusmock.MOCK_IFACE)
+        self.mock_interface = dbus.Interface(proxy, dbusmock.MOCK_IFACE)
+        self.handle = handle
+        self.logger = logger
+        self.cb_success = cb_success
+        self.cb_error = cb_error
+
+    def respond(
+        self,
+        response: Callable | Response,
+        delay: int = 0,
+        done_cb: Callable | None = None,
+    ) -> None:
+        def reply():
+            nonlocal response
+            logger.debug(f"Request {self.handle}: trying to reply")
+            if callable(response):
+                try:
+                    response = response()
+                except Exception as e:
+                    logger.critical(
+                        f"Request {self.handle}: failed getting response: {e}"
+                    )
+                    self.cb_error(e)
+                    self._unexport()
+                    return
+
+            assert response
+            logger.debug(f"Request {self.handle}: replying {response}")
+
+            self.cb_success(response.response, response.results)
+            self._unexport()
+
+            if done_cb:
+                done_cb()
+
+        self._export()
+
+        if delay > 0:
+            logger.debug(f"Request {self.handle}: scheduling delay of {delay}ms")
+            GLib.timeout_add(delay, reply)
+        else:
+            reply()
+
+    def wait_for_close(
+        self,
+        close_callback: Callable | None = None,
+    ) -> None:
+        def closed():
+            logger.debug(f"Request {self.handle}: closed")
+
+            self.mock.EmitSignal(
+                "org.freedesktop.impl.portal.Mock",
+                "RequestClosed",
+                "s",
+                (self.handle,),
+            )
+
+            if close_callback:
+                try:
+                    close_callback()
+                except Exception as e:
+                    logger.critical(
+                        f"Request {self.handle}: failed running close callback: {e}"
+                    )
+                    self.cb_error(e)
+                    self._unexport()
+                    return
+
+            response = Response(2, {})
+            self.cb_success(response.response, response.results)
+            self._unexport()
 
-        # Register for the Close() call on the impl.Request. If it gets
-        # called, use the side-channel RequestClosed signal so we can notify
-        # the test that the impl.Request was actually closed by the
-        # xdg-desktop-portal
         def cb_methodcall(name, args):
             if name == "Close":
-                self.closed = True
-                logger.debug(f"Close() on {self}")
-                if self._close_callback:
-                    self._close_callback()
-                self.mock.EmitSignal(
-                    "org.freedesktop.impl.portal.Mock",
-                    "RequestClosed",
-                    "s",
-                    (self.handle,),
-                )
-                self.mock.RemoveObject(self.handle)
+                closed()
 
-        mock_interface.connect_to_signal("MethodCalled", cb_methodcall)
+        self._export()
 
-    def export(self, close_callback: Optional[Callable] = None):
-        """
-        Create the object on the bus. If close_callback is not None, that
-        callback will be invoked in response to the Close() method called on
-        this object.
-        """
+        logger.debug(f"Request {self.handle}: waiting for x-d-p to call close")
+        self.mock_interface.connect_to_signal("MethodCalled", cb_methodcall)
+
+    def _export(self):
+        # In the future we can pass a class extending
+        # dbusmock.mockobject.DBusMockObject as mock_class to avoid going
+        # through the mock MethodCalled signal
         self.mock.AddObject(
             path=self.handle,
             interface="org.freedesktop.impl.portal.Request",
@@ -104,10 +156,8 @@ def export(self, close_callback: Optional[Callable] = None):
                 )
             ],
         )
-        self._close_callback = close_callback
-        return self
 
-    def unexport(self):
+    def _unexport(self):
         self.mock.RemoveObject(self.handle)
 
     def __str__(self):

tests/templates/access.py

@@ -5,7 +5,6 @@
 from tests.templates import Response, init_logger, ImplRequest
 
 import dbus.service
-from gi.repository import GLib
 
 
 BUS_NAME = "org.freedesktop.impl.portal.Test"
@@ -43,30 +42,20 @@ def AccessDialog(
     cb_success,
     cb_error,
 ):
-    try:
-        logger.debug(
-            f"AccessDialog({handle}, {app_id}, {parent_window}, {title}, {subtitle}, {body}, {options})"
-        )
-
-        def closed_callback():
-            response = Response(2, {})
-            logger.debug(f"AccessDialog Close() response {response}")
-            cb_success(response.response, response.results)
-
-        def reply_callback(request):
-            response = Response(self.response, {})
-            logger.debug(f"AccessDialog with response {response}")
-            request.unexport()
-            cb_success(response.response, response.results)
-
-        request = ImplRequest(self, BUS_NAME, handle)
-        if self.expect_close:
-            request.export(closed_callback)
-        else:
-            request.export()
-
-            logger.debug(f"scheduling delay of {self.delay}")
-            GLib.timeout_add(self.delay, reply_callback, request)
-    except Exception as e:
-        logger.critical(e)
-        cb_error(e)
+    logger.debug(
+        f"AccessDialog({handle}, {app_id}, {parent_window}, {title}, {subtitle}, {body}, {options})"
+    )
+
+    request = ImplRequest(
+        self,
+        BUS_NAME,
+        handle,
+        logger,
+        cb_success,
+        cb_error,
+    )
+
+    if self.expect_close:
+        request.wait_for_close()
+    else:
+        request.respond(Response(self.response, {}), delay=self.delay)

tests/templates/account.py

@@ -5,7 +5,6 @@
 from tests.templates import Response, init_logger, ImplRequest
 import dbus.service
 import dbus
-from gi.repository import GLib
 
 BUS_NAME = "org.freedesktop.impl.portal.Test"
 MAIN_OBJ = "/org/freedesktop/portal/desktop"
@@ -31,27 +30,18 @@ def load(mock, parameters={}):
     async_callbacks=("cb_success", "cb_error"),
 )
 def GetUserInformation(self, handle, app_id, window, options, cb_success, cb_error):
-    try:
-        logger.debug(f"GetUserInformation({handle}, {app_id}, {window}, {options})")
-
-        def closed_callback():
-            response = Response(2, {})
-            logger.debug(f"GetUserInformation Close() response {response}")
-            cb_success(response.response, response.results)
-
-        def reply_callback():
-            response = Response(self.response, self.results)
-            logger.debug(f"GetUserInformation with response {response}")
-            cb_success(response.response, response.results)
-
-        request = ImplRequest(self, BUS_NAME, handle)
-        if self.expect_close:
-            request.export(closed_callback)
-        else:
-            request.export()
-
-            logger.debug(f"scheduling delay of {self.delay}")
-            GLib.timeout_add(self.delay, reply_callback)
-    except Exception as e:
-        logger.critical(e)
-        cb_error(e)
+    logger.debug(f"GetUserInformation({handle}, {app_id}, {window}, {options})")
+
+    request = ImplRequest(
+        self,
+        BUS_NAME,
+        handle,
+        logger,
+        cb_success,
+        cb_error,
+    )
+
+    if self.expect_close:
+        request.wait_for_close()
+    else:
+        request.respond(Response(self.response, self.results), delay=self.delay)

tests/templates/appchooser.py

@@ -5,7 +5,6 @@
 from tests.templates import Response, init_logger, ImplRequest
 
 import dbus.service
-from gi.repository import GLib
 
 
 BUS_NAME = "org.freedesktop.impl.portal.Test"
@@ -43,32 +42,23 @@ def load(mock, parameters={}):
 def ChooseApplication(
     self, handle, app_id, parent_window, choices, options, cb_success, cb_error
 ):
-    try:
-        logger.debug(
-            f"ChooseApplication({handle}, {app_id}, {parent_window}, {options})"
-        )
-
-        def closed_callback():
-            response = Response(2, {})
-            logger.debug(f"ChooseApplication Close() response {response}")
-            cb_success(response.response, response.results)
-
-        def reply_callback():
-            response = Response(self.response, {})
-            logger.debug(f"ChooseApplication with response {response}")
-            cb_success(response.response, response.results)
+    logger.debug(
+        f"ChooseApplication({handle}, {app_id}, {parent_window}, {choices}, {options})"
+    )
 
-        request = ImplRequest(self, BUS_NAME, handle)
-        if self.expect_close:
-            request.export(closed_callback)
-        else:
-            request.export()
+    request = ImplRequest(
+        self,
+        BUS_NAME,
+        handle,
+        logger,
+        cb_success,
+        cb_error,
+    )
 
-            logger.debug(f"scheduling delay of {self.delay}")
-            GLib.timeout_add(self.delay, reply_callback)
-    except Exception as e:
-        logger.critical(e)
-        cb_error(e)
+    if self.expect_close:
+        request.wait_for_close()
+    else:
+        request.respond(Response(self.response, {}), delay=self.delay)
 
 
 @dbus.service.method(