-
Notifications
You must be signed in to change notification settings - Fork 16
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
[CSAPI] Fix exception after parsing procedure XML #252
[CSAPI] Fix exception after parsing procedure XML #252
Conversation
When parsing the XML for a (new) procedure, BaseResourceHandler.create() calls SmlProcessBindingSmlXml.deserialize() repeatedly until there is no more data to be parsed. However, the code that decides there is no more data did not actually work. After parsing the last object, the parser would still point at the last closing tag, so hasNext() would return true. However, the only event that then is still available is END_OF_DOCUMENT, which is not enough for nextTag(), which then throws. In practice, this meant that an object to be added would be added as expected but then an exception was raised. Note that this has not been tested with actually adding more than one object, since I could not figure out how to format multiple objects in a way they would be accepted at all (simply concatenating them produces "Illegal to have multiple roots"). This fixes opensensorhub#251
Note that I suspect the same problem exists in the SmlFeatureBindingSmlXml class. If this approach is accepted, I can apply the same fix there if needed (though I do not have a setup ready to test that). |
I see this and other PRs are closed without review and without comment, what is the plan here? I spent some time to submit these fixes, which solve actual problems I ran into. It would be nice to see these merged? |
Hi Matthijs, sorry about that. I didn't mean to close this PR. It was closed automatically when I deleted the v2 branch now that the work has moved to master. I didn't realize GitHub would close PRs based on a deleted branch automatically. |
Alright, I was able to recreate the v2 branch, reopen the PR and then change the base to master so we're good to go now. I am going to delete the v2 branch again so please track master from now on. |
I fixed this slightly differently in 4f5fc2e to avoid generating an exception. |
Ah, that explains, thanks :-)
Cool, that would be even better. But are you sure this works? Could you reproduce the original problem?
Edit: Nvm, I missed on change, see comment below |
Oh wait, you also switched from calling |
Yes, I was able to reproduce the problem and tested that this fix works too. |
When parsing the XML for a (new) procedure, BaseResourceHandler.create() calls SmlProcessBindingSmlXml.deserialize() repeatedly until there is no more data to be parsed.
However, the code that decides there is no more data did not actually work. After parsing the last object, the parser would still point at the last closing tag, so hasNext() would return true. However, the only event that then is still available is END_OF_DOCUMENT, which is not enough for nextTag(), which then throws.
In practice, this meant that an object to be added would be added as expected but then an exception was raised.
Note that this has not been tested with actually adding more than one object, since I could not figure out how to format multiple objects in a way they would be accepted at all (simply concatenating them produces "Illegal to have multiple roots").
This fixes #251