Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ signal-hook = "0.4"
sorted-vec = "0.8"
strip-ansi-escapes = "0.2"
strum = { version = "0.28", features = ["derive"] }
tears = "0.9.1"
tears = "0.10.2"
thousands = "0.2"
tokio = { version = "1", features = ["full"] }
tokio-util = "0.7"
Expand Down
7 changes: 3 additions & 4 deletions src/infrastructure/subscription/media.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use futures::stream;
use futures::stream::{BoxStream, StreamExt};
use nowhear::{MediaEvent, MediaSource, MediaSourceBuilder, MediaSourceError};
use tears::{SubscriptionId, SubscriptionSource};
use tears::SubscriptionSource;

#[derive(Clone, Debug, Default)]
pub struct MediaEvents;
Expand All @@ -15,6 +15,7 @@ impl MediaEvents {

impl SubscriptionSource for MediaEvents {
type Output = Result<MediaEvent, MediaSourceError>;
type Key = ();

fn stream(&self) -> BoxStream<'static, Self::Output> {
stream::once(async {
Expand All @@ -30,9 +31,7 @@ impl SubscriptionSource for MediaEvents {
.boxed()
}

fn id(&self) -> SubscriptionId {
SubscriptionId::of::<Self>(42)
}
fn key(&self) -> Self::Key {}
}

#[cfg(test)]
Expand Down
64 changes: 30 additions & 34 deletions src/infrastructure/subscription/nostr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use futures::{
StreamExt,
};
use nostr_sdk::prelude::*;
use tears::{SubscriptionId, SubscriptionSource};
use tears::SubscriptionSource;
use tokio::sync::{broadcast, mpsc, RwLock};

use crate::domain::nostr::feed_filter::{
Expand Down Expand Up @@ -336,6 +336,7 @@ impl NostrEvents {

impl SubscriptionSource for NostrEvents {
type Output = Message;
type Key = u64;

fn stream(&self) -> BoxStream<'static, Self::Output> {
let (msg_tx, msg_rx) = mpsc::unbounded_channel();
Expand Down Expand Up @@ -365,11 +366,10 @@ impl SubscriptionSource for NostrEvents {
.boxed()
}

fn id(&self) -> SubscriptionId {
// Use the Arc pointer address as a unique ID
// Same Arc<Client> instance = same ID, different Client instance = different ID
let ptr = Arc::as_ptr(&self.client) as usize as u64;
SubscriptionId::of::<Self>(ptr)
fn key(&self) -> Self::Key {
// Use the Arc pointer address as the structural key
// Same Arc<Client> instance = same key, different Client instance = different key
Arc::as_ptr(&self.client) as usize as u64
}
}

Expand All @@ -395,44 +395,40 @@ mod tests {
}

#[test]
fn test_subscription_id_uses_arc_pointer() {
fn test_subscription_key_uses_arc_pointer() {
use tears::SubscriptionSource;

let client = Arc::new(Client::default());
let nostr_events1 = NostrEvents::new(Arc::clone(&client));
let nostr_events2 = nostr_events1.clone();

// Same Arc<Client> should produce same ID
// Same Arc<Client> should produce same key
assert_eq!(
nostr_events1.id(),
nostr_events2.id(),
"Cloned NostrEvents should share the same Arc and produce the same ID"
nostr_events1.key(),
nostr_events2.key(),
"Cloned NostrEvents should share the same Arc and produce the same key"
);

// Verify ID is not zero (regression test for the bug where ID was always 0)
let id1 = nostr_events1.id();
// Verify key is not zero (regression test for the bug where ID was always 0)
let key1 = nostr_events1.key();
let ptr1 = Arc::as_ptr(&nostr_events1.client) as usize as u64;
assert_eq!(
SubscriptionId::of::<NostrEvents>(ptr1),
id1,
"ID should be based on Arc pointer address"
);
assert_eq!(ptr1, key1, "Key should be based on Arc pointer address");
assert_ne!(
ptr1, 0,
"Arc pointer address should not be zero in normal circumstances"
);

// Reusing the same Arc should produce the same ID
// Reusing the same Arc should produce the same key
let nostr_events3 = NostrEvents::new(Arc::clone(&client));
assert_eq!(
nostr_events1.id(),
nostr_events3.id(),
"Different NostrEvents instances with the same Arc<Client> should have the same ID"
nostr_events1.key(),
nostr_events3.key(),
"Different NostrEvents instances with the same Arc<Client> should have the same key"
);
}

#[test]
fn test_subscription_id_different_clients() {
fn test_subscription_key_different_clients() {
use tears::SubscriptionSource;

// Create two separate clients with different Arcs
Expand All @@ -442,14 +438,14 @@ mod tests {
let nostr_events1 = NostrEvents::new(Arc::clone(&client1));
let nostr_events2 = NostrEvents::new(Arc::clone(&client2));

// Different Arc<Client> instances should produce different IDs
// Different Arc<Client> instances should produce different keys
assert_ne!(
nostr_events1.id(),
nostr_events2.id(),
"Different Arc<Client> instances should produce different subscription IDs"
nostr_events1.key(),
nostr_events2.key(),
"Different Arc<Client> instances should produce different subscription keys"
);

// Verify both IDs use actual pointer addresses
// Verify both keys use actual pointer addresses
let ptr1 = Arc::as_ptr(&nostr_events1.client) as usize as u64;
let ptr2 = Arc::as_ptr(&nostr_events2.client) as usize as u64;
assert_ne!(
Expand All @@ -459,20 +455,20 @@ mod tests {
}

#[test]
fn test_subscription_id_different_arc_instances() {
fn test_subscription_key_different_arc_instances() {
use tears::SubscriptionSource;

let client = Client::default();

// Creating separate Arc instances produces different IDs
// Creating separate Arc instances produces different keys
let nostr_events1 = NostrEvents::new(Arc::new(client.clone()));
let nostr_events2 = NostrEvents::new(Arc::new(client));

// Different Arc instances should produce different IDs
// Different Arc instances should produce different keys
assert_ne!(
nostr_events1.id(),
nostr_events2.id(),
"Different Arc instances produce different subscription IDs"
nostr_events1.key(),
nostr_events2.key(),
"Different Arc instances produce different subscription keys"
);

// This demonstrates why you must share the same Arc<Client>
Expand Down
30 changes: 24 additions & 6 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
#![deny(warnings)]

use std::num::{NonZeroU32, NonZeroU64};

use clap::Parser;
use color_eyre::eyre::{eyre, Result};
use nostr_sdk::prelude::*;
use secrecy::ExposeSecret;
use tears::{subscription::time::Timer, Runtime};
use tears::{subscription::time::Timer, FrameRate, Runtime};

use nostui::{
application::config::Config,
Expand All @@ -25,9 +27,25 @@ fn tick_timer_from_rate(tick_rate: f64) -> Result<Timer> {
));
}

Timer::try_new(interval_ms as u64).ok_or_else(|| {
let interval_ms = NonZeroU64::new(interval_ms as u64).ok_or_else(|| {
eyre!("tick rate is too high to produce a non-zero millisecond timer interval: {tick_rate}")
})
})?;

Ok(Timer::new(interval_ms))
}

fn frame_rate_from_value(frame_rate: f64) -> Result<FrameRate> {
if !frame_rate.is_finite() || frame_rate <= 0.0 {
return Err(eyre!("frame rate must be a positive finite number"));
}
if frame_rate > f64::from(u32::MAX) {
return Err(eyre!("frame rate is too high: {frame_rate}"));
}

let frames_per_second = NonZeroU32::new(frame_rate as u32)
.ok_or_else(|| eyre!("frame rate is too low to produce a non-zero FPS: {frame_rate}"))?;

FrameRate::new(frames_per_second).map_err(|e| eyre!("invalid frame rate: {e}"))
}

async fn tokio_main() -> Result<()> {
Expand Down Expand Up @@ -79,7 +97,7 @@ async fn tokio_main() -> Result<()> {
"Starting Tears application with frame_rate: {}",
args.frame_rate
);
let runtime = Runtime::<TearsApp>::try_new(init_flags, args.frame_rate as u32)?;
let runtime = Runtime::<TearsApp>::new(init_flags, frame_rate_from_value(args.frame_rate)?);
let result = runtime.run(&mut terminal).await;

// Restore terminal
Expand All @@ -106,11 +124,11 @@ mod tests {
fn tick_timer_from_rate_accepts_positive_tick_rate() {
assert_eq!(
tick_timer_from_rate(16.0).expect("tick rate should be valid"),
Timer::try_new(62).expect("timer interval should be valid")
Timer::new(NonZeroU64::new(62).expect("non-zero"))
);
assert_eq!(
tick_timer_from_rate(1000.0).expect("tick rate should be valid"),
Timer::try_new(1).expect("timer interval should be valid")
Timer::new(NonZeroU64::new(1).expect("non-zero"))
);
}

Expand Down
10 changes: 6 additions & 4 deletions src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'a> Application for TearsApp<'a> {
// machine is to move the per-domain dispatch (`handle_timeline_msg`,
// `handle_editor_msg`, `handle_nostr_msg`, ...) into `AppState::update(AppMsg)`,
// leaving `TearsApp` as a thin tears adapter responsible only for IO-coupled
// concerns: key -> message mapping, subscriptions, and `Command::effect(Quit)`.
// concerns: key -> message mapping, subscriptions, and `Command::quit()`.
// That would make `AppState` own both state and transitions, and would let its
// fields become private (external code could only drive it via messages).
fn update(&mut self, msg: AppMsg) -> Command<Self::Message> {
Expand Down Expand Up @@ -187,7 +187,7 @@ impl<'a> TearsApp<'a> {
let _ = self.state.close_connection();

// Trigger the quit action
Command::effect(Action::Quit)
Command::quit()
}
SystemMsg::Resize(width, height) => {
log::debug!("Terminal resized to {width}x{height}");
Expand Down Expand Up @@ -433,6 +433,8 @@ impl<'a> TearsApp<'a> {

#[cfg(test)]
mod tests {
use std::num::NonZeroU64;

use super::*;
use crate::application::config::Config;
use crate::domain::nostr::FeedKind;
Expand All @@ -450,7 +452,7 @@ mod tests {
pubkey: keys.public_key(),
config,
nostr_client: client,
tick_timer: Timer::try_new(62).expect("test timer interval must be valid"),
tick_timer: Timer::new(NonZeroU64::new(62).expect("non-zero")),
};

let (app, _) = TearsApp::new(flags);
Expand Down Expand Up @@ -676,7 +678,7 @@ mod tests {
let cmd = app.update(quit_msg);
assert!(cmd.is_some());

// Command should be Action::Quit effect (we can't directly test this,
// Command should be a Command::quit() effect (we can't directly test this,
// but the system should have processed it)
// The test passes if no panic occurs
}
Expand Down
Loading