Skip to content

Commit

Permalink
Fix code review remarks
Browse files Browse the repository at this point in the history
  • Loading branch information
@chme
chme committed Jan 28, 2025
1 parent ffa9e40 commit 4680d3d
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 62 deletions.
2 changes: 1 addition & 1 deletion owntone.8
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Debug domains; available domains are: \fIconfig\fP, \fIdaap\fP,
\fIdb\fP, \fIhttpd\fP, \fImain\fP, \fImdns\fP, \fImisc\fP,
\fIrsp\fP, \fIscan\fP, \fIxcode\fP, \fIevent\fP, \fIhttp\fP, \fIremote\fP,
\fIdacp\fP, \fIffmpeg\fP, \fIartwork\fP, \fIplayer\fP, \fIraop\fP,
\fIlaudio\fP, \fIdmap\fP, \fIfdbperf\fP, \fIspotify\fP, \fIlastfm\fP,
\fIlaudio\fP, \fIdmap\fP, \fIfdbperf\fP, \fIspotify\fP, \fIscrobble\fP,
\fIcache\fP, \fImpd\fP, \fIstream\fP, \fIcast\fP, \fIfifo\fP, \fIlib\fP,
\fIweb\fP, \fIairplay\fP.
.TP
Expand Down
34 changes: 17 additions & 17 deletions src/lastfm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ param_sign(struct keyval *kv)
if (gc_err != GPG_ERR_NO_ERROR)
{
gpg_strerror_r(gc_err, ebuf, sizeof(ebuf));
DPRINTF(E_LOG, L_LASTFM, "Could not open MD5: %s\n", ebuf);
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Could not open MD5: %s\n", ebuf);
return -1;
}

Expand All @@ -99,7 +99,7 @@ param_sign(struct keyval *kv)
hash_bytes = gcry_md_read(md_hdl, GCRY_MD_MD5);
if (!hash_bytes)
{
DPRINTF(E_LOG, L_LASTFM, "Could not read MD5 hash\n");
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Could not read MD5 hash\n");
return -1;
}

Expand Down Expand Up @@ -163,22 +163,22 @@ response_process(struct http_client_ctx *ctx, char **errmsg)
body = (char *)evbuffer_pullup(ctx->input_body, -1);
if (!body || (strlen(body) == 0))
{
DPRINTF(E_LOG, L_LASTFM, "Empty response\n");
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Empty response\n");
return -1;
}

tree = xml_from_string(body);
if (!tree)
{
DPRINTF(E_LOG, L_LASTFM, "Failed to parse LastFM response:\n%s\n", body);
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Failed to parse LastFM response:\n%s\n", body);
return -1;
}

error = xml_get_val(tree, "lfm/error");
if (error)
{
DPRINTF(E_LOG, L_LASTFM, "Request to LastFM failed: %s\n", error);
DPRINTF(E_DBG, L_LASTFM, "LastFM response:\n%s\n", body);
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Request to LastFM failed: %s\n", error);
DPRINTF(E_DBG, L_SCROBBLE, "lastfm: LastFM response:\n%s\n", body);

if (errmsg)
*errmsg = atrim(error);
Expand All @@ -187,12 +187,12 @@ response_process(struct http_client_ctx *ctx, char **errmsg)
return -1;
}

DPRINTF(E_SPAM, L_LASTFM, "LastFM response:\n%s\n", body);
DPRINTF(E_SPAM, L_SCROBBLE, "lastfm: LastFM response:\n%s\n", body);

// Was it a scrobble request? Then do nothing. TODO: Check for error messages
if (xml_get_node(tree, "lfm/scrobbles/scrobble"))
{
DPRINTF(E_DBG, L_LASTFM, "Scrobble callback\n");
DPRINTF(E_DBG, L_SCROBBLE, "lastfm: Scrobble callback\n");
xml_free(tree);
return 0;
}
Expand All @@ -201,12 +201,12 @@ response_process(struct http_client_ctx *ctx, char **errmsg)
sk = atrim(xml_get_val(tree, "lfm/session/key"));
if (!sk)
{
DPRINTF(E_LOG, L_LASTFM, "Session key not found\n");
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Session key not found\n");
xml_free(tree);
return -1;
}

DPRINTF(E_INFO, L_LASTFM, "Got session key from LastFM: %s\n", sk);
DPRINTF(E_INFO, L_SCROBBLE, "lastfm: Got session key from LastFM: %s\n", sk);
db_admin_set(DB_ADMIN_LASTFM_SESSION_KEY, sk);

free(lastfm_session_key);
Expand Down Expand Up @@ -240,7 +240,7 @@ request_post(const char *url, struct keyval *kv, char **errmsg)
ret = param_sign(kv);
if (ret < 0)
{
DPRINTF(E_LOG, L_LASTFM, "Aborting request, param_sign failed\n");
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Aborting request, param_sign failed\n");
return -1;
}

Expand All @@ -249,7 +249,7 @@ request_post(const char *url, struct keyval *kv, char **errmsg)
request_body = http_form_urlencode(kv);
if (!request_body)
{
DPRINTF(E_LOG, L_LASTFM, "Aborting request, http_form_urlencode failed\n");
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Aborting request, http_form_urlencode failed\n");
return -1;
}

Expand Down Expand Up @@ -283,7 +283,7 @@ scrobble(int id)
mfi = db_file_fetch_byid(id);
if (!mfi)
{
DPRINTF(E_LOG, L_LASTFM, "Scrobble failed, track id %d is unknown\n", id);
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: Scrobble failed, track id %d is unknown\n", id);
return -1;
}

Expand Down Expand Up @@ -329,7 +329,7 @@ scrobble(int id)
return -1;
}

DPRINTF(E_INFO, L_LASTFM, "Scrobbling '%s' by '%s'\n", keyval_get(kv, "track"), keyval_get(kv, "artist"));
DPRINTF(E_INFO, L_SCROBBLE, "lastfm: Scrobbling '%s' by '%s'\n", keyval_get(kv, "track"), keyval_get(kv, "artist"));

ret = request_post(api_url, kv, NULL);

Expand Down Expand Up @@ -369,7 +369,7 @@ lastfm_login_user(const char *user, const char *password, char **errmsg)
struct keyval *kv;
int ret;

DPRINTF(E_LOG, L_LASTFM, "LastFM credentials file OK, logging in with username %s\n", user);
DPRINTF(E_LOG, L_SCROBBLE, "lastfm: LastFM credentials file OK, logging in with username %s\n", user);

// Stop active scrobbling session
stop_scrobbling();
Expand Down Expand Up @@ -424,7 +424,7 @@ lastfm_scrobble(int id)
if (lastfm_disabled)
return -1;

DPRINTF(E_DBG, L_LASTFM, "Got LastFM scrobble request\n");
DPRINTF(E_DBG, L_SCROBBLE, "lastfm: Got LastFM scrobble request\n");

return scrobble(id);
}
Expand All @@ -445,7 +445,7 @@ lastfm_init(void)
ret = db_admin_get(&lastfm_session_key, DB_ADMIN_LASTFM_SESSION_KEY);
if (ret < 0)
{
DPRINTF(E_DBG, L_LASTFM, "No valid LastFM session key\n");
DPRINTF(E_DBG, L_SCROBBLE, "lastfm: No valid LastFM session key\n");
lastfm_disabled = true;
}

Expand Down
72 changes: 32 additions & 40 deletions src/listenbrainz.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,18 @@
#include "logger.h"
#include "misc_json.h"

static const char *submit_listens_url = "https://api.listenbrainz.org/1/submit-listens";
static const char *validate_token_url = "https://api.listenbrainz.org/1/validate-token";
static const char *listenbrainz_submit_listens_url = "https://api.listenbrainz.org/1/submit-listens";
static const char *listenbrainz_validate_token_url = "https://api.listenbrainz.org/1/validate-token";
static bool listenbrainz_disabled = true;
static char *listenbrainz_token = NULL;
static time_t rate_limited_until = 0;
static time_t listenbrainz_rate_limited_until = 0;

static int
submit_listens(struct media_file_info *mfi)
{
struct http_client_ctx ctx;
struct http_client_ctx ctx = { 0 };
struct keyval kv_out = { 0 };
struct keyval kv_in = { 0 };
char auth_token[1024];
json_object *request_body;
json_object *listens;
Expand All @@ -51,11 +53,10 @@ submit_listens(struct media_file_info *mfi)
int32_t rate_limit_seconds = -1;
int ret;

memset(&ctx, 0, sizeof(struct http_client_ctx));
ctx.url = submit_listens_url;
ctx.url = listenbrainz_submit_listens_url;

// Set request headers
ctx.output_headers = calloc(1, sizeof(struct keyval));
ctx.output_headers = &kv_out;
snprintf(auth_token, sizeof(auth_token), "Token %s", listenbrainz_token);
keyval_add(ctx.output_headers, "Authorization", auth_token);
keyval_add(ctx.output_headers, "Content-Type", "application/json");
Expand Down Expand Up @@ -83,26 +84,26 @@ submit_listens(struct media_file_info *mfi)
ctx.output_body = json_object_to_json_string(request_body);

// Create input evbuffer for the response body and keyval for response headers
ctx.input_headers = calloc(1, sizeof(struct keyval));
ctx.input_headers = &kv_in;

// Send POST request for submit-listens endpoint
ret = http_client_request(&ctx, NULL);

// Process response
if (ret < 0)
{
DPRINTF(E_LOG, L_LBRAINZ, "Failed to scrobble '%s' by '%s'\n", mfi->title, mfi->artist);
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: Failed to scrobble '%s' by '%s'\n", mfi->title, mfi->artist);
goto out;
}

if (ctx.response_code == HTTP_OK)
{
DPRINTF(E_INFO, L_LBRAINZ, "Scrobbled '%s' by '%s'\n", mfi->title, mfi->artist);
rate_limited_until = 0;
DPRINTF(E_INFO, L_SCROBBLE, "lbrainz: Scrobbled '%s' by '%s'\n", mfi->title, mfi->artist);
listenbrainz_rate_limited_until = 0;
}
else if (ctx.response_code == 401)
{
DPRINTF(E_LOG, L_LBRAINZ, "Failed to scrobble '%s' by '%s', unauthorized, disable scrobbling\n", mfi->title,
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: Failed to scrobble '%s' by '%s', unauthorized, disable scrobbling\n", mfi->title,
mfi->artist);
listenbrainz_disabled = true;
}
Expand All @@ -112,14 +113,14 @@ submit_listens(struct media_file_info *mfi)
ret = safe_atoi32(x_rate_limit_reset_in, &rate_limit_seconds);
if (ret == 0 && rate_limit_seconds > 0)
{
rate_limited_until = time(NULL) + rate_limit_seconds;
listenbrainz_rate_limited_until = time(NULL) + rate_limit_seconds;
}
DPRINTF(E_INFO, L_LBRAINZ, "Failed to scrobble '%s' by '%s', rate limited for %d seconds\n", mfi->title,
DPRINTF(E_INFO, L_SCROBBLE, "lbrainz: Failed to scrobble '%s' by '%s', rate limited for %d seconds\n", mfi->title,
mfi->artist, rate_limit_seconds);
}
else
{
DPRINTF(E_LOG, L_LBRAINZ, "Failed to scrobble '%s' by '%s', response code: %d\n", mfi->title, mfi->artist,
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: Failed to scrobble '%s' by '%s', response code: %d\n", mfi->title, mfi->artist,
ctx.response_code);
}

Expand All @@ -128,23 +129,18 @@ submit_listens(struct media_file_info *mfi)
// Clean up
jparse_free(request_body);
if (ctx.output_headers)
{
keyval_clear(ctx.output_headers);
free(ctx.output_headers);
}
keyval_clear(ctx.output_headers);
if (ctx.input_headers)
{
keyval_clear(ctx.input_headers);
free(ctx.input_headers);
}
keyval_clear(ctx.input_headers);

return ret;
}

static int
validate_token(struct listenbrainz_status *status)
{
struct http_client_ctx ctx;
struct http_client_ctx ctx = { 0 };
struct keyval kv_out = { 0 };
char auth_token[1024];
char *response_body;
json_object *json_response = NULL;
Expand All @@ -153,11 +149,10 @@ validate_token(struct listenbrainz_status *status)
if (!listenbrainz_token)
return -1;

memset(&ctx, 0, sizeof(struct http_client_ctx));
ctx.url = validate_token_url;
ctx.url = listenbrainz_validate_token_url;

// Set request headers
ctx.output_headers = calloc(1, sizeof(struct keyval));
ctx.output_headers = &kv_out;
snprintf(auth_token, sizeof(auth_token), "Token %s", listenbrainz_token);
keyval_add(ctx.output_headers, "Authorization", auth_token);

Expand All @@ -174,13 +169,13 @@ validate_token(struct listenbrainz_status *status)
response_body = (char *)evbuffer_pullup(ctx.input_body, -1);
if (!response_body || (strlen(response_body) == 0))
{
DPRINTF(E_LOG, L_LBRAINZ, "Request for '%s' failed, response was empty\n", ctx.url);
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: Request for '%s' failed, response was empty\n", ctx.url);
goto out;
}

json_response = json_tokener_parse(response_body);
if (!json_response)
DPRINTF(E_LOG, L_LBRAINZ, "JSON parser returned an error for '%s'\n", ctx.url);
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: JSON parser returned an error for '%s'\n", ctx.url);

status->user_name = safe_strdup(jparse_str_from_obj(json_response, "user_name"));
status->token_valid = jparse_bool_from_obj(json_response, "valid");
Expand All @@ -193,10 +188,7 @@ validate_token(struct listenbrainz_status *status)
if (ctx.input_body)
evbuffer_free(ctx.input_body);
if (ctx.output_headers)
{
keyval_clear(ctx.output_headers);
free(ctx.output_headers);
}
keyval_clear(ctx.output_headers);

return ret;
}
Expand All @@ -211,16 +203,16 @@ listenbrainz_scrobble(int mfi_id)
if (listenbrainz_disabled)
return -1;

if (rate_limited_until > 0 && time(NULL) < rate_limited_until)
if (listenbrainz_rate_limited_until > 0 && time(NULL) < listenbrainz_rate_limited_until)
{
DPRINTF(E_INFO, L_LBRAINZ, "Rate limited, not scrobbling\n");
DPRINTF(E_INFO, L_SCROBBLE, "lbrainz: Rate limited, not scrobbling\n");
return -2;
}

mfi = db_file_fetch_byid(mfi_id);
if (!mfi)
{
DPRINTF(E_LOG, L_LBRAINZ, "Scrobble failed, track id %d is unknown\n", mfi_id);
DPRINTF(E_LOG, L_SCROBBLE, "lbrainz: Scrobble failed, track id %d is unknown\n", mfi_id);
return -1;
}

Expand Down Expand Up @@ -252,14 +244,14 @@ listenbrainz_token_set(const char *token)

if (!token)
{
DPRINTF(E_DBG, L_LBRAINZ, "Failed to update ListenBrainz token, no token provided\n");
DPRINTF(E_DBG, L_SCROBBLE, "lbrainz: Failed to update ListenBrainz token, no token provided\n");
return -1;
}

ret = db_admin_set(DB_ADMIN_LISTENBRAINZ_TOKEN, token);
if (ret < 0)
{
DPRINTF(E_DBG, L_LBRAINZ, "Failed to update ListenBrainz token, DB update failed\n");
DPRINTF(E_DBG, L_SCROBBLE, "lbrainz: Failed to update ListenBrainz token, DB update failed\n");
}
else
{
Expand All @@ -281,7 +273,7 @@ listenbrainz_token_delete(void)
ret = db_admin_delete(DB_ADMIN_LISTENBRAINZ_TOKEN);
if (ret < 0)
{
DPRINTF(E_DBG, L_LBRAINZ, "Failed to delete ListenBrainz token, DB delete query failed\n");
DPRINTF(E_DBG, L_SCROBBLE, "lbrainz: Failed to delete ListenBrainz token, DB delete query failed\n");
}
else
{
Expand Down Expand Up @@ -331,7 +323,7 @@ listenbrainz_init(void)

if (listenbrainz_disabled)
{
DPRINTF(E_DBG, L_LBRAINZ, "No valid ListenBrainz token\n");
DPRINTF(E_DBG, L_SCROBBLE, "lbrainz: No valid ListenBrainz token\n");
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/logger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static uint32_t logger_repeat_counter;
static uint32_t logger_last_hash;
static char *logfilename;
static FILE *logfile;
static char *labels[] = { "config", "daap", "db", "httpd", "http", "main", "mdns", "misc", "rsp", "scan", "xcode", "event", "remote", "dacp", "ffmpeg", "artwork", "player", "raop", "laudio", "dmap", "dbperf", "spotify", "lastfm", "cache", "mpd", "stream", "cast", "fifo", "lib", "web", "airplay", "rcp", "lbrainz" };
static char *labels[] = { "config", "daap", "db", "httpd", "http", "main", "mdns", "misc", "rsp", "scan", "xcode", "event", "remote", "dacp", "ffmpeg", "artwork", "player", "raop", "laudio", "dmap", "dbperf", "spotify", "scrobble", "cache", "mpd", "stream", "cast", "fifo", "lib", "web", "airplay", "rcp" };
static char *severities[] = { "FATAL", "LOG", "WARN", "INFO", "DEBUG", "SPAM" };


Expand Down
5 changes: 2 additions & 3 deletions src/logger.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#define L_DMAP 19
#define L_DBPERF 20
#define L_SPOTIFY 21
#define L_LASTFM 22
#define L_SCROBBLE 22
#define L_CACHE 23
#define L_MPD 24
#define L_STREAMING 25
Expand All @@ -38,9 +38,8 @@
#define L_WEB 29
#define L_AIRPLAY 30
#define L_RCP 31
#define L_LBRAINZ 32

#define N_LOGDOMAINS 33
#define N_LOGDOMAINS 32

/* Severities */
#define E_FATAL 0
Expand Down

0 comments on commit 4680d3d

Please sign in to comment.