Handle siblings with no content type #1
Conversation
When a deletion conflict occurs, attempting to read the `data` property of an `RObject` results in an exception being generated. The exception is raised because `riak-client` is unable to find a serializer for an object that is marked for deletion (read: no content_type). Since we cannot directly access `data`, inspect all of the siblings and reject the items having a `nil` `content_type`. This allows for conflict resolution to continue for the remaining siblings.
There was a problem hiding this comment.
If this is a port of the existing PR out for ripple, why does it look so different?
Also, just by way of background, is there a better way to get our Ripple client up to date than to further diverge from the original? Seems like there's going to have to be a day of reckoning at some point in the future?
| @@ -8,10 +8,10 @@ module Conflict | |||
| end | |||
|
|
|||
| describe 'calling the lambda returned by .to_proc' do | |||
| let(:sibling_1) { stub(:data => { '_type' => 'User' } ) } | |||
| let(:sibling_2) { stub(:data => { '_type' => 'User' } ) } | |||
| let(:sibling_1) { stub(:data => { '_type' => 'User' }, :content_type => 'application/json') } | |||
There was a problem hiding this comment.
Can we add this new content-type based test as a separate new test vs. overwriting a previous test (which implies it's not backwards compatible)?
There was a problem hiding this comment.
Hmm.. The content type was added to get the two existing resolver specs to pass since we expect objects to respond to content_type. All RObject instances respond to content_type so I believe this should be a safe change.
There was a problem hiding this comment.
@dtykocki I guess if we're ensuring backwards compatibility, you'd like to see that all existing tests can pass. Modifying the existing tests for such a fundamental library like Ripple makes me a little nervous.
There was a problem hiding this comment.
Agreed - making this change makes me just as nervous 😨.
I can add a respond_to? check to the rejection block before checking the content type. Since the stubs don't respond to content_type, we'll never attempt the nil check. The existing specs should still pass then.
I have a few issues with the existing PR and it won't solve the issue were running into:
|
I'd like to think there's a better way. I assumed since we have a fork that we gave up trying merge upstream. Also, the last release was back in 2012 so there's a chance it's not really maintained anymore? |
|
From what I gather, we depend heavily on ripple. Is it worth considering taking over maintenance of project? Totally not trying to add more to the current workload, just a thought 😄 . |
dtykocki
force-pushed
the
handle-siblings-with-no-content-type
branch
from
2f0006f to
d11eebb
Compare
@spreedly/product-dev