Records cannot be added to an empty ActiveJSON datastore#203
Records cannot be added to an empty ActiveJSON datastore#203davidstosik wants to merge 4 commits intoactive-hash:masterfrom
Conversation
kbrock
left a comment
kbrock
left a comment
There was a problem hiding this comment.
this looks nice.
consolidates the delete_all details.
not sure that records buys us that much but good enough
could you rebase this to fixup your spec and then we can get this merged
davidstosik
force-pushed
the
fix-empty-json-create
branch
from
d92f283 to
2b5072c
Compare
|
Sorry for the delay, this is rebased now. 👌 |
The expectation is that the code won't trigger an exception, but it
does:
ActiveJSON::Base.create does not fail when the loaded JSON was empty
Failure/Error: Empty.create()
NoMethodError:
undefined method `length' for nil:NilClass
# ./lib/active_hash/base.rb:135:in `insert'
# ./lib/active_hash/base.rb:497:in `save'
# ./lib/active_hash/base.rb:174:in `create'
# ./spec/active_json/base_spec.rb:51:in `block (3 levels) in <top (required)>'
…N datastore Simple fix would be to replace `@records = nil` with `@records = []`, but the suggested approach as a better impact on the code base, avoiding repetitions such as `@records || []`.
davidstosik
force-pushed
the
fix-empty-json-create
branch
from
2b5072c to
4ccff1a
Compare
|
This still seems to be a problem, so I rebased my branch. |
|
Darn. this is still outstanding. After we merge #268 I'd like to get this in. |
|
I thought we had a whole discussion here. I view active_hash as a static data store. And if you want to treat it as a writable database, there are other database projects that may be better suited (like sqlite). sqlite does support |
3 participants
Here's the simplest way to reproduce the error:
In some scenarios,
@recordscan benilwhen checking itslengthand 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 = nilwith@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.