Delegate cookies management to Grape::Request (#2549)

Author: ericproulx

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#2536](https://github.com/ruby-grape/grape/pull/2536): Update normalize_path like Rails - [@ericproulx](https://github.com/ericproulx).
 * [#2540](https://github.com/ruby-grape/grape/pull/2540): Introduce Params builder with symbolized short name - [@ericproulx](https://github.com/ericproulx).
 * [#2550](https://github.com/ruby-grape/grape/pull/2550): Drop ActiveSupport 6.0 - [@ericproulx](https://github.com/ericproulx).
+* [#2549](https://github.com/ruby-grape/grape/pull/2549): Delegate cookies management to `Grape::Request` - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/cookies.rb

@@ -2,41 +2,49 @@
 
 module Grape
   class Cookies
-    def initialize
-      @cookies = {}
-      @send_cookies = {}
-    end
+    extend Forwardable
 
-    def read(request)
-      request.cookies.each do |name, value|
-        @cookies[name.to_s] = value
-      end
+    DELETED_COOKIES_ATTRS = {
+      max_age: '0',
+      value: '',
+      expires: Time.at(0)
+    }.freeze
+
+    def_delegators :cookies, :[], :each
+
+    def initialize(rack_cookies)
+      @cookies = rack_cookies
+      @send_cookies = nil
     end
 
-    def write(header)
-      @cookies.select { |key, _value| @send_cookies[key] == true }.each do |name, value|
-        cookie_value = value.is_a?(Hash) ? value : { value: value }
-        Rack::Utils.set_cookie_header! header, name, cookie_value
+    def response_cookies
+      return unless @send_cookies
+
+      send_cookies.each do |name|
+        yield name, cookies[name]
       end
     end
 
-    def [](name)
-      @cookies[name.to_s]
+    def []=(name, value)
+      cookies[name] = value
+      send_cookies << name
     end
 
-    def []=(name, value)
-      @cookies[name.to_s] = value
-      @send_cookies[name.to_s] = true
+    # see https://github.com/rack/rack/blob/main/lib/rack/utils.rb#L338-L340
+    def delete(name, **opts)
+      self.[]=(name, opts.merge(DELETED_COOKIES_ATTRS))
     end
 
-    def each(&block)
-      @cookies.each(&block)
+    private
+
+    def cookies
+      return @cookies unless @cookies.is_a?(Proc)
+
+      @cookies = @cookies.call.with_indifferent_access
     end
 
-    # see https://github.com/rack/rack/blob/main/lib/rack/utils.rb#L338-L340
-    def delete(name, **opts)
-      options = opts.merge(max_age: '0', value: '', expires: Time.at(0))
-      self.[]=(name, options)
+    def send_cookies
+      @send_cookies ||= Set.new
     end
   end
 end

lib/grape/dsl/inside_route.rb

@@ -257,18 +257,6 @@ def content_type(val = nil)
         end
       end
 
-      # Set or get a cookie
-      #
-      # @example
-      #   cookies[:mycookie] = 'mycookie val'
-      #   cookies['mycookie-string'] = 'mycookie string val'
-      #   cookies[:more] = { value: '123', expires: Time.at(0) }
-      #   cookies.delete :more
-      #
-      def cookies
-        @cookies ||= Cookies.new
-      end
-
       # Allows you to define the response body as something other than the
       # return value.
       #

lib/grape/endpoint.rb

@@ -13,7 +13,8 @@ class Endpoint
     attr_accessor :block, :source, :options
     attr_reader :env, :request
 
-    def_delegators :request, :params, :headers
+    def_delegators :request, :params, :headers, :cookies
+    def_delegator :cookies, :response_cookies
 
     class << self
       def new(...)
@@ -164,10 +165,9 @@ def mount_in(router)
 
     def to_routes
       default_route_options = prepare_default_route_attributes
-      default_path_settings = prepare_default_path_settings
 
       map_routes do |method, raw_path|
-        prepared_path = Path.new(raw_path, namespace, default_path_settings)
+        prepared_path = Path.new(raw_path, namespace, prepare_default_path_settings)
         params = options[:route_options].present? ? options[:route_options].merge(default_route_options) : default_route_options
         route = Grape::Router::Route.new(method, prepared_path.origin, prepared_path.suffix, params)
         route.apply(self)
@@ -248,18 +248,16 @@ def inspect
 
     def run
       ActiveSupport::Notifications.instrument('endpoint_run.grape', endpoint: self, env: env) do
-        @header = Grape::Util::Header.new
         @request = Grape::Request.new(env, build_params_with: namespace_inheritable(:build_params_with))
         begin
-          cookies.read(@request)
           self.class.run_before_each(self)
           run_filters befores, :before
 
-          if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS])
-            allow_header_value = allowed_methods.join(', ')
-            raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allow_header_value)) unless options?
+          if env.key?(Grape::Env::GRAPE_ALLOWED_METHODS)
+            header['Allow'] = env[Grape::Env::GRAPE_ALLOWED_METHODS].join(', ')
+            raise Grape::Exceptions::MethodNotAllowed.new(header) unless options?
 
-            header Grape::Http::Headers::ALLOW, allow_header_value
+            header Grape::Http::Headers::ALLOW, header['Allow']
             response_object = ''
             status 204
           else
@@ -270,7 +268,7 @@ def run
           end
 
           run_filters afters, :after
-          cookies.write(header)
+          build_response_cookies
 
           # status verifies body presence when DELETE
           @body ||= response_object
@@ -332,24 +330,10 @@ def run_filters(filters, type = :other)
       extend post_extension if post_extension
     end
 
-    def befores
-      namespace_stackable(:befores)
-    end
-
-    def before_validations
-      namespace_stackable(:before_validations)
-    end
-
-    def after_validations
-      namespace_stackable(:after_validations)
-    end
-
-    def afters
-      namespace_stackable(:afters)
-    end
-
-    def finallies
-      namespace_stackable(:finallies)
+    %i[befores before_validations after_validations afters finallies].each do |method|
+      define_method method do
+        namespace_stackable(method)
+      end
     end
 
     def validations
@@ -417,5 +401,12 @@ def build_helpers
 
       Module.new { helpers.each { |mod_to_include| include mod_to_include } }
     end
+
+    def build_response_cookies
+      response_cookies do |name, value|
+        cookie_value = value.is_a?(Hash) ? value : { value: value }
+        Rack::Utils.set_cookie_header! header, name, cookie_value
+      end
+    end
   end
 end

lib/grape/request.rb

@@ -6,6 +6,7 @@ class Request < Rack::Request
     HTTP_PREFIX = 'HTTP_'
 
     alias rack_params params
+    alias rack_cookies cookies
 
     def initialize(env, build_params_with: nil)
       super(env)
@@ -20,6 +21,10 @@ def headers
       @headers ||= build_headers
     end
 
+    def cookies
+      @cookies ||= Grape::Cookies.new(-> { rack_cookies })
+    end
+
     # needs to be public until extensions param_builder are removed
     def grape_routing_args
       # preserve version from query string parameters

spec/grape/dsl/inside_route_spec.rb

@@ -165,12 +165,6 @@ def initialize
     end
   end
 
-  describe '#cookies' do
-    it 'returns an instance of Cookies' do
-      expect(subject.cookies).to be_a Grape::Cookies
-    end
-  end
-
   describe '#body' do
     describe 'set' do
       before do