Require a license, description, and author

This basic set of metadata will now be required for any package to be published.
Cargo will soon support publishing with a `license-file` attribute to override
the license validity check performed here.

cc rust-lang/cargo#940

src/bin/update-downloads.rs

@@ -156,7 +156,8 @@ mod test {
         let tx = conn.transaction().unwrap();
         let user = user(&tx);
         let krate = Crate::find_or_insert(&tx, "foo", user.id, &None, &None,
-                                          &None, &None, &[], &None, &None).unwrap();
+                                          &None, &None, &[], &None, &None,
+                                          &None).unwrap();
         let version = Version::insert(&tx, krate.id,
                                       &semver::Version::parse("1.0.0").unwrap(),
                                       &HashMap::new(), &[]).unwrap();
@@ -183,7 +184,7 @@ mod test {
         let user = user(&tx);
         let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
                                           &None, &None, &None, &[], &None,
-                                          &None).unwrap();
+                                          &None, &None).unwrap();
         let version = Version::insert(&tx, krate.id,
                                       &semver::Version::parse("1.0.0").unwrap(),
                                       &HashMap::new(), &[]).unwrap();

src/krate.rs

@@ -94,18 +94,34 @@ impl Crate {
                           readme: &Option<String>,
                           keywords: &[String],
                           repository: &Option<String>,
-                          license: &Option<String>) -> CargoResult<Crate> {
+                          license: &Option<String>,
+                          license_file: &Option<String>) -> CargoResult<Crate> {
         let description = description.as_ref().map(|s| s.as_slice());
         let homepage = homepage.as_ref().map(|s| s.as_slice());
         let documentation = documentation.as_ref().map(|s| s.as_slice());
         let readme = readme.as_ref().map(|s| s.as_slice());
         let repository = repository.as_ref().map(|s| s.as_slice());
-        let license = license.as_ref().map(|s| s.as_slice());
+        let mut license = license.as_ref().map(|s| s.as_slice());
+        let license_file = license_file.as_ref().map(|s| s.as_slice());
         let keywords = keywords.connect(",");
         try!(validate_url(homepage));
         try!(validate_url(documentation));
         try!(validate_url(repository));
-        try!(validate_license(license));
+
+        match license {
+            // If a license is given, validate it to make sure it's actually a
+            // valid license
+            Some(..) => try!(validate_license(license)),
+
+            // If no license is given, but a license file is given, flag this
+            // crate as having a nonstandard license. Note that we don't
+            // actually do anything else with license_file currently.
+            None if license_file.is_some() => {
+                license = Some("non-standard");
+            }
+
+            None => {}
+        }
 
         // TODO: like with users, this is sadly racy
         let stmt = try!(conn.prepare("UPDATE crates
@@ -598,7 +614,8 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
                                                &new_crate.readme,
                                                keywords.as_slice(),
                                                &new_crate.repository,
-                                               &new_crate.license));
+                                               &new_crate.license,
+                                               &new_crate.license_file));
     if krate.user_id != user.id {
         let owners = try!(krate.owners(try!(req.tx())));
         if !owners.iter().any(|o| o.id == user.id) {
@@ -705,6 +722,25 @@ fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)>
         human(format!("invalid upload request: {}", e))
     }));
 
+    // Make sure required fields are provided
+    fn empty(s: Option<&String>) -> bool { s.map_or(true, |s| s.is_empty()) }
+    let mut missing = Vec::new();
+
+    if empty(new.description.as_ref()) {
+        missing.push("description");
+    }
+    if empty(new.license.as_ref()) && empty(new.license_file.as_ref()) {
+        missing.push("license");
+    }
+    if new.authors.len() == 0 || new.authors.iter().all(|s| s.is_empty()) {
+        missing.push("authors");
+    }
+    if missing.len() > 0 {
+        return Err(human(format!("missing or empty metadata fields: {}. Please \
+            see http://doc.crates.io/manifest.html#package-metadata for \
+            how to upload metadata", missing.connect(", "))));
+    }
+
     let user = try!(req.user());
     Ok((new, user.clone()))
 }

src/tests/all.rs

