commit 19933c84f1094c89db2bfa8df73d599a1f1bfba4
parent 3b3b8246c83f9abd2d217ba4fbdf7483882abfc7
Author: William Casarin <jb55@jb55.com>
Date: Mon, 18 Nov 2024 17:06:52 -0800
onboarding: lookup profile after accounts are added
To reduce the side effects of this change, we introduce a new UnknownId
action type:
- SingleUnkIdAction
This can be returned from functions to signal that we need to do some
work to look for things. We add a `must_use` directive to this type
to ensure callers handle it.
Changelog-Fixed: Fix missing profiles when new accounts are added
Fixes: https://github.com/damus-io/notedeck/issues/356
Diffstat:
6 files changed, 146 insertions(+), 27 deletions(-)
diff --git a/src/account_manager.rs b/src/account_manager.rs
@@ -14,6 +14,7 @@ use crate::{
account_login_view::{AccountLoginResponse, AccountLoginView},
account_management::{AccountsView, AccountsViewResponse},
},
+ unknowns::SingleUnkIdAction,
};
use tracing::{error, info};
@@ -50,7 +51,7 @@ pub fn render_accounts_route(
accounts: &mut AccountManager,
login_state: &mut AcquireKeyState,
route: AccountsRoute,
-) {
+) -> SingleUnkIdAction {
let router = columns.column_mut(col).router_mut();
let resp = match route {
AccountsRoute::Accounts => AccountsView::new(ndb, accounts, img_cache)
@@ -68,13 +69,17 @@ pub fn render_accounts_route(
match resp {
AccountsRouteResponse::Accounts(response) => {
process_accounts_view_response(accounts, response, router);
+ SingleUnkIdAction::no_action()
}
AccountsRouteResponse::AddAccount(response) => {
- process_login_view_response(accounts, response);
+ let action = process_login_view_response(accounts, response);
*login_state = Default::default();
router.go_back();
+ action
}
}
+ } else {
+ SingleUnkIdAction::no_action()
}
}
@@ -153,14 +158,17 @@ impl AccountManager {
false
}
- pub fn add_account(&mut self, account: Keypair) -> bool {
+ #[must_use = "UnknownIdAction's must be handled. Use .process_unknown_id_action()"]
+ pub fn add_account(&mut self, account: Keypair) -> SingleUnkIdAction {
if self.has_account_pubkey(account.pubkey.bytes()) {
info!("already have account, not adding {}", account.pubkey);
- return false;
+ return SingleUnkIdAction::pubkey(account.pubkey);
}
+
let _ = self.key_store.add_key(&account);
+ let pk = account.pubkey;
self.accounts.push(account);
- true
+ SingleUnkIdAction::pubkey(pk)
}
pub fn num_accounts(&self) -> usize {
@@ -215,14 +223,16 @@ fn get_selected_index(accounts: &[UserAccount], keystore: &KeyStorageType) -> Op
None
}
-pub fn process_login_view_response(manager: &mut AccountManager, response: AccountLoginResponse) {
- match response {
+pub fn process_login_view_response(
+ manager: &mut AccountManager,
+ response: AccountLoginResponse,
+) -> SingleUnkIdAction {
+ let r = match response {
AccountLoginResponse::CreateNew => {
- manager.add_account(FullKeypair::generate().to_keypair());
+ manager.add_account(FullKeypair::generate().to_keypair())
}
- AccountLoginResponse::LoginWith(keypair) => {
- manager.add_account(keypair);
- }
- }
+ AccountLoginResponse::LoginWith(keypair) => manager.add_account(keypair),
+ };
manager.select_account(manager.num_accounts() - 1);
+ r
}
diff --git a/src/app.rs b/src/app.rs
@@ -419,9 +419,17 @@ impl Damus {
let num_keys = parsed_args.keys.len();
- for key in parsed_args.keys {
- info!("adding account: {}", key.pubkey);
- accounts.add_account(key);
+ let mut unknown_ids = UnknownIds::default();
+ let ndb = Ndb::new(&dbpath_str, &config).expect("ndb");
+
+ {
+ let txn = Transaction::new(&ndb).expect("txn");
+ for key in parsed_args.keys {
+ info!("adding account: {}", key.pubkey);
+ accounts
+ .add_account(key)
+ .process_action(&mut unknown_ids, &ndb, &txn);
+ }
}
if num_keys != 0 {
@@ -454,7 +462,6 @@ impl Damus {
.get_selected_account()
.as_ref()
.map(|a| a.pubkey.bytes());
- let ndb = Ndb::new(&dbpath_str, &config).expect("ndb");
let mut columns = if parsed_args.columns.is_empty() {
if let Some(serializable_columns) = storage::load_columns(&path) {
@@ -491,7 +498,7 @@ impl Damus {
Self {
pool,
debug,
- unknown_ids: UnknownIds::default(),
+ unknown_ids,
subscriptions: Subscriptions::default(),
since_optimize: parsed_args.since_optimize,
threads: NotesHolderStorage::default(),
diff --git a/src/nav.rs b/src/nav.rs
@@ -84,7 +84,7 @@ pub fn render_nav(col: usize, app: &mut Damus, ui: &mut egui::Ui) -> Option<Rend
ui,
),
Route::Accounts(amr) => {
- render_accounts_route(
+ let action = render_accounts_route(
ui,
&app.ndb,
col,
@@ -94,6 +94,8 @@ pub fn render_nav(col: usize, app: &mut Damus, ui: &mut egui::Ui) -> Option<Rend
&mut app.view_state.login,
*amr,
);
+ let txn = Transaction::new(&app.ndb).expect("txn");
+ action.process_action(&mut app.unknown_ids, &app.ndb, &txn);
None
}
Route::Relays => {
diff --git a/src/notes_holder.rs b/src/notes_holder.rs
@@ -6,7 +6,7 @@ use tracing::{debug, info, warn};
use crate::{
actionbar::NotesHolderResult, multi_subscriber::MultiSubscriber, note::NoteRef,
- notecache::NoteCache, timeline::TimelineTab, Error, Result,
+ notecache::NoteCache, timeline::TimelineTab, unknowns::NoteRefsUnkIdAction, Error, Result,
};
pub struct NotesHolderStorage<M: NotesHolder> {
@@ -109,19 +109,21 @@ pub trait NotesHolder {
notes: Vec<NoteRef>,
) -> Self;
- #[must_use = "UnknownIds::update_from_note_refs should be used on this result"]
- fn poll_notes_into_view(&mut self, txn: &Transaction, ndb: &Ndb) -> Result<()> {
+ fn poll_notes_into_view(
+ &mut self,
+ txn: &Transaction,
+ ndb: &Ndb,
+ ) -> Result<NoteRefsUnkIdAction> {
if let Some(multi_subscriber) = self.get_multi_subscriber() {
let reversed = true;
let note_refs: Vec<NoteRef> = multi_subscriber.poll_for_notes(ndb, txn)?;
self.get_view().insert(¬e_refs, reversed);
+ Ok(NoteRefsUnkIdAction::new(note_refs))
} else {
- return Err(Error::Generic(
+ Err(Error::Generic(
"NotesHolder unexpectedly has no MultiSubscriber".to_owned(),
- ));
+ ))
}
-
- Ok(())
}
/// Look for new thread notes since our last fetch
diff --git a/src/test_data.rs b/src/test_data.rs
@@ -1,7 +1,7 @@
use std::path::Path;
use enostr::{FullKeypair, Pubkey, RelayPool};
-use nostrdb::ProfileRecord;
+use nostrdb::{ProfileRecord, Transaction};
use crate::{user_account::UserAccount, Damus};
@@ -100,8 +100,11 @@ pub fn test_app() -> Damus {
let mut app = Damus::mock(path);
let accounts = get_test_accounts();
+ let txn = Transaction::new(&app.ndb).expect("txn");
for account in accounts {
- app.accounts_mut().add_account(account);
+ app.accounts_mut()
+ .add_account(account)
+ .process_action(&mut app.unknown_ids, &app.ndb, &txn)
}
app
diff --git a/src/unknowns.rs b/src/unknowns.rs
@@ -12,6 +12,74 @@ use std::collections::HashSet;
use std::time::{Duration, Instant};
use tracing::error;
+#[must_use = "process_action should be used on this result"]
+pub enum SingleUnkIdAction {
+ NoAction,
+ NeedsProcess(UnknownId),
+}
+
+#[must_use = "process_action should be used on this result"]
+pub enum NoteRefsUnkIdAction {
+ NoAction,
+ NeedsProcess(Vec<NoteRef>),
+}
+
+impl NoteRefsUnkIdAction {
+ pub fn new(refs: Vec<NoteRef>) -> Self {
+ NoteRefsUnkIdAction::NeedsProcess(refs)
+ }
+
+ pub fn no_action() -> Self {
+ Self::NoAction
+ }
+
+ pub fn process_action(
+ &self,
+ txn: &Transaction,
+ ndb: &Ndb,
+ unk_ids: &mut UnknownIds,
+ note_cache: &mut NoteCache,
+ ) {
+ match self {
+ Self::NoAction => {}
+ Self::NeedsProcess(refs) => {
+ UnknownIds::update_from_note_refs(txn, ndb, unk_ids, note_cache, refs);
+ }
+ }
+ }
+}
+
+impl SingleUnkIdAction {
+ pub fn new(id: UnknownId) -> Self {
+ SingleUnkIdAction::NeedsProcess(id)
+ }
+
+ pub fn no_action() -> Self {
+ Self::NoAction
+ }
+
+ pub fn pubkey(pubkey: Pubkey) -> Self {
+ SingleUnkIdAction::new(UnknownId::Pubkey(pubkey))
+ }
+
+ pub fn note_id(note_id: NoteId) -> Self {
+ SingleUnkIdAction::new(UnknownId::Id(note_id))
+ }
+
+ /// Some functions may return unknown id actions that need to be processed.
+ /// For example, when we add a new account we need to make sure we have the
+ /// profile for that account. This function ensures we add this to the
+ /// unknown id tracker without adding side effects to functions.
+ pub fn process_action(&self, ids: &mut UnknownIds, ndb: &Ndb, txn: &Transaction) {
+ match self {
+ Self::NeedsProcess(id) => {
+ ids.add_unknown_id_if_missing(ndb, txn, id);
+ }
+ Self::NoAction => {}
+ }
+ }
+}
+
/// Unknown Id searcher
#[derive(Default)]
pub struct UnknownIds {
@@ -121,6 +189,33 @@ impl UnknownIds {
}
}
+ pub fn add_unknown_id_if_missing(&mut self, ndb: &Ndb, txn: &Transaction, unk_id: &UnknownId) {
+ match unk_id {
+ UnknownId::Pubkey(pk) => self.add_pubkey_if_missing(ndb, txn, pk),
+ UnknownId::Id(note_id) => self.add_note_id_if_missing(ndb, txn, note_id),
+ }
+ }
+
+ pub fn add_pubkey_if_missing(&mut self, ndb: &Ndb, txn: &Transaction, pubkey: &Pubkey) {
+ // we already have this profile, skip
+ if ndb.get_profile_by_pubkey(txn, pubkey).is_ok() {
+ return;
+ }
+
+ self.ids.insert(UnknownId::Pubkey(*pubkey));
+ self.mark_updated();
+ }
+
+ pub fn add_note_id_if_missing(&mut self, ndb: &Ndb, txn: &Transaction, note_id: &NoteId) {
+ // we already have this note, skip
+ if ndb.get_note_by_id(txn, note_id.bytes()).is_ok() {
+ return;
+ }
+
+ self.ids.insert(UnknownId::Id(*note_id));
+ self.mark_updated();
+ }
+
pub fn update(
txn: &Transaction,
unknown_ids: &mut UnknownIds,
