Skip to content

Commit 1a0a3d0

Browse files
authored
admin/render_readmes: Migrate render_pkg_readme() and find_file_by_path() fns to async/await (#10070)
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 788e376 commit 1a0a3d0

File tree

3 files changed

+67
-51
lines changed

3 files changed

+67
-51
lines changed

Cargo.lock

Lines changed: 2 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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ doctest = true
4242

4343
[dependencies]
4444
anyhow = "=1.0.93"
45+
async-compression = { version = "=0.4.18", default-features = false, features = ["gzip", "tokio"] }
4546
async-trait = "=0.1.83"
4647
aws-credential-types = { version = "=1.2.1", features = ["hardcoded-credentials"] }
4748
aws-ip-ranges = "=0.927.0"
@@ -87,7 +88,7 @@ indexmap = { version = "=2.6.0", features = ["serde"] }
8788
indicatif = "=0.17.9"
8889
ipnetwork = "=0.20.0"
8990
json-subscriber = "=0.2.3"
90-
tikv-jemallocator = { version = "=0.6.0", features = ['unprefixed_malloc_on_supported_platforms', 'profiling'] }
91+
krata-tokio-tar = "=0.4.2"
9192
lettre = { version = "=0.11.10", default-features = false, features = ["file-transport", "smtp-transport", "hostname", "builder", "tokio1", "tokio1-native-tls"] }
9293
minijinja = "=2.5.0"
9394
mockall = "=0.13.1"
@@ -112,6 +113,7 @@ spdx = "=0.10.7"
112113
tar = "=0.4.43"
113114
tempfile = "=3.14.0"
114115
thiserror = "=2.0.3"
116+
tikv-jemallocator = { version = "=0.6.0", features = ['unprefixed_malloc_on_supported_platforms', 'profiling'] }
115117
tokio = { version = "=1.41.1", features = ["net", "signal", "io-std", "io-util", "rt-multi-thread", "macros", "process"]}
116118
tokio-postgres = "=0.7.12"
117119
tokio-util = "=0.7.12"

src/bin/crates-admin/render_readmes.rs

Lines changed: 62 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@ 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::io::StreamReader;
912

13+
use async_compression::tokio::bufread::GzipDecoder;
1014
use chrono::{NaiveDateTime, Utc};
1115
use crates_io::storage::Storage;
1216
use crates_io::tasks::spawn_blocking;
@@ -15,10 +19,9 @@ use crates_io_tarball::{Manifest, StringOrBool};
1519
use diesel::prelude::*;
1620
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;
1721
use diesel_async::{AsyncPgConnection, RunQueryDsl};
18-
use flate2::read::GzDecoder;
1922
use reqwest::{header, Client};
2023
use std::str::FromStr;
21-
use tar::{self, Archive};
24+
use tokio_tar::{self, Archive};
2225

2326
const USER_AGENT: &str = "crates-admin";
2427

@@ -168,22 +171,25 @@ async fn get_readme(
168171
));
169172
}
170173

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?
174+
let reader = response
175+
.bytes_stream()
176+
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e));
177+
let reader = StreamReader::new(reader);
178+
let reader = GzipDecoder::new(reader);
179+
let archive = Archive::new(reader);
180+
render_pkg_readme(archive, &pkg_name).await
179181
}
180182

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

184189
let manifest: Manifest = {
185190
let path = format!("{pkg_name}/Cargo.toml");
186191
let contents = find_file_by_path(&mut entries, Path::new(&path))
192+
.await
187193
.context("Failed to read Cargo.toml file")?;
188194

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

208214
let path = Path::new(pkg_name).join(&readme_path);
209215
let contents = find_file_by_path(&mut entries, Path::new(&path))
216+
.await
210217
.with_context(|| format!("Failed to read {} file", readme_path.display()))?;
211218

212219
// pkg_path_in_vcs Unsupported from admin::render_readmes. See #4095
213220
// Would need access to cargo_vcs_info
214221
let pkg_path_in_vcs = None;
215222

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)
223+
spawn_blocking(move || {
224+
let repository = manifest
225+
.package
226+
.as_ref()
227+
.and_then(|p| p.repository.as_ref())
228+
.and_then(|r| r.as_ref().as_local())
229+
.map(|s| s.as_str());
230+
text_to_html(&contents, &readme_path, repository, pkg_path_in_vcs)
231+
})
232+
.await?
224233
};
225234
Ok(rendered)
226235
}
227236

