From 458464c4d70b08be0cf803b55a72607935fa4305 Mon Sep 17 00:00:00 2001 From: "Gregory N. Schmit" Date: Wed, 1 Mar 2023 15:36:30 -0600 Subject: [PATCH] Ensure get_fields always returns strings, not symbols, for consistency. --- lib/rest_framework/controller_mixins/models.rb | 5 +++-- lib/rest_framework/filters.rb | 12 +++++------- lib/rest_framework/utils.rb | 6 ++++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/rest_framework/controller_mixins/models.rb b/lib/rest_framework/controller_mixins/models.rb index dcc948b..690c13d 100644 --- a/lib/rest_framework/controller_mixins/models.rb +++ b/lib/rest_framework/controller_mixins/models.rb @@ -91,9 +91,10 @@ def get_label(s) # Get the available fields. Returning `nil` indicates that anything should be accepted. If # `fallback` is true, then we should fallback to this controller's model columns, or an empty - # array. + # array. This should always return an array of strings, no symbols, and possibly `nil` (only if + # `fallback` is false). def get_fields(input_fields: nil, fallback: true) - input_fields ||= self.fields if fallback + input_fields ||= self.fields&.map(&:to_s) if fallback # If fields is a hash, then parse it. if input_fields.is_a?(Hash) diff --git a/lib/rest_framework/filters.rb b/lib/rest_framework/filters.rb index db096e9..f895301 100644 --- a/lib/rest_framework/filters.rb +++ b/lib/rest_framework/filters.rb @@ -14,14 +14,12 @@ class RESTFramework::ModelFilter < RESTFramework::BaseFilter # Get a list of filterset fields for the current action. Fallback to columns because we don't want # to try filtering by any query parameter because that could clash with other query parameters. def _get_fields - return ( - @controller.class.filterset_fields || @controller.get_fields(fallback: true) - ).map(&:to_s) + # Always return a list of strings; `@controller.get_fields` already does this. + return @controller.class.filterset_fields&.map(&:to_s) || @controller.get_fields(fallback: true) end # Filter params for keys allowed by the current action's filterset_fields/fields config. def _get_filter_params - # Map filterset fields to strings because query parameter keys are strings. fields = self._get_fields return @controller.request.query_parameters.select { |p, _| @@ -69,7 +67,7 @@ class RESTFramework::ModelOrderingFilter < RESTFramework::BaseFilter # Get a list of ordering fields for the current action. Do not fallback to columns in case the # user wants to order by a virtual column. def _get_fields - return (@controller.class.ordering_fields || @controller.get_fields)&.map(&:to_s) + return @controller.class.ordering_fields&.map(&:to_s) || @controller.get_fields end # Convert ordering string to an ordering configuration. @@ -119,10 +117,10 @@ class RESTFramework::ModelSearchFilter < RESTFramework::BaseFilter # common string-like columns by default. def _get_fields if search_fields = @controller.class.search_fields - return search_fields + return search_fields&.map(&:to_s) end - columns = @controller.class.get_model.columns_hash.keys + columns = @controller.class.get_model.column_names return @controller.get_fields(fallback: true).select { |f| f.in?(RESTFramework.config.search_columns) && f.in?(columns) } diff --git a/lib/rest_framework/utils.rb b/lib/rest_framework/utils.rb index 3ac071d..e8cb5ab 100644 --- a/lib/rest_framework/utils.rb +++ b/lib/rest_framework/utils.rb @@ -150,11 +150,13 @@ def self.parse_fields_hash(fields_hash, model, exclude_associations: nil) Rails.logger.warn("RRF: Unknown key in fields hash: #{k}") end - return parsed_fields + # We should always return strings, not symbols. + return parsed_fields.map(&:to_s) end # Get the fields for a given model, including not just columns (which includes - # foreign keys), but also associations. + # foreign keys), but also associations. Note that we always return an array of + # strings, not symbols. def self.fields_for(model, exclude_associations: nil) foreign_keys = model.reflect_on_all_associations(:belongs_to).map(&:foreign_key)