Skip to content

Don't parse Version#num for reads #610

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 1 commit into from
Mar 11, 2017
Merged

Don't parse Version#num for reads #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions src/bin/delete-version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
extern crate cargo_registry;
extern crate postgres;
extern crate time;
extern crate semver;

use std::env;
use std::io;
Expand Down Expand Up @@ -38,7 +37,6 @@ fn delete(tx: &postgres::transaction::Transaction) {
None => { println!("needs a version argument"); return }
Some(s) => s,
};
let version = semver::Version::parse(&version).unwrap();

let krate = Crate::find_by_name(tx, &name).unwrap();
let v = Version::find_by_num(tx, krate.id, &version).unwrap().unwrap();
Expand Down
1 change: 0 additions & 1 deletion src/bin/transfer-crates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
extern crate cargo_registry;
extern crate postgres;
extern crate time;
extern crate semver;

use std::env;
use std::io;
Expand Down
3 changes: 1 addition & 2 deletions src/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::fs::{self, File};
use std::io::prelude::*;
use std::path::{Path, PathBuf};

use semver;
use git2;
use rustc_serialize::json;

Expand Down Expand Up @@ -69,7 +68,7 @@ pub fn add_crate(app: &App, krate: &Crate) -> CargoResult<()> {
})
}

pub fn yank(app: &App, krate: &str, version: &semver::Version,
pub fn yank(app: &App, krate: &str, version: &str,
yanked: bool) -> CargoResult<()> {
let repo = app.git_repo.lock().unwrap();
let repo = &*repo;
Expand Down
19 changes: 10 additions & 9 deletions src/krate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::io::prelude::*;
use std::io;
use std::mem;
use std::sync::Arc;
use std::borrow::Cow;

use conduit::{Request, Response};
use conduit_router::RequestParams;
Expand Down Expand Up @@ -64,7 +65,7 @@ pub struct EncodableCrate {
pub badges: Option<Vec<EncodableBadge>>,
pub created_at: String,
pub downloads: i32,
pub max_version: String,
pub max_version: Cow<'static, str>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐮

pub description: Option<String>,
pub homepage: Option<String>,
pub documentation: Option<String>,
Expand Down Expand Up @@ -232,13 +233,13 @@ impl Crate {
}

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

pub fn encodable(self,
max_version: semver::Version,
max_version: Cow<'static, str>,
versions: Option<Vec<i32>>,
keywords: Option<&[Keyword]>,
categories: Option<&[Category]>,
Expand Down Expand Up @@ -268,7 +269,7 @@ impl Crate {
keywords: keyword_ids,
categories: category_ids,
badges: badges,
max_version: max_version.to_string(),
max_version: max_version,
documentation: documentation,
homepage: homepage,
description: description,
Expand All @@ -283,14 +284,14 @@ impl Crate {
}
}

pub fn max_version(&self, conn: &GenericConnection) -> CargoResult<semver::Version> {
pub fn max_version(&self, conn: &GenericConnection) -> CargoResult<Cow<'static, str>> {
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()))
.map(|r| Cow::Owned(r.get("num")))
.max_by_key(|v| semver::Version::parse(&v).ok())
.unwrap_or_else(|| Cow::Borrowed("0.0.0")))
}

pub fn versions(&self, conn: &GenericConnection) -> CargoResult<Vec<Version>> {
Expand Down Expand Up @@ -388,7 +389,7 @@ impl Crate {
features: &HashMap<String, Vec<String>>,
authors: &[String])
-> CargoResult<Version> {
match Version::find_by_num(conn, self.id, ver)? {
match Version::find_by_num(conn, self.id, &ver.to_string())? {
Some(..) => {
return Err(human(format!("crate version `{}` is already uploaded",
ver)))
Expand Down
7 changes: 4 additions & 3 deletions src/tests/all.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,10 @@ fn mock_user(req: &mut Request, u: User) -> User {
}

fn mock_crate(req: &mut Request, krate: Crate) -> (Crate, Version) {
mock_crate_vers(req, krate, &semver::Version::parse("1.0.0").unwrap())
mock_crate_vers(req, krate, "1.0.0")
}

fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
fn mock_crate_vers(req: &mut Request, krate: Crate, v: &str)
-> (Crate, Version) {
let user = req.extensions().find::<User>().unwrap();
let mut krate = Crate::find_or_insert(req.tx().unwrap(), &krate.name,
Expand All @@ -231,7 +231,8 @@ fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
&krate.license,
&None,
krate.max_upload_size).unwrap();
let v = krate.add_version(req.tx().unwrap(), v, &HashMap::new(), &[]);
let v = semver::Version::parse(v).unwrap();
let v = krate.add_version(req.tx().unwrap(), &v, &HashMap::new(), &[]);
(krate, v.unwrap())
}

Expand Down
43 changes: 16 additions & 27 deletions src/tests/krate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,14 +1156,12 @@ fn ignored_badges() {
fn reverse_dependencies() {
let (_b, app, middle) = ::app();

let v100 = semver::Version::parse("1.0.0").unwrap();
let v110 = semver::Version::parse("1.1.0").unwrap();
let mut req = ::req(app, Method::Get,
"/api/v1/crates/c1/reverse_dependencies");
::mock_user(&mut req, ::user("foo"));
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v110);
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.1.0");

::mock_dep(&mut req, &c2v1, &c1, None);
::mock_dep(&mut req, &c2v2, &c1, None);
Expand All @@ -1187,15 +1185,12 @@ fn reverse_dependencies() {
fn reverse_dependencies_when_old_version_doesnt_depend_but_new_does() {
let (_b, app, middle) = ::app();

let v100 = semver::Version::parse("1.0.0").unwrap();
let v110 = semver::Version::parse("1.1.0").unwrap();
let v200 = semver::Version::parse("2.0.0").unwrap();
let mut req = ::req(app, Method::Get,
"/api/v1/crates/c1/reverse_dependencies");
::mock_user(&mut req, ::user("foo"));
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v110);
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.1.0");
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");

::mock_dep(&mut req, &c2v2, &c1, None);

Expand All @@ -1210,14 +1205,12 @@ fn reverse_dependencies_when_old_version_doesnt_depend_but_new_does() {
fn reverse_dependencies_when_old_version_depended_but_new_doesnt() {
let (_b, app, middle) = ::app();

let v100 = semver::Version::parse("1.0.0").unwrap();
let v200 = semver::Version::parse("2.0.0").unwrap();
let mut req = ::req(app, Method::Get,
"/api/v1/crates/c1/reverse_dependencies");
::mock_user(&mut req, ::user("foo"));
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");

::mock_dep(&mut req, &c2v1, &c1, None);

Expand All @@ -1231,15 +1224,13 @@ fn reverse_dependencies_when_old_version_depended_but_new_doesnt() {
fn prerelease_versions_not_included_in_reverse_dependencies() {
let (_b, app, middle) = ::app();

let v100 = semver::Version::parse("1.0.0").unwrap();
let v110_pre = semver::Version::parse("1.1.0-pre").unwrap();
let mut req = ::req(app, Method::Get,
"/api/v1/crates/c1/reverse_dependencies");
::mock_user(&mut req, ::user("foo"));
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v110_pre);
let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), &v100);
let _ = ::mock_crate_vers(&mut req, ::krate("c3"), &v110_pre);
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.1.0-pre");
let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), "1.0.0");
let _ = ::mock_crate_vers(&mut req, ::krate("c3"), "1.1.0-pre");

::mock_dep(&mut req, &c3v1, &c1, None);

Expand All @@ -1254,14 +1245,12 @@ fn prerelease_versions_not_included_in_reverse_dependencies() {
fn yanked_versions_not_included_in_reverse_dependencies() {
let (_b, app, middle) = ::app();

let v100 = semver::Version::parse("1.0.0").unwrap();
let v200 = semver::Version::parse("2.0.0").unwrap();
let mut req = ::req(app, Method::Get,
"/api/v1/crates/c1/reverse_dependencies");
::mock_user(&mut req, ::user("foo"));
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");

::mock_dep(&mut req, &c2v2, &c1, None);

Expand Down
11 changes: 3 additions & 8 deletions src/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use util::{RequestUtils, CargoResult, ChainError, internal, human};
pub struct Version {
pub id: i32,
pub crate_id: i32,
pub num: semver::Version,
pub num: String,
pub updated_at: Timespec,
pub created_at: Timespec,
pub downloads: i32,
Expand Down Expand Up @@ -61,9 +61,8 @@ pub struct VersionLinks {
impl Version {
pub fn find_by_num(conn: &GenericConnection,
crate_id: i32,
num: &semver::Version)
num: &str)
-> CargoResult<Option<Version>> {
let num = num.to_string();
let stmt = conn.prepare("SELECT * FROM versions \
WHERE crate_id = $1 AND num = $2")?;
let rows = stmt.query(&[&crate_id, &num])?;
Expand Down Expand Up @@ -192,15 +191,14 @@ impl Version {

impl Model for Version {
fn from_row(row: &Row) -> Version {
let num: String = row.get("num");
let features: Option<String> = row.get("features");
let features = features.map(|s| {
json::decode(&s).unwrap()
}).unwrap_or_else(|| HashMap::new());
Version {
id: row.get("id"),
crate_id: row.get("crate_id"),
num: semver::Version::parse(&num).unwrap(),
num: row.get("num"),
updated_at: row.get("updated_at"),
created_at: row.get("created_at"),
downloads: row.get("downloads"),
Expand Down Expand Up @@ -271,9 +269,6 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
fn version_and_crate(req: &mut Request) -> CargoResult<(Version, Crate)> {
let crate_name = &req.params()["crate_id"];
let semver = &req.params()["version"];
let semver = semver::Version::parse(semver).map_err(|_| {
human(format!("invalid semver: {}", semver))
})?;
let tx = req.tx()?;
let krate = Crate::find_by_name(tx, crate_name)?;
let version = Version::find_by_num(tx, krate.id, &semver)?;
Expand Down