Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not fetch attribute values for excluded models #350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions lib/algoliasearch-rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,18 @@ def get_default_attributes(object)
end

def get_attribute_names(object)
get_attributes(object).keys
get_unresolved_attributes(object).keys
end

def attributes_to_hash(attributes, object)
if attributes
Hash[attributes.map { |name, value| [name.to_s, value.call(object) ] }]
Hash[attributes.map { |name, value| [name.to_s, value.is_a?(Proc) ? value.call(object) : value] }]
else
{}
end
end

def get_attributes(object)
def get_unresolved_attributes(object)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 164 would still resolve your attributes. should be changed as @serializer._attributes to get attribute names without actually calculating the values.

# If a serializer is set, we ignore attributes
# everything should be done via the serializer
if not @serializer.nil?
Expand All @@ -168,17 +168,23 @@ def get_attributes(object)
attributes = get_default_attributes(object)
else
# at least 1 `attribute ...` has been configured, therefore use ONLY the one configured
if is_active_record?(object)
object.class.unscoped do
attributes = attributes_to_hash(@attributes, object)
end
else
attributes = attributes_to_hash(@attributes, object)
end
attributes = @attributes
end
end

attributes.merge!(attributes_to_hash(@additional_attributes, object)) if @additional_attributes
attributes.merge!(@additional_attributes) if @additional_attributes
attributes
end

def get_attributes(object)
attributes = get_unresolved_attributes(object)
if is_active_record?(object)
object.class.unscoped do
attributes = attributes_to_hash(attributes, object)
end
else
attributes = attributes_to_hash(attributes, object)
end

if @options[:sanitize]
sanitizer = begin
Expand Down
25 changes: 24 additions & 1 deletion spec/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@
create_table :disabled_symbols do |t|
t.string :name
end
create_table :disabled_conditionals do |t|
t.string :name
end
create_table :encoded_strings do |t|
end
create_table :forward_to_replicas do |t|
Expand Down Expand Up @@ -208,6 +211,21 @@ def self.truth
end
end

class DisabledConditional < ActiveRecord::Base
include AlgoliaSearch

algoliasearch :synchronous => true, :index_name => safe_index_name("DisabledConditional"), :unless => :nil_name do
attribute :name_length do
# will crash if name is nil
name.length
end
end

def nil_name
name.nil?
end
end

module Namespaced
def self.table_name_prefix
'namespaced_'
Expand Down Expand Up @@ -506,7 +524,7 @@ class SerializedObject < ActiveRecord::Base
it "should push the name but not the other attribute" do
o = SerializedObject.new :name => 'test', :skip => 'skip me'
attributes = SerializedObject.algoliasearch_settings.get_attributes(o)
expect(attributes).to eq({:name => 'test', "_tags" => ['tag1', 'tag2']})
expect(attributes).to eq({"name" => 'test', "_tags" => ['tag1', 'tag2']})
end
end
end
Expand Down Expand Up @@ -1342,6 +1360,11 @@ class ReplicaThenSlave
DisabledSymbol.create :name => 'foo'
expect(DisabledSymbol.search('').size).to eq(0)
end

it "should disable the indexing using a conditional and not read attributes" do
DisabledConditional.create :name => nil
expect(DisabledConditional.search('').size).to eq(0)
end
end

describe 'NullableId' do
Expand Down