active-hash / active_hash

A readonly ActiveRecord-esque base class that lets you use a hash, a Yaml file or a custom file as the datasource
MIT License
1.2k stars 178 forks source link

Records cannot be added to an empty ActiveJSON datastore #203

Open davidstosik opened 4 years ago

davidstosik commented 4 years ago

Here's the simplest way to reproduce the error:

# test.rb

require "active_hash"

class Test < ActiveJSON::Base; end

Test.create()
$ echo '[]' > tests.json
$ ruby test.rb
Traceback (most recent call last):
        3: from test.rb:7:in `<main>'
        2: from /Users/david-stosik/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/active_hash-3.0.0/lib/active_hash/base.rb:174:in `create'
        1: from /Users/david-stosik/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/active_hash-3.0.0/lib/active_hash/base.rb:497:in `save'
/Users/david-stosik/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/active_hash-3.0.0/lib/active_hash/base.rb:135:in `insert': undefined method `length' for nil:NilClass (NoMethodError)

In some scenarios, @records can be nil when checking its length and appending a new record.

This PR adds a test case that reproduces the error above, and fixes it.

My first approach was to simply replace @records = nil with @records = [], but then I figured out I could do better and improve the code base a bit, avoiding in the process duplication of code such as @records || [].

All specs still pass.

Please let me know what you think.

davidstosik commented 4 years ago

Sorry for the delay, this is rebased now. 👌

davidstosik commented 2 years ago

This still seems to be a problem, so I rebased my branch.

kbrock commented 1 year ago

Darn. this is still outstanding. After we merge #268 I'd like to get this in. Please ping me if I haven't gotten this in by 5/15/2023