Skip to content

Commit

Permalink
fix archive index caching for source archives, don't ignore source fe…
Browse files Browse the repository at this point in the history
…tch errors
  • Loading branch information
syphar committed Nov 19, 2023
1 parent 62c620e commit d222d60
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 23 deletions.
50 changes: 45 additions & 5 deletions src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,18 +201,37 @@ impl AsyncStorage {
})
}

pub(crate) async fn source_file_exists(
&self,
name: &str,
version: &str,
latest_build_id: i32,
path: &str,
archive_storage: bool,
) -> Result<bool> {
Ok(if archive_storage {
self.exists_in_archive(&source_archive_path(name, version), latest_build_id, path)
.await?
} else {
// Add sources prefix, name and version to the path for accessing the file stored in the database
let remote_path = format!("sources/{name}/{version}/{path}");
self.exists(&remote_path).await?
})
}

#[context("fetching {path} from {name} {version} (archive: {archive_storage})")]
pub(crate) async fn fetch_source_file(
&self,
name: &str,
version: &str,
latest_build_id: i32,
path: &str,
archive_storage: bool,
) -> Result<Blob> {
Ok(if archive_storage {
self.get_from_archive(
&source_archive_path(name, version),
0, // we assume the source archive can't ever change for a release
latest_build_id,
path,
self.max_file_size_for(path),
None,
Expand Down Expand Up @@ -633,13 +652,34 @@ impl Storage {
&self,
name: &str,
version: &str,
latest_build_id: i32,
path: &str,
archive_storage: bool,
) -> Result<Blob> {
self.runtime.block_on(
self.inner
.fetch_source_file(name, version, path, archive_storage),
)
self.runtime.block_on(self.inner.fetch_source_file(
name,
version,
latest_build_id,
path,
archive_storage,
))
}

pub(crate) fn source_file_exists(
&self,
name: &str,
version: &str,
latest_build_id: i32,
path: &str,
archive_storage: bool,
) -> Result<bool> {
self.runtime.block_on(self.inner.source_file_exists(
name,
version,
latest_build_id,
path,
archive_storage,
))
}

pub(crate) fn rustdoc_file_exists(
Expand Down
9 changes: 8 additions & 1 deletion src/web/crate_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ impl CrateDetails {
.fetch_source_file(
&self.name,
&self.version,
self.latest_build_id.unwrap_or(0),
"Cargo.toml",
self.archive_storage,
)
Expand All @@ -282,7 +283,13 @@ impl CrateDetails {
};
for path in &paths {
match storage
.fetch_source_file(&self.name, &self.version, path, self.archive_storage)
.fetch_source_file(
&self.name,
&self.version,
self.latest_build_id.unwrap_or(0),
path,
self.archive_storage,
)
.await
{
Ok(readme) => {
Expand Down
64 changes: 47 additions & 17 deletions src/web/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
},
AsyncStorage,
};
use anyhow::Result;
use anyhow::{Context as _, Result};
use axum::{extract::Path, headers::HeaderMapExt, response::IntoResponse, Extension};

use postgres::Client;
Expand Down Expand Up @@ -222,34 +222,64 @@ pub(crate) async fn source_browser_handler(
}
};

let archive_storage = spawn_blocking({
let (archive_storage, latest_build_id): (bool, Option<i32>) = spawn_blocking({
// we probably could not use a SQL statement here, but rather a more selective
// version of `CrateDetails`, which doesn't fetch everything.
let pool = pool.clone();
let name = name.clone();
let version = version.clone();
move || {
let mut conn = pool.get()?;
Ok(conn
.query_one(
"SELECT archive_storage
FROM releases
INNER JOIN crates ON releases.crate_id = crates.id
WHERE
name = $1 AND
version = $2",
&[&name, &version],
)?
.get::<_, bool>(0))
let row = conn.query_one(
"SELECT
releases.archive_storage,
(
SELECT id
FROM builds
WHERE
builds.rid = releases.id AND
builds.build_status = TRUE
ORDER BY build_time DESC
LIMIT 1
) AS latest_build_id
FROM releases
INNER JOIN crates ON releases.crate_id = crates.id
WHERE
name = $1 AND
version = $2",
&[&name, &version],
)?;
Ok((row.get(0), row.get(1)))
}
})
.await?;

// try to get actual file first
// skip if request is a directory
let blob = if !path.ends_with('/') {
storage
.fetch_source_file(&name, &version, &path, archive_storage)
let blob = if !path.ends_with('/')
&& storage
.source_file_exists(
&name,
&version,
latest_build_id.unwrap_or(0),
&path,
archive_storage,
)
.await
.ok()
.context("error checking source file existence")?
{
Some(
storage
.fetch_source_file(
&name,
&version,
latest_build_id.unwrap_or(0),
&path,
archive_storage,
)
.await
.context("error fetching source file")?,
)
} else {
None
};
Expand Down

0 comments on commit d222d60

Please sign in to comment.