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

XmlObjectParser skips validation for sources without namespaces #9573 #10010

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vbradnitski
Copy link
Contributor

No description provided.

@vbradnitski vbradnitski force-pushed the issue-9573 branch 2 times, most recently from c768f32 to 395ca20 Compare January 30, 2023 16:30
@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Base: 83.99% // Head: 84.00% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (cb564a2) compared to base (e330e72).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head cb564a2 differs from pull request most recent head fe52d10. Consider uploading reports for the commit fe52d10 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #10010   +/-   ##
=========================================
  Coverage     83.99%   84.00%           
- Complexity    19290    19296    +6     
=========================================
  Files          2580     2580           
  Lines         67749    67769   +20     
  Branches       5464     5468    +4     
=========================================
+ Hits          56905    56927   +22     
+ Misses         8137     8134    -3     
- Partials       2707     2708    +1     
Impacted Files Coverage Δ
.../java/com/enonic/xp/xml/parser/XmlModelParser.java 100.00% <100.00%> (ø)
...java/com/enonic/xp/xml/parser/XmlObjectParser.java 87.71% <100.00%> (+7.16%) ⬆️
...nonic/xp/xml/parser/XmlRelationshipTypeParser.java 88.23% <100.00%> (+0.73%) ⬆️
.../enonic/xp/core/impl/export/xml/XmlNodeParser.java 93.70% <100.00%> (+1.24%) ⬆️
...java/com/enonic/xp/xml/schema/SchemaValidator.java 85.71% <0.00%> (-14.29%) ⬇️
...enonic/xp/xml/parser/XmlStyleDescriptorParser.java 89.74% <0.00%> (-5.13%) ⬇️
...xp/repo/impl/branch/storage/BranchServiceImpl.java 94.49% <0.00%> (+1.83%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

private String systemId;

private CharSource source;

public XmlObjectParser( final String namespace )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add default ctor that, if used, does not force any namespace. namespace = null

source.setSystemId( this.systemId );

final DOMResult result = VALIDATOR.validate( source );
return (Document) result.getNode();
}

private Document processDocument( final Document doc )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe better call it forceNamespace

Copy link
Contributor

@rymsha rymsha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to cancel this change

  • apps may suddenly stop working due to extra validation
  • we may want to focus on json schema for descriptors

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.

None yet

2 participants