From bb088e8d0d8bbebe2fd2b957460f4e2e322aa012 Mon Sep 17 00:00:00 2001 From: Alejandro Ventura De Moya Date: Mon, 21 Sep 2026 05:49:30 -0700 Subject: [PATCH] Firefly-2091: handle additional VOTable error formats --- .../firefly/server/db/DbDataIngestor.java | 9 +- .../ipac/table/io/TableParseHandler.java | 5 + .../caltech/ipac/table/io/VoTableReader.java | 94 +++++--- .../ipac/table/DbDataIngestorTest.java | 64 +++++ .../caltech/ipac/table/VoTableReaderTest.java | 218 ++++++++++++++++++ 5 files changed, 359 insertions(+), 31 deletions(-) create mode 100644 src/firefly/test/edu/caltech/ipac/table/DbDataIngestorTest.java diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DbDataIngestor.java b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DbDataIngestor.java index f196abdf38..9352c4b3b9 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DbDataIngestor.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DbDataIngestor.java @@ -84,11 +84,14 @@ static FileInfo ingestDuckReadable(FormatUtil.Format format, DbAdapter dbAdapter static FileInfo ingestVoTable(DbAdapter dbAdapter, String source, Consumer extraMetaSetter, int tblIdx, boolean searchForSpectrum) throws IOException, DataAccessException { if (dbAdapter instanceof DuckDbAdapter) { - VoTableReader.parse(new TableParseHandler.DbIngest(dbAdapter, extraMetaSetter, searchForSpectrum), source, tblIdx); + var handler = new TableParseHandler.DbIngest(dbAdapter, extraMetaSetter, searchForSpectrum); + VoTableReader.parse(handler, source, tblIdx); + if (!handler.hasTable()) throw new DataAccessException("No table found in the VOTable"); return new FileInfo(dbAdapter.getDbFile()); } else { - DataGroup table = VoTableReader.voToDataGroups(source, tblIdx)[0]; - return ingestTable(dbAdapter, table, searchForSpectrum); + DataGroup[] tables = VoTableReader.voToDataGroups(source, tblIdx); + if (tables.length == 0) throw new DataAccessException("No table found in the VOTable"); + return ingestTable(dbAdapter, tables[0], searchForSpectrum); } } diff --git a/src/firefly/java/edu/caltech/ipac/table/io/TableParseHandler.java b/src/firefly/java/edu/caltech/ipac/table/io/TableParseHandler.java index 03112a6c5e..180cd521cc 100644 --- a/src/firefly/java/edu/caltech/ipac/table/io/TableParseHandler.java +++ b/src/firefly/java/edu/caltech/ipac/table/io/TableParseHandler.java @@ -146,6 +146,11 @@ public void end() { if (appender != null) Try.it(() -> appender.close()); if (connWrapper != null) Try.it(() -> connWrapper.close()); } + + /** true once a table header has been ingested, i.e. the DATA table exists */ + public boolean hasTable() { + return table != null; + } } } diff --git a/src/firefly/java/edu/caltech/ipac/table/io/VoTableReader.java b/src/firefly/java/edu/caltech/ipac/table/io/VoTableReader.java index cb97061a59..e28e2252df 100644 --- a/src/firefly/java/edu/caltech/ipac/table/io/VoTableReader.java +++ b/src/firefly/java/edu/caltech/ipac/table/io/VoTableReader.java @@ -78,6 +78,8 @@ public class VoTableReader { private static final String MULTI_SPEC_UTYPE_LOWER= "ipac:multispectrum"; + private static final String NO_ERROR_MESSAGE = "The service reported an error but provided no message"; + private static final Pattern HMS_UCD_PATTERN = Pattern.compile( "POS_EQ_RA.*|pos\\.eq\\.ra.*", @@ -171,7 +173,7 @@ public static String getError(InputStream inputStream, String location) throws D VOElementFactory voFactory = new VOElementFactory(); voFactory.setStoragePolicy(PREFER_MEMORY); VOElement top = voFactory.makeVOElement(inputStream, null); - return getQueryStatusError(top); + return findErrorMessage(top); } catch (SAXException |IOException e) { LOG.error(e); throw new DataAccessException("unable to parse " + location + "\n"+ @@ -184,38 +186,74 @@ public static String getError(InputStream inputStream, String location) throws D // //==================================================================== - private static String getQueryStatusError(VOElement top) { - String error = null; - // check for errors: section 4.4 of http://www.ivoa.net/documents/DALI/20170517/REC-DALI-1.1.html - VOElement[] resources = top.getChildrenByName( "RESOURCE" ); - for (VOElement r : resources) { - if ("results".equals(r.getAttribute("type"))) { - VOElement[] infos = r.getChildrenByName("INFO"); - for (VOElement info : infos) { - if ("QUERY_STATUS".equals(info.getName()) && - "ERROR".equalsIgnoreCase(info.getAttribute("value"))) { - error = info.getTextContent(); + /** + * Look for an error message in this document, trying the shapes real archives emit in order of + * specificity. The first non-blank message wins; it is always trimmed, and null is returned when + * the document does not look like an error document. + * + * @param top the root of the votable + */ + private static String findErrorMessage(VOElement top) { + + boolean errorFlagged = false; // a QUERY_STATUS="ERROR" marker was seen, with or without a message + + // 1. DALI 1.1 section 4.4 / TAP: the marker is an INFO in the results RESOURCE, message in its text content. + // https://www.ivoa.net/documents/DALI/20170517/REC-DALI-1.1.html#sect:errors + // NED: the same marker as a PARAM, message in its DESCRIPTION, which the text content includes. + for (VOElement res : top.getChildrenByName("RESOURCE")) { + if (!"results".equals(res.getAttribute("type"))) continue; + for (String tag : new String[] {"INFO", "PARAM"}) { + for (VOElement el : res.getChildrenByName(tag)) { + if ("QUERY_STATUS".equals(el.getName()) && "ERROR".equalsIgnoreCase(el.getAttribute("value"))) { + errorFlagged = true; + String msg = trimToNull(el.getTextContent()); + if (msg != null) return msg; } } } } - if (error == null) { - // workaround for misplaced INFO attributes with errors - NodeList infos = top.getElementsByVOTagName("INFO"); // all descendant elements - String [] namesWithMisspelling = {"QUERY_STATUS","QUERY STATUS"}; - for (int i = 0; i < infos.getLength(); i++) { - Node node = infos.item(i); - if (node.getNodeType() == Node.ELEMENT_NODE) { - Element info = (Element) node; - String name = info.getAttribute("name"); - if (Arrays.asList(namesWithMisspelling).contains(name) - && "ERROR".equalsIgnoreCase(info.getAttribute("value"))) { - error = info.getTextContent(); - } - } + + // 2. the same marker, INFO or PARAM, but misplaced in the document or with the name missing the underscore. + List markers = descendantsByVOTagName(top, "INFO"); + markers.addAll(descendantsByVOTagName(top, "PARAM")); + for (Element el : markers) { + String name = el.getAttribute("name"); + if (("QUERY_STATUS".equalsIgnoreCase(name) || "QUERY STATUS".equalsIgnoreCase(name)) + && "ERROR".equalsIgnoreCase(el.getAttribute("value"))) { + errorFlagged = true; + String msg = trimToNull(el.getTextContent()); + if (msg != null) return msg; + } + } + + // 3. an INFO or PARAM anywhere in the document whose name or ID is "Error", with the message in its + // value attribute. The text content is used only when value is blank. + for (Element el : markers) { + if ("Error".equalsIgnoreCase(el.getAttribute("name")) || "Error".equalsIgnoreCase(el.getAttribute(ID))) { + String msg = trimToNull(el.getAttribute("value")); + if (msg == null) msg = trimToNull(el.getTextContent()); + if (msg != null) return msg; } } - return error; + + // 4. a QUERY_STATUS="ERROR" marker was found but carried no message: return a generic one. + // Otherwise return null. + return errorFlagged ? NO_ERROR_MESSAGE : null; + } + + /** all descendant elements with the given unqualified VOTable tag name, in document order */ + private static List descendantsByVOTagName(VOElement top, String voTagName) { + NodeList nodes = top.getElementsByVOTagName(voTagName); + List elements = new ArrayList<>(nodes.getLength()); + for (int i = 0; i < nodes.getLength(); i++) { + Node node = nodes.item(i); + if (node.getNodeType() == Node.ELEMENT_NODE) elements.add((Element) node); + } + return elements; + } + + private static String trimToNull(String s) { + return isEmpty(s) ? null : s.trim(); } private static VOElement getVoTableRoot(String location, StoragePolicy policy) throws IOException { @@ -269,7 +307,7 @@ private static List getAllTableElements(VOElement docRoot) throws }); if (tableAry.isEmpty()) { - String error = getQueryStatusError(docRoot); + String error = findErrorMessage(docRoot); if (error != null) { throw new IOException(error); } diff --git a/src/firefly/test/edu/caltech/ipac/table/DbDataIngestorTest.java b/src/firefly/test/edu/caltech/ipac/table/DbDataIngestorTest.java new file mode 100644 index 0000000000..3c0b0d946e --- /dev/null +++ b/src/firefly/test/edu/caltech/ipac/table/DbDataIngestorTest.java @@ -0,0 +1,64 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ +package edu.caltech.ipac.table; + +import edu.caltech.ipac.firefly.ConfigTest; +import edu.caltech.ipac.firefly.data.TableServerRequest; +import edu.caltech.ipac.firefly.server.db.DbDataIngestor; +import edu.caltech.ipac.firefly.server.db.DuckDbAdapter; +import edu.caltech.ipac.firefly.server.db.HsqlDbAdapter; +import edu.caltech.ipac.firefly.server.query.DataAccessException; +import org.junit.Assert; +import org.junit.Test; + +import java.io.File; +import java.nio.file.Files; + +public class DbDataIngestorTest extends ConfigTest { + + @Test + public void voTableWithoutTable() throws Exception { + File votable = File.createTempFile("no-table-", ".xml"); + votable.deleteOnExit(); + Files.writeString(votable.toPath(), """ + + + + +"""); + File dbFile = new File(votable.getParentFile(), votable.getName() + ".duckdb"); + dbFile.deleteOnExit(); + + try { + DbDataIngestor.ingestData(new TableServerRequest("test"), new DuckDbAdapter(dbFile), null, votable, 0); + Assert.fail("a VOTable without a TABLE must not ingest as if it succeeded"); + } catch (DataAccessException e) { + Assert.assertEquals("No table found in the VOTable", e.getMessage()); + } + } + + /** + * The same VOTable on a database other than DuckDB. + */ + @Test + public void voTableWithoutTableNonDuckDb() throws Exception { + File votable = File.createTempFile("no-table-", ".xml"); + votable.deleteOnExit(); + Files.writeString(votable.toPath(), """ + + + + +"""); + File dbFile = new File(votable.getParentFile(), votable.getName() + ".hsql"); + dbFile.deleteOnExit(); + + try { + DbDataIngestor.ingestData(new TableServerRequest("test"), new HsqlDbAdapter(dbFile), null, votable, 0); + Assert.fail("a VOTable without a TABLE must not ingest as if it succeeded"); + } catch (DataAccessException e) { + Assert.assertEquals("No table found in the VOTable", e.getMessage()); + } + } +} diff --git a/src/firefly/test/edu/caltech/ipac/table/VoTableReaderTest.java b/src/firefly/test/edu/caltech/ipac/table/VoTableReaderTest.java index f0167bfd6e..f240baf532 100644 --- a/src/firefly/test/edu/caltech/ipac/table/VoTableReaderTest.java +++ b/src/firefly/test/edu/caltech/ipac/table/VoTableReaderTest.java @@ -11,11 +11,15 @@ import edu.caltech.ipac.firefly.util.FileLoader; import edu.caltech.ipac.table.io.VoTableReader; import org.apache.logging.log4j.Level; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import java.io.ByteArrayInputStream; import java.io.File; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; /** * @author loi @@ -33,6 +37,220 @@ public static void setUp() { } +//==================================================================== +// error document detection +//==================================================================== + + /** + * captured: IRSA SIA, bad POS. The DALI 1.1 sec 4.4 shape. + * https://irsa.ipac.caltech.edu/SIA?COLLECTION=spitzer_seip&POS=xxx+83.6+22.0+0.1 + */ + @Test + public void daliQueryStatusError() throws Exception { + String votable = """ + + + Caltech/IPAC-IRSA IVOA Simple Image Access v2 Service + + UsageFault: BAD_REQUEST: Unknown shape in POS. Expected CIRCLE, RANGE, or POLYGON, but found: xxx + + +"""; + Assert.assertEquals("UsageFault: BAD_REQUEST: Unknown shape in POS. Expected CIRCLE, RANGE, or POLYGON, but found: xxx", + getError(votable)); + } + + /** + * NED example, unresolvable object name. The QUERY_STATUS marker is a PARAM, not an INFO, + * and the message is in its DESCRIPTION. + */ + @Test + public void nedQueryStatusParam() throws Exception { + String votable = """ + + + + GeneralFault: Service could not complete request; Failed to resolve input object name (6) + + + +"""; + Assert.assertEquals("GeneralFault: Service could not complete request; Failed to resolve input object name (6)", + getError(votable)); + } + + /** + * captured: IRSA SCS, RA=abc. Message in the value attribute, INFO at VOTABLE top level. + * https://irsa.ipac.caltech.edu/SCS?table=fp_psc&RA=abc&DEC=22.0&SR=0.01 + */ + @Test + public void scsErrorInValueAttribute() throws Exception { + String votable = """ + + + Caltech/IPAC-IRSA IVOA Simple Cone Search Service + + + +"""; + // not the top-level DESCRIPTION, which holds the service's name + Assert.assertEquals("BAD_REQUEST: The value of parameter 'RA' could not be converted to a number: abc", + getError(votable)); + } + + /** + * captured: HEASARC cone search, unknown table. Same shape, but nested in RESOURCE and with no ID attribute. + * https://heasarc.gsfc.nasa.gov/cgi-bin/vo/cone/coneGet.pl?table=xxxnosuchtable&RA=83.6&DEC=22.0&SR=0.1 + */ + @Test + public void scsErrorNestedWithoutId() throws Exception { + String votable = """ + + + + + + +"""; + Assert.assertEquals("Unknown table: No information available on xxxnosuchtable. Is this a valid table?", + getError(votable)); + } + + /** + * captured: VizieR cone search, unknown catalog. The Error INFO is one of nine, and a Warning follows it. + * https://vizier.cds.unistra.fr/viz-bin/conesearch/NOSUCHCAT?RA=83.6&DEC=22.0&SR=0.1 + * The body repeats that URL in its own INFO name="request", so it carries its provenance with it. + * Shortened: the DESCRIPTION is cut down and the XML comment after it is removed. + */ + @Test + public void scsErrorAmongManyInfos() throws Exception { + String votable = """ + + + + VizieR Astronomical Server vizier.cds.unistra.fr + In case of problem, please report to: cds-question@unistra.fr + + + IVOID of the protocol through which the data was retrieved + Query execution date + Full request URL + Email or URL to contact publisher + Software version + Data centre that produced the VOTable + + + + + + +"""; + // neither the Warning that follows it, nor the text content of the INFOs that precede it + Assert.assertEquals("Table or Catalog not found: NOSUCHCAT", getError(votable)); + } + + /** an error marker with nothing in it used to yield "", which is non-null and so reaches the user blank */ + @Test + public void errorWithNoMessageIsNotBlank() throws Exception { + String votable = """ + + + + + + +"""; + String error = getError(votable); + Assert.assertNotNull("an error document must not report a null error", error); + Assert.assertFalse("an error document must not report a blank error", error.trim().isEmpty()); + } + + /** + * A successful response is not an error document, even though it carries INFO, PARAM and a column + * named Error. Adapted, not verbatim: the shape is a real IRSA SIA success -- + * https://irsa.ipac.caltech.edu/SIA?COLLECTION=spitzer_seip&POS=circle+83.6+22.0+0.002 + * -- cut to one row, with the "Error" FIELD added by hand. Real SIA emits no + * element named Error, so only the two TABLEs and QUERY_STATUS="OK" carry over from the live body. + */ + @Test + public void successfulResponseHasNoError() throws Exception { + String votable = """ + + + Caltech/IPAC-IRSA IVOA Simple Image Access v2 Service + + + + + + + + +
spitzer_seip0.12
+ + + + + + + + +
cutout
+ + +"""; + Assert.assertNull(getError(votable)); + } + + @Test + public void errorMessageIsTrimmed() throws Exception { + String votable = """ + + + + + UsageFault: BAD_REQUEST: missing required parameter + + + +"""; + Assert.assertEquals("UsageFault: BAD_REQUEST: missing required parameter", getError(votable)); + } + + /** the Error INFO is read even when the document also carries a TABLE */ + @Test + public void errorIsReadAlongsideATable() throws Exception { + String votable = """ + + + + + + + + + +
83.633107
+ + +"""; + Assert.assertEquals("BAD_REQUEST: unreadable position", getError(votable)); + } + + private static String getError(String votable) throws Exception { + return VoTableReader.getError(toStream(votable), "n/a"); + } + + private static InputStream toStream(String votable) { + return new ByteArrayInputStream(votable.getBytes(StandardCharsets.UTF_8)); + } + + +//==================================================================== +// performance +//==================================================================== @Category({TestCategory.Perf.class}) @Test