Skip to content

SystemStackError when importing STI model #170

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

Closed
AaronRustad opened this issue Jul 15, 2014 · 4 comments
Closed

SystemStackError when importing STI model #170

AaronRustad opened this issue Jul 15, 2014 · 4 comments
Labels

Comments

@AaronRustad
Copy link
Contributor

OK, I know that STI is frowned upon here, but my application is what it is. ;-)

I'm seeing a SystemStackError when I try to import a model participating in an STI configuration.

class User < ActiveRecord::Base
  import UserSearchable
...
end

class ApiKey < User
...
end

User.import works as expected, but when I try to import ApiKey, I get a SystemStackError.

I've tracked it down to Elasticsearch::Model::Proxy::Base, and tried some basic/ugly debugging:

        def method_missing(method_name, *arguments, &block)
          sleep(1)
          puts "caller: #{caller.first}"
          puts "class : #{self.class.inspect}"
          puts "method: #{method_name}"
          puts "args  : #{arguments}"
          puts "target: #{target.inspect}"
          puts "respon: #{target.respond_to?(method_name)}"
          puts "_________________________"

          target.respond_to?(method_name) ? target.__send__(method_name, *arguments, &block) : super
        end

Which yields the following:

2.1.2 :002 > ApiKey.import
caller: /Users/arustad/.rvm/gems/ruby-2.1.2/gems/elasticsearch-model-0.1.4/lib/elasticsearch/model.rb:112:in `import'
class : Elasticsearch::Model::Proxy::ClassMethodsProxy
method: import
args  : []
target: ApiKey(id: integer, ...)
respon: true
_________________________
caller: /Users/arustad/.rvm/gems/ruby-2.1.2/gems/elasticsearch-model-0.1.4/lib/elasticsearch/model.rb:112:in `import'
class : Elasticsearch::Model::Proxy::ClassMethodsProxy
method: import
args  : []
target: ApiKey(id: integer, ...)
respon: true
_________________________

... REPEAT...

target.respond_to?(:import) returns true, so it calls target.__send__ on ApiKey. It then of course eventually raises a SystemStackError.

Any advice would be helpful. I know @karmi doesn't like STI all that much, but I am hoping to solve this either by modifying the elasticsearch ruby source or adjusting my application in some way (hopefully not changing the STI relationship).

Thanks!
AR.

@AaronRustad
Copy link
Contributor Author

I have this working now... I created an ApiKeySeachable and added that to the model. It contains a duplicate mapping to that of UserSearchable. I suppose this is reasonable, but given that there is an STI relationship between the two, shouldn't ApiKey make use of User's defined mapping? I'm still not sure why I was seeing a SystemStackError.

@karmi
Copy link
Contributor

karmi commented Jul 16, 2014

@AaronRustad Glad to hear that :) I'm abroad this week, so will reply later more thoroughly, but the way you approach it is correct -- just create another component, which encapsulates the functionality. (See eg. the IndexManager.rb from the persistence demo.)

@karmi
Copy link
Contributor

karmi commented Jul 29, 2014

Aaron, I was thinking about this, and I guess I still have the same opinion about STI: I don't want to invest time and energy into tweaking the codebase to support it... If there's a greater demand for it, I can always bite into it.

shouldn't ApiKey make use of User's defined mapping (...)

... where you get into the whole story of traversing class ancestors to see if they have defined any Elasticsearch mapping and so on. Nothing which couldn't be done, but I simply feel like it's a wrong approach -- when you consider the linked IndexManager class, I think it's immediately obvious that this is a much more maintanable and transparent code...

I'm leaning towards closing this, if you've managed to solve it. We can always re-open the issue when the demand is there...

@AaronRustad
Copy link
Contributor Author

Yes, I'm good to close this issue.

karmi pushed a commit that referenced this issue Jun 28, 2016
This patch adds support for inheriting index_name and document_type on an opt-in basis:

    Elasticsearch::Model.inheritance_enabled = true

    class Animal < ActiveRecord::Base
      document_type 'mammal'
      index_name 'mammals'
    end

    class Dog < Animal
    end

    Animal.document_type   # 'mammal'
    Animal.index_name      # 'mammals'
    Dog.document_type      # 'mammal'
    Dog.index_name         # 'mammals'

Closes #332

Related: #28, #92, #170, #344
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants