From 2df55efcfde3541e83d0ebe2647a60cb6b266b5c Mon Sep 17 00:00:00 2001
From: Ales Musil <amusil@redhat.com>
Date: Tue, 27 Oct 2020 11:47:21 +0100
Subject: [PATCH 1/4] frontend: Show feature flags

So far features were stored only in database.
Add link to the topbar which will lead to the
new features page. Features page will show
all relevant features with their subfeatures.
---
 src/db/types.rs                          |  6 +-
 src/test/fakes.rs                        |  9 +++
 src/web/crate_details.rs                 | 78 ++++++++++++++++++++++++
 src/web/features.rs                      | 30 +++++++++
 src/web/mod.rs                           | 19 +++++-
 src/web/routes.rs                        |  4 ++
 src/web/source.rs                        |  4 +-
 templates/crate/features.html            | 76 +++++++++++++++++++++++
 templates/header/package_navigation.html | 10 +++
 templates/rustdoc/topbar.html            |  8 +++
 10 files changed, 241 insertions(+), 3 deletions(-)
 create mode 100644 src/web/features.rs
 create mode 100644 templates/crate/features.html

diff --git a/src/db/types.rs b/src/db/types.rs
index e2a973676..eb80c473f 100644
--- a/src/db/types.rs
+++ b/src/db/types.rs
@@ -1,7 +1,7 @@
 use postgres_types::{FromSql, ToSql};
 use serde::Serialize;
 
-#[derive(Debug, Clone, PartialEq, Serialize, FromSql, ToSql)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, FromSql, ToSql)]
 #[postgres(name = "feature")]
 pub struct Feature {
     name: String,
@@ -12,4 +12,8 @@ impl Feature {
     pub fn new(name: String, subfeatures: Vec<String>) -> Self {
         Feature { name, subfeatures }
     }
+
+    pub fn is_private(&self) -> bool {
+        self.name.starts_with('_')
+    }
 }
diff --git a/src/test/fakes.rs b/src/test/fakes.rs
index 6344e60ec..dafae007c 100644
--- a/src/test/fakes.rs
+++ b/src/test/fakes.rs
@@ -210,6 +210,15 @@ impl<'a> FakeRelease<'a> {
         self
     }
 
+    pub(crate) fn features(mut self, opt_features: Option<HashMap<String, Vec<String>>>) -> Self {
+        if let Some(features) = opt_features {
+            self.package.features = features;
+        } else {
+            self.package.features = HashMap::new();
+        }
+        self
+    }
+
     /// Returns the release_id
     pub(crate) fn create(self) -> Result<i32, Error> {
         use std::fs;
diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs
index 9dfae8930..6d874443b 100644
--- a/src/web/crate_details.rs
+++ b/src/web/crate_details.rs
@@ -103,6 +103,7 @@ impl CrateDetails {
                 releases.license,
                 releases.documentation_url,
                 releases.default_target,
+                releases.features,
                 doc_coverage.total_items,
                 doc_coverage.documented_items,
                 doc_coverage.total_items_needing_examples,
@@ -148,6 +149,7 @@ impl CrateDetails {
             default_target: krate.get("default_target"),
             doc_targets: MetaData::parse_doc_targets(krate.get("doc_targets")),
             yanked: krate.get("yanked"),
+            features: MetaData::parse_features(krate.get("features")),
         };
 
         let documented_items: Option<i32> = krate.get("documented_items");
@@ -325,6 +327,7 @@ mod tests {
     use crate::test::{wrapper, TestDatabase};
     use failure::Error;
     use kuchiki::traits::TendrilSink;
+    use std::collections::HashMap;
 
     fn assert_last_successful_build_equals(
         db: &TestDatabase,
@@ -741,4 +744,79 @@ mod tests {
             Ok(())
         });
     }
+
+    #[test]
+    fn feature_flags_report_empty() {
+        wrapper(|env| {
+            env.fake_release()
+                .name("library")
+                .version("0.1.0")
+                .binary(false)
+                .features(None)
+                .create()?;
+
+            let page = kuchiki::parse_html().one(
+                env.frontend()
+                    .get("/crate/library/0.1.0/features")
+                    .send()?
+                    .text()?,
+            );
+            assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_ok());
+            Ok(())
+        });
+    }
+
+    #[test]
+    fn feature_private_feature_flags_are_hidden() {
+        wrapper(|env| {
+            let features = [("_private".into(), Vec::new())]
+                .iter()
+                .cloned()
+                .collect::<HashMap<String, Vec<String>>>();
+            env.fake_release()
+                .name("library")
+                .version("0.1.0")
+                .binary(false)
+                .features(Some(features))
+                .create()?;
+
+            let page = kuchiki::parse_html().one(
+                env.frontend()
+                    .get("/crate/library/0.1.0/features")
+                    .send()?
+                    .text()?,
+            );
+            assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_ok());
+            Ok(())
+        });
+    }
+
+    #[test]
+    fn feature_flags_without_default() {
+        wrapper(|env| {
+            let features = [("feature1".into(), Vec::new())]
+                .iter()
+                .cloned()
+                .collect::<HashMap<String, Vec<String>>>();
+            env.fake_release()
+                .name("library")
+                .version("0.1.0")
+                .binary(false)
+                .features(Some(features))
+                .create()?;
+
+            let page = kuchiki::parse_html().one(
+                env.frontend()
+                    .get("/crate/library/0.1.0/features")
+                    .send()?
+                    .text()?,
+            );
+            assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_err());
+            let def_len = page
+                .select_first(r#"b[data-id="default-feature-len"]"#)
+                .unwrap();
+            assert_eq!(def_len.text_contents(), "0");
+            Ok(())
+        });
+    }
 }
