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

Sending a single element in an array using XML format fails with the request being invalid #1662

Open
ramkumar-kr opened this issue Jul 18, 2017 · 4 comments
Labels

Comments

@ramkumar-kr
Copy link
Contributor

Hi,

I have an API which accepts requests using XML. It accepts an array of products. However for a case where there is only one product in the array, validation errors occur.
Sample request -
curl -X POST \ http://localhost:3000/v1/products \ -H 'cache-control: no-cache' \ -H 'content-type: application/xml' \ -d '<?xml version="1.0" encoding="UTF-8" ?> <admin> <product> <id>1</id> <price>100</price> </product> </admin>'

This is because when there is a single element in an array, the product will be parsed to a hash instead of an array of hash objects.
Expected - { admin: { products: [{ id: "1", price: "100" }] } }
Actual - { admin: { products: { id: "1", price: "100" } } }

the parameters are defined as

params do
  requires :admin, type: Hash do
    requires :products, type: Array do
      requires :id, type: String
      requires :price, type: String
    end
  end
end

Currently to overcome this, I run the following before_validation block

before_validation do
  params[:admin][:products] = [params[:admin][:products]] if params[:admin][:products].is_a?(Hash)
end

I was wondering since grape knows the request type (Array or Hash), can it convert the hash to an array before validating (similar to the before_validation block).

Notes:
Using Grape version : 1.0.0
Sample code - https://github.com/ramkumar-kr/grape-xml-error

@dblock
Copy link
Member

dblock commented Jul 18, 2017

In your example you send a single product, it cannot guess that it's an array, but maybe you're right and it should infer and coerce it it because of what's in the params block. I say write a spec for this next and lets see if we can implement/fix this.

@dblock dblock added the bug? label Jul 18, 2017
ramkumar-kr added a commit to ramkumar-kr/grape that referenced this issue Aug 10, 2017
@ramkumar-kr
Copy link
Contributor Author

Hi,
Sorry for the late reply.
I have created a Spec and have sent a pull request for it.

@Smeedy
Copy link

Smeedy commented Oct 20, 2017

We also bumped into this one the other day. This is due to the XML parsing of the arrays using the default ActiveSupport::XmlMini. We noticed that Nokogiri handles the XML arrays correctly using some test XML on the irb.

So the less intrusive solution is using MultiXML and Nokogiri as the parser in the project and not rewrite the params in the before hook.

So the use of an initializer like

require 'multi_xml'

# Force all the XML parsing to be handled by Nokogiri
MultiXml.parser = :nokogiri
MultiXml.parser = MultiXml::Parsers::Nokogiri # Same as above

will set the definition. The use of MultiXml will be picked up by the Grape framework and Bob's your uncle.

See: https://github.com/ruby-grape/grape#json-and-xml-processors

@dblock
Copy link
Member

dblock commented Oct 20, 2017

@Smeedy Do you think you can turn the spec linked above into a passing one and add some documentation and PR the thing?

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

3 participants