@@ -228,7 +228,8 @@ fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
                                       &krate.readme,
                                       krate.keywords.as_slice(),
                                       &krate.repository,
-                                      &krate.license).unwrap();
+                                      &krate.license,
+                                      &None).unwrap();
     Keyword::update_crate(req.tx().unwrap(), &krate,
                           krate.keywords.as_slice()).unwrap();
     let v = krate.add_version(req.tx().unwrap(), v, &HashMap::new(), &[]);

src/tests/krate.rs

@@ -157,21 +157,25 @@ fn new_req_full(app: Arc<App>, krate: Crate, version: &str,
 fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>)
                 -> Vec<u8> {
     let kws = krate.keywords.into_iter().map(u::Keyword).collect();
-    let json = u::NewCrate {
+    new_crate_to_body(&u::NewCrate {
         name: u::CrateName(krate.name),
         vers: u::CrateVersion(semver::Version::parse(version).unwrap()),
         features: HashMap::new(),
         deps: deps,
-        authors: Vec::new(),
-        description: krate.description,
+        authors: vec!["foo".to_string()],
+        description: Some("description".to_string()),
         homepage: krate.homepage,
         documentation: krate.documentation,
         readme: krate.readme,
         keywords: Some(u::KeywordList(kws)),
-        license: krate.license,
+        license: Some("MIT".to_string()),
+        license_file: None,
         repository: krate.repository,
-    };
-    let json = json::encode(&json);
+    })
+}
+
+fn new_crate_to_body(new_crate: &u::NewCrate) -> Vec<u8> {
+    let json = json::encode(&new_crate);
     let mut body = MemWriter::new();
     body.write_le_u32(json.len() as u32).unwrap();
     body.write_str(json.as_slice()).unwrap();
@@ -746,3 +750,51 @@ fn reverse_dependencies() {
     assert_eq!(deps.meta.total, 0);
     drop(req);
 }
+
+#[test]
+fn author_license_and_description_required() {
+    let (_b, app, middle) = ::app();
+    ::user("foo");
+
+    let mut req = ::req(app, conduit::Put, "/api/v1/crates/new");
+    let mut new_crate = u::NewCrate {
+        name: u::CrateName("foo".to_string()),
+        vers: u::CrateVersion(semver::Version::parse("1.0.0").unwrap()),
+        features: HashMap::new(),
+        deps: Vec::new(),
+        authors: Vec::new(),
+        description: None,
+        homepage: None,
+        documentation: None,
+        readme: None,
+        keywords: None,
+        license: None,
+        license_file: None,
+        repository: None,
+    };
+    req.with_body(new_crate_to_body(&new_crate));
+    let json = bad_resp!(middle.call(&mut req));
+    assert!(json.errors[0].detail.as_slice().contains("author") &&
+            json.errors[0].detail.as_slice().contains("description") &&
+            json.errors[0].detail.as_slice().contains("license"),
+            "{}", json.errors);
+
+    new_crate.license = Some("MIT".to_string());
+    new_crate.authors.push("".to_string());
+    req.with_body(new_crate_to_body(&new_crate));
+    let json = bad_resp!(middle.call(&mut req));
+    assert!(json.errors[0].detail.as_slice().contains("author") &&
+            json.errors[0].detail.as_slice().contains("description") &&
+            !json.errors[0].detail.as_slice().contains("license"),
+            "{}", json.errors);
+
+    new_crate.license = None;
+    new_crate.license_file = Some("foo".to_string());
+    new_crate.authors.push("foo".to_string());
+    req.with_body(new_crate_to_body(&new_crate));
+    let json = bad_resp!(middle.call(&mut req));
+    assert!(!json.errors[0].detail.as_slice().contains("author") &&
+            json.errors[0].detail.as_slice().contains("description") &&
+            !json.errors[0].detail.as_slice().contains("license"),
+            "{}", json.errors);
+}

src/upload.rs

@@ -20,6 +20,7 @@ pub struct NewCrate {
     pub readme: Option<String>,
     pub keywords: Option<KeywordList>,
     pub license: Option<String>,
+    pub license_file: Option<String>,
     pub repository: Option<String>,
 }