
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) 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) 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) 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) 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) 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.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »