Records cannot be added to an empty ActiveJSON datastore #203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kbrock
reviewed
Sep 16, 2020
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.