From 967db12876309d45ccec9a074927a9238a012577 Mon Sep 17 00:00:00 2001 From: namusyaka Date: Fri, 15 Jan 2016 03:55:43 +0900 Subject: [PATCH] Add header support for middleware Extract #headers from DSL::InsideRoute to DSL::Headers --- .rubocop_todo.yml | 13 ++---- CHANGELOG.md | 1 + lib/grape/dsl/headers.rb | 16 ++++++++ lib/grape/dsl/inside_route.rb | 12 +----- lib/grape/middleware/base.rb | 19 ++++++++- spec/grape/dsl/headers_spec.rb | 32 +++++++++++++++ spec/grape/dsl/inside_route_spec.rb | 18 --------- spec/grape/middleware/base_spec.rb | 62 +++++++++++++++++++++++++++++ 8 files changed, 135 insertions(+), 38 deletions(-) create mode 100644 lib/grape/dsl/headers.rb create mode 100644 spec/grape/dsl/headers_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9734fe96f4..69e4343dc7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,16 +1,11 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2015-11-23 18:15:01 -0500 using RuboCop version 0.35.1. +# on 2016-01-15 12:33:35 +0900 using RuboCop version 0.35.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 -Lint/DuplicatedKey: - Exclude: - - 'spec/grape/validations_spec.rb' - # Offense count: 1 Lint/NestedMethodDefinition: Exclude: @@ -33,12 +28,12 @@ Metrics/ClassLength: Metrics/CyclomaticComplexity: Max: 14 -# Offense count: 750 +# Offense count: 760 # Configuration parameters: AllowURI, URISchemes. Metrics/LineLength: Max: 215 -# Offense count: 44 +# Offense count: 45 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 36 @@ -62,7 +57,7 @@ Style/BlockDelimiters: - 'spec/grape/middleware/versioner/header_spec.rb' - 'spec/grape/request_spec.rb' -# Offense count: 105 +# Offense count: 106 # Configuration parameters: Exclude. Style/Documentation: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 93313ac457..34b6dc504e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * [#1232](https://github.com/ruby-grape/grape/pull/1232): Helpers are now available inside `rescue_from` - [@namusyaka](https://github.com/namusyaka). * [#1237](https://github.com/ruby-grape/grape/pull/1237): Allow multiple parameters in `given`, which behaves as if the scopes were nested in the inputted order - [@ochagata](https://github.com/ochagata). * [#1238](https://github.com/ruby-grape/grape/pull/1238): Call `after` of middleware on error - [@namusyaka](https://github.com/namusyaka). +* [#1243](https://github.com/ruby-grape/grape/pull/1243): Add `header` support for middleware - [@namusyaka](https://github.com/namusyaka). * Your contribution here. #### Fixes diff --git a/lib/grape/dsl/headers.rb b/lib/grape/dsl/headers.rb new file mode 100644 index 0000000000..e634cdfc58 --- /dev/null +++ b/lib/grape/dsl/headers.rb @@ -0,0 +1,16 @@ +module Grape + module DSL + module Headers + # Set an individual header or retrieve + # all headers that have been set. + def header(key = nil, val = nil) + if key + val ? header[key.to_s] = val : header.delete(key.to_s) + else + @header ||= {} + end + end + alias_method :headers, :header + end + end +end diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 704320d0a6..86f7a587eb 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -1,10 +1,12 @@ require 'active_support/concern' +require 'grape/dsl/headers' module Grape module DSL module InsideRoute extend ActiveSupport::Concern include Grape::DSL::Settings + include Grape::DSL::Headers # Denotes a situation where a DSL method has been invoked in a # filter which it should not yet be available in @@ -138,16 +140,6 @@ def status(status = nil) end end - # Set an individual header or retrieve - # all headers that have been set. - def header(key = nil, val = nil) - if key - val ? @header[key.to_s] = val : @header.delete(key.to_s) - else - @header - end - end - # Set response content-type def content_type(val = nil) if val diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index ced61811f5..c611aa3de7 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -1,9 +1,13 @@ +require 'grape/dsl/headers' + module Grape module Middleware class Base attr_reader :app, :env, :options TEXT_HTML = 'text/html'.freeze + include Grape::DSL::Headers + # @param [Rack Application] app The standard argument for a Rack middleware. # @param [Hash] options A hash of options, simply stored for use by subclasses. def initialize(app, options = {}) @@ -27,7 +31,10 @@ def call!(env) ensure after_response = after end - after_response || @app_response + + response = after_response || @app_response + merge_headers response + response end # @abstract @@ -65,6 +72,16 @@ def mime_types end types_without_params end + + private + + def merge_headers(response) + return unless headers.is_a?(Hash) + case response + when Rack::Response then response.headers.merge!(headers) + when Array then response[1].merge!(headers) + end + end end end end diff --git a/spec/grape/dsl/headers_spec.rb b/spec/grape/dsl/headers_spec.rb new file mode 100644 index 0000000000..de5763f103 --- /dev/null +++ b/spec/grape/dsl/headers_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +module Grape + module DSL + module HeadersSpec + class Dummy + include Grape::DSL::Headers + end + end + describe Headers do + subject { HeadersSpec::Dummy.new } + + describe '#header' do + describe 'set' do + before do + subject.header 'Name', 'Value' + end + + it 'returns value' do + expect(subject.header['Name']).to eq 'Value' + expect(subject.header('Name')).to eq 'Value' + end + end + + it 'returns nil' do + expect(subject.header['Name']).to be nil + expect(subject.header('Name')).to be nil + end + end + end + end +end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index b7b995d569..3c300e71ca 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -128,24 +128,6 @@ def initialize end end - describe '#header' do - describe 'set' do - before do - subject.header 'Name', 'Value' - end - - it 'returns value' do - expect(subject.header['Name']).to eq 'Value' - expect(subject.header('Name')).to eq 'Value' - end - end - - it 'returns nil' do - expect(subject.header['Name']).to be nil - expect(subject.header('Name')).to be nil - end - end - describe '#content_type' do describe 'set' do before do diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb index 7a3a2a4565..9f9137770e 100644 --- a/spec/grape/middleware/base_spec.rb +++ b/spec/grape/middleware/base_spec.rb @@ -118,4 +118,66 @@ def default_options end end end + + context 'header' do + module HeaderSpec + class ExampleWare < Grape::Middleware::Base + def before + header 'X-Test-Before', 'Hi' + end + + def after + header 'X-Test-After', 'Bye' + nil + end + end + end + + def app + Rack::Builder.app do + use HeaderSpec::ExampleWare + run ->(_) { [200, {}, ['Yeah']] } + end + end + + it 'is able to set a header' do + get '/' + expect(last_response.headers['X-Test-Before']).to eq('Hi') + expect(last_response.headers['X-Test-After']).to eq('Bye') + end + end + + context 'header overwrite' do + module HeaderOverwritingSpec + class ExampleWare < Grape::Middleware::Base + def before + header 'X-Test-Overwriting', 'Hi' + end + + def after + header 'X-Test-Overwriting', 'Bye' + nil + end + end + + class API < Grape::API + get('/') do + header 'X-Test-Overwriting', 'Yeah' + 'Hello' + end + end + end + + def app + Rack::Builder.app do + use HeaderOverwritingSpec::ExampleWare + run HeaderOverwritingSpec::API.new + end + end + + it 'overwrites header by after headers' do + get '/' + expect(last_response.headers['X-Test-Overwriting']).to eq('Bye') + end + end end