room.set_user_profile() breaks when logging in with bare username

If I log in as just "Coffee", that works just fine, but room.set_user_profile() breaks with 404 response because it tries to PUT */state/m.room.member/Coffee. This should be */state/m.room.member/@Coffee:matrix.org.

[Androbin]

For clarification:

• client > login_with_password_no_sync puts username = Coffee in a dictionary
• api > login sends that plain value to the server
• The server handles the "bare" username
• The server returns user_id = @Coffee:matrix.org

set_membership fails because user_id was mistakenly set to Coffee, not @Coffee:matrix.org

Right so far?

[unnikrishnan-ag]

will this issue be fixed by appending :matrix.org and an '@' at the beginning to such bare usernames within login_with_password_no_sync ?

@agunnik No, because the user may have their account on a server other than matrix.org.

If @Androbin is right and the server gives us the full user_id, the correct solution is to use that.

[aroneiermann]

It seems to me that this issue is fixed.

The following code works in the master branch just fine.

from matrix_client.client import MatrixClient
client = MatrixClient("REDACTED")
client.login_with_password_no_sync(username="dev03", password="REDACTED")
print(client.user_id) # @dev03:REDACTED
room = client.create_room("my_room4")
room.set_user_profile(displayname="Hello World!")

Indeed, reviewing the code, that code path wasn't broken. It does happen when you instantiate MatrixClient with a bare user_id and token. Apologies for using bad wording.

The documentation for the MatrixClient constructor was updated (e06a21d) around the time I created this issue, and clarifies that we need to persist the user_id we got from a previous login. It seems I missed this because I remember the previous documentation. So I think this documentation update alone closes this issue.

However, maybe the constructor could throw an exception if the user_id does not contain a colon, so the user is informed of their mistake immediately, rather than having weird errors happen somewhere down the line.

[non-Jedi]

I like @Matrixcoffee suggestion on this one since we've seen at least one person run into this bug already. Changing this to an enhancement in light of that (should only be a couple loc to fix).

[Androbin]

maybe the constructor could throw an exception if the user_id does not contain a colon

The Room and User constructors already have such sanity checks.
I will simply copy the user_id check from User to MatrixClient.

[Androbin]

Or maybe we should put that in separate file, e.g. checks.py and call that from API entry points?

def check_user_id(user_id):
    if not user_id.startswith("@"):
        raise ValueError("UserIDs start with @")

    if ":" not in user_id:
        raise ValueError("UserIDs must have a domain component, seperated by a :")

[non-Jedi]

I like the general sound of that better than copy-pasting. Also lets us improve those sanity-check type things as matrix changes all in one place. Usage should be something like:

if utils.well_formed_mxid(mxid):
    do_stuff()
else:
    raise DescriptiveException()