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

Xml proposal implementation: XML node structure change #20668

Merged

Conversation

rdhananjaya
Copy link
Member

@rdhananjaya rdhananjaya commented Jan 20, 2020

Purpose

This PR adds the new XML node structure, xml parser, xml serializer as a part of revamping xml support. This is a part of #19570

Fixes #19832

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

rdhananjaya and others added 30 commits November 6, 2019 11:11
via special casing attribute addition method
Output stream does not get all characters as internal buffers of
xml stream was not flushed properly
Allow XMLFactory.createXMLElement() method to provide default ns for
the element being created.
…lerina-lang into xml-proposal-impl

� Conflicts:
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/XMLItem.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/XMLSequence.java
With BValue API improvements, a new BXml interface was introduced.
This commit fixes  merge conflicts that was created as a result of
updating to that new BXml interface related code.
to new xml node structure
… into xml-proposal-impl

� Conflicts:
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/XMLFactory.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/XMLItem.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/XMLSequence.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/XMLValue.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/api/BValueCreator.java
�	bvm/ballerina-runtime/src/main/java/org/ballerinalang/jvm/values/api/BXML.java
�	langlib/lang.xml/src/main/java/org/ballerinalang/langlib/xml/Concat.java
�	langlib/lang.xml/src/main/java/org/ballerinalang/langlib/xml/Elements.java
�	langlib/lang.xml/src/main/java/org/ballerinalang/langlib/xml/Filter.java
�	langlib/lang.xml/src/main/java/org/ballerinalang/langlib/xml/Map.java
�	stdlib/http/src/test/java/org/ballerinalang/stdlib/services/nativeimpl/request/RequestNativeFunctionNegativeTest.java
�	stdlib/http/src/test/java/org/ballerinalang/stdlib/services/nativeimpl/response/ResponseNativeFunctionNegativeTest.java
�	stdlib/io/src/main/java/org/ballerinalang/stdlib/io/nativeimpl/WriteXml.java
�	stdlib/jsonutils/src/main/java/org/ballerinalang/stdlib/jsonutils/FromXML.java
�	stdlib/xmlutils/src/main/java/org/ballerinalang/stdlib/xmlutils/JSONToXMLConverter.java
�	stdlib/xslt/src/main/java/org/ballerinalang/xslt/XsltTransformer.java
�	tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/statements/arrays/ArrayTest.java
@rdhananjaya rdhananjaya added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label Jan 20, 2020
@rdhananjaya rdhananjaya added this to the Ballerina 1.2.0 milestone Jan 20, 2020
}
}

public void write(BXML xmlValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we define throws since this a public method?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't throw any checked exceptions, in that case, do we need to do that.

// element doesn't have NS URI in it's name.
if ((qName.getNamespaceURI() == null || qName.getNamespaceURI().isEmpty())) {
for (String s : currentNSLevel) {
if (s.startsWith("xmlns")) {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we use constants for "xmlns", "", "}", etc.?

/**
* XML tree builder for Ballerina xml node structure using {@code XMLStreamReader}.
*
* @since 1.1.0
Copy link
Member

Choose a reason for hiding this comment

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

Versions need to be updated?


private void handleXMLStreamException(Exception e) {
// todo: do e.getMessage contain all the information? verify
throw new BallerinaException(e.getMessage(), e);
Copy link
Member

Choose a reason for hiding this comment

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

I guess the reason should be {ballerina/lang.xml}XxxError

xmlStreamWriter = xmlOutputFactory.createXMLStreamWriter(outputStream);
parentNSSet = new ArrayDeque<>();
} catch (XMLStreamException e) {
throw new BallerinaException(e);
Copy link
Member

Choose a reason for hiding this comment

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

Error reasons need to be fixed?


private void writeXMLComment(XMLComment xmlValue) throws XMLStreamException {
xmlStreamWriter.writeComment(xmlValue.getTextValue());

Copy link
Member

Choose a reason for hiding this comment

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

Extra line?

}

@Override
public abstract OMNode value();
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is still being used for testing (Jvm values to BVM values conversion)


@Override
public OMNode value() {
CharacterDataImpl characterData = new CharacterDataImpl();
Copy link
Member

Choose a reason for hiding this comment

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

formatting issue?

@rdhananjaya rdhananjaya merged commit 6841d30 into ballerina-platform:xml-proposal-impl Jan 28, 2020
@rdhananjaya
Copy link
Member Author

This PR was mistakenly merged by pushing to ballerina-platform instead of 'rdhananjaya`.
This PR is replaced by #20774

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants