Borked PR: Add get_latest_about_message #7

Closed
notplants wants to merge 6 commits from get-latest-about-message into kvt
3 changed files with 142 additions and 11 deletions

View File

@ -36,6 +36,8 @@ pub enum GolgiError {
Sbot(String),
/// JSON serialization or deserialization error.
SerdeJson(JsonError),
/// Error decoding typed ssb message from content.
ContentTypeDecode(String),
/// Error decoding UTF8 string from bytes
Utf8Parse {
/// The underlying parse error.
@ -54,6 +56,7 @@ impl std::error::Error for GolgiError {
GolgiError::Rpc(ref err) => Some(err),
GolgiError::Sbot(_) => None,
GolgiError::SerdeJson(ref err) => Some(err),
GolgiError::ContentTypeDecode(_) => None,
GolgiError::Utf8Parse{ ref source} => Some(source),
}
}
@ -74,6 +77,11 @@ impl std::fmt::Display for GolgiError {
GolgiError::Sbot(ref err) => write!(f, "Sbot returned an error response: {}", err),
GolgiError::SerdeJson(_) => write!(f, "Failed to serialize JSON slice"),
//GolgiError::WhoAmI(ref err) => write!(f, "{}", err),
GolgiError::ContentTypeDecode(ref err) => write!(
f,
"Failed to decode typed message from ssb message content: {}",
err
),
GolgiError::Utf8Parse{ source } => write!(f, "Failed to deserialize UTF8 from bytes: {}", source),
}
}

View File

@ -29,16 +29,55 @@ pub struct SsbMessageValue {
pub signature: String,
}
// Enum representing the different possible message content types
#[derive(Debug)]
pub enum SsbMessageContentType {
About,
Vote,
Post,
Contact,
None
notplants marked this conversation as resolved
Review

This feels like an anti-pattern (having an Option as a variant). It's better to wrap the SsbMessageContentType in an Option to express "might or might not exist".

This feels like an anti-pattern (having an `Option` as a variant). It's better to wrap the `SsbMessageContentType` in an `Option` to express "might or might not exist".
}
impl SsbMessageValue {
/// Gets the type field of the message content if found,
/// and if not returns "none"
pub fn get_message_type(&self) -> String {
let msg_type: String = self
.content
.get("type")
.map(|msg_type| msg_type.to_string())
.unwrap_or_else(|| "none".to_string());
msg_type
/// Gets the type field of the message content as an enum, if found.
/// if no type field is found, it returns a SsbMessageContentType::None
/// if a type field is found with an invalid format it returns a GolgiError::ContentTypeDecode
pub fn get_message_type(&self) -> Result<SsbMessageContentType, GolgiError> {
let msg_type = self
.content
.get("type");
notplants marked this conversation as resolved
Review

If our message doesn't have a type then we should return from the function right away.

In this case I'd recommend returning an error (a message without a type field in content is erroneous):

let msg_type = self
    .content
    .get("type")
    .ok_or(Err(GolgiError::ContentType))?;

This approach simplifies the matching because msg_type ends up as a &Value. You can then convert the value to &str and match on it straight away.

If our message doesn't have a type then we should return from the function right away. In this case I'd recommend returning an error (a message without a `type` field in `content` is erroneous): ```rust let msg_type = self .content .get("type") .ok_or(Err(GolgiError::ContentType))?; ``` This approach simplifies the matching because `msg_type` ends up as a `&Value`. You can then convert the value to `&str` and match on it straight away.
let enum_type = match msg_type {
Some(mtype) => {
let mtype_str: &str = mtype.as_str().ok_or(GolgiError::ContentTypeDecode("type field with invalid format".to_string()))?;
notplants marked this conversation as resolved
Review

This error variant is something like ContentTypeValue("type field value is not a string as expected").

This error variant is something like `ContentTypeValue("type field value is not a string as expected")`.
match mtype_str {
"about" => SsbMessageContentType::About,
"post" => SsbMessageContentType::Post,
"vote" => SsbMessageContentType::Vote,
"contact" => SsbMessageContentType::Contact,
_ => return Err(GolgiError::ContentTypeDecode("type field with unknown value".to_string()))
notplants marked this conversation as resolved
Review

I can think of two approaches here to handle the "pattern not matched" case:

  1. Return an error (as you have), but use a different error variant (this error is not the same as "we expected a string but got some other value"). Something like GolgiError::UnknownContentType.

  2. Add a variant to the SsbMessageContentType enum to represent any unknown / unrecognized / unsupported content type:

pub enum SsbMessageContentType {
    About,
    Vote,
    Post,
    Contact,
    Unrecognized,
}

I prefer the second solution since it allows us to represent all valid (ie. string-type) message content types in our SsbMessageContentType type.

I can think of two approaches here to handle the "pattern not matched" case: 1) Return an error (as you have), but use a different error variant (this error is not the same as "we expected a string but got some other value"). Something like `GolgiError::UnknownContentType`. 2) Add a variant to the `SsbMessageContentType` `enum` to represent any unknown / unrecognized / unsupported content type: ```rust pub enum SsbMessageContentType { About, Vote, Post, Contact, Unrecognized, } ``` I prefer the second solution since it allows us to represent all valid (ie. string-type) message content types in our `SsbMessageContentType` type.
}
},
None => {
SsbMessageContentType::None
}
};
Ok(enum_type)
}
/// Helper function which returns true if this message is of the given type,
/// and false if the type does not match or is not found
pub fn is_message_type(&self, message_type: SsbMessageContentType) -> bool {
let self_message_type = self.get_message_type();
match self_message_type {
Ok(mtype) => {
matches!(mtype, message_type)
}
Err(_err) => {
false
}
}
}
/// Converts the content json value into an SsbMessageContent enum,

