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
adding support for XML parsing and shale type mapping #48
base: main
Are you sure you want to change the base?
Conversation
@ronaldtse I've tried to make the structure for parsing XML as close to other structures as possible. is this correct or do we need to change something here? |
@HassanAkbar actually this is not quite what I wanted. We need a way to allow users to "access XML element and attributes". So the user should provide an XML schema, and also an XML document, like this, and allow the user to access elements: |
@ronaldtse I don't fully understand what you mean, But I will look into it and get back to you with questions related to this after some research. |
@ronaldtse I've looked at https://www.shalerb.org/#compiling-json-and-xml-schema, and figured how From what I understand the flow is something like,
|
The user will provide both:
Steps:
An XSD can be standalone (if it's complete), or it may include other XSDs via
We want to support both, but let's finish XML first and do JSON next?
Yes. |
hash[mapping.name] = content.strip | ||
end | ||
else | ||
if object.class.attributes[mapping.attribute].collection? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert if nested inside else to elsif.
serialize_to_hash(document) | ||
end | ||
|
||
def serialize_to_hash(object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assignment Branch Condition size for serialize_to_hash is too high. [<11, 45, 16> 49.01/15]
Cyclomatic complexity for serialize_to_hash is too high. [13/6]
Method has too many lines. [25/10]
Perceived complexity for serialize_to_hash is too high. [16/7]
element | ||
end | ||
|
||
def get_content_value(content_mapping, element, only, except, delegates, instance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid parameter lists longer than 5 parameters. [6/5]
Line is too long. [90/80]
mapper.send(content_mapping.method_to, instance, element, | ||
doc, context) | ||
else | ||
mapper.send(content_mapping.method_to, instance, element, doc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [82/80]
# @return [::REXML::Document, ::Nokogiri::Document, ::Ox::Document] | ||
# | ||
# @api public | ||
def as_xml( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid parameter lists longer than 5 parameters. [8/5]
def load_schema(schema) | ||
result = Shale::Schema.from_xml([schema]) | ||
|
||
result.values.each do |klass| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use each_value instead of values.each.
klass = klass.gsub(/^require.*?\n/, "") | ||
klass = klass.gsub(/< Shale::Mapper/, "< Lutaml::Xml::Mapper") | ||
|
||
eval(klass, TOPLEVEL_BINDING) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of eval is a serious security risk.
def self.load_schema(schema, root_schema) | ||
result = Shale::Schema.from_xml([schema]) | ||
|
||
result.values.each do |klass| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use each_value instead of values.each.
"Address" => { | ||
"City" => "London", | ||
"ZIP" => "E1 6AN", | ||
"content" => ["Oxford Street"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a comma after the last item of a multiline hash.
|
||
RSpec.describe Lutaml::Xml::LutamlPath::DocumentWrapper do | ||
describe ".parse" do | ||
subject(:lutaml_path) { described_class.new(Lutaml::Xml::Parsers::Xml.parse(xml_file_path)) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [97/80]
if mapping == "content" | ||
attribute_name = object.class.xml_mapping.content.attribute.to_s | ||
hash[attribute_name] ||= [] | ||
hash[attribute_name] << content.strip unless content.strip&.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid calling empty? with the safe navigation operator in conditionals.
serialize_to_hash(document) | ||
end | ||
|
||
def serialize_to_hash(object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assignment Branch Condition size for serialize_to_hash is too high. [<11, 45, 15> 48.69/15]
Cyclomatic complexity for serialize_to_hash is too high. [12/6]
Method has too many lines. [25/10]
Perceived complexity for serialize_to_hash is too high. [15/7]
@ronaldtse I have updated this PR. Is this what you were looking for ? |
result.values.each do |klass| | ||
# Temporary solution will update in the parser | ||
klass = klass.gsub(/^require.*?\n/, "") | ||
klass = klass.gsub(/< Shale::Mapper/, "< Lutaml::Xml::Mapper") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to fork the gem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronaldtse Sure, I'll fork this and open a PR in that repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronaldtse Should I fork the gem and move everything there or only update this part of the code in the forked gem and keep everything else here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronaldtse Forked the gem at lutaml/shale -> https://github.com/lutaml/shale
Adding support for XML parsing and Shale-type mapping.
Fixes #47