Support & use stable endpoints for MSC4151

[turt2live]

matrix-org/matrix-spec-proposals#4151 has finished FCP.

See #17373 for unstable endpoint removal

[anoadragon453]

Given we don't expect the functionality to differ between the stable and unstable versions of this endpoint, you can avoid the code duplication by setting PATTERNS from within the __init__ method of ReportRoomRestServlet based on the experimental config value:

class ReportRoomRestServlet(RestServlet): 
   # Note: Top-level PATTERNS definition removed
 
    def __init__(self, hs: "HomeServer"):
        super().__init__()
        self.hs = hs
        self.auth = hs.get_auth()
        self.clock = hs.get_clock()
        self.store = hs.get_datastores().main

        self.PATTERNS = client_patterns(
	        "/rooms/(?P<room_id>[^/]*)/report$",
	        releases=("v3",),
            # TODO: Remove the unstable variant after 2-3 releases
            # https://github.com/element-hq/synapse/issues/17373
	        unstable=hs.config.experimental.msc4151_enabled,
            v1=False,
	    )

[turt2live]

actually, how would this know to use unstable/org.matrix.msc4151 as its version?

[anoadragon453]

Ah, true. In that case you could instead do:

class ReportRoomRestServlet(RestServlet): 
    PATTERNS = client_patterns(
        "/rooms/(?P<room_id>[^/]*)/report$"),
        releases=("v3",),
        unstable=False,
        v1=False,
    )
 
    def __init__(self, hs: "HomeServer"):
        super().__init__()
        self.hs = hs
        self.auth = hs.get_auth()
        self.clock = hs.get_clock()
        self.store = hs.get_datastores().main

        # TODO: Remove the unstable variant after 2-3 releases
        # https://github.com/element-hq/synapse/issues/17373
        if hs.config.experimental.msc4151_enabled:
            self.PATTERNS.append(
                re.compile(
                    f"^{CLIENT_API_PREFIX}/unstable/org.matrix.msc4151"
                    "/rooms/(?P<room_id>[^/]*)/report$"
                )
            )

[anoadragon453]

Note that the default value for the releases argument is (r0, v3). I assume we only want to make this available on v3?

Note the unstable endpoint was available on r0 as well.

[turt2live]

For cleanliness it should be v3 only - is there a common way to declare that? It's not great that the endpoint was on r0 all this time too :(

[anoadragon453]

The usual method is releases=("v3",).