Skip to content

Fix mutable default settings bug#109

Merged
ledermann merged 5 commits intoledermann:masterfrom
farkmarnum:fix-mutable-default-settings-bug
Mar 3, 2022
Merged

Fix mutable default settings bug#109
ledermann merged 5 commits intoledermann:masterfrom
farkmarnum:fix-mutable-default-settings-bug

Conversation

@farkmarnum
Copy link
Contributor

@farkmarnum farkmarnum commented Feb 28, 2022

Overview

Fixes #108.

  • Modifies RailsSettings::SettingObject#_get_value so that it returns a "deep dup" of the default value, as opposed to a direct reference to the default value.
  • Adds tests for the cases where a default value is an Array or Hash

How to test

Pull in RailsSettings::SettingObject from master and run tests:

git co master lib/rails-settings/setting_object.rb
bundle exec rspec

The new tests fail (along with an additional test that is affected by this, since User.default_settings gets modified).

Alternatively, you can use the code in the issue referenced above to reproduce.

Notes

This approach to using Marshal to deep dup an object is documented in the book "The Ruby Programming Language" (page 83):

Another use for Marshal.dump and Marshal.load is to create deep copies of objects:

def deepcopy(o)
  Marshal.load(Marshal.dump(o))
end

Flanagan, David. The Ruby Programming Language. “O’Reilly Media, Inc.,” 2008.

@ledermann ledermann merged commit 3157c21 into ledermann:master Mar 3, 2022
@ledermann
Copy link
Owner

Great find and great documentation, thanks a lot!

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.

default_settings nested Arrays and Hashes are mutable

2 participants