Gracefully handle eager-loading of a Search prior to DB creation

[mnarayan01]

Gracefully handle eager-loading a Search when the database/table corresponding to the search object has not yet been created (e.g. when using rake assets:precompile).

If an exception occurs, we assume that the column does exist, and thus generate a search method which queries on that column. Obviously this search will generate a DB exception if it is ever run.

[coveralls]

Changes Unknown when pulling 29a3a18 on mnarayan01:master into * on nathanl:master*.

[nathanl]

I'm not opposed to this change in general, but I don't want to use an inline rescue because it rescues any possible exception that descends from StandardError and could mask a completely different issue. (For instance, if model_class_for somehow returns something that doesn't have a columns_hash method, the MethodMissing would be hidden.)

Would you mind changing this to rescue the specific exception you expect?

[mnarayan01]

Unfortunately, the exception type is going to be adapter dependent (e.g. Mysql2::Error < StandardError), so I don't have a great way to filter other than StandardError.

My initial take is that rescuing StandardError is actually the correct action here, assuming I'm correct on the intended function: the false branch is there solely to provide the developer with a more informative message if the column is missing. If, however, the column is missing, the true branch will still result in an exception being thrown, it just won't be the customized searchlight one. Using your example (involving column_hash not being defined), currently there just would not be any way to call searches(attr), even if a custom search_attr method has been defined.

That said, I could move the conditional (https://github.com/nathanl/searchlight/pull/15/files#L0L23) to a function, which would allow me (or anyone else) to easily monkey-patch it.

[nathanl]

I see your point. Yeah, I would say to move the conditional to a method: if model_has_db_attribute?(attribute_name) or something. I propose you have that method rescue your adapter's error and let others add a rescue for their adapter's error if they get an exception there.

What do you think?

[nathanl]

Somewhat tangentially: I'm wondering if the adapters themselves are worthwhile. Their whole purpose is to keep you from having to define a few boilerplate search methods, which 1) may not be what you want anyway, and 2) could be easily defined by the user en masse using metaprogramming.

@mnarayan01, do you think adapters are worth the trouble? @adamhunter, do you still think so?

[coveralls]

Changes Unknown when pulling 6c7f5bd on mnarayan01:master into * on nathanl:master*.

[mnarayan01]

Updated to use model_has_db_attribute? as discussed.

Re: Adapters...I'm not personally using them, but I can see the utility they provide both for people who are completely new to Rails (and don't know how to create the appropriate methods), and people who are familiar with Rails (and don't want to create the methods). That said, it would be nice if they were either moved to different gems (probably more administrative overhead than its worth) or at least not auto-required. The latter would force the user to require them in an initializer or via the Gemfile if desired, but it would totally solve my issue.

On a related note, consider the following scenario using a fairly standard (AFAIK) workflow with capistrano:

1. cap deploy:web:disable - Bypass the Rails app
2. cap deploy - Deploy updates and restart the rails app
3. cap deploy:migrate - Perform database migrations
4. cap deploy:web:enable - Remove the Rails app bypass

If the migration added a column (e.g. column1) and a Searchlight::Search searched on that column using searches(:column1), there would be an oddity in that the adapter generated search method would be incorrect, since the column did not exist at the time the Searchlight::Search was eager-loaded in step (2).

This doesn't affect me as I'm not using the adapter based methods...just thought I'd throw it out there.

[nathanl]

Merged. Thank you!

Also, you make an excellent point regarding the deploy process. I don't like the fact that Searchlight could introduce weird behavior in the name of being helpful.

I may rip adapters out or rework them as you suggest at some point. If the code you just contributed disappears with the adapters, please know that you will still have contributed to the project by helping me understand the problems. :)