Skip to content

Make RateStore marshalable (add #marshal_dump) - #31

Merged
sunny merged 1 commit into
RubyMoney:mainfrom
jtarchie:marshalable-rate-store
Jul 27, 2026
Merged

Make RateStore marshalable (add #marshal_dump)#31
sunny merged 1 commit into
RubyMoney:mainfrom
jtarchie:marshalable-rate-store

Conversation

@jtarchie

Copy link
Copy Markdown
Contributor

Problem

Marshaling a Money::Bank::VariableExchange whose store is an
ActiveCurrency::RateStore fails, because RateStore has no #marshal_dump.

VariableExchange#marshal_load rebuilds the store with:

@store = store_info.shift.new(*store_info)

so it expects the store's #marshal_dump to return
[StoreClass, *constructor_args]. Without a custom #marshal_dump,
reloading a marshaled bank raises.

This comes up when the configured default_bank uses a RateStore and
something marshals it, e.g. a Rails app with a Marshal-based cache or
message serializer that caches the default bank.

Fix

Add #marshal_dump to RateStore returning [self.class]. RateStore
takes no constructor arguments, so that is all VariableExchange#marshal_load
needs to rebuild it on load.

Test

Added specs asserting RateStore#marshal_dump returns [described_class]
and that a VariableExchange backed by a RateStore round-trips through
Marshal.

@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! Could you pull the changes from main (updated GitHub test workflow) and add an entry to the CHANGELOG as well, please? 🙏🏻

@jtarchie
jtarchie force-pushed the marshalable-rate-store branch from 4c78826 to ded6a67 Compare July 22, 2026 16:21
@jtarchie

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review! Rebased onto the latest main (picking up the updated CI workflow) and added a CHANGELOG entry under Unreleased. Should be good to go 🙏

@sunny

sunny commented Jul 27, 2026

Copy link
Copy Markdown
Member

Would you mind bringing in the changes from main again? I don't know why the test workflow is not running 🤔

Money::Bank::VariableExchange#marshal_load rebuilds the store with
store_info.shift.new(*store_info), which requires the store's
#marshal_dump to return [StoreClass, *constructor_args]. Without it,
marshaling a bank whose store is a RateStore fails, which happens when
the configured default bank is cached under a Marshal-based serializer.

RateStore takes no constructor arguments, so returning [self.class] is
enough to rebuild it on load.
@jtarchie
jtarchie force-pushed the marshalable-rate-store branch from ded6a67 to da8cd28 Compare July 27, 2026 18:35
@jtarchie

Copy link
Copy Markdown
Contributor Author

Done — rebased onto main so it now includes #33.

Looks like the run is triggering now but sitting in action_required: since this is my first PR to this repo, GitHub is holding the workflow for a maintainer to approve. There should be an "Approve and run workflows" button on the checks — mind giving it a click? 🙏

@sunny

sunny commented Jul 27, 2026

Copy link
Copy Markdown
Member

#33 fixed it, thank you @yukideluxe!

@sunny
sunny merged commit 7b3a66b into RubyMoney:main Jul 27, 2026
4 checks passed
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.

2 participants