sshaw / ddex

DDEX metadata serialization for Ruby
https://metadatagui.com
52 stars 41 forks source link

Fix output XML tag order and match ERN XSD definition #17

Open ancorcruz opened 5 years ago

ancorcruz commented 5 years ago

Fixes https://github.com/sshaw/ddex/issues/16

ERN v3.2 XSD defines XML sequences for some elements (enforces elements order).

DDEX.write generates XML with the elements in the wrong order, for example:

<DisplayArtist>
  <ArtistRole>MainArtist</ArtistRole>
  <PartyName>
    <FullName>Bruno Mars</FullName>
  </PartyName>
</DisplayArtist>

however, the valid XML is:

<DisplayArtist>
  <PartyName>
    <FullName>Bruno Mars</FullName>
  </PartyName>
  <ArtistRole>MainArtist</ArtistRole>
</DisplayArtist>

or

<ImageDetailsByTerritory>
  <TechnicalImageDetails>
    <TechnicalResourceDetailsReference>T12345</TechnicalResourceDetailsReference>
    <ImageCodecType>JPEG</ImageCodecType>
    <ImageHeight>800</ImageHeight>
    <ImageWidth>800</ImageWidth>
    <File>
      <FileName>ABCDEF_T12345.jpg</FileName>
      <FilePath>resources/</FilePath>
      <HashSum>
        <HashSum>aBcDeFgH12345XyZ</HashSum>
        <HashSumAlgorithmType>SHA1</HashSumAlgorithmType>
      </HashSum>
    </File>
  </TechnicalImageDetails>
  <TerritoryCode>Worldwide</TerritoryCode>
</ImageDetailsByTerritory>

while the valid XML is:

<ImageDetailsByTerritory>
  <TerritoryCode>Worldwide</TerritoryCode>
  <TechnicalImageDetails>
    <TechnicalResourceDetailsReference>T12345</TechnicalResourceDetailsReference>
    <ImageCodecType>JPEG</ImageCodecType>
    <ImageHeight>800</ImageHeight>
    <ImageWidth>800</ImageWidth>
    <File>
      <FileName>ABCDEF_T12345.jpg</FileName>
      <FilePath>resources/</FilePath>
      <HashSum>
        <HashSum>aBcDeFgH12345XyZ</HashSum>
        <HashSumAlgorithmType>SHA1</HashSumAlgorithmType>
      </HashSum>
    </File>
  </TechnicalImageDetails>
</ImageDetailsByTerritory>

The root of the issue is in classes that include ROXML and inherit from another class that also includes ROXML, both defining xml_accessors.

ROXML appends superclass roxml_attrs to the class in use ones, however, in order to build valid DDEX XML messages we need to list the superclass attributes before the ones on the child class (as far as I have been able to check, be aware, the specification is bloated).

Please, check the classes for the previous XML examples:

class DDEX::V20100712::DDEXC::DisplayArtist < DDEX::V20100712::DDEXC::PartyDescriptor
  ...
  xml_accessor :artist_roles, :as => [DDEX::V20100712::DDEXC::ArtistRole], :from => "ArtistRole", :required => false
  ...
end

class DDEX::V20100712::DDEXC::PartyDescriptor < Element
  ...
  xml_accessor :party_names, :as => [DDEX::V20100712::DDEXC::PartyName], :from => "PartyName", :required => false
  ...
end

class DDEX::ERN::V32::ImageDetailsByTerritory < DDEX::V20100712::DDEXC::ImageDetailsByTerritory
  ...
  xml_accessor :technical_image_details, :as => [DDEX::ERN::V32::TechnicalImageDetails], :from => "TechnicalImageDetails", :required => false
  ...
end

class DDEX::V20100712::DDEXC::ImageDetailsByTerritory < Element
  ...
  xml_accessor :excluded_territory_codes, :as => [], :from => "ExcludedTerritoryCode", :required => false
  xml_accessor :territory_codes, :as => [], :from => "TerritoryCode", :required => false
  ...
end

As a solution to this issue, DDEX::Element overwrites ROXML module's class method roxml_attrs to prepend the superclass attributes instead.

ancorcruz commented 5 years ago

I'm trying to add some specs that probe the issue and also port the patch (tested) to ROXML gem.

sshaw commented 5 years ago

I'm trying to add some specs that probe the issue and also port the patch (tested) to ROXML gem.

Great thanks. Would be a good idea to add a spec that validates output against schema.

ancorcruz commented 5 years ago

that is a really good idea.

Also, I'm going to try and replace test_xml gem with equivalent-xml as it has support to ensure the order of the entities in the document.