Skip to content

Remove the max_version column from Crates #592

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.

Sign up for GitHub

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 6 commits into from
Mar 7, 2017
Merged

Remove the max_version column from Crates #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f1a66c1
Remove the `max_version` column from Crates
sgrif Mar 5, 2017
eedadbe
Pull in tests from #582
yodaldevoid Mar 7, 2017
31b44cc
Make the `max_version` migration reversible
sgrif Mar 7, 2017
99afd2f
Disable "Version does not exist" error if the version is null
yodaldevoid Mar 4, 2017
f44f544
Don't use version num 0.0.0, all versions have been yanked
carols10cents Mar 7, 2017
1310349
Remove the migration to drop the column, we'll do that in a separate …
carols10cents Mar 7, 2017
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 app/routes/crate/version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default Ember.Route.extend({
const maxVersion = crate.get('max_version');

// Fall back to the crate's `max_version` property
if (!requestedVersion) {
if (!requestedVersion && maxVersion !== '0.0.0') {
params.version_num = maxVersion;
}

Expand All @@ -29,7 +29,7 @@ export default Ember.Route.extend({
return crate.get('versions')
.then(versions => {
const version = versions.find(version => version.get('num') === params.version_num);
if (!version) {
if (params.version_num && !version) {
this.controllerFor('application').set('nextFlashError',
`Version '${params.version_num}' of crate '${crate.get('name')}' does not exist`);
}
Expand Down
58 changes: 32 additions & 26 deletions src/krate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub struct Crate {
pub updated_at: Timespec,
pub created_at: Timespec,
pub downloads: i32,
pub max_version: semver::Version,
pub description: Option<String>,
pub homepage: Option<String>,
pub documentation: Option<String>,
Expand Down Expand Up @@ -233,18 +232,20 @@ impl Crate {
}

pub fn minimal_encodable(self,
max_version: semver::Version,
badges: Option<Vec<Badge>>) -> EncodableCrate {
self.encodable(None, None, None, badges)
self.encodable(max_version, None, None, None, badges)
}

pub fn encodable(self,
max_version: semver::Version,
versions: Option<Vec<i32>>,
keywords: Option<&[Keyword]>,
categories: Option<&[Category]>,
badges: Option<Vec<Badge>>)
-> EncodableCrate {
let Crate {
name, created_at, updated_at, downloads, max_version, description,
name, created_at, updated_at, downloads, description,
homepage, documentation, license, repository,
readme: _, id: _, max_upload_size: _,
} = self;
Expand All @@ -255,7 +256,7 @@ impl Crate {
let keyword_ids = keywords.map(|kws| kws.iter().map(|kw| kw.keyword.clone()).collect());
let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.slug.clone()).collect());
let badges = badges.map(|bs| {
bs.iter().map(|b| b.clone().encodable()).collect()
bs.into_iter().map(|b| b.encodable()).collect()
});
EncodableCrate {
id: name.clone(),
Expand All @@ -282,6 +283,16 @@ impl Crate {
}
}

pub fn max_version(&self, conn: &GenericConnection) -> CargoResult<semver::Version> {
let stmt = conn.prepare("SELECT num FROM versions WHERE crate_id = $1
AND yanked = 'f'")?;
let rows = stmt.query(&[&self.id])?;
Ok(rows.iter()
.map(|r| semver::Version::parse(&r.get::<_, String>("num")).unwrap())
.max()
.unwrap_or_else(|| semver::Version::parse("0.0.0").unwrap()))
}

pub fn versions(&self, conn: &GenericConnection) -> CargoResult<Vec<Version>> {
let stmt = conn.prepare("SELECT * FROM versions \
WHERE crate_id = $1")?;
Expand Down Expand Up @@ -384,14 +395,6 @@ impl Crate {
}
None => {}
}
let zero = semver::Version::parse("0.0.0").unwrap();
if *ver > self.max_version || self.max_version == zero {
self.max_version = ver.clone();
}
let stmt = conn.prepare("UPDATE crates SET max_version = $1
WHERE id = $2 RETURNING updated_at")?;
let rows = stmt.query(&[&self.max_version.to_string(), &self.id])?;
self.updated_at = rows.get(0).get("updated_at");
Version::insert(conn, self.id, ver, features, authors)
}

Expand Down Expand Up @@ -434,34 +437,34 @@ impl Crate {
INNER JOIN crates
ON crates.id = versions.crate_id
WHERE dependencies.crate_id = $1
AND versions.num = crates.max_version
AND versions.num = $2
";
let fetch_sql = format!("SELECT DISTINCT ON (crate_downloads, crate_name)
dependencies.*,
crates.downloads AS crate_downloads,
crates.name AS crate_name
{}
ORDER BY crate_downloads DESC
OFFSET $2
LIMIT $3",
OFFSET $3
LIMIT $4",
select_sql);
let count_sql = format!("SELECT COUNT(DISTINCT(crates.id)) {}", select_sql);

let stmt = conn.prepare(&fetch_sql)?;
let vec: Vec<_> = stmt.query(&[&self.id, &offset, &limit])?
let max_version = self.max_version(conn)?.to_string();
let vec: Vec<_> = stmt.query(&[&self.id, &max_version, &offset, &limit])?
.iter()
.map(|r| (Model::from_row(&r), r.get("crate_name"), r.get("crate_downloads")))
.collect();
let stmt = conn.prepare(&count_sql)?;
let cnt: i64 = stmt.query(&[&self.id])?.iter().next().unwrap().get(0);
let cnt: i64 = stmt.query(&[&self.id, &max_version])?.iter().next().unwrap().get(0);

Ok((vec, cnt))
}
}

impl Model for Crate {
fn from_row(row: &Row) -> Crate {
let max: String = row.get("max_version");
Crate {
id: row.get("id"),
name: row.get("name"),
Expand All @@ -472,7 +475,6 @@ impl Model for Crate {
documentation: row.get("documentation"),
homepage: row.get("homepage"),
readme: row.get("readme"),
max_version: semver::Version::parse(&max).unwrap(),
license: row.get("license"),
repository: row.get("repository"),
max_upload_size: row.get("max_upload_size"),
Expand Down Expand Up @@ -603,8 +605,9 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
let mut crates = Vec::new();
for row in stmt.query(&args)?.iter() {
let krate: Crate = Model::from_row(&row);
let badges = krate.badges(conn);
crates.push(krate.minimal_encodable(badges.ok()));
let badges = krate.badges(conn)?;
let max_version = krate.max_version(conn)?;
crates.push(krate.minimal_encodable(max_version, Some(badges)));
}

// Query for the total count of crates
Expand Down Expand Up @@ -637,10 +640,11 @@ pub fn summary(req: &mut Request) -> CargoResult<Response> {

let to_crates = |stmt: pg::stmt::Statement| -> CargoResult<Vec<_>> {
let rows = stmt.query(&[])?;
Ok(rows.iter().map(|r| {
rows.iter().map(|r| {
let krate: Crate = Model::from_row(&r);
krate.minimal_encodable(None)
}).collect::<Vec<EncodableCrate>>())
let max_version = krate.max_version(tx)?;
Ok(krate.minimal_encodable(max_version, None))
}).collect()
};
let new_crates = tx.prepare("SELECT * FROM crates \
ORDER BY created_at DESC LIMIT 10")?;
Expand Down Expand Up @@ -692,6 +696,7 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
let kws = krate.keywords(conn)?;
let cats = krate.categories(conn)?;
let badges = krate.badges(conn)?;
let max_version = krate.max_version(conn)?;

#[derive(RustcEncodable)]
struct R {
Expand All @@ -702,7 +707,7 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
}
Ok(req.json(&R {
krate: krate.clone().encodable(
Some(ids), Some(&kws), Some(&cats), Some(badges)
max_version, Some(ids), Some(&kws), Some(&cats), Some(badges)
),
versions: versions.into_iter().map(|v| {
v.encodable(&krate.name)
Expand Down Expand Up @@ -785,6 +790,7 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
&krate,
new_crate.badges.unwrap_or_else(HashMap::new)
)?;
let max_version = krate.max_version(req.tx()?)?;

// Upload the crate to S3
let mut handle = req.app().handle();
Expand Down Expand Up @@ -855,7 +861,7 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
#[derive(RustcEncodable)]
struct R { krate: EncodableCrate, warnings: Warnings }
Ok(req.json(&R {
krate: krate.minimal_encodable(None),
krate: krate.minimal_encodable(max_version, None),
warnings: warnings
}))
}
Expand Down
1 change: 0 additions & 1 deletion src/tests/all.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ fn krate(name: &str) -> Crate {
updated_at: time::now().to_timespec(),
created_at: time::now().to_timespec(),
downloads: 10,
max_version: semver::Version::parse("0.0.0").unwrap(),
documentation: None,
homepage: None,
description: None,
Expand Down
41 changes: 41 additions & 0 deletions src/tests/http-data/krate_publish_after_yank_max_version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
===REQUEST 339
PUT http://alexcrichton-test.s3.amazonaws.com/crates/fyk_max/fyk_max-1.0.0.crate HTTP/1.1
Accept: */*
Proxy-Connection: Keep-Alive
Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
Content-Length: 0
Host: alexcrichton-test.s3.amazonaws.com
Content-Type: application/x-tar
Date: Sun, 28 Jun 2015 14:07:17 -0700


===RESPONSE 258
HTTP/1.1 200
x-amz-request-id: CB0E925D8E3AB3E8
x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
date: Sun, 28 Jun 2015 21:07:51 GMT
etag: "d41d8cd98f00b204e9800998ecf8427e"
content-length: 0
server: AmazonS3


===REQUEST 339
PUT http://alexcrichton-test.s3.amazonaws.com/crates/fyk_max/fyk_max-2.0.0.crate HTTP/1.1
Accept: */*
Proxy-Connection: Keep-Alive
Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
Content-Length: 0
Host: alexcrichton-test.s3.amazonaws.com
Content-Type: application/x-tar
Date: Sun, 28 Jun 2015 14:07:17 -0700


===RESPONSE 258
HTTP/1.1 200
x-amz-request-id: CB0E925D8E3AB3E8
x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
date: Sun, 28 Jun 2015 21:07:51 GMT
etag: "d41d8cd98f00b204e9800998ecf8427e"
content-length: 0
server: AmazonS3

41 changes: 41 additions & 0 deletions src/tests/http-data/krate_yank_max_version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
===REQUEST 339
PUT http://alexcrichton-test.s3.amazonaws.com/crates/fyk_max/fyk_max-1.0.0.crate HTTP/1.1
Accept: */*
Proxy-Connection: Keep-Alive
Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
Content-Length: 0
Host: alexcrichton-test.s3.amazonaws.com
Content-Type: application/x-tar
Date: Sun, 28 Jun 2015 14:07:17 -0700


===RESPONSE 258
HTTP/1.1 200
x-amz-request-id: CB0E925D8E3AB3E8
x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
date: Sun, 28 Jun 2015 21:07:51 GMT
etag: "d41d8cd98f00b204e9800998ecf8427e"
content-length: 0
server: AmazonS3


===REQUEST 339
PUT http://alexcrichton-test.s3.amazonaws.com/crates/fyk_max/fyk_max-2.0.0.crate HTTP/1.1
Accept: */*
Proxy-Connection: Keep-Alive
Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
Content-Length: 0
Host: alexcrichton-test.s3.amazonaws.com
Content-Type: application/x-tar
Date: Sun, 28 Jun 2015 14:07:17 -0700


===RESPONSE 258
HTTP/1.1 200
x-amz-request-id: CB0E925D8E3AB3E8
x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
date: Sun, 28 Jun 2015 21:07:51 GMT
etag: "d41d8cd98f00b204e9800998ecf8427e"
content-length: 0
server: AmazonS3

Loading