From 6b6e59da2e8783e69d10c7de0edc9807f8f3ca0e Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sat, 15 May 2021 15:27:40 +0000 Subject: [PATCH 01/17] Initial SQLx implementation (variables). not yet wired up to bot. - Adds migrations for the necessary tables. - Implements the user variables database functions. - Adds sqlx metadata for 'offline' use so we can build without a database. --- .env | 2 + .gitignore | 1 + Cargo.lock | 434 ++++++++++++++++++++++- Cargo.toml | 10 + sqlx-data.json | 45 +++ src/bin/dicebot.rs | 4 + src/db.rs | 1 + src/db/sqlite/errors.rs | 81 +++++ src/db/sqlite/mod.rs | 80 +++++ src/db/sqlite/variables.rs | 92 +++++ src/error.rs | 5 +- src/lib.rs | 1 + src/migrate_cli.rs | 17 + src/migrator.rs | 33 ++ src/migrator/migrations/V1__variables.rs | 18 + src/migrator/migrations/V2__rooms.rs | 31 ++ src/migrator/migrations/mod.rs | 2 + 17 files changed, 855 insertions(+), 2 deletions(-) create mode 100644 .env create mode 100644 sqlx-data.json create mode 100644 src/db/sqlite/errors.rs create mode 100644 src/db/sqlite/mod.rs create mode 100644 src/db/sqlite/variables.rs create mode 100644 src/migrate_cli.rs create mode 100644 src/migrator.rs create mode 100644 src/migrator/migrations/V1__variables.rs create mode 100644 src/migrator/migrations/V2__rooms.rs create mode 100644 src/migrator/migrations/mod.rs diff --git a/.env b/.env new file mode 100644 index 0000000..be42785 --- /dev/null +++ b/.env @@ -0,0 +1,2 @@ +DATABASE_URL="sqlite://test-db/dicebot.sqlite" +SQLX_OFFLINE="true" diff --git a/.gitignore b/.gitignore index 6c5b343..5030d40 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ bot-db* # We store a disabled async test in this file bigboy .#* +*.sqlite diff --git a/Cargo.lock b/Cargo.lock index ff8523e..7e0aea0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,6 +66,23 @@ dependencies = [ "opaque-debug", ] +[[package]] +name = "ahash" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "739f4a8db6605981345c5654f3a85b056ce52f37a39d34da03f25bf2151ea16e" + +[[package]] +name = "ahash" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "796540673305a66d127804eef19ad696f1f204b8c1025aaca4958c17eab32877" +dependencies = [ + "getrandom 0.2.2", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "0.7.18" @@ -98,6 +115,15 @@ dependencies = [ "syn", ] +[[package]] +name = "atoi" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616896e05fc0e2649463a93a15183c6a16bf03413a7af88ef1285ddedfa9cda5" +dependencies = [ + "num-traits", +] + [[package]] name = "atomic" version = "0.5.0" @@ -138,6 +164,12 @@ dependencies = [ "tokio", ] +[[package]] +name = "barrel" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d67c978b1322c8031145b1f6c236fc371292f52c565bc96018b2971afcbffe1" + [[package]] name = "base-x" version = "0.2.8" @@ -165,6 +197,18 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "bitvec" +version = "0.19.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8942c8d352ae1838c9dda0b0ca2ab657696ef2232a20147cf1b30ae1a9cb4321" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "block-buffer" version = "0.9.0" @@ -174,6 +218,12 @@ dependencies = [ "generic-array", ] +[[package]] +name = "build_const" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4ae4235e6dac0694637c763029ecea1a2ec9e4e06ec2729bd21ba4d9c863eb7" + [[package]] name = "bumpalo" version = "3.6.1" @@ -227,6 +277,19 @@ dependencies = [ "zeroize", ] +[[package]] +name = "chrono" +version = "0.4.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "670ad68c9088c2a963aaa298cb369688cf3f9465ce5e2d4ca10e6e0098a1ce73" +dependencies = [ + "libc", + "num-integer", + "num-traits", + "time 0.1.43", + "winapi", +] + [[package]] name = "cipher" version = "0.2.5" @@ -292,6 +355,15 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dcb25d077389e53838a8158c8e99174c5a9d902dee4904320db714f3c653ffba" +[[package]] +name = "crc" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d663548de7f5cca343f1e0a48d14dcfb0e9eb4e079ec58883b7251539fa10aeb" +dependencies = [ + "build_const", +] + [[package]] name = "crc32fast" version = "1.2.1" @@ -301,6 +373,16 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crossbeam-channel" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06ed27e177f16d65f0f0c22a213e17c696ace5dd64b14258b52f9417ccb52db4" +dependencies = [ + "cfg-if", + "crossbeam-utils", +] + [[package]] name = "crossbeam-epoch" version = "0.9.4" @@ -314,6 +396,16 @@ dependencies = [ "scopeguard", ] +[[package]] +name = "crossbeam-queue" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f6cb3c7f5b8e51bc3ebb73a2327ad4abdbd119dc13223f14f961d2f38486756" +dependencies = [ + "cfg-if", + "crossbeam-utils", +] + [[package]] name = "crossbeam-utils" version = "0.8.4" @@ -389,11 +481,20 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0" +[[package]] +name = "dotenv" +version = "0.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77c90badedccf4105eca100756a0b1289e191f6fcbdadd3cee1d2f614f97da8f" + [[package]] name = "either" version = "1.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" +dependencies = [ + "serde", +] [[package]] name = "encoding_rs" @@ -417,6 +518,18 @@ dependencies = [ "termcolor", ] +[[package]] +name = "fallible-iterator" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fnv" version = "1.0.7" @@ -458,6 +571,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "funty" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed34cd105917e91daa4da6b3728c47b068749d6a62c59811f06ed2ac71d9da7" + [[package]] name = "futf" version = "0.1.4" @@ -671,6 +790,27 @@ name = "hashbrown" version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" +dependencies = [ + "ahash 0.4.7", +] + +[[package]] +name = "hashlink" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d99cf782f0dc4372d26846bec3de7804ceb5df083c2d4462c0b8d2330e894fa8" +dependencies = [ + "hashbrown", +] + +[[package]] +name = "heck" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87cbf45460356b7deeb5e3415b5563308c0a9b057c85e12b06ad551f98d0a6ac" +dependencies = [ + "unicode-segmentation", +] [[package]] name = "hermit-abi" @@ -681,6 +821,12 @@ dependencies = [ "libc", ] +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "hmac" version = "0.10.1" @@ -901,6 +1047,17 @@ version = "0.2.94" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18794a8ad5b29321f790b55d93dfba91e125cb1a9edbd4f8e3150acc771c1a5e" +[[package]] +name = "libsqlite3-sys" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64d31059f22935e6c31830db5249ba2b7ecd54fd73a9909286f0a67aa55c2fbd" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "lock_api" version = "0.4.4" @@ -1136,6 +1293,19 @@ dependencies = [ "version_check", ] +[[package]] +name = "nom" +version = "6.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7413f999671bd4745a7b624bd370a569fb6bc574b23c83a3c5ed2e453f3d5e2" +dependencies = [ + "bitvec", + "funty", + "lexical-core", + "memchr", + "version_check", +] + [[package]] name = "ntapi" version = "0.3.6" @@ -1145,6 +1315,25 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-integer" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2cc698a63b549a70bc047073d2949cce27cd1c7b0a4a862d08a8031bc2801db" +dependencies = [ + "autocfg", + "num-traits", +] + +[[package]] +name = "num-traits" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290" +dependencies = [ + "autocfg", +] + [[package]] name = "num_cpus" version = "1.13.0" @@ -1433,6 +1622,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "radium" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "941ba9d78d8e2f7ce474c015eea4d9c6d25b6a3327f9832ee29a4de27f91bbb8" + [[package]] name = "rand" version = "0.7.3" @@ -1543,6 +1738,50 @@ dependencies = [ "redox_syscall", ] +[[package]] +name = "refinery" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e29bd9c881127d714f4b5b9fdd9ea7651f3dd254922e959a10f6ada620e841da" +dependencies = [ + "refinery-core", + "refinery-macros", +] + +[[package]] +name = "refinery-core" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53260bc01535ea10c553ce0fc410609ba2dc0a9f4c9b4503e0af842dd4a6f89d" +dependencies = [ + "async-trait", + "cfg-if", + "chrono", + "lazy_static", + "log", + "regex", + "rusqlite", + "serde", + "siphasher", + "thiserror", + "toml", + "url", + "walkdir", +] + +[[package]] +name = "refinery-macros" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a79ff62c9b674b62c06a09cc8becf06cbafba9952afa1d8174e7e15f2c4ed43" +dependencies = [ + "proc-macro2", + "quote", + "refinery-core", + "regex", + "syn", +] + [[package]] name = "regex" version = "1.5.4" @@ -1775,6 +2014,21 @@ dependencies = [ "syn", ] +[[package]] +name = "rusqlite" +version = "0.24.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d5f38ee71cbab2c827ec0ac24e76f82eca723cee92c509a65f67dee393c25112" +dependencies = [ + "bitflags", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "memchr", + "smallvec", +] + [[package]] name = "rustc_version" version = "0.2.3" @@ -1790,6 +2044,15 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "schannel" version = "0.1.19" @@ -1876,6 +2139,7 @@ version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "799e97dc9fdae36a5c8b8f2cae9ce2ee9fdce2058c57a93e6099d919fd982f79" dependencies = [ + "indexmap", "itoa", "ryu", "serde", @@ -1965,6 +2229,105 @@ dependencies = [ "winapi", ] +[[package]] +name = "sqlformat" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d86e3c77ff882a828346ba401a7ef4b8e440df804491c6064fe8295765de71c" +dependencies = [ + "lazy_static", + "maplit", + "nom 6.1.2", + "regex", + "unicode_categories", +] + +[[package]] +name = "sqlx" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2739d54a2ae9fdd0f545cb4e4b5574efb95e2ec71b7f921678e246fb20dcaaf" +dependencies = [ + "sqlx-core", + "sqlx-macros", +] + +[[package]] +name = "sqlx-core" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1cad9cae4ca8947eba1a90e8ec7d3c59e7a768e2f120dc9013b669c34a90711" +dependencies = [ + "ahash 0.6.3", + "atoi", + "bitflags", + "byteorder", + "bytes", + "crc", + "crossbeam-channel", + "crossbeam-queue", + "crossbeam-utils", + "either", + "futures-channel", + "futures-core", + "futures-util", + "hashlink", + "hex", + "itoa", + "libc", + "libsqlite3-sys", + "log", + "memchr", + "once_cell", + "parking_lot", + "percent-encoding", + "serde", + "sha2", + "smallvec", + "sqlformat", + "sqlx-rt", + "stringprep", + "thiserror", + "tokio-stream", + "url", + "whoami", +] + +[[package]] +name = "sqlx-macros" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "01caee2b3935b4efe152f3262afbe51546ce3b1fc27ad61014e1b3cf5f55366e" +dependencies = [ + "dotenv", + "either", + "futures", + "heck", + "hex", + "once_cell", + "proc-macro2", + "quote", + "serde", + "serde_json", + "sha2", + "sqlx-core", + "sqlx-rt", + "syn", + "url", +] + +[[package]] +name = "sqlx-rt" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ce2e16b6774c671cc183e1d202386fdf9cde1e8468c1894a7f2a63eb671c4f4" +dependencies = [ + "native-tls", + "once_cell", + "tokio", + "tokio-native-tls", +] + [[package]] name = "standback" version = "0.2.17" @@ -2054,6 +2417,16 @@ dependencies = [ "quote", ] +[[package]] +name = "stringprep" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ee348cb74b87454fff4b551cbf727025810a004f88aeacae7f85b87f4e9a1c1" +dependencies = [ + "unicode-bidi", + "unicode-normalization", +] + [[package]] name = "subtle" version = "2.4.0" @@ -2083,6 +2456,12 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "tempfile" version = "3.2.0" @@ -2113,6 +2492,7 @@ name = "tenebrous-dicebot" version = "0.10.0" dependencies = [ "async-trait", + "barrel", "bincode", "byteorder", "combine", @@ -2125,11 +2505,13 @@ dependencies = [ "log", "matrix-sdk", "memmem", - "nom", + "nom 5.1.2", "phf", "rand 0.8.3", + "refinery", "serde", "sled", + "sqlx", "thiserror", "tokio", "toml", @@ -2270,6 +2652,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-stream" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8864d706fdb3cc0843a49647ac892720dac98a6eeb818b77190592cf4994066" +dependencies = [ + "futures-core", + "pin-project-lite", + "tokio", +] + [[package]] name = "tokio-util" version = "0.6.7" @@ -2371,6 +2764,12 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "unicode-segmentation" +version = "1.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb0d2e7be6ae3a5fa87eed5fb451aff96f2573d2694942e40543ae0bbe19c796" + [[package]] name = "unicode-width" version = "0.1.8" @@ -2383,6 +2782,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" +[[package]] +name = "unicode_categories" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e" + [[package]] name = "unindent" version = "0.1.7" @@ -2439,6 +2844,17 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe" +[[package]] +name = "walkdir" +version = "2.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "808cf2735cd4b6866113f648b791c6adc5714537bc222d9347bb203386ffda56" +dependencies = [ + "same-file", + "winapi", + "winapi-util", +] + [[package]] name = "want" version = "0.3.0" @@ -2539,6 +2955,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "whoami" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4abacf325c958dfeaf1046931d37f2a901b6dfe0968ee965a29e94c6766b2af6" +dependencies = [ + "wasm-bindgen", + "web-sys", +] + [[package]] name = "wildmatch" version = "2.1.0" @@ -2585,6 +3011,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "wyz" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85e60b0d1b5f99db2556934e21937020776a5d31520bf169e851ac44e6420214" + [[package]] name = "xml5ever" version = "0.16.1" diff --git a/Cargo.toml b/Cargo.toml index ec26237..acb3a22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,10 @@ repository = 'https://git.agnos.is/projectmoon/matrix-dicebot' keywords = ["games", "dice", "matrix", "bot"] categories = ["games"] +[[bin]] +name = "dicebot-migrate" +path = "src/migrate_cli.rs" + [dependencies] log = "0.4" env_logger = "0.8" @@ -32,6 +36,12 @@ bincode = "1.3" html2text = "0.2" phf = { version = "0.8", features = ["macros"] } matrix-sdk = { git = "https://github.com/matrix-org/matrix-rust-sdk", branch = "master" } +refinery = { version = "0.5", features = ["rusqlite"]} +barrel = { version = "0.6", features = ["sqlite3"] } + +[dependencies.sqlx] +version = "0.5" +features = [ "offline", "sqlite", "runtime-tokio-native-tls" ] [dependencies.serde] version = "1" diff --git a/sqlx-data.json b/sqlx-data.json new file mode 100644 index 0000000..9d050f1 --- /dev/null +++ b/sqlx-data.json @@ -0,0 +1,45 @@ +{ + "db": "SQLite", + "636b1b868eaf04cd234fbf17747d94a66e81f7bc1b060ba14151dbfaf40eeefc": { + "query": "SELECT value as \"value: i32\" FROM user_variables\n WHERE user_id = ? AND room_id = ? AND key = ?", + "describe": { + "columns": [ + { + "name": "value: i32", + "ordinal": 0, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 3 + }, + "nullable": [ + false + ] + } + }, + "d6558668b7395b95ded8da71c80963ddde957abdcc3c68b03431f8e904e0d21f": { + "query": "SELECT key, value as \"value: i32\" FROM user_variables\n WHERE room_id = ?", + "describe": { + "columns": [ + { + "name": "key", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "value: i32", + "ordinal": 1, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + } + } +} \ No newline at end of file diff --git a/src/bin/dicebot.rs b/src/bin/dicebot.rs index 1f4ff5c..8ed69ff 100644 --- a/src/bin/dicebot.rs +++ b/src/bin/dicebot.rs @@ -7,6 +7,7 @@ use tenebrous_dicebot::bot::DiceBot; use tenebrous_dicebot::config::*; use tenebrous_dicebot::db::Database; use tenebrous_dicebot::error::BotError; +use tenebrous_dicebot::migrator; use tenebrous_dicebot::state::DiceBotState; #[tokio::main] @@ -33,6 +34,9 @@ async fn run() -> Result<(), BotError> { db.migrate(cfg.migration_version())?; + let sqlite_path = format!("{}/dicebot.sqlite", cfg.database_path()); + migrator::migrate(&sqlite_path).await?; + match DiceBot::new(&cfg, &state, &db) { Ok(bot) => bot.run().await?, Err(e) => println!("Error connecting: {:?}", e), diff --git a/src/db.rs b/src/db.rs index b50e92a..4036e3c 100644 --- a/src/db.rs +++ b/src/db.rs @@ -12,6 +12,7 @@ pub mod errors; pub mod migrations; pub mod rooms; pub mod schema; +pub mod sqlite; pub mod state; pub mod variables; diff --git a/src/db/sqlite/errors.rs b/src/db/sqlite/errors.rs new file mode 100644 index 0000000..0078b4e --- /dev/null +++ b/src/db/sqlite/errors.rs @@ -0,0 +1,81 @@ +use sled::transaction::{TransactionError, UnabortableTransactionError}; +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum MigrationError { + #[error("cannot downgrade to an older database version")] + CannotDowngrade, + + #[error("migration for version {0} not defined")] + MigrationNotFound(u32), + + #[error("migration failed: {0}")] + MigrationFailed(String), +} + +//TODO better combining of key and value in certain errors (namely +//I32SchemaViolation). +#[derive(Error, Debug)] +pub enum DataError { + #[error("value does not exist for key: {0}")] + KeyDoesNotExist(String), + + #[error("too many entries")] + TooManyEntries, + + #[error("expected i32, but i32 schema was violated")] + I32SchemaViolation, + + #[error("unexpected or corruptd data bytes")] + InvalidValue, + + #[error("expected string ref, but utf8 schema was violated: {0}")] + Utf8RefSchemaViolation(#[from] std::str::Utf8Error), + + #[error("expected string, but utf8 schema was violated: {0}")] + Utf8SchemaViolation(#[from] std::string::FromUtf8Error), + + #[error("internal database error: {0}")] + InternalError(#[from] sled::Error), + + #[error("transaction error: {0}")] + TransactionError(#[from] sled::transaction::TransactionError), + + #[error("unabortable transaction error: {0}")] + UnabortableTransactionError(#[from] UnabortableTransactionError), + + #[error("data migration error: {0}")] + MigrationError(#[from] MigrationError), + + #[error("deserialization error: {0}")] + DeserializationError(#[from] bincode::Error), + + #[error("sqlx error: {0}")] + SqlxError(#[from] sqlx::Error), +} + +/// This From implementation is necessary to deal with the recursive +/// error type in the error enum. We defined a transaction error, but +/// the only place we use it is when converting from +/// sled::transaction::TransactionError. This converter +/// extracts the inner data error from transaction aborted errors, and +/// forwards anything else onward as-is, but wrapped in DataError. +impl From> for DataError { + fn from(error: TransactionError) -> Self { + match error { + TransactionError::Abort(data_err) => data_err, + TransactionError::Storage(storage_err) => { + DataError::TransactionError(TransactionError::Storage(storage_err)) + } + } + } +} + +/// Automatically aborts transactions that hit a DataError by using +/// the try (question mark) operator when this trait implementation is +/// in scope. +impl From for sled::transaction::ConflictableTransactionError { + fn from(error: DataError) -> Self { + sled::transaction::ConflictableTransactionError::Abort(error) + } +} diff --git a/src/db/sqlite/mod.rs b/src/db/sqlite/mod.rs new file mode 100644 index 0000000..ac49130 --- /dev/null +++ b/src/db/sqlite/mod.rs @@ -0,0 +1,80 @@ +use async_trait::async_trait; +use errors::DataError; +use sqlx::sqlite::{SqliteConnectOptions, SqlitePool, SqlitePoolOptions}; +use sqlx::ConnectOptions; +use sqlx::Connection; +use std::collections::HashMap; +use std::path::Path; +use std::str::FromStr; + +pub mod errors; +pub mod variables; + +// TODO move this up to the top once we delete sled. Traits will be the +// main API, then we can have different impls for different DBs. +#[async_trait] +pub(crate) trait Variables { + async fn get_user_variables( + &self, + user: &str, + room_id: &str, + ) -> Result, DataError>; + + async fn get_variable_count(&self, user: &str, room_id: &str) -> Result; + + async fn get_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + ) -> Result; + + async fn set_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + value: i32, + ) -> Result<(), DataError>; + + async fn delete_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + ) -> Result<(), DataError>; +} + +pub struct Database { + conn: SqlitePool, +} + +impl Database { + fn new_db(conn: SqlitePool) -> Result { + let database = Database { conn: conn.clone() }; + + Ok(database) + } + + pub async fn new(path: &str) -> Result { + //Create database if missing. + let conn = SqliteConnectOptions::from_str(path)? + .create_if_missing(true) + .connect() + .await?; + + drop(conn); + + //Return actual conncetion pool. + let conn = SqlitePoolOptions::new() + .max_connections(5) + .connect(path) + .await?; + + Self::new_db(conn) + } + + pub async fn new_temp() -> Result { + Self::new("sqlite::memory:").await + } +} diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs new file mode 100644 index 0000000..6e30edd --- /dev/null +++ b/src/db/sqlite/variables.rs @@ -0,0 +1,92 @@ +use super::errors::DataError; +use super::{Database, Variables}; +use async_trait::async_trait; +use std::collections::HashMap; + +struct UserVariableRow { + key: String, + value: i32, +} + +#[async_trait] +impl Variables for Database { + async fn get_user_variables( + &self, + user: &str, + room_id: &str, + ) -> Result, DataError> { + let rows = sqlx::query!( + r#"SELECT key, value as "value: i32" FROM user_variables + WHERE room_id = ?"#, + room_id, + ) + .fetch_all(&self.conn) + .await?; + + Ok(rows.into_iter().map(|row| (row.key, row.value)).collect()) + } + + async fn get_variable_count(&self, user: &str, room_id: &str) -> Result { + Ok(1) + } + + async fn get_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + ) -> Result { + let row = sqlx::query!( + r#"SELECT value as "value: i32" FROM user_variables + WHERE user_id = ? AND room_id = ? AND key = ?"#, + user, + room_id, + variable_name + ) + .fetch_one(&self.conn) + .await?; + + Ok(row.value) + } + + async fn set_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + value: i32, + ) -> Result<(), DataError> { + sqlx::query( + "INSERT INTO user_variables + (user_id, room_id, variable_name, value) + values (?, ?, ?, ?)", + ) + .bind(user) + .bind(room_id) + .bind(variable_name) + .bind(value) + .execute(&self.conn) + .await?; + + Ok(()) + } + + async fn delete_user_variable( + &self, + user: &str, + room_id: &str, + variable_name: &str, + ) -> Result<(), DataError> { + sqlx::query( + "DELETE FROM user_variables + WHERE user_id = ? AND room_id = ? AND variable_name = ?", + ) + .bind(user) + .bind(room_id) + .bind(variable_name) + .execute(&self.conn) + .await?; + + Ok(()) + } +} diff --git a/src/error.rs b/src/error.rs index 69b2004..ca7bd93 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,6 @@ -use crate::commands::CommandError; use crate::config::ConfigError; use crate::db::errors::DataError; +use crate::{commands::CommandError, migrator::migrations}; use thiserror::Error; #[derive(Error, Debug)] @@ -73,6 +73,9 @@ pub enum BotError { #[error("database error")] DatabaseError(#[from] sled::Error), + #[error("database migration error: {0}")] + SqliteError(#[from] crate::migrator::MigrationError), + #[error("too many commands or message was too large")] MessageTooLarge, diff --git a/src/lib.rs b/src/lib.rs index d67ade8..5c27f63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ pub mod error; mod help; pub mod logic; pub mod matrix; +pub mod migrator; pub mod models; mod parser; pub mod state; diff --git a/src/migrate_cli.rs b/src/migrate_cli.rs new file mode 100644 index 0000000..11cbbbc --- /dev/null +++ b/src/migrate_cli.rs @@ -0,0 +1,17 @@ +use std::env; + +pub mod migrator; + +#[tokio::main] +async fn main() -> Result<(), migrator::MigrationError> { + let args: Vec = env::args().collect(); + let db_path: &str = match &args[..] { + [_, path] => path.as_ref(), + [_, _, ..] => panic!("Expected exactly 0 or 1 argument"), + _ => "dicebot.sqlite", + }; + + println!("Using database: {}", db_path); + + migrator::migrate(db_path).await +} diff --git a/src/migrator.rs b/src/migrator.rs new file mode 100644 index 0000000..33be059 --- /dev/null +++ b/src/migrator.rs @@ -0,0 +1,33 @@ +use log::info; +use refinery::config::{Config, ConfigDbType}; +use sqlx::sqlite::SqliteConnectOptions; +use sqlx::ConnectOptions; +use std::str::FromStr; +use thiserror::Error; + +pub mod migrations; + +#[derive(Error, Debug)] +pub enum MigrationError { + #[error("sqlite connection error: {0}")] + SqlxError(#[from] sqlx::Error), + + #[error("refinery migration error: {0}")] + RefineryError(#[from] refinery::Error), +} + +/// Run database migrations against the sqlite database. +pub async fn migrate(db_path: &str) -> Result<(), MigrationError> { + //Create database if missing. + let conn = SqliteConnectOptions::from_str(&format!("sqlite://{}", db_path))? + .create_if_missing(true) + .connect() + .await?; + + drop(conn); + + let mut conn = Config::new(ConfigDbType::Sqlite).set_db_path(db_path); + info!("Running migrations"); + migrations::runner().run(&mut conn)?; + Ok(()) +} diff --git a/src/migrator/migrations/V1__variables.rs b/src/migrator/migrations/V1__variables.rs new file mode 100644 index 0000000..298233a --- /dev/null +++ b/src/migrator/migrations/V1__variables.rs @@ -0,0 +1,18 @@ +use barrel::backend::Sqlite; +use barrel::{types, Migration}; +use log::info; + +pub fn migration() -> String { + let mut m = Migration::new(); + info!("Applying migration: {}", file!()); + + m.create_table("user_variables", |t| { + t.add_column("id", types::primary()); + t.add_column("room_id", types::text()); + t.add_column("user_id", types::text()); + t.add_column("key", types::text()); + t.add_column("value", types::integer()); + }); + + m.make::() +} diff --git a/src/migrator/migrations/V2__rooms.rs b/src/migrator/migrations/V2__rooms.rs new file mode 100644 index 0000000..14f463b --- /dev/null +++ b/src/migrator/migrations/V2__rooms.rs @@ -0,0 +1,31 @@ +use barrel::backend::Sqlite; +use barrel::{types, Migration}; +use log::info; + +pub fn migration() -> String { + let mut m = Migration::new(); + info!("Applying migration: {}", file!()); + + //Table for basic room information: room ID, room name + m.create_table("room_info", move |t| { + t.add_column("id", types::primary()); + t.add_column("room_id", types::text()); + t.add_column("room_name", types::text()); + }); + + //Table of users in rooms. + m.create_table("room_users", move |t| { + t.add_column("id", types::primary()); + t.add_column("room_id", types::text()); + t.add_column("username", types::text()); + }); + + //Table of room ID, event ID, event timestamp + m.create_table("room_events", move |t| { + t.add_column("id", types::primary()); + t.add_column("room_id", types::text()); + t.add_column("event_id", types::text()); + t.add_column("event_timestamp", types::integer()); + }); + m.make::() +} diff --git a/src/migrator/migrations/mod.rs b/src/migrator/migrations/mod.rs new file mode 100644 index 0000000..b237433 --- /dev/null +++ b/src/migrator/migrations/mod.rs @@ -0,0 +1,2 @@ +use refinery::include_migration_mods; +include_migration_mods!("src/migrator/migrations"); -- 2.40.1 From cf9ce638929e43c8f1cf2fd39dffda916cc6d1b5 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sat, 15 May 2021 23:45:26 +0000 Subject: [PATCH 02/17] Replace application-level database connectivity. - Some database methods not yet implemented. - Unit tests create temp files that are not cleaned up (but they should be). --- .gitignore | 1 + Cargo.lock | 1 + Cargo.toml | 3 ++ src/bin/dicebot-cmd.rs | 4 +-- src/bin/dicebot.rs | 8 ++--- src/bot.rs | 7 +++-- src/bot/event_handlers.rs | 19 +++++------ src/cofd/dice.rs | 41 +++++++++++++++--------- src/commands.rs | 13 ++++++-- src/commands/variables.rs | 26 ++++++++++------ src/context.rs | 2 +- src/cthulhu/dice.rs | 64 ++++++++++++++++++++++++++++---------- src/db/sqlite/mod.rs | 44 +++++++++++++++++++++++--- src/db/sqlite/rooms.rs | 43 +++++++++++++++++++++++++ src/db/sqlite/state.rs | 14 +++++++++ src/db/sqlite/variables.rs | 2 +- src/dice.rs | 7 +++-- src/error.rs | 3 ++ src/logic.rs | 24 +++++++++----- 19 files changed, 250 insertions(+), 76 deletions(-) create mode 100644 src/db/sqlite/rooms.rs create mode 100644 src/db/sqlite/state.rs diff --git a/.gitignore b/.gitignore index 5030d40..6cb4892 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ bot-db* bigboy .#* *.sqlite +.tmp* diff --git a/Cargo.lock b/Cargo.lock index 7e0aea0..3c6318c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2512,6 +2512,7 @@ dependencies = [ "serde", "sled", "sqlx", + "tempfile", "thiserror", "tokio", "toml", diff --git a/Cargo.toml b/Cargo.toml index acb3a22..93eb562 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,3 +50,6 @@ features = ['derive'] [dependencies.tokio] version = "1" features = [ "full" ] + +[dev-dependencies] +tempfile = "3" \ No newline at end of file diff --git a/src/bin/dicebot-cmd.rs b/src/bin/dicebot-cmd.rs index 79ce2b9..b80fc12 100644 --- a/src/bin/dicebot-cmd.rs +++ b/src/bin/dicebot-cmd.rs @@ -2,7 +2,7 @@ use matrix_sdk::identifiers::room_id; use tenebrous_dicebot::commands; use tenebrous_dicebot::commands::ResponseExtractor; use tenebrous_dicebot::context::{Context, RoomContext}; -use tenebrous_dicebot::db::Database; +use tenebrous_dicebot::db::sqlite::Database; use tenebrous_dicebot::error::BotError; use url::Url; @@ -17,7 +17,7 @@ async fn main() -> Result<(), BotError> { let homeserver = Url::parse("http://example.com")?; let context = Context { - db: Database::new_temp()?, + db: Database::new_temp().await?, matrix_client: &matrix_sdk::Client::new(homeserver) .expect("Could not create matrix client"), room: RoomContext { diff --git a/src/bin/dicebot.rs b/src/bin/dicebot.rs index 8ed69ff..d4ee65f 100644 --- a/src/bin/dicebot.rs +++ b/src/bin/dicebot.rs @@ -5,7 +5,7 @@ use log::error; use std::sync::{Arc, RwLock}; use tenebrous_dicebot::bot::DiceBot; use tenebrous_dicebot::config::*; -use tenebrous_dicebot::db::Database; +use tenebrous_dicebot::db::sqlite::Database; use tenebrous_dicebot::error::BotError; use tenebrous_dicebot::migrator; use tenebrous_dicebot::state::DiceBotState; @@ -29,12 +29,10 @@ async fn run() -> Result<(), BotError> { .expect("Need a config as an argument"); let cfg = Arc::new(read_config(config_path)?); - let db = Database::new(&cfg.database_path())?; + let sqlite_path = format!("{}/dicebot.sqlite", cfg.database_path()); + let db = Database::new(&sqlite_path).await?; let state = Arc::new(RwLock::new(DiceBotState::new(&cfg))); - db.migrate(cfg.migration_version())?; - - let sqlite_path = format!("{}/dicebot.sqlite", cfg.database_path()); migrator::migrate(&sqlite_path).await?; match DiceBot::new(&cfg, &state, &db) { diff --git a/src/bot.rs b/src/bot.rs index b3d8531..d7e11d1 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1,7 +1,8 @@ use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor}; use crate::config::*; use crate::context::{Context, RoomContext}; -use crate::db::Database; +use crate::db::sqlite::Database; +use crate::db::sqlite::DbState; use crate::error::BotError; use crate::matrix; use crate::state::DiceBotState; @@ -134,7 +135,7 @@ impl DiceBot { // Pull device ID from database, if it exists. Then write it // to DB if the library generated one for us. - let device_id: Option = self.db.state.get_device_id()?; + let device_id: Option = self.db.get_device_id().await?; let device_id: Option<&str> = device_id.as_deref(); client @@ -143,7 +144,7 @@ impl DiceBot { if device_id.is_none() { let device_id = client.device_id().await.ok_or(BotError::NoDeviceIdFound)?; - self.db.state.set_device_id(device_id.as_str())?; + self.db.set_device_id(device_id.as_str()).await?; info!("Recorded new device ID: {}", device_id.as_str()); } else { info!("Using existing device ID: {}", device_id.unwrap()); diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index 98f4072..7ceb1b5 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -4,7 +4,8 @@ * SDK example code. */ use super::DiceBot; -use crate::db::Database; +use crate::db::sqlite::Database; +use crate::db::sqlite::Rooms; use crate::error::BotError; use crate::logic::record_room_information; use async_trait::async_trait; @@ -90,9 +91,9 @@ async fn should_process_message<'a>( Ok((msg_body, sender_username)) } -fn should_process_event(db: &Database, room_id: &str, event_id: &str) -> bool { - db.rooms - .should_process(room_id, event_id) +async fn should_process_event(db: &Database, room_id: &str, event_id: &str) -> bool { + db.should_process(room_id, event_id) + .await .unwrap_or_else(|e| { error!( "Database error when checking if we should process an event: {}", @@ -116,7 +117,7 @@ impl EventHandler for DiceBot { let room_id_str = room_id.as_str(); let username = &event.state_key; - if !should_process_event(&self.db, room_id_str, event.event_id.as_str()) { + if !should_process_event(&self.db, room_id_str, event.event_id.as_str()).await { return; } @@ -135,7 +136,7 @@ impl EventHandler for DiceBot { let result = if event_affects_us && !adding_user { info!("Clearing all information for room ID {}", room_id); - self.db.rooms.clear_info(room_id_str) + self.db.clear_info(room_id_str).await } else if event_affects_us && adding_user { info!("Joined room {}; recording room information", room_id); record_room_information( @@ -148,10 +149,10 @@ impl EventHandler for DiceBot { .await } else if !event_affects_us && adding_user { info!("Adding user {} to room ID {}", username, room_id); - self.db.rooms.add_user_to_room(username, room_id_str) + self.db.add_user_to_room(username, room_id_str).await } else if !event_affects_us && !adding_user { info!("Removing user {} from room ID {}", username, room_id); - self.db.rooms.remove_user_from_room(username, room_id_str) + self.db.remove_user_from_room(username, room_id_str).await } else { debug!("Ignoring a room member event: {:#?}", event); Ok(()) @@ -196,7 +197,7 @@ impl EventHandler for DiceBot { }; let room_id = room.room_id().as_str(); - if !should_process_event(&self.db, room_id, event.event_id.as_str()) { + if !should_process_event(&self.db, room_id, event.event_id.as_str()).await { return; } diff --git a/src/cofd/dice.rs b/src/cofd/dice.rs index 5cf86d2..f269b1c 100644 --- a/src/cofd/dice.rs +++ b/src/cofd/dice.rs @@ -325,7 +325,8 @@ pub async fn roll_pool(pool: &DicePoolWithContext<'_>) -> Result for ExecutionError { fn from(error: crate::db::errors::DataError) -> Self { - Self(BotError::DataError(error)) + Self(DataError(error)) + } +} + +impl From for ExecutionError { + fn from(error: crate::db::sqlite::errors::DataError) -> Self { + Self(SqliteDataError(error)) } } @@ -129,9 +136,9 @@ mod tests { )); } - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn unrecognized_command() { - let db = crate::db::Database::new_temp().unwrap(); + let db = crate::db::sqlite::Database::new_temp().await.unwrap(); let homeserver = Url::parse("http://example.com").unwrap(); let ctx = Context { db: db, diff --git a/src/commands/variables.rs b/src/commands/variables.rs index 326041d..c03ca70 100644 --- a/src/commands/variables.rs +++ b/src/commands/variables.rs @@ -1,6 +1,7 @@ use super::{Command, Execution, ExecutionResult}; use crate::context::Context; -use crate::db::errors::DataError; +use crate::db::sqlite::errors::DataError; +use crate::db::sqlite::Variables; use crate::db::variables::UserAndRoom; use async_trait::async_trait; @@ -13,8 +14,10 @@ impl Command for GetAllVariablesCommand { } async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult { - let key = UserAndRoom(&ctx.username, &ctx.room_id().as_str()); - let variables = ctx.db.variables.get_user_variables(&key)?; + let variables = ctx + .db + .get_user_variables(&ctx.username, ctx.room_id().as_str()) + .await?; let mut variable_list: Vec = variables .into_iter() @@ -43,8 +46,10 @@ impl Command for GetVariableCommand { async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult { let name = &self.0; - let key = UserAndRoom(&ctx.username, &ctx.room_id().as_str()); - let result = ctx.db.variables.get_user_variable(&key, name); + let result = ctx + .db + .get_user_variable(&ctx.username, ctx.room_id().as_str(), name) + .await; let value = match result { Ok(num) => format!("{} = {}", name, num), @@ -68,9 +73,10 @@ impl Command for SetVariableCommand { async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult { let name = &self.0; let value = self.1; - let key = UserAndRoom(&ctx.username, ctx.room_id().as_str()); - ctx.db.variables.set_user_variable(&key, name, value)?; + ctx.db + .set_user_variable(&ctx.username, ctx.room_id().as_str(), name, value) + .await?; let content = format!("{} = {}", name, value); let html = format!("Set Variable: {}", content); @@ -88,8 +94,10 @@ impl Command for DeleteVariableCommand { async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult { let name = &self.0; - let key = UserAndRoom(&ctx.username, ctx.room_id().as_str()); - let result = ctx.db.variables.delete_user_variable(&key, name); + let result = ctx + .db + .delete_user_variable(&ctx.username, ctx.room_id().as_str(), name) + .await; let value = match result { Ok(()) => format!("{} now unset", name), diff --git a/src/context.rs b/src/context.rs index e971b39..04f2b40 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,4 +1,4 @@ -use crate::db::Database; +use crate::db::sqlite::Database; use matrix_sdk::identifiers::RoomId; use matrix_sdk::room::Joined; use matrix_sdk::Client; diff --git a/src/cthulhu/dice.rs b/src/cthulhu/dice.rs index e13cf84..95a0469 100644 --- a/src/cthulhu/dice.rs +++ b/src/cthulhu/dice.rs @@ -1,7 +1,11 @@ +use crate::db::sqlite::Variables; use crate::error::{BotError, DiceRollingError}; use crate::parser::{Amount, Element}; use crate::{context::Context, db::variables::UserAndRoom}; use crate::{dice::calculate_single_die_amount, parser::DiceParsingError}; +use rand::rngs::StdRng; +use rand::Rng; +use rand::SeedableRng; use std::convert::TryFrom; use std::fmt; @@ -270,10 +274,11 @@ macro_rules! is_variable { }; } -///A version of DieRoller that uses a rand::Rng to roll numbers. -struct RngDieRoller(R); +/// A die roller than can have an RNG implementation injected, but +/// must be thread-safe. Required for the async dice rolling code. +struct RngDieRoller(R); -impl DieRoller for RngDieRoller { +impl DieRoller for RngDieRoller { fn roll(&mut self) -> u32 { self.0.gen_range(0..=9) } @@ -361,7 +366,7 @@ pub async fn regular_roll( let target = calculate_single_die_amount(&roll_with_ctx.0.amount, roll_with_ctx.1).await?; let target = u32::try_from(target).map_err(|_| DiceRollingError::InvalidAmount)?; - let mut roller = RngDieRoller(rand::thread_rng()); + let mut roller = RngDieRoller::(SeedableRng::from_entropy()); let rolled_dice = roll_regular_dice(&roll_with_ctx.0.modifier, target, &mut roller); Ok(ExecutedDiceRoll { @@ -371,11 +376,12 @@ pub async fn regular_roll( }) } -fn update_skill(ctx: &Context, variable: &str, value: u32) -> Result<(), BotError> { +async fn update_skill(ctx: &Context<'_>, variable: &str, value: u32) -> Result<(), BotError> { use std::convert::TryInto; let value: i32 = value.try_into()?; - let key = UserAndRoom(ctx.username, ctx.room_id().as_str()); - ctx.db.variables.set_user_variable(&key, variable, value)?; + ctx.db + .set_user_variable(&ctx.username, &ctx.room_id().as_str(), variable, value) + .await?; Ok(()) } @@ -397,12 +403,14 @@ pub async fn advancement_roll( return Err(DiceRollingError::InvalidAmount.into()); } - let mut roller = RngDieRoller(rand::thread_rng()); + let mut roller = RngDieRoller::(SeedableRng::from_entropy()); let roll = roll_advancement_dice(target, &mut roller); + drop(roller); + if roll.successful && is_variable!(existing_skill) { let variable_name: &str = extract_variable(existing_skill)?; - update_skill(roll_with_ctx.1, variable_name, roll.new_skill_amount())?; + update_skill(roll_with_ctx.1, variable_name, roll.new_skill_amount()).await?; } Ok(ExecutedAdvancementRoll { target, roll }) @@ -411,7 +419,7 @@ pub async fn advancement_roll( #[cfg(test)] mod tests { use super::*; - use crate::db::Database; + use crate::db::sqlite::Database; use crate::parser::{Amount, Element, Operator}; use url::Url; @@ -474,7 +482,7 @@ mod tests { assert!(matches!(result, Err(DiceParsingError::WrongElementType))); } - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn regular_roll_rejects_negative_numbers() { let roll = DiceRoll { amount: Amount { @@ -484,7 +492,15 @@ mod tests { modifier: DiceRollModifier::Normal, }; - let db = Database::new_temp().unwrap(); + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + let db = Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap(); + let homeserver = Url::parse("http://example.com").unwrap(); let ctx = Context { db: db, @@ -503,7 +519,7 @@ mod tests { )); } - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn advancement_roll_rejects_negative_numbers() { let roll = AdvancementRoll { existing_skill: Amount { @@ -512,7 +528,15 @@ mod tests { }, }; - let db = Database::new_temp().unwrap(); + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + let db = Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap(); + let homeserver = Url::parse("http://example.com").unwrap(); let ctx = Context { db: db, @@ -531,7 +555,7 @@ mod tests { )); } - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn advancement_roll_rejects_big_numbers() { let roll = AdvancementRoll { existing_skill: Amount { @@ -540,7 +564,15 @@ mod tests { }, }; - let db = Database::new_temp().unwrap(); + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + let db = Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap(); + let homeserver = Url::parse("http://example.com").unwrap(); let ctx = Context { db: db, diff --git a/src/db/sqlite/mod.rs b/src/db/sqlite/mod.rs index ac49130..426e2eb 100644 --- a/src/db/sqlite/mod.rs +++ b/src/db/sqlite/mod.rs @@ -2,14 +2,43 @@ use async_trait::async_trait; use errors::DataError; use sqlx::sqlite::{SqliteConnectOptions, SqlitePool, SqlitePoolOptions}; use sqlx::ConnectOptions; -use sqlx::Connection; -use std::collections::HashMap; -use std::path::Path; +use std::clone::Clone; +use std::collections::{HashMap, HashSet}; use std::str::FromStr; +use crate::models::RoomInfo; + pub mod errors; +pub mod rooms; +pub mod state; pub mod variables; +#[async_trait] +pub(crate) trait DbState { + async fn get_device_id(&self) -> Result, DataError>; + + async fn set_device_id(&self, device_id: &str) -> Result<(), DataError>; +} + +#[async_trait] +pub(crate) trait Rooms { + async fn should_process(&self, room_id: &str, event_id: &str) -> Result; + + async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError>; + + async fn get_room_info(&self, room_id: &str) -> Result, DataError>; + + async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError>; + + async fn get_users_in_room(&self, room_id: &str) -> Result, DataError>; + + async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError>; + + async fn remove_user_from_room(&self, username: &str, room_id: &str) -> Result<(), DataError>; + + async fn clear_info(&self, room_id: &str) -> Result<(), DataError>; +} + // TODO move this up to the top once we delete sled. Traits will be the // main API, then we can have different impls for different DBs. #[async_trait] @@ -52,7 +81,6 @@ pub struct Database { impl Database { fn new_db(conn: SqlitePool) -> Result { let database = Database { conn: conn.clone() }; - Ok(database) } @@ -78,3 +106,11 @@ impl Database { Self::new("sqlite::memory:").await } } + +impl Clone for Database { + fn clone(&self) -> Self { + Database { + conn: self.conn.clone(), + } + } +} diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs new file mode 100644 index 0000000..797c0d3 --- /dev/null +++ b/src/db/sqlite/rooms.rs @@ -0,0 +1,43 @@ +use super::errors::DataError; +use super::{Database, Rooms}; +use crate::models::RoomInfo; +use async_trait::async_trait; +use std::collections::{HashMap, HashSet}; + +#[async_trait] +impl Rooms for Database { + async fn should_process(&self, room_id: &str, event_id: &str) -> Result { + Ok(true) + } + + async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError> { + Ok(()) + } + + async fn get_room_info(&self, room_id: &str) -> Result, DataError> { + Ok(Some(RoomInfo { + room_id: "".to_string(), + room_name: "".to_string(), + })) + } + + async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError> { + Ok(HashSet::new()) + } + + async fn get_users_in_room(&self, room_id: &str) -> Result, DataError> { + Ok(HashSet::new()) + } + + async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { + Ok(()) + } + + async fn remove_user_from_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { + Ok(()) + } + + async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { + Ok(()) + } +} diff --git a/src/db/sqlite/state.rs b/src/db/sqlite/state.rs new file mode 100644 index 0000000..74a2a3f --- /dev/null +++ b/src/db/sqlite/state.rs @@ -0,0 +1,14 @@ +use super::errors::DataError; +use super::{Database, DbState}; +use async_trait::async_trait; + +#[async_trait] +impl DbState for Database { + async fn get_device_id(&self) -> Result, DataError> { + Ok(None) + } + + async fn set_device_id(&self, device_id: &str) -> Result<(), DataError> { + Ok(()) + } +} diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs index 6e30edd..0aa023d 100644 --- a/src/db/sqlite/variables.rs +++ b/src/db/sqlite/variables.rs @@ -58,7 +58,7 @@ impl Variables for Database { ) -> Result<(), DataError> { sqlx::query( "INSERT INTO user_variables - (user_id, room_id, variable_name, value) + (user_id, room_id, key, value) values (?, ?, ?, ?)", ) .bind(user) diff --git a/src/dice.rs b/src/dice.rs index 5b18867..897225d 100644 --- a/src/dice.rs +++ b/src/dice.rs @@ -1,4 +1,5 @@ use crate::context::Context; +use crate::db::sqlite::Variables; use crate::db::variables::UserAndRoom; use crate::error::BotError; use crate::error::DiceRollingError; @@ -22,8 +23,10 @@ pub async fn calculate_single_die_amount( /// it cannot find a variable defined, or if the database errors. pub async fn calculate_dice_amount(amounts: &[Amount], ctx: &Context<'_>) -> Result { let stream = stream::iter(amounts); - let key = UserAndRoom(&ctx.username, ctx.room_id().as_str()); - let variables = &ctx.db.variables.get_user_variables(&key)?; + let variables = &ctx + .db + .get_user_variables(&ctx.username, ctx.room_id().as_str()) + .await?; use DiceRollingError::VariableNotFound; let dice_amount: i32 = stream diff --git a/src/error.rs b/src/error.rs index ca7bd93..3efd786 100644 --- a/src/error.rs +++ b/src/error.rs @@ -21,6 +21,9 @@ pub enum BotError { #[error("database error: {0}")] DataError(#[from] DataError), + #[error("sqlite database error: {0}")] + SqliteDataError(#[from] crate::db::sqlite::errors::DataError), + #[error("the message should not be processed because it failed validation")] ShouldNotProcessError, diff --git a/src/logic.rs b/src/logic.rs index 3b03190..8375754 100644 --- a/src/logic.rs +++ b/src/logic.rs @@ -1,12 +1,14 @@ -use crate::db::errors::DataError; +use crate::db::sqlite::errors::DataError; +use crate::db::sqlite::Rooms; use crate::matrix; use crate::models::RoomInfo; +use futures::stream::{self, StreamExt, TryStreamExt}; use matrix_sdk::{self, identifiers::RoomId, Client}; /// Record the information about a room, including users in it. pub async fn record_room_information( client: &Client, - db: &crate::db::Database, + db: &crate::db::sqlite::Database, room_id: &RoomId, room_display_name: &str, our_username: &str, @@ -21,11 +23,19 @@ pub async fn record_room_information( // TODO this and the username adding should be one whole // transaction in the db. - db.rooms.insert_room_info(&info)?; + db.insert_room_info(&info).await?; - usernames + let filtered_usernames = usernames .into_iter() - .filter(|username| username != our_username) - .map(|username| db.rooms.add_user_to_room(&username, room_id_str)) - .collect() //Make use of collect impl on Result. + .filter(|username| username != our_username); + + // Async collect into vec of results, then use into_iter of result + // to go to from Result> to just Result<()>. Easier than + // attempting to async-collect our way to a single Result<()>. + stream::iter(filtered_usernames) + .then(|username| async move { db.add_user_to_room(&username, &room_id_str).await }) + .collect::>>() + .await + .into_iter() + .collect() } -- 2.40.1 From 9798821b7b10e88f4c68c2f933fd289970c086b8 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sun, 16 May 2021 14:17:03 +0000 Subject: [PATCH 03/17] Implement room and dbstate for sqlite. --- sqlx-data.json | 96 +++++++++++++++++++++ src/bot.rs | 6 -- src/db/sqlite/errors.rs | 5 ++ src/db/sqlite/rooms.rs | 111 +++++++++++++++++++++++-- src/db/sqlite/state.rs | 15 +++- src/migrator/migrations/V3__dbstate.rs | 15 ++++ 6 files changed, 235 insertions(+), 13 deletions(-) create mode 100644 src/migrator/migrations/V3__dbstate.rs diff --git a/sqlx-data.json b/sqlx-data.json index 9d050f1..835daf1 100644 --- a/sqlx-data.json +++ b/sqlx-data.json @@ -1,5 +1,47 @@ { "db": "SQLite", + "0ccdb5669e5819628096b8d2f81fef361f73dbcbc27428208a1a4e315a8c7880": { + "query": "SELECT room_id, room_name FROM room_info\n WHERE room_id = ?", + "describe": { + "columns": [ + { + "name": "room_id", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "room_name", + "ordinal": 1, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + } + }, + "19d89370cac05c1bc4de0eb3508712da9ca133b1cf9445b5407d238f89c3ab0c": { + "query": "SELECT device_id FROM bot_state limit 1", + "describe": { + "columns": [ + { + "name": "device_id", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 0 + }, + "nullable": [ + false + ] + } + }, "636b1b868eaf04cd234fbf17747d94a66e81f7bc1b060ba14151dbfaf40eeefc": { "query": "SELECT value as \"value: i32\" FROM user_variables\n WHERE user_id = ? AND room_id = ? AND key = ?", "describe": { @@ -18,6 +60,60 @@ ] } }, + "ad52bc29fc1eef2bbff8b5f7316047f81ea31b1f910d0e0226125fea89530aa2": { + "query": "SELECT room_id FROM room_users\n WHERE username = ?", + "describe": { + "columns": [ + { + "name": "room_id", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + } + }, + "b302d586e5ac4c72c2970361ea5a5936c0b8c6dad10033c626a0ce0404cadb25": { + "query": "SELECT username FROM room_users\n WHERE room_id = ?", + "describe": { + "columns": [ + { + "name": "username", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + } + }, + "bba0fc255e7c30d1d2d9468c68ba38db6e8a13be035aa1152933ba9247b14f8c": { + "query": "SELECT event_id FROM room_events\n WHERE room_id = ? AND event_id = ?", + "describe": { + "columns": [ + { + "name": "event_id", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + false + ] + } + }, "d6558668b7395b95ded8da71c80963ddde957abdcc3c68b03431f8e904e0d21f": { "query": "SELECT key, value as \"value: i32\" FROM user_variables\n WHERE room_id = ?", "describe": { diff --git a/src/bot.rs b/src/bot.rs index d7e11d1..b941789 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -161,12 +161,6 @@ impl DiceBot { let client = self.client.clone(); self.login(&client).await?; - // Initial sync without event handler prevents responding to - // messages received while bot was offline. TODO: selectively - // respond to old messages? e.g. comands missed while offline. - info!("Performing intial sync (no commands will be responded to)"); - self.client.sync_once(SyncSettings::default()).await?; - client.set_event_handler(Box::new(self)).await; info!("Listening for commands"); diff --git a/src/db/sqlite/errors.rs b/src/db/sqlite/errors.rs index 0078b4e..dd06b77 100644 --- a/src/db/sqlite/errors.rs +++ b/src/db/sqlite/errors.rs @@ -1,3 +1,5 @@ +use std::num::TryFromIntError; + use sled::transaction::{TransactionError, UnabortableTransactionError}; use thiserror::Error; @@ -52,6 +54,9 @@ pub enum DataError { #[error("sqlx error: {0}")] SqlxError(#[from] sqlx::Error), + + #[error("numeric conversion error")] + NumericConversionError(#[from] TryFromIntError), } /// This From implementation is necessary to deal with the recursive diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 797c0d3..9078a79 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -2,42 +2,141 @@ use super::errors::DataError; use super::{Database, Rooms}; use crate::models::RoomInfo; use async_trait::async_trait; +use sqlx::SqlitePool; use std::collections::{HashMap, HashSet}; +use std::time::{SystemTime, UNIX_EPOCH}; + +async fn record_event(conn: &SqlitePool, event_id: &str, room_id: &str) -> Result<(), DataError> { + use std::convert::TryFrom; + let now: i64 = i64::try_from( + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("Clock has gone backwards") + .as_secs(), + )?; + + sqlx::query( + r#"INSERT INTO room_events + (room_id, event_id, event_timestamp) + VALUES (?, ?, ?)"#, + ) + .bind(room_id) + .bind(event_id) + .bind(now) + .execute(conn) + .await?; + + Ok(()) +} #[async_trait] impl Rooms for Database { async fn should_process(&self, room_id: &str, event_id: &str) -> Result { - Ok(true) + let row = sqlx::query!( + r#"SELECT event_id FROM room_events + WHERE room_id = ? AND event_id = ?"#, + room_id, + event_id + ) + .fetch_optional(&self.conn) + .await?; + + match row { + Some(_) => Ok(false), + None => { + record_event(&self.conn, room_id, event_id).await?; + Ok(true) + } + } } async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError> { + sqlx::query(r#"INSERT INTO room_info (room_id, room_name) VALUES (?, ?)"#) + .bind(&info.room_id) + .bind(&info.room_name) + .execute(&self.conn) + .await?; + Ok(()) } async fn get_room_info(&self, room_id: &str) -> Result, DataError> { - Ok(Some(RoomInfo { - room_id: "".to_string(), - room_name: "".to_string(), + let info = sqlx::query!( + r#"SELECT room_id, room_name FROM room_info + WHERE room_id = ?"#, + room_id + ) + .fetch_optional(&self.conn) + .await?; + + Ok(info.map(|i| RoomInfo { + room_id: i.room_id, + room_name: i.room_name, })) } async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError> { - Ok(HashSet::new()) + let room_ids = sqlx::query!( + r#"SELECT room_id FROM room_users + WHERE username = ?"#, + user_id + ) + .fetch_all(&self.conn) + .await?; + + Ok(room_ids.into_iter().map(|row| row.room_id).collect()) } async fn get_users_in_room(&self, room_id: &str) -> Result, DataError> { - Ok(HashSet::new()) + let usernames = sqlx::query!( + r#"SELECT username FROM room_users + WHERE room_id = ?"#, + room_id + ) + .fetch_all(&self.conn) + .await?; + + Ok(usernames.into_iter().map(|row| row.username).collect()) } async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { + self.remove_user_from_room(username, room_id).await.ok(); + + sqlx::query("INSERT INTO room_users (room_id, username) VALUES (?, ?)") + .bind(room_id) + .bind(username) + .execute(&self.conn) + .await?; + Ok(()) } async fn remove_user_from_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { + sqlx::query("DELETE FROM room_users where username = ? AND room_id = ?") + .bind(username) + .bind(room_id) + .execute(&self.conn) + .await?; + Ok(()) } async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { + sqlx::query("DELETE FROM room_info where room_id = ?") + .bind(room_id) + .execute(&self.conn) + .await?; + + sqlx::query("DELETE FROM room_users where room_id = ?") + .bind(room_id) + .execute(&self.conn) + .await?; + + sqlx::query("DELETE FROM room_events where room_id = ?") + .bind(room_id) + .execute(&self.conn) + .await?; + Ok(()) } } diff --git a/src/db/sqlite/state.rs b/src/db/sqlite/state.rs index 74a2a3f..2830df4 100644 --- a/src/db/sqlite/state.rs +++ b/src/db/sqlite/state.rs @@ -5,10 +5,23 @@ use async_trait::async_trait; #[async_trait] impl DbState for Database { async fn get_device_id(&self) -> Result, DataError> { - Ok(None) + let state = sqlx::query!(r#"SELECT device_id FROM bot_state limit 1"#) + .fetch_optional(&self.conn) + .await?; + + Ok(state.map(|s| s.device_id)) } async fn set_device_id(&self, device_id: &str) -> Result<(), DataError> { + sqlx::query( + r#"INSERT INTO bot_state + (device_id) + VALUES (?)"#, + ) + .bind(device_id) + .execute(&self.conn) + .await?; + Ok(()) } } diff --git a/src/migrator/migrations/V3__dbstate.rs b/src/migrator/migrations/V3__dbstate.rs new file mode 100644 index 0000000..0ae82c2 --- /dev/null +++ b/src/migrator/migrations/V3__dbstate.rs @@ -0,0 +1,15 @@ +use barrel::backend::Sqlite; +use barrel::{types, Migration}; +use log::info; + +pub fn migration() -> String { + let mut m = Migration::new(); + info!("Applying migration: {}", file!()); + + //Basic state table with only device ID for now. Uses only one row. + m.create_table("bot_state", move |t| { + t.add_column("device_id", types::text()); + }); + + m.make::() +} -- 2.40.1 From bfc5609ab6704a82028ff4d51ca524bdbcf1ca82 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sun, 16 May 2021 21:39:19 +0000 Subject: [PATCH 04/17] Add proper constraints to db tables. Report errors when listing users. --- src/bot.rs | 18 ++++++---- src/bot/event_handlers.rs | 12 +++++-- src/error.rs | 4 +-- src/logic.rs | 18 +++++++--- src/matrix.rs | 13 +++++--- src/migrator/migrations/V1__variables.rs | 1 - src/migrator/migrations/V2__rooms.rs | 26 ++++++++------- src/migrator/migrations/V4__room_events.rs | 38 ++++++++++++++++++++++ 8 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 src/migrator/migrations/V4__room_events.rs diff --git a/src/bot.rs b/src/bot.rs index b941789..926a76a 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -161,19 +161,25 @@ impl DiceBot { let client = self.client.clone(); self.login(&client).await?; + // Initial sync without event handler prevents responding to + // messages received while bot was offline. TODO: selectively + // respond to old messages? e.g. comands missed while offline. + //info!("Performing intial sync (no commands will be responded to)"); + // self.client.sync_once(SyncSettings::default()).await?; + client.set_event_handler(Box::new(self)).await; info!("Listening for commands"); - let token = client - .sync_token() - .await - .ok_or(BotError::SyncTokenRequired)?; + // let token = client + // .sync_token() + // .await + // .ok_or(BotError::SyncTokenRequired)?; - let settings = SyncSettings::default().token(token); + // let settings = SyncSettings::default().token(token); // TODO replace with sync_with_callback for cleaner shutdown // process. - client.sync(settings).await; + client.sync(SyncSettings::default()).await; Ok(()) } diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index 7ceb1b5..24ac3c1 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -136,7 +136,7 @@ impl EventHandler for DiceBot { let result = if event_affects_us && !adding_user { info!("Clearing all information for room ID {}", room_id); - self.db.clear_info(room_id_str).await + self.db.clear_info(room_id_str).await.map_err(|e| e.into()) } else if event_affects_us && adding_user { info!("Joined room {}; recording room information", room_id); record_room_information( @@ -149,10 +149,16 @@ impl EventHandler for DiceBot { .await } else if !event_affects_us && adding_user { info!("Adding user {} to room ID {}", username, room_id); - self.db.add_user_to_room(username, room_id_str).await + self.db + .add_user_to_room(username, room_id_str) + .await + .map_err(|e| e.into()) } else if !event_affects_us && !adding_user { info!("Removing user {} from room ID {}", username, room_id); - self.db.remove_user_from_room(username, room_id_str).await + self.db + .remove_user_from_room(username, room_id_str) + .await + .map_err(|e| e.into()) } else { debug!("Ignoring a room member event: {:#?}", event); Ok(()) diff --git a/src/error.rs b/src/error.rs index 3efd786..f091d98 100644 --- a/src/error.rs +++ b/src/error.rs @@ -36,10 +36,10 @@ pub enum BotError { #[error("error in matrix state store: {0}")] MatrixStateStoreError(#[from] matrix_sdk::StoreError), - #[error("uncategorized matrix SDK error")] + #[error("uncategorized matrix SDK error: {0}")] MatrixError(#[from] matrix_sdk::Error), - #[error("uncategorized matrix SDK base error")] + #[error("uncategorized matrix SDK base error: {0}")] MatrixBaseError(#[from] matrix_sdk::BaseError), #[error("future canceled")] diff --git a/src/logic.rs b/src/logic.rs index 8375754..5be59a6 100644 --- a/src/logic.rs +++ b/src/logic.rs @@ -1,5 +1,6 @@ use crate::db::sqlite::errors::DataError; use crate::db::sqlite::Rooms; +use crate::error::BotError; use crate::matrix; use crate::models::RoomInfo; use futures::stream::{self, StreamExt, TryStreamExt}; @@ -12,9 +13,12 @@ pub async fn record_room_information( room_id: &RoomId, room_display_name: &str, our_username: &str, -) -> Result<(), DataError> { +) -> Result<(), BotError> { + //Clear out any old room info first. + db.clear_info(room_id.as_str()).await?; + let room_id_str = room_id.as_str(); - let usernames = matrix::get_users_in_room(&client, &room_id).await; + let usernames = matrix::get_users_in_room(&client, &room_id).await?; let info = RoomInfo { room_id: room_id_str.to_owned(), @@ -29,12 +33,18 @@ pub async fn record_room_information( .into_iter() .filter(|username| username != our_username); + println!("Users to add to room: {:?}", filtered_usernames); + // Async collect into vec of results, then use into_iter of result // to go to from Result> to just Result<()>. Easier than // attempting to async-collect our way to a single Result<()>. stream::iter(filtered_usernames) - .then(|username| async move { db.add_user_to_room(&username, &room_id_str).await }) - .collect::>>() + .then(|username| async move { + db.add_user_to_room(&username, &room_id_str) + .await + .map_err(|e| e.into()) + }) + .collect::>>() .await .into_iter() .collect() diff --git a/src/matrix.rs b/src/matrix.rs index 173a2ae..7ff73c4 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -20,16 +20,19 @@ fn extract_error_message(error: MatrixError) -> String { } /// Retrieve a list of users in a given room. -pub async fn get_users_in_room(client: &Client, room_id: &RoomId) -> Vec { +pub async fn get_users_in_room( + client: &Client, + room_id: &RoomId, +) -> Result, MatrixError> { if let Some(joined_room) = client.get_joined_room(room_id) { - let members = joined_room.joined_members().await.ok().unwrap_or_default(); + let members = joined_room.joined_members().await?; - members + Ok(members .into_iter() .map(|member| member.user_id().to_string()) - .collect() + .collect()) } else { - vec![] + Ok(vec![]) } } diff --git a/src/migrator/migrations/V1__variables.rs b/src/migrator/migrations/V1__variables.rs index 298233a..ec8f063 100644 --- a/src/migrator/migrations/V1__variables.rs +++ b/src/migrator/migrations/V1__variables.rs @@ -7,7 +7,6 @@ pub fn migration() -> String { info!("Applying migration: {}", file!()); m.create_table("user_variables", |t| { - t.add_column("id", types::primary()); t.add_column("room_id", types::text()); t.add_column("user_id", types::text()); t.add_column("key", types::text()); diff --git a/src/migrator/migrations/V2__rooms.rs b/src/migrator/migrations/V2__rooms.rs index 14f463b..5568a01 100644 --- a/src/migrator/migrations/V2__rooms.rs +++ b/src/migrator/migrations/V2__rooms.rs @@ -1,31 +1,33 @@ use barrel::backend::Sqlite; -use barrel::{types, Migration}; +use barrel::{types, types::Type, Migration}; use log::info; +fn primary_uuid() -> Type { + types::text().unique(true).primary(true).nullable(false) +} + +fn autoincrement_int() -> Type { + types::integer() + .increments(true) + .unique(true) + .primary(false) +} + pub fn migration() -> String { let mut m = Migration::new(); info!("Applying migration: {}", file!()); //Table for basic room information: room ID, room name m.create_table("room_info", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); + t.add_column("room_id", primary_uuid()); t.add_column("room_name", types::text()); }); //Table of users in rooms. m.create_table("room_users", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); + t.add_column("room_id", autoincrement_int()); t.add_column("username", types::text()); }); - //Table of room ID, event ID, event timestamp - m.create_table("room_events", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); - t.add_column("event_id", types::text()); - t.add_column("event_timestamp", types::integer()); - }); m.make::() } diff --git a/src/migrator/migrations/V4__room_events.rs b/src/migrator/migrations/V4__room_events.rs new file mode 100644 index 0000000..d9d487b --- /dev/null +++ b/src/migrator/migrations/V4__room_events.rs @@ -0,0 +1,38 @@ +use barrel::backend::Sqlite; +use barrel::{types, types::Type, Migration}; +use log::info; + +fn primary_uuid() -> Type { + types::text().unique(true).nullable(false) +} + +fn autoincrement_int() -> Type { + types::integer() + .increments(true) + .unique(true) + .primary(false) +} + +pub fn migration() -> String { + let mut m = Migration::new(); + info!("Applying migration: {}", file!()); + + //Table of room ID, event ID, event timestamp + m.create_table("room_events", move |t| { + t.add_column("room_id", types::text().nullable(false)); + t.add_column("event_id", types::text().nullable(false)); + t.add_column("event_timestamp", types::integer()); + }); + + let mut res = m.make::(); + + //This is a hack that gives us a composite primary key. + if res.ends_with(");") { + res.pop(); + res.pop(); + } + + let x = format!("{}, PRIMARY KEY (room_id, event_id));", res); + println!("{}", x); + x +} -- 2.40.1 From 66fb6e7cf8e2b3f7688eb50f8f8b9aafe0998971 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sun, 16 May 2021 22:22:06 +0000 Subject: [PATCH 05/17] Fix various issues with room events and related logic. - Processing events multiple times when re-joining rooms. - Always thinking we've not processed an event/constraint violations (arguments were reversed in record_event). - Not handling errors when fetchin users in a room, and instead just suppressing them. Now, we handle errors! - Also update dependencies (attempt to fix ID too big bug, but no fix). --- Cargo.lock | 38 +++++++++++++++++++------------------- src/bot.rs | 13 ------------- src/bot/event_handlers.rs | 21 ++++++++++++++------- src/db/sqlite/rooms.rs | 9 +++------ 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c6318c..2c5c2a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -342,9 +342,9 @@ checksum = "ea221b5284a47e40033bf9b66f35f984ec0ea2931eb03505246cd27a963f981b" [[package]] name = "cpufeatures" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "281f563b2c3a0e535ab12d81d3c5859045795256ad269afa7c19542585b68f93" +checksum = "ed00c67cb5d0a7d64a44f6ad2668db7e7530311dd53ea79bcd4fb022c64911c8" dependencies = [ "libc", ] @@ -1123,7 +1123,7 @@ checksum = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08" [[package]] name = "matrix-sdk" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "backoff", "bytes", @@ -1147,7 +1147,7 @@ dependencies = [ [[package]] name = "matrix-sdk-base" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "chacha20poly1305", "dashmap", @@ -1170,7 +1170,7 @@ dependencies = [ [[package]] name = "matrix-sdk-common" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "async-trait", "futures", @@ -1186,7 +1186,7 @@ dependencies = [ [[package]] name = "matrix-sdk-crypto" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "aes-ctr", "aes-gcm", @@ -1845,7 +1845,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.0.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "assign", "js_int", @@ -1861,7 +1861,7 @@ dependencies = [ [[package]] name = "ruma-api" version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "bytes", "http", @@ -1877,7 +1877,7 @@ dependencies = [ [[package]] name = "ruma-api-macros" version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1888,7 +1888,7 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.10.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "assign", "bytes", @@ -1908,7 +1908,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.5.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "indexmap", "js_int", @@ -1923,7 +1923,7 @@ dependencies = [ [[package]] name = "ruma-events" version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "indoc", "js_int", @@ -1938,7 +1938,7 @@ dependencies = [ [[package]] name = "ruma-events-macros" version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1949,7 +1949,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.1.0-alpha.2" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "js_int", "ruma-api", @@ -1964,7 +1964,7 @@ dependencies = [ [[package]] name = "ruma-identifiers" version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "paste", "ruma-identifiers-macros", @@ -1977,7 +1977,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-macros" version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "quote", "ruma-identifiers-validation", @@ -1987,12 +1987,12 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.3.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" [[package]] name = "ruma-serde" version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "bytes", "form_urlencoded", @@ -2006,7 +2006,7 @@ dependencies = [ [[package]] name = "ruma-serde-macros" version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", diff --git a/src/bot.rs b/src/bot.rs index 926a76a..a208882 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -161,22 +161,9 @@ impl DiceBot { let client = self.client.clone(); self.login(&client).await?; - // Initial sync without event handler prevents responding to - // messages received while bot was offline. TODO: selectively - // respond to old messages? e.g. comands missed while offline. - //info!("Performing intial sync (no commands will be responded to)"); - // self.client.sync_once(SyncSettings::default()).await?; - client.set_event_handler(Box::new(self)).await; info!("Listening for commands"); - // let token = client - // .sync_token() - // .await - // .ok_or(BotError::SyncTokenRequired)?; - - // let settings = SyncSettings::default().token(token); - // TODO replace with sync_with_callback for cleaner shutdown // process. client.sync(SyncSettings::default()).await; diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index 24ac3c1..7ffa293 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -20,9 +20,9 @@ use matrix_sdk::{ room::Room, EventHandler, }; -use std::clone::Clone; use std::ops::Sub; use std::time::{Duration, SystemTime}; +use std::{clone::Clone, time::UNIX_EPOCH}; /// Check if a message is recent enough to actually process. If the /// message is within "oldest_message_age" seconds, this function @@ -32,7 +32,11 @@ fn check_message_age( event: &SyncMessageEvent, oldest_message_age: u64, ) -> bool { - let sending_time = event.origin_server_ts; + let sending_time = event + .origin_server_ts + .to_system_time() + .unwrap_or(UNIX_EPOCH); + let oldest_timestamp = SystemTime::now().sub(Duration::from_secs(oldest_message_age)); if sending_time > oldest_timestamp { @@ -127,17 +131,20 @@ impl EventHandler for DiceBot { false }; + // user_joing is true if a user is joining this room, and + // false if they have left for some reason. This user may be + // us, or another user in the room. use MembershipChange::*; - let adding_user = match event.membership_change() { + let user_joining = match event.membership_change() { Joined => true, Banned | Left | Kicked | KickedAndBanned => false, _ => return, }; - let result = if event_affects_us && !adding_user { + let result = if event_affects_us && !user_joining { info!("Clearing all information for room ID {}", room_id); self.db.clear_info(room_id_str).await.map_err(|e| e.into()) - } else if event_affects_us && adding_user { + } else if event_affects_us && user_joining { info!("Joined room {}; recording room information", room_id); record_room_information( &self.client, @@ -147,13 +154,13 @@ impl EventHandler for DiceBot { &event.state_key, ) .await - } else if !event_affects_us && adding_user { + } else if !event_affects_us && user_joining { info!("Adding user {} to room ID {}", username, room_id); self.db .add_user_to_room(username, room_id_str) .await .map_err(|e| e.into()) - } else if !event_affects_us && !adding_user { + } else if !event_affects_us && !user_joining { info!("Removing user {} from room ID {}", username, room_id); self.db .remove_user_from_room(username, room_id_str) diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 9078a79..298c911 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -6,7 +6,7 @@ use sqlx::SqlitePool; use std::collections::{HashMap, HashSet}; use std::time::{SystemTime, UNIX_EPOCH}; -async fn record_event(conn: &SqlitePool, event_id: &str, room_id: &str) -> Result<(), DataError> { +async fn record_event(conn: &SqlitePool, room_id: &str, event_id: &str) -> Result<(), DataError> { use std::convert::TryFrom; let now: i64 = i64::try_from( SystemTime::now() @@ -122,6 +122,8 @@ impl Rooms for Database { } async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { + // We do not clear event history here, because if we rejoin a + // room, we would re-process events we've already seen. sqlx::query("DELETE FROM room_info where room_id = ?") .bind(room_id) .execute(&self.conn) @@ -132,11 +134,6 @@ impl Rooms for Database { .execute(&self.conn) .await?; - sqlx::query("DELETE FROM room_events where room_id = ?") - .bind(room_id) - .execute(&self.conn) - .await?; - Ok(()) } } -- 2.40.1 From a665293268b9dc97739681eeaa2f69649841a5e4 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Mon, 17 May 2021 23:12:27 +0000 Subject: [PATCH 06/17] Fix recording of room users, better logging. - Fix constraint violations when recording users in rooms (migration fix). - Switched to tracing_subscriber to get log events from matrix SDK. - Remove "Applying migration" messages, and rely on refinery to log instead. - Log when an outgoing error is encountered. --- Cargo.lock | 201 ++++++++++++------ Cargo.toml | 2 +- src/bin/dicebot.rs | 15 +- src/bot.rs | 13 +- src/logic.rs | 2 - src/matrix.rs | 2 +- src/migrator/migrations/V1__variables.rs | 2 - .../{V2__rooms.rs => V2__room_info.rs} | 15 -- src/migrator/migrations/V3__dbstate.rs | 2 - src/migrator/migrations/V4__room_events.rs | 18 +- src/migrator/migrations/V5__room_users.rs | 22 ++ 11 files changed, 179 insertions(+), 115 deletions(-) rename src/migrator/migrations/{V2__rooms.rs => V2__room_info.rs} (55%) create mode 100644 src/migrator/migrations/V5__room_users.rs diff --git a/Cargo.lock b/Cargo.lock index 2c5c2a1..95caf01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -92,6 +92,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "ansi_term" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +dependencies = [ + "winapi", +] + [[package]] name = "arrayvec" version = "0.5.2" @@ -133,17 +142,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "atty" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" -dependencies = [ - "hermit-abi", - "libc", - "winapi", -] - [[package]] name = "autocfg" version = "1.0.1" @@ -505,19 +503,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "env_logger" -version = "0.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17392a012ea30ef05a610aa97dfb49496e71c9f676b27879922ea5bdf60d9d3f" -dependencies = [ - "atty", - "humantime", - "log", - "regex", - "termcolor", -] - [[package]] name = "fallible-iterator" version = "0.2.0" @@ -896,12 +881,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "05842d0d43232b23ccb7060ecb0f0626922c21f30012e97b767b30afd4a5d4b9" -[[package]] -name = "humantime" -version = "2.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a3a5bfb195931eeb336b2a7b4d761daec841b97f947d34394601737a7bba5e4" - [[package]] name = "hyper" version = "0.14.7" @@ -1114,6 +1093,15 @@ dependencies = [ "xml5ever", ] +[[package]] +name = "matchers" +version = "0.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f099785f7595cc4b4553a174ce30dd7589ef93391ff414dbb67f62392b9e0ce1" +dependencies = [ + "regex-automata", +] + [[package]] name = "matches" version = "0.1.8" @@ -1123,7 +1111,7 @@ checksum = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08" [[package]] name = "matrix-sdk" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#cd77441d1bf8318ecf4cc9e1886696558ea6ed7b" dependencies = [ "backoff", "bytes", @@ -1147,7 +1135,7 @@ dependencies = [ [[package]] name = "matrix-sdk-base" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#cd77441d1bf8318ecf4cc9e1886696558ea6ed7b" dependencies = [ "chacha20poly1305", "dashmap", @@ -1170,7 +1158,7 @@ dependencies = [ [[package]] name = "matrix-sdk-common" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#cd77441d1bf8318ecf4cc9e1886696558ea6ed7b" dependencies = [ "async-trait", "futures", @@ -1186,7 +1174,7 @@ dependencies = [ [[package]] name = "matrix-sdk-crypto" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#cd77441d1bf8318ecf4cc9e1886696558ea6ed7b" dependencies = [ "aes-ctr", "aes-gcm", @@ -1793,6 +1781,16 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "regex-automata" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1ded71d66a4a97f5e961fd0cb25a5f366a42a41570d16a763a69c092c26ae4" +dependencies = [ + "byteorder", + "regex-syntax", +] + [[package]] name = "regex-syntax" version = "0.6.25" @@ -1844,8 +1842,9 @@ dependencies = [ [[package]] name = "ruma" -version = "0.0.3" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b4014a3def1ed10e7fa0fe28fec48418f2deb6135e7f5fe15901ba1302b581c" dependencies = [ "assign", "js_int", @@ -1860,8 +1859,9 @@ dependencies = [ [[package]] name = "ruma-api" -version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51df85b3e2c4097abc60919864502083def5c3b12982b0c46f6431e5b1e1476d" dependencies = [ "bytes", "http", @@ -1876,8 +1876,9 @@ dependencies = [ [[package]] name = "ruma-api-macros" -version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe79932728de6a753163f4f30acfd70ebe4355c35fc638edb3f47c7cf47ab128" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1887,8 +1888,9 @@ dependencies = [ [[package]] name = "ruma-client-api" -version = "0.10.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fda53002660ebdf6eca81102bf4362d0f9234a4d3f7a3e5f9878ed7c1f133843" dependencies = [ "assign", "bytes", @@ -1907,8 +1909,9 @@ dependencies = [ [[package]] name = "ruma-common" -version = "0.5.0" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c87e1a95f3625cae17772e98b3479059e7aa18593fb623bcff498d32d399d956" dependencies = [ "indexmap", "js_int", @@ -1922,8 +1925,9 @@ dependencies = [ [[package]] name = "ruma-events" -version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15c222b0f008461b00b039e76319c3184633346605b8201b24f54a6be99cbd98" dependencies = [ "indoc", "js_int", @@ -1937,8 +1941,9 @@ dependencies = [ [[package]] name = "ruma-events-macros" -version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "270df7148218b4d45d11bc1621c4ce5da8571abaf2db3f407a4b0f983a48625c" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1948,8 +1953,9 @@ dependencies = [ [[package]] name = "ruma-federation-api" -version = "0.1.0-alpha.2" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40fe42fd5951c688b5c7b0c32ed74d973d21c33ba31f9d64d40bf4f98c01eb48" dependencies = [ "js_int", "ruma-api", @@ -1963,8 +1969,9 @@ dependencies = [ [[package]] name = "ruma-identifiers" -version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.19.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b63396866eab1d0dafe5bd3c1af2e16f1f71a725fcc89a3e3f42572bc20c65a" dependencies = [ "paste", "ruma-identifiers-macros", @@ -1976,8 +1983,9 @@ dependencies = [ [[package]] name = "ruma-identifiers-macros" -version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.19.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a77e9bf8fd334e944d40f041155d4950da75a9b64cd05046ffc8ea670ad4ad12" dependencies = [ "quote", "ruma-identifiers-validation", @@ -1987,12 +1995,14 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.3.0" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecb664a32ba1cccf0d5cec34bef6bfccc042b54b8b5f9610729a128fcdf569a5" [[package]] name = "ruma-serde" -version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c888d69db1d5f134cbce7101f5610f1802966f359646ff48dbaf0f433d7bbd9f" dependencies = [ "bytes", "form_urlencoded", @@ -2005,8 +2015,9 @@ dependencies = [ [[package]] name = "ruma-serde-macros" -version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be418f3ee1593ebf1522d9ace1b1de0455bbcdd69c5d584c00d41e7717f8d0af" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -2176,6 +2187,15 @@ dependencies = [ "opaque-debug", ] +[[package]] +name = "sharded-slab" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79c719719ee05df97490f80a45acfc99e5a30ce98a1e4fb67aee422745ae14e3" +dependencies = [ + "lazy_static", +] + [[package]] name = "signal-hook-registry" version = "1.3.0" @@ -2497,7 +2517,6 @@ dependencies = [ "byteorder", "combine", "dirs", - "env_logger", "futures", "html2text", "indoc", @@ -2516,19 +2535,11 @@ dependencies = [ "thiserror", "tokio", "toml", + "tracing-subscriber", "url", "zerocopy", ] -[[package]] -name = "termcolor" -version = "1.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" -dependencies = [ - "winapi-util", -] - [[package]] name = "thiserror" version = "1.0.24" @@ -2549,6 +2560,15 @@ dependencies = [ "syn", ] +[[package]] +name = "thread_local" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8018d24e04c95ac8790716a5987d0fec4f8b27249ffa0f7d33f1369bdfb88cbd" +dependencies = [ + "once_cell", +] + [[package]] name = "time" version = "0.1.43" @@ -2735,6 +2755,49 @@ dependencies = [ "tracing", ] +[[package]] +name = "tracing-log" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6923477a48e41c1951f1999ef8bb5a3023eb723ceadafe78ffb65dc366761e3" +dependencies = [ + "lazy_static", + "log", + "tracing-core", +] + +[[package]] +name = "tracing-serde" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb65ea441fbb84f9f6748fd496cf7f63ec9af5bca94dd86456978d055e8eb28b" +dependencies = [ + "serde", + "tracing-core", +] + +[[package]] +name = "tracing-subscriber" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa5553bf0883ba7c9cbe493b085c29926bd41b66afc31ff72cf17ff4fb60dcd5" +dependencies = [ + "ansi_term", + "chrono", + "lazy_static", + "matchers", + "regex", + "serde", + "serde_json", + "sharded-slab", + "smallvec", + "thread_local", + "tracing", + "tracing-core", + "tracing-log", + "tracing-serde", +] + [[package]] name = "try-lock" version = "0.2.3" diff --git a/Cargo.toml b/Cargo.toml index 93eb562..88de99b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ path = "src/migrate_cli.rs" [dependencies] log = "0.4" -env_logger = "0.8" +tracing-subscriber = "0.2" toml = "0.5" nom = "5" rand = "0.8" diff --git a/src/bin/dicebot.rs b/src/bin/dicebot.rs index d4ee65f..8be23db 100644 --- a/src/bin/dicebot.rs +++ b/src/bin/dicebot.rs @@ -1,7 +1,7 @@ //Needed for nested Result handling from tokio. Probably can go away after 1.47.0. #![type_length_limit = "7605144"] -use env_logger::Env; use log::error; +use std::env; use std::sync::{Arc, RwLock}; use tenebrous_dicebot::bot::DiceBot; use tenebrous_dicebot::config::*; @@ -9,13 +9,18 @@ use tenebrous_dicebot::db::sqlite::Database; use tenebrous_dicebot::error::BotError; use tenebrous_dicebot::migrator; use tenebrous_dicebot::state::DiceBotState; +use tracing_subscriber::filter::EnvFilter; #[tokio::main] async fn main() { - env_logger::Builder::from_env( - Env::default().default_filter_or("tenebrous_dicebot=info,dicebot=info"), - ) - .init(); + let filter = if env::var("RUST_LOG").is_ok() { + EnvFilter::from_default_env() + } else { + EnvFilter::new("tenebrous_dicebot=info,dicebot=info,refinery=info") + }; + + tracing_subscriber::fmt().with_env_filter(filter).init(); + match run().await { Ok(_) => (), Err(e) => error!("Error: {}", e), diff --git a/src/bot.rs b/src/bot.rs index a208882..58b9b20 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -8,7 +8,7 @@ use crate::matrix; use crate::state::DiceBotState; use dirs; use futures::stream::{self, StreamExt}; -use log::info; +use log::{error, info}; use matrix_sdk::{self, identifiers::EventId, room::Joined, Client, ClientConfig, SyncSettings}; use std::clone::Clone; use std::path::PathBuf; @@ -62,6 +62,13 @@ async fn handle_single_result( room: &Joined, event_id: EventId, ) { + if cmd_result.is_err() { + error!( + "Command execution error: {}", + cmd_result.as_ref().err().unwrap() + ); + } + let html = cmd_result.message_html(respond_to); matrix::send_message(client, room.room_id(), &html, Some(event_id)).await; } @@ -87,6 +94,10 @@ async fn handle_multiple_results( }) .collect(); + for result in errors.iter() { + error!("Command execution error: '{}' - {}", result.0, result.1); + } + let message = if errors.len() == 0 { format!("{}: Executed {} commands", respond_to, results.len()) } else { diff --git a/src/logic.rs b/src/logic.rs index 5be59a6..7b67ce0 100644 --- a/src/logic.rs +++ b/src/logic.rs @@ -33,8 +33,6 @@ pub async fn record_room_information( .into_iter() .filter(|username| username != our_username); - println!("Users to add to room: {:?}", filtered_usernames); - // Async collect into vec of results, then use into_iter of result // to go to from Result> to just Result<()>. Easier than // attempting to async-collect our way to a single Result<()>. diff --git a/src/matrix.rs b/src/matrix.rs index 7ff73c4..6c8e7d8 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -53,7 +53,7 @@ pub async fn send_message( )); content.relates_to = reply_to.map(|event_id| Relation::Reply { - in_reply_to: InReplyTo { event_id }, + in_reply_to: InReplyTo::new(event_id), }); let content = AnyMessageEventContent::RoomMessage(content); diff --git a/src/migrator/migrations/V1__variables.rs b/src/migrator/migrations/V1__variables.rs index ec8f063..a494bc7 100644 --- a/src/migrator/migrations/V1__variables.rs +++ b/src/migrator/migrations/V1__variables.rs @@ -1,10 +1,8 @@ use barrel::backend::Sqlite; use barrel::{types, Migration}; -use log::info; pub fn migration() -> String { let mut m = Migration::new(); - info!("Applying migration: {}", file!()); m.create_table("user_variables", |t| { t.add_column("room_id", types::text()); diff --git a/src/migrator/migrations/V2__rooms.rs b/src/migrator/migrations/V2__room_info.rs similarity index 55% rename from src/migrator/migrations/V2__rooms.rs rename to src/migrator/migrations/V2__room_info.rs index 5568a01..fb6ebb2 100644 --- a/src/migrator/migrations/V2__rooms.rs +++ b/src/migrator/migrations/V2__room_info.rs @@ -1,21 +1,12 @@ use barrel::backend::Sqlite; use barrel::{types, types::Type, Migration}; -use log::info; fn primary_uuid() -> Type { types::text().unique(true).primary(true).nullable(false) } -fn autoincrement_int() -> Type { - types::integer() - .increments(true) - .unique(true) - .primary(false) -} - pub fn migration() -> String { let mut m = Migration::new(); - info!("Applying migration: {}", file!()); //Table for basic room information: room ID, room name m.create_table("room_info", move |t| { @@ -23,11 +14,5 @@ pub fn migration() -> String { t.add_column("room_name", types::text()); }); - //Table of users in rooms. - m.create_table("room_users", move |t| { - t.add_column("room_id", autoincrement_int()); - t.add_column("username", types::text()); - }); - m.make::() } diff --git a/src/migrator/migrations/V3__dbstate.rs b/src/migrator/migrations/V3__dbstate.rs index 0ae82c2..3bb25c0 100644 --- a/src/migrator/migrations/V3__dbstate.rs +++ b/src/migrator/migrations/V3__dbstate.rs @@ -1,10 +1,8 @@ use barrel::backend::Sqlite; use barrel::{types, Migration}; -use log::info; pub fn migration() -> String { let mut m = Migration::new(); - info!("Applying migration: {}", file!()); //Basic state table with only device ID for now. Uses only one row. m.create_table("bot_state", move |t| { diff --git a/src/migrator/migrations/V4__room_events.rs b/src/migrator/migrations/V4__room_events.rs index d9d487b..8657a98 100644 --- a/src/migrator/migrations/V4__room_events.rs +++ b/src/migrator/migrations/V4__room_events.rs @@ -1,21 +1,7 @@ use barrel::backend::Sqlite; use barrel::{types, types::Type, Migration}; -use log::info; - -fn primary_uuid() -> Type { - types::text().unique(true).nullable(false) -} - -fn autoincrement_int() -> Type { - types::integer() - .increments(true) - .unique(true) - .primary(false) -} - pub fn migration() -> String { let mut m = Migration::new(); - info!("Applying migration: {}", file!()); //Table of room ID, event ID, event timestamp m.create_table("room_events", move |t| { @@ -32,7 +18,5 @@ pub fn migration() -> String { res.pop(); } - let x = format!("{}, PRIMARY KEY (room_id, event_id));", res); - println!("{}", x); - x + format!("{}, PRIMARY KEY (room_id, event_id));", res) } diff --git a/src/migrator/migrations/V5__room_users.rs b/src/migrator/migrations/V5__room_users.rs new file mode 100644 index 0000000..eedec4d --- /dev/null +++ b/src/migrator/migrations/V5__room_users.rs @@ -0,0 +1,22 @@ +use barrel::backend::Sqlite; +use barrel::{types, types::Type, Migration}; + +pub fn migration() -> String { + let mut m = Migration::new(); + + //Table of users in rooms. + m.create_table("room_users", move |t| { + t.add_column("room_id", types::text()); + t.add_column("username", types::text()); + }); + + let mut res = m.make::(); + + //This is a hack that gives us a composite primary key. + if res.ends_with(");") { + res.pop(); + res.pop(); + } + + format!("{}, PRIMARY KEY (room_id, username));", res) +} -- 2.40.1 From 9f97a6cb431f5c411d89b107deb4ec9f666aa439 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 14:27:15 +0000 Subject: [PATCH 07/17] Implement variable count; fix listing all variables returning values for all users. --- sqlx-data.json | 66 ++++++++++++++++++++++++-------------- src/db/sqlite/variables.rs | 14 ++++++-- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/sqlx-data.json b/sqlx-data.json index 835daf1..125b734 100644 --- a/sqlx-data.json +++ b/sqlx-data.json @@ -42,6 +42,30 @@ ] } }, + "59313c67900a1a9399389720b522e572f181ae503559cd2b49d6305acb9e2207": { + "query": "SELECT key, value as \"value: i32\" FROM user_variables\n WHERE room_id = ? AND user_id = ?", + "describe": { + "columns": [ + { + "name": "key", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "value: i32", + "ordinal": 1, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + false, + false + ] + } + }, "636b1b868eaf04cd234fbf17747d94a66e81f7bc1b060ba14151dbfaf40eeefc": { "query": "SELECT value as \"value: i32\" FROM user_variables\n WHERE user_id = ? AND room_id = ? AND key = ?", "describe": { @@ -60,6 +84,24 @@ ] } }, + "711d222911c1258365a6a0de1fe00eeec4686fd3589e976e225ad599e7cfc75d": { + "query": "SELECT count(*) as \"count: i32\" FROM user_variables\n WHERE room_id = ? and user_id = ?", + "describe": { + "columns": [ + { + "name": "count: i32", + "ordinal": 0, + "type_info": "Int" + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + false + ] + } + }, "ad52bc29fc1eef2bbff8b5f7316047f81ea31b1f910d0e0226125fea89530aa2": { "query": "SELECT room_id FROM room_users\n WHERE username = ?", "describe": { @@ -113,29 +155,5 @@ false ] } - }, - "d6558668b7395b95ded8da71c80963ddde957abdcc3c68b03431f8e904e0d21f": { - "query": "SELECT key, value as \"value: i32\" FROM user_variables\n WHERE room_id = ?", - "describe": { - "columns": [ - { - "name": "key", - "ordinal": 0, - "type_info": "Text" - }, - { - "name": "value: i32", - "ordinal": 1, - "type_info": "Int64" - } - ], - "parameters": { - "Right": 1 - }, - "nullable": [ - false, - false - ] - } } } \ No newline at end of file diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs index 0aa023d..c35ee14 100644 --- a/src/db/sqlite/variables.rs +++ b/src/db/sqlite/variables.rs @@ -17,8 +17,9 @@ impl Variables for Database { ) -> Result, DataError> { let rows = sqlx::query!( r#"SELECT key, value as "value: i32" FROM user_variables - WHERE room_id = ?"#, + WHERE room_id = ? AND user_id = ?"#, room_id, + user ) .fetch_all(&self.conn) .await?; @@ -27,7 +28,16 @@ impl Variables for Database { } async fn get_variable_count(&self, user: &str, room_id: &str) -> Result { - Ok(1) + let row = sqlx::query!( + r#"SELECT count(*) as "count: i32" FROM user_variables + WHERE room_id = ? and user_id = ?"#, + room_id, + user + ) + .fetch_optional(&self.conn) + .await?; + + Ok(row.map(|r| r.count).unwrap_or(0)) } async fn get_user_variable( -- 2.40.1 From e539dcac1f0d886612da7237a1f26b268ff3e6d7 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 14:50:49 +0000 Subject: [PATCH 08/17] Move migrations to sqlite directory. Remove in-memory temp db until refinery supports sqlx. --- Cargo.toml | 4 +--- src/bin/dicebot-cmd.rs | 10 +++++++++- src/bin/dicebot.rs | 3 --- src/cofd/dice.rs | 10 +++++++--- src/commands.rs | 7 ++++++- src/cthulhu/dice.rs | 6 +++--- src/db/sqlite/errors.rs | 16 +--------------- .../sqlite}/migrator/migrations/V1__variables.rs | 0 .../sqlite}/migrator/migrations/V2__room_info.rs | 0 .../sqlite}/migrator/migrations/V3__dbstate.rs | 0 .../migrator/migrations/V4__room_events.rs | 0 .../migrator/migrations/V5__room_users.rs | 0 src/db/sqlite/migrator/migrations/mod.rs | 2 ++ src/{migrator.rs => db/sqlite/migrator/mod.rs} | 0 src/db/sqlite/mod.rs | 8 ++++---- src/error.rs | 4 ++-- src/lib.rs | 1 - src/migrate_cli.rs | 3 +-- src/migrator/migrations/mod.rs | 2 -- 19 files changed, 36 insertions(+), 40 deletions(-) rename src/{ => db/sqlite}/migrator/migrations/V1__variables.rs (100%) rename src/{ => db/sqlite}/migrator/migrations/V2__room_info.rs (100%) rename src/{ => db/sqlite}/migrator/migrations/V3__dbstate.rs (100%) rename src/{ => db/sqlite}/migrator/migrations/V4__room_events.rs (100%) rename src/{ => db/sqlite}/migrator/migrations/V5__room_users.rs (100%) create mode 100644 src/db/sqlite/migrator/migrations/mod.rs rename src/{migrator.rs => db/sqlite/migrator/mod.rs} (100%) delete mode 100644 src/migrator/migrations/mod.rs diff --git a/Cargo.toml b/Cargo.toml index 88de99b..b166367 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ phf = { version = "0.8", features = ["macros"] } matrix-sdk = { git = "https://github.com/matrix-org/matrix-rust-sdk", branch = "master" } refinery = { version = "0.5", features = ["rusqlite"]} barrel = { version = "0.6", features = ["sqlite3"] } +tempfile = "3" [dependencies.sqlx] version = "0.5" @@ -50,6 +51,3 @@ features = ['derive'] [dependencies.tokio] version = "1" features = [ "full" ] - -[dev-dependencies] -tempfile = "3" \ No newline at end of file diff --git a/src/bin/dicebot-cmd.rs b/src/bin/dicebot-cmd.rs index b80fc12..dc38d71 100644 --- a/src/bin/dicebot-cmd.rs +++ b/src/bin/dicebot-cmd.rs @@ -15,9 +15,17 @@ async fn main() -> Result<(), BotError> { }; let homeserver = Url::parse("http://example.com")?; + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let db = Database::new( + db_path + .path() + .to_str() + .expect("Could not get path to temporary db"), + ) + .await?; let context = Context { - db: Database::new_temp().await?, + db: db, matrix_client: &matrix_sdk::Client::new(homeserver) .expect("Could not create matrix client"), room: RoomContext { diff --git a/src/bin/dicebot.rs b/src/bin/dicebot.rs index 8be23db..a5abbb1 100644 --- a/src/bin/dicebot.rs +++ b/src/bin/dicebot.rs @@ -7,7 +7,6 @@ use tenebrous_dicebot::bot::DiceBot; use tenebrous_dicebot::config::*; use tenebrous_dicebot::db::sqlite::Database; use tenebrous_dicebot::error::BotError; -use tenebrous_dicebot::migrator; use tenebrous_dicebot::state::DiceBotState; use tracing_subscriber::filter::EnvFilter; @@ -38,8 +37,6 @@ async fn run() -> Result<(), BotError> { let db = Database::new(&sqlite_path).await?; let state = Arc::new(RwLock::new(DiceBotState::new(&cfg))); - migrator::migrate(&sqlite_path).await?; - match DiceBot::new(&cfg, &state, &db) { Ok(bot) => bot.run().await?, Err(e) => println!("Error connecting: {:?}", e), diff --git a/src/cofd/dice.rs b/src/cofd/dice.rs index f269b1c..aae34b8 100644 --- a/src/cofd/dice.rs +++ b/src/cofd/dice.rs @@ -475,8 +475,12 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn rejects_large_expression_test() { + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); let homeserver = Url::parse("http://example.com").unwrap(); - let db = Database::new_temp().await.unwrap(); + let db = Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap(); + let ctx = Context { db: db, matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(), @@ -508,7 +512,7 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn converts_to_chance_die_test() { let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); - crate::migrator::migrate(db_path.path().to_str().unwrap()) + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) .await .unwrap(); @@ -545,7 +549,7 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn can_resolve_variables_test() { let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); - crate::migrator::migrate(db_path.path().to_str().unwrap()) + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) .await .unwrap(); diff --git a/src/commands.rs b/src/commands.rs index f0aa160..4dd24cd 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -138,8 +138,13 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn unrecognized_command() { - let db = crate::db::sqlite::Database::new_temp().await.unwrap(); + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let db = crate::db::sqlite::Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap(); + let homeserver = Url::parse("http://example.com").unwrap(); + let ctx = Context { db: db, matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(), diff --git a/src/cthulhu/dice.rs b/src/cthulhu/dice.rs index 95a0469..fffaeaa 100644 --- a/src/cthulhu/dice.rs +++ b/src/cthulhu/dice.rs @@ -493,7 +493,7 @@ mod tests { }; let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); - crate::migrator::migrate(db_path.path().to_str().unwrap()) + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) .await .unwrap(); @@ -529,7 +529,7 @@ mod tests { }; let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); - crate::migrator::migrate(db_path.path().to_str().unwrap()) + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) .await .unwrap(); @@ -565,7 +565,7 @@ mod tests { }; let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); - crate::migrator::migrate(db_path.path().to_str().unwrap()) + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) .await .unwrap(); diff --git a/src/db/sqlite/errors.rs b/src/db/sqlite/errors.rs index dd06b77..2132120 100644 --- a/src/db/sqlite/errors.rs +++ b/src/db/sqlite/errors.rs @@ -3,20 +3,6 @@ use std::num::TryFromIntError; use sled::transaction::{TransactionError, UnabortableTransactionError}; use thiserror::Error; -#[derive(Error, Debug)] -pub enum MigrationError { - #[error("cannot downgrade to an older database version")] - CannotDowngrade, - - #[error("migration for version {0} not defined")] - MigrationNotFound(u32), - - #[error("migration failed: {0}")] - MigrationFailed(String), -} - -//TODO better combining of key and value in certain errors (namely -//I32SchemaViolation). #[derive(Error, Debug)] pub enum DataError { #[error("value does not exist for key: {0}")] @@ -47,7 +33,7 @@ pub enum DataError { UnabortableTransactionError(#[from] UnabortableTransactionError), #[error("data migration error: {0}")] - MigrationError(#[from] MigrationError), + MigrationError(#[from] super::migrator::MigrationError), #[error("deserialization error: {0}")] DeserializationError(#[from] bincode::Error), diff --git a/src/migrator/migrations/V1__variables.rs b/src/db/sqlite/migrator/migrations/V1__variables.rs similarity index 100% rename from src/migrator/migrations/V1__variables.rs rename to src/db/sqlite/migrator/migrations/V1__variables.rs diff --git a/src/migrator/migrations/V2__room_info.rs b/src/db/sqlite/migrator/migrations/V2__room_info.rs similarity index 100% rename from src/migrator/migrations/V2__room_info.rs rename to src/db/sqlite/migrator/migrations/V2__room_info.rs diff --git a/src/migrator/migrations/V3__dbstate.rs b/src/db/sqlite/migrator/migrations/V3__dbstate.rs similarity index 100% rename from src/migrator/migrations/V3__dbstate.rs rename to src/db/sqlite/migrator/migrations/V3__dbstate.rs diff --git a/src/migrator/migrations/V4__room_events.rs b/src/db/sqlite/migrator/migrations/V4__room_events.rs similarity index 100% rename from src/migrator/migrations/V4__room_events.rs rename to src/db/sqlite/migrator/migrations/V4__room_events.rs diff --git a/src/migrator/migrations/V5__room_users.rs b/src/db/sqlite/migrator/migrations/V5__room_users.rs similarity index 100% rename from src/migrator/migrations/V5__room_users.rs rename to src/db/sqlite/migrator/migrations/V5__room_users.rs diff --git a/src/db/sqlite/migrator/migrations/mod.rs b/src/db/sqlite/migrator/migrations/mod.rs new file mode 100644 index 0000000..a7ece4d --- /dev/null +++ b/src/db/sqlite/migrator/migrations/mod.rs @@ -0,0 +1,2 @@ +use refinery::include_migration_mods; +include_migration_mods!("src/db/sqlite/migrator/migrations"); diff --git a/src/migrator.rs b/src/db/sqlite/migrator/mod.rs similarity index 100% rename from src/migrator.rs rename to src/db/sqlite/migrator/mod.rs diff --git a/src/db/sqlite/mod.rs b/src/db/sqlite/mod.rs index 426e2eb..480f502 100644 --- a/src/db/sqlite/mod.rs +++ b/src/db/sqlite/mod.rs @@ -9,6 +9,7 @@ use std::str::FromStr; use crate::models::RoomInfo; pub mod errors; +pub mod migrator; pub mod rooms; pub mod state; pub mod variables; @@ -93,6 +94,9 @@ impl Database { drop(conn); + //Migrate database. + migrator::migrate(&path).await?; + //Return actual conncetion pool. let conn = SqlitePoolOptions::new() .max_connections(5) @@ -101,10 +105,6 @@ impl Database { Self::new_db(conn) } - - pub async fn new_temp() -> Result { - Self::new("sqlite::memory:").await - } } impl Clone for Database { diff --git a/src/error.rs b/src/error.rs index f091d98..3cc9376 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,6 @@ use crate::config::ConfigError; use crate::db::errors::DataError; -use crate::{commands::CommandError, migrator::migrations}; +use crate::{commands::CommandError, db::sqlite::migrator}; use thiserror::Error; #[derive(Error, Debug)] @@ -77,7 +77,7 @@ pub enum BotError { DatabaseError(#[from] sled::Error), #[error("database migration error: {0}")] - SqliteError(#[from] crate::migrator::MigrationError), + SqliteError(#[from] migrator::MigrationError), #[error("too many commands or message was too large")] MessageTooLarge, diff --git a/src/lib.rs b/src/lib.rs index 5c27f63..d67ade8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,6 @@ pub mod error; mod help; pub mod logic; pub mod matrix; -pub mod migrator; pub mod models; mod parser; pub mod state; diff --git a/src/migrate_cli.rs b/src/migrate_cli.rs index 11cbbbc..9478b13 100644 --- a/src/migrate_cli.rs +++ b/src/migrate_cli.rs @@ -1,6 +1,5 @@ use std::env; - -pub mod migrator; +use tenebrous_dicebot::db::sqlite::migrator; #[tokio::main] async fn main() -> Result<(), migrator::MigrationError> { diff --git a/src/migrator/migrations/mod.rs b/src/migrator/migrations/mod.rs deleted file mode 100644 index b237433..0000000 --- a/src/migrator/migrations/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -use refinery::include_migration_mods; -include_migration_mods!("src/migrator/migrations"); -- 2.40.1 From 257f3a066c2b401ac235c27b5f5fa1472dcd4784 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 15:17:30 +0000 Subject: [PATCH 09/17] Some user variable tests. --- src/db/sqlite/variables.rs | 65 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs index c35ee14..eda8962 100644 --- a/src/db/sqlite/variables.rs +++ b/src/db/sqlite/variables.rs @@ -55,6 +55,10 @@ impl Variables for Database { ) .fetch_one(&self.conn) .await?; + // .map_err(|e| match e { + // sqlx::Error::RowNotFound => Err(DataError::KeyDoesNotExist(variable_name.clone())), + // _ => Err(e.into()), + // })?; Ok(row.value) } @@ -100,3 +104,64 @@ impl Variables for Database { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + async fn create_db() -> Database { + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap() + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn set_and_get_variable_test() { + use super::super::Variables; + let db = create_db().await; + + db.set_user_variable("myuser", "myroom", "myvariable", 1) + .await + .expect("Could not set variable"); + + let value = db + .get_user_variable("myuser", "myroom", "myvariable") + .await + .expect("Could not get variable"); + + assert_eq!(value, 1); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn get_missing_variable_test() { + use super::super::Variables; + let db = create_db().await; + + let value = db.get_user_variable("myuser", "myroom", "myvariable").await; + + println!("{:?}", value); + assert!(value.is_err()); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn get_other_user_variable_test() { + use super::super::Variables; + let db = create_db().await; + + db.set_user_variable("myuser1", "myroom", "myvariable", 1) + .await + .expect("Could not set variable"); + + let value = db + .get_user_variable("myuser2", "myroom", "myvariable") + .await; + + println!("{:?}", value); + assert!(value.is_err()); + } +} -- 2.40.1 From 5e899cd962bb192763ee7da1cc590acf924dd3a7 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 15:37:20 +0000 Subject: [PATCH 10/17] Return key not found error if value not found for user. --- src/db/sqlite/variables.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs index eda8962..622415b 100644 --- a/src/db/sqlite/variables.rs +++ b/src/db/sqlite/variables.rs @@ -53,14 +53,11 @@ impl Variables for Database { room_id, variable_name ) - .fetch_one(&self.conn) + .fetch_optional(&self.conn) .await?; - // .map_err(|e| match e { - // sqlx::Error::RowNotFound => Err(DataError::KeyDoesNotExist(variable_name.clone())), - // _ => Err(e.into()), - // })?; - Ok(row.value) + row.map(|r| r.value) + .ok_or_else(|| DataError::KeyDoesNotExist(variable_name.to_string())) } async fn set_user_variable( @@ -144,8 +141,11 @@ mod tests { let value = db.get_user_variable("myuser", "myroom", "myvariable").await; - println!("{:?}", value); assert!(value.is_err()); + assert!(matches!( + value.err().unwrap(), + DataError::KeyDoesNotExist(_) + )); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -161,7 +161,10 @@ mod tests { .get_user_variable("myuser2", "myroom", "myvariable") .await; - println!("{:?}", value); assert!(value.is_err()); + assert!(matches!( + value.err().unwrap(), + DataError::KeyDoesNotExist(_) + )); } } -- 2.40.1 From d1c04b8817b83999db0a3fc4ea86db51dd8989b7 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 21:39:48 +0000 Subject: [PATCH 11/17] Tests for all of the variables DB api. --- src/db/sqlite/variables.rs | 83 +++++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/src/db/sqlite/variables.rs b/src/db/sqlite/variables.rs index 622415b..f93f499 100644 --- a/src/db/sqlite/variables.rs +++ b/src/db/sqlite/variables.rs @@ -90,7 +90,7 @@ impl Variables for Database { ) -> Result<(), DataError> { sqlx::query( "DELETE FROM user_variables - WHERE user_id = ? AND room_id = ? AND variable_name = ?", + WHERE user_id = ? AND room_id = ? AND key = ?", ) .bind(user) .bind(room_id) @@ -104,6 +104,7 @@ impl Variables for Database { #[cfg(test)] mod tests { + use super::super::Variables; use super::*; async fn create_db() -> Database { @@ -167,4 +168,84 @@ mod tests { DataError::KeyDoesNotExist(_) )); } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn count_variables_test() { + let db = create_db().await; + + for variable_name in &["var1", "var2", "var3"] { + db.set_user_variable("myuser", "myroom", variable_name, 1) + .await + .expect("Could not set variable"); + } + + let count = db + .get_variable_count("myuser", "myroom") + .await + .expect("Could not get count."); + + assert_eq!(count, 3); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn count_variables_respects_user_id() { + let db = create_db().await; + + for variable_name in &["var1", "var2", "var3"] { + db.set_user_variable("different-user", "myroom", variable_name, 1) + .await + .expect("Could not set variable"); + } + + let count = db + .get_variable_count("myuser", "myroom") + .await + .expect("Could not get count."); + + assert_eq!(count, 0); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn count_variables_respects_room_id() { + let db = create_db().await; + + for variable_name in &["var1", "var2", "var3"] { + db.set_user_variable("myuser", "different-room", variable_name, 1) + .await + .expect("Could not set variable"); + } + + let count = db + .get_variable_count("myuser", "myroom") + .await + .expect("Could not get count."); + + assert_eq!(count, 0); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn delete_variable_test() { + let db = create_db().await; + + for variable_name in &["var1", "var2", "var3"] { + db.set_user_variable("myuser", "myroom", variable_name, 1) + .await + .expect("Could not set variable"); + } + + db.delete_user_variable("myuser", "myroom", "var1") + .await + .expect("Could not delete variable."); + + let count = db + .get_variable_count("myuser", "myroom") + .await + .expect("Could not get count"); + + assert_eq!(count, 2); + + let var1 = db.get_user_variable("myuser", "myroom", "var1").await; + assert!(var1.is_err()); + assert!(matches!(var1.err().unwrap(), DataError::KeyDoesNotExist(_))); + } } -- 2.40.1 From 1c4cd3d1390683ccdd9d272a49adb923b2964650 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Tue, 18 May 2021 22:15:03 +0000 Subject: [PATCH 12/17] Add tests for rooms db API --- src/db/sqlite/rooms.rs | 185 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 298c911..a6a565f 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -100,6 +100,9 @@ impl Rooms for Database { } async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { + // This is here because it is possible to process a bunch of + // user join/leave events at once, and we don't want to cause + // constraint violation errors. self.remove_user_from_room(username, room_id).await.ok(); sqlx::query("INSERT INTO room_users (room_id, username) VALUES (?, ?)") @@ -137,3 +140,185 @@ impl Rooms for Database { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::super::Rooms; + use super::*; + + async fn create_db() -> Database { + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap() + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn should_process_test() { + let db = create_db().await; + + let first_check = db + .should_process("myroom", "myeventid") + .await + .expect("should_process failed in first insert"); + + assert_eq!(first_check, true); + + let second_check = db + .should_process("myroom", "myeventid") + .await + .expect("should_process failed in first insert"); + + assert_eq!(second_check, false); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn insert_and_get_room_info_test() { + let db = create_db().await; + + let info = RoomInfo { + room_id: "myroomid".to_string(), + room_name: "myroomname".to_string(), + }; + + db.insert_room_info(&info) + .await + .expect("Could not insert room info."); + + let retrieved_info = db + .get_room_info("myroomid") + .await + .expect("Could not retrieve room info."); + + assert!(retrieved_info.is_some()); + assert_eq!(info, retrieved_info.unwrap()); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn get_room_info_constraint_test() { + let db = create_db().await; + + let info = RoomInfo { + room_id: "myroomid".to_string(), + room_name: "myroomname".to_string(), + }; + + db.insert_room_info(&info) + .await + .expect("Could not insert room info."); + + let second_attempt = db.insert_room_info(&info).await; + + assert!(second_attempt.is_err()); + assert!(matches!( + second_attempt.err().unwrap(), + DataError::SqlxError(sqlx::Error::Database(_)) + )); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn add_user_to_room_test() { + let db = create_db().await; + + db.add_user_to_room("myuser", "myroom") + .await + .expect("Could not add user to room."); + + let users_in_room = db + .get_users_in_room("myroom") + .await + .expect("Could not get users in room."); + + assert_eq!(users_in_room.len(), 1); + assert!(users_in_room.contains("myuser")); + + let rooms_for_user = db + .get_rooms_for_user("myuser") + .await + .expect("Could not get rooms for user"); + + assert_eq!(rooms_for_user.len(), 1); + assert!(rooms_for_user.contains("myroom")); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn add_user_to_room_does_not_have_constraint_violation() { + let db = create_db().await; + + db.add_user_to_room("myuser", "myroom") + .await + .expect("Could not add user to room."); + + let second_attempt = db.add_user_to_room("myuser", "myroom").await; + + assert!(second_attempt.is_ok()); + + let users_in_room = db + .get_users_in_room("myroom") + .await + .expect("Could not get users in room."); + + assert_eq!(users_in_room.len(), 1); + assert!(users_in_room.contains("myuser")); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn remove_user_from_room_test() { + let db = create_db().await; + + db.add_user_to_room("myuser", "myroom") + .await + .expect("Could not add user to room."); + + let remove_attempt = db.remove_user_from_room("myuser", "myroom").await; + + assert!(remove_attempt.is_ok()); + + let users_in_room = db + .get_users_in_room("myroom") + .await + .expect("Could not get users in room."); + + assert_eq!(users_in_room.len(), 0); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn clear_info_test() { + let db = create_db().await; + + let info = RoomInfo { + room_id: "myroomid".to_string(), + room_name: "myroomname".to_string(), + }; + + db.insert_room_info(&info) + .await + .expect("Could not insert room info."); + + db.add_user_to_room("myuser", "myroom") + .await + .expect("Could not add user to room."); + + db.clear_info("myroom") + .await + .expect("Could not clear room info"); + + let users_in_room = db + .get_users_in_room("myroom") + .await + .expect("Could not get users in room."); + + assert_eq!(users_in_room.len(), 0); + + let room_info = db + .get_room_info("myroom") + .await + .expect("Could not get room info."); + + assert!(room_info.is_none()); + } +} -- 2.40.1 From 43d8f9574fd817a9eeb60c318b7881c17d970d31 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Wed, 19 May 2021 21:06:28 +0000 Subject: [PATCH 13/17] Allow 'upserts' in insert_room_info. Add a few more room db tests. --- src/db/sqlite/rooms.rs | 85 +++++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index a6a565f..9e89850 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -51,6 +51,13 @@ impl Rooms for Database { } async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError> { + //Clear out old info first, because we want this to be an "upsert." + sqlx::query("DELETE FROM room_info where room_id = ?") + .bind(&info.room_id) + .execute(&self.conn) + .await + .ok(); + sqlx::query(r#"INSERT INTO room_info (room_id, room_name) VALUES (?, ?)"#) .bind(&info.room_id) .bind(&info.room_name) @@ -199,25 +206,37 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn get_room_info_constraint_test() { + async fn insert_room_info_updates_existing() { let db = create_db().await; - let info = RoomInfo { + let info1 = RoomInfo { room_id: "myroomid".to_string(), room_name: "myroomname".to_string(), }; - db.insert_room_info(&info) + db.insert_room_info(&info1) .await - .expect("Could not insert room info."); + .expect("Could not insert room info1."); - let second_attempt = db.insert_room_info(&info).await; + let info2 = RoomInfo { + room_id: "myroomid".to_string(), + room_name: "myroomname2".to_string(), + }; - assert!(second_attempt.is_err()); - assert!(matches!( - second_attempt.err().unwrap(), - DataError::SqlxError(sqlx::Error::Database(_)) - )); + db.insert_room_info(&info2) + .await + .expect("Could not update room info after first insert"); + + let retrieved_info = db + .get_room_info("myroomid") + .await + .expect("Could not get room info"); + + assert!(retrieved_info.is_some()); + let retrieved_info = retrieved_info.unwrap(); + + assert_eq!(retrieved_info.room_id, "myroomid"); + assert_eq!(retrieved_info.room_name, "myroomname2"); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -286,6 +305,44 @@ mod tests { assert_eq!(users_in_room.len(), 0); } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn clear_info_does_not_delete_other_rooms() { + let db = create_db().await; + + let info1 = RoomInfo { + room_id: "myroomid".to_string(), + room_name: "myroomname".to_string(), + }; + + let info2 = RoomInfo { + room_id: "myroomid2".to_string(), + room_name: "myroomname2".to_string(), + }; + + db.insert_room_info(&info1) + .await + .expect("Could not insert room info1."); + + db.insert_room_info(&info2) + .await + .expect("Could not insert room info2."); + + db.add_user_to_room("myuser", &info1.room_id) + .await + .expect("Could not add user to room."); + + db.clear_info(&info1.room_id) + .await + .expect("Could not clear room info1"); + + let room_info2 = db + .get_room_info(&info2.room_id) + .await + .expect("Could not get room info2."); + + assert!(room_info2.is_some()); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn clear_info_test() { let db = create_db().await; @@ -299,23 +356,23 @@ mod tests { .await .expect("Could not insert room info."); - db.add_user_to_room("myuser", "myroom") + db.add_user_to_room("myuser", &info.room_id) .await .expect("Could not add user to room."); - db.clear_info("myroom") + db.clear_info(&info.room_id) .await .expect("Could not clear room info"); let users_in_room = db - .get_users_in_room("myroom") + .get_users_in_room(&info.room_id) .await .expect("Could not get users in room."); assert_eq!(users_in_room.len(), 0); let room_info = db - .get_room_info("myroom") + .get_room_info(&info.room_id) .await .expect("Could not get room info."); -- 2.40.1 From 7eee16961eef0e81c7e81c067086a4254f3f4b38 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Wed, 19 May 2021 21:16:39 +0000 Subject: [PATCH 14/17] Add tests for dbstate. --- src/db/sqlite/state.rs | 63 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/db/sqlite/state.rs b/src/db/sqlite/state.rs index 2830df4..0d831cb 100644 --- a/src/db/sqlite/state.rs +++ b/src/db/sqlite/state.rs @@ -13,6 +13,13 @@ impl DbState for Database { } async fn set_device_id(&self, device_id: &str) -> Result<(), DataError> { + // This will have to be updated if we ever add another column + // to this table! + sqlx::query("DELETE FROM bot_state") + .execute(&self.conn) + .await + .ok(); + sqlx::query( r#"INSERT INTO bot_state (device_id) @@ -25,3 +32,59 @@ impl DbState for Database { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::super::DbState; + use super::*; + + async fn create_db() -> Database { + let db_path = tempfile::NamedTempFile::new_in(".").unwrap(); + crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap()) + .await + .unwrap(); + + Database::new(db_path.path().to_str().unwrap()) + .await + .unwrap() + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn set_and_get_device_id() { + let db = create_db().await; + + db.set_device_id("device_id") + .await + .expect("Could not set device ID"); + + let device_id = db.get_device_id().await.expect("Could not get device ID"); + + assert!(device_id.is_some()); + assert_eq!(device_id.unwrap(), "device_id"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn no_device_id_set_returns_none() { + let db = create_db().await; + let device_id = db.get_device_id().await.expect("Could not get device ID"); + assert!(device_id.is_none()); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn can_update_device_id() { + let db = create_db().await; + + db.set_device_id("device_id") + .await + .expect("Could not set device ID"); + + db.set_device_id("device_id2") + .await + .expect("Could not set device ID"); + + let device_id = db.get_device_id().await.expect("Could not get device ID"); + + assert!(device_id.is_some()); + assert_eq!(device_id.unwrap(), "device_id2"); + } +} -- 2.40.1 From a3b39ee42c3a0d3df6f6a3b55a57160759d43df3 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Wed, 19 May 2021 21:34:11 +0000 Subject: [PATCH 15/17] Use ON CONFLICT and transactions where appropriate. --- src/db/sqlite/rooms.rs | 50 ++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 9e89850..14e8ce4 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -51,18 +51,15 @@ impl Rooms for Database { } async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError> { - //Clear out old info first, because we want this to be an "upsert." - sqlx::query("DELETE FROM room_info where room_id = ?") - .bind(&info.room_id) - .execute(&self.conn) - .await - .ok(); - - sqlx::query(r#"INSERT INTO room_info (room_id, room_name) VALUES (?, ?)"#) - .bind(&info.room_id) - .bind(&info.room_name) - .execute(&self.conn) - .await?; + sqlx::query( + r#"INSERT INTO room_info (room_id, room_name) VALUES (?, ?) + ON CONFLICT(room_id) DO UPDATE SET room_name = ?"#, + ) + .bind(&info.room_id) + .bind(&info.room_name) + .bind(&info.room_name) + .execute(&self.conn) + .await?; Ok(()) } @@ -70,7 +67,7 @@ impl Rooms for Database { async fn get_room_info(&self, room_id: &str) -> Result, DataError> { let info = sqlx::query!( r#"SELECT room_id, room_name FROM room_info - WHERE room_id = ?"#, + WHERE room_id = ?"#, room_id ) .fetch_optional(&self.conn) @@ -85,7 +82,7 @@ impl Rooms for Database { async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError> { let room_ids = sqlx::query!( r#"SELECT room_id FROM room_users - WHERE username = ?"#, + WHERE username = ?"#, user_id ) .fetch_all(&self.conn) @@ -107,16 +104,14 @@ impl Rooms for Database { } async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { - // This is here because it is possible to process a bunch of - // user join/leave events at once, and we don't want to cause - // constraint violation errors. - self.remove_user_from_room(username, room_id).await.ok(); - - sqlx::query("INSERT INTO room_users (room_id, username) VALUES (?, ?)") - .bind(room_id) - .bind(username) - .execute(&self.conn) - .await?; + sqlx::query( + "INSERT INTO room_users (room_id, username) VALUES (?, ?) + ON CONFLICT DO NOTHING", + ) + .bind(room_id) + .bind(username) + .execute(&self.conn) + .await?; Ok(()) } @@ -134,16 +129,19 @@ impl Rooms for Database { async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { // We do not clear event history here, because if we rejoin a // room, we would re-process events we've already seen. + let mut tx = self.conn.begin().await?; + sqlx::query("DELETE FROM room_info where room_id = ?") .bind(room_id) - .execute(&self.conn) + .execute(&mut tx) .await?; sqlx::query("DELETE FROM room_users where room_id = ?") .bind(room_id) - .execute(&self.conn) + .execute(&mut tx) .await?; + tx.commit().await?; Ok(()) } } -- 2.40.1 From 6b6be06c8975f03d0013a754c860a7b341988e80 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Wed, 19 May 2021 21:36:16 +0000 Subject: [PATCH 16/17] Update sqlx offline json. --- sqlx-data.json | 52 +++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/sqlx-data.json b/sqlx-data.json index 125b734..9edc662 100644 --- a/sqlx-data.json +++ b/sqlx-data.json @@ -1,29 +1,5 @@ { "db": "SQLite", - "0ccdb5669e5819628096b8d2f81fef361f73dbcbc27428208a1a4e315a8c7880": { - "query": "SELECT room_id, room_name FROM room_info\n WHERE room_id = ?", - "describe": { - "columns": [ - { - "name": "room_id", - "ordinal": 0, - "type_info": "Text" - }, - { - "name": "room_name", - "ordinal": 1, - "type_info": "Text" - } - ], - "parameters": { - "Right": 1 - }, - "nullable": [ - false, - false - ] - } - }, "19d89370cac05c1bc4de0eb3508712da9ca133b1cf9445b5407d238f89c3ab0c": { "query": "SELECT device_id FROM bot_state limit 1", "describe": { @@ -102,8 +78,8 @@ ] } }, - "ad52bc29fc1eef2bbff8b5f7316047f81ea31b1f910d0e0226125fea89530aa2": { - "query": "SELECT room_id FROM room_users\n WHERE username = ?", + "7248c8ae30bbe4bc5866e80cc277312c7f8cb9af5a8801fd8eaf178fd99eae18": { + "query": "SELECT room_id FROM room_users\n WHERE username = ?", "describe": { "columns": [ { @@ -120,6 +96,30 @@ ] } }, + "97f5d58f62baca51efd8c295ca6737d1240923c69c973621cd0a718ac9eed99f": { + "query": "SELECT room_id, room_name FROM room_info\n WHERE room_id = ?", + "describe": { + "columns": [ + { + "name": "room_id", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "room_name", + "ordinal": 1, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + } + }, "b302d586e5ac4c72c2970361ea5a5936c0b8c6dad10033c626a0ce0404cadb25": { "query": "SELECT username FROM room_users\n WHERE room_id = ?", "describe": { -- 2.40.1 From 5630b4ed204bfb0a1c622dcebb0c4234e81fd030 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Thu, 20 May 2021 15:30:44 +0000 Subject: [PATCH 17/17] Add sled migration utility. --- src/bin/migrate_sled.rs | 37 +++++++++++++++++++++++++++++++++++++ src/db.rs | 2 +- src/db/errors.rs | 3 +++ src/db/sqlite/mod.rs | 2 +- src/db/variables.rs | 39 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 src/bin/migrate_sled.rs diff --git a/src/bin/migrate_sled.rs b/src/bin/migrate_sled.rs new file mode 100644 index 0000000..0e7be05 --- /dev/null +++ b/src/bin/migrate_sled.rs @@ -0,0 +1,37 @@ +use tenebrous_dicebot::db::sqlite::{Database as SqliteDatabase, Variables}; +use tenebrous_dicebot::db::Database; +use tenebrous_dicebot::error::BotError; + +#[tokio::main] +async fn main() -> Result<(), BotError> { + let sled_path = std::env::args() + .skip(1) + .next() + .expect("Need a path to a Sled database as an arument."); + + let sqlite_path = std::env::args() + .skip(2) + .next() + .expect("Need a path to an sqlite database as an arument."); + + let db = Database::new(&sled_path)?; + + let all_variables = db.variables.get_all_variables()?; + + let sql_db = SqliteDatabase::new(&sqlite_path).await?; + + for var in all_variables { + if let ((username, room_id, variable_name), value) = var { + println!( + "Migrating {}::{}::{} = {} to sql", + username, room_id, variable_name, value + ); + + sql_db + .set_user_variable(&username, &room_id, &variable_name, value) + .await; + } + } + + Ok(()) +} diff --git a/src/db.rs b/src/db.rs index 4036e3c..e714957 100644 --- a/src/db.rs +++ b/src/db.rs @@ -19,7 +19,7 @@ pub mod variables; #[derive(Clone)] pub struct Database { db: Db, - pub(crate) variables: Variables, + pub variables: Variables, pub(crate) migrations: Migrations, pub(crate) rooms: Rooms, pub(crate) state: DbState, diff --git a/src/db/errors.rs b/src/db/errors.rs index 8b1757f..c776594 100644 --- a/src/db/errors.rs +++ b/src/db/errors.rs @@ -26,6 +26,9 @@ pub enum DataError { #[error("expected i32, but i32 schema was violated")] I32SchemaViolation, + #[error("parse error")] + ParseError(#[from] std::num::ParseIntError), + #[error("unexpected or corruptd data bytes")] InvalidValue, diff --git a/src/db/sqlite/mod.rs b/src/db/sqlite/mod.rs index 480f502..e470764 100644 --- a/src/db/sqlite/mod.rs +++ b/src/db/sqlite/mod.rs @@ -43,7 +43,7 @@ pub(crate) trait Rooms { // TODO move this up to the top once we delete sled. Traits will be the // main API, then we can have different impls for different DBs. #[async_trait] -pub(crate) trait Variables { +pub trait Variables { async fn get_user_variables( &self, user: &str, diff --git a/src/db/variables.rs b/src/db/variables.rs index 1c3834c..947e78b 100644 --- a/src/db/variables.rs +++ b/src/db/variables.rs @@ -10,6 +10,8 @@ use std::str; use zerocopy::byteorder::I32; use zerocopy::AsBytes; +use super::errors; + pub(super) mod migrations; #[derive(Clone)] @@ -67,6 +69,9 @@ fn alter_room_variable_count( Ok(new_count) } +/// Room ID, Username, Variable Name +pub type AllVariablesKey = (String, String, String); + impl Variables { pub(in crate::db) fn new(db: &sled::Db) -> Result { Ok(Variables { @@ -75,6 +80,40 @@ impl Variables { }) } + pub fn get_all_variables(&self) -> Result, DataError> { + use std::convert::TryFrom; + let variables: Result, DataError> = self + .room_user_variables + .scan_prefix("") + .map(|entry| match entry { + Ok((key, raw_value)) => { + let keys: Vec<_> = key + .split(|&b| b == 0xfe || b == 0xff) + .map(|b| str::from_utf8(b)) + .collect(); + + if let &[Ok(room_id), Ok(username), Ok(variable_name), ..] = keys.as_slice() { + Ok(( + ( + room_id.to_owned(), + username.to_owned(), + variable_name.to_owned(), + ), + convert_i32(&raw_value)?, + )) + } else { + Err(errors::DataError::InvalidValue) + } + } + Err(e) => Err(e.into()), + }) + .collect(); + + // Convert tuples to hash map with collect(), inferred via + // return type. + variables.map(|entries| entries.into_iter().collect()) + } + pub fn get_user_variables( &self, key: &UserAndRoom<'_>, -- 2.40.1