yosiat / panko_serializer

High Performance JSON Serialization for ActiveRecord & Ruby Objects
https://panko.dev
MIT License
591 stars 36 forks source link

Attrribute methods not running #73

Open PhilCoggins opened 4 years ago

PhilCoggins commented 4 years ago

Posted this one to Slack, but haven't noticed activity there in a few months so reposting into an issue for brevity. Using latest tag 0.7.3, Ruby 2.6.6.

I've been beating my head against an issue all day long that I just can't figure out. I'm looking for some guidance or tips on how to debug this.

Panko is serializing json differently between different containers on both CircleCI and Heroku Production. I have three Serializers, one of them (ContactSerializer) is returning nil instead of the result of the method:

class CommentSerializer < Panko::Serializer
  has_one :user, serializer: UserSerializer, only: %i[id scoped_contact]
end

class UserSerializer < Panko::Serializer
  has_one :scoped_contact,
          serializer: ContactSerializer,
          name: :contact,
          only: %i[id photo_url full_name]
end

class ContactSerializer < Panko::Serializer
  attributes(*Contact.attribute_names.map(&:to_sym))
  attributes(:photo_url, :full_name)

  has_one :user, serializer: UserSerializer

  def photo_url
    ContactDecorator.new(object).photo_or_gravatar_url(88)
  end

  def full_name
    object.full_name
  end
end

In some containers, it is returning {:contact=>{:full_name=>nil, :id=>131, :photo_url=>nil} and in other containers, it returns valid values for full_name and photo_url. On the failing containers, I can comment out has_one :user, serializer: UserSerializer and it works fine, but why would this behavior be different on identical environments?

My best hunch right now is that there is a race condition when my gems are installed with bundle install --jobs=4, possible that Panko native extensions are conflicting with another library? Again, just need some direction on how to troubleshoot this. I have tried to reinstall gems without concurrent install with bundle install --force, but this does not resolve the issue on the failed containers. Any help would be much appreciated!

yosiat commented 4 years ago

Hi @PhilCoggins ,

I don't think this can issue can be because of how gems are installed on your system. Is there any difference between those containers - Are they are serving the same endpoint and sometimes returns different results?

Can you try and create minimal reproduction of your code using https://github.com/rails/rails/blob/master/guides/bug_report_templates/active_record_gem.rb - schema, serializers and create an object that has bad serialization and I'll try to reproduce it as well?

PhilCoggins commented 4 years ago

Hi @yosiat thank you for the speedy response.

The containers are identical AFAICT, and the test case is the exact same and data is returned as expected, but somehow the attribute methods are not being called (though the standard attribute methods are). I've been working on a reliable reproduction, I will update this issue as soon as I get one.

PhilCoggins commented 4 years ago

@yosiat You are correct, this was not an issue with gem installation. My UserSerializer and ContactSerializer were a circular dependency, where ContactSerializer would attempt to load UserSerializer when it encountered the has_one :user relationship, which in turn would try to load ContactSerializer when it encountered the has_one :scoped_contact relationship.

To be honest, I have no idea how my autoloader (Zeitwerk) resolves this, but the reason this was intermittent is because the load order of the files was inconsistent between containers. If the ContactSerializer was loaded before the UserSerializer, I would get the error condition with full_name and photo_url being nil (presumably due to some method_missing logic?). If the UserSerializer got loaded first, then it would succeed. My best guess is that ContactSerializer simply stopped loading any code after it encountered the has_one :user relationship, possibly to resolve the circular dependency, and thus full_name and photo_url would never get defined. Or maybe they were defined, but UserSerializer is holding a reference to a version of ContactSerializer that has changed after UserSerializer was defined.

My "fix" was to move the has_one :user declaration below the full_name and photo_url methods, no more intermittent errors 🎉 .

I think this could be made more reliable. When you define relationships on an ActiveRecord model, they allow you to pass an argument class_name (like passing serializer in Panko) that is the constant name as a string, that is then lazily loaded. This is very useful as it will likely only be called after all files have been loaded.

Would you consider supporting a similar API that uses serializer: "UserSerializer", or alternatively I've seen something like serializer: -> { UserSerializer } in other libraries. IMO the current behavior should be deprecated - it seems that circular dependencies with these serializers would be fairly common. For example I might have a PostSerializer and CommentSerializer that refer to each other when they are serialized (If I want post JSON, then I would to include comments, and if I want comment JSON, then I would it to include the post). The nature of Rails' autoloading makes these types of issues very difficult to debug, I'd hate for your other users to to have to go through this.

I wrote a test script which helped me identify this issue, but it's not a true repro as these single-file test scrips don't load classes and files in the same manner as would happen in a Rails environment. If you need I'd be happy to push a sample Rails app to demo this issue.

yosiat commented 3 years ago

@PhilCoggins sorry for replying one year later!

I implemented your suggestion here - https://github.com/panko-serializer/panko_serializer/pull/108, I hope it will solve this issue with circular dependency you have.

If you can run you tests with this branch and let me know if it solves the issue it will be amazing.