kennethnym/novem
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: prefer Arc<str> to String

Browse Source
This commit is contained in:
Kenneth
2026-05-23 18:45:44 +01:00
parent 1ef91cb41e
commit 1843622540
15 changed files with 524 additions and 544 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/api/auth.rs
View File

@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::{collections::HashMap, sync::Arc};
use serde::Deserialize;
@@ -11,9 +11,9 @@ pub struct CreateDeviceCode;
#[derive(Deserialize)]
pub(crate) struct DeviceCodeResponse {
pub device_code: String,
pub user_code: String,
pub vertification_uri: Option<String>,
pub device_code: Arc<str>,
pub user_code: Arc<str>,
pub vertification_uri: Option<Arc<str>>,
pub expires_in: u16,
// minimum number of seconds between polling for access token
@@ -46,14 +46,14 @@ impl query::QueryFn for CreateDeviceCode {
#[derive(Clone)]
pub struct RequestAccessToken {
pub device_code: String,
pub device_code: Arc<str>,
}
#[derive(Deserialize)]
pub struct RequestAccessTokenResponse {
pub access_token: String,
pub token_type: String,
pub scope: String,
pub access_token: Arc<str>,
pub token_type: Arc<str>,
pub scope: Arc<str>,
}
impl query::QueryFn for RequestAccessToken {

113
src/api/issues.rs
View File

@@ -1,4 +1,4 @@
use std::ops::Deref;
use std::sync::Arc;
use graphql_client::GraphQLQuery;
use serde::Deserialize;
@@ -21,33 +21,7 @@ type GitObjectID = String;
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Deserialize)]
#[serde(transparent)]
#[repr(transparent)]
pub(crate) struct Id(String);
impl Deref for Id {
type Target = String;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl From<&str> for Id {
fn from(value: &str) -> Self {
Self(value.to_owned())
}
}
impl From<String> for Id {
fn from(value: String) -> Self {
Self(value)
}
}
impl From<Id> for String {
fn from(value: Id) -> Self {
value.0
}
}
pub(crate) struct Id(pub(crate) Arc<str>);
#[derive(Debug, Deserialize)]
pub(crate) struct PullRequestPaginatedResponse {
@@ -59,32 +33,32 @@ pub(crate) struct PullRequestPaginatedResponse {
#[derive(Debug, Deserialize)]
pub(crate) struct PullRequest {
pub(crate) id: Id,
pub(crate) title: String,
pub(crate) title: Arc<str>,
pub(crate) state: PullRequestState,
pub(crate) is_draft: bool,
pub(crate) repo_slug: String,
pub(crate) repo_slug: Arc<str>,
}
#[derive(Debug, Deserialize)]
pub(crate) struct DetailedPullRequest {
pub(crate) title: String,
pub(crate) title: Arc<str>,
pub(crate) state: PullRequestState,
pub(crate) is_draft: bool,
pub(crate) body: String,
pub(crate) body: Arc<str>,
pub(crate) created_at: Option<chrono::DateTime<chrono::FixedOffset>>,
pub(crate) author: Option<super::user::Actor>,
pub(crate) base_branch_name: String,
pub(crate) base_repo_slug: String,
pub(crate) base_ref: String,
pub(crate) head_branch_name: String,
pub(crate) head_ref: String,
pub(crate) head_repo_slug: String,
pub(crate) base_branch_name: Arc<str>,
pub(crate) base_repo_slug: Arc<str>,
pub(crate) base_ref: Arc<str>,
pub(crate) head_branch_name: Arc<str>,
pub(crate) head_ref: Arc<str>,
pub(crate) head_repo_slug: Arc<str>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct PullRequestTimeline {
pub(crate) items: Vec<PullRequestTimelineItem>,
pub(crate) end_cursor: Option<String>,
pub(crate) end_cursor: Option<Arc<str>>,
pub(crate) has_next_page: bool,
}
@@ -245,12 +219,6 @@ pub(crate) struct TimelineActor {
pub(crate) avatar_url: Option<String>,
}
impl std::fmt::Display for Id {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize)]
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
pub(crate) enum PullRequestState {
@@ -310,6 +278,24 @@ pub(crate) struct ListPullRequests {
pub page: u32,
}
impl std::fmt::Display for Id {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
}
}
impl From<String> for Id {
fn from(value: String) -> Self {
Self(value.into())
}
}
impl From<&str> for Id {
fn from(value: &str) -> Self {
Self(value.into())
}
}
impl query::QueryFn for ListPullRequests {
type Data = PullRequestPaginatedResponse;
type Error = api::Error;
@@ -357,13 +343,14 @@ impl query::QueryFn for ListPullRequests {
| PullRequestPaginationQuerySearchEdgesNode::PullRequest(p) => {
Some(PullRequest {
id: p.id.into(),
title: p.title,
title: p.title.into(),
state: p.state,
is_draft: p.is_draft,
repo_slug: format!(
"{}/{}",
p.repository.owner.login, p.repository.name
),
)
.into(),
})
}
| _ => None,
@@ -399,7 +386,7 @@ impl query::QueryFn for FetchPullRequest {
}
let gql = PullRequestQuery::build_query(pull_request_query::Variables {
id: self.id.clone().into(),
id: self.id.to_string(),
});
let res = c.github_graphql_request(&gql)?.send().await?;
@@ -421,26 +408,26 @@ impl query::QueryFn for FetchPullRequest {
})?;
Ok(DetailedPullRequest {
title: p.title,
title: p.title.into(),
state: p.state,
is_draft: p.is_draft,
body: p.body,
body: p.body.into(),
author: p.author.map(|it| api::user::Actor {
login: it.login,
avatar_url: it.avatar_url,
login: it.login.into(),
avatar_url: it.avatar_url.into(),
}),
base_repo_slug: p
.base_repository
.map(|it| it.name_with_owner)
.map(|it| it.name_with_owner.into())
.unwrap_or_default(),
base_branch_name: p.base_ref_name,
base_ref: p.base_ref_oid,
base_branch_name: p.base_ref_name.into(),
base_ref: p.base_ref_oid.into(),
head_repo_slug: p
.head_repository
.map(|it| it.name_with_owner)
.map(|it| it.name_with_owner.into())
.unwrap_or_default(),
head_branch_name: p.head_ref_name,
head_ref: p.head_ref_oid,
head_branch_name: p.head_ref_name.into(),
head_ref: p.head_ref_oid.into(),
created_at: Some(created_at),
})
}
@@ -473,7 +460,7 @@ impl query::QueryFn for FetchPullRequestFileTree {
} else {
let gql =
PullRequestFileTreeQuery::build_query(pull_request_file_tree_query::Variables {
id: self.id.clone().into(),
id: self.id.to_string(),
first: self.first,
});
@@ -558,7 +545,7 @@ impl query::QueryFn for FetchPullRequestFileTree {
pub(crate) struct FetchPullRequestTimeline {
pub(crate) id: Id,
pub(crate) first: i64,
pub(crate) after: Option<String>,
pub(crate) after: Option<Arc<str>>,
}
impl query::QueryFn for FetchPullRequestTimeline {
@@ -841,9 +828,9 @@ impl query::QueryFn for FetchPullRequestTimeline {
} else {
let gql =
PullRequestTimelineQuery::build_query(pull_request_timeline_query::Variables {
id: self.id.clone().into(),
id: self.id.to_string(),
first: self.first,
after: self.after.clone(),
after: self.after.as_ref().map(|it| it.to_string()),
});
let res = c.github_graphql_request(&gql)?.send().await?;
@@ -892,7 +879,7 @@ impl query::QueryFn for FetchPullRequestTimeline {
Ok(PullRequestTimeline {
items,
end_cursor: timeline.page_info.end_cursor,
end_cursor: timeline.page_info.end_cursor.map(|it| it.into()),
has_next_page: timeline.page_info.has_next_page,
})
}

100
src/api/mock.rs
View File

@@ -175,12 +175,12 @@ mod tests {
assert_eq!(merged.state, issues::PullRequestState::Merged);
assert!(merged.body.contains("| Stage | Owner | Status |"));
assert_eq!(
merged.author.as_ref().map(|author| author.login.as_str()),
merged.author.as_ref().map(|author| author.login.as_ref()),
Some("rorycraft")
);
assert_eq!(merged.base_branch_name.as_str(), "main");
assert_eq!(merged.base_branch_name.as_ref(), "main");
assert_eq!(
merged.head_branch_name.as_str(),
merged.head_branch_name.as_ref(),
"feat/release-handoff-checklist"
);
assert_eq!(
@@ -196,12 +196,12 @@ mod tests {
documented_failover
.author
.as_ref()
.map(|author| author.login.as_str()),
.map(|author| author.login.as_ref()),
Some("kennethnym")
);
assert_eq!(documented_failover.base_branch_name.as_str(), "main");
assert_eq!(documented_failover.base_branch_name.as_ref(), "main");
assert_eq!(
documented_failover.head_branch_name.as_str(),
documented_failover.head_branch_name.as_ref(),
"docs/manual-failover-steps"
);
assert_eq!(
@@ -209,17 +209,17 @@ mod tests {
Some(chrono::DateTime::parse_from_rfc3339("2026-04-24T06:40:00Z").unwrap())
);
assert!(dashboard_markdown.body.contains("```rust"));
assert_eq!(dashboard_markdown.base_branch_name.as_str(), "main");
assert_eq!(dashboard_markdown.base_branch_name.as_ref(), "main");
assert_eq!(
dashboard_markdown.head_branch_name.as_str(),
dashboard_markdown.head_branch_name.as_ref(),
"feat/cached-issue-pane"
);
assert_eq!(
dashboard_markdown.base_ref.as_str(),
dashboard_markdown.base_ref.as_ref(),
"5e8745bfcc0c90c226d3c6af84226d6d4a5ec2d1"
);
assert_eq!(
dashboard_markdown.head_ref.as_str(),
dashboard_markdown.head_ref.as_ref(),
"2bc41de7731b9ef48f7d64ee9f0d5f497dbe0a51"
);
assert_eq!(
@@ -230,20 +230,20 @@ mod tests {
cached_repo_picker
.author
.as_ref()
.map(|author| author.login.as_str()),
.map(|author| author.login.as_ref()),
Some("kennethnym")
);
assert_eq!(cached_repo_picker.base_branch_name.as_str(), "main");
assert_eq!(cached_repo_picker.base_branch_name.as_ref(), "main");
assert_eq!(
cached_repo_picker.head_branch_name.as_str(),
cached_repo_picker.head_branch_name.as_ref(),
"feat/cached-repo-picker"
);
assert_eq!(
cached_repo_picker.base_ref.as_str(),
cached_repo_picker.base_ref.as_ref(),
"5e8745bfcc0c90c226d3c6af84226d6d4a5ec2d1"
);
assert_eq!(
cached_repo_picker.head_ref.as_str(),
cached_repo_picker.head_ref.as_ref(),
"13af7d0b48a6ce0b22d48c9b6c1c78dfcd94e6a0"
);
assert_eq!(
@@ -254,12 +254,12 @@ mod tests {
worker_split
.author
.as_ref()
.map(|author| author.login.as_str()),
.map(|author| author.login.as_ref()),
Some("leaferiksen")
);
assert_eq!(worker_split.base_branch_name.as_str(), "main");
assert_eq!(worker_split.base_branch_name.as_ref(), "main");
assert_eq!(
worker_split.head_branch_name.as_str(),
worker_split.head_branch_name.as_ref(),
"feat/worker-context-envelope"
);
assert_eq!(
@@ -270,12 +270,12 @@ mod tests {
spacing_tokens
.author
.as_ref()
.map(|author| author.login.as_str()),
.map(|author| author.login.as_ref()),
Some("mariahops")
);
assert_eq!(spacing_tokens.base_branch_name.as_str(), "main");
assert_eq!(spacing_tokens.base_branch_name.as_ref(), "main");
assert_eq!(
spacing_tokens.head_branch_name.as_str(),
spacing_tokens.head_branch_name.as_ref(),
"chore/dashboard-spacing-scale"
);
assert_eq!(
@@ -294,13 +294,13 @@ mod tests {
let base_query = fetch_file_content(
"kennethnym/novem",
"src/query.rs",
Some(dashboard_markdown.base_ref.as_str()),
Some(dashboard_markdown.base_ref.as_ref()),
)
.expect("base query fixture should exist");
let head_query = fetch_file_content(
"kennethnym/novem",
"src/query.rs",
Some(dashboard_markdown.head_ref.as_str()),
Some(dashboard_markdown.head_ref.as_ref()),
)
.expect("head query fixture should exist");
let base_query = std::str::from_utf8(base_query.as_ref())
@@ -317,13 +317,13 @@ mod tests {
let base_repo = fetch_file_content(
"kennethnym/novem",
"src/api/repo.rs",
Some(cached_repo_picker.base_ref.as_str()),
Some(cached_repo_picker.base_ref.as_ref()),
)
.expect("base repo fixture should exist");
let head_repo = fetch_file_content(
"kennethnym/novem",
"src/api/repo.rs",
Some(cached_repo_picker.head_ref.as_str()),
Some(cached_repo_picker.head_ref.as_ref()),
)
.expect("head repo fixture should exist");
let base_repo =
@@ -364,15 +364,15 @@ mod tests {
for path in file_paths {
fetch_file_content(
dashboard_markdown.base_repo_slug.as_str(),
dashboard_markdown.base_repo_slug.as_ref(),
path,
Some(dashboard_markdown.base_ref.as_str()),
Some(dashboard_markdown.base_ref.as_ref()),
)
.unwrap_or_else(|_| panic!("base fixture should exist for {path}"));
fetch_file_content(
dashboard_markdown.head_repo_slug.as_str(),
dashboard_markdown.head_repo_slug.as_ref(),
path,
Some(dashboard_markdown.head_ref.as_str()),
Some(dashboard_markdown.head_ref.as_ref()),
)
.unwrap_or_else(|_| panic!("head fixture should exist for {path}"));
}
@@ -440,36 +440,30 @@ mod tests {
.expect("third timeline fixture json should parse");
let first_page_nodes = match first_page.node.as_ref() {
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => {
pull_request
.timeline_items
.nodes
.as_ref()
.expect("first timeline fixture page should contain timeline nodes")
}
| _ => panic!("first timeline fixture page should resolve to a pull request node"),
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => pull_request
.timeline_items
.nodes
.as_ref()
.expect("first timeline fixture page should contain timeline nodes"),
| _ => panic!("first timeline fixture page should resolve to a pull request node"),
};
let second_page_nodes = match second_page.node.as_ref() {
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => {
pull_request
.timeline_items
.nodes
.as_ref()
.expect("second timeline fixture page should contain timeline nodes")
}
| _ => panic!("second timeline fixture page should resolve to a pull request node"),
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => pull_request
.timeline_items
.nodes
.as_ref()
.expect("second timeline fixture page should contain timeline nodes"),
| _ => panic!("second timeline fixture page should resolve to a pull request node"),
};
let third_page_nodes = match third_page.node.as_ref() {
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => {
pull_request
.timeline_items
.nodes
.as_ref()
.expect("third timeline fixture page should contain timeline nodes")
}
| _ => panic!("third timeline fixture page should resolve to a pull request node"),
| Some(issues::PullRequestTimelineResponseNode::PullRequest(pull_request)) => pull_request
.timeline_items
.nodes
.as_ref()
.expect("third timeline fixture page should contain timeline nodes"),
| _ => panic!("third timeline fixture page should resolve to a pull request node"),
};
assert_eq!(

45
src/api/repo.rs
View File

@@ -1,11 +1,10 @@
use futures::{FutureExt, TryFutureExt};
use std::sync::Arc;
use reqwest::Method;
use serde::Deserialize;
use tokio::sync::OwnedRwLockReadGuard;
use crate::{
api,
query::{self, Query, fetch_query},
api, query,
util::{self, file},
};
@@ -31,9 +30,9 @@ pub struct Owner {
#[derive(Debug, Clone)]
pub struct FileRef {
pub repo_slug: String,
pub path: String,
pub reff: Option<String>,
pub repo_slug: Arc<str>,
pub path: Arc<str>,
pub reff: Option<Arc<str>>,
}
#[derive(Clone)]
@@ -67,9 +66,9 @@ impl query::QueryFn for List {
#[derive(Clone)]
pub struct FetchFileContent {
pub repo_slug: String,
pub path: String,
pub reff: Option<String>,
pub repo_slug: Arc<str>,
pub path: Arc<str>,
pub reff: Option<Arc<str>>,
}
impl query::QueryFn for FetchFileContent {
@@ -79,8 +78,8 @@ impl query::QueryFn for FetchFileContent {
fn key(&self) -> query::Key {
match &self.reff {
| Some(reff) => format!("repo/fetch/{}/{}/{}", self.repo_slug, self.path, reff).into(),
| None => format!("repo/fetch/{}/{}", self.repo_slug, self.path).into(),
| Some(reff) => format!("repo/fetch/{}/{}/{}", self.repo_slug, self.path, reff).into(),
| None => format!("repo/fetch/{}/{}", self.repo_slug, self.path).into(),
}
}
@@ -95,11 +94,11 @@ impl query::QueryFn for FetchFileContent {
}
let path = match &self.reff {
| Some(reff) => format!(
"/repos/{}/contents/{}?ref={}",
self.repo_slug, self.path, reff
),
| None => format!("/repos/{}/contents/{}", self.repo_slug, self.path),
| Some(reff) => format!(
"/repos/{}/contents/{}?ref={}",
self.repo_slug, self.path, reff
),
| None => format!("/repos/{}/contents/{}", self.repo_slug, self.path),
};
let res = c
@@ -137,8 +136,8 @@ impl query::QueryFn for FetchFileDiff {
c: &<FetchFileDiff as query::QueryFn>::Context,
) -> Result<Option<bytes::Bytes>, api::Error> {
let path = match &r.reff {
| Some(reff) => format!("/repos/{}/contents/{}?ref={}", r.repo_slug, r.path, reff),
| None => format!("/repos/{}/contents/{}", r.repo_slug, r.path),
| Some(reff) => format!("/repos/{}/contents/{}?ref={}", r.repo_slug, r.path, reff),
| None => format!("/repos/{}/contents/{}", r.repo_slug, r.path),
};
let res = c
@@ -160,10 +159,10 @@ impl query::QueryFn for FetchFileDiff {
let (old, new) = tokio::join!(fetch_content(&self.base, c), fetch_content(&self.head, c),);
match (old, new) {
| (Ok(Some(old)), Ok(Some(new))) => Ok(util::diff::diff_content(old, new)),
| _ => Err(api::Error::MalformedResponse(
"failed to fetch content".to_string(),
)),
| (Ok(Some(old)), Ok(Some(new))) => Ok(util::diff::diff_content(old, new)),
| _ => Err(api::Error::MalformedResponse(
"failed to fetch content".to_string(),
)),
}
}
}

16
src/api/user.rs
View File

@@ -1,4 +1,4 @@
use std::ops::Deref;
use std::{ops::Deref, sync::Arc};
use reqwest::Method;
use serde::{Deserialize, Serialize};
@@ -12,18 +12,18 @@ pub struct Id(u64);
#[derive(Debug, Deserialize)]
pub struct User {
pub login: String,
pub login: Arc<str>,
pub id: Id,
pub avatar_url: String,
pub html_url: String,
pub name: Option<String>,
pub email: Option<String>,
pub avatar_url: Arc<str>,
pub html_url: Arc<str>,
pub name: Option<Arc<str>>,
pub email: Option<Arc<str>>,
}
#[derive(Debug, Deserialize)]
pub(crate) struct Actor {
pub(crate) login: String,
pub(crate) avatar_url: String,
pub(crate) login: Arc<str>,
pub(crate) avatar_url: Arc<str>,
}
impl Deref for Id {