Remove support for legacy application service paths (#15964)

changelog.d/15964.removal

@@ -0,0 +1 @@
+Remove support for legacy application service paths.

synapse/appservice/api.py

@@ -17,8 +17,6 @@ import urllib.parse
 from typing import (
     TYPE_CHECKING,
     Any,
-    Awaitable,
-    Callable,
     Dict,
     Iterable,
     List,
@@ -30,7 +28,7 @@ from typing import (
 )
 
 from prometheus_client import Counter
-from typing_extensions import Concatenate, ParamSpec, TypeGuard
+from typing_extensions import ParamSpec, TypeGuard
 
 from synapse.api.constants import EventTypes, Membership, ThirdPartyEntityKind
 from synapse.api.errors import CodeMessageException, HttpResponseException
@@ -80,9 +78,7 @@ sent_todevice_counter = Counter(
 
 HOUR_IN_MS = 60 * 60 * 1000
 
-
 APP_SERVICE_PREFIX = "/_matrix/app/v1"
-APP_SERVICE_UNSTABLE_PREFIX = "/_matrix/app/unstable"
 
 P = ParamSpec("P")
 R = TypeVar("R")
@@ -128,47 +124,6 @@ class ApplicationServiceApi(SimpleHttpClient):
             hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS
         )
 
-    async def _send_with_fallbacks(
-        self,
-        service: "ApplicationService",
-        prefixes: List[str],
-        path: str,
-        func: Callable[Concatenate[str, P], Awaitable[R]],
-        *args: P.args,
-        **kwargs: P.kwargs,
-    ) -> R:
-        """
-        Attempt to call an application service with multiple paths, falling back
-        until one succeeds.
-
-        Args:
-            service: The appliacation service, this provides the base URL.
-            prefixes: A last of paths to try in order for the requests.
-            path: A suffix to append to each prefix.
-            func: The function to call, the first argument will be the full
-                endpoint to fetch. Other arguments are provided by args/kwargs.
-
-        Returns:
-            The return value of func.
-        """
-        for i, prefix in enumerate(prefixes, start=1):
-            uri = f"{service.url}{prefix}{path}"
-            try:
-                return await func(uri, *args, **kwargs)
-            except HttpResponseException as e:
-                # If an error is received that is due to an unrecognised path,
-                # fallback to next path (if one exists). Otherwise, consider it
-                # a legitimate error and raise.
-                if i < len(prefixes) and is_unknown_endpoint(e):
-                    continue
-                raise
-            except Exception:
-                # Unexpected exceptions get sent to the caller.
-                raise
-
-        # The function should always exit via the return or raise above this.
-        raise RuntimeError("Unexpected fallback behaviour. This should never be seen.")
-
     async def query_user(self, service: "ApplicationService", user_id: str) -> bool:
         if service.url is None:
             return False
@@ -177,11 +132,8 @@ class ApplicationServiceApi(SimpleHttpClient):
         assert service.hs_token is not None
 
         try:
-            response = await self._send_with_fallbacks(
+            response = await self.get_json(
-                service,
+                f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}",
-                [APP_SERVICE_PREFIX, ""],
-                f"/users/{urllib.parse.quote(user_id)}",
-                self.get_json,
                 {"access_token": service.hs_token},
                 headers={"Authorization": [f"Bearer {service.hs_token}"]},
             )
@@ -203,11 +155,8 @@ class ApplicationServiceApi(SimpleHttpClient):
         assert service.hs_token is not None
 
         try:
-            response = await self._send_with_fallbacks(
+            response = await self.get_json(
-                service,
+                f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}",
-                [APP_SERVICE_PREFIX, ""],
-                f"/rooms/{urllib.parse.quote(alias)}",
-                self.get_json,
                 {"access_token": service.hs_token},
                 headers={"Authorization": [f"Bearer {service.hs_token}"]},
             )
@@ -245,11 +194,8 @@ class ApplicationServiceApi(SimpleHttpClient):
                 **fields,
                 b"access_token": service.hs_token,
             }
-            response = await self._send_with_fallbacks(
+            response = await self.get_json(
-                service,
+                f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
-                [APP_SERVICE_PREFIX, APP_SERVICE_UNSTABLE_PREFIX],
-                f"/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
-                self.get_json,
                 args=args,
                 headers={"Authorization": [f"Bearer {service.hs_token}"]},
             )
@@ -285,11 +231,8 @@ class ApplicationServiceApi(SimpleHttpClient):
             # This is required by the configuration.
             assert service.hs_token is not None
             try:
-                info = await self._send_with_fallbacks(
+                info = await self.get_json(
-                    service,
+                    f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}",
-                    [APP_SERVICE_PREFIX, APP_SERVICE_UNSTABLE_PREFIX],
-                    f"/thirdparty/protocol/{urllib.parse.quote(protocol)}",
-                    self.get_json,
                     {"access_token": service.hs_token},
                     headers={"Authorization": [f"Bearer {service.hs_token}"]},
                 )
@@ -401,11 +344,8 @@ class ApplicationServiceApi(SimpleHttpClient):
                 }
 
         try:
-            await self._send_with_fallbacks(
+            await self.put_json(
-                service,
+                f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}",
-                [APP_SERVICE_PREFIX, ""],
-                f"/transactions/{urllib.parse.quote(str(txn_id))}",
-                self.put_json,
                 json_body=body,
                 args={"access_token": service.hs_token},
                 headers={"Authorization": [f"Bearer {service.hs_token}"]},

tests/appservice/test_api.py

@@ -16,7 +16,6 @@ from unittest.mock import Mock
 
 from twisted.test.proto_helpers import MemoryReactor
 
-from synapse.api.errors import HttpResponseException
 from synapse.appservice import ApplicationService
 from synapse.server import HomeServer
 from synapse.types import JsonDict
@@ -107,58 +106,6 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
         self.assertEqual(self.request_url, URL_LOCATION)
         self.assertEqual(result, SUCCESS_RESULT_LOCATION)
 
-    def test_fallback(self) -> None:
-        """
-        Tests that the fallback to legacy URLs works.
-        """
-        SUCCESS_RESULT_USER = [
-            {
-                "protocol": PROTOCOL,
-                "userid": "@a:user",
-                "fields": {
-                    "more": "fields",
-                },
-            }
-        ]
-
-        URL_USER = f"{URL}/_matrix/app/v1/thirdparty/user/{PROTOCOL}"
-        FALLBACK_URL_USER = f"{URL}/_matrix/app/unstable/thirdparty/user/{PROTOCOL}"
-
-        self.request_url = None
-        self.v1_seen = False
-
-        async def get_json(
-            url: str,
-            args: Mapping[Any, Any],
-            headers: Mapping[Union[str, bytes], Sequence[Union[str, bytes]]],
-        ) -> List[JsonDict]:
-            # Ensure the access token is passed as both a header and query arg.
-            if not headers.get("Authorization") or not args.get(b"access_token"):
-                raise RuntimeError("Access token not provided")
-
-            self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
-            self.assertEqual(args.get(b"access_token"), TOKEN)
-            self.request_url = url
-            if url == URL_USER:
-                self.v1_seen = True
-                raise HttpResponseException(404, "NOT_FOUND", b"NOT_FOUND")
-            elif url == FALLBACK_URL_USER:
-                return SUCCESS_RESULT_USER
-            else:
-                raise RuntimeError(
-                    "URL provided was invalid. This should never be seen."
-                )
-
-        # We assign to a method, which mypy doesn't like.
-        self.api.get_json = Mock(side_effect=get_json)  # type: ignore[assignment]
-
-        result = self.get_success(
-            self.api.query_3pe(self.service, "user", PROTOCOL, {b"some": [b"field"]})
-        )
-        self.assertTrue(self.v1_seen)
-        self.assertEqual(self.request_url, FALLBACK_URL_USER)
-        self.assertEqual(result, SUCCESS_RESULT_USER)
-
     def test_claim_keys(self) -> None:
         """
         Tests that the /keys/claim response is properly parsed for missing