Disallow periods at the start and end of new usernames

[robertknight]

Work towards resolving an ambiguity when parsing mentions out of text like "I like this @bob." by disallowing periods at the start and end of usernames.

There are a small number (~2000 or <0.1%) of existing accounts which do contain periods at the start or end of usernames. We need to continue to allow those accounts to be used, but we can get away with not supporting extracting mentions for those users from text.

Fixes #9335.

Testing:

1. Try to create a new user account that starts or ends with a period. This should be rejected, with a helpful validation error.
2. Try to log in to an existing user account that starts or ends with a period. This should work as before.
3. Try to rename a user account to start or end with a period. This should be rejected.

[robertknight]

The USERNAME_PATTERN regex now implicitly sets a minimum length of 3 (at least one start, middle and end char) so it also fails. I think that's fine, but for this test only check for the length message specifically.

[robertknight]

Requested a review from @seanh in case there are any issues with changing the username pattern that I've missed.

[seanh]

👍

[seanh]

Will this affect the user-update API, for those users whose existing usernames start or end with a .?

[robertknight]

The user update API does not validate the format of the username that I can see, which makes it compatible with old usernames. I have added a note about the change in the comment for USERNAME_PATTERN to make future readers aware.

[seanh]

FYI we're trying to stop using h-matchers nowadays as there's various problems with it (it's implemented in a very clever way that makes it impossible to maintain, it has a lot of features that we never use, and it ruins pytest's assertion failure messages). I'm not sure what the plain Python / pytest alternative to usages like this would be, however. You could use mock.ANY but that doesn't assert that it's a string. Alternatively a simple classic-style matcher could be implemented.

(mock.ANY is implemented using the classic-style matchers approach. The original request for h-matchers was just to cut-paste existing classic-style matchers from app tests into a shared library, but we got h-matchers instead.)

[seanh]

Looks like we used to have an instance_of matcher for just this purpose:

h/tests/common/matchers.py

Lines 52 to 62 in c480243

	class instance_of(Matcher): # noqa: N801
	"""An object __eq__ to any object which is an instance of `type_`."""
	def __init__(self, type_):
	self.type = type_
	def __eq__(self, other):
	return isinstance(other, self.type)
	def __repr__(self):
	return '<instance of {!r}>'.format(self.type)

. Could build some alias matchers on top of that, like any_string() etc.

[robertknight]

FYI we're trying to stop using h-matchers nowadays as there's various problems with it (it's implemented in a very clever way that makes it impossible to maintain, it has a lot of features that we never use, and it ruins pytest's assertion failure messages).

Could we perhaps distill h-matchers into a more limited subset of the functionality that covers the main use cases? Preferably in such a way that the API migration is easy. The "legacy" API could be moved into a submodule in h-matchers to facilitate incremental migration.

[robertknight]

I have swapped Any.string() for mock.ANY here, since that is fine for the current use case.

[seanh]

Could we perhaps distill h-matchers into a more limited subset of the functionality that covers the main use cases? Preferably in such a way that the API migration is easy. The "legacy" API could be moved into a submodule in h-matchers to facilitate incremental migration.

I think unfortunately not easily, given the way h-matchers is implemented.

There's only a handful of types of assertion that h-matchers tends to be used for across our tests, and each of those types of assertion can be replaced with a simpler approach, e.g. just a plain assertion, or mock.ANY, or a simple matcher class, etc. In that way I think we can phase out h-matchers.