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

add deep_freeze refinement to support #260

Merged
merged 3 commits into from Jun 5, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions pakyow-core/lib/pakyow/core/app.rb
Expand Up @@ -2,6 +2,7 @@
require "pakyow/support/definable"
require "pakyow/support/hookable"
require "pakyow/support/recursive_require"
require "pakyow/support/deep_freeze"

require "pakyow/core/helpers"
require "pakyow/core/router"
Expand Down Expand Up @@ -318,6 +319,10 @@ def reset
end
end

using Support::DeepFreeze

unfreezable :builder

# @api private
def initialize(environment, builder: nil, &block)
@environment = environment
Expand Down
24 changes: 0 additions & 24 deletions pakyow-core/lib/pakyow/core/router.rb
Expand Up @@ -748,30 +748,6 @@ def merge(router)
merge_templates(router.templates)
end

# @api private
def freeze
# TODO: let's instead have a deep freeze method that freezes the object and its ivars, recursively
hooks.each do |_, hooks_arr|
hooks_arr.each(&:freeze)
hooks_arr.freeze
end

children.each(&:freeze)

routes.each do |_, routes_arr|
routes_arr.each(&:freeze)
routes_arr.freeze
end

matcher.freeze
hooks.freeze
children.freeze
routes.freeze
templates.freeze

super
end

# @api private
def exception_for_class(klass, exceptions: {})
self.exceptions.merge(exceptions)[klass]
Expand Down
13 changes: 0 additions & 13 deletions pakyow-core/lib/pakyow/core/routing/route.rb
Expand Up @@ -59,19 +59,6 @@ def populated_path(path_to_self, **params)
}.join("/"))
end

def freeze
hooks.each do |_, hooks_arr|
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we also need to refine Hash to freeze values. Let me know if I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right you are. I just pushed an addition that refines Hash. I'm freezing keys and values. Do you think that is the right behavior? Or should it only be values?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, keys/values make sense.

hooks_arr.each(&:freeze)
hooks_arr.freeze
end

path.freeze
pipeline.freeze
hooks.freeze

super
end

protected

def create_matcher_from_path(path)
Expand Down
52 changes: 52 additions & 0 deletions pakyow-support/lib/pakyow/support/deep_freeze.rb
@@ -0,0 +1,52 @@
module Pakyow
module Support
module DeepFreeze
refine Object.singleton_class do
def unfreezable_variables
@unfreezable_variables ||= []
end

private

def unfreezable(*ivars)
ivars = ivars.map { |ivar| "@#{ivar}".to_sym }
unfreezable_variables.concat(ivars)
unfreezable_variables.uniq!
end
end

refine Object do
def deep_freeze
return self if frozen?

self.freeze

freezable_variables.each do |name|
instance_variable_get(name).deep_freeze
end

self
end

private

def freezable_variables
instance_variables - self.class.unfreezable_variables
end
end

refine Array do
def deep_freeze
return self if frozen?

self.freeze

each(&:deep_freeze)

self
end
end
end
end
end

17 changes: 3 additions & 14 deletions pakyow-support/lib/pakyow/support/definable.rb
@@ -1,4 +1,5 @@
require "pakyow/support/deep_dup"
require "pakyow/support/deep_freeze"

module Pakyow
module Support
Expand Down Expand Up @@ -28,6 +29,7 @@ module Support
#
module Definable
using DeepDup
using DeepFreeze

def self.included(base)
base.extend ClassAPI
Expand Down Expand Up @@ -58,7 +60,7 @@ def initialize(&block)
end

# instance state is now immutable
freeze
deep_freeze
end

# Returns register instances for state.
Expand All @@ -68,13 +70,6 @@ def state_for(type)
@state[type].instances
end

# @api private
def freeze
@state.each { |_, state| state.freeze }
@state.freeze
super
end

module ClassAPI
attr_reader :state, :inherited_state

Expand Down Expand Up @@ -201,12 +196,6 @@ def register(instance, priority: :default)
reprioritize!
end

def freeze
instances.each(&:freeze)
instances.freeze
super
end

def reset
object.reset
@instances = []
Expand Down
99 changes: 99 additions & 0 deletions pakyow-support/spec/deep_freeze_spec.rb
@@ -0,0 +1,99 @@
require "pakyow/support/deep_freeze"

using Pakyow::Support::DeepFreeze

class MyObject
unfreezable :fire

attr_reader :fire, :water

def initialize
@fire = 'fire'
@water = 'water'
end
end

RSpec.describe Pakyow::Support::DeepFreeze do
let :obj_with_unfreezable {
MyObject.new
}

describe "deep_freeze" do
it "returns the object" do
obj = Object.new

expect(obj.deep_freeze.object_id).to be(obj.object_id)
end

it "deep freezes an Object without ivars" do
obj = Object.new
obj.deep_freeze

expect(obj).to be_frozen
end

it "deep freezes an Object with ivars" do
ivar_value = Object.new
obj = Object.new
obj.instance_variable_set(:@ivar_name, ivar_value)

obj.deep_freeze

expect(obj).to be_frozen
expect(ivar_value).to be_frozen
end

it "deep freezes ivars" do
objects = Array.new(5) { Object.new }
objects.reduce do |parent, child|
parent.instance_variable_set(:@child, child)
end

objects.first.deep_freeze

objects.each do |obj|
expect(obj).to be_frozen
end
end

it "deep freezes ivars without loop" do
objects = Array.new(5, Object.new)
objects.reduce do |parent, child|
parent.instance_variable_set(:@child, child)
end

objects.first.deep_freeze

objects.each do |obj|
expect(obj).to be_frozen
end
end

it "deep freezes array items" do
objects = Array.new(5) { Object.new }

objects.deep_freeze

objects.each do |obj|
expect(obj).to be_frozen
end

expect(objects).to be_frozen
end

it "doesn't freeze unfreezeable" do
obj_with_unfreezable.deep_freeze

expect(obj_with_unfreezable).to be_frozen
expect(obj_with_unfreezable.water).to be_frozen
expect(obj_with_unfreezable.fire).not_to be_frozen
end

it "returns an array of unfreezeable ivars" do
vars = MyObject.unfreezable_variables

expect(vars).to be_instance_of(Array)
end
end
end