ruby-rdf / rdf-rdfa

Ruby RDFa reader/writer for RDF.rb.
http://ruby-rdf.github.com/rdf-rdfa
The Unlicense
35 stars 11 forks source link

Class methods are added after using Reader.each_statement #9

Closed slamorsi closed 12 years ago

slamorsi commented 12 years ago

Reference - http://stackoverflow.com/questions/7579365/routing-error-after-call-to-rdfrdfareader

In a Rails 3.1, Ruby 1.9.2 app, where ever I use the following code:

RDF::RDFa::Reader.open("http://www.tripadvisor.com/Hotel_Review-g186525-d280839-Reviews-Gerald_s_Place-Edinburgh_Scotland.html") do |r|
      Rails.logger.debug('de nada')
      r.each_statement do |statement|
        #Rails.logger.debug('ok')
        next
      end
    end

Every class being touched gets these new methods as a difference between the class's .method call before and after the code block:

[:attr, :attr_reader, :attr_writer, :attr_accessor, :remove_method, :undef_method, :all_instance_methods, :ancestor?, :attr_setter, :basename, :can, :class?, :class_def, :module_def, :instance_method_defined?, :singleton_method_defined?, :module_method_defined?, :class_method_defined?, :home, :homename, :modname, :housing, :instance_method!, :wrap_method, :wrap, :nodef, :remove, :integrate, :is?, :is, :methodize, :+, :-, :*, :pathize, :set, :spacename, :to_obj]

So if I'm in a Rails controller 'BaseItem' and the RDFa code is there, the difference between BaseItem.methods (and super classes of BaseItem) before and after RDFa code is the above mentioned methods.

This causes Rails routing to break the next time any resource is requested with an error like the following:

ActionController::RoutingError (private method `redefine_method' called for #<Class:0x000001058527c0>):
  app/models/base_item.rb:3:in `<class:BaseItem>'
  app/models/base_item.rb:2:in `<top (required)>'
  app/controllers/base_items_controller.rb:3:in `<top (required)>'
gkellogg commented 12 years ago

Can you please provide a reproducible example? Running this code in IRB doesn't result in other objects being modified.

Also, is it specific to RDFa or for any other RDF serialization (N-Triples would be good to try) Try http://rdf.rubyforge.org/doap.nt.

Does it only happen in Rails, or will it happen in other without?

If you can provide a gist with something I can run, I'll be able to look at it sooner.

slamorsi commented 12 years ago

At first I thought it was only RDFa but going through N-Triples statements also causes the issue. I've only been trying this in Rails but I'll try to reproduce - or make a sample rails app. Thanks, I'll put something up soon.

slamorsi commented 12 years ago

I haven't been able to reproduce this outside of Rails but I've narrowed it down to an issue with ActiveRecord. I've set up a sample app at https://github.com/slamorsi/rdfTest

There are two models, Test and TestChild. The error is reproducible if Test and TestChild are related - right now Test has a has_many relationship with TestChild. The root of the app goes to test#index which has sample RDF/RDFa code. If you load the page once then refresh, you'll see the 'redefine method called...' error. If there's no relationship between the models or if no RDF/RDFa statements.each code is executed, everything works fine. I don't know what could possibly cause this.

gkellogg commented 12 years ago

Thanks very much for taking the time to create a test case. I suspect that something in RDF.rb is infecting Class, and ActiveRecord is colliding with this.

slamorsi commented 12 years ago

Of course. Hope you were able to see the error. I've rummaged through the code and couldn't find much that would cause issues - do you have a suggestion as to where I might look closer in RDF?

gkellogg commented 12 years ago

It's on my queue to look at, and I'm sure it's someplace in RDF.rb, but I'm a bit over booked at the present. I hope to get to it in the next couple of weeks.

If the matter's urgent, and there's some commercial demand for it to be addressed sooner, I'd be happy to consult with you directly.

If you'd like to look at it yourself, I'd probably start by reducing the test case to the barest minimum and instrument a local copy of RDF.rb to identify where the problem is introduced.

slamorsi commented 12 years ago

No rush right now - thanks. I'll keep digging myself when I can.

slamorsi commented 12 years ago

Not a real solution but a clue: Commenting out line 1088 (a call to super) in active record (3.1.1) lib/active_record/base.rb fixes the issue. The method_id before the error occurs is 'redefine_method' whether or not a call to the rdf enumarator .each but an exception is only thrown if .each is used before reloading the page.

def method_missing(method_id, *arguments, &block)
          if match = DynamicFinderMatch.match(method_id)
            attribute_names = match.attribute_names
            super unless all_attributes_exists?(attribute_names)
            if !arguments.first.is_a?(Hash) && arguments.size < attribute_names.size
              ActiveSupport::Deprecation.warn(<<-eowarn)
Calling dynamic finder with less number of arguments than the number of attributes in method name is deprecated and will raise an ArguementError in the next version of Rails. Please passing `nil' to the argument you want it to be nil.
                eowarn
            end
            if match.finder?
              options = arguments.extract_options!
              relation = options.any? ? scoped(options) : scoped
              relation.send :find_by_attributes, match, attribute_names, *arguments
            elsif match.instantiator?
              scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block
            end
          elsif match = DynamicScopeMatch.match(method_id)
            attribute_names = match.attribute_names
            super unless all_attributes_exists?(attribute_names)
            if arguments.size < attribute_names.size
              ActiveSupport::Deprecation.warn(
                "Calling dynamic scope with less number of arguments than the number of attributes in " \
                "method name is deprecated and will raise an ArguementError in the next version of Rails. " \
                "Please passing `nil' to the argument you want it to be nil."
              )
            end
            if match.scope?
              self.class_eval <<-METHOD, __FILE__, __LINE__ + 1
                def self.#{method_id}(*args)                                    # def self.scoped_by_user_name_and_password(*args)
                  attributes = Hash[[:#{attribute_names.join(',:')}].zip(args)] #   attributes = Hash[[:user_name, :password].zip(args)]
                                                                                #
                  scoped(:conditions => attributes)                             #   scoped(:conditions => attributes)
                end                                                             # end
              METHOD
              send(method_id, *arguments)
            end
          #else
          #  super
          end
        end
gkellogg commented 12 years ago

Thanks for following up on this, I've been swamped with a lot of other work, so haven't had a chance to track it down myself. RDF.rb does some method meta-programming as well, and this is no-doubt the source of the interference.

RDF::Reader.each enumerates readers, and RDF::Reader#each is an alias for #each_statement. This is defined in RDF::Enumerable to invoke #each in the containing class. There may be some confusion going on here.

Gregg

On Nov 18, 2011, at 10:39 AM, slamorsi wrote:

Not a real solution but a clue: Commenting out line 1088 (a call to super) in active record (3.1.1) lib/active_record/base.rb fixes the issue. The method_id before the error occurs is 'redefine_method' whether or not a call to the rdf enumarator .each but an exception is only thrown if .each is used before reloading the page.

def method_missing(method_id, *arguments, &block)
         if match = DynamicFinderMatch.match(method_id)
           attribute_names = match.attribute_names
           super unless all_attributes_exists?(attribute_names)
           if !arguments.first.is_a?(Hash) && arguments.size < attribute_names.size
             ActiveSupport::Deprecation.warn(<<-eowarn)
Calling dynamic finder with less number of arguments than the number of attributes in method name is deprecated and will raise an ArguementError in the next version of Rails. Please passing `nil' to the argument you want it to be nil.
               eowarn
           end
           if match.finder?
             options = arguments.extract_options!
             relation = options.any? ? scoped(options) : scoped
             relation.send :find_by_attributes, match, attribute_names, *arguments
           elsif match.instantiator?
             scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block
           end
         elsif match = DynamicScopeMatch.match(method_id)
           attribute_names = match.attribute_names
           super unless all_attributes_exists?(attribute_names)
           if arguments.size < attribute_names.size
             ActiveSupport::Deprecation.warn(
               "Calling dynamic scope with less number of arguments than the number of attributes in " \
               "method name is deprecated and will raise an ArguementError in the next version of Rails. " \
               "Please passing `nil' to the argument you want it to be nil."
             )
           end
           if match.scope?
             self.class_eval <<-METHOD, __FILE__, __LINE__ + 1
               def self.#{method_id}(*args)                                    # def self.scoped_by_user_name_and_password(*args)
                 attributes = Hash[[:#{attribute_names.join(',:')}].zip(args)] #   attributes = Hash[[:user_name, :password].zip(args)]
                                                                               #
                 scoped(:conditions => attributes)                             #   scoped(:conditions => attributes)
               end                                                             # end
             METHOD
             send(method_id, *arguments)
           end
         #else
         #  super
         end
       end

Reply to this email directly or view it on GitHub: https://github.com/gkellogg/rdf-rdfa/issues/9#issuecomment-2792609

slamorsi commented 12 years ago

I had some time to look into this again and managed to hunt it down to the requiring of facets in profile/x(ht)ml.rb. There's a conflict that's been noted in https://github.com/rubyworks/facets/issues/40 . Either removing the require statement in the x(ht)ml.rb files or using the solution in facet's ticket works (requiring facets before require all in application.rb for rails). I'm using the latter solution - what do you think?

gkellogg commented 12 years ago

Wow! Thanks for tracking this down. I think I can do without facets, so I'll just re-write. I should be able to get updates out soon.

Gregg Kellogg Sent from my iPhone

On Dec 7, 2011, at 2:20 PM, "slamorsi" reply@reply.github.com wrote:

I had some time to look into this again and managed to hunt it down to the requiring of facets in profile/x(ht)ml.rb. There's a conflict that's been noted in https://github.com/rubyworks/facets/issues/40 . Either removing the require statement in the x(ht)ml.rb files or using the solution in facet's ticket works (requiring facets before require all in application.rb for rails). I'm using solution #2 - what do you think?


Reply to this email directly or view it on GitHub: https://github.com/gkellogg/rdf-rdfa/issues/9#issuecomment-3054503

slamorsi commented 12 years ago

Great! Thanks for a great gem :)

gkellogg commented 12 years ago

I updated the repo with an update that removes the use of the facet gem (I was only using it for alias_method_chain). Could you add this to your Gemfile, or however you're testing, and let me know if it fixes the problem?

slamorsi commented 12 years ago

Looks good now. Thanks!

gkellogg commented 12 years ago

This is fixed in d751e556ec0036e9ede3b8978b32874d2d922068 and 7caef3093cf8742211efa10efb42aa19b360dc75