Auto merge of #3178 - Turbo87:encodable-version, r=pietroalbini

Version|Crate: Replace `encodable()` methods

This PR is similar to #3168 and brings us one step closer to removing the `models -> views` dependency by inverting the relationship for the `Version` model and `EncodableVersion` view.

r? `@pietroalbini`

Author: bors

src/controllers/krate/metadata.rs

@@ -41,7 +41,13 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult {
             .zip(krates)
             .zip(recent_downloads)
             .map(|((top_versions, krate), recent_downloads)| {
-                Ok(krate.minimal_encodable(&top_versions, None, false, recent_downloads))
+                Ok(EncodableCrate::from_minimal(
+                    krate,
+                    &top_versions,
+                    None,
+                    false,
+                    recent_downloads,
+                ))
             })
             .collect()
     };
@@ -165,7 +171,8 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
         categories: Vec<EncodableCategory>,
     }
     Ok(req.json(&R {
-        krate: krate.clone().encodable(
+        krate: EncodableCrate::from(
+            krate.clone(),
             &top_versions,
             Some(ids),
             Some(&kws),
@@ -176,7 +183,7 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
         ),
         versions: versions_publishers_and_audit_actions
             .into_iter()
-            .map(|(v, pb, aas)| v.encodable(&krate.name, pb, aas))
+            .map(|(v, pb, aas)| EncodableVersion::from(v, &krate.name, pb, aas))
             .collect(),
         keywords: kws.into_iter().map(Keyword::into).collect(),
         categories: cats.into_iter().map(Category::into).collect(),
@@ -226,7 +233,7 @@ pub fn versions(req: &mut dyn RequestExt) -> EndpointResult {
     let versions = versions_and_publishers
         .into_iter()
         .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter())
-        .map(|((v, pb), aas)| v.encodable(crate_name, pb, aas))
+        .map(|((v, pb), aas)| EncodableVersion::from(v, crate_name, pb, aas))
         .collect();
 
     #[derive(Serialize)]
@@ -271,7 +278,7 @@ pub fn reverse_dependencies(req: &mut dyn RequestExt) -> EndpointResult {
         .into_iter()
         .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter())
         .map(|((version, krate_name, published_by), actions)| {
-            version.encodable(&krate_name, published_by, actions)
+            EncodableVersion::from(version, &krate_name, published_by, actions)
         })
         .collect();
 

src/controllers/krate/publish.rs

@@ -14,7 +14,7 @@ use crate::models::{
 
 use crate::render;
 use crate::util::{read_fill, read_le_u32, Maximums};
-use crate::views::{EncodableCrateUpload, GoodCrate, PublishWarnings};
+use crate::views::{EncodableCrate, EncodableCrateUpload, GoodCrate, PublishWarnings};
 
 pub const MISSING_RIGHTS_ERROR_MESSAGE: &str =
     "this crate exists but you don't seem to be an owner. \
@@ -218,7 +218,7 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
         };
 
         Ok(req.json(&GoodCrate {
-            krate: krate.minimal_encodable(&top_versions, None, false, None),
+            krate: EncodableCrate::from_minimal(krate, &top_versions, None, false, None),
             warnings,
         }))
     })

src/controllers/krate/search.rs

@@ -226,7 +226,8 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
         .zip(badges)
         .map(
             |((((max_version, krate), perfect_match), recent_downloads), badges)| {
-                krate.minimal_encodable(
+                EncodableCrate::from_minimal(
+                    krate,
                     &max_version,
                     Some(badges),
                     perfect_match,

src/controllers/user/me.rs

@@ -83,7 +83,7 @@ pub fn updates(req: &mut dyn RequestExt) -> EndpointResult {
     let versions = data
         .into_iter()
         .map(|(version, crate_name, published_by, actions)| {
-            version.encodable(&crate_name, published_by, actions)
+            EncodableVersion::from(version, &crate_name, published_by, actions)
         })
         .collect();
 

src/controllers/version/deprecated.rs

@@ -41,7 +41,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
         .into_iter()
         .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter())
         .map(|((version, crate_name, published_by), actions)| {
-            version.encodable(&crate_name, published_by, actions)
+            EncodableVersion::from(version, &crate_name, published_by, actions)
         })
         .collect();
 
@@ -76,6 +76,6 @@ pub fn show_by_id(req: &mut dyn RequestExt) -> EndpointResult {
         version: EncodableVersion,
     }
     Ok(req.json(&R {
-        version: version.encodable(&krate.name, published_by, audit_actions),
+        version: EncodableVersion::from(version, &krate.name, published_by, audit_actions),
     }))
 }

src/controllers/version/metadata.rs

@@ -81,6 +81,6 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
         version: EncodableVersion,
     }
     Ok(req.json(&R {
-        version: version.encodable(&krate.name, published_by, actions),
+        version: EncodableVersion::from(version, &krate.name, published_by, actions),
     }))
 }

src/models/krate.rs

@@ -10,20 +10,15 @@ use crate::controllers::helpers::pagination::*;
 use crate::email;
 use crate::models::version::TopVersions;
 use crate::models::{
-    Badge, Category, CrateOwner, CrateOwnerInvitation, Keyword, NewCrateOwnerInvitation, Owner,
-    OwnerKind, ReverseDependency, User, Version,
+    Badge, CrateOwner, CrateOwnerInvitation, NewCrateOwnerInvitation, Owner, OwnerKind,
+    ReverseDependency, User, Version,
 };
 use crate::util::errors::{cargo_err, AppResult};
-use crate::views::{EncodableCrate, EncodableCrateLinks};
 
 use crate::models::helpers::with_count::*;
 use crate::publish_rate_limit::PublishRateLimit;
 use crate::schema::*;
 
-/// Hosts in this list are known to not be hosting documentation,
-/// and are possibly of malicious intent e.g. ad tracking networks, etc.
-const DOCUMENTATION_BLOCKLIST: [&str; 2] = ["rust-ci.org", "rustless.org"];
-
 #[derive(Debug, Queryable, Identifiable, Associations, Clone, Copy)]
 #[belongs_to(Crate)]
 #[primary_key(crate_id)]
@@ -296,127 +291,6 @@ impl Crate {
             && prefix_part.map_or(true, Crate::valid_feature_prefix)
     }
 
-    pub fn minimal_encodable(
-        self,
-        top_versions: &TopVersions,
-        badges: Option<Vec<Badge>>,
-        exact_match: bool,
-        recent_downloads: Option<i64>,
-    ) -> EncodableCrate {
-        self.encodable(
-            top_versions,
-            None,
-            None,
-            None,
-            badges,
-            exact_match,
-            recent_downloads,
-        )
-    }
-
-    #[allow(clippy::too_many_arguments)]
-    pub fn encodable(
-        self,
-        top_versions: &TopVersions,
-        versions: Option<Vec<i32>>,
-        keywords: Option<&[Keyword]>,
-        categories: Option<&[Category]>,
-        badges: Option<Vec<Badge>>,
-        exact_match: bool,
-        recent_downloads: Option<i64>,
-    ) -> EncodableCrate {
-        let Crate {
-            name,
-            created_at,
-            updated_at,
-            downloads,
-            description,
-            homepage,
-            documentation,
-            repository,
-            ..
-        } = self;
-        let versions_link = match versions {
-            Some(..) => None,
-            None => Some(format!("/api/v1/crates/{}/versions", name)),
-        };
-        let keyword_ids = keywords.map(|kws| kws.iter().map(|kw| kw.keyword.clone()).collect());
-        let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.slug.clone()).collect());
-        let badges = badges.map(|bs| bs.into_iter().map(Badge::into).collect());
-        let documentation = Crate::remove_blocked_documentation_urls(documentation);
-
-        let max_version = top_versions
-            .highest
-            .as_ref()
-            .map(|v| v.to_string())
-            .unwrap_or_else(|| "0.0.0".to_string());
-
-        let newest_version = top_versions
-            .newest
-            .as_ref()
-            .map(|v| v.to_string())
-            .unwrap_or_else(|| "0.0.0".to_string());
-
-        let max_stable_version = top_versions.highest_stable.as_ref().map(|v| v.to_string());
-
-        EncodableCrate {
-            id: name.clone(),
-            name: name.clone(),
-            updated_at,
-            created_at,
-            downloads,
-            recent_downloads,
-            versions,
-            keywords: keyword_ids,
-            categories: category_ids,
-            badges,
-            max_version,
-            newest_version,
-            max_stable_version,
-            documentation,
-            homepage,
-            exact_match,
-            description,
-            repository,
-            links: EncodableCrateLinks {
-                version_downloads: format!("/api/v1/crates/{}/downloads", name),
-                versions: versions_link,
-                owners: Some(format!("/api/v1/crates/{}/owners", name)),
-                owner_team: Some(format!("/api/v1/crates/{}/owner_team", name)),
-                owner_user: Some(format!("/api/v1/crates/{}/owner_user", name)),
-                reverse_dependencies: format!("/api/v1/crates/{}/reverse_dependencies", name),
-            },
-        }
-    }
-
-    /// Return `None` if the documentation URL host matches a blocked host
-    fn remove_blocked_documentation_urls(url: Option<String>) -> Option<String> {
-        // Handles if documentation URL is None
-        let url = match url {
-            Some(url) => url,
-            None => return None,
-        };
-
-        // Handles unsuccessful parsing of documentation URL
-        let parsed_url = match Url::parse(&url) {
-            Ok(parsed_url) => parsed_url,
-            Err(_) => return None,
-        };
-
-        // Extract host string from documentation URL
-        let url_host = match parsed_url.host_str() {
-            Some(url_host) => url_host,
-            None => return None,
-        };
-
-        // Match documentation URL host against blocked host array elements
-        if DOCUMENTATION_BLOCKLIST.contains(&url_host) {
-            None
-        } else {
-            Some(url)
-        }
-    }
-
     /// Return both the newest (most recently updated) and
     /// highest version (in semver order) for the current crate.
     pub fn top_versions(&self, conn: &PgConnection) -> QueryResult<TopVersions> {
@@ -576,39 +450,6 @@ mod tests {
         assert_err!(krate.validate());
     }
 
-    #[test]
-    fn documentation_blocked_no_url_provided() {
-        assert_eq!(Crate::remove_blocked_documentation_urls(None), None);
-    }
-
-    #[test]
-    fn documentation_blocked_invalid_url() {
-        assert_eq!(
-            Crate::remove_blocked_documentation_urls(Some(String::from("not a url"))),
-            None
-        );
-    }
-
-    #[test]
-    fn documentation_blocked_url_contains_partial_match() {
-        assert_eq!(
-            Crate::remove_blocked_documentation_urls(Some(String::from(
-                "http://rust-ci.organists.com"
-            )),),
-            Some(String::from("http://rust-ci.organists.com"))
-        );
-    }
-
-    #[test]
-    fn documentation_blocked_url() {
-        assert_eq!(
-            Crate::remove_blocked_documentation_urls(Some(String::from(
-                "http://rust-ci.org/crate/crate-0.1/doc/crate-0.1",
-            ),),),
-            None
-        );
-    }
-
     #[test]
     fn valid_name() {
         assert!(Crate::valid_name("foo"));

src/models/version.rs

@@ -5,9 +5,8 @@ use diesel::prelude::*;
 
 use crate::util::errors::{cargo_err, AppResult};
 
-use crate::models::{Crate, Dependency, User, VersionOwnerAction};
+use crate::models::{Crate, Dependency, User};
 use crate::schema::*;
-use crate::views::{EncodableAuditAction, EncodableVersion, EncodableVersionLinks};
 
 // Queryable has a custom implementation below
 #[derive(Clone, Identifiable, Associations, Debug, Queryable, Deserialize, Serialize)]
@@ -80,55 +79,6 @@ impl TopVersions {
 }
 
 impl Version {
-    pub fn encodable(
-        self,
-        crate_name: &str,
-        published_by: Option<User>,
-        audit_actions: Vec<(VersionOwnerAction, User)>,
-    ) -> EncodableVersion {
-        let Version {
-            id,
-            num,
-            updated_at,
-            created_at,
-            downloads,
-            features,
-            yanked,
-            license,
-            crate_size,
-            ..
-        } = self;
-        let num = num.to_string();
-        EncodableVersion {
-            dl_path: format!("/api/v1/crates/{}/{}/download", crate_name, num),
-            readme_path: format!("/api/v1/crates/{}/{}/readme", crate_name, num),
-            num: num.clone(),
-            id,
-            krate: crate_name.to_string(),
-            updated_at,
-            created_at,
-            downloads,
-            features,
-            yanked,
-            license,
-            links: EncodableVersionLinks {
-                dependencies: format!("/api/v1/crates/{}/{}/dependencies", crate_name, num),
-                version_downloads: format!("/api/v1/crates/{}/{}/downloads", crate_name, num),
-                authors: format!("/api/v1/crates/{}/{}/authors", crate_name, num),
-            },
-            crate_size,
-            published_by: published_by.map(User::into),
-            audit_actions: audit_actions
-                .into_iter()
-                .map(|(audit_action, user)| EncodableAuditAction {
-                    action: audit_action.action.into(),
-                    user: user.into(),
-                    time: audit_action.time,
-                })
-                .collect(),
-        }
-    }
-
     /// Returns (dependency, crate dependency name)
     pub fn dependencies(&self, conn: &PgConnection) -> QueryResult<Vec<(Dependency, String)>> {
         Dependency::belonging_to(self)