diff --git a/src/web/features.rs b/src/web/features.rs
new file mode 100644
index 000000000..5155a59f3
--- /dev/null
+++ b/src/web/features.rs
@@ -0,0 +1,30 @@
+use crate::{
+    db::Pool,
+    impl_webpage,
+    web::{page::WebPage, MetaData},
+};
+use iron::{IronResult, Request, Response};
+use router::Router;
+use serde::Serialize;
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
+struct FeaturesPage {
+    metadata: MetaData,
+}
+
+impl_webpage! {
+    FeaturesPage = "crate/features.html",
+}
+
+pub fn build_features_handler(req: &mut Request) -> IronResult<Response> {
+    let router = extension!(req, Router);
+    let name = cexpect!(req, router.find("name"));
+    let version = cexpect!(req, router.find("version"));
+
+    let mut conn = extension!(req, Pool).get()?;
+
+    FeaturesPage {
+        metadata: cexpect!(req, MetaData::from_crate(&mut conn, &name, &version)),
+    }
+    .into_response(req)
+}
diff --git a/src/web/mod.rs b/src/web/mod.rs
index 7e26847e2..db8408a8c 100644
--- a/src/web/mod.rs
+++ b/src/web/mod.rs
@@ -81,6 +81,7 @@ mod builds;
 mod crate_details;
 mod error;
 mod extensions;
+mod features;
 mod file;
 pub(crate) mod metrics;
 mod releases;
@@ -90,6 +91,7 @@ mod sitemap;
 mod source;
 mod statics;
 
+use crate::db::types::Feature;
 use crate::{impl_webpage, Context};
 use chrono::{DateTime, Utc};
 use error::Nope;
@@ -520,6 +522,7 @@ pub(crate) struct MetaData {
     pub(crate) default_target: String,
     pub(crate) doc_targets: Vec<String>,
     pub(crate) yanked: bool,
+    pub(crate) features: Option<Vec<Feature>>,
 }
 
 impl MetaData {
@@ -533,7 +536,8 @@ impl MetaData {
                        releases.rustdoc_status,
                        releases.default_target,
                        releases.doc_targets,
-                       releases.yanked
+                       releases.yanked,
+                       releases.features
                 FROM releases
                 INNER JOIN crates ON crates.id = releases.crate_id
                 WHERE crates.name = $1 AND releases.version = $2",
@@ -552,6 +556,7 @@ impl MetaData {
             default_target: row.get(5),
             doc_targets: MetaData::parse_doc_targets(row.get(6)),
             yanked: row.get(7),
+            features: MetaData::parse_features(row.get(8)),
         })
     }
 
@@ -566,6 +571,14 @@ impl MetaData {
             })
             .unwrap_or_else(Vec::new)
     }
+
+    pub(crate) fn parse_features(features: Option<Vec<Feature>>) -> Option<Vec<Feature>> {
+        features.map(|vec| {
+            vec.into_iter()
+                .filter(|feature| !feature.is_private())
+                .collect()
+        })
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize)]
@@ -844,6 +857,7 @@ mod test {
                 "arm64-unknown-linux-gnu".to_string(),
             ],
             yanked: false,
+            features: None,
         };
 
         let correct_json = json!({
@@ -858,6 +872,7 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
+            "features": null
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
@@ -875,6 +890,7 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
+            "features": null,
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
@@ -892,6 +908,7 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
+            "features": null,
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
diff --git a/src/web/routes.rs b/src/web/routes.rs
index 71c8b171e..44b808471 100644
--- a/src/web/routes.rs
+++ b/src/web/routes.rs
@@ -87,6 +87,10 @@ pub(super) fn build_routes() -> Routes {
         "/crate/:name/:version/builds/:id",
         super::builds::build_list_handler,
     );
+    routes.internal_page(
+        "/crate/:name/:version/features",
+        super::features::build_features_handler,
+    );
     routes.internal_page(
         "/crate/:name/:version/source",
         SimpleRedirect::new(|url| url.set_path(&format!("{}/", url.path()))),
diff --git a/src/web/source.rs b/src/web/source.rs
index 30bb48fa7..a88dea7ac 100644
--- a/src/web/source.rs
+++ b/src/web/source.rs
@@ -58,7 +58,8 @@ impl FileList {
                         releases.files,
                         releases.default_target,
                         releases.doc_targets,
-                        releases.yanked
+                        releases.yanked,
+                        releases.features
                 FROM releases
                 LEFT OUTER JOIN crates ON crates.id = releases.crate_id
                 WHERE crates.name = $1 AND releases.version = $2",
@@ -137,6 +138,7 @@ impl FileList {
                     default_target: rows[0].get(6),
                     doc_targets: MetaData::parse_doc_targets(rows[0].get(7)),
                     yanked: rows[0].get(8),
+                    features: MetaData::parse_features(rows[0].get(9)),
                 },
                 files: file_list,
             })
diff --git a/templates/crate/features.html b/templates/crate/features.html
new file mode 100644
index 000000000..3dc5319c8
--- /dev/null
+++ b/templates/crate/features.html
@@ -0,0 +1,76 @@
+{%- extends "base.html" -%}
+{%- import "header/package_navigation.html" as navigation -%}
+
+{%- block title -%}
+    {{ macros::doc_title(name=metadata.name, version=metadata.version) }}
+{%- endblock title -%}
+
+{%- block topbar -%}
+  {%- set latest_version = "" -%}
+  {%- set latest_path = "" -%}
+  {%- set target = "" -%}
+  {%- set inner_path = metadata.target_name ~ "/index.html" -%}
+  {%- set is_latest_version = true -%}
+  {%- set is_prerelease = false -%}
+  {%- include "rustdoc/topbar.html" -%}
+{%- endblock topbar -%}
+
+{%- block header -%}
+    {{ navigation::package_navigation(metadata=metadata, active_tab="features") }}
+{%- endblock header -%}
+
+{%- block body -%}
+    <div class="container package-page-container">
+        <div class="pure-g">
+            <div class="pure-u-1 pure-u-sm-7-24 pure-u-md-5-24">
+                <div class="pure-menu package-menu">
+                    <ul class="pure-menu-list">
+                        <li class="pure-menu-heading">Feature flags</li>
+                        {%- if metadata.features -%}
+                            {%- for feature in metadata.features -%}
+                                <li class="pure-menu-item">
+                                    <a href="#{{ feature.name }}" class="pure-menu-link" style="text-align:center;">
+                                        {{ feature.name }}
+                                    </a>
+                                </li>
+                            {%- endfor -%}
+                        {%- else -%}
+                            <li class="pure-menu-item">
+                                <span style="font-size: 13px;">Feature flags are not available for this version.</span>
+                            </li>
+                        {%- endif -%}
+                    </ul>
+                </div>
+            </div>
+
+            <div class="pure-u-1 pure-u-sm-17-24 pure-u-md-19-24 package-details" id="main">
+                <h1>{{ metadata.name }}</h1>
+                {%- if metadata.features -%}
+                    <p>This version has <b>{{ metadata.features | length }}</b> feature flags, <b data-id="default-feature-len">
+                        {%- if metadata.features[0].name == 'default' -%}
+                            {{ metadata.features[0].subfeatures | length }}
+                        {%- else -%}
+                            0
+                        {%- endif -%}
+                    </b> of them being enabled by <b>default</b>.</p>
+                    {%- for feature in metadata.features -%}
+                        <h3 id="{{ feature.name }}">{{ feature.name }}</h3>
+                        <ul class="pure-menu-list">
+                            {%- if feature.subfeatures -%}
+                                {%- for subfeature in feature.subfeatures -%}
+                                    <li class="pure-menu-item">
+                                        <span>{{ subfeature }}</span>
+                                    </li>
+                                {%- endfor -%}
+                            {%- else -%}
+                                <p>This feature flag does not enable additional features.</p>
+                            {%- endif -%}
+                        </ul>
+                    {%- endfor -%}
+                {%- else -%}
+                    <p data-id="empty-features">Feature flags are not available for this release.</p>
+                {%- endif -%}
+            </div>
+        </div>
+    </div>
+{%- endblock body -%}
diff --git a/templates/header/package_navigation.html b/templates/header/package_navigation.html
index 38cd6802d..702285b27 100644
--- a/templates/header/package_navigation.html
+++ b/templates/header/package_navigation.html
@@ -8,6 +8,7 @@
         * `crate`
         * `source`
         * `builds`
+        * `features`
 
     Note: `false` here is acting as a pseudo-null value since you can't directly construct null values
            and tera requires all parameters without defaults to be filled
@@ -85,6 +86,15 @@ <h1 id="crate-title">
                                 <span class="title"> Builds</span>
                             </a>
                         </li>
+
+                        {# The features tab #}
+                        <li class="pure-menu-item">
+                            <a href="/crate/{{ crate_path | safe }}/features"
+                               class="pure-menu-link{% if active_tab == 'features' %} pure-menu-active{% endif %}">
+                                {{ "flag" | fas }}
+                                <span class="title">Feature flags</span>
+                            </a>
+                        </li>
                     </ul>
                 </div>
             </div>
diff --git a/templates/rustdoc/topbar.html b/templates/rustdoc/topbar.html
index 868a0a7d6..1d6b0c8a2 100644
--- a/templates/rustdoc/topbar.html
+++ b/templates/rustdoc/topbar.html
@@ -229,6 +229,14 @@
                 </li>
             {%- endfor -%}
         </ul>
+    </li>{#
+    Display the features available in current build
+    #}
+    <li class="pure-menu-item">
+        <a href="{{ crate_url | safe }}/features" title="Browse available feature flags of {{ metadata.name }}-{{ metadata.version }}" class="pure-menu-link">
+            {{ "flag" | fas }}
+            <span class="title">Feature flags</span>
+        </a>
     </li>
 </ul>
 

From 902515f43b9e8c0529d827113a8939b95f55df90 Mon Sep 17 00:00:00 2001
From: Ales Musil <amusil@redhat.com>
Date: Tue, 10 Nov 2020 16:42:34 +0100
Subject: [PATCH 2/4] frontend: Distinguish between empty and null features

Show different page for empty feature flags and
null feature flags.
---
 src/test/fakes.rs             |  8 ++------
 src/web/crate_details.rs      | 34 ++++++++++++++++++++++++++++------
 templates/crate/features.html | 12 +++++++++---
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/test/fakes.rs b/src/test/fakes.rs
index dafae007c..14d6350e4 100644
--- a/src/test/fakes.rs
+++ b/src/test/fakes.rs
@@ -210,12 +210,8 @@ impl<'a> FakeRelease<'a> {
         self
     }
 
-    pub(crate) fn features(mut self, opt_features: Option<HashMap<String, Vec<String>>>) -> Self {
-        if let Some(features) = opt_features {
-            self.package.features = features;
-        } else {
-            self.package.features = HashMap::new();
-        }
+    pub(crate) fn features(mut self, features: HashMap<String, Vec<String>>) -> Self {
+        self.package.features = features;
         self
     }
 
diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs
index 6d874443b..58b3d5b97 100644
--- a/src/web/crate_details.rs
+++ b/src/web/crate_details.rs
@@ -751,8 +751,7 @@ mod tests {
             env.fake_release()
                 .name("library")
                 .version("0.1.0")
-                .binary(false)
-                .features(None)
+                .features(HashMap::new())
                 .create()?;
 
             let page = kuchiki::parse_html().one(
@@ -776,8 +775,7 @@ mod tests {
             env.fake_release()
                 .name("library")
                 .version("0.1.0")
-                .binary(false)
-                .features(Some(features))
+                .features(features)
                 .create()?;
 
             let page = kuchiki::parse_html().one(
@@ -801,8 +799,7 @@ mod tests {
             env.fake_release()
                 .name("library")
                 .version("0.1.0")
-                .binary(false)
-                .features(Some(features))
+                .features(features)
                 .create()?;
 
             let page = kuchiki::parse_html().one(
@@ -819,4 +816,29 @@ mod tests {
             Ok(())
         });
     }
+
+    #[test]
+    fn feature_flags_report_null() {
+        wrapper(|env| {
+            let id = env
+                .fake_release()
+                .name("library")
+                .version("0.1.0")
+                .features(HashMap::new())
+                .create()?;
+
+            env.db()
+                .conn()
+                .query("UPDATE releases SET features = NULL WHERE id = $1", &[&id])?;
+
+            let page = kuchiki::parse_html().one(
+                env.frontend()
+                    .get("/crate/library/0.1.0/features")
+                    .send()?
+                    .text()?,
+            );
+            assert!(page.select_first(r#"p[data-id="null-features"]"#).is_ok());
+            Ok(())
+        });
+    }
 }
diff --git a/templates/crate/features.html b/templates/crate/features.html
index 3dc5319c8..e74e89bdf 100644
--- a/templates/crate/features.html
+++ b/templates/crate/features.html
@@ -34,9 +34,13 @@
                                     </a>
                                 </li>
                             {%- endfor -%}
+                        {%- elif metadata.features is iterable -%}
+                            <li class="pure-menu-item">
+                                <span style="font-size: 13px;">This release does not have any feature flags.</span>
+                            </li>
                         {%- else -%}
                             <li class="pure-menu-item">
-                                <span style="font-size: 13px;">Feature flags are not available for this version.</span>
+                                <span style="font-size: 13px;">Feature flags data are not available for this release.</span>
                             </li>
                         {%- endif -%}
                     </ul>
@@ -52,7 +56,7 @@ <h1>{{ metadata.name }}</h1>
                         {%- else -%}
                             0
                         {%- endif -%}
-                    </b> of them being enabled by <b>default</b>.</p>
+                    </b> of them enabled by <b>default</b>.</p>
                     {%- for feature in metadata.features -%}
                         <h3 id="{{ feature.name }}">{{ feature.name }}</h3>
                         <ul class="pure-menu-list">
@@ -67,8 +71,10 @@ <h3 id="{{ feature.name }}">{{ feature.name }}</h3>
                             {%- endif -%}
                         </ul>
                     {%- endfor -%}
+                {%- elif metadata.features is iterable  -%}
+                    <p data-id="empty-features">This release does not have any feature flags.</p>
                 {%- else -%}
-                    <p data-id="empty-features">Feature flags are not available for this release.</p>
+                    <p data-id="null-features">Feature flags data are not available for this release.</p>
                 {%- endif -%}
             </div>
         </div>

From 1937862f9d38dba81c21270c63d21e16404d6a2f Mon Sep 17 00:00:00 2001
From: Ales Musil <amusil@redhat.com>
Date: Wed, 11 Nov 2020 13:48:03 +0100
Subject: [PATCH 3/4] frontend: Order default features in tree structure

Order features that are enabled by default in flat
tree structure. At the same time report correct
number of features being enabled by default.
---
 src/db/types.rs               |  4 +--
 src/web/crate_details.rs      | 35 +++++++++++++++++++--
 src/web/features.rs           | 58 +++++++++++++++++++++++++++++++++++
 src/web/mod.rs                | 18 +----------
 src/web/source.rs             |  4 +--
 templates/crate/features.html | 20 +++++-------
 6 files changed, 101 insertions(+), 38 deletions(-)

diff --git a/src/db/types.rs b/src/db/types.rs
index eb80c473f..74d5f1e12 100644
--- a/src/db/types.rs
+++ b/src/db/types.rs
@@ -4,8 +4,8 @@ use serde::Serialize;
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, FromSql, ToSql)]
 #[postgres(name = "feature")]
 pub struct Feature {
-    name: String,
-    subfeatures: Vec<String>,
+    pub(crate) name: String,
+    pub(crate) subfeatures: Vec<String>,
 }
 
 impl Feature {
diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs
index 58b3d5b97..700d66621 100644
--- a/src/web/crate_details.rs
+++ b/src/web/crate_details.rs
@@ -103,7 +103,6 @@ impl CrateDetails {
                 releases.license,
                 releases.documentation_url,
                 releases.default_target,
-                releases.features,
                 doc_coverage.total_items,
                 doc_coverage.documented_items,
                 doc_coverage.total_items_needing_examples,
@@ -149,7 +148,6 @@ impl CrateDetails {
             default_target: krate.get("default_target"),
             doc_targets: MetaData::parse_doc_targets(krate.get("doc_targets")),
             yanked: krate.get("yanked"),
-            features: MetaData::parse_features(krate.get("features")),
         };
 
         let documented_items: Option<i32> = krate.get("documented_items");
@@ -817,6 +815,38 @@ mod tests {
         });
     }
 
+    #[test]
+    fn feature_flags_with_nested_default() {
+        wrapper(|env| {
+            let features = [
+                ("default".into(), vec!["feature1".into()]),
+                ("feature1".into(), vec!["feature2".into()]),
+                ("feature2".into(), Vec::new()),
+            ]
+            .iter()
+            .cloned()
+            .collect::<HashMap<String, Vec<String>>>();
+            env.fake_release()
+                .name("library")
+                .version("0.1.0")
+                .features(features)
+                .create()?;
+
+            let page = kuchiki::parse_html().one(
+                env.frontend()
+                    .get("/crate/library/0.1.0/features")
+                    .send()?
+                    .text()?,
+            );
+            assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_err());
+            let def_len = page
+                .select_first(r#"b[data-id="default-feature-len"]"#)
+                .unwrap();
+            assert_eq!(def_len.text_contents(), "3");
+            Ok(())
+        });
+    }
+
     #[test]
     fn feature_flags_report_null() {
         wrapper(|env| {
@@ -824,7 +854,6 @@ mod tests {
                 .fake_release()
                 .name("library")
                 .version("0.1.0")
-                .features(HashMap::new())
                 .create()?;
 
             env.db()
diff --git a/src/web/features.rs b/src/web/features.rs
index 5155a59f3..2b519ed51 100644
--- a/src/web/features.rs
+++ b/src/web/features.rs
@@ -1,3 +1,4 @@
+use crate::db::types::Feature;
 use crate::{
     db::Pool,
     impl_webpage,
@@ -6,10 +7,13 @@ use crate::{
 use iron::{IronResult, Request, Response};
 use router::Router;
 use serde::Serialize;
+use std::collections::{HashMap, VecDeque};
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 struct FeaturesPage {
     metadata: MetaData,
+    features: Option<Vec<Feature>>,
+    default_len: usize,
 }
 
 impl_webpage! {
@@ -22,9 +26,63 @@ pub fn build_features_handler(req: &mut Request) -> IronResult<Response> {
     let version = cexpect!(req, router.find("version"));
 
     let mut conn = extension!(req, Pool).get()?;
+    let rows = ctry!(
+        req,
+        conn.query(
+            "SELECT releases.features FROM releases
+            INNER JOIN crates ON crates.id = releases.crate_id
+            WHERE crates.name = $1 AND releases.version = $2",
+            &[&name, &version]
+        )
+    );
+
+    let row = cexpect!(req, rows.get(0));
+
+    let mut default_len = 0;
+    let features = row
+        .get::<'_, usize, Option<Vec<Feature>>>(0)
+        .map(|raw| {
+            raw.into_iter()
+                .filter(|feature| !feature.is_private())
+                .map(|feature| (feature.name.clone(), feature))
+                .collect::<HashMap<String, Feature>>()
+        })
+        .map(|mut feature_map| {
+            let mut features = get_tree_structure_from_default(&mut feature_map);
+            let mut remaining = feature_map
+                .into_iter()
+                .map(|(_, feature)| feature)
+                .collect::<Vec<Feature>>();
+            remaining.sort_by_key(|feature| feature.subfeatures.len());
+
+            default_len = features.len();
+
+            features.extend(remaining.into_iter().rev());
+            features
+        });
 
     FeaturesPage {
         metadata: cexpect!(req, MetaData::from_crate(&mut conn, &name, &version)),
+        features,
+        default_len,
     }
     .into_response(req)
 }
+
+fn get_tree_structure_from_default(feature_map: &mut HashMap<String, Feature>) -> Vec<Feature> {
+    let mut features = Vec::new();
+    let mut queue: VecDeque<String> = VecDeque::new();
+
+    queue.push_back("default".into());
+    while !queue.is_empty() {
+        let name = queue.pop_front().unwrap();
+        if let Some(feature) = feature_map.remove(&name) {
+            feature
+                .subfeatures
+                .iter()
+                .for_each(|sub| queue.push_back(sub.clone()));
+            features.push(feature);
+        }
+    }
+    features
+}
diff --git a/src/web/mod.rs b/src/web/mod.rs
index db8408a8c..0a58ea2b2 100644
--- a/src/web/mod.rs
+++ b/src/web/mod.rs
@@ -91,7 +91,6 @@ mod sitemap;
 mod source;
 mod statics;
 
-use crate::db::types::Feature;
 use crate::{impl_webpage, Context};
 use chrono::{DateTime, Utc};
 use error::Nope;
@@ -522,7 +521,6 @@ pub(crate) struct MetaData {
     pub(crate) default_target: String,
     pub(crate) doc_targets: Vec<String>,
     pub(crate) yanked: bool,
-    pub(crate) features: Option<Vec<Feature>>,
 }
 
 impl MetaData {
@@ -536,8 +534,7 @@ impl MetaData {
                        releases.rustdoc_status,
                        releases.default_target,
                        releases.doc_targets,
-                       releases.yanked,
-                       releases.features
+                       releases.yanked
                 FROM releases
                 INNER JOIN crates ON crates.id = releases.crate_id
                 WHERE crates.name = $1 AND releases.version = $2",
@@ -556,7 +553,6 @@ impl MetaData {
             default_target: row.get(5),
             doc_targets: MetaData::parse_doc_targets(row.get(6)),
             yanked: row.get(7),
-            features: MetaData::parse_features(row.get(8)),
         })
     }
 
@@ -571,14 +567,6 @@ impl MetaData {
             })
             .unwrap_or_else(Vec::new)
     }
-
-    pub(crate) fn parse_features(features: Option<Vec<Feature>>) -> Option<Vec<Feature>> {
-        features.map(|vec| {
-            vec.into_iter()
-                .filter(|feature| !feature.is_private())
-                .collect()
-        })
-    }
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize)]
@@ -857,7 +845,6 @@ mod test {
                 "arm64-unknown-linux-gnu".to_string(),
             ],
             yanked: false,
-            features: None,
         };
 
         let correct_json = json!({
@@ -872,7 +859,6 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
-            "features": null
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
@@ -890,7 +876,6 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
-            "features": null,
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
@@ -908,7 +893,6 @@ mod test {
                 "arm64-unknown-linux-gnu",
             ],
             "yanked": false,
-            "features": null,
         });
 
         assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
diff --git a/src/web/source.rs b/src/web/source.rs
index a88dea7ac..30bb48fa7 100644
--- a/src/web/source.rs
+++ b/src/web/source.rs
@@ -58,8 +58,7 @@ impl FileList {
                         releases.files,
                         releases.default_target,
                         releases.doc_targets,
-                        releases.yanked,
-                        releases.features
+                        releases.yanked
                 FROM releases
                 LEFT OUTER JOIN crates ON crates.id = releases.crate_id
                 WHERE crates.name = $1 AND releases.version = $2",
@@ -138,7 +137,6 @@ impl FileList {
                     default_target: rows[0].get(6),
                     doc_targets: MetaData::parse_doc_targets(rows[0].get(7)),
                     yanked: rows[0].get(8),
-                    features: MetaData::parse_features(rows[0].get(9)),
                 },
                 files: file_list,
             })
diff --git a/templates/crate/features.html b/templates/crate/features.html
index e74e89bdf..37e63e04f 100644
--- a/templates/crate/features.html
+++ b/templates/crate/features.html
@@ -26,15 +26,15 @@
                 <div class="pure-menu package-menu">
                     <ul class="pure-menu-list">
                         <li class="pure-menu-heading">Feature flags</li>
-                        {%- if metadata.features -%}
-                            {%- for feature in metadata.features -%}
+                        {%- if features -%}
+                            {%- for feature in features -%}
                                 <li class="pure-menu-item">
                                     <a href="#{{ feature.name }}" class="pure-menu-link" style="text-align:center;">
                                         {{ feature.name }}
                                     </a>
                                 </li>
                             {%- endfor -%}
-                        {%- elif metadata.features is iterable -%}
+                        {%- elif features is iterable -%}
                             <li class="pure-menu-item">
                                 <span style="font-size: 13px;">This release does not have any feature flags.</span>
                             </li>
@@ -49,15 +49,9 @@
 
             <div class="pure-u-1 pure-u-sm-17-24 pure-u-md-19-24 package-details" id="main">
                 <h1>{{ metadata.name }}</h1>
-                {%- if metadata.features -%}
-                    <p>This version has <b>{{ metadata.features | length }}</b> feature flags, <b data-id="default-feature-len">
-                        {%- if metadata.features[0].name == 'default' -%}
-                            {{ metadata.features[0].subfeatures | length }}
-                        {%- else -%}
-                            0
-                        {%- endif -%}
-                    </b> of them enabled by <b>default</b>.</p>
-                    {%- for feature in metadata.features -%}
+                {%- if features -%}
+                    <p>This version has <b>{{ features | length }}</b> feature flags, <b data-id="default-feature-len">{{ default_len }}</b> of them enabled by <b>default</b>.</p>
+                    {%- for feature in features -%}
                         <h3 id="{{ feature.name }}">{{ feature.name }}</h3>
                         <ul class="pure-menu-list">
                             {%- if feature.subfeatures -%}
@@ -71,7 +65,7 @@ <h3 id="{{ feature.name }}">{{ feature.name }}</h3>
                             {%- endif -%}
                         </ul>
                     {%- endfor -%}
-                {%- elif metadata.features is iterable  -%}
+                {%- elif features is iterable  -%}
                     <p data-id="empty-features">This release does not have any feature flags.</p>
                 {%- else -%}
                     <p data-id="null-features">Feature flags data are not available for this release.</p>

From 6730aa201c68cbe6ee34e50e7c419ebab2d3764a Mon Sep 17 00:00:00 2001
From: Ales Musil <amusil@redhat.com>
Date: Thu, 12 Nov 2020 08:56:25 +0100
Subject: [PATCH 4/4] frontend: Make features code more modular

By making it more modular it is easier to add
unit tests fro current behavior.
---
 src/web/features.rs | 176 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 154 insertions(+), 22 deletions(-)

diff --git a/src/web/features.rs b/src/web/features.rs
index 2b519ed51..ea13cf5bc 100644
--- a/src/web/features.rs
+++ b/src/web/features.rs
@@ -9,6 +9,8 @@ use router::Router;
 use serde::Serialize;
 use std::collections::{HashMap, VecDeque};
 
+const DEFAULT_NAME: &str = "default";
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 struct FeaturesPage {
     metadata: MetaData,
@@ -38,28 +40,14 @@ pub fn build_features_handler(req: &mut Request) -> IronResult<Response> {
 
     let row = cexpect!(req, rows.get(0));
 
+    let mut features = None;
     let mut default_len = 0;
-    let features = row
-        .get::<'_, usize, Option<Vec<Feature>>>(0)
-        .map(|raw| {
-            raw.into_iter()
-                .filter(|feature| !feature.is_private())
-                .map(|feature| (feature.name.clone(), feature))
-                .collect::<HashMap<String, Feature>>()
-        })
-        .map(|mut feature_map| {
-            let mut features = get_tree_structure_from_default(&mut feature_map);
-            let mut remaining = feature_map
-                .into_iter()
-                .map(|(_, feature)| feature)
-                .collect::<Vec<Feature>>();
-            remaining.sort_by_key(|feature| feature.subfeatures.len());
-
-            default_len = features.len();
-
-            features.extend(remaining.into_iter().rev());
-            features
-        });
+
+    if let Some(raw) = row.get(0) {
+        let result = order_features_and_count_default_len(raw);
+        features = Some(result.0);
+        default_len = result.1;
+    }
 
     FeaturesPage {
         metadata: cexpect!(req, MetaData::from_crate(&mut conn, &name, &version)),
@@ -69,11 +57,26 @@ pub fn build_features_handler(req: &mut Request) -> IronResult<Response> {
     .into_response(req)
 }
 
+fn order_features_and_count_default_len(raw: Vec<Feature>) -> (Vec<Feature>, usize) {
+    let mut feature_map = get_feature_map(raw);
+    let mut features = get_tree_structure_from_default(&mut feature_map);
+    let mut remaining: Vec<_> = feature_map
+        .into_iter()
+        .map(|(_, feature)| feature)
+        .collect();
+    remaining.sort_by_key(|feature| feature.subfeatures.len());
+
+    let default_len = features.len();
+
+    features.extend(remaining.into_iter().rev());
+    (features, default_len)
+}
+
 fn get_tree_structure_from_default(feature_map: &mut HashMap<String, Feature>) -> Vec<Feature> {
     let mut features = Vec::new();
     let mut queue: VecDeque<String> = VecDeque::new();
 
-    queue.push_back("default".into());
+    queue.push_back(DEFAULT_NAME.into());
     while !queue.is_empty() {
         let name = queue.pop_front().unwrap();
         if let Some(feature) = feature_map.remove(&name) {
@@ -86,3 +89,132 @@ fn get_tree_structure_from_default(feature_map: &mut HashMap<String, Feature>) -
     }
     features
 }
+
+fn get_feature_map(raw: Vec<Feature>) -> HashMap<String, Feature> {
+    raw.into_iter()
+        .filter(|feature| !feature.is_private())
+        .map(|feature| (feature.name.clone(), feature))
+        .collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::db::types::Feature;
+    use crate::web::features::{
+        get_feature_map, get_tree_structure_from_default, order_features_and_count_default_len,
+        DEFAULT_NAME,
+    };
+
+    #[test]
+    fn test_feature_map_filters_private() {
+        let private1 = Feature::new("_private1".into(), vec!["feature1".into()]);
+        let feature2 = Feature::new("feature2".into(), Vec::new());
+
+        let raw = vec![private1.clone(), feature2.clone()];
+        let feature_map = get_feature_map(raw);
+
+        assert_eq!(feature_map.len(), 1);
+        assert!(feature_map.contains_key(&feature2.name));
+        assert!(!feature_map.contains_key(&private1.name));
+    }
+
+    #[test]
+    fn test_default_tree_structure_with_nested_default() {
+        let default = Feature::new(DEFAULT_NAME.into(), vec!["feature1".into()]);
+        let non_default = Feature::new("non-default".into(), Vec::new());
+        let feature1 = Feature::new(
+            "feature1".into(),
+            vec!["feature2".into(), "feature3".into()],
+        );
+        let feature2 = Feature::new("feature2".into(), Vec::new());
+        let feature3 = Feature::new("feature3".into(), Vec::new());
+
+        let raw = vec![
+            default.clone(),
+            non_default.clone(),
+            feature3.clone(),
+            feature2.clone(),
+            feature1.clone(),
+        ];
+        let mut feature_map = get_feature_map(raw);
+        let default_tree = get_tree_structure_from_default(&mut feature_map);
+
+        assert_eq!(feature_map.len(), 1);
+        assert_eq!(default_tree.len(), 4);
+        assert!(feature_map.contains_key(&non_default.name));
+        assert!(!feature_map.contains_key(&default.name));
+        assert_eq!(default_tree[0], default);
+        assert_eq!(default_tree[1], feature1);
+        assert_eq!(default_tree[2], feature2);
+        assert_eq!(default_tree[3], feature3);
+    }
+
+    #[test]
+    fn test_default_tree_structure_without_default() {
+        let feature1 = Feature::new(
+            "feature1".into(),
+            vec!["feature2".into(), "feature3".into()],
+        );
+        let feature2 = Feature::new("feature2".into(), Vec::new());
+        let feature3 = Feature::new("feature3".into(), Vec::new());
+
+        let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
+        let mut feature_map = get_feature_map(raw);
+        let default_tree = get_tree_structure_from_default(&mut feature_map);
+
+        assert_eq!(feature_map.len(), 3);
+        assert_eq!(default_tree.len(), 0);
+        assert!(feature_map.contains_key(&feature1.name));
+        assert!(feature_map.contains_key(&feature2.name));
+        assert!(feature_map.contains_key(&feature3.name));
+    }
+
+    #[test]
+    fn test_default_tree_structure_single_default() {
+        let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
+        let non_default = Feature::new("non-default".into(), Vec::new());
+
+        let raw = vec![default.clone(), non_default.clone()];
+        let mut feature_map = get_feature_map(raw);
+        let default_tree = get_tree_structure_from_default(&mut feature_map);
+
+        assert_eq!(feature_map.len(), 1);
+        assert_eq!(default_tree.len(), 1);
+        assert!(feature_map.contains_key(&non_default.name));
+        assert!(!feature_map.contains_key(&default.name));
+        assert_eq!(default_tree[0], default);
+    }
+
+    #[test]
+    fn test_order_features_and_get_len_without_default() {
+        let feature1 = Feature::new(
+            "feature1".into(),
+            vec!["feature10".into(), "feature11".into()],
+        );
+        let feature2 = Feature::new("feature2".into(), vec!["feature20".into()]);
+        let feature3 = Feature::new("feature3".into(), Vec::new());
+
+        let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
+        let (features, default_len) = order_features_and_count_default_len(raw);
+
+        assert_eq!(features.len(), 3);
+        assert_eq!(default_len, 0);
+        assert_eq!(features[0], feature1);
+        assert_eq!(features[1], feature2);
+        assert_eq!(features[2], feature3);
+    }
+
+    #[test]
+    fn test_order_features_and_get_len_single_default() {
+        let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
+        let non_default = Feature::new("non-default".into(), Vec::new());
+
+        let raw = vec![default.clone(), non_default.clone()];
+        let (features, default_len) = order_features_and_count_default_len(raw);
+
+        assert_eq!(features.len(), 2);
+        assert_eq!(default_len, 1);
+        assert_eq!(features[0], default);
+        assert_eq!(features[1], non_default);
+    }
+}