Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mongo Mapper master not compatible #268

Open
plusplus opened this issue Jul 10, 2013 · 3 comments
Open

Mongo Mapper master not compatible #268

plusplus opened this issue Jul 10, 2013 · 3 comments

Comments

@plusplus
Copy link

The master branch of mongo mapper isn't happy with the state_machine latest gem. I'm just putting this here to remind me to look at it later. I've raised an issue there as well.

This repo demonstrates the issue: mongomapper/mongomapper#531

The mongo mapper issue is here: mongomapper/mongomapper#531

@cheald
Copy link

cheald commented Jul 10, 2013

Hi there, I'm responsible for the changes in MM that have surfaced this. I think that strictly speaking, this is a bug in state_machine that was exposed by recent changes to MM's internals.

In lib/state_machine/integrations/mongo_mapper.rb:

def define_state_initializer
  define_helper :instance, <<-end_eval, __FILE__, __LINE__ + 1
    def initialize(*args)
      self.class.state_machines.initialize_states(self, :static => :force) { super }
    end
  end_eval
end

state_machine is doing its initialization before MongoMapper's #initialize gets to run. One of the things we're doing to improve performance is to do initial setup on initialize, and then relying on that to be there rather than checking that every time we try to write a key. Previously, this worked because MM would automagically define keys on-the-fly and didn't rely on instance state to do so. This is great, except it's also really slow.

I'm going to see if I can get this working in MM in a not-terribly-slow fashion, but if it'd be possible to call super and then call initialize_states, that would be ideal.

cheald added a commit to mongomapper/mongomapper that referenced this issue Jul 10, 2013
…acked #initialize before we've gotten to run our own #initialize. Closes #531. Closes pluginaweek/state_machine#268.
@plusplus
Copy link
Author

FWIW: I can work around this by converting:

state_machine initial: :shopping  do
  # ...

to:

def initialize(*args)
  super
  self.class.state_machines.initialize_states(self, static: :force)
end

state_machine initial: :shopping, initialize: false do
  # ...

Which would suit me fine if it avoids slow downs in MM and/or nasty code and doesn't do BAD things to my business logic (to the tests!)

@cheald
Copy link

cheald commented Jul 10, 2013

I think that would be more correct since the state machine setup relies on
MM's internals, so it's only fair to let it do its setup first.

That being said, I've fixed the issue in a way that shouldn't be too awful
performance wise, so it's really just an abstract exercise at this point.
Master should be working without any more changes necessary.
On Jul 9, 2013 6:50 PM, "Julian Russell" [email protected] wrote:

FWIW: I can work around this by converting:

state_machine initial: :shopping do

...

to:

def initialize(*args)
super
self.class.state_machines.initialize_states(self, static: :force)
end

state_machine initial: :shopping, initialize: false do

...

Which would suit me fine if it avoids slow downs in MM and/or nasty code
and doesn't do BAD things to my business logic (to the tests!)


Reply to this email directly or view it on GitHubhttps://github.com//issues/268#issuecomment-20717205
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants