Closed enmand closed 8 years ago
Some background: if I recall the history of ClientLookupSet correctly, @jlatt wanted to be lazy and let sqllite do the equality-testing of usernames, instead of coding something up.
And I believe the reason he left it as in-memory rather than on-disk is because there are more considerations if the clients info persists across ircd restarts/crashes.
For example, (just tested this), suppose I start the ircd and I connect with the nick edmund
. I stop the server (or it crashes). The client info is still in the DB (not surprising):
$ sqlite3 ergonomadic.db
sqlite> .schema client
CREATE TABLE client (
nickname TEXT NOT NULL COLLATE NOCASE UNIQUE,
userhost TEXT NOT NULL COLLATE NOCASE,
UNIQUE (nickname, userhost) ON CONFLICT REPLACE);
CREATE UNIQUE INDEX idx_nick ON client (nickname COLLATE NOCASE);
CREATE UNIQUE INDEX idx_uh ON client (userhost COLLATE NOCASE);
sqlite> select * from client;
edmund|edmund!edmund@localhost
Now, start the ircd again and connect as edmund
again.
$ ./ergonomadic run -conf ergonomadic.conf
2015/10/19 17:31:12 irc.example.com listening on localhost:6667
2015/10/19 17:31:12 irc.example.com listening on [::1]:6667
2015/10/19 17:31:12 ergonomadic-1.4.4 running
2015/10/19 17:31:12 irc.example.com listening on :8080
2015/10/19 17:31:21 irc.example.com accept: 127.0.0.1:40150
2015/10/19 17:31:21 127.0.0.1:40150 → PASS test
2015/10/19 17:31:21 127.0.0.1:40150 → NICK edmund
2015/10/19 17:31:21 127.0.0.1:40150 → USER edmund edmund localhost :Edmund Huber
2015/10/19 17:31:21 ClientDB.Add: UNIQUE constraint failed: client.nickname
It works fine, but because the clients table was persisted there's a validation failure in the db, which arguably should cause an error rather than just outputting a warning.
I think that before merging this we should think a little more about what else needs to change, considering that the clients data persists now. Or for example, would it make more sense to do nick registration, bans, etc as a separate table rather than overloading the clients table, which is really only meant right now to capture the state of connected clients?
@enmand, if you want to take this up again, just reopen the PR. I assume this is abandoned for now.
This PR persists the clients table to the on-disk database, instead of an in-memory one.
Additional future work might include: