#1240 ✓resolved
Shane Hanna

Adapter as query factory.

Reported by Shane Hanna | April 12th, 2010 @ 02:25 AM

I think I've managed to route all query construction inside DM through the adapter (as a query factory). Specifically this helps out adapters like my dm-sphinx-adapter which have multiple query 'modes' and other query options that I'd like to handle with a subclassed query object.

http://github.com/shanna/dm-core/compare/query_factory

Though the change is simple it requires that each repository have an adapter instance setup before any queries are constructed. This to me sounds like a reasonable requirement though I'm stuck on failing specs and would like a nudge in the right direction please. I /think/ the specs are failing because they are done on repositories that don't have an adapter setup. I'm not sure how wide spread this is because RSpec falls down pretty early on thanks to what looks like an unrelated bug (calling map{} on a nil object).

Cheers,
Shane.

Comments and changes to this ticket

  • Shane Hanna

    Shane Hanna April 28th, 2010 @ 12:35 AM

    I've updated the specs adding supported_by blocks around the specs that now require an adapter be set up. I don't know how specific you get with the supported_by blocking normally. I can find tune the calls to supported_by down to specific tests if need be.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 3rd, 2010 @ 03:09 PM

    • State changed from “new” to “accepted”
    • Assigned user set to “Dan Kubb (dkubb)”

    I think the code you have is fine. I try not to use supported_by except where the underlying code requires an adapter instance.

    I did rebase what you have against the master branch, and I noticed a few spec failures related to SEL. Did you see the same thing on your end?

  • Shane Hanna

    Shane Hanna May 4th, 2010 @ 08:02 PM

    Bah you are right. I'll try and figure out the SEL stuff. I've tracked it down to but it's a bit of a mystery so far since the only thing that changed is the call stack is a little longer. The code breaking the SEL tests is:

           #
       # @api private
       def query
-        Query.new(repository, self, current_scope).freeze
+        repository.new_query(self, current_scope).freeze
       end

    Simply calling repository.new_query in: lib/dm-core/model/scope.rb #query in void context (above the old Query.new) still triggers the SEL test failures which makes me think I've broken some black magic somewhere.

    Shane.

  • Shane Hanna

    Shane Hanna May 6th, 2010 @ 09:07 PM

    Branch updated so the tests pass but it isn't so pretty.

    The issue was the Query used to create the @articles collection sets up the adapter now because Repository#new_query calls adapter.new_query(...) (AbstractAdapter#new_query) triggering the ||= in Repository#adapter which creates the adapter instance sooner. Phew, I think thats right.

    Shane.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 10th, 2010 @ 12:04 PM

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 10th, 2010 @ 12:05 PM

    • State changed from “accepted” to “resolved”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Tags

Referenced by

Pages