Skip to content

Commit ddc630d

Browse files
carols10centscarols10cents
authored
Merge pull request #615 from rust-lang/revert-610-sg-dont-parse-semver-for-reads
Revert "Don't parse `Version#num` for reads"
2 parents a64ca12 + a5c1ce3 commit ddc630d

File tree

7 files changed

+52
-34
lines changed

7 files changed

+52
-34
lines changed

‎src/bin/delete-version.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
extern crate cargo_registry;
1111
extern crate postgres;
1212
extern crate time;
13+
extern crate semver;
1314

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

4143
let krate = Crate::find_by_name(tx, &name).unwrap();
4244
let v = Version::find_by_num(tx, krate.id, &version).unwrap().unwrap();

‎src/bin/transfer-crates.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
extern crate cargo_registry;
99
extern crate postgres;
1010
extern crate time;
11+
extern crate semver;
1112

1213
use std::env;
1314
use std::io;

‎src/git.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::fs::{self, File};
44
use std::io::prelude::*;
55
use std::path::{Path, PathBuf};
66

7+
use semver;
78
use git2;
89
use rustc_serialize::json;
910

@@ -68,7 +69,7 @@ pub fn add_crate(app: &App, krate: &Crate) -> CargoResult<()> {
6869
})
6970
}
7071

71-
pub fn yank(app: &App, krate: &str, version: &str,
72+
pub fn yank(app: &App, krate: &str, version: &semver::Version,
7273
yanked: bool) -> CargoResult<()> {
7374
let repo = app.git_repo.lock().unwrap();
7475
let repo = &*repo;

‎src/krate.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use std::io::prelude::*;
55
use std::io;
66
use std::mem;
77
use std::sync::Arc;
8-
use std::borrow::Cow;
98

109
use conduit::{Request, Response};
1110
use conduit_router::RequestParams;
@@ -65,7 +64,7 @@ pub struct EncodableCrate {
6564
pub badges: Option<Vec<EncodableBadge>>,
6665
pub created_at: String,
6766
pub downloads: i32,
68-
pub max_version: Cow<'static, str>,
67+
pub max_version: String,
6968
pub description: Option<String>,
7069
pub homepage: Option<String>,
7170
pub documentation: Option<String>,
@@ -233,13 +232,13 @@ impl Crate {
233232
}
234233

235234
pub fn minimal_encodable(self,
236-
max_version: Cow<'static, str>,
235+
max_version: semver::Version,
237236
badges: Option<Vec<Badge>>) -> EncodableCrate {
238237
self.encodable(max_version, None, None, None, badges)
239238
}
240239

241240
pub fn encodable(self,
242-
max_version: Cow<'static, str>,
241+
max_version: semver::Version,
243242
versions: Option<Vec<i32>>,
244243
keywords: Option<&[Keyword]>,
245244
categories: Option<&[Category]>,
@@ -269,7 +268,7 @@ impl Crate {
269268
keywords: keyword_ids,
270269
categories: category_ids,
271270
badges: badges,
272-
max_version: max_version,
271+
max_version: max_version.to_string(),
273272
documentation: documentation,
274273
homepage: homepage,
275274
description: description,
@@ -284,14 +283,14 @@ impl Crate {
284283
}
285284
}
286285

287-
pub fn max_version(&self, conn: &GenericConnection) -> CargoResult<Cow<'static, str>> {
286+
pub fn max_version(&self, conn: &GenericConnection) -> CargoResult<semver::Version> {
288287
let stmt = conn.prepare("SELECT num FROM versions WHERE crate_id = $1
289288
AND yanked = 'f'")?;
290289
let rows = stmt.query(&[&self.id])?;
291290
Ok(rows.iter()
292-
.map(|r| Cow::Owned(r.get("num")))
293-
.max_by_key(|v| semver::Version::parse(&v).ok())
294-
.unwrap_or_else(|| Cow::Borrowed("0.0.0")))
291+
.map(|r| semver::Version::parse(&r.get::<_, String>("num")).unwrap())
292+
.max()
293+
.unwrap_or_else(|| semver::Version::parse("0.0.0").unwrap()))
295294
}
296295

297296
pub fn versions(&self, conn: &GenericConnection) -> CargoResult<Vec<Version>> {
@@ -389,7 +388,7 @@ impl Crate {
389388
features: &HashMap<String, Vec<String>>,
390389
authors: &[String])
391390
-> CargoResult<Version> {
392-
match Version::find_by_num(conn, self.id, &ver.to_string())? {
391+
match Version::find_by_num(conn, self.id, ver)? {
393392
Some(..) => {
394393
return Err(human(format!("crate version `{}` is already uploaded",
395394
ver)))

‎src/tests/all.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,10 @@ fn sign_in_as(req: &mut Request, user: &User) {
219219
}
220220

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

225-
fn mock_crate_vers(req: &mut Request, krate: Crate, v: &str)
225+
fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
226226
-> (Crate, Version) {
227227
let user = req.extensions().find::<User>().unwrap();
228228
let mut krate = Crate::find_or_insert(req.tx().unwrap(), &krate.name,
@@ -234,8 +234,7 @@ fn mock_crate_vers(req: &mut Request, krate: Crate, v: &str)
234234
&krate.license,
235235
&None,
236236
krate.max_upload_size).unwrap();
237-
let v = semver::Version::parse(v).unwrap();
238-
let v = krate.add_version(req.tx().unwrap(), &v, &HashMap::new(), &[]);
237+
let v = krate.add_version(req.tx().unwrap(), v, &HashMap::new(), &[]);
239238
(krate, v.unwrap())
240239
}
241240

‎src/tests/krate.rs

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,12 +1156,14 @@ fn ignored_badges() {
11561156
fn reverse_dependencies() {
11571157
let (_b, app, middle) = ::app();
11581158

1159+
let v100 = semver::Version::parse("1.0.0").unwrap();
1160+
let v110 = semver::Version::parse("1.1.0").unwrap();
11591161
let mut req = ::req(app, Method::Get,
11601162
"/api/v1/crates/c1/reverse_dependencies");
11611163
::mock_user(&mut req, ::user("foo"));
1162-
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
1163-
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
1164-
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.1.0");
1164+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1165+
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1166+
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v110);
11651167

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

1190+
let v100 = semver::Version::parse("1.0.0").unwrap();
1191+
let v110 = semver::Version::parse("1.1.0").unwrap();
1192+
let v200 = semver::Version::parse("2.0.0").unwrap();
11881193
let mut req = ::req(app, Method::Get,
11891194
"/api/v1/crates/c1/reverse_dependencies");
11901195
::mock_user(&mut req, ::user("foo"));
1191-
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.1.0");
1192-
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
1193-
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");
1196+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v110);
1197+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1198+
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
11941199

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

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

1213+
let v100 = semver::Version::parse("1.0.0").unwrap();
1214+
let v200 = semver::Version::parse("2.0.0").unwrap();
12081215
let mut req = ::req(app, Method::Get,
12091216
"/api/v1/crates/c1/reverse_dependencies");
12101217
::mock_user(&mut req, ::user("foo"));
1211-
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
1212-
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
1213-
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");
1218+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1219+
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1220+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
12141221

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

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

1234+
let v100 = semver::Version::parse("1.0.0").unwrap();
1235+
let v110_pre = semver::Version::parse("1.1.0-pre").unwrap();
12271236
let mut req = ::req(app, Method::Get,
12281237
"/api/v1/crates/c1/reverse_dependencies");
12291238
::mock_user(&mut req, ::user("foo"));
1230-
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
1231-
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.1.0-pre");
1232-
let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), "1.0.0");
1233-
let _ = ::mock_crate_vers(&mut req, ::krate("c3"), "1.1.0-pre");
1239+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1240+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v110_pre);
1241+
let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), &v100);
1242+
let _ = ::mock_crate_vers(&mut req, ::krate("c3"), &v110_pre);
12341243

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

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

1257+
let v100 = semver::Version::parse("1.0.0").unwrap();
1258+
let v200 = semver::Version::parse("2.0.0").unwrap();
12481259
let mut req = ::req(app, Method::Get,
12491260
"/api/v1/crates/c1/reverse_dependencies");
12501261
::mock_user(&mut req, ::user("foo"));
1251-
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), "1.0.0");
1252-
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), "1.0.0");
1253-
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), "2.0.0");
1262+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1263+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1264+
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
12541265

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

