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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ancorcruz
Copy link
Contributor

Fixes #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.

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:

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

however, the valid XML is:

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

or

```XML
<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:

```XML
<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:

```ruby
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
Copy link
Contributor Author

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

@sshaw
Copy link
Owner

sshaw commented Jul 20, 2018

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants