Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hashie::Extensions::Persistable #262

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/hashie.rb
Expand Up @@ -13,6 +13,7 @@ module Extensions
autoload :DeepMerge, 'hashie/extensions/deep_merge'
autoload :IgnoreUndeclared, 'hashie/extensions/ignore_undeclared'
autoload :IndifferentAccess, 'hashie/extensions/indifferent_access'
autoload :HashInitializer, 'hashie/extensions/hash_initializer'
autoload :MergeInitializer, 'hashie/extensions/merge_initializer'
autoload :MethodAccess, 'hashie/extensions/method_access'
autoload :MethodQuery, 'hashie/extensions/method_access'
Expand All @@ -25,6 +26,7 @@ module Extensions
autoload :PrettyInspect, 'hashie/extensions/pretty_inspect'
autoload :KeyConversion, 'hashie/extensions/key_conversion'
autoload :MethodAccessWithOverride, 'hashie/extensions/method_access'
autoload :Persistable, 'hashie/extensions/persistable'

module Parsers
autoload :YamlErbParser, 'hashie/extensions/parsers/yaml_erb_parser'
Expand Down
24 changes: 24 additions & 0 deletions lib/hashie/extensions/hash_initializer.rb
@@ -0,0 +1,24 @@
module Hashie
module Extensions
# Marker module to indicate has a single-argument hash constructor.
module HashInitializer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I created Hashie::Extensions::HashInitializer that's just a marker module, because I didn't have a way to tell what classes could be initialized from a Hash. That's the behavior for Mash and Dash, but that's not how the initializer works (by default) for ::Hash or Hashie::Hash. It is the behavior if you mixin Hashie::Extensions::MergeInitializer, though. I'm open to other ideas, though, perhaps a .from_hash method instead of a marker module?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is OK.

end

# Doesn't work...
# module MergeInitializer
# include HashInitializer
# end
end

class Hash
# Not unless MergeInititalizer is included
end

class Mash
include Extensions::HashInitializer
end

class Dash
include Extensions::HashInitializer
end
end
38 changes: 38 additions & 0 deletions lib/hashie/extensions/persistable.rb
@@ -0,0 +1,38 @@
module Hashie
module Extensions
module Persistable
CANNOT_INCLUDE = 'Peristable can only be used on classes with Hashie::Extensions::HashInitializer'
Copy link
Member

Choose a reason for hiding this comment

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

We don't do this around Hashie as far as I can see. Maybe just specialize an exception that inherits from StandardError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is it you don't do? Constants? ArgumentError?

Happy to make the change just wasn't really sure what you meant.

Copy link
Member

Choose a reason for hiding this comment

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

I think just this:

class MissingHashInitializer < StandardError; end

I would just not include the lengthy explanation, if we do it should be a localizable thing, etcetera.

Copy link
Member

Choose a reason for hiding this comment

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

If we want text in exceptions, I think I want something evolved like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

I think the message is important because a user probably won't know what a MissingHashInitializer means. I can see the value of a localizable system, but I'd rather have that be a separate issue/PR, since there's other errors with messages (e.g. here and here).

How about if I do this, and then you can create a separate issue for localization of error messages?

class MissingHashInitializer < StandardError
  def initialize(msg = 'Peristable can only be used on classes with Hashie::Extensions::HashInitializer', *args)
    super
  end
end

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. As long as we do it the same way everywhere. We can leave nice(r) errors for some future implementation.

CANNOT_SAVE = 'Cannot save unless persistable_file is set or the target file is passed as a parameter'

module ClassMethods
def load(file)
new(YAML.load(File.read(file))).tap do |h|
h.persistable_file = file
end
end
end

def self.included(base)
# Can only include into classes with a hash initializer
fail ArgumentError, CANNOT_INCLUDE unless
base.include?(Hashie::Extensions::HashInitializer) ||
base.include?(Hashie::Extensions::MergeInitializer)
base.extend ClassMethods
super
end

def save(file = nil)
self.persistable_file = file unless file.nil?
fail ArgumentError, CANNOT_SAVE if persistable_file.nil?
File.write(persistable_file, YAML.dump(self))
persistable_file
end

def persistable_file=(file)
@persistable_file = Pathname(file)
end

attr_reader :persistable_file
end
end
end
3 changes: 3 additions & 0 deletions spec/fixtures/persistable/hello_world.yaml
@@ -0,0 +1,3 @@
---
foo: bar
msg: Hello, world!
101 changes: 101 additions & 0 deletions spec/hashie/extensions/persistable_spec.rb
@@ -0,0 +1,101 @@
require 'tempfile'

module Hashie
module Extensions
RSpec.describe Persistable do
shared_examples 'loads YAML' do | test_class |
describe '.load' do
subject(:hash) { test_class.load('spec/fixtures/persistable/hello_world.yaml') }
it 'instantiates a instance from a YAML file' do
expect(hash).to be_a_kind_of ::Hash
expect(hash).to eq(
'foo' => 'bar',
'msg' => 'Hello, world!'
)
end

it 'remembers the filename' do
expect(hash.persistable_file).to eq(Pathname('spec/fixtures/persistable/hello_world.yaml'))
end
end
end

shared_examples 'saves YAML' do | test_class |
describe '#save' do
let(:obj) do
test_class.new(
'foo' => 'bar!',
'msg' => 'saved!'
)
end

it 'raises an error if persistable_file is not set' do
expect { obj.save }.to raise_error(ArgumentError, /cannot save unless persistable_file is set/i)
end

it 'saves to persistable_file' do
tmpfile = Tempfile.new(['hash', '.yaml'])
obj.persistable_file = tmpfile
expect(obj.save).to eq(Pathname(tmpfile))
expect(test_class.load(tmpfile)).to eq(obj)
end

it 'changes and saves to persistable_file (passed as an argument)' do
tmpfile = Tempfile.new(['hash', '.yaml'])
expect(obj.save(tmpfile)).to eq(Pathname(tmpfile))
expect(obj.persistable_file).to eq(Pathname(tmpfile))
expect(test_class.load(tmpfile)).to eq(obj)
end
end

describe '#[]=' do
context 'with autosave enabled'
pending 'autosaves'
context 'without autosave enabled' do
pending 'does not autosave'
end
end
end

context 'with Hash' do
class MyHash < ::Hash
include Hashie::Extensions::MergeInitializer
include Hashie::Extensions::Persistable
end

include_examples 'loads YAML', MyHash
include_examples 'saves YAML', MyHash
end

context 'with Hashie::Hash' do
class MyHashieHash < Hashie::Hash
include Hashie::Extensions::MergeInitializer
include Hashie::Extensions::Persistable
end

include_examples 'loads YAML', MyHashieHash
include_examples 'saves YAML', MyHashieHash
end

context 'with Hashie::Mash' do
class MyHashieMash < Hashie::Mash
include Hashie::Extensions::Persistable
end

include_examples 'loads YAML', MyHashieMash
include_examples 'saves YAML', MyHashieMash
end

context 'with Hashie::Dash' do
class MyHashieDash < Hashie::Dash
include Hashie::Extensions::Persistable
property 'foo'
property 'msg'
end

include_examples 'loads YAML', MyHashieDash
include_examples 'saves YAML', MyHashieDash
end
end
end
end