Handlers #186
Handlers #186
Conversation
| @Override | ||
| public void startElement(String uri, String localName, String qName, Attributes attributes) | ||
| throws SAXException { | ||
| if (localName.equals("dataset")) { |
There was a problem hiding this comment.
Should this throw an exception if it sees a second dataset?
There was a problem hiding this comment.
I don't think the old code specifies throwing an exception on getting a second child dataset,
https://github.com/ERDDAP/erddap/blob/main/WEB-INF/classes/gov/noaa/pfel/erddap/dataset/EDDGridFromEDDTable.java#L113-L127
But I am guessing since the tEDDTable variable will update to the last child dataset which means there is no point in adding multiple, you are implying to add a check so that we only have one child dataset?
There was a problem hiding this comment.
Yeah, add a check to make sure there's only one child dataset. Allowing multiple to be parsed could cause issues, and I think it's better to catch those early with explicit error messages.
| public void startElement(String uri, String localName, String qName, Attributes attributes) | ||
| throws SAXException { | ||
| switch (localName) { | ||
| case "dataset" -> { |
There was a problem hiding this comment.
Should this throw an exception if it sees a second dataset?
|
How does this look? |
This is good, thanks for adding the checks. There are some types that can have multiple children (EDDGridSideBySide, EDDTableAggregateRows, EDDGridAggregateExistingDimension). |
|
Don't forget the |
Description