Skip to content

Clear the cached currency keys in Currency.reset! - #1228

Merged
yukideluxe merged 2 commits into
RubyMoney:mainfrom
lenamonj:currency-reset-clears-keys
Sep 16, 2026
Merged

yukideluxe merged 2 commits into
RubyMoney:mainfrom
lenamonj:currency-reset-clears-keys

Conversation

@lenamonj

@lenamonj lenamonj commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Currency.reset! reloads the table and clears the instance and iso_numeric caches, but not the memoized key set that Currency.new gates on. After register, a lookup, then reset!, Currency.new still admitted the removed id and raised NoMethodError from initialize_data! instead of UnknownCurrency, which find does not rescue.

reset! now clears the key set as well. The new spec fails on main and passes with the change; rspec and rubocop are clean. CHANGELOG and AUTHORS updated.

sunny
sunny previously approved these changes Sep 4, 2026

@sunny sunny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! \o/

@lenamonj

lenamonj commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

The red jruby-10.0 job is the pre-existing Money::Currency#initialize is thread safe spec (currency_spec.rb:311), which pushes to a plain Array from two threads and does not call reset!. The same job is green on #1227 and on main. Could you re-run it? Happy to push an empty commit instead if that is easier.

@yukideluxe yukideluxe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!! I fixed the merge conflict in the CHANGELOG 😊

@yukideluxe
yukideluxe merged commit 227f736 into RubyMoney:main Sep 16, 2026
9 checks passed
@lenamonj
lenamonj deleted the currency-reset-clears-keys branch September 16, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants