Entry store updates review

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Move to `tokio` async runtime [#75](https://github.com/p2panda/aquadoggo/pull/75)
 - Implement SQL storage using `p2panda_rs` storage provider traits [#80](https://github.com/p2panda/aquadoggo/pull/80)
 - Improve `Signal` efficiency in `ServiceManager` [#95](https://github.com/p2panda/aquadoggo/pull/95)
+- `EntryStore` improvements [#123](https://github.com/p2panda/aquadoggo/pull/123)
 
 ## [0.2.0]
 

aquadoggo/src/db/models/entry.rs

@@ -5,8 +5,8 @@ use sqlx::FromRow;
 
 /// Struct representing the actual SQL row of `Entry`.
 ///
-/// We store the u64 integer values of `log_id` and `seq_num` as strings since not all database
-/// backend support large numbers.
+/// We store the u64 integer values of `log_id` and `seq_num` as strings since SQLite doesn't
+/// support storing unsigned 64 bit integers.
 #[derive(FromRow, Debug, Serialize, Clone, PartialEq)]
 #[serde(rename_all = "camelCase")]
 pub struct EntryRow {

aquadoggo/src/db/models/log.rs

@@ -7,8 +7,8 @@ use sqlx::FromRow;
 /// This serves as an indexing layer on top of the lower-level bamboo entries. The node updates
 /// this data according to what it sees in the newly incoming entries.
 ///
-/// We store the u64 integer values of `log_id` as a string here since not all database backends
-/// support large numbers.
+/// We store the u64 integer values of `log_id` as a string here since SQLite doesn't support
+/// storing unsigned 64 bit integers.
 #[derive(FromRow, Debug, Clone)]
 pub struct LogRow {
     /// Public key of the author.

aquadoggo/src/db/models/mod.rs

@@ -1,4 +1,7 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
-pub mod entry;
-pub mod log;
+mod entry;
+mod log;
+
+pub use self::log::LogRow;
+pub use entry::EntryRow;

aquadoggo/src/db/provider.rs

@@ -6,8 +6,8 @@ use p2panda_rs::document::DocumentId;
 use p2panda_rs::hash::Hash;
 use p2panda_rs::storage_provider::traits::StorageProvider;
 
-use crate::db::stores::entry::StorageEntry;
-use crate::db::stores::log::StorageLog;
+use crate::db::stores::StorageEntry;
+use crate::db::stores::StorageLog;
 use crate::db::Pool;
 use crate::errors::StorageProviderResult;
 use crate::rpc::{EntryArgsRequest, EntryArgsResponse, PublishEntryRequest, PublishEntryResponse};

aquadoggo/src/db/stores/entry.rs

@@ -14,11 +14,14 @@ use p2panda_rs::schema::SchemaId;
 use p2panda_rs::storage_provider::errors::EntryStorageError;
 use p2panda_rs::storage_provider::traits::{AsStorageEntry, EntryStore};
 
-use crate::db::models::entry::EntryRow;
+use crate::db::models::EntryRow;
 use crate::db::provider::SqlStorage;
 
+/// Represents a stored entry in a way that is conducive to implementing storage traits.
+///
+/// In the case of `EntryRow`, the traits could have been implemented directly on it but this
+/// structure was chosen to align with the way the other stores are structured.
 #[derive(Debug, Clone, PartialEq)]
-
 pub struct StorageEntry {
     entry_signed: EntrySigned,
     operation_encoded: OperationEncoded,
@@ -118,9 +121,8 @@ impl AsStorageEntry for StorageEntry {
 impl EntryStore<StorageEntry> for SqlStorage {
     /// Insert an entry into storage.
     ///
-    /// Returns a result containing `true` when the insertion occured (one row affected)
-    /// returns `false` when an unexpected number of rows was affected. Errors when
-    /// a fatal storage error occured.
+    /// Returns an error if the insertion doesn't result in exactly one
+    /// affected row.
     async fn insert_entry(&self, entry: StorageEntry) -> Result<(), EntryStorageError> {
         let insert_entry_result = query(
             "
@@ -257,7 +259,7 @@ impl EntryStore<StorageEntry> for SqlStorage {
                 author = $1
                 AND log_id = $2
             ORDER BY
-                CAST(seq_num AS INTEGER) DESC
+                CAST(seq_num AS NUMERIC) DESC
             LIMIT
                 1
             ",
@@ -275,7 +277,7 @@ impl EntryStore<StorageEntry> for SqlStorage {
     ///
     /// Returns a result containing a vector of all entries which follow the passed
     /// schema (identified by it's `SchemaId`). If no entries exist, or the schema
-    /// is not known by this node, then an empty vecot is returned.
+    /// is not known by this node, then an empty vector is returned.
     async fn get_entries_by_schema(
         &self,
         schema: &SchemaId,
@@ -307,7 +309,7 @@ impl EntryStore<StorageEntry> for SqlStorage {
         Ok(entries.into_iter().map(|row| row.into()).collect())
     }
 
-    /// Get all entries of a given schema
+    /// Get all entries of a given schema.
     ///
     /// Returns a result containing a vector of all entries which follow the passed
     /// schema (identified by it's `SchemaId`). If no entries exist, or the schema
@@ -335,9 +337,9 @@ impl EntryStore<StorageEntry> for SqlStorage {
             WHERE
                 author = $1
                 AND log_id = $2
-                AND CAST(seq_num AS INTEGER) BETWEEN $3 and $4
+                AND CAST(seq_num AS NUMERIC) BETWEEN $3 and $4
             ORDER BY
-                CAST(seq_num AS INTEGER)
+                CAST(seq_num AS NUMERIC)
             ",
         )
         .bind(author.as_str())
@@ -388,9 +390,9 @@ impl EntryStore<StorageEntry> for SqlStorage {
             WHERE
                 author = $1
                 AND log_id = $2
-                AND CAST(seq_num AS INTEGER) IN ({})
+                AND CAST(seq_num AS NUMERIC) IN ({})
             ORDER BY
-                CAST(seq_num AS INTEGER) DESC
+                CAST(seq_num AS NUMERIC) DESC
             ",
             cert_pool_seq_nums
         );
@@ -501,7 +503,11 @@ mod tests {
         .unwrap();
         let result = storage_provider.insert_entry(duplicate_doggo_entry).await;
 
-        assert_eq!(result.unwrap_err().to_string(), "Error occured during `EntryStorage` request in storage provider: error returned from database: UNIQUE constraint failed: entries.author, entries.log_id, entries.seq_num")
+        assert_eq!(
+            result.unwrap_err().to_string(),
+            "Error occured during `EntryStorage` request in storage provider: error returned from \
+            database: UNIQUE constraint failed: entries.author, entries.log_id, entries.seq_num"
+        )
     }
 
     #[tokio::test]
@@ -622,7 +628,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn gets_next_n_entries_after_seq() {
+    async fn get_paginated_log_entries() {
         let storage_provider = test_db(50).await;
 
         let key_pair = KeyPair::from_private_key_str(DEFAULT_PRIVATE_KEY).unwrap();

aquadoggo/src/db/stores/log.rs

@@ -11,7 +11,10 @@ use p2panda_rs::storage_provider::errors::LogStorageError;
 use p2panda_rs::storage_provider::traits::{AsStorageLog, LogStore};
 
 use crate::db::provider::SqlStorage;
-
+/// Represents a stored log in a way that is conducive to implementing storage traits.
+///
+/// In the case of `LogRow`, the traits could have been implemented directly on it but this
+/// structure was chosen to align with the way the other stores are structured.
 pub struct StorageLog {
     author: Author,
     log_id: LogId,
@@ -105,8 +108,12 @@ impl LogStore<StorageLog> for SqlStorage {
         .map_err(|e| LogStorageError::Custom(e.to_string()))?;
 
         // Wrap u64 inside of `P2PandaLog` instance
-        let log_id: Option<LogId> =
-            result.map(|str| str.parse().expect("Corrupt u64 integer found in database"));
+        let log_id: Option<LogId> = result.map(|str| {
+            str.parse().expect(&format!(
+                "Corrupt u64 integer found in database: '{0}'",
+                &str
+            ))
+        });
 
         Ok(log_id)
     }
@@ -132,7 +139,12 @@ impl LogStore<StorageLog> for SqlStorage {
         // Convert all strings representing u64 integers to `LogId` instances
         let mut log_ids: Vec<LogId> = result
             .iter_mut()
-            .map(|str| str.parse().expect("Corrupt u64 integer found in database"))
+            .map(|str| {
+                str.parse().expect(&format!(
+                    "Corrupt u64 integer found in database: '{0}'",
+                    &str
+                ))
+            })
             .collect();
 
         // The log id selection below expects log ids in sorted order. We can't easily use SQL

aquadoggo/src/db/stores/mod.rs

@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
-pub mod entry;
-pub mod log;
+mod entry;
+mod log;
 #[cfg(test)]
 mod test_utils;
+
+pub use self::log::StorageLog;
+pub use entry::StorageEntry;

aquadoggo/src/rpc/response.rs

@@ -6,7 +6,7 @@ use p2panda_rs::entry::{LogId, SeqNum};
 use p2panda_rs::hash::Hash;
 use p2panda_rs::storage_provider::traits::{AsEntryArgsResponse, AsPublishEntryResponse};
 
-use crate::db::models::entry::EntryRow;
+use crate::db::models::EntryRow;
 
 /// Response body of `panda_getEntryArguments`.
 ///