Skip to content

Commit b9b0fe9

Browse files
Nemo157syphar
authored andcommitted
Use redirects for semver status requests
1 parent a8c60fb commit b9b0fe9

File tree

3 files changed

+63
-17
lines changed

3 files changed

+63
-17
lines changed

src/test/mod.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,53 +149,65 @@ fn assert_redirect_common(
149149
}
150150

151151
/// Makes sure that a URL redirects to a specific page, but doesn't check that the target exists
152+
///
153+
/// Returns the redirect response
152154
#[context("expected redirect from {path} to {expected_target}")]
153155
pub(crate) fn assert_redirect_unchecked(
154156
path: &str,
155157
expected_target: &str,
156158
web: &TestFrontend,
157-
) -> Result<()> {
158-
assert_redirect_common(path, expected_target, web).map(|_| ())
159+
) -> Result<Response> {
160+
assert_redirect_common(path, expected_target, web)
159161
}
160162

161163
/// Makes sure that a URL redirects to a specific page, but doesn't check that the target exists
164+
///
165+
/// Returns the redirect response
162166
#[context("expected redirect from {path} to {expected_target}")]
163167
pub(crate) fn assert_redirect_cached_unchecked(
164168
path: &str,
165169
expected_target: &str,
166170
cache_policy: cache::CachePolicy,
167171
web: &TestFrontend,
168172
config: &Config,
169-
) -> Result<()> {
173+
) -> Result<Response> {
170174
let redirect_response = assert_redirect_common(path, expected_target, web)?;
171175
assert_cache_control(&redirect_response, cache_policy, config);
172-
Ok(())
176+
Ok(redirect_response)
173177
}
174178

175179
/// Make sure that a URL redirects to a specific page, and that the target exists and is not another redirect
180+
///
181+
/// Returns the redirect response
176182
#[context("expected redirect from {path} to {expected_target}")]
177-
pub(crate) fn assert_redirect(path: &str, expected_target: &str, web: &TestFrontend) -> Result<()> {
178-
assert_redirect_common(path, expected_target, web)?;
183+
pub(crate) fn assert_redirect(
184+
path: &str,
185+
expected_target: &str,
186+
web: &TestFrontend,
187+
) -> Result<Response> {
188+
let redirect_response = assert_redirect_common(path, expected_target, web)?;
179189

180190
let response = web.get_no_redirect(expected_target).send()?;
181191
let status = response.status();
182192
if !status.is_success() {
183193
anyhow::bail!("failed to GET {expected_target}: {status}");
184194
}
185195

186-
Ok(())
196+
Ok(redirect_response)
187197
}
188198

189199
/// Make sure that a URL redirects to a specific page, and that the target exists and is not another redirect.
190200
/// Also verifies that the redirect's cache-control header matches the provided cache policy.
201+
///
202+
/// Returns the redirect response
191203
#[context("expected redirect from {path} to {expected_target}")]
192204
pub(crate) fn assert_redirect_cached(
193205
path: &str,
194206
expected_target: &str,
195207
cache_policy: cache::CachePolicy,
196208
web: &TestFrontend,
197209
config: &Config,
198-
) -> Result<()> {
210+
) -> Result<Response> {
199211
let redirect_response = assert_redirect_common(path, expected_target, web)?;
200212
assert_cache_control(&redirect_response, cache_policy, config);
201213

@@ -205,7 +217,7 @@ pub(crate) fn assert_redirect_cached(
205217
anyhow::bail!("failed to GET {expected_target}: {status}");
206218
}
207219

208-
Ok(())
220+
Ok(redirect_response)
209221
}
210222

211223
pub(crate) struct TestEnvironment {

src/web/rustdoc.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2149,11 +2149,14 @@ mod test {
21492149
.archive_storage(archive_storage)
21502150
.rustdoc_file("tokio/time/index.html")
21512151
.create()?;
2152+
21522153
assert_redirect(
21532154
"/tokio/0.2.21/tokio/time",
21542155
"/tokio/0.2.21/tokio/time/index.html",
21552156
env.frontend(),
2156-
)
2157+
)?;
2158+
2159+
Ok(())
21572160
})
21582161
}
21592162

