Skip to content

Commit

Permalink
clippy: Make functions that are not async not be async
Browse files Browse the repository at this point in the history
  • Loading branch information
Hocuri committed Dec 27, 2024
1 parent 70ac6a7 commit 0469519
Show file tree
Hide file tree
Showing 14 changed files with 84 additions and 142 deletions.
25 changes: 11 additions & 14 deletions deltachat-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3827,20 +3827,17 @@ pub unsafe extern "C" fn dc_msg_set_file_and_deduplicate(
let ffi_msg = &mut *msg;
let ctx = &*ffi_msg.context;

block_on(async move {
ffi_msg
.message
.set_file_and_deduplicate(
ctx,
&as_path(file),
to_opt_string_lossy(name).as_deref(),
to_opt_string_lossy(filemime).as_deref(),
)
.await
.context("failed to set file")
.log_err(&*ffi_msg.context)
.ok();
});
ffi_msg
.message
.set_file_and_deduplicate(
ctx,
&as_path(file),
to_opt_string_lossy(name).as_deref(),
to_opt_string_lossy(filemime).as_deref(),
)
.context("failed to set file")
.log_err(&*ffi_msg.context)
.ok();
}

#[no_mangle]
Expand Down
87 changes: 42 additions & 45 deletions src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,11 @@ impl<'a> BlobObject<'a> {
///
/// This is done in a in way which avoids race-conditions when multiple files are
/// concurrently created.
pub async fn create_and_deduplicate(
context: &'a Context,
src: &Path,
) -> Result<BlobObject<'a>> {
pub fn create_and_deduplicate(context: &'a Context, src: &Path) -> Result<BlobObject<'a>> {
// `create_and_deduplicate{_from_bytes}()` do blocking I/O, but can still be called
// from an async context thanks to `block_in_place()`.
// Tokio's "async" I/O functions are also just thin wrappers around the blocking I/O syscalls,
// so we are doing essentially the same here.
task::block_in_place(|| {
let blobdir = context.get_blobdir();
if !(src.starts_with(blobdir) || src.starts_with("$BLOBDIR/")) {
Expand Down Expand Up @@ -186,41 +187,39 @@ impl<'a> BlobObject<'a> {
/// the file will be renamed to a hash of the file data.
///
/// The `data` will be written into the file without race-conditions.
pub async fn create_and_deduplicate_from_bytes(
///
/// This function does blocking I/O, but it can still be called from an async context
/// because `block_in_place()` is used to leave the async runtime if necessary.
pub fn create_and_deduplicate_from_bytes(
context: &'a Context,
data: &[u8],
) -> Result<BlobObject<'a>> {
task::block_in_place(|| BlobObject::create_and_deduplicate_from_bytes_inner(context, data))
}
task::block_in_place(|| {
let blobdir = context.get_blobdir();
let blob = BlobObject::from_hash(blobdir, blake3::hash(data))?;
let new_path = blob.to_abs_path();

fn create_and_deduplicate_from_bytes_inner(
context: &'a Context,
data: &[u8],
) -> Result<BlobObject<'a>> {
let blobdir = context.get_blobdir();
let blob = BlobObject::from_hash(blobdir, blake3::hash(data))?;
let new_path = blob.to_abs_path();

// This call to `std::fs::write` is thread safe because all threads write the same data.
if std::fs::write(&new_path, data).is_err() {
if new_path.exists() {
// Looks like the file is read-only and exists already
// TODO: Maybe we should check if the file contents are the same,
// or at least if the length is the same, and overwrite if not.

// Set the file to be modified "now", so that it's not deleted during housekeeping
let f = std::fs::File::open(&new_path).context("File::open")?;
f.set_modified(SystemTime::now()).context("set_modified")?;
} else {
// Try to create the blob directory
std::fs::create_dir_all(blobdir).log_err(context).ok();
std::fs::write(&new_path, data).context("fs::write")?;
// This call to `std::fs::write` is thread safe because all threads write the same data.
if std::fs::write(&new_path, data).is_err() {
if new_path.exists() {
// Looks like the file is read-only and exists already
// TODO: Maybe we should check if the file contents are the same,
// or at least if the length is the same, and overwrite if not.

// Set the file to be modified "now", so that it's not deleted during housekeeping
let f = std::fs::File::open(&new_path).context("File::open")?;
f.set_modified(SystemTime::now()).context("set_modified")?;
} else {
// Try to create the blob directory
std::fs::create_dir_all(blobdir).log_err(context).ok();
std::fs::write(&new_path, data).context("fs::write")?;
}
}
}

set_readonly(&new_path).log_err(context).ok();
context.emit_event(EventType::NewBlobFile(blob.as_name().to_string()));
Ok(blob)
set_readonly(&new_path).log_err(context).ok();
context.emit_event(EventType::NewBlobFile(blob.as_name().to_string()));
Ok(blob)
})
}

fn from_hash(blobdir: &Path, hash: blake3::Hash) -> Result<BlobObject<'_>> {
Expand Down Expand Up @@ -668,7 +667,7 @@ impl<'a> BlobObject<'a> {
encode_img(&img, ofmt, &mut encoded)?;
}

self.name = BlobObject::create_and_deduplicate_from_bytes_inner(context, &encoded)
self.name = BlobObject::create_and_deduplicate_from_bytes(context, &encoded)
.context("failed to write recoded blob to file")?
.name;
}
Expand Down Expand Up @@ -1504,8 +1503,7 @@ mod tests {
}

let mut msg = Message::new(viewtype);
msg.set_file_and_deduplicate(&alice, &file, Some(&file_name), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some(&file_name), None)?;
let chat = alice.create_chat(&bob).await;
if set_draft {
chat.id.set_draft(&alice, Some(&mut msg)).await.unwrap();
Expand Down Expand Up @@ -1563,8 +1561,7 @@ mod tests {
.await
.context("failed to write file")?;
let mut msg = Message::new(Viewtype::Image);
msg.set_file_and_deduplicate(&alice, &file, Some("file.gif"), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some("file.gif"), None)?;
let chat = alice.create_chat(&bob).await;
let sent = alice.send_msg(chat.id, &mut msg).await;
let bob_msg = bob.recv_msg(&sent).await;
Expand All @@ -1588,7 +1585,7 @@ mod tests {

let path = t.get_blobdir().join("anyfile.dat");
fs::write(&path, b"bla").await?;
let blob = BlobObject::create_and_deduplicate(&t, &path).await?;
let blob = BlobObject::create_and_deduplicate(&t, &path)?;
assert_eq!(blob.name, "$BLOBDIR/ce940175885d7b78f7b7e9f1396611f");
assert_eq!(path.exists(), false);

Expand All @@ -1599,19 +1596,19 @@ mod tests {
assert_eq!(fs::read(&blob.to_abs_path()).await?, b"bla");

fs::write(&path, b"bla").await?;
let blob2 = BlobObject::create_and_deduplicate(&t, &path).await?;
let blob2 = BlobObject::create_and_deduplicate(&t, &path)?;
assert_eq!(blob2.name, blob.name);

let path_outside_blobdir = t.dir.path().join("anyfile.dat");
fs::write(&path_outside_blobdir, b"bla").await?;
let blob_res = BlobObject::create_and_deduplicate(&t, &path_outside_blobdir).await;
let blob_res = BlobObject::create_and_deduplicate(&t, &path_outside_blobdir);
assert!(
blob_res.is_err(),
"Files outside the blobdir should not be allowed in create_and_deduplicate()"
);

fs::write(&path, b"blabla").await?;
let blob3 = BlobObject::create_and_deduplicate(&t, &path).await?;
let blob3 = BlobObject::create_and_deduplicate(&t, &path)?;
assert_ne!(blob3.name, blob.name);

Ok(())
Expand All @@ -1622,7 +1619,7 @@ mod tests {
let t = TestContext::new().await;

fs::remove_dir(t.get_blobdir()).await?;
let blob = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla").await?;
let blob = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla")?;
assert_eq!(blob.name, "$BLOBDIR/ce940175885d7b78f7b7e9f1396611f");

// The file should be read-only:
Expand All @@ -1638,7 +1635,7 @@ mod tests {
fs::write(&temp_file, b"temporary data").await?;
SystemTime::shift(Duration::from_secs(65 * 60));

let blob2 = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla").await?;
let blob2 = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla")?;
assert_eq!(blob2.name, blob.name);

// The modification time of the file should be updated
Expand All @@ -1649,7 +1646,7 @@ mod tests {
assert!(blob2.to_abs_path().exists());
assert_eq!(temp_file.exists(), false);

let blob3 = BlobObject::create_and_deduplicate_from_bytes(&t, b"blabla").await?;
let blob3 = BlobObject::create_and_deduplicate_from_bytes(&t, b"blabla")?;
assert_ne!(blob3.name, blob.name);

Ok(())
Expand Down
18 changes: 5 additions & 13 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6465,8 +6465,7 @@ mod tests {
tokio::fs::write(&file, bytes).await?;

let mut msg = Message::new(Viewtype::Sticker);
msg.set_file_and_deduplicate(&alice, &file, Some(filename), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some(filename), None)?;

let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
let mime = sent_msg.payload();
Expand Down Expand Up @@ -6531,7 +6530,6 @@ mod tests {
// Images without force_sticker should be turned into [Viewtype::Image]
let mut msg = Message::new(Viewtype::Sticker);
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
.await
.unwrap();
let file = msg.get_file(&alice).unwrap();
let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
Expand All @@ -6541,7 +6539,6 @@ mod tests {
// Images with `force_sticker = true` should keep [Viewtype::Sticker]
let mut msg = Message::new(Viewtype::Sticker);
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
.await
.unwrap();
msg.force_sticker();
let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
Expand All @@ -6552,7 +6549,6 @@ mod tests {
// even on drafted messages
let mut msg = Message::new(Viewtype::Sticker);
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
.await
.unwrap();
msg.force_sticker();
alice_chat
Expand Down Expand Up @@ -6594,8 +6590,7 @@ mod tests {
let file = alice.get_blobdir().join(file_name);
tokio::fs::write(&file, bytes).await?;
let mut msg = Message::new(Viewtype::Sticker);
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)?;

// send sticker to bob
let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await;
Expand Down Expand Up @@ -7190,8 +7185,7 @@ mod tests {
let file = t.get_blobdir().join(name);
tokio::fs::write(&file, bytes).await?;
let mut msg = Message::new(msg_type);
msg.set_file_and_deduplicate(t, &file, Some(name), None)
.await?;
msg.set_file_and_deduplicate(t, &file, Some(name), None)?;
send_msg(t, chat_id, &mut msg).await
}

Expand Down Expand Up @@ -7345,8 +7339,7 @@ mod tests {
let file = alice.get_blobdir().join("harmless_file.\u{202e}txt.exe");
fs::write(&file, "aaa").await?;
let mut msg = Message::new(Viewtype::File);
msg.set_file_and_deduplicate(&alice, &file, Some("harmless_file.\u{202e}txt.exe"), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some("harmless_file.\u{202e}txt.exe"), None)?;
let msg = bob.recv_msg(&alice.send_msg(chat_id, &mut msg).await).await;

// the file bob receives should not contain BIDI-control characters
Expand Down Expand Up @@ -7685,8 +7678,7 @@ mod tests {
let file = alice.get_blobdir().join("screenshot.png");
tokio::fs::write(&file, bytes).await?;
let mut msg = Message::new(Viewtype::Image);
msg.set_file_and_deduplicate(&alice, &file, Some("screenshot.png"), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some("screenshot.png"), None)?;

let alice_chat = alice.create_chat(&bob).await;
let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await;
Expand Down
4 changes: 1 addition & 3 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,7 @@ mod tests {
let file = alice.get_blobdir().join("minimal.xdc");
tokio::fs::write(&file, include_bytes!("../test-data/webxdc/minimal.xdc")).await?;
let mut instance = Message::new(Viewtype::File);
instance
.set_file_and_deduplicate(&alice, &file, None, None)
.await?;
instance.set_file_and_deduplicate(&alice, &file, None, None)?;
let _sent1 = alice.send_msg(chat_id, &mut instance).await;

alice
Expand Down
1 change: 0 additions & 1 deletion src/imex/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ mod tests {
fs::write(&file, "i am attachment").await.unwrap();
let mut msg = Message::new(Viewtype::File);
msg.set_file_and_deduplicate(&ctx0, &file, Some("hello.txt"), Some("text/plain"))
.await
.unwrap();
send_msg(&ctx0, self_chat.id, &mut msg).await.unwrap();

Expand Down
3 changes: 1 addition & 2 deletions src/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1077,8 +1077,7 @@ Content-Disposition: attachment; filename="location.kml"
let file = alice.get_blobdir().join(file_name);
tokio::fs::write(&file, bytes).await?;
let mut msg = Message::new(Viewtype::Image);
msg.set_file_and_deduplicate(&alice, &file, Some("logo.png"), None)
.await?;
msg.set_file_and_deduplicate(&alice, &file, Some("logo.png"), None)?;
let sent = alice.send_msg(alice_chat.id, &mut msg).await;
let alice_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?;
assert_eq!(alice_msg.has_location(), false);
Expand Down
15 changes: 6 additions & 9 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1097,14 +1097,14 @@ impl Message {
/// In order to deduplicate files that contain the same data,
/// the file will be renamed to a hash of the file data.
/// The file must not be modified after this function was called.
pub async fn set_file_and_deduplicate(
pub fn set_file_and_deduplicate(
&mut self,
context: &Context,
file: &Path,
name: Option<&str>,
filemime: Option<&str>,
) -> Result<()> {
let blob = BlobObject::create_and_deduplicate(context, file).await?;
let blob = BlobObject::create_and_deduplicate(context, file)?;
if let Some(name) = name {
self.param.set(Param::Filename, name);
} else {
Expand All @@ -1122,14 +1122,14 @@ impl Message {
/// In order to deduplicate files that contain the same data,
/// the filename will be a hash of the file data.
/// The file must not be modified after this function was called.
pub async fn set_file_from_bytes(
pub fn set_file_from_bytes(
&mut self,
context: &Context,
name: &str,
data: &[u8],
filemime: Option<&str>,
) -> Result<()> {
let blob = BlobObject::create_and_deduplicate_from_bytes(context, data).await?;
let blob = BlobObject::create_and_deduplicate_from_bytes(context, data)?;
self.param.set(Param::Filename, name);
self.param.set(Param::File, blob.as_name());
self.param.set_optional(Param::MimeType, filemime);
Expand All @@ -1146,7 +1146,6 @@ impl Message {
);
let vcard = contact::make_vcard(context, contacts).await?;
self.set_file_from_bytes(context, "vcard.vcf", vcard.as_bytes(), None)
.await
}

/// Updates message state from the vCard attachment.
Expand Down Expand Up @@ -2601,8 +2600,7 @@ mod tests {

let file_bytes = include_bytes!("../test-data/image/screenshot.png");
let mut msg = Message::new(Viewtype::Image);
msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)
.await?;
msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)?;
let sent_msg = bob.send_msg(bob_chat_id, &mut msg).await;
let msg = alice.recv_msg(&sent_msg).await;
assert_eq!(msg.download_state, DownloadState::Available);
Expand Down Expand Up @@ -2671,8 +2669,7 @@ mod tests {

let file_bytes = include_bytes!("../test-data/image/screenshot.png");
let mut msg = Message::new(Viewtype::Image);
msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)
.await?;
msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)?;
let sent_msg = bob.send_msg(bob_chat_id, &mut msg).await;
let msg = alice.recv_msg(&sent_msg).await;
assert_eq!(msg.download_state, DownloadState::Available);
Expand Down
3 changes: 1 addition & 2 deletions src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2530,8 +2530,7 @@ mod tests {
// Long messages are truncated and MimeMessage::decoded_data is set for them. We need
// decoded_data to check presence of the necessary headers.
msg.set_text("a".repeat(constants::DC_DESIRED_TEXT_LEN + 1));
msg.set_file_from_bytes(&bob, "foo.bar", "content".as_bytes(), None)
.await?;
msg.set_file_from_bytes(&bob, "foo.bar", "content".as_bytes(), None)?;
let sent = bob.send_msg(chat, &mut msg).await;
assert!(msg.get_showpadlock());
assert!(sent.payload.contains("\r\nSubject: [...]\r\n"));
Expand Down
3 changes: 1 addition & 2 deletions src/mimeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,8 +1360,7 @@ impl MimeMessage {
/* we have a regular file attachment,
write decoded data to new blob object */

let blob = match BlobObject::create_and_deduplicate_from_bytes(context, decoded_data).await
{
let blob = match BlobObject::create_and_deduplicate_from_bytes(context, decoded_data) {
Ok(blob) => blob,
Err(err) => {
error!(
Expand Down
Loading

0 comments on commit 0469519

Please sign in to comment.