Auto merge of #3049 - Turbo87:semver-req, r=jtgeibel

Save original dependency requirement string in the database when publishing

This PR should resolve #3047 by saving the original dependency requirement string in the database when publishing, and returning the original string too when calling the `GET /crates/:crate_id/:version/dependencies` endpoint.

Before #2990 we were turning `1.2.3` into `^1.2.3` automatically. After that PR were turning it into `>=1.2.3, <2.0.0`, and now, with this PR, we would keep it at `1.2.3` and it would be up to the API client to prefix the `^`, if necessary.

r? `@jtgeibel`

/cc `@dtolnay`

Author: bors

src/models/dependency.rs

@@ -20,7 +20,7 @@ pub struct Dependency {
     pub id: i32,
     pub version_id: i32,
     pub crate_id: i32,
-    pub req: semver::VersionReq,
+    pub req: String,
     pub optional: bool,
     pub default_features: bool,
     pub features: Vec<String>,
@@ -56,7 +56,7 @@ impl Dependency {
             id: self.id,
             version_id: self.version_id,
             crate_id: crate_name.into(),
-            req: self.req.to_string(),
+            req: self.req,
             optional: self.optional,
             default_features: self.default_features,
             features: self.features,
@@ -95,7 +95,7 @@ pub fn add_dependencies(
             let krate:Crate = Crate::by_exact_name(&dep.name)
                 .first(&*conn)
                 .map_err(|_| cargo_err(&format_args!("no known crate named `{}`", &*dep.name)))?;
-            if dep.version_req == semver::VersionReq::parse("*").unwrap() {
+            if semver::VersionReq::parse(&dep.version_req.0) == semver::VersionReq::parse("*") {
                 return Err(cargo_err(WILDCARD_ERROR_MESSAGE));
             }
 

src/tests/builders/dependency.rs

@@ -15,7 +15,7 @@ impl DependencyBuilder {
             explicit_name_in_toml: None,
             name: name.to_string(),
             registry: None,
-            version_req: u::EncodableCrateVersionReq(semver::VersionReq::parse(">= 0").unwrap()),
+            version_req: u::EncodableCrateVersionReq("> 0".to_string()),
         }
     }
 
@@ -38,10 +38,7 @@ impl DependencyBuilder {
     /// Panics if the `version_req` string specified isn't a valid `semver::VersionReq`.
     #[track_caller]
     pub fn version_req(mut self, version_req: &str) -> Self {
-        self.version_req = u::EncodableCrateVersionReq(
-            semver::VersionReq::parse(version_req)
-                .expect("version req isn't a valid semver::VersionReq"),
-        );
+        self.version_req = u::EncodableCrateVersionReq(version_req.to_string());
         self
     }
 

src/tests/builders/publish.rs

@@ -180,8 +180,7 @@ impl PublishBuilder {
         self
     }
 
-    /// Consume this builder to make the Put request body
-    pub fn body(self) -> Vec<u8> {
+    pub fn build(self) -> (String, Vec<u8>) {
         let new_crate = u::EncodableCrateUpload {
             name: u::EncodableCrateName(self.krate_name.clone()),
             vers: u::EncodableCrateVersion(self.version),
@@ -209,7 +208,16 @@ impl PublishBuilder {
             links: None,
         };
 
-        let json = serde_json::to_string(&new_crate).unwrap();
+        (serde_json::to_string(&new_crate).unwrap(), self.tarball)
+    }
+
+    /// Consume this builder to make the Put request body
+    pub fn body(self) -> Vec<u8> {
+        let (json, tarball) = self.build();
+        PublishBuilder::create_publish_body(&json, &tarball)
+    }
+
+    pub fn create_publish_body(json: &str, tarball: &[u8]) -> Vec<u8> {
         let mut body = Vec::new();
         body.extend(
             [
@@ -223,7 +231,6 @@ impl PublishBuilder {
         );
         body.extend(json.as_bytes().iter().cloned());
 
-        let tarball = &self.tarball;
         body.extend(&[
             tarball.len() as u8,
             (tarball.len() >> 8) as u8,

src/tests/http-data/krate_publish_new_krate_with_broken_dependency_requirement

@@ -0,0 +1,69 @@
+[
+  {
+    "request": {
+      "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/new_dep/new_dep-1.0.0.crate",
+      "method": "PUT",
+      "headers": [
+        [
+          "host",
+          "alexcrichton-test.s3.amazonaws.com"
+        ],
+        [
+          "authorization",
+          "AWS AKIAICL5IWUZYWWKA7JA:lH91IZJxBDr21vB3eSZXwXTQelk="
+        ],
+        [
+          "date",
+          "Fri, 15 Sep 2017 07:53:06 -0700"
+        ],
+        [
+          "content-type",
+          "application/x-tar"
+        ],
+        [
+          "accept-encoding",
+          "gzip"
+        ],
+        [
+          "content-length",
+          "35"
+        ],
+        [
+          "accept",
+          "*/*"
+        ]
+      ],
+      "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
+    },
+    "response": {
+      "status": 200,
+      "headers": [
+        [
+          "Server",
+          "AmazonS3"
+        ],
+        [
+          "content-length",
+          "0"
+        ],
+        [
+          "x-amz-request-id",
+          "8190F375F5651D47"
+        ],
+        [
+          "x-amz-id-2",
+          "PQEXq2S9IgVfG/u0l2cjHuWU8EDPlRSwxqai2oNPCz8TyX/CEwn2ydL+A1qnNK3Ng+FpC9ukVl8="
+        ],
+        [
+          "date",
+          "Fri, 15 Sep 2017 14:53:07 GMT"
+        ],
+        [
+          "ETag",
+          "\"f9016ad360cebb4fe2e6e96e5949f022\""
+        ]
+      ],
+      "body": ""
+    }
+  }
+]

src/tests/krate/dependencies.rs

@@ -5,8 +5,8 @@ use cargo_registry::views::EncodableDependency;
 use http::StatusCode;
 
 #[derive(Deserialize)]
-struct Deps {
-    dependencies: Vec<EncodableDependency>,
+pub struct Deps {
+    pub dependencies: Vec<EncodableDependency>,
 }
 
 #[test]

src/tests/krate/publish.rs

@@ -173,7 +173,9 @@ fn new_with_renamed_dependency() {
 
 #[test]
 fn new_krate_with_dependency() {
-    let (app, _, user, token) = TestApp::full().with_token();
+    use super::dependencies::Deps;
+
+    let (app, anon, user, token) = TestApp::full().with_token();
 
     app.db(|conn| {
         // Insert a crate directly into the database so that new_dep can depend on it
@@ -183,12 +185,56 @@ fn new_krate_with_dependency() {
         CrateBuilder::new("foo-dep", user.as_model().id).expect_build(conn);
     });
 
-    let dependency = DependencyBuilder::new("foo-dep");
+    let dependency = DependencyBuilder::new("foo-dep").version_req("1.0.0");
 
     let crate_to_publish = PublishBuilder::new("new_dep")
         .version("1.0.0")
         .dependency(dependency);
+
     token.enqueue_publish(crate_to_publish).good();
+
+    let dependencies = anon
+        .get::<Deps>("/api/v1/crates/new_dep/1.0.0/dependencies")
+        .good()
+        .dependencies;
+
+    assert_eq!(dependencies.len(), 1);
+    assert_eq!(dependencies[0].crate_id, "foo-dep");
+    assert_eq!(dependencies[0].req, "1.0.0");
+}
+
+#[test]
+fn new_krate_with_broken_dependency_requirement() {
+    let (app, _, user, token) = TestApp::full().with_token();
+
+    app.db(|conn| {
+        // Insert a crate directly into the database so that new_dep can depend on it
+        // The name choice of `foo-dep` is important! It has the property of
+        // name != canon_crate_name(name) and is a regression test for
+        // https://github.com/rust-lang/crates.io/issues/651
+        CrateBuilder::new("foo-dep", user.as_model().id).expect_build(conn);
+    });
+
+    let dependency = DependencyBuilder::new("foo-dep").version_req("1.2.3");
+
+    let crate_to_publish = PublishBuilder::new("new_dep")
+        .version("1.0.0")
+        .dependency(dependency);
+
+    // create a request body with `version_req: "broken"`
+    let (json, tarball) = crate_to_publish.build();
+    let new_json = json.replace("\"version_req\":\"1.2.3\"", "\"version_req\":\"broken\"");
+    assert_ne!(json, new_json);
+    let body = PublishBuilder::create_publish_body(&new_json, &tarball);
+
+    let response = token
+        .put::<serde_json::Value>("/api/v1/crates/new", &body)
+        .good();
+
+    assert_eq!(
+        response,
+        json!({"errors": [{"detail": "invalid upload request: invalid value: string \"broken\", expected a valid version req at line 1 column 136"}]})
+    );
 }
 
 #[test]

src/views/krate_publish.rs

@@ -41,7 +41,7 @@ pub struct EncodableCrateName(pub String);
 #[derive(Debug, Deref)]
 pub struct EncodableCrateVersion(pub semver::Version);
 #[derive(Debug, Deref)]
-pub struct EncodableCrateVersionReq(pub semver::VersionReq);
+pub struct EncodableCrateVersionReq(pub String);
 #[derive(Serialize, Debug, Deref, Default)]
 pub struct EncodableKeywordList(pub Vec<EncodableKeyword>);
 #[derive(Serialize, Debug, Deref)]
@@ -152,7 +152,7 @@ impl<'de> Deserialize<'de> for EncodableCrateVersionReq {
     fn deserialize<D: Deserializer<'de>>(d: D) -> Result<EncodableCrateVersionReq, D::Error> {
         let s = String::deserialize(d)?;
         match semver::VersionReq::parse(&s) {
-            Ok(v) => Ok(EncodableCrateVersionReq(v)),
+            Ok(_) => Ok(EncodableCrateVersionReq(s)),
             Err(..) => {
                 let value = de::Unexpected::Str(&s);
                 let expected = "a valid version req";
@@ -162,15 +162,6 @@ impl<'de> Deserialize<'de> for EncodableCrateVersionReq {
     }
 }
 
-impl<T: ?Sized> PartialEq<T> for EncodableCrateVersionReq
-where
-    semver::VersionReq: PartialEq<T>,
-{
-    fn eq(&self, rhs: &T) -> bool {
-        self.0 == *rhs
-    }
-}
-
 impl<'de> Deserialize<'de> for EncodableKeywordList {
     fn deserialize<D: Deserializer<'de>>(d: D) -> Result<EncodableKeywordList, D::Error> {
         let inner = <Vec<EncodableKeyword> as Deserialize<'de>>::deserialize(d)?;