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

only :order! keys are converted - error with Gyoku #461

Closed
guzart opened this issue Jun 10, 2013 · 11 comments
Closed

only :order! keys are converted - error with Gyoku #461

guzart opened this issue Jun 10, 2013 · 11 comments
Assignees
Milestone

Comments

@guzart
Copy link

guzart commented Jun 10, 2013

Unfortunately am stuck using 1.8.7

Settings:

convert_request_keys_to :camelelcase
element_form_default :qualified

Savon is sending Gyoku a hash where the :order! keys are converted, but the message keys are not. Which results in Gyoku raising the following error

Missing elements in :order! [:user_name, :email_count]
gyoku-1.0.0/lib/gyoku/hash.rb:76:in `order'
hashable = [:email_count, :user_name]
orderable = ['userName', 'emailCount']

Seems like the breaking point is in qualified_message.rb:27, where

@used_namespaces[newpath] => nil
and it only adds the namespaces (converted keys) to the :order! array

Thanks

@guzart
Copy link
Author

guzart commented Jun 10, 2013

If I comment out line 27 it works as expected, and all tests pass.

Another take is if I pass the key_converter to the qualified message constructor and use the translated key for the result hash when @used_namespaces[newpath] is nil.
guzart@adfdc04

I ran the specs and they all pass, not sure how or in what situation @used_namespaces variable is used.

@emschwar
Copy link

I think my problem is related, though I'm on 1.9.3.

I have created my client with :convert_request_keys_to => :none, but when I try to pass in :order!, the order keys are converted anyway, which makes it impossible, as far as I can tell, to order anything.

If my message hash looks like:

{  :FooBARBaz => "quux",
   :Pagination => { :Page => 1, :PageSize => 20 }
   :order! => [ :FooBARBaz, :Pagination ]
}

Then I get

Missing elements in :order! [:FooBARBaz, :Pagination]
[Gyoku::Hash] orderable: ["fooBarBaz", "pagination"]
[Gyoku::Hash] hashable: [:FooBARBaz, :Pagination]

but if I pass in

{  :FooBARBaz => "quux",
   :Pagination => { :Page => 1, :PageSize => 20 }
   :order! => [ 'fooBarBaz', 'pagination']
}

I get the same error. I'll try both workarounds and and let you know if any are successful.

@emschwar
Copy link

Commenting out line 27 succeeded, but I'm not (yet) comfortable deploying with it until I better understand what it does.

@guzart
Copy link
Author

guzart commented Jun 11, 2013

Totally, commenting code seems sketchy.
The other approach did not work? I thought it would work out better.

@emschwar
Copy link

yeah, passing the key converter to the qualified message constructor didn't work. Possibly because I'm not actually doing any conversion? I'm not sure.

@emschwar
Copy link

Yikes! I ran rake spec in savon after commenting out that line, and nothing failed. :-\

@emschwar
Copy link

A little experimentation shows that guzart/savon@adfdc04 fails because passing in @key_converter to Savon::QualifiedMessage in Savon::Message#to_s results in calling

Gyoku.xml_tag(:FooBARBaz, :key_converter => :none)

, which returns "FooBARBaz", not :FooBARBaz

@emschwar
Copy link

emschwar/savon@79731a6 works for me, but is definitely sub-optimal. I'd love some suggestions on how to improve it, and where to put some tests (I don't see a good spot anywhere, to be honest).

rubiii added a commit that referenced this issue Jun 30, 2013
@rubiii
Copy link
Contributor

rubiii commented Jun 30, 2013

hey guys,

i can see various problems in that area. the code in that message class is ridiculous.
your problems both take a different route in the code but i think i fixed them both with a simple change.

could you please check whether 4742208 fixes your problems?

@ghost ghost assigned rubiii Jun 30, 2013
@guzart
Copy link
Author

guzart commented Jul 1, 2013

it's working for me. Thanks

@emschwar
Copy link

Thanks, @rubiii, I'll check it out this afternoon.

@rubiii rubiii mentioned this issue Jul 20, 2013
27 tasks
@rubiii rubiii closed this as completed Jul 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants