itstommymorgan / asari

a Ruby wrapper for AWS CloudSearch.
52 stars 51 forks source link

Using object class name and id to build the id when adding, updating and... #10

Open mmeys opened 11 years ago

mmeys commented 11 years ago

... deleting active record object from index. This allow to use the asari_index method on multiple active record models indexed in the same domain.

itstommymorgan commented 11 years ago

Hi @mmeys, a couple of issues -

1) This PR is incomplete, I think - it updates all of the AR hooks, but doesn't update the asari_search method.

2) Only storing the numeric ID is actually an intentional design, although it's one I'm willing to discuss. First, in most cases, storing your data models in different tables means that they're different enough that you don't want to search them together. If you do want to search them together, then you're probably going to need to build some custom search logic around it anyway. Second, for these "simple" cases, having just the numeric ID makes the AR lookup in asari_search much faster, so it's optimized for performance.

mmeys commented 11 years ago

Hi @duwanis.

Thanks for your answer.

For the first issue, I guess you were talking about the asari_find method (and not asari_search). I corrected it in the last commit and I also changed the format of the id to fit amazon requirements (no uppercase and no '-').

I understand your point of view for the second issue but when you implement a global search bar for an application, you may have to display results from different models. For example, Facebook's search bar will return people, pages, groups, etc. which are different models but that you may want to index in a unique domain under a global representation format. Finally for the issue of performance my correction just add a regex on the returned id when mapping the result from the search. Most of the time, these results will be paginated so it will not be very costly.

itstommymorgan commented 11 years ago

In re 1) - yeah, brainfart :)

In re 2) Right, the question wasn't "is this ever useful," the question is "should this be the default behavior?" I'm leaning towards no because the way it's currently implemented is simple and easy to override if you want (we do that very thing at Treehouse, actually, for our library search), I don't think it represents the majority of the use cases, and you're very likely going to need to write custom code to fit all those different models into one meaningful search index anyway (in our experience). But like I said I'm willing to discuss that matter further.

mmeys commented 11 years ago

Indeed, you're right! I'll propose a new version of asari_find.

mmeys commented 11 years ago

Hi @duwanis,

I corrected the asari_find method to filter the results with the current model. I also added the possibility to do a query that contains both boolean parameters and classic parameters. This is very useful to do searches with a filter on a facet.