publish: Move feature name validation to validate_dependencies() fn

Author: Turbo87

src/controllers/krate/publish.rs

@@ -443,6 +443,14 @@ pub fn validate_dependencies(deps: &[EncodableCrateDependency]) -> AppResult<()>
             )));
         }
 
+        for feature in &dep.features {
+            if !Crate::valid_feature(feature) {
+                return Err(cargo_err(&format_args!(
+                    "\"{feature}\" is an invalid feature name",
+                )));
+            }
+        }
+
         if let Some(registry) = &dep.registry {
             if !registry.is_empty() {
                 return Err(cargo_err(&format_args!("Dependency `{}` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.", dep.name)));

src/tests/builders/dependency.rs

@@ -1,5 +1,4 @@
 use crates_io::views::krate_publish as u;
-use crates_io::views::krate_publish::EncodableFeature;
 
 /// A builder for constructing a dependency of another crate.
 pub struct DependencyBuilder {
@@ -53,17 +52,11 @@ impl DependencyBuilder {
     /// Consume this builder to create a `u::CrateDependency`. If the dependent crate doesn't
     /// already exist, publishing a crate with this dependency will fail.
     pub fn build(self) -> u::EncodableCrateDependency {
-        let features = self
-            .features
-            .into_iter()
-            .map(|d| EncodableFeature(d))
-            .collect();
-
         u::EncodableCrateDependency {
             name: self.name,
             optional: false,
             default_features: true,
-            features,
+            features: self.features,
             version_req: self.version_req,
             target: None,
             kind: None,

src/tests/builders/publish.rs

@@ -236,17 +236,10 @@ fn convert_dependency(
         ),
     };
 
-    let features = encoded
-        .features
-        .iter()
-        .map(|f| f.to_string())
-        .collect::<Vec<_>>()
-        .none_or_filled();
-
     let dependency = DependencyDetail {
         version: Some(encoded.version_req.to_string()),
         registry: encoded.registry.clone(),
-        features,
+        features: encoded.features.clone().none_or_filled(),
         optional: match encoded.optional {
             true => Some(true),
             false => None,

src/tests/krate/publish/snapshots/all__krate__publish__dependencies__invalid_feature_name.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"🍺\", expected a valid feature name at line 1 column 111"
+      "detail": "\"🍺\" is an invalid feature name"
     }
   ]
 }

src/views/krate_publish.rs

@@ -3,9 +3,6 @@
 //! to and from structs. The serializing is only utilised in
 //! integration tests.
 
-use diesel::pg::Pg;
-use diesel::serialize::{self, Output, ToSql};
-use diesel::sql_types::Text;
 use serde::{de, Deserialize, Deserializer, Serialize};
 
 use crate::models::krate::MAX_NAME_LENGTH;
@@ -27,7 +24,7 @@ pub struct EncodableCrateDependency {
     pub optional: bool,
     pub default_features: bool,
     pub name: String,
-    pub features: Vec<EncodableFeature>,
+    pub features: Vec<String>,
     pub version_req: EncodableCrateVersionReq,
     pub target: Option<String>,
     pub kind: Option<DependencyKind>,
@@ -73,28 +70,6 @@ impl<'de> Deserialize<'de> for EncodableDependencyName {
     }
 }
 
-#[derive(Serialize, Clone, Debug, Deref)]
-pub struct EncodableFeature(pub String);
-
-impl<'de> Deserialize<'de> for EncodableFeature {
-    fn deserialize<D: Deserializer<'de>>(d: D) -> Result<EncodableFeature, D::Error> {
-        let s = String::deserialize(d)?;
-        if !Crate::valid_feature(&s) {
-            let value = de::Unexpected::Str(&s);
-            let expected = "a valid feature name";
-            Err(de::Error::invalid_value(value, &expected))
-        } else {
-            Ok(EncodableFeature(s))
-        }
-    }
-}
-
-impl ToSql<Text, Pg> for EncodableFeature {
-    fn to_sql(&self, out: &mut Output<'_, '_, Pg>) -> serialize::Result {
-        ToSql::<Text, Pg>::to_sql(&**self, &mut out.reborrow())
-    }
-}
-
 #[derive(Serialize, Debug, Deref)]
 pub struct EncodableCrateVersion(pub semver::Version);
 
@@ -128,15 +103,3 @@ impl<'de> Deserialize<'de> for EncodableCrateVersionReq {
         }
     }
 }
-
-#[test]
-fn feature_deserializes_for_valid_features() {
-    use serde_json as json;
-
-    assert_ok!(json::from_str::<EncodableFeature>("\"foo\""));
-    assert_err!(json::from_str::<EncodableFeature>("\"\""));
-    assert_err!(json::from_str::<EncodableFeature>("\"/\""));
-    assert_err!(json::from_str::<EncodableFeature>("\"%/%\""));
-    assert_ok!(json::from_str::<EncodableFeature>("\"a/a\""));
-    assert_ok!(json::from_str::<EncodableFeature>("\"32-column-tables\""));
-}