One idea: allow before_filter to customize engine args. Test needed

[jrochkind]

No description provided.

[jrochkind]

@magibney says:

That said, I like #36, and I believe it would certainly work for our use case. The only downside I can think of is that if you have multiple different engines in a single deployment, and custom filtering for each, #36 wouldn't allow a clear way to scope per engine (I think you'd be stuck with top-level if/elses, although this would surely get the job done).

There are ways you could do this without if-else. I think you can actually put whatever you want as a configuration, so you could:

BentoSearch.register_engine("gbs") do |conf|
       conf.engine = "BentoSearch::GoogleBooksEngine"
       conf.api_key = "my_google_api_key"
       conf.my_custom_engine_params_logic = lambda do |arg, arg2|
         whatever_hash_returned
       end
end

SearchController.before_action do |controller|
     controller.engine_args = 
         controller.engine.configuration.my_custom_engine_params_logic.call(controller.params, whatever)
end

I think that would work, or something along those lines, but haven't tested it. BentoSearch is meant to give you the tools you need to easily and elegantly build the architecture that works for you, not neccesarily provide every architecture you might need out of the box. But I'm still not certain how common this use case is and how pretty we need to make it.

One other potential downside to #36: in its current state, it bypasses safe_search_args (unless the filtering code goes out of its way to call it), and thus bypasses public_settable_search_args as well. #30 would safe_search_args safely placed between the raw user request and the custom filtering.

So, we could have it insist on calling safe_search_args even if you have a custom filter. I'm not sure if that's better or worse -- if you're customizing, maybe you want to do your own thing there, and not be forced into the safe_search_args architecture? If you do want to use it currently, you do have to make sure to call safe_search_args yourself in your custom code -- is that too far 'out of it's way'?

That said, some final points:

1. I'm afraid I really don't like Protocol specific args engine extension #35, I think this should be at the controller-level. Protocol specific args engine extension #35 is a complication of the Engine architecture that I think is going to be hard to keep straight, I don't like it. So I'd rather get this approach filling the use case.

2. The SearchController is really pretty basic, and is meant to get you started with basic use cases. On the one hand, I think maybe this is a basic use-case it should support. On the other hand, the intention is if you need something really customized -- just don't use the SearchController, write your own controller (that either sub-classes the existing one or not), and make it do exactly what you want. This is meant to be quite straightforward -- you should be able to easily swap in your own controller, and have it just work with existing stuff including ajax helpers. Let me know if you want a write-up example.

3. I still don't totally understand your use case. Would you be willing to share details of it? I think this might help clarify how common a use case or class of use cases we have here that should maybe be supported out of the box, and also help me understand if a given solution supports it well or not.

[jrochkind]

If you don't want to share details of your use case publically, you can email me instead. But I'd really love to understand why and what you're actually needing to do here, and I really don't right now!

[magibney]

This all looks great to me. I wasn't intending to press on #35, and I'm fine with it being closed; I was just thinking out loud about some of its potential benefits. Thanks for pointing out the possibility of packaging the params_logic in the conf. I wasn't aware that that was a possibility, and something along the lines of what you suggest seems to answer my main (though minor, at that) concern.

re: safe_search_args, no, not out of of the way to call from filter code, and I agree that what's gained in full flexibility outweighs the potential downside of enabling one to accidentally bypass some default safety checks.

For completeness' sake, I will email you about use case details, but I'm pretty certain (especially with the approach you suggest in this comment) that #36 will work more than adequately for my use case.

Thanks for all the consideration you've put into this!

[magibney]

Had to change @engine_args to @engine_params in lines 80-81, to match the declared attr_writer on line 51, but with that small change, this works great!

Example code as it worked for me:

BentoSearch.register_engine('summon') do |conf|
  ...
  conf.check_auth = lambda do |param_hash, request|
    auth_header = request.headers['authorization']
    if verify_auth?(auth_header)
      param_hash[:auth] = true
    else
      param_hash[:auth] = false
    end
    param_hash
  end
end

BentoSearch::SearchController.before_action do |controller|
  check_auth = controller.engine.configuration.check_auth
  if check_auth != nil
    engine_params = controller.safe_search_args(controller.engine, controller.params)
    engine_params = check_auth.call(engine_params, controller.request)
    controller.engine_params = engine_params
  end
end

[jrochkind]

Thanks, fixed that bug. Just gotta figure out how to write a test for this.