Skip to content

Commit

Permalink
clippy tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
zaknesler committed Mar 19, 2024
1 parent 25f2627 commit 2e168e5
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 65 deletions.
21 changes: 10 additions & 11 deletions src/api/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Client<WithoutToken> {
/// Generate a new URL to authorize a user, along with a CSRF token to be verified from Spotify's response
pub fn new_authorize_url(&self) -> (Url, CsrfToken) {
self.oauth
.authorize_url(|| CsrfToken::new_random())
.authorize_url(CsrfToken::new_random)
.add_extra_param("show_dialog", "true")
.add_scopes(SPOTIFY_OAUTH2_SCOPES.iter().map(|scope| Scope::new(scope.to_string())))
.url()
Expand Down Expand Up @@ -197,7 +197,6 @@ impl Client<WithToken> {
None,
)
.await
.map_err(|err| err.into())
}

/// Get all tracks saved by the current user, returning only the ID/URI data
Expand All @@ -223,10 +222,7 @@ impl Client<WithToken> {
}

/// Remove tracks from the current user's saved tracks by ID
pub async fn current_user_saved_tracks_remove_ids(
&self,
ids: &Vec<TrackId>,
) -> ClientResult<()> {
pub async fn current_user_saved_tracks_remove_ids(&self, ids: &[TrackId]) -> ClientResult<()> {
tracing::debug!("DELETE /me/tracks");

// Endpoint can only be sent a maximum of 50 IDs
Expand Down Expand Up @@ -296,11 +292,14 @@ impl Client<WithToken> {
.await;

// An empty response means success
Ok(match res {
Ok(_) => (),

match res {
Err(_err @ ClientError::EmptyResponse) => (),
Err(err) => return Err(err),
})
_ => (),
};

Ok(())
}

/// Get all tracks in a playlist, returning only the ID/URI data
Expand Down Expand Up @@ -334,7 +333,7 @@ impl Client<WithToken> {
pub async fn playlist_add_ids(
&self,
PlaylistId(id): &PlaylistId,
ids: &Vec<TrackId>,
ids: &[TrackId],
) -> ClientResult<Vec<SnapshotId>> {
tracing::debug!("POST /playlists/{}/tracks", id);

Expand Down Expand Up @@ -365,7 +364,7 @@ impl Client<WithToken> {
pub async fn playlist_remove_ids(
&self,
PlaylistId(id): &PlaylistId,
ids: &Vec<TrackId>,
ids: &[TrackId],
) -> ClientResult<Vec<SnapshotId>> {
tracing::debug!("DELETE /playlists/{}/tracks", id);

Expand Down
10 changes: 5 additions & 5 deletions src/api/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ impl TryFrom<StandardTokenResponse<EmptyExtraTokenFields, BasicTokenType>> for T
) -> Result<Self, Self::Error> {
let scopes = res.scopes().ok_or_else(|| ClientError::SpotifyDidNotReturnScopes)?.clone();
let expires_in = chrono::Duration::from_std(
res.expires_in()
.ok_or_else(|| ClientError::SpotifyDidNotReturnExpiresIn)?
.clone(),
res.expires_in().ok_or_else(|| ClientError::SpotifyDidNotReturnExpiresIn)?,
)?;

let expiration_offset =
chrono::Duration::try_seconds(EXPIRATION_OFFSET_SECONDS).expect("offset out of bounds");

// Set the expiration to 1 minute before the "expires_in" duration returned from Spotify
let expires_at =
chrono::Utc::now() + expires_in - chrono::Duration::seconds(EXPIRATION_OFFSET_SECONDS);
let expires_at = chrono::Utc::now() + expires_in - expiration_offset;

Ok(Token {
access_token: res.access_token().secret().to_string(),
Expand Down
4 changes: 2 additions & 2 deletions src/api/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ pub async fn check_playlist_editable(
user_id: &UserId,
) -> ClientResult<bool> {
// Get playlist details
let playlist = client.playlist_partial(&id).await?;
let playlist = client.playlist_partial(id).await?;

// If the user owns the playlist, don't try making request
if playlist.owner.id == *user_id {
return Ok(true);
}

// Attempt to update name to itself to check permissions
let res = client.playlist_update_name(&id, &playlist.name).await;
let res = client.playlist_update_name(id, &playlist.name).await;

Ok(match res {
Ok(_) => true,
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Config {
pub fn try_parse() -> BaseResult<Config> {
// Split at the first underscore to use the first word as the section identifier
let config = Figment::new()
.merge(Env::raw().map(|key| key.as_str().to_lowercase().replacen("_", ".", 1).into()))
.merge(Env::raw().map(|key| key.as_str().to_lowercase().replacen('_', ".", 1).into()))
.extract()?;

Ok(config)
Expand Down
10 changes: 5 additions & 5 deletions src/db/model/watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ impl TryFrom<&Row<'_>> for Watcher {
playlist_to: PlaylistType::try_from_value(&row.get::<_, String>(3)?)?,
should_remove: row.get(4)?,
sync_interval: row.get::<_, String>(5)?.parse()?,
last_sync_at: row.get::<_, Option<String>>(6)?.map(|val| val.parse().ok()).flatten(),
next_sync_at: row.get::<_, Option<String>>(7)?.map(|val| val.parse().ok()).flatten(),
last_sync_at: row.get::<_, Option<String>>(6)?.and_then(|val| val.parse().ok()),
next_sync_at: row.get::<_, Option<String>>(7)?.and_then(|val| val.parse().ok()),
created_at: row.get::<_, String>(8)?.parse()?,
})
}
Expand Down Expand Up @@ -73,9 +73,9 @@ impl FromStr for SyncInterval {
impl From<SyncInterval> for chrono::Duration {
fn from(value: SyncInterval) -> Self {
match value {
SyncInterval::Hour => chrono::Duration::hours(1),
SyncInterval::Day => chrono::Duration::days(1),
SyncInterval::Week => chrono::Duration::weeks(1),
SyncInterval::Hour => chrono::Duration::try_hours(1).expect("won't overflow"),
SyncInterval::Day => chrono::Duration::try_days(1).expect("won't overflow"),
SyncInterval::Week => chrono::Duration::try_weeks(1).expect("won't overflow"),
}
}
}
5 changes: 2 additions & 3 deletions src/db/repo/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,11 @@ impl TransferRepo {
/// Fetch all transfers for a watcher by ID.
#[allow(dead_code)]
pub fn get_transfers_for_watcher(&self, id: u32) -> DbResult<Vec<Transfer>> {
Ok(self
.ctx
self.ctx
.db
.get()?
.prepare(format!("SELECT {COLUMNS} FROM transfers WHERE transfers.id = ?1").as_ref())?
.query_and_then(params![id], |row| Transfer::try_from(row))?
.collect::<DbResult<Vec<_>>>()?)
.collect::<DbResult<Vec<_>>>()
}
}
1 change: 1 addition & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub type BaseResult<T> = Result<T, BaseError>;

#[allow(clippy::enum_variant_names)]
#[derive(thiserror::Error, Debug)]
pub enum BaseError {
#[error(transparent)]
Expand Down
6 changes: 4 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ async fn run(config: Config) -> BaseResult<()> {
pin_mut!(web, sync);

// Wait for either process to finish (i.e. return an error) and exit
Ok(select! {
select! {
result = web => result?,
result = sync => result?,
})
};

Ok(())
}
4 changes: 2 additions & 2 deletions src/sync/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ pub type SyncResult<T> = Result<T, SyncError>;
#[derive(thiserror::Error, Debug)]
pub enum SyncError {
#[error("invalid transfer: {0}")]
InvalidTransferError(String),
InvalidTransfer(String),

#[error("could not find user: {0}")]
UserNotFoundError(String),
UserNotFound(String),

#[error(transparent)]
DbError(#[from] crate::db::error::DbError),
Expand Down
15 changes: 8 additions & 7 deletions src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ pub async fn init(ctx: AppContext) -> SyncResult<()> {
.unwrap()
.with_nanosecond(0)
.unwrap()
.checked_add_signed(chrono::Duration::minutes(
ctx.config.sync.check_interval_mins.into(),
))
.checked_add_signed(
chrono::Duration::try_minutes(ctx.config.sync.check_interval_mins.into())
.expect("interval out of bounds"),
)
.unwrap();

let time_until_next_update = tokio::time::Duration::from_millis(
Expand Down Expand Up @@ -69,7 +70,7 @@ async fn execute(ctx: AppContext) -> SyncResult<()> {
for watcher in to_sync {
let user = user_repo
.find_user_by_uri(&watcher.user_uri)?
.ok_or_else(|| SyncError::UserNotFoundError(watcher.user_uri.clone()))?;
.ok_or_else(|| SyncError::UserNotFound(watcher.user_uri.clone()))?;

let (client, _) = client::Client::from_user_ensure_refreshed(ctx.clone(), user).await?;

Expand Down Expand Up @@ -102,9 +103,9 @@ pub async fn sync_watcher(
watcher: &Watcher,
now: DateTime<Utc>,
) -> SyncResult<u32> {
let res = sync_watcher_inner(ctx.clone(), client, &watcher_repo, watcher, &now).await;
let res = sync_watcher_inner(ctx.clone(), client, watcher_repo, watcher, &now).await;

let num_tracks = res.as_ref().unwrap_or(&0).clone();
let num_tracks = *res.as_ref().unwrap_or(&0);

// Only log if we've actually transferred tracks
if num_tracks == 0 {
Expand All @@ -131,7 +132,7 @@ async fn sync_watcher_inner(
now: &DateTime<Utc>,
) -> SyncResult<u32> {
let num_tracks_transferred =
transfer::PlaylistTransfer::new(ctx, client).try_transfer(&watcher).await?;
transfer::PlaylistTransfer::new(ctx, client).try_transfer(watcher).await?;

watcher_repo.update_watcher_last_sync_at(watcher.id, *now)?;

Expand Down
26 changes: 11 additions & 15 deletions src/sync/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl PlaylistTransfer {
}

if watcher.playlist_from == watcher.playlist_to {
return Err(SyncError::InvalidTransferError(
return Err(SyncError::InvalidTransfer(
"cannot transfer to the same playlist".to_owned(),
));
}
Expand Down Expand Up @@ -67,7 +67,7 @@ impl PlaylistTransfer {
async fn remove_tracks_from_playlist(
&self,
playlist_from: &PlaylistType,
ids_to_remove: &Vec<TrackId>,
ids_to_remove: &[TrackId],
) -> SyncResult<()> {
match playlist_from {
PlaylistType::Saved => {
Expand All @@ -93,23 +93,19 @@ impl PlaylistTransfer {
let playlist_track_ids = self.get_playlist_track_ids(to_id).await?;

// Get only the saved tracks that are not already in the target playlist and add them
let ids_to_insert = self.get_ids_to_insert(&ids_to_transfer, &playlist_track_ids);
let ids_to_insert = self.get_ids_to_insert(ids_to_transfer, &playlist_track_ids);
if !ids_to_insert.is_empty() {
self.client.playlist_add_ids(to_id, &ids_to_insert).await?;
}

return Ok(ids_to_insert
.len()
.try_into()
.expect("size cant possibly be bigger than u32"));
Ok(ids_to_insert.len().try_into().expect("size cant possibly be bigger than u32"))
}
PlaylistType::Saved => {
// We don't want to support transferring to saved tracks (for now; I just don't see the point)
return Err(SyncError::InvalidTransferError(
"cannot transfer to saved tracks".to_owned(),
));
}
};

// We don't want to support transferring to saved tracks (for now; I just don't see the point)
PlaylistType::Saved => Err(SyncError::InvalidTransfer(
"cannot transfer to saved tracks".to_owned(),
)),
}
}

/// Fetch the unique IDs in the specified playlist
Expand All @@ -125,7 +121,7 @@ impl PlaylistTransfer {

/// Find the IDs that are not in the target playlist, and return them reversed so they may be inserted in the correct order
fn get_ids_to_insert(&self, from: &HashSet<TrackId>, to: &HashSet<TrackId>) -> Vec<TrackId> {
let mut ids_to_insert = from.difference(&to).map(|track| track.clone()).collect::<Vec<_>>();
let mut ids_to_insert = from.difference(to).cloned().collect::<Vec<_>>();

// Since we read them in order from newest to oldest, we want to insert them oldest first so we retain this order
ids_to_insert.reverse();
Expand Down
16 changes: 6 additions & 10 deletions src/web/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,12 @@ impl IntoResponse for WebError {
fn maybe_get_response(error: &WebError) -> Option<(StatusCode, Value)> {
Some(match error {
WebError::NotFoundError => (StatusCode::NOT_FOUND, Value::String(error.to_string())),
WebError::DbError(outer) => match outer {
crate::db::error::DbError::SQLiteError(inner) => match inner {
rusqlite::Error::QueryReturnedNoRows => (
StatusCode::NOT_FOUND,
Value::String(WebError::NotFoundError.to_string()),
),
_ => return None,
},
_ => return None,
},
WebError::DbError(crate::db::error::DbError::SQLiteError(
rusqlite::Error::QueryReturnedNoRows,
)) => (
StatusCode::NOT_FOUND,
Value::String(WebError::NotFoundError.to_string()),
),
WebError::ClientError(outer) => match outer {
crate::api::error::ClientError::ApiError { status, message } => (
axum::http::StatusCode::from_u16(*status)
Expand Down
2 changes: 1 addition & 1 deletion src/web/router/watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async fn create_watcher(

if let PlaylistType::Id(id) = &from {
let user_id = UserId::parse_from_input(&session.user.user_uri)?;
match api::util::check_playlist_editable(&session.client, &id, &user_id).await {
match api::util::check_playlist_editable(&session.client, id, &user_id).await {
Ok(false) if data.should_remove => return Err(WebError::InvalidFormData(
"You do not have permission to edit the source playlist. You must disable track removal.".into(),
)),
Expand Down
2 changes: 1 addition & 1 deletion src/web/util/cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use tower_cookies::{
Cookie,
};

pub fn unset_cookie<'a>(key: &'a str) -> Cookie<'a> {
pub fn unset_cookie(key: &str) -> Cookie<'_> {
CookieBuilder::new(key, "")
.path("/")
.expires(OffsetDateTime::now_utc().checked_sub(1.days()))
Expand Down

0 comments on commit 2e168e5

Please sign in to comment.