Skip to content

Commit f063b44

Browse files
committed
admin/render_readmes: Migrate render_pkg_readme() and find_file_by_path() fns to async/await
It would be great if we could just remove the `spawn_blocking()` call, but the `text_to_html()` call could be blocking in some cases.
1 parent 3bfe1cf commit f063b44

File tree

3 files changed

+69
-51
lines changed

3 files changed

+69
-51
lines changed

Cargo.lock

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ tracing-subscriber = { version = "=0.3.18", features = ["env-filter", "json"] }
122122
typomania = { version = "=0.1.2", default-features = false }
123123
url = "=2.5.4"
124124
unicode-xid = "=0.2.6"
125+
async-compression = { version = "=0.4.18", default-features = false, features = ["gzip", "tokio"] }
126+
krata-tokio-tar = "=0.4.2"
127+
tokio-util = { version = "=0.7.12", features = ["compat"] }
125128

126129
[dev-dependencies]
127130
bytes = "=1.8.0"

src/bin/crates-admin/render_readmes.rs

Lines changed: 62 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,23 @@ use crates_io::{
44
models::Version,
55
schema::{crates, readme_renderings, versions},
66
};
7-
use std::path::PathBuf;
8-
use std::{io::Read, path::Path, sync::Arc};
7+
use futures_util::{StreamExt, TryStreamExt};
8+
use std::path::{Path, PathBuf};
9+
use std::{future, sync::Arc};
10+
use tokio::io::{AsyncRead, AsyncReadExt};
11+
use tokio_util::compat::FuturesAsyncReadCompatExt;
912

13+
use async_compression::tokio::bufread::GzipDecoder;
1014
use chrono::{NaiveDateTime, Utc};
1115
use crates_io::storage::Storage;
12-
use crates_io::tasks::spawn_blocking;
1316
use crates_io_markdown::text_to_html;
1417
use crates_io_tarball::{Manifest, StringOrBool};
1518
use diesel::prelude::*;
1619
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;
1720
use diesel_async::{AsyncPgConnection, RunQueryDsl};
18-
use flate2::read::GzDecoder;
1921
use reqwest::{header, Client};
2022
use std::str::FromStr;
21-
use tar::{self, Archive};
23+
use tokio_tar::{self, Archive};
2224

2325
const USER_AGENT: &str = "crates-admin";
2426

@@ -168,22 +170,25 @@ async fn get_readme(
168170
));
169171
}
170172

171-
let body = response.bytes().await?;
172-
173-
spawn_blocking(move || {
174-
let reader = GzDecoder::new(&*body);
175-
let archive = Archive::new(reader);
176-
render_pkg_readme(archive, &pkg_name)
177-
})
178-
.await?
173+
let reader = response
174+
.bytes_stream()
175+
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))
176+
.into_async_read();
177+
let reader = GzipDecoder::new(reader.compat());
178+
let archive = Archive::new(reader);
179+
render_pkg_readme(archive, &pkg_name).await
179180
}
180181