View File

@ -5,7 +5,7 @@ use kuska_handshake::async_std::BoxStream;
use kuska_sodiumoxide::crypto::{auth, sign::ed25519};
use kuska_ssb::{
api::{
dto::{content::SubsetQuery, CreateHistoryStreamIn},
dto::{CreateHistoryStreamIn},
ApiCaller,
},
discovery, keystore,
@ -14,9 +14,12 @@ use kuska_ssb::{
};
use crate::error::GolgiError;
use crate::messages::{SsbMessageKVT, SsbMessageContent, SsbMessageValue};
use crate::messages::{SsbMessageKVT, SsbMessageContent, SsbMessageValue, SsbMessageContentType};
use crate::utils;
// re-export types from kuska
pub use kuska_ssb::api::dto::content::SubsetQuery;
/// The Scuttlebutt identity, keys and configuration parameters for connecting to a local sbot
/// instance, as well as handles for calling RPC methods and receiving responses.
pub struct Sbot {
@ -146,6 +149,87 @@ impl Sbot {
self.publish(msg).await
}
/// Wrapper for publish which constructs and publishes an about name message appropriately from a string.
///
/// # Arguments
///
/// * `name` - A reference to a string slice which represents the text to be published as an about name.
pub async fn publish_name(&mut self, name: &str) -> Result<String, GolgiError> {
let msg = SsbMessageContent::About {
about: self.id.to_string(),
name: Some(name.to_string()),
title: None,
branch: None,
image: None,
description: None,
location: None,
start_datetime: None,
};
self.publish(msg).await
}
/// Get the about messages for a particular user in order of recency.
pub async fn get_about_messages(&mut self, ssb_id: &str) -> Result<Vec<SsbMessageValue>, GolgiError> {
let query = SubsetQuery::Author{
op: "author".to_string(),
feed: ssb_id.to_string(),
};
let kvts: Vec<SsbMessageKVT> = self.get_subset(query).await?;
let messages: Vec<SsbMessageValue> = kvts.into_iter().map(|kvt| kvt.value).collect();
// TODO: after fixing sbot regression,
notplants marked this conversation as resolved
Review

👍

👍
// change this subset query to filter by type about in addition to author
// and remove this filter section
// filter down to about messages
let mut about_messages: Vec<SsbMessageValue> = messages.into_iter().filter(|msg| {
msg.is_message_type(SsbMessageContentType::About)
}).collect();
// TODO: use subset query to order messages instead of doing it this way
about_messages.sort_by(|a, b| {
b.timestamp.partial_cmp(&a.timestamp).unwrap()
});
// return about messages
Ok(about_messages)
}
/// Get value of latest about message with given key from given user
pub async fn get_latest_about_message(&mut self, ssb_id: &str, key: &str) -> Result<Option<String>, GolgiError> {
// vector of about messages with most recent at the front of the vector
let about_messages = self.get_about_messages(ssb_id).await?;
// iterate through the vector looking for an about message with the given key
// the first one we find is the most recent
for msg in about_messages {
Review

I had some fun playing with a more compact solution (took me a few hours...I'm not great with iterators and transformation yet).

Here's what I came up with:

let about_messages = self.get_about_messages(ssb_id).await?;

// iterate through all the about messages
let latest_about = about_messages
    .iter()
    // find the first msg that contains the field `key`
    .find(|msg| msg.content.get(key).is_some())
    // map the found msg (`Some(SsbMessageValue)`) to a `Some(Some(&Value))`
    .map(|msg| msg.content.get(key))
    // flatten `Some(Some(&Value))` into `Some(&Value)`
    .flatten()
    // map `Some(&Value)` to `Some(Some(&str))`
    .map(|msg_val| msg_val.as_str())
    // flatten `Some(Some(&str))` to `Some(&str)`
    .flatten()
    // map `Some(&str))` to `Some(String)`
    .map(|msg_str| msg_str.to_string());

// return value is either `Ok(Some(String))` or `Ok(None)`
Ok(latest_about)
I had some fun playing with a more compact solution (took me a few hours...I'm not great with iterators and transformation yet). Here's what I came up with: ```rust let about_messages = self.get_about_messages(ssb_id).await?; // iterate through all the about messages let latest_about = about_messages .iter() // find the first msg that contains the field `key` .find(|msg| msg.content.get(key).is_some()) // map the found msg (`Some(SsbMessageValue)`) to a `Some(Some(&Value))` .map(|msg| msg.content.get(key)) // flatten `Some(Some(&Value))` into `Some(&Value)` .flatten() // map `Some(&Value)` to `Some(Some(&str))` .map(|msg_val| msg_val.as_str()) // flatten `Some(Some(&str))` to `Some(&str)` .flatten() // map `Some(&str))` to `Some(String)` .map(|msg_str| msg_str.to_string()); // return value is either `Ok(Some(String))` or `Ok(None)` Ok(latest_about) ```
Review

The nice thing about these iterator methods is being able to avoid nested match statements, especially those that have multiple arms which don't do much (e.g. just return continue).

The nice thing about these iterator methods is being able to avoid nested `match` statements, especially those that have multiple arms which don't do much (e.g. just return `continue`).
Review

nice! the comments on this are great. makes sense. helpful to see flatten used like this too.

nice! the comments on this are great. makes sense. helpful to see flatten used like this too.
let value = msg.content.get(key);
match value {
Some(val) => {
let val_as_str_option = val.as_str();
match val_as_str_option {
Some(val_as_str) => {
return Ok(Some(val_as_str.to_string()))
},
None => {
// if it is an improperly formatted field (not a string)
// then just continue
continue
}
}
},
None => {
continue
}
}
}
// if no about message with given key was found, then return None
Ok(None)
}
pub async fn get_name(&mut self, ssb_id: &str) -> Result<Option<String>, GolgiError> {
self.get_latest_about_message(ssb_id, "name").await
}
pub async fn get_description(&mut self, ssb_id: &str) -> Result<Option<String>, GolgiError> {
self.get_latest_about_message(ssb_id, "description").await
}
/// Call the `createHistoryStream` RPC method and return a vector
/// of SsbMessageValue.
pub async fn create_history_stream(