Add via param to hierarchy enpoint

[kfiven]

Pull Request Checklist

Implementation of MSC4235 as per suggestion in pull request 17750.

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct
  (run the linters)

[kfiven]

@anoadragon453 pinging you for visibility since you reviewed the previous PR to fix this.

[CLAassistant]

All committers have signed the CLA.

[sandhose]

If that's not too annoying to add, I would appreciate a test for the new remote_room_hosts parameter on the get_room_hierarchy function. You can look at existing tests in tests/handlers/test_room_summary.py

[kfiven]

Hi @sandhose,

I'm experiencing some workflow failures and I'm unsure if they're related to my recent changes. Could you take a look and help me troubleshoot the issue?

Thanks!

[sandhose]

I'm experiencing some workflow failures and I'm unsure if they're related to my recent changes. Could you take a look and help me troubleshoot the issue?

This is likely a test being flaky, I kicked it off again

[sandhose]

Sorry for the delay on the review. After looking at it again, I just wanted to change the remote_room_hosts to a list instead of a tuple, to make it more consistent with the rest

[anoadragon453]

@sandhose heads up that the last commit caused unit tests to fail:

Click to see unit test failure logs

[FAIL]
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/handlers/test_room_summary.py", line 1130, in test_fed_remote_room_hosts
    result = self.get_success(
  File "/home/runner/work/synapse/synapse/tests/unittest.py", line 693, in get_success
    return self.successResultOf(deferred)
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.9/lib/python3.9/site-packages/twisted/trial/_synctest.py", line 732, in successResultOf
    self.fail(
twisted.trial.unittest.FailTest: Success result expected on <Deferred at 0x7f9d0fd0a970 current result: None>, found failure result instead:
Traceback (most recent call last):
  File "/home/runner/work/synapse/synapse/tests/unittest.py", line 691, in get_success
    deferred: Deferred[TV] = ensureDeferred(d)  # type: ignore[arg-type]
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.9/lib/python3.9/site-packages/twisted/internet/defer.py", line 1358, in ensureDeferred
    return Deferred.fromCoroutine(coro)
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.9/lib/python3.9/site-packages/twisted/internet/defer.py", line 1325, in fromCoroutine
    return _cancellableInlineCallbacks(coro)
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.9/lib/python3.9/site-packages/twisted/internet/defer.py", line 2197, in _cancellableInlineCallbacks
    _inlineCallbacks(None, gen, status, _copy_context())
--- <exception caught here> ---
  File "/home/runner/.cache/pypoetry/virtualenvs/matrix-synapse-pswDeSvb-py3.9/lib/python3.9/site-packages/twisted/internet/defer.py", line 2014, in _inlineCallbacks
    result = context.run(gen.send, result)
  File "/home/runner/work/synapse/synapse/synapse/handlers/room_summary.py", line 166, in get_room_hierarchy
    return await self._pagination_response_cache.wrap(
  File "/home/runner/work/synapse/synapse/synapse/util/caches/response_cache.py", line 254, in wrap
    entry = self._get(key)
  File "/home/runner/work/synapse/synapse/synapse/util/caches/response_cache.py", line 146, in _get
    entry = self._result_cache.get(key)
builtins.TypeError: unhashable type: 'list'


tests.handlers.test_room_summary.SpaceSummaryTestCase.test_fed_remote_room_hosts
-------------------------------------------------------------------------------
Ran 4018 tests in 300.576s

FAILED (skips=417, failures=1, successes=3600)

[kfiven]

@sandhose Hi, do you want me to revert the changes that broke the tests? It seems like param as a list doesn’t seem to work with caching.

[anoadragon453]

Hi @kfiven. As weekly maintainer I've taken over this PR.

I've reverted the change from tuple -> list as directed by @sandhose, added a comment as to why it must be a tuple, and fixed the merge conflicts.

This change now LGTM!