JsonApiClient / json_api_client

Build client libraries compliant with specification defined by jsonapi.org
MIT License
362 stars 186 forks source link

JsonApiClient::Resource#initialize raises error due to forget_change! #375

Closed jethroo closed 3 years ago

jethroo commented 3 years ago

We have a custom resource inheriting from JsonApiClient::Resource and we are switching from v1.3.0 to v1.18.0 now and are noticing following issue:

[13] pry(main)> Reports::ApiReport.ancestors.slice(0..1)
=> [Reports::ApiReport, JsonApiClient::Resource]
[14] pry(main)> Reports::ApiReport.new({})
NoMethodError: undefined method `delete' for nil:NilClass
from /Users/carstenwirth/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/json_api_client-1.18.0/lib/json_api_client/helpers/dirty.rb:22:in `forget_change!'

this is due to forget_change! is calling delete on an uninitialized instance variable @changed_attributes upon initialization:

https://github.com/JsonApiClient/json_api_client/blob/master/lib/json_api_client/resource.rb#L357 https://github.com/JsonApiClient/json_api_client/blob/master/lib/json_api_client/helpers/dirty.rb#L22

to prevent this it should be called on the instance method changed_attributes which will initialize it in case it hasn't:

      def forget_change!(attr)
        changed_attributes.delete(attr.to_s)
      end

our current workaround is to initialize it ourselfs:

class ApiReport < JsonApiClient::Resource
    def initialize(params)
      @changed_attributes = {}
      super(params)
    end

It would be nice to have it fixed in the gem itself though. Or did we get something wrong here? WDYT?

jethroo commented 3 years ago

ok sorry the issue was on our end overloading def property_for(name)

the suggested fix is still valid though since delete might be called on an unitialized @changed_attributes