User: optimise names/avatars storing and updating

The current mechanism relied on a complicated and fragile machinery
around setNameForRoom() and setAvatarForRoom() that maintained the
"most used" entity for a given user along with "other" ones. Given that
per-room avatars are pretty rare in Matrix, it's also been inefficient
as kitsune-benchmark-set_ForRoom branch shows. The new mechanism stores
the "default" (as per user profile) name and avatar and maintains
a singleton map of avatar objects across all users. Per-user profile
only (normally) exists for the local user so there's yet another
inefficiency that will be fixed further down the road by introducing
a separate user profile class.

4 files changed, 152 insertions, 310 deletions

diff --git a/lib/room.cpp b/lib/room.cpp
index 970fe56b..ac9db318 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -192,7 +192,6 @@ public:
 
     // void inviteUser(User* u); // We might get it at some point in time.
     void insertMemberIntoMap(User* u);
-    void renameMember(User* u, const QString& oldName);
     void removeMemberFromMap(const QString& username, User* u);
 
     // This updates the room displayname field (which is the way a room
@@ -238,6 +237,15 @@ public:
         return static_cast<const EventT*>(evt);
     }
 
+//    template <typename EventT>
+//    const auto& getCurrentStateContent(const QString& stateKey = {}) const
+//    {
+//        if (const auto* evt =
+//                currentState.value({ EventT::matrixTypeId(), stateKey }, nullptr))
+//            return evt->content();
+//        return EventT::content_type()
+//    }
+
     bool isEventNotable(const TimelineItem& ti) const
     {
         return !ti->isRedacted() && ti->senderId() != connection->userId()
@@ -1344,18 +1352,6 @@ void Room::Private::insertMemberIntoMap(User* u)
         emit q->memberRenamed(namesakes.front());
 }
 
