Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ratelimit): Return Duration from Ratelimit functions #6283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(ratelimit): Return Duration from Ratelimit functions #6283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion deltachat-jsonrpc/src/api/types/webxdc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use std::time::Duration;

use anyhow::Context as _;
use deltachat::{
context::Context,
message::{Message, MsgId},
Expand Down Expand Up @@ -72,7 +75,11 @@ impl WebxdcMessageInfo {
source_code_url: maybe_empty_string_to_option(source_code_url),
internet_access,
self_addr,
send_update_interval,
send_update_interval: send_update_interval
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the future i suggest to add the _ms suffix, otherwise one can think it's in seconds. Moreover we have other intervals in API which are indeed in seconds

.checked_add(Duration::from_micros(999))
.context("Overflow occurred")?
.as_millis()
.try_into()?,
send_update_max_size,
})
}
Expand Down
8 changes: 4 additions & 4 deletions deltachat-ratelimit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ impl Ratelimit {
self.until_can_send_at(SystemTime::now())
}

/// Returns minimum possible update interval in milliseconds.
pub fn update_interval(&self) -> usize {
(self.window.as_millis() as f64 / self.quota) as usize
/// Returns the minimum possible sending interval.
pub fn min_send_interval(&self) -> Duration {
self.window.div_f64(self.quota)
}
}

Expand All @@ -107,7 +107,7 @@ mod tests {

let mut ratelimit = Ratelimit::new_at(Duration::new(60, 0), 3.0, now);
assert!(ratelimit.can_send_at(now));
assert_eq!(ratelimit.update_interval(), 20_000);
assert_eq!(ratelimit.min_send_interval(), Duration::new(20, 0));

// Send burst of 3 messages.
ratelimit.send_at(now);
Expand Down
9 changes: 5 additions & 4 deletions src/webxdc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod maps_integration;
use std::cmp::max;
use std::collections::HashMap;
use std::path::Path;
use std::time::Duration;

use anyhow::{anyhow, bail, ensure, format_err, Context as _, Result};

Expand Down Expand Up @@ -111,9 +112,9 @@ pub struct WebxdcInfo {
/// Address to be used for `window.webxdc.selfAddr` in JS land.
pub self_addr: String,

/// Milliseconds to wait before calling `sendUpdate()` again since the last call.
/// Time to wait before calling `sendUpdate()` again since the last call.
/// Should be exposed to `window.sendUpdateInterval` in JS land.
pub send_update_interval: usize,
pub send_update_interval: Duration,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope i don't break things here? This is Serialize, but it seems that the true public API is the JSON-RPC one

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best way to find out is adding a JSON-RPC test, I am also not sure.

Copy link
Collaborator Author

@iequidoo iequidoo Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already test_webxdc(), but it tests WebxdcMessageInfo defined in deltachat-jsonrpc/src/api/types/webxdc.rs, not this struct.

Still, this WebxdcInfo is public, so maybe there are some other implementations or core bindings using it, that's why i asked.

EDIT: Webxdc manifest files doesn't contain send_update_interval, so this doesn't break parsing them at least.


/// Maximum number of bytes accepted for a serialized update object.
/// Should be exposed to `window.sendUpdateMaxSize` in JS land.
Expand Down Expand Up @@ -961,7 +962,7 @@ impl Message {
request_integration,
internet_access,
self_addr,
send_update_interval: context.ratelimit.read().await.update_interval(),
send_update_interval: context.ratelimit.read().await.min_send_interval(),
send_update_max_size: RECOMMENDED_FILE_SIZE as usize,
})
}
Expand Down Expand Up @@ -2262,7 +2263,7 @@ sth_for_the = "future""#
let info = instance.get_webxdc_info(&t).await?;
assert_eq!(info.name, "minimal.xdc");
assert_eq!(info.icon, WEBXDC_DEFAULT_ICON.to_string());
assert_eq!(info.send_update_interval, 10000);
assert_eq!(info.send_update_interval, Duration::new(10, 0));
assert_eq!(info.send_update_max_size, RECOMMENDED_FILE_SIZE as usize);

let mut instance = create_webxdc_instance(
Expand Down
Loading