Skip to content

Commit c23c8c1

Browse files
committed
Deal with TopVersions everywhere
This contains far too much repeated code. It does basically work and the relevant test passes! I'll refactor the redundant code into a shared function.
1 parent 1058fc2 commit c23c8c1

File tree

5 files changed

+60
-16
lines changed

5 files changed

+60
-16
lines changed

src/controllers/krate/metadata.rs

+24-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
//! index or cached metadata which was extracted (client side) from the
55
//! `Cargo.toml` file.
66
7+
use chrono::NaiveDateTime;
8+
79
use crate::controllers::prelude::*;
810
use crate::models::{
911
Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
@@ -31,10 +33,29 @@ pub fn summary(req: &mut dyn Request) -> AppResult<Response> {
3133
versions
3234
.grouped_by(&krates)
3335
.into_iter()
34-
.map(|versions| Version::max(versions.into_iter().map(|v| v.num)).to_string())
36+
.map(|versions| {
37+
let pairs: Vec<_> = versions
38+
.into_iter()
39+
.map(|version| (version.created_at, version.num.to_string()))
40+
.collect();
41+
TopVersions {
42+
newest: pairs
43+
.to_owned()
44+
.into_iter()
45+
.max()
46+
.unwrap_or((NaiveDateTime::from_timestamp(0, 0), "0.0.0".to_owned()))
47+
.1,
48+
highest: Version::max(
49+
pairs
50+
.into_iter()
51+
.map(|(_, s)| semver::Version::parse(&s).unwrap()),
52+
)
53+
.to_string(),
54+
}
55+
})
3556
.zip(krates)
36-
.map(|(max_version, krate)| {
37-
Ok(krate.minimal_encodable(&max_version, None, false, None))
57+
.map(|(top_versions, krate)| {
58+
Ok(krate.minimal_encodable(&top_versions, None, false, None))
3859
})
3960
.collect()
4061
};

src/controllers/krate/publish.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult<Response> {
215215
};
216216

217217
Ok(req.json(&GoodCrate {
218-
krate: krate.minimal_encodable(&top_versions.highest, None, false, None),
218+
krate: krate.minimal_encodable(&top_versions, None, false, None),
219219
warnings,
220220
}))
221221
})

src/controllers/krate/search.rs

+22-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
//! Endpoint for searching and discovery functionality
22
3+
use chrono::NaiveDateTime;
34
use diesel::dsl::*;
45
use diesel_full_text_search::*;
56

67
use crate::controllers::helpers::Paginate;
78
use crate::controllers::prelude::*;
9+
use crate::models::krate::TopVersions;
810
use crate::models::{Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, Version};
911
use crate::schema::*;
1012
use crate::views::EncodableCrate;
@@ -179,7 +181,26 @@ pub fn search(req: &mut dyn Request) -> AppResult<Response> {
179181
.load::<Version>(&*conn)?
180182
.grouped_by(&crates)
181183
.into_iter()
182-
.map(|versions| Version::max(versions.into_iter().map(|v| v.num)).to_string());
184+
.map(|versions| {
185+
let pairs: Vec<_> = versions
186+
.into_iter()
187+
.map(|version| (version.created_at, version.num.to_string()))
188+
.collect();
189+
TopVersions {
190+
newest: pairs
191+
.to_owned()
192+
.into_iter()
193+
.max()
194+
.unwrap_or((NaiveDateTime::from_timestamp(0, 0), "0.0.0".to_owned()))
195+
.1,
196+
highest: Version::max(
197+
pairs
198+
.into_iter()
199+
.map(|(_, s)| semver::Version::parse(&s).unwrap()),
200+
)
201+
.to_string(),
202+
}
203+
});
183204

184205
let badges = CrateBadge::belonging_to(&crates)
185206
.select((badges::crate_id, badges::all_columns))

src/models/krate.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -289,16 +289,13 @@ impl Crate {
289289

290290
pub fn minimal_encodable(
291291
self,
292-
max_version: &str,
292+
top_versions: &TopVersions,
293293
badges: Option<Vec<Badge>>,
294294
exact_match: bool,
295295
recent_downloads: Option<i64>,
296296
) -> EncodableCrate {
297297
self.encodable(
298-
&TopVersions {
299-
highest: max_version.to_owned(),
300-
newest: max_version.to_owned(),
301-
},
298+
top_versions,
302299
None,
303300
None,
304301
None,

src/tests/krate.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -1249,7 +1249,6 @@ fn summary_new_crates() {
12491249
let krate4 = CrateBuilder::new("just_updated_patch", user.id)
12501250
.version(VersionBuilder::new("0.1.0"))
12511251
.version(VersionBuilder::new("0.2.0"))
1252-
.version(VersionBuilder::new("0.1.1"))
12531252
.expect_build(conn);
12541253

12551254
CrateBuilder::new("with_downloads", user.id)
@@ -1277,13 +1276,19 @@ fn summary_new_crates() {
12771276
.execute(&*conn)
12781277
.unwrap();
12791278

1280-
// Adjust 'just_updated_patch' to be a little newer than 'just_updated'
1281-
update(&krate4)
1282-
.set(crates::updated_at.eq(updated + chrono::Duration::seconds(2)))
1279+
let plus_two = Utc::now().naive_utc() + chrono::Duration::seconds(2);
1280+
let newer = VersionBuilder::new("0.1.1").expect_build(krate4.id, user.id, conn);
1281+
1282+
// Update the patch version to be newer than the other versions, including the higher one.
1283+
update(&newer)
1284+
.set(versions::created_at.eq(plus_two))
12831285
.execute(&*conn)
12841286
.unwrap();
12851287

1286-
// TODO: Make sure that the most recently updated version for just_updated_patch is 0.1.1.
1288+
update(&krate4)
1289+
.set(crates::updated_at.eq(plus_two))
1290+
.execute(&*conn)
1291+
.unwrap();
12871292
});
12881293

12891294
let json: SummaryResponse = anon.get("/api/v1/summary").good();
@@ -1301,7 +1306,7 @@ fn summary_new_crates() {
13011306

13021307
assert_eq!(json.just_updated[0].name, "just_updated_patch");
13031308
assert_eq!(json.just_updated[0].max_version, "0.2.0");
1304-
// FIXME: assert_eq!(json.just_updated[0].newest_version, "0.1.1");
1309+
assert_eq!(json.just_updated[0].newest_version, "0.1.1");
13051310

13061311
assert_eq!(json.just_updated[1].name, "just_updated");
13071312
assert_eq!(json.just_updated[1].max_version, "0.1.2");

0 commit comments

Comments
 (0)