Commit d47a1e4d authored by Brad King's avatar Brad King Committed by Kitware Robot
Browse files

Merge topic 'prerelease-cleanups'

16dc2160 untry: convert from `try!()` to the `?` operator
46e4cca4 rustfmt: apply sensible suggestions
41a0fe6b clippy: impl `Copy` for structures which make sense
2ceb6b20

 clippy: document private items
Acked-by: Kitware Robot's avatarKitware Robot <kwrobot@kitware.com>
Reviewed-by: Brad King's avatarBrad King <brad.king@kitware.com>
Merge-request: !69
parents c62fb467 16dc2160
......@@ -28,7 +28,9 @@ use std::fmt::{self, Debug};
///
/// Separate users should use separate instances of this.
pub struct Gitlab {
/// The base URL to use for API calls.
base_url: Url,
/// The secret token to use when communicating with Gitlab.
token: String,
}
......@@ -86,9 +88,10 @@ impl Gitlab {
Self::_new("http", host, token.to_string())
}
/// Internal method to create a new Gitlab client.
fn _new(protocol: &str, host: &str, token: String) -> Result<Self> {
let base_url = try!(Url::parse(&format!("{}://{}/api/v3/", protocol, host))
.chain_err(|| ErrorKind::UrlParse));
let base_url = Url::parse(&format!("{}://{}/api/v3/", protocol, host))
.chain_err(|| ErrorKind::UrlParse)?;
let api = Gitlab {
base_url: base_url,
......@@ -96,7 +99,7 @@ impl Gitlab {
};
// Ensure the API is working.
let _: UserPublic = try!(api._get("user"));
let _: UserPublic = api._get("user")?;
Ok(api)
}
......@@ -118,7 +121,7 @@ impl Gitlab {
/// Find a user by username.
pub fn user_by_name<T: UserResult>(&self, name: &str) -> Result<T> {
let mut users = try!(self._get_paged_with_param("users", &[("username", name)]));
let mut users = self._get_paged_with_param("users", &[("username", name)])?;
users.pop()
.ok_or_else(|| Error::from_kind(ErrorKind::Gitlab("no such user".to_string())))
}
......@@ -161,6 +164,7 @@ impl Gitlab {
self._get(&format!("projects/{}/hooks/{}", project, hook))
}
/// Convert a boolean parameter into an HTTP request value.
fn bool_param_value(value: bool) -> &'static str {
if value {
"true"
......@@ -169,6 +173,7 @@ impl Gitlab {
}
}
/// HTTP parameters required to register to a project.
fn event_flags(events: WebhookEvents) -> Vec<(&'static str, &'static str)> {
vec![("build_events", Self::bool_param_value(events.build())),
("issues_events", Self::bool_param_value(events.issues())),
......@@ -180,8 +185,7 @@ impl Gitlab {
}
/// Add a project hook.
pub fn add_hook(&self, project: ProjectId, url: &str, events: WebhookEvents)
-> Result<Hook> {
pub fn add_hook(&self, project: ProjectId, url: &str, events: WebhookEvents) -> Result<Hook> {
let mut flags = Self::event_flags(events);
flags.push(("url", url));
......@@ -236,8 +240,7 @@ impl Gitlab {
}
/// Get comments on a commit.
pub fn commit_comments(&self, project: ProjectId, commit: &str)
-> Result<Vec<CommitNote>> {
pub fn commit_comments(&self, project: ProjectId, commit: &str) -> Result<Vec<CommitNote>> {
self._get_paged(&format!("projects/{}/repository/commits/{}/comments",
project,
commit))
......@@ -286,8 +289,7 @@ impl Gitlab {
}
/// Get the latest builds of a commit.
pub fn commit_latest_builds(&self, project: ProjectId, commit: &str)
-> Result<Vec<Build>> {
pub fn commit_latest_builds(&self, project: ProjectId, commit: &str) -> Result<Vec<Build>> {
self._get_paged(&format!("projects/{}/repository/commits/{}/builds", project, commit))
}
......@@ -411,45 +413,49 @@ impl Gitlab {
self._post_with_param(path, &[("body", content)])
}
/// Create a URL to an API endpoint.
fn _mk_url(&self, url: &str) -> Result<Url> {
debug!(target: "gitlab", "api call {}", url);
Ok(try!(self.base_url.join(url).chain_err(|| ErrorKind::UrlParse)))
Ok(self.base_url.join(url).chain_err(|| ErrorKind::UrlParse)?)
}
/// Create a URL to an API endpoint with query parameters.
fn _mk_url_with_param<I, K, V>(&self, url: &str, param: I) -> Result<Url>
where I: IntoIterator,
I::Item: Borrow<(K, V)>,
K: AsRef<str>,
V: AsRef<str>,
{
let mut full_url = try!(self._mk_url(url));
let mut full_url = self._mk_url(url)?;
full_url.query_pairs_mut().extend_pairs(param);
Ok(full_url)
}
// Refactored code which talks to Gitlab and transforms error messages properly.
/// Refactored code which talks to Gitlab and transforms error messages properly.
fn _comm<T>(&self, req: RequestBuilder) -> Result<T>
where T: Deserialize,
{
let req = req.header(GitlabPrivateToken(self.token.to_string()));
let rsp = try!(req.send().chain_err(|| ErrorKind::Communication));
let rsp = req.send().chain_err(|| ErrorKind::Communication)?;
if !rsp.status().is_success() {
let v = try!(serde_json::from_reader(rsp).chain_err(|| ErrorKind::Deserialize));
let v = serde_json::from_reader(rsp).chain_err(|| ErrorKind::Deserialize)?;
return Err(Error::from_gitlab(v));
}
let v = try!(serde_json::from_reader(rsp).chain_err(|| ErrorKind::Deserialize));
let v = serde_json::from_reader(rsp).chain_err(|| ErrorKind::Deserialize)?;
debug!(target: "gitlab",
"received data: {:?}",
v);
Ok(try!(serde_json::from_value::<T>(v).chain_err(|| ErrorKind::Deserialize)))
Ok(serde_json::from_value::<T>(v).chain_err(|| ErrorKind::Deserialize)?)
}
/// Create a `GET` request to an API endpoint.
fn _get<T: Deserialize>(&self, url: &str) -> Result<T> {
let param: &[(&str, &str)] = &[];
self._get_with_param(url, param)
}
/// Create a `GET` request to an API endpoint with query parameters.
fn _get_with_param<T, I, K, V>(&self, url: &str, param: I) -> Result<T>
where T: Deserialize,
I: IntoIterator,
......@@ -457,31 +463,34 @@ impl Gitlab {
K: AsRef<str>,
V: AsRef<str>,
{
let full_url = try!(self._mk_url_with_param(url, param));
let req = try!(Client::new().chain_err(|| ErrorKind::Communication)).get(full_url);
let full_url = self._mk_url_with_param(url, param)?;
let req = Client::new().chain_err(|| ErrorKind::Communication)?.get(full_url);
self._comm(req)
}
/// Create a `POST` request to an API endpoint.
fn _post<T: Deserialize>(&self, url: &str) -> Result<T> {
let param: &[(&str, &str)] = &[];
self._post_with_param(url, param)
}
/// Create a `POST` request to an API endpoint with query parameters.
fn _post_with_param<T, U>(&self, url: &str, param: U) -> Result<T>
where T: Deserialize,
U: Serialize,
{
let full_url = try!(self._mk_url(url));
let req = try!(Client::new().chain_err(|| ErrorKind::Communication)).post(full_url).form(&param);
let full_url = self._mk_url(url)?;
let req = Client::new().chain_err(|| ErrorKind::Communication)?.post(full_url).form(&param);
self._comm(req)
}
// Handle paginated queries. Returns all results.
/// Handle paginated queries. Returns all results.
fn _get_paged<T: Deserialize>(&self, url: &str) -> Result<Vec<T>> {
let param: &[(&str, &str)] = &[];
self._get_paged_with_param(url, param)
}
/// Handle paginated queries with query parameters. Returns all results.
fn _get_paged_with_param<T, I, K, V>(&self, url: &str, param: I) -> Result<Vec<T>>
where T: Deserialize,
I: IntoIterator,
......@@ -493,7 +502,7 @@ impl Gitlab {
let per_page = 100;
let per_page_str = &format!("{}", per_page);
let full_url = try!(self._mk_url_with_param(url, param));
let full_url = self._mk_url_with_param(url, param)?;
let mut results: Vec<T> = vec![];
......@@ -502,9 +511,9 @@ impl Gitlab {
let mut page_url = full_url.clone();
page_url.query_pairs_mut()
.extend_pairs(&[("page", page_str), ("per_page", per_page_str)]);
let req = try!(Client::new().chain_err(|| ErrorKind::Communication)).get(page_url);
let req = Client::new().chain_err(|| ErrorKind::Communication)?.get(page_url);
let page: Vec<T> = try!(self._comm(req));
let page: Vec<T> = self._comm(req)?;
let page_len = page.len();
results.extend(page);
......
......@@ -34,7 +34,7 @@ pub enum GitlabHook {
impl Deserialize for GitlabHook {
fn deserialize<D: Deserializer>(deserializer: &mut D) -> Result<Self, D::Error> {
let val = try!(Value::deserialize(deserializer));
let val = Value::deserialize(deserializer)?;
// Look for `object_kind` first because some web hooks also have `event_name` which would
// cause a false match here.
......
......@@ -370,7 +370,7 @@ pub enum SystemHook {
impl Deserialize for SystemHook {
fn deserialize<D: Deserializer>(deserializer: &mut D) -> Result<Self, D::Error> {
let val = try!(Value::deserialize(deserializer));
let val = Value::deserialize(deserializer)?;
let event_name = match val.pointer("/event_name") {
Some(&Value::String(ref name)) => name.to_string(),
......
......@@ -136,7 +136,7 @@ fn test_read_issue() {
assert_eq!(assignee.name, "Ben Boeckel");
assert_eq!(assignee.state, UserState::Active);
assert_eq!(assignee.avatar_url,
"https://secure.gravatar.com/avatar/2f5f7e99190174edb5a2f66b8653b0b2?s=80&d=identicon");
"https://secure.gravatar.com/avatar/2f5f7e99190174edb5a2f66b8653b0b2?s=80&d=identicon");
assert_eq!(assignee.id, UserId::new(13));
} else {
panic!("expected to have an assignee for the issue");
......@@ -186,7 +186,7 @@ fn test_read_issue_reference() {
assert_eq!(assignee.name, "Ben Boeckel");
assert_eq!(assignee.state, UserState::Active);
assert_eq!(assignee.avatar_url,
"https://secure.gravatar.com/avatar/2f5f7e99190174edb5a2f66b8653b0b2?s=80&d=identicon");
"https://secure.gravatar.com/avatar/2f5f7e99190174edb5a2f66b8653b0b2?s=80&d=identicon");
assert_eq!(assignee.id, UserId::new(13));
} else {
panic!("expected to have an assignee for the issue");
......@@ -497,7 +497,8 @@ fn test_read_user_public() {
assert_eq!(user_public.state, UserState::Active);
assert_eq!(user_public.avatar_url,
"https://secure.gravatar.com/avatar/2f5f7e99190174edb5a2f66b8653b0b2?s=80&d=identicon");
assert_eq!(user_public.web_url, "https://gitlab.kitware.com/ben.boeckel");
assert_eq!(user_public.web_url,
"https://gitlab.kitware.com/ben.boeckel");
assert_eq!(user_public.created_at,
UTC.ymd(2015, 2, 26)
.and_hms_milli(17, 23, 28, 730));
......
......@@ -549,7 +549,7 @@ pub struct Project {
pub permissions: Option<Permissions>,
}
#[derive(Serialize, Deserialize, Debug, Clone)]
#[derive(Serialize, Deserialize, Debug, Clone, Copy)]
/// Statistics about a project.
pub struct ProjectStatistics {
/// The number of commits in the repository.
......@@ -714,7 +714,7 @@ pub struct Group {
pub statistics: Option<ProjectStatistics>,
}
#[derive(Serialize, Deserialize, Debug, Clone)]
#[derive(Serialize, Deserialize, Debug, Clone, Copy)]
/// Statistics about a group.
pub struct GroupStatistics {
/// The size, in bytes, of the total storage required for the group.
......@@ -1144,7 +1144,7 @@ impl Serialize for IssueReference {
impl Deserialize for IssueReference {
fn deserialize<D: Deserializer>(deserializer: &mut D) -> Result<Self, D::Error> {
let val = try!(Value::deserialize(deserializer));
let val = Value::deserialize(deserializer)?;
serde_json::from_value::<Issue>(val.clone())
.map(IssueReference::Internal)
......
......@@ -17,8 +17,7 @@ extern crate serde_json;
use self::serde_json::Value;
use super::types::{BuildId, IssueId, IssueState, MergeRequestId, MergeRequestState, MergeStatus,
MilestoneId, NoteId, NoteType, NoteableId, ObjectId, ProjectId, SnippetId,
UserId};
MilestoneId, NoteId, NoteType, NoteableId, ObjectId, ProjectId, SnippetId, UserId};
#[derive(Debug, Clone, Copy)]
/// A wrapper struct for dates in web hooks.
......@@ -35,7 +34,7 @@ impl Serialize for HookDate {
impl Deserialize for HookDate {
fn deserialize<D: Deserializer>(deserializer: &mut D) -> Result<Self, D::Error> {
let val = try!(String::deserialize(deserializer));
let val = String::deserialize(deserializer)?;
UTC.datetime_from_str(&val, "%Y-%m-%d %H:%M:%S UTC")
.or_else(|_| {
......@@ -695,7 +694,7 @@ pub enum WebHook {
impl Deserialize for WebHook {
fn deserialize<D: Deserializer>(deserializer: &mut D) -> Result<Self, D::Error> {
let val = try!(Value::deserialize(deserializer));
let val = Value::deserialize(deserializer)?;
let object_kind = match val.pointer("/object_kind") {
Some(&Value::String(ref kind)) => kind.to_string(),
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment