Fix for webxmlopenstruct (on JRuby 9.3)#521
Conversation
I believe this doesn't change any behaviour on either version (although weird behaviour may exist when users set keys with string values explicitly).
On JRuby 9.3 new_ostruct_member! checks if it already knows a new key by checking the `@table` instance variable, before checking whether a method is already defined (I assume because it's faster in the common case), the way `@table` is handled with auto-vivifying values made this not work properly for nested assignments. E.g. `config.webxml.rails.env = 'production'` would auto-vivify config.webxml.rails without defining a singleton method. On JRuby 9.3 we could scrap method_missing, it has the right behaviour (i.e. the workaround for jruby#366 added in 9245f4c is no longer required). Unfortunately it's still required on JRuby 9.2
|
Looking deeper in the open struct code the current does it work by removing |
|
The answer is that this does work for 9.3, but not for 9.2, because the version of ostruct used in 9.2.21 will check that a key is present in the table in it's method_missing implementation with It's in particular the changes in jruby/jruby@d6f4cc1 , which seem to be included in every version from 9.1 up to (but not including) 9.3, so we can't drop the implementation of method_missing. For the moment I went with the approach that required the least changes to the code., but the right approach probably is to not use OpenStruct. OpenStruct doesn't seem to fit the purpose of the class entirely (not allowing for nested assignments) and from Jruby 9.3 onwards I expect the ostruct code to be updated more frequently as it's now included through pom.xml instead of through direct inclusion of the code. |
|
@joerixaop Thanks for the carefully added context to the PR description and for the fix & analysis. (Thanks, @deivid-rodriguez for review!) |
5 participants
On JRuby 9.3 the fix from #513 does not work, because the
modifiablemethod has disappeared.On JRuby 9.2 replacing
modifiable[new_ostruct_member!(mname)]=withself[mname]=is not a problem as the existing implementation of#[]=takes care of both checking that it's still modifiable and callingnew_ostruct_member!.