-void Room::Private::renameMember(User* u, const QString& oldName)
-{
-    if (u->name(q) == oldName) {
-        qCWarning(MAIN) << "Room::Private::renameMember(): the user "
-                        << u->fullName(q)
-                        << "is already known in the room under a new name.";
-    } else if (membersMap.contains(oldName, u)) {
-        removeMemberFromMap(oldName, u);
-        insertMemberIntoMap(u);
-    }
-}
-
 void Room::Private::removeMemberFromMap(const QString& username, User* u)
 {
     User* namesake = nullptr;
@@ -2396,14 +2392,66 @@ Room::Changes Room::processStateEvent(const RoomEvent& e)
     if (!e.isStateEvent())
         return Change::NoChange;
 
-    const auto* oldStateEvent =
-        std::exchange(d->currentState[{ e.matrixType(), e.stateKey() }],
-                      static_cast<const StateEventBase*>(&e));
+    // Find a value (create empty if necessary) and get a reference to it
+    // getCurrentState<> is not used here because it (creates and) returns
+    // a stub if a value is not found, and what's needed here is a "real" event
+    // or nullptr.
+    const auto*& curStateEvent =
+        d->currentState[{ e.matrixType(), e.stateKey() }];
+    // Prepare for the state change
+    visit(e, [this, oldRme = static_cast<const RoomMemberEvent*>(curStateEvent)](
+                 const RoomMemberEvent& rme) {
+        auto* u = user(rme.userId());
+        if (!u) { // ???
+            qCCritical(MAIN)
+                << "Could not get a user object for" << rme.userId();
+            return;
+        }
+        const auto prevMembership = oldRme ? oldRme->membership()
+                                           : MembershipType::Leave;
+        switch (prevMembership) {
+        case MembershipType::Invite:
+            if (rme.membership() != prevMembership) {
+                d->usersInvited.removeOne(u);
+                Q_ASSERT(!d->usersInvited.contains(u));
+            }
+            break;
+        case MembershipType::Join:
+            switch (rme.membership()) {
+            case MembershipType::Join: // rename/avatar change or no-op
+                if (rme.displayName() != oldRme->displayName()) {
+                    emit memberAboutToRename(u, rme.displayName());
+                    d->removeMemberFromMap(u->name(this), u);
+                }
+                break;
+            case MembershipType::Invite:
+                qCWarning(MAIN) << "Membership change from Join to Invite:"
+                                << rme;
+                [[fallthrough]];
+            default: // whatever the new membership, it's no more Join
+                d->removeMemberFromMap(u->name(this), u);
+                emit userRemoved(u);
+            }
+            break;
+        default:
+            if (rme.membership() == MembershipType::Invite
+                || rme.membership() == MembershipType::Join) {
+                d->membersLeft.removeOne(u);
+                Q_ASSERT(!d->membersLeft.contains(u));
+            }
+        }
+    });
+
+    // Change the state
+    const auto* const oldStateEvent =
+        std::exchange(curStateEvent, static_cast<const StateEventBase*>(&e));
     Q_ASSERT(!oldStateEvent
              || (oldStateEvent->matrixType() == e.matrixType()
                  && oldStateEvent->stateKey() == e.stateKey()));
     if (!is<RoomMemberEvent>(e)) // Room member events are too numerous
-        qCDebug(STATE) << "Room state event:" << e;
+        qCDebug(STATE) << "Updated room state:" << e;
+
+    // Update internal structures as per the change and work out the return value
 
     // clang-format off
     return visit(e
@@ -2411,10 +2459,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e)
             return NameChange;
         }
         , [] (const RoomAliasesEvent&) {
-            // clang-format on
-            // This event has been removed by MSC-2432
-            return NoChange;
-            // clang-format off
+            return NoChange; // This event has been removed by MSC2432
         }
         , [this, oldStateEvent] (const RoomCanonicalAliasEvent& cae) {
             // clang-format on
@@ -2460,63 +2505,28 @@ Room::Changes Room::processStateEvent(const RoomEvent& e)
             auto* u = user(evt.userId());
             const auto* oldMemberEvent =
                 static_cast<const RoomMemberEvent*>(oldStateEvent);
-            u->processEvent(evt, this, oldMemberEvent == nullptr);
             const auto prevMembership = oldMemberEvent
                                             ? oldMemberEvent->membership()
                                             : MembershipType::Leave;
-            if (u == localUser() && evt.membership() == MembershipType::Invite
-                && evt.isDirect())
-                connection()->addToDirectChats(this, user(evt.senderId()));
-
-            switch (prevMembership) {
-            case MembershipType::Invite:
-                if (evt.membership() != prevMembership) {
-                    d->usersInvited.removeOne(u);
-                    Q_ASSERT(!d->usersInvited.contains(u));
-                }
-                break;
-            case MembershipType::Join:
-                if (evt.membership() == MembershipType::Invite)
-                    qCWarning(MAIN) << "Invalid membership change from "
-                                       "Join to Invite:"
-                                    << evt;
-                if (evt.membership() != prevMembership) {
-                    disconnect(u, &User::nameAboutToChange, this, nullptr);
-                    disconnect(u, &User::nameChanged, this, nullptr);
-                    d->removeMemberFromMap(u->name(this), u);
-                    emit userRemoved(u);
-                }
-                break;
-            default:
-                if (evt.membership() == MembershipType::Invite
-                    || evt.membership() == MembershipType::Join) {
-                    d->membersLeft.removeOne(u);
-                    Q_ASSERT(!d->membersLeft.contains(u));
-                }
-            }
-
             switch (evt.membership()) {
             case MembershipType::Join:
                 if (prevMembership != MembershipType::Join) {
                     d->insertMemberIntoMap(u);
-                    connect(u, &User::nameAboutToChange, this,
-                            [=](QString newName, QString, const Room* context) {
-                                if (context == this)
-                                    emit memberAboutToRename(u, newName);
-                            });
-                    connect(u, &User::nameChanged, this,
-                            [=](QString, QString oldName, const Room* context) {
-                                if (context == this) {
-                                    d->renameMember(u, oldName);
-                                    emit memberRenamed(u);
-                                }
-                            });
                     emit userAdded(u);
+                } else {
+                    if (oldMemberEvent->displayName() != evt.displayName()) {
+                        d->insertMemberIntoMap(u);
+                        emit memberRenamed(u);
+                    }
+                    if (oldMemberEvent->avatarUrl() != evt.avatarUrl())
+                        emit memberAvatarChanged(u);
                 }
                 break;
             case MembershipType::Invite:
                 if (!d->usersInvited.contains(u))
                     d->usersInvited.push_back(u);
+                if (u == localUser() && evt.isDirect())
+                    connection()->addToDirectChats(this, user(evt.senderId()));
                 break;
             default:
                 if (!d->membersLeft.contains(u))
diff --git a/lib/room.h b/lib/room.h
index 1ddff517..7fc03c19 100644
--- a/lib/room.h
+++ b/lib/room.h
@@ -652,6 +652,7 @@ signals:
     void userRemoved(Quotient::User* user);
     void memberAboutToRename(Quotient::User* user, QString newName);
     void memberRenamed(Quotient::User* user);
+    void memberAvatarChanged(Quotient::User* user);
     /// The list of members has changed
     /** Emitted no more than once per sync, this is a good signal to
      * for cases when some action should be done upon any change in
diff --git a/lib/user.cpp b/lib/user.cpp
index 09cf72a2..ffa4efb9 100644
--- a/lib/user.cpp
+++ b/lib/user.cpp
@@ -38,150 +38,26 @@
 #include <functional>
 
 using namespace Quotient;
-using namespace std::placeholders;
 using std::move;
 
 class User::Private {
 public:
-    static Avatar makeAvatar(QUrl url) { return Avatar(move(url)); }
-
     Private(QString userId) : id(move(userId)), hueF(stringToHueF(id)) { }
 
     QString id;
-
-    QString mostUsedName;
-    QMultiHash<QString, const Room*> otherNames;
     qreal hueF;
-    Avatar mostUsedAvatar { makeAvatar({}) };
-    std::vector<Avatar> otherAvatars;
-    auto otherAvatar(const QUrl& url)
-    {
-        return std::find_if(otherAvatars.begin(), otherAvatars.end(),
-                            [&url](const auto& av) { return av.url() == url; });
-    }
-    QMultiHash<QUrl, const Room*> avatarsToRooms;
-
-    mutable int totalRooms = 0;
 
-    QString nameForRoom(const Room* r, const QString& hint = {}) const;
-    void setNameForRoom(const Room* r, QString newName, const QString& oldName);
-    QUrl avatarUrlForRoom(const Room* r, const QUrl& hint = {}) const;
-    void setAvatarForRoom(const Room* r, const QUrl& newUrl, const QUrl& oldUrl);
-
-    void setAvatarOnServer(QString contentUri, User* q);
+    QString defaultName;
+    Avatar defaultAvatar;
+    // NB: This container is ever-growing. Even if the user no more scrolls
+    // the timeline that far back, historical avatars are still kept around.
+    // This is consistent with the rest of Quotient, as room timelines
+    // are never vacuumed either. This will probably change in the future.
+    /// Map of mediaId to Avatar objects
+    static UnorderedMap<QString, Avatar> otherAvatars;
 };
 
-QString User::Private::nameForRoom(const Room* r, const QString& hint) const
-{
-    // If the hint is accurate, this function is O(1) instead of O(n)
-    if (!hint.isNull() && (hint == mostUsedName || otherNames.contains(hint, r)))
-        return hint;
-    return otherNames.key(r, mostUsedName);
-}
-
-static constexpr int MIN_JOINED_ROOMS_TO_LOG = 20;
-
-void User::Private::setNameForRoom(const Room* r, QString newName,
-                                   const QString& oldName)
-{
-    Q_ASSERT(oldName != newName);
-    Q_ASSERT(oldName == mostUsedName || otherNames.contains(oldName, r));
-    if (totalRooms < 2) {
-        Q_ASSERT_X(totalRooms > 0 && otherNames.empty(), __FUNCTION__,
-                   "Internal structures inconsistency");
-        mostUsedName = move(newName);
-        return;
-    }
-    otherNames.remove(oldName, r);
-    if (newName != mostUsedName) {
-        // Check if the newName is about to become most used.
-        if (otherNames.count(newName) >= totalRooms - otherNames.size()) {
-            Q_ASSERT(totalRooms > 1);
-            QElapsedTimer et;
-            if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) {
-                qCDebug(MAIN) << "Switching the most used name of user" << id
-                              << "from" << mostUsedName << "to" << newName;
-                qCDebug(MAIN) << "The user is in" << totalRooms << "rooms";
-                et.start();
-            }
-
-            for (auto* r1: r->connection()->allRooms())
-                if (nameForRoom(r1) == mostUsedName)
-                    otherNames.insert(mostUsedName, r1);
-
-            mostUsedName = newName;
-            otherNames.remove(newName);
-            if (totalRooms > MIN_JOINED_ROOMS_TO_LOG)
-                qCDebug(PROFILER) << et << "to switch the most used name";
-        } else
-            otherNames.insert(newName, r);
-    }
-}
-
-QUrl User::Private::avatarUrlForRoom(const Room* r, const QUrl& hint) const
-{
-    // If the hint is accurate, this function is O(1) instead of O(n)
-    if (hint == mostUsedAvatar.url() || avatarsToRooms.contains(hint, r))
-        return hint;
-    auto it = std::find(avatarsToRooms.begin(), avatarsToRooms.end(), r);
-    return it == avatarsToRooms.end() ? mostUsedAvatar.url() : it.key();
-}
-
-void User::Private::setAvatarForRoom(const Room* r, const QUrl& newUrl,
-                                     const QUrl& oldUrl)
-{
-    Q_ASSERT(oldUrl != newUrl);
-    Q_ASSERT(oldUrl == mostUsedAvatar.url()
-             || avatarsToRooms.contains(oldUrl, r));
-    if (totalRooms < 2) {
-        Q_ASSERT_X(totalRooms > 0 && otherAvatars.empty(), __FUNCTION__,
-                   "Internal structures inconsistency");
-        mostUsedAvatar.updateUrl(newUrl);
-        return;
-    }
-    avatarsToRooms.remove(oldUrl, r);
-    if (!avatarsToRooms.contains(oldUrl)) {
-        auto it = otherAvatar(oldUrl);
-        if (it != otherAvatars.end())
-            otherAvatars.erase(it);
-    }
-    if (newUrl != mostUsedAvatar.url()) {
-        // Check if the new avatar is about to become most used.
-        const auto newUrlUsage = avatarsToRooms.count(newUrl);
-        if (newUrlUsage >= totalRooms - avatarsToRooms.size()) {
-            QElapsedTimer et;
-            if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) {
-                qCInfo(MAIN)
-                    << "Switching the most used avatar of user" << id
-                    << "from" << mostUsedAvatar.url().toDisplayString() << "to"
-                    << newUrl.toDisplayString();
-                et.start();
-            }
-            avatarsToRooms.remove(newUrl);
-            auto nextMostUsedIt = otherAvatar(newUrl);
-            if (nextMostUsedIt == otherAvatars.end()) {
-                qCCritical(MAIN)
-                    << id << "doesn't have" << newUrl.toDisplayString()
-                    << "in otherAvatars though it seems to be used in"
-                    << newUrlUsage << "rooms";
-                Q_ASSERT(false);
-                otherAvatars.emplace_back(makeAvatar(newUrl));
-                nextMostUsedIt = otherAvatars.end() - 1;
-            }
-            std::swap(mostUsedAvatar, *nextMostUsedIt);
-            for (const auto* r1: r->connection()->allRooms())
-                if (avatarUrlForRoom(r1) == nextMostUsedIt->url())
-                    avatarsToRooms.insert(nextMostUsedIt->url(), r1);
-
-            if (totalRooms > MIN_JOINED_ROOMS_TO_LOG)
-                qCDebug(PROFILER) << et << "to switch the most used avatar";
-        } else {
-            if (otherAvatar(newUrl) == otherAvatars.end())
-                otherAvatars.emplace_back(makeAvatar(newUrl));
-            avatarsToRooms.insert(newUrl, r);
-        }
-    }
-}
+decltype(User::Private::otherAvatars) User::Private::otherAvatars {};
 
 User::User(QString userId, Connection* connection)
     : QObject(connection), d(new Private(move(userId)))
@@ -210,43 +86,31 @@ bool User::isGuest() const
 
 int User::hue() const { return int(hueF() * 359); }
 
-QString User::name(const Room* room) const { return d->nameForRoom(room); }
-
-QString User::rawName(const Room* room) const { return name(room); }
-
-void User::updateName(const QString& newName, const Room* room)
+QString User::name(const Room* room) const
 {
-    updateName(newName, d->nameForRoom(room), room);
+    return room ? room->getCurrentState<RoomMemberEvent>(id())->displayName()
+                : d->defaultName;
 }
 
-void User::updateName(const QString& newName, const QString& oldName,
-                      const Room* room)
-{
-    Q_ASSERT(oldName == d->mostUsedName
-             || d->otherNames.contains(oldName, room));
-    if (newName != oldName) {
-        emit nameAboutToChange(newName, oldName, room);
-        d->setNameForRoom(room, newName, oldName);
-        emit nameChanged(newName, oldName, room);
-    }
-}
-
-void User::updateAvatarUrl(const QUrl& newUrl, const QUrl& oldUrl,
-                           const Room* room)
-{
-    Q_ASSERT(oldUrl == d->mostUsedAvatar.url()
-             || d->avatarsToRooms.contains(oldUrl, room));
-    if (newUrl != oldUrl) {
-        d->setAvatarForRoom(room, newUrl, oldUrl);
-        emit avatarChanged(this, room);
-    }
-}
+QString User::rawName(const Room* room) const { return name(room); }
 
 void User::rename(const QString& newName)
 {
     const auto actualNewName = sanitized(newName);
+    if (actualNewName == d->defaultName)
+        return; // Nothing to do
+
     connect(connection()->callApi<SetDisplayNameJob>(id(), actualNewName),
-            &BaseJob::success, this, [=] { updateName(actualNewName); });
+            &BaseJob::success, this, [this, actualNewName] {
+                // Check again, it could have changed meanwhile
+                if (actualNewName != d->defaultName) {
+                    d->defaultName = actualNewName;
+                    emit defaultNameChanged();
+                } else
+                    qCWarning(MAIN)
+                        << "User" << id() << "already has profile name set to"
+                        << actualNewName;
+            });
 }
 
 void User::rename(const QString& newName, const Room* r)
@@ -262,22 +126,37 @@ void User::rename(const QString& newName, const Room* r)
     const auto actualNewName = sanitized(newName);
     MemberEventContent evtC;
     evtC.displayName = actualNewName;
-    connect(r->setState<RoomMemberEvent>(id(), move(evtC)), &BaseJob::success,
-            this, [=] { updateName(actualNewName, r); });
+    r->setState<RoomMemberEvent>(id(), move(evtC));
+    // The state will be updated locally after it arrives with sync
+}
+
+template <typename SourceT>
+inline bool User::doSetAvatar(SourceT&& source)
+{
+    return d->defaultAvatar.upload(
+        connection(), source, [this](const QString& contentUri) {
+            auto* j = connection()->callApi<SetAvatarUrlJob>(id(), contentUri);
+            connect(j, &BaseJob::success, this,
+                    [this, newUrl = QUrl(contentUri)] {
+                        if (newUrl == d->defaultAvatar.url()) {
+                            d->defaultAvatar.updateUrl(move(newUrl));
+                            emit defaultAvatarChanged();
+                        } else
+                            qCWarning(MAIN) << "User" << id()
+                                            << "already has avatar URL set to"
+                                            << newUrl.toDisplayString();
+                    });
+        });
 }
 
 bool User::setAvatar(const QString& fileName)
 {
-    return avatarObject().upload(connection(), fileName,
-                                 std::bind(&Private::setAvatarOnServer,
-                                           d.data(), _1, this));
+    return doSetAvatar(fileName);
 }
 
 bool User::setAvatar(QIODevice* source)
 {
-    return avatarObject().upload(connection(), source,
-                                 std::bind(&Private::setAvatarOnServer,
-                                           d.data(), _1, this));
+    return doSetAvatar(source);
 }
 
 void User::requestDirectChat() { connection()->requestDirectChat(this); }
@@ -288,34 +167,29 @@ void User::unmarkIgnore() { connection()->removeFromIgnoredUsers(this); }
 
 bool User::isIgnored() const { return connection()->isIgnored(this); }
 
-void User::Private::setAvatarOnServer(QString contentUri, User* q)
-{
-    auto* j = q->connection()->callApi<SetAvatarUrlJob>(id, contentUri);
-    connect(j, &BaseJob::success, q,
-            [=] { q->updateAvatarUrl(contentUri, avatarUrlForRoom(nullptr)); });
-}
-
 QString User::displayname(const Room* room) const
 {
-    if (room)
-        return room->roomMembername(this);
-
-    const auto name = d->nameForRoom(nullptr);
-    return name.isEmpty() ? d->id : name;
+    return room                       ? room->roomMembername(this)
+           : d->defaultName.isEmpty() ? d->id
+                                      : d->defaultName;
 }
 
 QString User::fullName(const Room* room) const
 {
-    const auto name = d->nameForRoom(room);
-    return name.isEmpty() ? id() : name % " (" % id() % ')';
+    const auto displayName = name(room);
+    return displayName.isEmpty() ? id() : (displayName % " (" % id() % ')');
 }
 
 QString User::bridged() const { return {}; }
 
 const Avatar& User::avatarObject(const Room* room) const
 {
-    auto it = d->otherAvatar(d->avatarUrlForRoom(room));
-    return it != d->otherAvatars.end() ? *it : d->mostUsedAvatar;
+    if (!room)
+        return d->defaultAvatar;
+
+    const auto& url = room->getCurrentState<RoomMemberEvent>(id())->avatarUrl();
+    const auto& mediaId = url.authority() + url.path();
+    return d->otherAvatars.try_emplace(mediaId, url).first->second;
 }
 
 QImage User::avatar(int dimension, const Room* room)
@@ -331,10 +205,7 @@ QImage User::avatar(int width, int height, const Room* room)
 QImage User::avatar(int width, int height, const Room* room,
                     const Avatar::get_callback_t& callback)
 {
-    return avatarObject(room).get(connection(), width, height, [=] {
-        emit avatarChanged(this, room);
-        callback();
-    });
+    return avatarObject(room).get(connection(), width, height, callback);
 }
 
 QString User::avatarMediaId(const Room* room) const
@@ -347,30 +218,4 @@ QUrl User::avatarUrl(const Room* room) const
     return avatarObject(room).url();
 }
 
-void User::processEvent(const RoomMemberEvent& event, const Room* room,
-                        bool firstMention)
-{
-    Q_ASSERT(room);
-
-    if (firstMention)
-        ++d->totalRooms;
-
-    if (event.membership() != MembershipType::Invite
-        && event.membership() != MembershipType::Join)
-        return;
-
-    if (event.prevContent()) {
-        // FIXME: the hint doesn't work for bridged users
-        auto oldNameHint = d->nameForRoom(room,
-                                          event.prevContent()->displayName);
-        updateName(event.displayName(), oldNameHint, room);
-        updateAvatarUrl(event.avatarUrl(),
-                        d->avatarUrlForRoom(room, event.prevContent()->avatarUrl),
-                        room);
-    } else {
-        updateName(event.displayName(), room);
-        updateAvatarUrl(event.avatarUrl(), d->avatarUrlForRoom(room), room);
-    }
-}
-
 qreal User::hueF() const { return d->hueF; }
diff --git a/lib/user.h b/lib/user.h
index e4328f1d..a3b22480 100644
--- a/lib/user.h
+++ b/lib/user.h
@@ -33,13 +33,12 @@ class User : public QObject {
     Q_PROPERTY(bool isGuest READ isGuest CONSTANT)
     Q_PROPERTY(int hue READ hue CONSTANT)
     Q_PROPERTY(qreal hueF READ hueF CONSTANT)
-    Q_PROPERTY(QString name READ name NOTIFY nameChanged)
-    Q_PROPERTY(QString displayName READ displayname NOTIFY nameChanged STORED false)
-    Q_PROPERTY(QString fullName READ fullName NOTIFY nameChanged STORED false)
-    Q_PROPERTY(QString bridgeName READ bridged NOTIFY nameChanged STORED false)
-    Q_PROPERTY(QString avatarMediaId READ avatarMediaId NOTIFY avatarChanged
-                   STORED false)
-    Q_PROPERTY(QUrl avatarUrl READ avatarUrl NOTIFY avatarChanged)
+    Q_PROPERTY(QString name READ name NOTIFY defaultNameChanged)
+    Q_PROPERTY(QString displayName READ displayname NOTIFY defaultNameChanged STORED false)
+    Q_PROPERTY(QString fullName READ fullName NOTIFY defaultNameChanged STORED false)
+    Q_PROPERTY(QString bridgeName READ bridged NOTIFY defaultNameChanged STORED false)
+    Q_PROPERTY(QString avatarMediaId READ avatarMediaId NOTIFY defaultAvatarChanged STORED false)
+    Q_PROPERTY(QUrl avatarUrl READ avatarUrl NOTIFY defaultAvatarChanged)
 public:
     User(QString userId, Connection* connection);
     ~User() override;
@@ -123,20 +122,14 @@ public:
     QString avatarMediaId(const Room* room = nullptr) const;
     QUrl avatarUrl(const Room* room = nullptr) const;
 
-    /// This method is for internal use and should not be called
-    /// from client code
-    // FIXME: Move it away to private
-    void processEvent(const RoomMemberEvent& event, const Room* r,
-                      bool firstMention);
-
 public slots:
-    /** Set a new name in the global user profile */
+    /// Set a new name in the global user profile
     void rename(const QString& newName);
-    /** Set a new name for the user in one room */
+    /// Set a new name for the user in one room
     void rename(const QString& newName, const Room* r);
-    /** Upload the file and use it as an avatar */
+    /// Upload the file and use it as an avatar
     bool setAvatar(const QString& fileName);
-    /** Upload contents of the QIODevice and set that as an avatar */
+    /// Upload contents of the QIODevice and set that as an avatar
     bool setAvatar(QIODevice* source);
     /// Create or find a direct chat with this user
     /*! The resulting chat is returned asynchronously via
@@ -151,21 +144,14 @@ public slots:
     bool isIgnored() const;
 
 signals:
-    void nameAboutToChange(QString newName, QString oldName,
-                           const Quotient::Room* roomContext);
-    void nameChanged(QString newName, QString oldName,
-                     const Quotient::Room* roomContext);
-    void avatarChanged(Quotient::User* user, const Quotient::Room* roomContext);
-
-private slots:
-    void updateName(const QString& newName, const Room* room = nullptr);
-    void updateName(const QString& newName, const QString& oldName,
-                    const Room* room = nullptr);
-    void updateAvatarUrl(const QUrl& newUrl, const QUrl& oldUrl,
-                         const Room* room = nullptr);
+    void defaultNameChanged();
+    void defaultAvatarChanged();
 
 private:
     class Private;
     QScopedPointer<Private> d;
+
+    template <typename SourceT>
+    bool doSetAvatar(SourceT&& source);
 };
 } // namespace Quotient