core: Fix SQLite realname/avatarurl handling
authorShane Synan <digitalcircuit36939@gmail.com>
Fri, 11 May 2018 05:36:21 +0000 (00:36 -0500)
committerManuel Nickschas <sputnick@quassel-irc.org>
Wed, 23 May 2018 22:33:28 +0000 (00:33 +0200)
In insert_message, when selecting a senderid also match on "realname"
and "avatarurl".  This fixes cases where sometimes senderid would
refer to a sender without a realname after a realname was added, etc.

Test case: start up core, client.  Send several messages quickly after
joining.  Shut down core, check SQLite database "sender" table.

Before this fix, once a sender entry is put in, sometimes
permutations of realname/avatarurl would NOT create a new sender
entry, clearly visible in QuasselDroid's display of of the realname.

After this fix, changing realname/avatarurl causes new sender entries
to be added every single time there's a change, as expected, and they
display correctly in QuasselDroid.

PostgreSqlStorage was immune to this, but for once, not due to being
a better database, but due to how the queries are constructed.  For
PostgreSQL, senders were always fetched separately, properly checking
avatarurl and realname.

NOTE: The realname/avatarurl columns can be NULL values.  Due to
this, we need to coalesce them to '' in order to use the same queries
rather than "column = some value" and "column IS NULL".  Both column
and the input parameter need coalesced in case one or the other is
NULL.  As there's minimal functional difference in protocol handling
between '' and NULL, we consider them the same.

(This could be squashed down if preferred, I just wanted to document
 what was needed to fix this and why only SQLite was changed.)

src/core/SQL/SQLite/insert_message.sql
src/core/sqlitestorage.cpp

index 0a91299..f7faa29 100644 (file)
@@ -1,2 +1,5 @@
 INSERT INTO backlog (time, bufferid, type, flags, senderid, senderprefixes, message)
 INSERT INTO backlog (time, bufferid, type, flags, senderid, senderprefixes, message)
-VALUES (:time, :bufferid, :type, :flags, (SELECT senderid FROM sender WHERE sender = :sender), :senderprefixes, :message)
+VALUES (:time, :bufferid, :type, :flags,
+       (SELECT senderid FROM sender WHERE sender = :sender AND coalesce(realname, '') = coalesce(:realname, '') AND coalesce(avatarurl, '') = coalesce(:avatarurl, '')),
+       :senderprefixes, :message
+)
index 7666679..1e1b798 100644 (file)
@@ -1708,6 +1708,8 @@ bool SqliteStorage::logMessage(Message &msg)
         logMessageQuery.bindValue(":type", msg.type());
         logMessageQuery.bindValue(":flags", (int)msg.flags());
         logMessageQuery.bindValue(":sender", msg.sender());
         logMessageQuery.bindValue(":type", msg.type());
         logMessageQuery.bindValue(":flags", (int)msg.flags());
         logMessageQuery.bindValue(":sender", msg.sender());
+        logMessageQuery.bindValue(":realname", msg.realName());
+        logMessageQuery.bindValue(":avatarurl", msg.avatarUrl());
         logMessageQuery.bindValue(":senderprefixes", msg.senderPrefixes());
         logMessageQuery.bindValue(":message", msg.contents());
 
         logMessageQuery.bindValue(":senderprefixes", msg.senderPrefixes());
         logMessageQuery.bindValue(":message", msg.contents());
 
@@ -1789,6 +1791,8 @@ bool SqliteStorage::logMessages(MessageList &msgs)
             logMessageQuery.bindValue(":type", msg.type());
             logMessageQuery.bindValue(":flags", (int)msg.flags());
             logMessageQuery.bindValue(":sender", msg.sender());
             logMessageQuery.bindValue(":type", msg.type());
             logMessageQuery.bindValue(":flags", (int)msg.flags());
             logMessageQuery.bindValue(":sender", msg.sender());
+            logMessageQuery.bindValue(":realname", msg.realName());
+            logMessageQuery.bindValue(":avatarurl", msg.avatarUrl());
             logMessageQuery.bindValue(":senderprefixes", msg.senderPrefixes());
             logMessageQuery.bindValue(":message", msg.contents());
 
             logMessageQuery.bindValue(":senderprefixes", msg.senderPrefixes());
             logMessageQuery.bindValue(":message", msg.contents());