Add some type hints to datastore

[dklimpel]

Pull Request Checklist

• 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.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Signed-off-by: Dirk Klimpel dirk@klimpel.org

[DMRobertson]

Might clash with #12716.

[dklimpel]

Might clash with #12716.

Thanks. I will wait some time for #12716.

[DMRobertson]

Might clash with #12716.

Thanks. I will wait some time for #12716.

Don't feel like you have to---I'm happy to fixup any merges on either PR!

[dklimpel]

In this PR are still a few challanges and I need more time.

[dklimpel]

I need support for the last problem:

synapse/state/__init__.py:259: error: Incompatible return value type (got "FrozenSet[str]", expected "Set[str]")  [return-value]

When I am modify the parts in __init__.py it creates new problems.

synapse/federation/sender/__init__.py:437: error: Incompatible types in assignment (expression has type "FrozenSet[str]", variable has type "Optional[Set[str]]")  [assignment]
synapse/federation/sender/__init__.py:449: error: Item "None" of "Optional[Set[str]]" has no attribute "__iter__" (not iterable)  [union-attr]

[clokep]

synapse/state/__init__.py:259: error: Incompatible return value type (got "FrozenSet[str]", expected "Set[str]")  [return-value]

Changing this seems like the proper change to make! 👍

synapse/federation/sender/__init__.py:437: error: Incompatible types in assignment (expression has type "FrozenSet[str]", variable has type "Optional[Set[str]]")  [assignment]
synapse/federation/sender/__init__.py:449: error: Item "None" of "Optional[Set[str]]" has no attribute "__iter__" (not iterable)  [union-attr]

I think we need to make two changes here:

1. On line 412, we can set: destinations: Optional[Collection[str]] = None (we only care that it is iterable and about set and frozenset are...)
2. All uses of destination on line 447 and beyond need to be renamed, e.g. sharded_destinations.

[dklimpel]

I did not find a good type hint for on_invalidate

synapse/synapse/storage/databases/main/roommember.py

Lines 634 to 636 in 60828b9

	async def get_rooms_for_user(
	self, user_id: str, on_invalidate=None
	) -> FrozenSet[str]:

Calls a here:

synapse/synapse/storage/databases/main/roommember.py

Lines 653 to 665 in 60828b9

	async def get_users_who_share_room_with_user(
	self, user_id: str, cache_context: _CacheContext
	) -> Set[str]:
	"""Returns the set of users who share a room with `user_id`"""
	room_ids = await self.get_rooms_for_user(
	user_id, on_invalidate=cache_context.invalidate
	)
	user_who_share_room = set()
	for room_id in room_ids:
	user_ids = await self.get_users_in_room(
	room_id, on_invalidate=cache_context.invalidate
	)

And returns None:

synapse/synapse/util/caches/descriptors.py

Lines 550 to 552 in cde8af9

	def invalidate(self) -> None:
	"""Invalidates the cache entry referred to by the context."""
	self._cache.invalidate(self._cache_key)

[clokep]

I did not find a good type hint for on_invalidate

I think on_invalidate: Optional[Callable[[], None]] I think would work?

[dklimpel]

I think on_invalidate: Optional[Callable[[], None]] I think would work?

It seems to be good, but
synapse/storage/databases/main/roommember.py:644: error: Argument "on_invalidate" to "get_rooms_for_user_with_stream_ordering" of "RoomMemberWorkerStore" has incompatible type "Optional[Callable[[], None]]"; expected "Callable[[], None]" [arg-type]

synapse/synapse/storage/databases/main/roommember.py

Lines 478 to 481 in 60828b9

	@cached(max_entries=500000, iterable=True, prune_unread_entries=False)
	async def get_rooms_for_user_with_stream_ordering(
	self, user_id: str
	) -> FrozenSet[GetRoomsForUserWithStreamOrdering]:

[DMRobertson]

Many thanks for this

[DMRobertson]

I think on_invalidate: Optional[Callable[[], None]] I think would work?

It seems to be good, but synapse/storage/databases/main/roommember.py:644: error: Argument "on_invalidate" to "get_rooms_for_user_with_stream_ordering" of "RoomMemberWorkerStore" has incompatible type "Optional[Callable[[], None]]"; expected "Callable[[], None]" [arg-type]

The conflict comes from deep in the bowels of how @cached works and involves our mypy plugin:

synapse/scripts-dev/mypy_synapse_plugin.py

Lines 73 to 86 in 30c8e7e

	# Third, we add an optional "on_invalidate" argument.
	#
	# This is a callable which accepts no input and returns nothing.
	calltyp = CallableType(
	arg_types=[],
	arg_kinds=[],
	arg_names=[],
	ret_type=NoneType(),
	fallback=ctx.api.named_generic_type("builtins.function", []),
	)
	arg_types.append(calltyp)
	arg_names.append("on_invalidate")
	arg_kinds.append(ARG_NAMED_OPT) # Arg is an optional kwarg.

Looking at these three places, I think it would be safe to widen the plugin to allow an on_invalidate to be None?

synapse/synapse/util/caches/descriptors.py

Line 223 in 2fcf4b3

invalidate_callback = kwargs.pop("on_invalidate", None)

synapse/synapse/util/caches/descriptors.py

Line 337 in 2fcf4b3

invalidate_callback = kwargs.pop("on_invalidate", None)

synapse/synapse/util/caches/descriptors.py

Line 432 in 2fcf4b3

invalidate_callback = kwargs.pop("on_invalidate", None)

What do you think @erikjohnston (as the author of the mypy plugin)?

[DMRobertson]

I think this LGTM. I'll have a go at updating the mypy plugin separately.