def construct_search_query
query = '*'
query << 'AND (' << params[:search_all] << ')' if params[:search_all] && params[:search_all].length > 3
query << ' AND (' << params[:search_any].split(' ').join('OR') <<')' if params[:search_any] && params[:search_any].length > 3
query<< ' AND (\"' << params[:search_exact] <<'\")' if params[:search_exact] && params[:search_exact].length > 3
query << ' AND NOT (' << params[:search_exclude] <<')' if params[:search_exclude] && params[:search_exclude].length > 3
query << ' AND (title:' << params[:search_title] <<')' if params[:search_title] && params[:search_title].length > 3
query << ' AND (author_names:' << params[:search_author] <<')' if params[:search_author] && params[:search_author].length > 3
query << ' AND (publisher_name:' << params[:search_publisher] <<')' if params[:search_publisher] && params[:search_publisher].length >3
query << ' AND (description:' << params[:search_description] <<')' if params[:search_description] && params[:search_description].length > 3
query << ' AND (tags:' << params[:search_tags] <<')' if params[:search_tags] && params[:search_tags].length > 3
return query
end
Refactorings
No refactoring yet !
PabloBM
July 7, 2008, July 07, 2008 08:21, permalink
Not sure of what you are trying to do there, as that doesn't look like valid SQL to me, but you surely can do a bit better at least in the second half of the method. See example
def construct_search_query
query = '*'
query << 'AND (' << params[:search_all] << ')' if params[:search_all] && params[:search_all].length > 3
query << ' AND (' << params[:search_any].split(' ').join('OR') <<')' if params[:search_any] && params[:search_any].length > 3
query<< ' AND (\"' << params[:search_exact] <<'\")' if params[:search_exact] && params[:search_exact].length > 3
query << ' AND NOT (' << params[:search_exclude] <<')' if params[:search_exclude] && params[:search_exclude].length > 3
valid_params = {
:search_title => 'title',
:search_author => 'author_names',
:search_publisher => 'publisher_name',
:search_description => 'description',
:search_tags => 'tags'
}
valid_params.each do |key, field|
query << " AND (title:#{params[key]})" if params[key] && params[key].length > 3
end
query
end
Jeremy Weiskotten
July 7, 2008, July 07, 2008 13:44, permalink
Slight correction to Pablo's refactoring: need to parameterize the "title" portion of the query. I also extracted the logic that determines if a parameter should be considered to the method #valid_param? to DRY things up a bit. Also using string interpolation to build all of the parts of the query, which I think is easier to read than concatenation.
def valid_param?(key)
params[key] && params[key].length > 3
end
def construct_search_query
query = '*'
query << "AND (#{params[:search_all]})" if valid_param?(:search_all)
query << " AND (#{params[:search_any].split(' ').join('OR')})" if valid_param?(:search_any)
query << " AND (\"#{params[:search_exact]}\")" if valid_param?(:search_exact)
query << " AND NOT (#{params[:search_exclude]})" if valid_param?(:search_exclude)
{ :search_title => 'title',
:search_author => 'author_names',
:search_publisher => 'publisher_name',
:search_description => 'description',
:search_tags => 'tags' }.each do |param, field|
query << " AND (#{field}:#{params[param]})" if valid_param?(param)
end
query
end
PabloBM
July 7, 2008, July 07, 2008 13:47, permalink
Ooopsie. Indeed, Jeremy. Thanks for pointing out.
David Calavera
July 7, 2008, July 07, 2008 14:02, permalink
def construct_search_query
search = {
:search_all => ' AND (%s)',
:search_any => ' AND (%s)',
:search_exact => ' AND (\"%s\")',
:search_exclude => ' AND NOT (%s)'
}
valid_params = {
:search_title => 'title',
:search_author => 'author_names',
:search_publisher => 'publisher_name',
:search_description => 'description',
:search_tags => 'tags'
}
query = '*'
params = params.reject {|k, v| v.nil? || v.length <= 3}
params.keys.each do |k|
if (search[k])
query << search[k] % params[k]
else
query << " AND (%s:%s)" % [valid_params[k], params[k]]
end
end
query
end
Maciej Piechotka
July 7, 2008, July 07, 2008 14:55, permalink
DON'T DO IT!
It is broken by design - you can easily inject SQL Code into it.
PS. What it has in common with ferret?
foxdemon
July 8, 2008, July 08, 2008 07:15, permalink
Want to add rapid full-text searching to your Rails app? Acts_As_Ferret is the way to go.
http://www.railsenvy.com/2007/2/19/acts-as-ferret-tutorial
This method constructs a search querry for ferret (not SQL), so no SQL injection.
I want to construct a search query for the Rails acts_as_ferret plugin, but as you can see I'm... a noob. Surely there is a more elegant way to solve this problem.
http://www.railsenvy.com/2007/2/19/acts-as-ferret-tutorial