228237
/// Search an entry by its path in a Tar archive.
229-
fn find_file_by_path<R: Read>(
230-
entries: &mut tar::Entries<'_, R>,
238+
async fn find_file_by_path<R: AsyncRead + Unpin>(
239+
entries: &mut tokio_tar::Entries<R>,
231240
path: &Path,
232241
) -> anyhow::Result<String> {
233242
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-
})
243+
.filter_map(|entry| future::ready(entry.ok()))
244+
.filter(|entry| future::ready(entry.path().is_ok_and(|p| p == path)))
245+
.next()
246+
.await
239247
.ok_or_else(|| anyhow!("Failed to find tarball entry: {}", path.display()))?;
240248

241249
let mut contents = String::new();
242250
file.read_to_string(&mut contents)
251+
.await
243252
.context("Failed to read file contents")?;
244253

245254
Ok(contents)
@@ -252,8 +261,8 @@ pub mod tests {
252261

253262
use super::render_pkg_readme;
254263

255-
#[test]
256-
fn test_render_pkg_readme() {
264+
#[tokio::test]
265+
async fn test_render_pkg_readme() {
257266
let serialized_archive = TarballBuilder::new()
258267
.add_file(
259268
"foo-0.0.1/Cargo.toml",
@@ -267,13 +276,14 @@ readme = "README.md"
267276
.add_file("foo-0.0.1/README.md", b"readme")
268277
.build_unzipped();
269278

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

275-
#[test]
276-
fn test_render_pkg_no_readme() {
285+
#[tokio::test]
286+
async fn test_render_pkg_no_readme() {
277287
let serialized_archive = TarballBuilder::new()
278288
.add_file(
279289
"foo-0.0.1/Cargo.toml",
@@ -283,14 +293,13 @@ readme = "README.md"
283293
)
284294
.build_unzipped();
285295

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

292-
#[test]
293-
fn test_render_pkg_implicit_readme() {
301+
#[tokio::test]
302+
async fn test_render_pkg_implicit_readme() {
294303
let serialized_archive = TarballBuilder::new()
295304
.add_file(
296305
"foo-0.0.1/Cargo.toml",
@@ -303,13 +312,14 @@ version = "0.0.1"
303312
.add_file("foo-0.0.1/README.md", b"readme")
304313
.build_unzipped();
305314

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

311-
#[test]
312-
fn test_render_pkg_readme_w_link() {
321+
#[tokio::test]
322+
async fn test_render_pkg_readme_w_link() {
313323
let serialized_archive = TarballBuilder::new()
314324
.add_file(
315325
"foo-0.0.1/Cargo.toml",
@@ -324,13 +334,14 @@ repository = "https://github.com/foo/foo"
324334
.add_file("foo-0.0.1/README.md", b"readme [link](./Other.md)")
325335
.build_unzipped();
326336

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

332-
#[test]
333-
fn test_render_pkg_readme_not_at_root() {
343+
#[tokio::test]
344+
async fn test_render_pkg_readme_not_at_root() {
334345
let serialized_archive = TarballBuilder::new()
335346
.add_file(
336347
"foo-0.0.1/Cargo.toml",
@@ -348,8 +359,9 @@ repository = "https://github.com/foo/foo"
348359
)
349360
.build_unzipped();
350361

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

0 commit comments

Comments
 (0)