Skip to content

Clean up TopVersions code #3113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/controllers/krate/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::controllers::helpers::pagination::PaginationOptions;

use crate::models::{
Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
User, Version, VersionOwnerAction,
TopVersions, User, Version, VersionOwnerAction,
};
use crate::schema::*;
use crate::views::{
Expand Down Expand Up @@ -37,7 +37,7 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult {
versions
.grouped_by(&krates)
.into_iter()
.map(|versions| Version::top(versions.into_iter().map(|v| (v.created_at, v.num))))
.map(TopVersions::from_versions)
.zip(krates)
.zip(recent_downloads)
.map(|((top_versions, krate), recent_downloads)| {
Expand Down
6 changes: 4 additions & 2 deletions src/controllers/krate/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use diesel_full_text_search::*;
use crate::controllers::cargo_prelude::*;
use crate::controllers::helpers::Paginate;
use crate::controllers::util::AuthenticatedUser;
use crate::models::{Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, Version};
use crate::models::{
Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, TopVersions, Version,
};
use crate::schema::*;
use crate::util::errors::{bad_request, ChainError};
use crate::views::EncodableCrate;
Expand Down Expand Up @@ -207,7 +209,7 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
let versions = versions
.grouped_by(&crates)
.into_iter()
.map(|versions| Version::top(versions.into_iter().map(|v| (v.created_at, v.num))));
.map(TopVersions::from_versions);

let badges: Vec<CrateBadge> = CrateBadge::belonging_to(&crates)
.select((badges::crate_id, badges::all_columns))
Expand Down
2 changes: 1 addition & 1 deletion src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub use self::rights::Rights;
pub use self::team::{NewTeam, Team};
pub use self::token::{ApiToken, CreatedApiToken};
pub use self::user::{NewUser, User};
pub use self::version::{NewVersion, Version};
pub use self::version::{NewVersion, TopVersions, Version};

pub mod helpers;

Expand Down
18 changes: 15 additions & 3 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,18 @@ impl Crate {
let badges = badges.map(|bs| bs.into_iter().map(Badge::into).collect());
let documentation = Crate::remove_blocked_documentation_urls(documentation);

let max_version = top_versions
.highest
.as_ref()
.map(|v| v.to_string())
.unwrap_or_else(|| "0.0.0".to_string());

let newest_version = top_versions
.newest
.as_ref()
.map(|v| v.to_string())
.unwrap_or_else(|| "0.0.0".to_string());

EncodableCrate {
id: name.clone(),
name: name.clone(),
Expand All @@ -356,8 +368,8 @@ impl Crate {
keywords: keyword_ids,
categories: category_ids,
badges,
max_version: top_versions.highest.to_string(),
newest_version: top_versions.newest.to_string(),
max_version,
newest_version,
documentation,
homepage,
exact_match,
Expand Down Expand Up @@ -407,7 +419,7 @@ impl Crate {
pub fn top_versions(&self, conn: &PgConnection) -> QueryResult<TopVersions> {
use crate::schema::versions::dsl::*;

Ok(Version::top(
Ok(TopVersions::from_date_version_pairs(
self.versions().select((updated_at, num)).load(conn)?,
))
}
Expand Down
75 changes: 31 additions & 44 deletions src/models/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,29 @@ pub struct NewVersion {
/// Typically used for a single crate.
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct TopVersions {
pub highest: semver::Version,
pub newest: semver::Version,
/// The "highest" version in terms of semver
pub highest: Option<semver::Version>,
/// The "newest" version in terms of publishing date
pub newest: Option<semver::Version>,
}

/// A default semver value, "0.0.0", for use in TopVersions
fn default_semver_version() -> semver::Version {
semver::Version {
major: 0,
minor: 0,
patch: 0,
pre: vec![],
build: vec![],
impl TopVersions {
/// Return both the newest (most recently updated) and the
/// highest version (in semver order) for a list of `Version` instances.
pub fn from_versions(versions: Vec<Version>) -> Self {
Self::from_date_version_pairs(versions.into_iter().map(|v| (v.created_at, v.num)))
}

/// Return both the newest (most recently updated) and the
/// highest version (in semver order) for a collection of date/version pairs.
pub fn from_date_version_pairs<T>(pairs: T) -> Self
where
T: Clone + IntoIterator<Item = (NaiveDateTime, semver::Version)>,
{
let newest = pairs.clone().into_iter().max().map(|(_, v)| v);
let highest = pairs.into_iter().map(|(_, v)| v).max();

Self { newest, highest }
}
}

Expand Down Expand Up @@ -115,30 +126,6 @@ impl Version {
.load(conn)
}

/// Return both the newest (most recently updated) and the
/// highest version (in semver order) for a collection of date/version pairs.
pub fn top<T>(pairs: T) -> TopVersions
where
T: Clone + IntoIterator<Item = (NaiveDateTime, semver::Version)>,
{
TopVersions {
newest: pairs
.clone()
.into_iter()
.max()
.unwrap_or((
NaiveDateTime::from_timestamp(0, 0),
default_semver_version(),
))
.1,
highest: pairs
.into_iter()
.map(|(_, v)| v)
.max()
.unwrap_or_else(default_semver_version),
}
}

pub fn record_readme_rendering(version_id_: i32, conn: &PgConnection) -> QueryResult<usize> {
use crate::schema::readme_renderings::dsl::*;
use diesel::dsl::now;
Expand Down Expand Up @@ -255,7 +242,7 @@ impl NewVersion {

#[cfg(test)]
mod tests {
use super::{TopVersions, Version};
use super::TopVersions;
use chrono::NaiveDateTime;

#[track_caller]
Expand All @@ -272,10 +259,10 @@ mod tests {
fn top_versions_empty() {
let versions = vec![];
assert_eq!(
Version::top(versions),
TopVersions::from_date_version_pairs(versions),
TopVersions {
highest: version("0.0.0"),
newest: version("0.0.0"),
highest: None,
newest: None,
}
);
}
Expand All @@ -284,10 +271,10 @@ mod tests {
fn top_versions_single() {
let versions = vec![(date("2020-12-03T12:34:56"), version("1.0.0"))];
assert_eq!(
Version::top(versions),
TopVersions::from_date_version_pairs(versions),
TopVersions {
highest: version("1.0.0"),
newest: version("1.0.0"),
highest: Some(version("1.0.0")),
newest: Some(version("1.0.0")),
}
);
}
Expand All @@ -300,10 +287,10 @@ mod tests {
(date("2020-12-03T12:34:56"), version("1.1.0")),
];
assert_eq!(
Version::top(versions),
TopVersions::from_date_version_pairs(versions),
TopVersions {
highest: version("2.0.0-alpha.1"),
newest: version("1.1.0"),
highest: Some(version("2.0.0-alpha.1")),
newest: Some(version("1.1.0")),
}
);
}
Expand Down