#1027 ✓resolved
Ashley Moran

merb_datamapper opens repository block too low down

Reported by Ashley Moran | September 8th, 2009 @ 07:00 AM | in 0.10.1

Insidious little Merb/DataMapper/ExtLib bug here. Say you have this Merb controller:

class MyController < Application
  before :load_resource

  def my_action
    @resource.name = "My Name"
    @resource.save
  end

  private

  def load_resource
    @resource = MyModel.get(1)
  end
end

If you make a request that runs the #my_action action, the code that runs in the before filter is not run in the repository block provided by merb_datamapper. ie the save to @resource happens in an identity map, was loaded out of one.

This is because the ExtLib hooks are called separately before the #_call_action method. To include before/after filters in the repository block, you need to wrap #_dispatch instead:

class Application < Merb::Controller
  override! :_dispatch
  def _dispatch(*)
    DataMapper.repository do |r|
      Merb.logger.debug "In repository block #{r.name}"
      super
    end
  end
end

Note that #_dispatch is not marked as an overridable method in the Merb source, although it is part of the plugin API.

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) September 8th, 2009 @ 11:41 AM

    • State changed from “new” to “confirmed”

    I just spoke with Yehuda about this ticket, and he said that methods prefixed with "_" should not be overridden in Merb plugins.

    He did say a better choice for this might be to override process_action though. Can you test it out?

    If for some reason process_action doesn't work, I'd suggest opening a ticket in the Merb tracker and referencing it here. When it's resolved, we can update merb_datamapper to use the approved approach.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) September 8th, 2009 @ 11:36 PM

    @ashley: I'm unfamiliar with merb's internals atm, but that was what Yehuda said. I'm sure there's something equivalent, just with a different name... he may be a bit rusty on merb internals atm :)

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) September 18th, 2009 @ 05:20 AM

    • State changed from “confirmed” to “accepted”
    • Tag set to “merb_datamapper”
    • Assigned user set to “Martin Gamsjaeger (snusnu)”

    I will look into this. I'll push it to my fork of merb so that it can be merged with merb master.

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) September 24th, 2009 @ 10:50 AM

    • Milestone set to “0.10.1”

    Guys,

    I just pushed fixes for this issue to both 1.0.x and master branches of my merb fork. merb_datamapper now wraps the request at a rack level. I'm confident that the fixes will be merged into both official merb branches, so that merb-1.0.13 as well as merb-1.1 won't have this issue anymore.

    Have a look at it at http://github.com/snusnu/merb/commit/67a225bf3482933c1746e66d0818d8...

    cheers
    snusnu

  • Martin Gamsjaeger (snusnu)

    Martin Gamsjaeger (snusnu) September 24th, 2009 @ 06:19 PM

    • State changed from “accepted” to “resolved”

    Pushed to wycats/merb/1.0.x and wycats/merb/master

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

Pages