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

Fix Lyrics fetch for AZLyrics and Genius #12

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

TheConner
Copy link
Contributor

Fix errors from azLyrics and genius (re: #9)

To fix test suite errors related to genius, the configuration selectors had to be updated.

To fix azLyrics tests, new functionality for fetching tokens was added:

  • AZLyrics generates a token that is embedded on the page
  • Token does not come from a standard API endpoint, rather, it is inlined in a JS file that then embeds the token on input form elements as a hidden attribute (unfortunately can't extract it from the initial page returned from the server)
  • Despite being inlined, the token in the JS file does change depending on time / location.

To resolve this I added an optional "token" config section for AZ lyrics that fetches the token JS file, extracts the token, and then uses it on subsequent requests.

JSoup replaced org.jsoup.safety.Whitelist with org.jsoup.safety.Safelist. Purely a naming change, no difference of functionality.

This bumps the JSoup version (re: jagrosh#10) and handles the breaking changes. This is needed as the related MusicBot project has recently bumped the JSoup version to 1.15.3 (re: jagrosh/MusicBot@a7be2c4) causing runtime errors as it depends on JLyrics, which depends on a different JSoup version.
To fix test suite errors related to genius, the configuration selectors had to be updated.

To fix azLyrics tests:
- AZLyrics generates a token that is embedded on the page
- Token does not come from a standard API endpoint, rather, it is inlined in a JS file that then embeds the token on input form elements as a hidden attribute (unfortunately can't extract it from the initial page returned from the server)
To resolve, I added an optional "token" config section for AZ lyrics that fetches the JS file, extracts the token, and then uses it on subsequent requests.
@jagrosh
Copy link
Owner

jagrosh commented Sep 22, 2022

Have you tested these changes and do the tests pass?

@TheConner
Copy link
Contributor Author

Yes, the tests pass for me with these changes -- are there any issues with the tests passing on your end?
image

@jagrosh
Copy link
Owner

jagrosh commented Sep 22, 2022

I haven't had a chance to test this yet, but I'd imagine that (in the current state of things) if the tests pass then this does indeed work.

@jagrosh jagrosh merged commit 01cf4ee into jagrosh:master Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants