Validation failures

[ghost]

trafficstars

Algolia should only attempt to index an object if there aren't any validation failures. I've run into an issue where the gem is attempting to access a property that's nil and therefore Rails throws a undefined method '...' for nil:NilClass error.

Here's the set up: my model has a user_id property, which can't be nil. If I disable the Algolia-specific code in my model, Active Record rejects the save because the validation of user_id fails if I try to save it as nil. However, with Algolia enabled on that model I get the nil:NilClass error instead because I'm indexing a property of the related user, which is obviously nil in this case because user_id is nil.

Looking through algoliasearch-rails.rb, it seems to me as though you should return false from algolia_must_reindex? if object responds to valid? and valid? returns false.

Here's my abbreviated code for context:

class Video < ApplicationRecord
  include AlgoliaSearch

  belongs_to :user

  validates :user_id, presence: { message: 'cannot be blank, and must already exist within betamax' }

  algoliasearch per_environment: true do
    attribute :name, :author, :description, :created, :thumbnail
    attribute :tags do
      tag_names
    end
    attributesToIndex %w(tags name unordered(description) author)
  end

  def author
    user.name
  end
end

Even though the model fails validation, Algolia is still trying to access author, which returns nil if user_id is nil.

[redox]

Hey @micpringle, thank you for reporting such issue. It's indeed not an expected behavior.

Even though the model fails validation, Algolia is still trying to access author, which returns nil if user_id is nil.

Do you mind sharing with us the backtrace so I can identify the issue more easily?

[ghost]

Hey @redox,

Apologies for the belated response but I've been out of the country on vacation, and had limited and very flaky internet access.

I'm afraid I don't have the stack trace since I was forced to put a workaround in place so we could continue to move forward with the project. However, it should be relatively straightforward for you to replicate the issue using the code I've provided above.

Let me know if you have any questions or comments.

[marckohlbrugge]

I'm running into the same issue. Algolia tries to access my indexed attributes even when the record is invalid. This occasionally leads to similar problems as @micpringle described.

It seems like the after_validation callback is executed even if the record is invalid. A simple fix would be to first check whether the record is valid before proceeding with the rest of the callback.

[thdaraujo]

I was able to reproduce the issue on master. It seems that an attribute is read after_validation. Even if you change it to after_commit :index!, it still tries to read an attribute after you run validations.

It's a bit surprising, because I would like to be able to validate an invalid model and run validations as normal. But it's not possible, because an error is raised instead. See example and reproduction scripts below:

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'algoliasearch-rails', git: 'https://github.com/algolia/algoliasearch-rails.git', branch: 'master'
  gem 'rails', '6.0.3.6'
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

AlgoliaSearch.configuration = { application_id: '<APP_ID>', api_key: '<API_KEY>' }

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new($stdout)

ActiveRecord::Schema.define do
  create_table 'users', force: :cascade do |t|
    t.string 'name'
  end
  
  create_table 'user_settings', force: :cascade do |t|
    t.belongs_to :user, null: false
  end
end

class UserSetting < ActiveRecord::Base
  include AlgoliaSearch
  
  belongs_to :user, required: true, touch: true

  algoliasearch do
    attribute(:user_id) { user.id } # <------ raises error here
    attribute(:username) { user.name }
  end
end

class User < ActiveRecord::Base
  has_one :user_setting, dependent: :destroy
end

class BugTest < Minitest::Test
  def test_validations
    ActiveRecord::Base.transaction do
        user = User.create!(name: 'user name')

        UserSetting.without_auto_index do
            UserSetting.create!(user: user)
        end

        user_setting = UserSetting.last
        user_setting.user_id = nil

        # it should return false instead of raising an error
        assert_raises NoMethodError do
          user_setting.valid?
        end
        
        refute user_setting.valid?
    end
  end
end

# to reproduce, run:
# $ ruby algolia_issue119.rb

# ---------------------
# this error is raised:

# BugTest#test_validations:
# NoMethodError: undefined method `id' for nil:NilClass
#     algolia_issue119.rb:40:in `block (2 levels) in <class:UserSetting>'

I'm not entirely sure what causes the problem, but it seems related to the method algolia_mark_must_reindex being run on the after_validation callback.

If this is indeed a bug, what would be a good way to fix it? I can try and submit a fix, just want to make sure this is not the intended behaviour.