src/web/status.rs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::cache::CachePolicy;
22
use crate::{
33
db::Pool,
44
utils::spawn_blocking,
5-
web::{error::AxumResult, match_version_axum},
5+
web::{axum_redirect, error::AxumResult, match_version_axum, MatchSemver},
66
};
77
use axum::{
88
extract::{Extension, Path},
@@ -21,10 +21,18 @@ pub(crate) async fn status_handler(
2121
// We use an async block to emulate a try block so that we can apply the above CORS header
2222
// and cache policy to both successful and failed responses
2323
async move {
24-
let (version, id) = match_version_axum(&pool, &name, Some(&req_version))
24+
let (version, id) = match match_version_axum(&pool, &name, Some(&req_version))
2525
.await?
2626
.exact_name_only()?
27-
.into_parts();
27+
{
28+
MatchSemver::Exact((version, id)) | MatchSemver::Latest((version, id)) => {
29+
(version, id)
30+
}
31+
MatchSemver::Semver((version, _)) => {
32+
let redirect = axum_redirect(format!("/crate/{name}/{version}/status.json"))?;
33+
return Ok(redirect.into_response());
34+
}
35+
};
2836

2937
let rustdoc_status: bool = spawn_blocking({
3038
move || {
@@ -42,10 +50,12 @@ pub(crate) async fn status_handler(
4250
})
4351
.await?;
4452

45-
AxumResult::Ok(Json(serde_json::json!({
53+
let json = Json(serde_json::json!({
4654
"version": version,
4755
"doc_status": rustdoc_status,
48-
})))
56+
}));
57+
58+
AxumResult::Ok(json.into_response())
4959
}
5060
.await,
5161
)
@@ -54,7 +64,7 @@ pub(crate) async fn status_handler(
5464
#[cfg(test)]
5565
mod tests {
5666
use crate::{
57-
test::{assert_cache_control, wrapper},
67+
test::{assert_cache_control, assert_redirect, wrapper},
5868
web::cache::CachePolicy,
5969
};
6070
use reqwest::StatusCode;
@@ -63,6 +73,7 @@ mod tests {
6373
#[test_case("latest")]
6474
#[test_case("0.1")]
6575
#[test_case("0.1.0")]
76+
#[test_case("=0.1.0"; "exact_version")]
6677
fn status(version: &str) {
6778
wrapper(|env| {
6879
env.fake_release().name("foo").version("0.1.0").create()?;
@@ -88,9 +99,28 @@ mod tests {
8899
});
89100
}
90101

102+
#[test_case("0.1")]
103+
#[test_case("*")]
104+
fn redirect(version: &str) {
105+
wrapper(|env| {
106+
env.fake_release().name("foo").version("0.1.0").create()?;
107+
108+
let redirect = assert_redirect(
109+
&format!("/crate/foo/{version}/status.json"),
110+
"/crate/foo/0.1.0/status.json",
111+
env.frontend(),
112+
)?;
113+
assert_cache_control(&redirect, CachePolicy::NoStoreMustRevalidate, &env.config());
114+
assert_eq!(redirect.headers()["access-control-allow-origin"], "*");
115+
116+
Ok(())
117+
});
118+
}
119+
91120
#[test_case("latest")]
92121
#[test_case("0.1")]
93122
#[test_case("0.1.0")]
123+
#[test_case("=0.1.0"; "exact_version")]
94124
fn failure(version: &str) {
95125
wrapper(|env| {
96126
env.fake_release()
@@ -124,14 +154,15 @@ mod tests {
124154
#[test_case("bar", "0.1")]
125155
#[test_case("bar", "0.1.0")]
126156
// version not found
157+
#[test_case("foo", "=0.1.0"; "exact_version")]
127158
#[test_case("foo", "0.2")]
128159
#[test_case("foo", "0.2.0")]
129160
// invalid semver
130161
#[test_case("foo", "0,1")]
131162
#[test_case("foo", "0,1,0")]
132163
fn not_found(krate: &str, version: &str) {
133164
wrapper(|env| {
134-
env.fake_release().name("foo").version("0.1.0").create()?;
165+
env.fake_release().name("foo").version("0.1.1").create()?;
135166

136167
let response = env
137168
.frontend()

0 commit comments

Comments
 (0)