181-
fn render_pkg_readme<R: Read>(mut archive: Archive<R>, pkg_name: &str) -> anyhow::Result<String> {
182+
async fn render_pkg_readme<R: AsyncRead + Unpin>(
183+
mut archive: Archive<R>,
184+
pkg_name: &str,
185+
) -> anyhow::Result<String> {
182186
let mut entries = archive.entries().context("Invalid tar archive entries")?;
183187

184188
let manifest: Manifest = {
185189
let path = format!("{pkg_name}/Cargo.toml");
186190
let contents = find_file_by_path(&mut entries, Path::new(&path))
191+
.await
187192
.context("Failed to read Cargo.toml file")?;
188193

189194
Manifest::from_str(&contents).context("Failed to parse manifest file")?
@@ -207,39 +212,42 @@ fn render_pkg_readme<R: Read>(mut archive: Archive<R>, pkg_name: &str) -> anyhow
207212

208213
let path = Path::new(pkg_name).join(&readme_path);
209214
let contents = find_file_by_path(&mut entries, Path::new(&path))
215+
.await
210216
.with_context(|| format!("Failed to read {} file", readme_path.display()))?;
211217

212218
// pkg_path_in_vcs Unsupported from admin::render_readmes. See #4095
213219
// Would need access to cargo_vcs_info
214220
let pkg_path_in_vcs = None;
215221

216-
let repository = manifest
217-
.package
218-
.as_ref()
219-
.and_then(|p| p.repository.as_ref())
220-
.and_then(|r| r.as_ref().as_local())
221-
.map(|s| s.as_str());
222-
223-
text_to_html(&contents, &readme_path, repository, pkg_path_in_vcs)
222+
spawn_blocking(move || {
223+
let repository = manifest
224+
.package
225+
.as_ref()
226+
.and_then(|p| p.repository.as_ref())
227+
.and_then(|r| r.as_ref().as_local())
228+
.map(|s| s.as_str());
229+
text_to_html(&contents, &readme_path, repository, pkg_path_in_vcs)
230+
})
231+
.await?
224232
};
225233
Ok(rendered)
226234
}
227235

228236
/// Search an entry by its path in a Tar archive.
229-
fn find_file_by_path<R: Read>(
230-
entries: &mut tar::Entries<'_, R>,
237+
async fn find_file_by_path<R: AsyncRead + Unpin>(
238+
entries: &mut tokio_tar::Entries<R>,
231239
path: &Path,
232240
) -> anyhow::Result<String> {
233241
let mut file = entries
234-
.filter_map(|entry| entry.ok())
235-
.find(|file| match file.path() {
236-
Ok(p) => p == path,
237-
Err(_) => false,
238-
})
242+
.filter_map(|entry| future::ready(entry.ok()))
243+
.filter(|entry| future::ready(entry.path().is_ok_and(|p| p == path)))
244+
.next()
245+
.await
239246
.ok_or_else(|| anyhow!("Failed to find tarball entry: {}", path.display()))?;
240247

241248
let mut contents = String::new();
242249
file.read_to_string(&mut contents)
250+
.await
243251
.context("Failed to read file contents")?;
244252

245253
Ok(contents)
@@ -252,8 +260,8 @@ pub mod tests {
252260

253261
use super::render_pkg_readme;
254262

255-
#[test]
256-
fn test_render_pkg_readme() {
263+
#[tokio::test]
264+
async fn test_render_pkg_readme() {
257265
let serialized_archive = TarballBuilder::new()
258266
.add_file(
259267
"foo-0.0.1/Cargo.toml",
@@ -267,13 +275,14 @@ readme = "README.md"
267275
.add_file("foo-0.0.1/README.md", b"readme")
268276
.build_unzipped();
269277

270-
let result =
271-
render_pkg_readme(tar::Archive::new(&*serialized_archive), "foo-0.0.1").unwrap();
278+
let result = render_pkg_readme(tokio_tar::Archive::new(&*serialized_archive), "foo-0.0.1")
279+
.await
280+
.unwrap();
272281
assert!(result.contains("readme"))
273282
}
274283

275-
#[test]
276-
fn test_render_pkg_no_readme() {
284+
#[tokio::test]
285+
async fn test_render_pkg_no_readme() {
277286
let serialized_archive = TarballBuilder::new()
278287
.add_file(
279288
"foo-0.0.1/Cargo.toml",
@@ -283,14 +292,13 @@ readme = "README.md"
283292
)
284293
.build_unzipped();
285294

286-
assert_err!(render_pkg_readme(
287-
tar::Archive::new(&*serialized_archive),
288-
"foo-0.0.1"
289-
));
295+
assert_err!(
296+
render_pkg_readme(tokio_tar::Archive::new(&*serialized_archive), "foo-0.0.1").await
297+
);
290298
}
291299

292-
#[test]
293-
fn test_render_pkg_implicit_readme() {
300+
#[tokio::test]
301+
async fn test_render_pkg_implicit_readme() {
294302
let serialized_archive = TarballBuilder::new()
295303
.add_file(
296304
"foo-0.0.1/Cargo.toml",
@@ -303,13 +311,14 @@ version = "0.0.1"
303311
.add_file("foo-0.0.1/README.md", b"readme")
304312
.build_unzipped();
305313

306-
let result =
307-
render_pkg_readme(tar::Archive::new(&*serialized_archive), "foo-0.0.1").unwrap();
314+
let result = render_pkg_readme(tokio_tar::Archive::new(&*serialized_archive), "foo-0.0.1")
315+
.await
316+
.unwrap();
308317
assert!(result.contains("readme"))
309318
}
310319

311-
#[test]
312-
fn test_render_pkg_readme_w_link() {
320+
#[tokio::test]
321+
async fn test_render_pkg_readme_w_link() {
313322
let serialized_archive = TarballBuilder::new()
314323
.add_file(
315324
"foo-0.0.1/Cargo.toml",
@@ -324,13 +333,14 @@ repository = "https://github.com/foo/foo"
324333
.add_file("foo-0.0.1/README.md", b"readme [link](./Other.md)")
325334
.build_unzipped();
326335

327-
let result =
328-
render_pkg_readme(tar::Archive::new(&*serialized_archive), "foo-0.0.1").unwrap();
336+
let result = render_pkg_readme(tokio_tar::Archive::new(&*serialized_archive), "foo-0.0.1")
337+
.await
338+
.unwrap();
329339
assert!(result.contains("\"https://github.com/foo/foo/blob/HEAD/./Other.md\""))
330340
}
331341

332-
#[test]
333-
fn test_render_pkg_readme_not_at_root() {
342+
#[tokio::test]
343+
async fn test_render_pkg_readme_not_at_root() {
334344
let serialized_archive = TarballBuilder::new()
335345
.add_file(
336346
"foo-0.0.1/Cargo.toml",
@@ -348,8 +358,9 @@ repository = "https://github.com/foo/foo"
348358
)
349359
.build_unzipped();
350360

351-
let result =
352-
render_pkg_readme(tar::Archive::new(&*serialized_archive), "foo-0.0.1").unwrap();
361+
let result = render_pkg_readme(tokio_tar::Archive::new(&*serialized_archive), "foo-0.0.1")
362+
.await
363+
.unwrap();
353364
assert!(result.contains("docs/readme"));
354365
assert!(result.contains("\"https://github.com/foo/foo/blob/HEAD/docs/./Other.md\""))
355366
}

0 commit comments

Comments
 (0)