Jan
Jan

Reputation: 16074

define a classname in a variable without instanciate it

I am writing a query helper for my controllers to clean them up.

Normally I chain my queries depending on the given params

query = User.where(name: params[:name]
query = query.where("age > ?", params[:name]) if params[:name]
query = query.where(active: true) if params[:active]
# and so on

Now I wanna move them into a query_helper mdoule

query should take an initial value (or extracting it from the name of the controller where it's included.

Problem is, I can't pass a classname itself to query, it only works when I instantiate the class in the query like this query =

I could write

query_helper.rb

module QueryHelper

  attr_accessible :target

  # wont work
  def query(target_class = @query)
    @query = @query ? @query : eval(target_class.to_s.constantize)
  end

  # works, but only in the first run...
  # queries need a class only which is not instantiated.
  def query(target_class = @query)
    @query = @query ? @query : eval(target_class.to_s.constantize).new # .new
  end

  def query_name(name)
    query.where(name: name)
  end

  def query_age(age)
    query.where("age > ?", age)
  end

end

to use them like this:

query(:user) # select Model to search on
query_name(params[:name])
query_age(params[:age])
render json: query

# and so on

I know I can attach a query (or joins) in the query method to get it working, but I would decide after the first use what to do with query

def query(target)
  @query = @query || eval(target.to_s.constantize).joins(:comments)
end

I get it working in console with this: (but not in my method)

  :user.to_s.capitalize.constantize.new.class.where(name: "John Doe").count
  :user.to_s.capitalize.constantize.where(name: "John Doe").count

I hope you get the point what I try to achieve

Thank you in advance

Upvotes: 0

Views: 119

Answers (3)

max
max

Reputation: 101821

You can lookup constants from a string by using Module.const_get:

Module.const_get("TrueClass")

ActiveSupport also provides the String#constantize and String#safe_constantize methods which are somewhat more fluent and deal with module nesting slightly differently. eval should not be used here.

If you for example want to lookup a model based on the controller name you would do it by:

class ApplicationController < ActiveRecord::Base
  def self.model_name
    self.name.chomp("Controller")
  end

  def self.model_class
    model_name.constantize
  end
end

After that your question and attempted solution kind of falls apart. I don't think this is really suited to a helper method - instead just create a class that takes a set of parameters and a scope/class as arguments:

class Query
  include ActiveModel::Attributes
  include ActiveModel::AttributeAssignment

  attr_accessor :scope
  delegate :model, to: :scope

  # @param scope_or_class  [Class, ActiveRecord::Relation, nil]
  #   what we are querying
  # @param params [Hash]
  #   the parameters we are querying
  def initialize(scope_or_class = nil, **params)
     @scope = resolve_scope
     assign_attributes(params)
  end

  # Applies "filters" to the base scope
  #   you can override how any attribute is queried by providing a 
  # @return [ActiveRecord::Relation]
  def resolve
    self.attributes.compact.inject(scope.dup) do |memo, (key, val)|
       val = resolve_attribute(key)
       if val.is_a?(ActiveRecord::Relation)
         memo.merge(val)
       else
         memo.merge(scope.where(key => self.send(key)))
       end 
    end
  end

  private

  def resolve_scope(scope_or_class)
    case scope_or_class
    when Class
      scope_or_class.to_model.all
    when ActiveRecord::Relation
      scope_or_class
    when String
      scope_or_class.constantize.all
    when Nil
      self.class.name.chomp("Query").constantize
    end
  end

  def resolve_attribute(key)
    scope.respond_to?(key)
      scope.send(key, send(key))
    else
      send(key)
    end
  end
end

This is really just a boilerplate parent class that lets you use ActiveRecord::Attributes to assign attributes to your query objects.

This is the specific implementation:

class UserQuery < Query
  attribute :name
  attribute :age
end

We can override how age is queried by creating a scope:

class User
  scope :age, -> (age){ arel_table[:age].gt(age) }
end
# Infers model class
@users = UserQuery.new(**params.permit(:age, :name)).resolve
# Explicit class
@admins = UserQuery.new(Admin, **params.permit(:age, :name)).resolve
# With a scope
@active= UserQuery.new(User.where(active: true), **params.permit(:age, :name)).resolve

# model class from user input 
# I don't really recommend this. Its just here for completeness
model = begin
  # Do not let users intialize any class!
  if WHITELIST.include?(params[:model])
    params[:model]
  else
    DefaultClass
  end
end

@x = UserQuery.new(model, **params.permit(:age, :name)).resolve

The advantage is that it gives you a separate object that takes explicit input instead of putting even more responsiblities into your controller. This makes the code easier to test.

Remember that even if you extract the functionality into a module you're not really removing the responsibility - you're just tucking it away in another file.

While this approach might seem needlessly heavy/complex it really shines if you want to do stuff like add validations:

class UserQuery < Query
  include ActiveModel::Validations
  attribute :name
  attribute :age
 
  validates name: { minimum: 2 }
end

If you include the whole ActiveModel::Model you can also use it for form bindings:

<%= form_with(model: @query || UserQuery.new, url: '/users') do |f| %>
  <%= f.text_field(:name) %> 
<% end %>

Upvotes: 0

Sergio Tulentsev
Sergio Tulentsev

Reputation: 230296

Your proposed cleanup is not much of a cleanup, frankly. Your controller is still orchestrating the querying. The only thing you add is more complexity. The same effect, sans the query helper, can be achieved by using scopes

class User
  scope :with_name, ->(name) { where(name: name) }
  scope :active, -> { where(active: true) }
end

class UsersController
  def index
    @users = User.all
    @users = @users.with_name(params[:name]) if params[:name]
    @users = @users.active if params[:active]
    render json: @users
  end
end

What I would do is extract the query into a query object. Something like this:

class UsersController
  def index
    @users = QueryObjects::User::Index.call(params)

    render json: @users
  end
end

class QueryObjects::User::Index
  def self.call(params)
    new(params).call
  end
  
  attr_reader :name, :active
  def initialize(params)
    @name = params[:name]
    @active = params[:active]
    @result = User.all
  end
  
  def call
    query_name
    query_active
    result
  end
  
  private
  
  attr_reader :result
  
  def query_name
    @result = @result.with_name(name)
  end
  
  def query_active
    @result = @result.active
  end
end 

Upvotes: 1

tadman
tadman

Reputation: 211560

This already exists in Rails, it's called scopes. You can easily re-write what you have here:

scope :name, -> (name) { where(name: name) }
scope :age, -> (age) { where('age >', age) }
scope :active, -> (active = true) { where(active: active) }

Now you can do User.name('example').age(42).active and get matching user records.

To simplify this some more just add a simple "apply" method that will deal with the scope calls that may or may not happen:

def self.apply_scopes(scopes)
  scopes.inject(self) do |s, (name, args)|
    s.send(name, *args)
  end
end

Which can be used like:

User.apply_scopes(
  name: 'example',
  age: 42,
  active: true
)

Note: Be absolutely certain you're not allowing arbitrary calls here to methods on User that should not be accessed. It's worth having a list of allowed keys here so some punk doesn't send in delete_all: true somehow.

Upvotes: 1

Related Questions