‎src/version.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use util::{RequestUtils, CargoResult, ChainError, internal, human};
2525
pub struct Version {
2626
pub id: i32,
2727
pub crate_id: i32,
28-
pub num: String,
28+
pub num: semver::Version,
2929
pub updated_at: Timespec,
3030
pub created_at: Timespec,
3131
pub downloads: i32,
@@ -61,8 +61,9 @@ pub struct VersionLinks {
6161
impl Version {
6262
pub fn find_by_num(conn: &GenericConnection,
6363
crate_id: i32,
64-
num: &str)
64+
num: &semver::Version)
6565
-> CargoResult<Option<Version>> {
66+
let num = num.to_string();
6667
let stmt = conn.prepare("SELECT * FROM versions \
6768
WHERE crate_id = $1 AND num = $2")?;
6869
let rows = stmt.query(&[&crate_id, &num])?;
@@ -191,14 +192,15 @@ impl Version {
191192

192193
impl Model for Version {
193194
fn from_row(row: &Row) -> Version {
195+
let num: String = row.get("num");
194196
let features: Option<String> = row.get("features");
195197
let features = features.map(|s| {
196198
json::decode(&s).unwrap()
197199
}).unwrap_or_else(|| HashMap::new());
198200
Version {
199201
id: row.get("id"),
200202
crate_id: row.get("crate_id"),
201-
num: row.get("num"),
203+
num: semver::Version::parse(&num).unwrap(),
202204
updated_at: row.get("updated_at"),
203205
created_at: row.get("created_at"),
204206
downloads: row.get("downloads"),
@@ -269,6 +271,9 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
269271
fn version_and_crate(req: &mut Request) -> CargoResult<(Version, Crate)> {
270272
let crate_name = &req.params()["crate_id"];
271273
let semver = &req.params()["version"];
274+
let semver = semver::Version::parse(semver).map_err(|_| {
275+
human(format!("invalid semver: {}", semver))
276+
})?;
272277
let tx = req.tx()?;
273278
let krate = Crate::find_by_name(tx, crate_name)?;
274279
let version = Version::find_by_num(tx, krate.id, &semver)?;

0 commit comments

Comments
 (0)