Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paper over a few edge cases in RepoCommits #677

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/list_forks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async fn main() -> octocrab::Result<()> {
.await?;

for f in forks {
println!("fork: {}", f.owner.unwrap().login);
println!("fork: {:?}", f.owner.unwrap().login);
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion examples/list_gists_for_token_holder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async fn main() -> octocrab::Result<()> {
}

println!(
"User '{username}' has {count} gists:",
"User '{username:?}' has {count} gists:",
username = current_user_name,
count = gists.len()
);
Expand Down
24 changes: 23 additions & 1 deletion src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ pub struct IssuePullRequest {
#[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize, Deserialize)]
#[non_exhaustive]
pub struct Author {
pub login: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub login: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right, given that GH fixed the API upstream?

#656 looks correct (after a rebase/merge), instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I'll close this in favor of #656

pub id: UserId,
pub node_id: String,
pub avatar_url: Url,
Expand Down Expand Up @@ -441,6 +442,27 @@ where
}
}

/// Sometimes github returns an empty map instead of a null.
fn empty_map_is_none<'de, D, T>(deserializer: D) -> Result<Option<T>, D::Error>
where
T: Deserialize<'de>,
D: Deserializer<'de>,
{
// try to deserialize our input map to generic serde
#[derive(Deserialize)]
#[serde(untagged)]
enum Helper<I> {
Value(Option<I>),
Empty {},
}
let helper = Helper::deserialize(deserializer)?;
let res = match helper {
Helper::Value(inner) => inner,
Helper::Empty {} => None,
};
Ok(res)
}

/// The full profile for a user
#[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize, Deserialize)]
#[non_exhaustive]
Expand Down
2 changes: 2 additions & 0 deletions src/models/repos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ pub struct RepoCommit {
pub html_url: String,
pub comments_url: String,
pub commit: RepoCommitPage,
#[serde(deserialize_with = "crate::models::empty_map_is_none")]
pub author: Option<Author>,
#[serde(deserialize_with = "crate::models::empty_map_is_none")]
pub committer: Option<Author>,
pub parents: Vec<Commit>,

Expand Down
2 changes: 1 addition & 1 deletion tests/current_user_orgs_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ async fn should_return_page_with_invitations() {
{
assert_eq!(items.len(), 2);
assert_eq!(items[0].role, "admin");
assert_eq!(items[0].user.login, "davidmhewitt");
assert_eq!(items[0].user.login.as_deref(), Some("davidmhewitt"));
assert_eq!(items[0].organization.login, "elementary");
assert_eq!(items[1].organization.login, "EpicGames");
}
Expand Down
2 changes: 1 addition & 1 deletion tests/follow_redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const REPO: &str = "repo";
async fn should_return_page_with_users() {
let star_gazers: Vec<StarGazer> =
serde_json::from_str(include_str!("resources/stargazers.json")).unwrap();
let login1: String = star_gazers[0].user.as_ref().unwrap().login.clone();
let login1: Option<String> = star_gazers[0].user.as_ref().unwrap().login.clone();
let page_response = FakePage { items: star_gazers };
let template = ResponseTemplate::new(200).set_body_json(&page_response);
let mock_server = setup_api(template).await;
Expand Down
2 changes: 1 addition & 1 deletion tests/org_installations_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ async fn should_return_org_installation() {
} = result.unwrap();
{
assert_eq!(installation_id, InstallationId(1));
assert_eq!(login, "github");
assert_eq!(login.as_deref(), Some("github"));
}
}
2 changes: 1 addition & 1 deletion tests/org_members_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const USERNAME: &str = "mona";
async fn should_return_page_with_users() {
let org_members: Author =
serde_json::from_str(include_str!("resources/org_members.json")).unwrap();
let login: String = org_members.login.clone();
let login: Option<String> = org_members.login.clone();
let page_response = FakePage {
items: vec![org_members],
};
Expand Down
2 changes: 1 addition & 1 deletion tests/projects_project_get_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ async fn should_get_projects_by_its_id() {
let result = project.unwrap();

assert_eq!(result.name, "Organization Roadmap");
assert_eq!(result.creator.login, "octocat");
assert_eq!(result.creator.login.as_deref(), Some("octocat"));
}
2 changes: 1 addition & 1 deletion tests/projects_user_list_projects_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,5 @@ async fn should_list_user_projects() {
let Page { items, .. } = result.unwrap();

assert_eq!(items[0].name, "My Projects");
assert_eq!(items[1].creator.login, "octocat");
assert_eq!(items[1].creator.login.as_deref(), Some("octocat"));
}
2 changes: 1 addition & 1 deletion tests/pull_request_commits_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ async fn should_return_pull_request_commits() {
let RepoCommit { author, .. } = commits.items.first().unwrap();

{
assert_eq!(author.clone().unwrap().login, "octocat");
assert_eq!(author.clone().unwrap().login.as_deref(), Some("octocat"));
}
}
2 changes: 1 addition & 1 deletion tests/repo_contributors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ async fn should_return_repo_contributors() {
} = contributors.items.first().unwrap();

{
assert_eq!(login, "XAMPPRocky");
assert_eq!(login.as_deref(), Some("XAMPPRocky"));
assert!(*contributions > 0);
}
}
62 changes: 62 additions & 0 deletions tests/repos_commit_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,65 @@ async fn should_return_list_of_commits() {
);
});
}

#[tokio::test]
async fn should_parse_all_null_commit_author() {
let repos_list_commits_json: Vec<RepoCommit> = serde_json::from_str(include_str!(
"resources/repos_list_commits_null_authors.json"
))
.unwrap();
let template = ResponseTemplate::new(201).set_body_json(&repos_list_commits_json);
let mock_server = setup_repos_commits_api(template).await;
let client = setup_octocrab(&mock_server.uri());

let result = client
.repos(OWNER.to_owned(), REPO.to_owned())
.list_commits()
.send()
.await;

assert!(
result.is_ok(),
"expected successful result, got error: {:#?}",
result
);

let result = result.unwrap();
let result = result.items;

assert!(
result.len() == 1,
"expected '1' for len(), got {:#?}",
result.len()
);

result.iter().for_each(|commit| {
println!("Commit = {:#?}", commit);
assert!(
commit.sha == "24606b5f326a1356f031dd06431cfb0beddd475f",
"expected '24606b5f326a1356f031dd06431cfb0beddd475f' value, got {:#?}",
commit.sha
);

assert!(
commit
.commit
.author
.as_ref()
.unwrap()
.date
.unwrap()
.to_rfc3339()
== "2024-04-30T08:54:10+00:00",
"expected '2024-04-30T08:54:10+00:00' value, got {:#?}",
commit
.commit
.author
.as_ref()
.unwrap()
.date
.unwrap()
.to_rfc3339()
);
});
}
6 changes: 3 additions & 3 deletions tests/repos_stargazers_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const REPO: &str = "repo";
async fn should_return_page_with_users() {
let star_gazers: Vec<StarGazer> =
serde_json::from_str(include_str!("resources/stargazers.json")).unwrap();
let login1: String = star_gazers[0].user.as_ref().unwrap().login.clone();
let login1: Option<String> = star_gazers[0].user.as_ref().unwrap().login.clone();
let page_response = FakePage { items: star_gazers };
let template = ResponseTemplate::new(200).set_body_json(&page_response);
let mock_server = setup_api(template).await;
Expand All @@ -65,8 +65,8 @@ async fn should_return_page_with_users() {
async fn should_return_page_with_all_users() {
let star_gazers: Vec<StarGazer> =
serde_json::from_str(include_str!("resources/stargazers.json")).unwrap();
let login1: String = star_gazers[0].user.as_ref().unwrap().login.clone();
let login2: String = star_gazers[1].user.as_ref().unwrap().login.clone();
let login1: Option<String> = star_gazers[0].user.as_ref().unwrap().login.clone();
let login2: Option<String> = star_gazers[1].user.as_ref().unwrap().login.clone();
let page_response = FakePage { items: star_gazers };
let template = ResponseTemplate::new(200).set_body_json(&page_response);
let mock_server = setup_api(template).await;
Expand Down
124 changes: 42 additions & 82 deletions tests/resources/repos_list_commits.json
Original file line number Diff line number Diff line change
@@ -1,83 +1,43 @@
[
{
"url": "https://api.github.com/repos/owner/my_repo/commits/24606b5f326a1356f031dd06431cfb0beddd475f",
"sha": "24606b5f326a1356f031dd06431cfb0beddd475f",
"node_id": "C_kwDOExNuVtoAKDIzNjA2YjVmMzI2YTEzNTZmMDMxZGQwNjQzMWNmYjBiZWRkZDQ3NWY",
"html_url": "https://github.com/owner/my_repo/commit/24606b5f326a1356f031dd06431cfb0beddd475f",
"comments_url": "https://api.github.com/repos/owner/my_repo/commits/24606b5f326a1356f031dd06431cfb0beddd475f/comments",
"commit": {
"url": "https://api.github.com/repos/owner/my_repo/git/commits/24606b5f326a1356f031dd06431cfb0beddd475f",
"author": {
"name": "First Lastname",
"email": "first.last@orgcom",
"date": "2024-04-30T08:54:10Z"
},
"committer": {
"name": "GitHub",
"email": "[email protected]",
"date": "2024-04-30T08:54:10Z"
},
"message": "Improved logs (#21717)",
"comment_count": 0,
"tree": {
"sha": "cadf65e5b5e1ba2b6293e5f95cb3012868b82082",
"url": "https://api.github.com/repos/owner/my_repo/git/trees/cadf65e5b5e1ba2b6293e5f95cb3012868b82082"
},
"verification": {
"verified": true,
"reason": "valid",
"payload": "tree cadf65e5b5e1ba2b6293e5f95cb3012868b82082\nparent 7cf97159fe31b0e011bbb24109b5e10d278f551f\nauthor First Samuel Leal Lastname [email protected] +0200\ncommitter [email protected] 1714467250 +0200\n\nImproved logs related to GetActiveVersionedContainer Error (#21717)\n\n",
"signature": "-----BEGIN PGP SIGNATURE-----\n\nwsFcBAABCAAQBQJnNLGyCRD1/dR8FrJdSOVEqr39d3HZaUPGvI/WyKJKZhFkjI/aMk\nOd2QBPCn/QE51VqkzxNy\n=Cjql\n-----END PGP SIGNATURE-----\n"
}
},
"author": {
"login": "flastname",
"id": 8143842,
"node_id": "MDQ6VXNlcjkxNDI4NDA=",
"avatar_url": "https://avatars.githubusercontent.com/u/8143842?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/flastname",
"html_url": "https://github.com/flastname",
"followers_url": "https://api.github.com/users/flastname/followers",
"following_url": "https://api.github.com/users/flastname/following%7B/other_user",
"gists_url": "https://api.github.com/users/flastname/gists%7B/gist_id",
"starred_url": "https://api.github.com/users/flastname/starred%7B/owner%7B/repo",
"subscriptions_url": "https://api.github.com/users/flastname/subscriptions",
"organizations_url": "https://api.github.com/users/flastname/orgs",
"repos_url": "https://api.github.com/users/flastname/repos",
"events_url": "https://api.github.com/users/flastname/events%7B/privacy",
"received_events_url": "https://api.github.com/users/flastname/received_events",
"type": "User",
"site_admin": false,
"patch_url": null
},
"committer": {
"login": "web-flow",
"id": 29874547,
"node_id": "MDQ6VXNlcjE5ODY0NDQ3",
"avatar_url": "https://avatars.githubusercontent.com/u/19864447?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/web-flow",
"html_url": "https://github.com/web-flow",
"followers_url": "https://api.github.com/users/web-flow/followers",
"following_url": "https://api.github.com/users/web-flow/following%7B/other_user",
"gists_url": "https://api.github.com/users/web-flow/gists%7B/gist_id",
"starred_url": "https://api.github.com/users/web-flow/starred%7B/owner%7B/repo",
"subscriptions_url": "https://api.github.com/users/web-flow/subscriptions",
"organizations_url": "https://api.github.com/users/web-flow/orgs",
"repos_url": "https://api.github.com/users/web-flow/repos",
"events_url": "https://api.github.com/users/web-flow/events%7B/privacy",
"received_events_url": "https://api.github.com/users/web-flow/received_events",
"type": "User",
"site_admin": false,
"patch_url": null
},
"parents": [
{
"url": "https://api.github.com/repos/owner/my_repo/commits/7cf97159fe31b0e011bbb24109b5e10d278f551f",
"sha": "7cf97159fe31b0e011bbb24109b5e10d278f551f",
"html_url": "https://github.com/owner/my_repo/commit/7cf97159fe31b0e011bbb24109b5e10d278f551f"
}
]
}
]
{
"url": "https://api.github.com/repos/owner/my_repo/commits/24606b5f326a1356f031dd06431cfb0beddd475f",
"sha": "24606b5f326a1356f031dd06431cfb0beddd475f",
"node_id": "C_kwDOExNuVtoAKDIzNjA2YjVmMzI2YTEzNTZmMDMxZGQwNjQzMWNmYjBiZWRkZDQ3NWY",
"html_url": "https://github.com/owner/my_repo/commit/24606b5f326a1356f031dd06431cfb0beddd475f",
"comments_url": "https://api.github.com/repos/owner/my_repo/commits/24606b5f326a1356f031dd06431cfb0beddd475f/comments",
"commit": {
"url": "https://api.github.com/repos/owner/my_repo/git/commits/24606b5f326a1356f031dd06431cfb0beddd475f",
"author": {
"name": "First Lastname",
"email": "first.last@orgcom",
"date": "2024-04-30T08:54:10Z"
},
"committer": {
"name": "GitHub",
"email": "[email protected]",
"date": "2024-04-30T08:54:10Z"
},
"message": "Improved logs (#21717)",
"comment_count": 0,
"tree": {
"sha": "cadf65e5b5e1ba2b6293e5f95cb3012868b82082",
"url": "https://api.github.com/repos/owner/my_repo/git/trees/cadf65e5b5e1ba2b6293e5f95cb3012868b82082"
},
"verification": {
"verified": true,
"reason": "valid",
"payload": "tree cadf65e5b5e1ba2b6293e5f95cb3012868b82082\nparent 7cf97159fe31b0e011bbb24109b5e10d278f551f\nauthor First Samuel Leal Lastname [email protected] +0200\ncommitter [email protected] 1714467250 +0200\n\nImproved logs related to GetActiveVersionedContainer Error (#21717)\n\n",
"signature": "-----BEGIN PGP SIGNATURE-----\n\nwsFcBAABCAAQBQJnNLGyCRD1/dR8FrJdSOVEqr39d3HZaUPGvI/WyKJKZhFkjI/aMk\nOd2QBPCn/QE51VqkzxNy\n=Cjql\n-----END PGP SIGNATURE-----\n"
}
},
"author": {},
"committer": null,
"parents": [
{
"url": "https://api.github.com/repos/owner/my_repo/commits/7cf97159fe31b0e011bbb24109b5e10d278f551f",
"sha": "7cf97159fe31b0e011bbb24109b5e10d278f551f",
"html_url": "https://github.com/owner/my_repo/commit/7cf97159fe31b0e011bbb24109b5e10d278f551f"
}
]
}
]
Loading
Loading