search

RubyMoney / eu_central_bank

A gem that calculates the exchange rate using published rates from European Central Bank. Compatible with the money gem
MIT License
217 stars 130 forks source link

`options` removed from Money 6.13.5 #101

Closed prognostikos closed 4 years ago

prognostikos commented 4 years ago

Seems that this gem isn't compatible with the latest money version (6.13.5)

NameError: undefined local variable or method `options' for #<Money::RatesStore::StoreWithHistoricalDataSupport:0x000055ad13572018>
Did you mean?  @options
/home/circleci/mrclean/vendor/bundle/ruby/2.6.0/gems/eu_central_bank-1.4.2/lib/money/rates_store/store_with_historical_data_support.rb:23:in `transaction'
/home/circleci/mrclean/vendor/bundle/ruby/2.6.0/gems/eu_central_bank-1.4.2/lib/eu_central_bank.rb:179:in `update_parsed_rates'
/home/circleci/mrclean/vendor/bundle/ruby/2.6.0/gems/eu_central_bank-1.4.2/lib/eu_central_bank.rb:29:in `update_rates'
/home/circleci/mrclean/app/better_now/accounting/ecb_bank.rb:25:in `update_rates'
/home/circleci/mrclean/app/better_now/accounting/ecb_bank.rb:20:in `initialize'
/home/circleci/mrclean/app/better_now/accounting/ecb_bank.rb:12:in `new'
/home/circleci/mrclean/app/better_now/accounting/ecb_bank.rb:12:in `setup'
/home/circleci/mrclean/config/initializers/money_exchange_bank.rb:4:in `<top (********)>'

This looks like the change that causes the problem: https://github.com/RubyMoney/money/pull/898/files#diff-70409fffc5690f30fb7495abab7a7dbfL103

ioquatix commented 4 years ago

https://github.com/RubyMoney/eu_central_bank/blob/b413b1c3dfbbc0146cbaa4de935eed8e8fc6fbd3/lib/money/rates_store/store_with_historical_data_support.rb#L13-L33 is horribly broken.

As soon as @in_transaction = true is set, any other thread can enter in the block without synchronisation.

If result = block.call raises an exception, @in_transaction = false is never called.

I'm not even sure when options[:without_mutex] is valid (semantically) or how the user should even know that during the construction of the store.

shailesh-kalamkar commented 4 years ago

@ioquatix It would be helpful to know if you found any alternative.

ioquatix commented 4 years ago

@shailesh-kalamkar An alternative to what?

shailesh-kalamkar commented 4 years ago

@ioquatix An alternative to this gem which is using the same API. Basically, would like to know how did you overcome the issue?

Thanks.

ioquatix commented 4 years ago

@shailesh-kalamkar https://github.com/ioquatix/latinum - it doesn't support the same interface (for good reasons), but it's pretty similar.

shailesh-kalamkar commented 4 years ago

Thank you so much.

antstorm commented 4 years ago

Sorry about this, 6.13.6 is now available that include a fix for this regression

jules-w2 commented 4 years ago

https://github.com/RubyMoney/money/issues/900#issuecomment-562070633

antstorm commented 4 years ago

Sorry again, just published eu_central_bank 1.5.0 that should finally fix this and rely on a thread-safe implementation of the superclass for synchronisation

Please ping me if you are still having issues with it

ioquatix commented 4 years ago

@antstorm great work!