models/version: Move license validation into publish endpoint

Author: Turbo87

src/controllers/krate/publish.rs

@@ -19,6 +19,7 @@ use crate::models::{
 
 use crate::middleware::log_request::RequestLogExt;
 use crate::models::token::EndpointScope;
+use crate::models::version::validate_license_expr;
 use crate::rate_limiter::LimitedAction;
 use crate::schema::*;
 use crate::sql::canon_crate_name;
@@ -115,7 +116,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
         let package = tarball_info.manifest.package.unwrap();
 
         let description = package.description.map(|it| it.as_local().unwrap());
-        let license = package.license.map(|it| it.as_local().unwrap());
+        let mut license = package.license.map(|it| it.as_local().unwrap());
         let license_file = package.license_file.map(|it| it.as_local().unwrap());
 
         // Make sure required fields are provided
@@ -136,6 +137,15 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             return Err(cargo_err(&message));
         }
 
+        if let Some(ref license) = license {
+            validate_license_expr(license)?;
+        } else if license_file.is_some() {
+            // 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.
+            license = Some(String::from("non-standard"));
+        }
+
         // Create a transaction on the database, if there are no errors,
         // commit the transactions to record a new or updated crate.
         conn.transaction(|conn| {

src/models/version.rs

@@ -139,7 +139,7 @@ impl NewVersion {
         num: &semver::Version,
         features: &BTreeMap<String, Vec<String>>,
         license: Option<String>,
-        license_file: Option<&str>,
+        #[allow(unused_variables)] license_file: Option<&str>,
         crate_size: i32,
         published_by: i32,
         checksum: String,
@@ -148,7 +148,7 @@ impl NewVersion {
     ) -> AppResult<Self> {
         let features = serde_json::to_value(features)?;
 
-        let mut new_version = NewVersion {
+        Ok(NewVersion {
             crate_id,
             num: num.to_string(),
             features,
@@ -158,11 +158,7 @@ impl NewVersion {
             checksum,
             links,
             rust_version,
-        };
-
-        new_version.validate_license(license_file)?;
-
-        Ok(new_version)
+        })
     }
 
     pub fn save(&self, conn: &mut PgConnection, published_by_email: &str) -> AppResult<Version> {
@@ -195,18 +191,6 @@ impl NewVersion {
             Ok(version)
         })
     }
-
-    fn validate_license(&mut self, license_file: Option<&str>) -> AppResult<()> {
-        if let Some(ref license) = self.license {
-            validate_license_expr(license)?;
-        } else if license_file.is_some() {
-            // 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.
-            self.license = Some(String::from("non-standard"));
-        }
-        Ok(())
-    }
 }
 
 pub fn validate_license_expr(s: &str) -> AppResult<()> {