Skip to content

Commit

Permalink
Simpify design.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Mar 9, 2017
1 parent 134d353 commit 3396ddf
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 121 deletions.
2 changes: 2 additions & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
--format documentation
--color
--require spec_helper
--warnings
39 changes: 6 additions & 33 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,52 +20,25 @@ Add this line to your application's Gemfile:
gem 'rack-freeze'
```

And then execute:

$ bundle

Or install it yourself as:

$ gem install rack-freeze

## Usage

For existing rack middleware, simply wrap it:
Add this to your config.ru:

```ruby
use Rack::Freeze[Rack::Logger]
require 'rack/freeze'
```

This will make a subclass of `Rack::Logger` if required with a working implementation of `#freeze`.

In your `config.ru`, you prepare your app using the `#warmup method`;

```ruby
warmup do |app|
# Recursively freeze all the middleware so that mutation bugs are detected.
app.freeze
end
```
Now all your middleware will be frozen by default.

### What bugs does this fix?

So, instead of writing
It guarantees, within the limits of the freeze API, that middleware won't mutate during a request.

```ruby
use External::Middleware
use Middleware
```

you write

```ruby
use Rack::Freeze[External::Middleware]
```

That ensures that `External::Middleware` will correctly freeze itself and all subsequent apps. Additionally, if `External::Middleware` mutates it's state, it will throw an exception. In a multi-threaded web-server, unprotected mutation of internal state will lead to undefined behavior.

### Thar be the Monkeys

Some Rack middleware is not easy to patch in a generic way, e.g. `Rack::URLMap`. As these are identified, they will be monkey patched by this gem automatically. Going forward, I hope to bring attention to this issue and ideally integrate these changes directly into Rack.
If `Middleware` mutates it's state, it will throw an exception. In a multi-threaded web-server, unprotected mutation of internal state will lead to undefined behavior.

## Contributing

Expand Down
33 changes: 2 additions & 31 deletions lib/rack/freeze.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,5 @@

require_relative 'freeze/version'

require_relative 'freeze/sendfile'
require_relative 'freeze/show_exceptions'
require_relative 'freeze/urlmap'

module Rack
module Freeze
# Check if the given klass overrides `Kernel#freeze`.
def self.implements_freeze?(klass)
klass.instance_method(:freeze).owner != Kernel
end

# Generate a subclass with a generic #freeze method to freeze all instance variables.
def self.[] klass
# Check if the class already has a custom implementation of #freeze.. which we assume works correctly.
return klass if implements_freeze?(klass)

subclass = Class.new(klass) do
def freeze
# This ensures that all class variables are frozen.
self.instance_variables.each do |name|
self.instance_variable_get(name).freeze
end

super
end
end

return subclass
end
end
end
require_relative 'freeze/freezer'
require_relative 'freeze/builder'
18 changes: 12 additions & 6 deletions lib/rack/freeze/urlmap.rb → lib/rack/freeze/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,21 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

require 'rack/builder'
require_relative 'freezer'

module Rack
class URLMap
def freeze
@map.each do |location, app|
location.freeze
app.freeze
module Freeze
module Builder
def use(klass, *args, &block)
super(Freeze[klass], *args, &block)
end

super
def to_app
super.freeze
end
end
end

Builder.prepend(Freeze::Builder)
end
28 changes: 22 additions & 6 deletions lib/rack/freeze/sendfile.rb → lib/rack/freeze/freezer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,29 @@
# THE SOFTWARE.

module Rack
class Sendfile
def freeze
@app.freeze
@variation.freeze
@mappings.freeze
module Freeze
# Check if the given klass overrides `Kernel#freeze`.
def self.implements_freeze?(klass)
klass.instance_method(:freeze).owner != Kernel
end

# Generate a subclass with a generic #freeze method to freeze all instance variables.
def self.[] klass
# Check if the class already has a custom implementation of #freeze.. which we assume works correctly.
return klass if implements_freeze?(klass)

subclass = Class.new(klass) do
def freeze
# This ensures that all class variables are frozen.
self.instance_variables.each do |name|
self.instance_variable_get(name).freeze
end

super
end
end

super
return subclass
end
end
end
29 changes: 0 additions & 29 deletions lib/rack/freeze/show_exceptions.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/rack/freeze/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@

module Rack
module Freeze
VERSION = "1.0.0"
VERSION = "1.1.0"
end
end
30 changes: 16 additions & 14 deletions rack-freeze.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@
require_relative 'lib/rack/freeze/version'

Gem::Specification.new do |spec|
spec.name = "rack-freeze"
spec.version = Rack::Freeze::VERSION
spec.authors = ["Samuel Williams"]
spec.email = ["[email protected]"]
spec.name = "rack-freeze"
spec.version = Rack::Freeze::VERSION
spec.authors = ["Samuel Williams"]
spec.email = ["[email protected]"]

spec.summary = "Provides a policy for frozen rack middleware."
spec.homepage = "https://github.com/ioquatix/rack-freeze"
spec.summary = "Provides a policy for frozen rack middleware."
spec.homepage = "https://github.com/ioquatix/rack-freeze"

spec.files = `git ls-files -z`.split("\x0").reject do |f|
f.match(%r{^(test|spec|features)/})
end
spec.require_paths = ["lib"]

spec.add_development_dependency "bundler", "~> 1.14"
spec.add_development_dependency "rake", "~> 10.0"
spec.add_development_dependency "rspec", "~> 3.0"
spec.files = `git ls-files -z`.split("\x0").reject do |f|
f.match(%r{^(test|spec|features)/})
end
spec.require_paths = ["lib"]

spec.add_dependency "rack", "~> 2.0"

spec.add_development_dependency "bundler", "~> 1.14"
spec.add_development_dependency "rake", "~> 10.0"
spec.add_development_dependency "rspec", "~> 3.0"
end
41 changes: 41 additions & 0 deletions spec/rack/freeze/builder_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

RSpec.shared_context "app builder" do
it "should generate frozen app" do
# We need to give lexical scope for ruby to find it :)
middleware = described_class

app = Rack::Builder.new do
use middleware

run lambda { |env| [404, {}, []] }
end.to_app

expect(app.frozen?).to be_truthy
end
end

class NonConformingMiddleware
def initialize(app)
@app = app
end

def call(env)
return @app.call(env)
end
end

RSpec.describe NonConformingMiddleware do
include_context "app builder"
end

class ConformingMiddleware < NonConformingMiddleware
def freeze
@app.freeze

super
end
end

RSpec.describe ConformingMiddleware do
include_context "app builder"
end
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require "spec_helper"

RSpec.describe Rack::Freeze do
it "should detect class has \#freeze" do
Expand Down

0 comments on commit 3396ddf

Please sign in to comment.