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

Fixed empty source XML parsing #2655 #2658

Merged
merged 2 commits into from
Apr 25, 2021
Merged

Conversation

Seetaramayya
Copy link
Contributor

Added unit test to prove issue exists and afterwords fixed it

case AsyncXMLStreamReader.EVENT_INCOMPLETE =>
if (!isClosed(in)) pull(in)
else failStage(new IllegalStateException("Stream finished before event was fully parsed."))
case AsyncXMLStreamReader.EVENT_INCOMPLETE if isClosed(in) && !started => completeStage()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, I like the checks on left hand side of => (actions on the right hand side of =>). If it is not good then I'll convert it to if elseif else

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

case AsyncXMLStreamReader.EVENT_INCOMPLETE if isClosed(in) && !started => completeStage()
case AsyncXMLStreamReader.EVENT_INCOMPLETE if isClosed(in) =>
failStage(new IllegalStateException("Stream finished before event was fully parsed."))
case AsyncXMLStreamReader.EVENT_INCOMPLETE if !isClosed(in) => pull(in)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case AsyncXMLStreamReader.EVENT_INCOMPLETE if !isClosed(in) => pull(in)
case AsyncXMLStreamReader.EVENT_INCOMPLETE => pull(in)

@ennru ennru added this to the 3.0.0-M2 milestone Apr 25, 2021
@ennru ennru merged commit cd583c0 into akka:master Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants