guilhermesad / rspotify

A ruby wrapper for the Spotify Web API
MIT License
713 stars 286 forks source link

Class variables get cleared by rails #102

Open ArcticXWolf opened 7 years ago

ArcticXWolf commented 7 years ago

First, this is a great gem :) However, I found a little bug.

You use the User.oauth_post method in the Playlist.add_tracks! This method calls oauth_header and oauth_send. Oauth_send refreshes a users auth token, if it is expired or not available, but oauth_header does not.

RSpotify::VERBS.each do |verb|
  define_singleton_method "oauth_#{verb}" do |user_id, path, *params|
    params << oauth_header(user_id)
    oauth_send(user_id, verb, path, *params)
  end
end

def self.oauth_header(user_id)
  {
    'Authorization' => "Bearer #{@@users_credentials[user_id]['token']}",
    'Content-Type'  => 'application/json'
  }
end
private_class_method :oauth_header

My problem especially is, that Rails sometimes clears ALL class variables (in this case @@user_credentials). When only calling oauth_send, the class variable gets set again, but in this case, oauth_header gets called first, causing it to fail with a

NameError (uninitialized class variable @@users_credentials in RSpotify::User):

I temporarily fixed it through manually recreating the specified user (RSpotify::User.new(auth_hash)) before each call to add_tracks!, but you should either call User.refresh_token before oauth_header or use something else than class variables. They are just a cache in this case after all and not meant to be reliable.

Thanks!

EDIT: I just tried to correct this myself and issue a pull request, but this problem would not be solved by a call to refresh_token, because refresh_token needs the @@user_credentials as well. I guess in the case of cleared class variables, the library needs the persisted data from the users anyway (like I did in my workaround). So if there is no other solution to this problem, I would at least mention it in the README.

dgilperez commented 7 years ago

Same here with a fresh Rails 5 app. Any ideas or workaround?

ArcticXWolf commented 7 years ago

Like I stated in my original post, you can just call RSpotify::User.new() with the old auth_hash, it refreshs the token in that case.