Skip to content

Commit f5f1982

Browse files
KixironJoshua Nelson
Kixiron
and
Joshua Nelson
committed
Apply suggestions from code review
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
1 parent 9655e92 commit f5f1982

File tree

4 files changed

+64
-117
lines changed

4 files changed

+64
-117
lines changed

‎src/web/crate_details.rs

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ use iron::prelude::*;
88
use iron::{status, Url};
99
use postgres::Connection;
1010
use router::Router;
11-
use serde::ser::{Serialize, SerializeStruct, Serializer};
11+
use serde::{
12+
ser::{SerializeStruct, Serializer},
13+
Serialize,
14+
};
1215
use serde_json::Value;
1316

1417
// TODO: Add target name and versions
@@ -51,7 +54,15 @@ impl Serialize for CrateDetails {
5154
where
5255
S: Serializer,
5356
{
54-
let mut state = serializer.serialize_struct("CrateDetails", 28)?;
57+
// Make sure that the length parameter passed to serde is correct by
58+
// adding the someness of `readme` and `rustdoc` to the total. `true`
59+
// is 1 and `false` is 0, so it increments if the value is some (and therefore
60+
// needs to be serialized)
61+
let mut state = serializer.serialize_struct(
62+
"CrateDetails",
63+
26 + self.readme.is_some() as usize + self.rustdoc.is_some() as usize,
64+
)?;
65+
5566
state.serialize_field("metadata", &self.metadata)?;
5667
state.serialize_field("name", &self.name)?;
5768
state.serialize_field("version", &self.version)?;
@@ -94,7 +105,7 @@ impl Serialize for CrateDetails {
94105
}
95106
}
96107

97-
#[derive(Debug, Eq, PartialEq, serde::Serialize)]
108+
#[derive(Debug, Eq, PartialEq, Serialize)]
98109
pub struct Release {
99110
pub version: String,
100111
pub build_status: bool,
@@ -626,7 +637,7 @@ mod tests {
626637
let time = time::get_time();
627638
let mut details = CrateDetails::default_tester(time);
628639

629-
let correct_json = json!({
640+
let mut correct_json = json!({
630641
"name": "rcc",
631642
"version": "100.0.0",
632643
"description": null,
@@ -665,50 +676,20 @@ mod tests {
665676

666677
assert_eq!(correct_json, serde_json::to_value(&details).unwrap());
667678

668-
details.description = Some("serde does stuff".to_string());
669-
details.owners = vec![("Owner".to_string(), "owner@ownsstuff.com".to_string())];
670-
671679
let authors = vec![("Somebody".to_string(), "somebody@somebody.com".to_string())];
680+
let owners = vec![("Owner".to_string(), "owner@ownsstuff.com".to_string())];
681+
let description = "serde does stuff".to_string();
682+
683+
correct_json["description"] = Value::String(description.clone());
684+
correct_json["owners"] = serde_json::to_value(&owners).unwrap();
685+
correct_json["authors_json"] = serde_json::to_value(&authors).unwrap();
686+
correct_json["authors"] = serde_json::to_value(&authors).unwrap();
687+
688+
details.description = Some(description);
689+
details.owners = owners;
672690
details.authors_json = Some(serde_json::to_value(&authors).unwrap());
673691
details.authors = authors;
674692

675-
let correct_json = json!({
676-
"name": "rcc",
677-
"version": "100.0.0",
678-
"description": "serde does stuff",
679-
"authors": [["Somebody", "somebody@somebody.com"]],
680-
"owners": [["Owner", "owner@ownsstuff.com"]],
681-
"authors_json": [["Somebody", "somebody@somebody.com"]],
682-
"dependencies": null,
683-
"release_time": super::super::duration_to_str(time),
684-
"build_status": true,
685-
"last_successful_build": null,
686-
"rustdoc_status": true,
687-
"repository_url": null,
688-
"homepage_url": null,
689-
"keywords": null,
690-
"have_examples": true,
691-
"target_name": "x86_64-unknown-linux-gnu",
692-
"releases": [],
693-
"github": true,
694-
"yanked": false,
695-
"github_stars": null,
696-
"github_forks": null,
697-
"github_issues": null,
698-
"metadata": {
699-
"name": "serde",
700-
"version": "1.0.0",
701-
"description": "serde does stuff",
702-
"target_name": null,
703-
"rustdoc_status": true,
704-
"default_target": "x86_64-unknown-linux-gnu"
705-
},
706-
"is_library": true,
707-
"doc_targets": [],
708-
"license": null,
709-
"documentation_url": null
710-
});
711-
712693
assert_eq!(correct_json, serde_json::to_value(&details).unwrap());
713694
}
714695

‎src/web/page.rs

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ impl<T: Serialize> Serialize for Page<T> {
116116
where
117117
S: Serializer,
118118
{
119-
let mut state = serializer.serialize_struct("Page", 10)?;
119+
// Make sure that the length parameter passed to serde is correct by
120+
// adding the someness of the global alert to the total. `true`
121+
// is 1 and `false` is 0, so it increments if the value is some (and therefore
122+
// needs to be serialized)
123+
let mut state =
124+
serializer.serialize_struct("Page", 9 + crate::GLOBAL_ALERT.is_some() as usize)?;
120125

121126
if let Some(ref title) = self.title {
122127
state.serialize_field("title", title)?;
@@ -132,10 +137,7 @@ impl<T: Serialize> Serialize for Page<T> {
132137
state.serialize_field("cratesfyi_version", crate::BUILD_VERSION)?;
133138
state.serialize_field(
134139
"cratesfyi_version_safe",
135-
&crate::BUILD_VERSION
136-
.replace(" ", "-")
137-
.replace("(", "")
138-
.replace(")", ""),
140+
&build_version_safe(crate::BUILD_VERSION),
139141
)?;
140142
state.serialize_field("varss", &self.varss)?;
141143
state.serialize_field("varsb", &self.varsb)?;
@@ -145,10 +147,15 @@ impl<T: Serialize> Serialize for Page<T> {
145147
}
146148
}
147149

150+
fn build_version_safe(version: &str) -> String {
151+
version.replace(" ", "-").replace("(", "").replace(")", "")
152+
}
153+
148154
#[cfg(test)]
149155
mod tests {
150156
use super::super::releases::{self, Release};
151157
use super::*;
158+
use iron::Url;
152159
use serde_json::json;
153160

154161
#[test]
@@ -193,10 +200,7 @@ mod tests {
193200
"varsi": { "test3": 1337 },
194201
"rustc_resource_suffix": &*RUSTC_RESOURCE_SUFFIX,
195202
"cratesfyi_version": crate::BUILD_VERSION,
196-
"cratesfyi_version_safe": crate::BUILD_VERSION
197-
.replace(" ", "-")
198-
.replace("(", "")
199-
.replace(")", ""),
203+
"cratesfyi_version_safe": build_version_safe(crate::BUILD_VERSION),
200204
"has_global_alert": crate::GLOBAL_ALERT.is_some()
201205
});
202206

@@ -238,18 +242,27 @@ mod tests {
238242
fn serialize_global_alert() {
239243
let alert = GlobalAlert {
240244
url: "http://www.hasthelargehadroncolliderdestroyedtheworldyet.com/",
241-
text: "THE WORLD IS ENDING",
245+
text: "THE WORLD WILL SOON END",
242246
css_class: "THE END IS NEAR",
243247
fa_icon: "https://gph.is/1uOvmqR",
244248
};
245249

246250
let correct_json = json!({
247251
"url": "http://www.hasthelargehadroncolliderdestroyedtheworldyet.com/",
248-
"text": "THE WORLD IS ENDING",
252+
"text": "THE WORLD WILL SOON END",
249253
"css_class": "THE END IS NEAR",
250254
"fa_icon": "https://gph.is/1uOvmqR"
251255
});
252256

253257
assert_eq!(correct_json, serde_json::to_value(&alert).unwrap());
254258
}
259+
260+
#[test]
261+
fn build_version_url_safe() {
262+
let safe = format!(
263+
"https://docs.rs/builds/{}",
264+
build_version_safe(crate::BUILD_VERSION)
265+
);
266+
assert!(Url::parse(&safe).is_ok());
267+
}
255268
}

‎src/web/releases.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,14 +1006,16 @@ mod tests {
10061006

10071007
#[test]
10081008
fn serialize_releases() {
1009-
let time = time::get_time();
1009+
let current_time = time::get_time();
1010+
let now = time::at(current_time);
1011+
10101012
let mut release = Release {
10111013
name: "serde".to_string(),
10121014
version: "0.0.0".to_string(),
10131015
description: Some("serde makes things other things".to_string()),
10141016
target_name: Some("x86_64-pc-windows-msvc".to_string()),
10151017
rustdoc_status: true,
1016-
release_time: time,
1018+
release_time: current_time,
10171019
stars: 100,
10181020
};
10191021

@@ -1023,8 +1025,8 @@ mod tests {
10231025
"description": "serde makes things other things",
10241026
"target_name": "x86_64-pc-windows-msvc",
10251027
"rustdoc_status": true,
1026-
"release_time": duration_to_str(time),
1027-
"release_time_rfc3339": time::at(time).rfc3339().to_string(),
1028+
"release_time": duration_to_str(current_time),
1029+
"release_time_rfc3339": now.rfc3339().to_string(),
10281030
"stars": 100
10291031
});
10301032

@@ -1037,8 +1039,8 @@ mod tests {
10371039
"description": "serde makes things other things",
10381040
"target_name": null,
10391041
"rustdoc_status": true,
1040-
"release_time": duration_to_str(time),
1041-
"release_time_rfc3339": time::at(time).rfc3339().to_string(),
1042+
"release_time": duration_to_str(current_time),
1043+
"release_time_rfc3339": now.rfc3339().to_string(),
10421044
"stars": 100
10431045
});
10441046

@@ -1051,8 +1053,8 @@ mod tests {
10511053
"description": null,
10521054
"target_name": null,
10531055
"rustdoc_status": true,
1054-
"release_time": duration_to_str(time),
1055-
"release_time_rfc3339": time::at(time).rfc3339().to_string(),
1056+
"release_time": duration_to_str(current_time),
1057+
"release_time_rfc3339": now.rfc3339().to_string(),
10561058
"stars": 100
10571059
});
10581060

‎src/web/source.rs

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -29,55 +29,6 @@ struct File {
2929
}
3030

3131
/// A list of source files
32-
///
33-
///
34-
/// Rust:
35-
///
36-
/// ```ignore
37-
/// FileList {
38-
/// metadata: MetaData {
39-
/// name: "rcc",
40-
/// version: "0.0.0",
41-
/// description: Some("it compiles an unholy language"),
42-
/// target_name: None,
43-
/// rustdoc_status: true,
44-
/// default_target: "x86_64-unknown-linux-gnu",
45-
/// },
46-
/// files: vec![
47-
/// File {
48-
/// name: "main.rs",
49-
/// file_type: FileType::RustSource,
50-
/// },
51-
/// File {
52-
/// name: "lib.rs",
53-
/// file_type: FileType::RustSource,
54-
/// },
55-
/// ],
56-
/// }
57-
/// ```
58-
///
59-
/// Json:
60-
///
61-
/// ```json
62-
/// {
63-
/// "metadata": {
64-
/// "name": "rcc",
65-
/// "version": "0.0.0",
66-
/// "description": "it compiles an unholy language",
67-
/// "target_name": null,
68-
/// "rustdoc_status": true,
69-
/// "default_target": "x86_64-unknown-linux-gnu",
70-
/// },
71-
/// "files": [{
72-
/// "name": "main.rs",
73-
/// "file_type_rust_source": true,
74-
/// }, {
75-
/// "name": "lib.rs",
76-
/// "file_type_rust_source": true,
77-
/// }],
78-
/// }
79-
/// ```
80-
///
8132
struct FileList {
8233
metadata: MetaData,
8334
files: Vec<File>,
@@ -93,7 +44,7 @@ impl Serialize for FileList {
9344

9445
let mut files = Vec::with_capacity(self.files.len());
9546
for file in &self.files {
96-
let mut map = HashMap::with_capacity(3);
47+
let mut map = HashMap::with_capacity(2);
9748
map.insert("name", Value::String(file.name.to_owned()));
9849

9950
let file_type = match file.file_type {
@@ -119,10 +70,10 @@ impl FileList {
11970
///
12071
/// ```text
12172
/// [
122-
/// ["text/plain",".gitignore"],
123-
/// ["text/x-c","src/reseeding.rs"],
124-
/// ["text/x-c","src/lib.rs"],
125-
/// ["text/x-c","README.md"],
73+
/// ["text/plain", ".gitignore"],
74+
/// ["text/x-c", "src/reseeding.rs"],
75+
/// ["text/x-c", "src/lib.rs"],
76+
/// ["text/x-c", "README.md"],
12677
/// ...
12778
/// ]
12879
/// ```

0 commit comments

Comments
 (0)