Remove Grape::Util::Registrable + Small refactors (#2475)

ericproulx · web-flow · commit fb67ea9940ea · 2024-07-22T16:10:10.000-04:00
* Refactor ErrorFormatter, Parser and Formatter
Remove Grape::Util::Registrable
Add Grape::MimeTypes + spec
Small refactors

* Add CHANGELOG.md
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,11 +2,13 @@
 
 #### Features
 
+* [#2475](https://github.com/ruby-grape/grape/pull/2475): Remove Grape::Util::Registrable - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes
 
 * [#2471](https://github.com/ruby-grape/grape/pull/2471): Fix absence of original_exception and/or backtrace even if passed in error! - [@numbata](https://github.com/numbata).
+* Your contribution here.
 
 ### 2.1.3 (2024-07-13)
 
diff --git a/lib/grape/content_types.rb b/lib/grape/content_types.rb
@@ -2,24 +2,29 @@
 
 module Grape
   module ContentTypes
-    extend Util::Registrable
+    module_function
 
     # Content types are listed in order of preference.
-    CONTENT_TYPES = {
+    DEFAULTS = {
       xml: 'application/xml',
       serializable_hash: 'application/json',
       json: 'application/json',
       binary: 'application/octet-stream',
       txt: 'text/plain'
     }.freeze
 
-    class << self
-      def content_types_for_settings(settings)
-        settings&.inject(:merge!)
-      end
+    MIME_TYPES = Grape::ContentTypes::DEFAULTS.except(:serializable_hash).invert.freeze
+
+    def content_types_for(from_settings)
+      from_settings.presence || DEFAULTS
+    end
+
+    def mime_types_for(from_settings)
+      return MIME_TYPES if from_settings == Grape::ContentTypes::DEFAULTS
 
-      def content_types_for(from_settings)
-        from_settings.presence || Grape::ContentTypes::CONTENT_TYPES.merge(default_elements)
+      from_settings.each_with_object({}) do |(k, v), types_without_params|
+        # remove optional parameter
+        types_without_params[v.split(';', 2).first] = k
       end
     end
   end
diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb
@@ -17,18 +17,16 @@ def default_format(new_format = nil)
         # Specify the format for the API's serializers.
         # May be `:json`, `:xml`, `:txt`, etc.
         def format(new_format = nil)
-          if new_format
-            namespace_inheritable(:format, new_format.to_sym)
-            # define the default error formatters
-            namespace_inheritable(:default_error_formatter, Grape::ErrorFormatter.formatter_for(new_format, **{}))
-            # define a single mime type
-            mime_type = content_types[new_format.to_sym]
-            raise Grape::Exceptions::MissingMimeType.new(new_format) unless mime_type
-
-            namespace_stackable(:content_types, new_format.to_sym => mime_type)
-          else
-            namespace_inheritable(:format)
-          end
+          return namespace_inheritable(:format) unless new_format
+
+          symbolic_new_format = new_format.to_sym
+          namespace_inheritable(:format, symbolic_new_format)
+          namespace_inheritable(:default_error_formatter, Grape::ErrorFormatter.formatter_for(symbolic_new_format))
+
+          content_type = content_types[symbolic_new_format]
+          raise Grape::Exceptions::MissingMimeType.new(new_format) unless content_type
+
+          namespace_stackable(:content_types, symbolic_new_format => content_type)
         end
 
         # Specify a custom formatter for a content-type.
@@ -43,12 +41,10 @@ def parser(content_type, new_parser)
 
         # Specify a default error formatter.
         def default_error_formatter(new_formatter_name = nil)
-          if new_formatter_name
-            new_formatter = Grape::ErrorFormatter.formatter_for(new_formatter_name, **{})
-            namespace_inheritable(:default_error_formatter, new_formatter)
-          else
-            namespace_inheritable(:default_error_formatter)
-          end
+          return namespace_inheritable(:default_error_formatter) unless new_formatter_name
+
+          new_formatter = Grape::ErrorFormatter.formatter_for(new_formatter_name)
+          namespace_inheritable(:default_error_formatter, new_formatter)
         end
 
         def error_formatter(format, options)
diff --git a/lib/grape/error_formatter.rb b/lib/grape/error_formatter.rb
@@ -2,34 +2,22 @@
 
 module Grape
   module ErrorFormatter
-    extend Util::Registrable
+    module_function
 
-    class << self
-      def builtin_formatters
-        @builtin_formatters ||= {
-          serializable_hash: Grape::ErrorFormatter::Json,
-          json: Grape::ErrorFormatter::Json,
-          jsonapi: Grape::ErrorFormatter::Json,
-          txt: Grape::ErrorFormatter::Txt,
-          xml: Grape::ErrorFormatter::Xml
-        }
-      end
+    DEFAULTS = {
+      serializable_hash: Grape::ErrorFormatter::Json,
+      json: Grape::ErrorFormatter::Json,
+      jsonapi: Grape::ErrorFormatter::Json,
+      txt: Grape::ErrorFormatter::Txt,
+      xml: Grape::ErrorFormatter::Xml
+    }.freeze
 
-      def formatters(**options)
-        builtin_formatters.merge(default_elements).merge!(options[:error_formatters] || {})
-      end
+    def formatter_for(format, error_formatters = nil, default_error_formatter = nil)
+      select_formatter(error_formatters, format) || default_error_formatter || DEFAULTS[:txt]
+    end
 
-      def formatter_for(api_format, **options)
-        spec = formatters(**options)[api_format]
-        case spec
-        when nil
-          options[:default_error_formatter] || Grape::ErrorFormatter::Txt
-        when Symbol
-          method(spec)
-        else
-          spec
-        end
-      end
+    def select_formatter(error_formatters, format)
+      error_formatters&.key?(format) ? error_formatters[format] : DEFAULTS[format]
     end
   end
 end
diff --git a/lib/grape/formatter.rb b/lib/grape/formatter.rb
@@ -2,34 +2,24 @@
 
 module Grape
   module Formatter
-    extend Util::Registrable
+    module_function
 
-    class << self
-      def builtin_formatters
-        @builtin_formatters ||= {
-          json: Grape::Formatter::Json,
-          jsonapi: Grape::Formatter::Json,
-          serializable_hash: Grape::Formatter::SerializableHash,
-          txt: Grape::Formatter::Txt,
-          xml: Grape::Formatter::Xml
-        }
-      end
+    DEFAULTS = {
+      json: Grape::Formatter::Json,
+      jsonapi: Grape::Formatter::Json,
+      serializable_hash: Grape::Formatter::SerializableHash,
+      txt: Grape::Formatter::Txt,
+      xml: Grape::Formatter::Xml
+    }.freeze
 
-      def formatters(**options)
-        builtin_formatters.merge(default_elements).merge!(options[:formatters] || {})
-      end
+    DEFAULT_LAMBDA_FORMATTER = ->(obj, _env) { obj }
 
-      def formatter_for(api_format, **options)
-        spec = formatters(**options)[api_format]
-        case spec
-        when nil
-          ->(obj, _env) { obj }
-        when Symbol
-          method(spec)
-        else
-          spec
-        end
-      end
+    def formatter_for(api_format, formatters)
+      select_formatter(formatters, api_format) || DEFAULT_LAMBDA_FORMATTER
+    end
+
+    def select_formatter(formatters, api_format)
+      formatters&.key?(api_format) ? formatters[api_format] : DEFAULTS[api_format]
     end
   end
 end
diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb
@@ -61,22 +61,20 @@ def response
         @app_response = Rack::Response.new(@app_response[2], @app_response[0], @app_response[1])
       end
 
-      def content_type_for(format)
-        HashWithIndifferentAccess.new(content_types)[format]
+      def content_types
+        @content_types ||= Grape::ContentTypes.content_types_for(options[:content_types])
       end
 
-      def content_types
-        ContentTypes.content_types_for(options[:content_types])
+      def mime_types
+        @mime_types ||= Grape::ContentTypes.mime_types_for(content_types)
       end
 
-      def content_type
-        content_type_for(env[Grape::Env::API_FORMAT] || options[:format]) || TEXT_HTML
+      def content_type_for(format)
+        content_types_indifferent_access[format]
       end
 
-      def mime_types
-        @mime_types ||= content_types.each_pair.with_object({}) do |(k, v), types_without_params|
-          types_without_params[v.split(';').first] = k
-        end
+      def content_type
+        content_type_for(env[Grape::Env::API_FORMAT] || options[:format]) || TEXT_HTML
       end
 
       private
@@ -89,6 +87,10 @@ def merge_headers(response)
         when Array          then response[1].merge!(headers)
         end
       end
+
+      def content_types_indifferent_access
+        @content_types_indifferent_access ||= content_types.with_indifferent_access
+      end
     end
   end
 end
diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb
@@ -45,7 +45,7 @@ def rack_response(status, headers, message)
 
       def format_message(message, backtrace, original_exception = nil)
         format = env[Grape::Env::API_FORMAT] || options[:format]
-        formatter = Grape::ErrorFormatter.formatter_for(format, **options)
+        formatter = Grape::ErrorFormatter.formatter_for(format, options[:error_formatters], options[:default_error_formatter])
         return formatter.call(message, backtrace, options, env, original_exception) if formatter
 
         throw :error,
diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb
@@ -54,7 +54,7 @@ def build_formatted_response(status, headers, bodies)
 
       def fetch_formatter(headers, options)
         api_format = mime_types[headers[Rack::CONTENT_TYPE]] || env[Grape::Env::API_FORMAT]
-        Grape::Formatter.formatter_for(api_format, **options)
+        Grape::Formatter.formatter_for(api_format, options[:formatters])
       end
 
       # Set the content type header for the API format if it is not already present.
@@ -97,7 +97,7 @@ def read_rack_input(body)
         fmt = request.media_type ? mime_types[request.media_type] : options[:default_format]
 
         throw :error, status: 415, message: "The provided content-type '#{request.media_type}' is not supported." unless content_type_for(fmt)
-        parser = Grape::Parser.parser_for fmt, **options
+        parser = Grape::Parser.parser_for fmt, options[:parsers]
         if parser
           begin
             body = (env[Grape::Env::API_REQUEST_BODY] = parser.call(body, env))
diff --git a/lib/grape/parser.rb b/lib/grape/parser.rb
@@ -2,32 +2,16 @@
 
 module Grape
   module Parser
-    extend Util::Registrable
+    module_function
 
-    class << self
-      def builtin_parsers
-        @builtin_parsers ||= {
-          json: Grape::Parser::Json,
-          jsonapi: Grape::Parser::Json,
-          xml: Grape::Parser::Xml
-        }
-      end
+    DEFAULTS = {
+      json: Grape::Parser::Json,
+      jsonapi: Grape::Parser::Json,
+      xml: Grape::Parser::Xml
+    }.freeze
 
-      def parsers(**options)
-        builtin_parsers.merge(default_elements).merge!(options[:parsers] || {})
-      end
-
-      def parser_for(api_format, **options)
-        spec = parsers(**options)[api_format]
-        case spec
-        when nil
-          nil
-        when Symbol
-          method(spec)
-        else
-          spec
-        end
-      end
+    def parser_for(format, parsers = nil)
+      parsers&.key?(format) ? parsers[format] : DEFAULTS[format]
     end
   end
 end
diff --git a/lib/grape/util/accept_header_handler.rb b/lib/grape/util/accept_header_handler.rb
@@ -13,7 +13,7 @@ def initialize(accept_header:, versions:, **options)
         @cascade = options.fetch(:cascade, true)
       end
 
-      def match_best_quality_media_type!(content_types: Grape::ContentTypes::CONTENT_TYPES, allowed_methods: nil)
+      def match_best_quality_media_type!(content_types: Grape::ContentTypes::DEFAULTS, allowed_methods: nil)
         return unless vendor
 
         strict_header_checks!
diff --git a/lib/grape/util/registrable.rb b/lib/grape/util/registrable.rb
diff --git a/spec/grape/content_types_spec.rb b/spec/grape/content_types_spec.rb
diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb
diff --git a/spec/grape/parser_spec.rb b/spec/grape/parser_spec.rb
