lutaml / shale

Shale is a Ruby object mapper and serializer for JSON, YAML, TOML, CSV and XML. It allows you to parse JSON, YAML, TOML, CSV and XML data and convert it into Ruby data structures, as well as serialize data structures into JSON, YAML, TOML, CSV or XML.
https://shalerb.org/
MIT License
0 stars 1 forks source link

Shale cannot deal with default XML namespaces #1

Closed opoudjis closed 2 months ago

opoudjis commented 3 months ago

Shale as it stands cannot deal with default namespaces, e.g. <metanorma-collection xmlns="http://www.metanorma.org">

If I specify

xml do
      root "metanorma-collection"
      namespace "http://metanorma.org", "m"

I get <m:metanorma-collection>, and namespacing for elements mapping to strings, but not elements generated through supplied methods.

If I specify

xml do
      root "metanorma-collection"
      namespace "http://metanorma.org", ""

I get <: metanorma-collection>

If I specify

xml do
      root "metanorma-collection"
      namespace "http://metanorma.org", nil

The namespace is completely ignored.

This is screwing me over in my effort to adopt Shale in Metanorma and refactor it, which is awful enough already without me having to deal with this as well.

ronaldtse commented 3 months ago

Thanks @opoudjis for this.

ronaldtse commented 3 months ago

From @andrew2net https://github.com/metanorma/metanorma/issues/383#issuecomment-2120719515

Yes, it needs to monkey patch the add_namespace method https://github.com/kgiszczak/shale/blob/8ec3be64b96d2d9bc77ddfb6638a6b3d3587a063/lib/shale/adapter/nokogiri/document.rb#L65-L67

opoudjis commented 3 months ago

The desired behaviour is,

xml do
      root "metanorma-collection"
      namespace "http://metanorma.org", nil

(a) gives me <metanorma-collection xmlns="http://metanorma.org"> (b) works in both from_xml and to_xml. I note with dismay that Shale's tests appear to test only the latter in their rspec. The former breaks: if I introduce namespaces manually in the root, I think it no longer finds child elements.

If we are to be good corporate citizens and feed these changes back to Shale, we should be doing so for the other two XML adapters supplied as well.

opoudjis commented 3 months ago

@HassanAkbar I am being forced into some very unpleasant workarounds because I cannot do this. @ronaldtse clearly has not communicated this, but this is not a nice-to-have, it is holding up work I am doing right now.

HassanAkbar commented 3 months ago

@opoudjis I'm looking into it.

HassanAkbar commented 3 months ago

@opoudjis From what I understand, for the following input

class Address < Shale::Mapper
  attribute :city, Shale::Type::String
  attribute :street, Shale::Type::String
  attribute :zip, Shale::Type::String

  xml do
    root "address"
    namespace "https://foobar.com", "foo"

    map_element "city", to: :city
    map_element "street", to: :street
    map_element "zip", to: :zip
  end
end

address = Address.from_xml(<<~DATA)
  <address>
    <city>London</city>
    <street>Oxford Street</street>
    <zip>E1 6AN</zip>
  </address>
DATA

address.to_xml

we need to generate the following output

<foo:address xmlns:foo="https://foobar.com">
  <foo:city>London</foo:city>                               
  <foo:street>Oxford Street</foo:street>                    
  <foo:zip>E1 6AN</foo:zip>                                 
</foo:address> 

I just want to confirm what the output should be in this case? When the prefix is nil for the namespace

class Address < Shale::Mapper
  attribute :city, Shale::Type::String
  attribute :street, Shale::Type::String
  attribute :zip, Shale::Type::String

  xml do
    root "address"
    namespace "https://foobar.com", nil

    map_element "city", to: :city
    map_element "street", to: :street
    map_element "zip", to: :zip
  end
end

address = Address.from_xml(<<~DATA)
  <address>
    <city>London</city>
    <street>Oxford Street</street>
    <zip>E1 6AN</zip>
  </address>
DATA

address.to_xml
andrew2net commented 3 months ago

@HassanAkbar I think you need to fix this method:

# gems/ruby-3.2.0/gems/shale-1.1.0/lib/shale/adapter/nokogiri/document.rb
...
        def add_namespace(prefix, namespace)
          @namespaces[prefix] = namespace if prefix && namespace
        end
...

It blocks namespaces with nil prefixes. The solution could be:

...
        def add_namespace(prefix, namespace)
          @namespaces[prefix] = namespace if namespace
        end
...

I've tried to monkey-patch this and it works for me, but it needs to be tested.

HassanAkbar commented 3 months ago

@andrew2net Thank you for pointing this out, I can change this for sure.

But I still don't know what to test after changing this. What do we really want to achieve here ?

andrew2net commented 3 months ago

@HassanAkbar isn't the described desired behavior enough?

opoudjis commented 3 months ago

What we both want is:

class Address < Shale::Mapper
  attribute :city, Shale::Type::String
  attribute :street, Shale::Type::String
  attribute :zip, Shale::Type::String

  xml do
    root "address"
    namespace "https://foobar.com", nil

    map_element "city", to: :city
    map_element "street", to: :street
    map_element "zip", to: :zip
  end
end

address = Address.from_xml(<<~DATA)
  <address xmlns="https://foobar.com">
    <city>London</city>
    <street>Oxford Street</street>
    <zip>E1 6AN</zip>
  </address>
DATA

address.to_xml

We don't want to switch all our code to <foo:xxx>.

And we do want to confirm that XML both reads in and outputs with this namespacing; the testing like I said is currently only in one direction.