From 75d5614f9cc286b88139b818733b8a51379bd858 Mon Sep 17 00:00:00 2001 From: loi Date: Tue, 22 Sep 2026 14:40:22 -0700 Subject: [PATCH] FIREFLY-2108: Deprecate DsvTableIO and move its delimited-file reading functionality into DuckDbReadable - Centralize DdColumn-to-DataType mapping to prevent divergence. --- .../firefly/server/catquery/SDSSQuery.java | 6 +- .../ipac/firefly/server/db/BaseDbAdapter.java | 40 +-- .../firefly/server/db/DbDataIngestor.java | 3 +- .../firefly/server/db/DuckDbReadable.java | 86 ++++- .../firefly/server/db/EmbeddedDbUtil.java | 156 ++++++---- .../imagesources/IrsaMasterDataSource.java | 45 +-- .../edu/caltech/ipac/table/TableUtil.java | 11 +- .../edu/caltech/ipac/table/io/DsvTableIO.java | 1 + .../IrsaMasterDataSourceTest.java | 54 ++++ .../caltech/ipac/table/DuckDbAdapterTest.java | 293 +++++++++++++++++- 10 files changed, 551 insertions(+), 144 deletions(-) create mode 100644 src/firefly/test/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSourceTest.java diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/catquery/SDSSQuery.java b/src/firefly/java/edu/caltech/ipac/firefly/server/catquery/SDSSQuery.java index fe00b5c81d..02cd6ff766 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/catquery/SDSSQuery.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/catquery/SDSSQuery.java @@ -25,7 +25,8 @@ import edu.caltech.ipac.table.IpacTableUtil; import edu.caltech.ipac.table.TableMeta; import edu.caltech.ipac.table.TableUtil; -import edu.caltech.ipac.table.io.DsvTableIO; +import edu.caltech.ipac.util.FormatUtil; +import edu.caltech.ipac.firefly.server.db.DuckDbReadable; import edu.caltech.ipac.table.io.IpacTableReader; import edu.caltech.ipac.table.io.IpacTableWriter; import edu.caltech.ipac.table.query.DataGroupQuery; @@ -34,7 +35,6 @@ import edu.caltech.ipac.util.download.URLDownload; import edu.caltech.ipac.visualize.plot.CoordinateSys; import edu.caltech.ipac.visualize.plot.WorldPt; -import org.apache.commons.csv.CSVFormat; import java.io.BufferedOutputStream; import java.io.BufferedReader; @@ -164,7 +164,7 @@ protected File loadDataFile(TableServerRequest request) throws IOException, Data // check for errors in returned file evaluateCVS(csv); - DataGroup dg = DsvTableIO.parse(csv, CSVFormat.DEFAULT.withCommentMarker('#')); + DataGroup dg = csv.length() == 0 ? null : DuckDbReadable.read(FormatUtil.Format.CSV, csv.getAbsolutePath()); if (dg == null) { _log.info("no data found for search"); return null; diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/db/BaseDbAdapter.java b/src/firefly/java/edu/caltech/ipac/firefly/server/db/BaseDbAdapter.java index b832f4ff5e..f10684b041 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/db/BaseDbAdapter.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/db/BaseDbAdapter.java @@ -734,35 +734,12 @@ protected Object[] getDdFrom(DataType dt, int colIdx) { case ROW_NUM -> 1_000_001; default -> colIdx; }; - return new Object[] { - dt.getKeyName(), - dt.getLabel(), - dt.getTypeDesc(), - dt.getUnits(), - dt.getNullString(), - dt.getFormat(), - dt.getFmtDisp(), - dt.getWidth(), - dt.getVisibility().name(), - dt.isSortable(), - dt.isFilterable(), - dt.isFixed(), - dt.getDesc(), - dt.getEnumVals(), - dt.getID(), - dt.getPrecision(), - dt.getUCD(), - dt.getUType(), - dt.getRef(), - dt.getMaxValue(), - dt.getMinValue(), - Util.serialize(dt.getLinkInfos()), // index(21) is used in HsqlDbAdapter. if it changes, update. - dt.getDataOptions(), - dt.getArraySize(), - dt.getCellRenderer(), - dt.getSortByCols(), - colIdx - }; + Object[] row = new Object[EmbeddedDbUtil.DD_COLS.size() + 1]; + for (int i = 0; i < EmbeddedDbUtil.DD_COLS.size(); i++) { + row[i] = EmbeddedDbUtil.DD_COLS.get(i).get().apply(dt); + } + row[row.length-1] = colIdx; // order_index is the column's position, not something dt holds + return row; } // insert column info into the table @@ -964,7 +941,6 @@ Object dbToDD(DataGroup dg, ResultSet rs) { // if this column is not in DataGroup. no need to update the info if (dtype != null) { EmbeddedDbUtil.dbToDataType(dtype, rs); - handleSpecialDTypes(dtype, dg, rs); } }; } catch (SQLException e) { @@ -973,10 +949,6 @@ Object dbToDD(DataGroup dg, ResultSet rs) { return 0; } - void handleSpecialDTypes(DataType dtype, DataGroup dg, ResultSet rs) { - applyIfNotEmpty(deserialize(rs, "links"), v -> dtype.setLinkInfos((List) v)); - } - public void copyDDFromSource(String tblName, String sourceTbl) { List cnames = getColumnNamesFromSys(tblName, "'"); String ddSql = "select * from %s_DD".formatted(sourceTbl) + 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 2fe4b6a208..f196abdf38 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 @@ -9,7 +9,6 @@ import edu.caltech.ipac.firefly.server.query.DataAccessException; import edu.caltech.ipac.firefly.server.util.JsonToDataGroup; import edu.caltech.ipac.table.DataGroup; -import edu.caltech.ipac.table.io.DsvTableIO; import edu.caltech.ipac.table.io.FITSTableReader; import edu.caltech.ipac.table.io.IpacTableReader; import edu.caltech.ipac.table.io.SpectrumMetaInspector; @@ -78,7 +77,7 @@ static FileInfo ingestDuckReadable(FormatUtil.Format format, DbAdapter dbAdapter } else if (format == PARQUET) { throw new DataAccessException("Unsupported format (%s), file: %s".formatted(format, source)); } else { - DataGroup table = DsvTableIO.parse(new File(source), format); + DataGroup table = DuckDbReadable.read(format, source); // to avoid using DsvTableIO.parse return ingestTable(dbAdapter, table, searchForSpectrum); } } diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbReadable.java b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbReadable.java index 71b9ea3267..17b3e6c47f 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbReadable.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/db/DuckDbReadable.java @@ -11,6 +11,7 @@ import edu.caltech.ipac.firefly.server.util.QueryUtil; import edu.caltech.ipac.firefly.server.util.StopWatch; import edu.caltech.ipac.table.DataGroup; +import edu.caltech.ipac.table.DataType; import edu.caltech.ipac.table.io.VoTableReader; import edu.caltech.ipac.table.io.VoTableWriter; import edu.caltech.ipac.util.FileUtil; @@ -30,6 +31,8 @@ import javax.annotation.Nonnull; import static edu.caltech.ipac.firefly.core.Util.Opt.ifNotNull; +import static edu.caltech.ipac.firefly.server.db.EmbeddedDbUtil.applyInfoToDataType; +import static edu.caltech.ipac.firefly.server.db.EmbeddedDbUtil.dbToDataGroup; import static edu.caltech.ipac.table.TableUtil.getAliasName; /** @@ -116,6 +119,49 @@ public DataGroup getInfo(String source) throws DataAccessException { return table; } + /** Same as {@link #read(FormatUtil.Format, String, Consumer)}, with no extra meta. */ + public static DataGroup read(FormatUtil.Format format, String source) throws DataAccessException { + return read(format, source, null); + } + + /** + * Reads the given source with the adapter that handles this format. + * @param format format of the source + * @return the source as a DataGroup, or null if no adapter reads this format + * @see #read(String, Consumer) + */ + public static DataGroup read(FormatUtil.Format format, String source, Consumer extraMetaSetter) throws DataAccessException { + var adapter = getDetachedAdapter(format); + return adapter == null ? null : adapter.read(source, extraMetaSetter); + } + + /** Same as {@link #read(String, Consumer)}, with no extra meta. */ + public DataGroup read(String source) throws DataAccessException { + return read(source, null); + } + + /** + * Reads the given source into a DataGroup. The data is read into memory; no dbFile needed. + * @param source can be a local file path or a URL + * @param extraMetaSetter additional meta to apply to the returned table + * @return the source as a DataGroup + */ + public DataGroup read(String source, Consumer extraMetaSetter) throws DataAccessException { + StopWatch.getInstance().start("read: " + source); + DataGroup tableMeta = getTableMeta(source, extraMetaSetter); // the source's schema, with all of its meta applied + String sql = "SELECT * from %s".formatted(sqlReadSource(source)); + try { + DataGroup table = getJdbcTmpl().query(sql, rs -> dbToDataGroup(rs, tableMeta)); // adds the rows to it + StopWatch.getInstance().printLog("read: " + source); + return table; + } catch (Exception e) { + LOGGER.warn("read failed with error: " + e.getMessage(), + "sql: " + sql, + "source: " + source); + throw handleSqlExp("Query failed", e); + } + } + /** * Ingest data directly from a source file. This file can be local or remote. * @param source can be a local file path or a URL @@ -179,20 +225,46 @@ String sqlReadSource(String srcFile) { return "read_parquet('%s')".formatted(srcFile); } + /** + * Start from the Parquet schema then apply embedded VOTable metadata on top, ensuring column info matches the actual file. + */ @Override protected DataGroup getTableMeta(String source, Consumer extraMetaSetter) throws DataAccessException { + DataGroup tableMeta = super.getTableMeta(source, null); // the schema, straight from the file + ifNotNull(readVoTableMeta(source)).apply(voMeta -> applyVoMeta(tableMeta, voMeta)); + if (extraMetaSetter != null) extraMetaSetter.accept(tableMeta); + return tableMeta; + } + + /** + * Applies VOTable metadata onto the given table + */ + private static void applyVoMeta(DataGroup table, DataGroup voMeta) { + for (DataType col : table.getDataDefinitions()) { + DataType info = voMeta.getDataDefintion(col.getKeyName()); // exact match first; + if (info == null) info = voMeta.getDataDefintion(col.getKeyName(), true); // then, ignore case + applyInfoToDataType(col, info); + } + table.setTitle(voMeta.getTitle()); + table.setTableMeta(voMeta.getTableMeta()); + table.setGroupInfos(voMeta.getGroupInfos()); + table.setLinkInfos(voMeta.getLinkInfos()); + table.setParamInfos(voMeta.getParamInfos()); + table.setResourceInfos(voMeta.getResourceInfos()); + } + + /** + * @return the VOTable stored in the Parquet metadata, or null if unavailable or unreadable + */ + private DataGroup readVoTableMeta(String source) { var jdbc = JdbcFactory.getTemplate(getDbInstance()); try { var votable = jdbc.queryForObject( "SELECT decode(value) FROM parquet_kv_metadata('%s') where key = 'IVOA.VOTable-Parquet.content'".formatted(source), String.class); - if (votable != null) { - DataGroup tableMeta = VoTableReader.voToDataGroups(new ByteArrayInputStream(votable.getBytes()), false)[0]; - if (tableMeta != null && extraMetaSetter != null) extraMetaSetter.accept(tableMeta); - return tableMeta; - } - } catch (Exception ignored) {} // ignored if it can't read - return super.getTableMeta(source, extraMetaSetter); + return votable == null ? null : + VoTableReader.voToDataGroups(new ByteArrayInputStream(votable.getBytes()), false)[0]; + } catch (Exception ignored) { return null; } // ignored if it can't read } public void export(TableServerRequest treq, OutputStream out) throws DataAccessException { diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/db/EmbeddedDbUtil.java b/src/firefly/java/edu/caltech/ipac/firefly/server/db/EmbeddedDbUtil.java index 150103e6fa..acfe3c23fc 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/db/EmbeddedDbUtil.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/db/EmbeddedDbUtil.java @@ -31,6 +31,8 @@ import java.time.LocalDate; import java.time.LocalDateTime; import java.util.*; +import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.Date; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -61,37 +63,73 @@ public class EmbeddedDbUtil { private static final Logger.LoggerImpl logger = Logger.getLogger(); private static final int MAX_COL_ENUM_COUNT = AppProperties.getIntProperty("max.col.enum.count", 32); - static final String DD_INSERT_SQL = "insert into %s_DD values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; - static final String DD_CREATE_SQL = "create table %s_DD "+ - "(" + - " cname varchar(64000)" + - ", label varchar(64000)" + - ", type varchar(255)" + - ", units varchar(255)" + - ", null_str varchar(255)" + - ", format varchar(255)" + - ", fmtDisp varchar(64000)" + - ", width int" + - ", visibility varchar(255)" + - ", sortable boolean" + - ", filterable boolean" + - ", fixed boolean" + - ", description varchar(64000)" + - ", enumVals varchar(64000)" + - ", ID varchar(64000)" + - ", precision varchar(64000)" + - ", ucd varchar(64000)" + - ", utype varchar(64000)" + - ", ref varchar(64000)" + - ", maxValue varchar(64000)" + - ", minValue varchar(64000)" + - ", links TEXT" + - ", dataOptions varchar(64000)" + - ", arraySize varchar(255)" + - ", cellRenderer varchar(64000)" + - ", sortByCols varchar(64000)" + - ", order_index int" + - ")"; + /** reads one DD column out of a ResultSet row; the type-specific getter matters, ie. getInt() gives 0 where getObject() gives null */ + interface RsDDReader { Object read(ResultSet rs, String col) throws SQLException; } + + /** + * Column's info: The DD table schema, read/write logic, and DataType mapping are all derived from it. + * @param name the DD column name + * @param dbType its SQL type + * @param get the value to store, taken from a DataType + * @param set applies the stored value to a DataType; null for column-identifying info + * @param read pulls the value out of a DD row + */ + record DdCol(String name, String dbType, Function get, + BiConsumer set, RsDDReader read) {} + + private static DdCol text(String name, int size, Function get, BiConsumer set) { + return new DdCol(name, "varchar(%d)".formatted(size), get, set, ResultSet::getString); + } + private static String asStr(Object v) { return (String) v; } + + /** every piece of column info the DD carries, in DD column order. order_index is appended separately; it is + * the column's position in the table rather than anything a DataType holds. */ + static final List DD_COLS = List.of( + text("cname", 64000, DataType::getKeyName, null), + text("label", 64000, DataType::getLabel, (dt,v) -> dt.setLabel(asStr(v))), + text("type", 255, DataType::getTypeDesc, (dt,v) -> { + dt.setTypeDesc(asStr(v)); + dt.setDataType(DataType.descToType(asStr(v), dt.getDataType())); // if desc is unknown, keep the type we already have + }), + text("units", 255, DataType::getUnits, (dt,v) -> dt.setUnits(asStr(v))), + text("null_str", 255, DataType::getNullString, (dt,v) -> dt.setNullString(asStr(v))), + text("format", 255, DataType::getFormat, (dt,v) -> dt.setFormat(asStr(v))), + text("fmtDisp", 64000, DataType::getFmtDisp, (dt,v) -> dt.setFmtDisp(asStr(v))), + new DdCol("width", "int", DataType::getWidth, + (dt,v) -> dt.setWidth((Integer) v), ResultSet::getInt), + text("visibility", 255, dt -> dt.getVisibility().name(), + (dt,v) -> dt.setVisibility(DataType.Visibility.valueOf(asStr(v)))), + new DdCol("sortable", "boolean", DataType::isSortable, + (dt,v) -> dt.setSortable((Boolean) v), ResultSet::getBoolean), + new DdCol("filterable", "boolean", DataType::isFilterable, + (dt,v) -> dt.setFilterable((Boolean) v), ResultSet::getBoolean), + new DdCol("fixed", "boolean", DataType::isFixed, + (dt,v) -> dt.setFixed((Boolean) v), ResultSet::getBoolean), + text("description", 64000, DataType::getDesc, (dt,v) -> dt.setDesc(asStr(v))), + text("enumVals", 64000, DataType::getEnumVals, (dt,v) -> dt.setEnumVals(asStr(v))), + text("ID", 64000, DataType::getID, (dt,v) -> dt.setID(asStr(v))), + text("precision", 64000, DataType::getPrecision, (dt,v) -> dt.setPrecision(asStr(v))), + text("ucd", 64000, DataType::getUCD, (dt,v) -> dt.setUCD(asStr(v))), + text("utype", 64000, DataType::getUType, (dt,v) -> dt.setUType(asStr(v))), + text("ref", 64000, DataType::getRef, (dt,v) -> dt.setRef(asStr(v))), + text("maxValue", 64000, DataType::getMaxValue, (dt,v) -> dt.setMaxValue(asStr(v))), + text("minValue", 64000, DataType::getMinValue, (dt,v) -> dt.setMinValue(asStr(v))), + new DdCol("links", "TEXT", dt -> Util.serialize(dt.getLinkInfos()), + (dt,v) -> applyIfNotEmpty(Try.it(() -> Util.deserialize(asStr(v))).get(), + o -> dt.setLinkInfos((List) o)), + ResultSet::getString), + text("dataOptions", 64000, DataType::getDataOptions, (dt,v) -> dt.setDataOptions(asStr(v))), + text("arraySize", 255, DataType::getArraySize, (dt,v) -> dt.setArraySize(asStr(v))), + text("cellRenderer",64000, DataType::getCellRenderer, (dt,v) -> dt.setCellRenderer(asStr(v))), + text("sortByCols", 64000, DataType::getSortByCols, (dt,v) -> dt.setSortByCols(asStr(v))), + text("xtype", 255, DataType::getXType, (dt,v) -> dt.setXType(asStr(v))) + ); + + static final String DD_INSERT_SQL = "insert into %s_DD values (" + + String.join(",", Collections.nCopies(DD_COLS.size() + 1, "?")) + ")"; + static final String DD_CREATE_SQL = "create table %s_DD (" + + DD_COLS.stream().map(c -> "%s %s".formatted(c.name(), c.dbType())).collect(Collectors.joining(", ")) + + ", order_index int)"; static final String META_INSERT_SQL = "insert into %s_META values (?,?,?)"; static final String META_CREATE_SQL = "create table %s_META "+ "(" + @@ -381,45 +419,39 @@ public static int dbToDD(DataGroup dg, ResultSet rs) { return 0; } + /** Applies non-empty values from a DD row to dtype in place. */ public static void dbToDataType(DataType dtype, ResultSet rs) { try { - applyIfNotEmpty(rs.getString("type"), (s) -> { - dtype.setTypeDesc(s); - dtype.setDataType(DataType.descToType(s, dtype.getDataType())); // if desc is unknown, use what's in the database. - }); + applyInfo(dtype, col -> col.read().read(rs, col.name())); + } catch (Exception e) { + logger.warn(e); + } + } - applyIfNotEmpty(rs.getString("label"), dtype::setLabel); - applyIfNotEmpty(rs.getString("units"), dtype::setUnits); - dtype.setNullString(rs.getString("null_str")); - applyIfNotEmpty(rs.getString("format"), dtype::setFormat); - applyIfNotEmpty(rs.getString("fmtDisp"), dtype::setFmtDisp); - applyIfNotEmpty(rs.getInt("width"), dtype::setWidth); - applyIfNotEmpty(rs.getString("visibility"), v -> dtype.setVisibility(DataType.Visibility.valueOf(v))); - applyIfNotEmpty(rs.getString("description"), dtype::setDesc); - applyIfNotEmpty(rs.getBoolean("sortable"), dtype::setSortable); - applyIfNotEmpty(rs.getBoolean("filterable"), dtype::setFilterable); - applyIfNotEmpty(rs.getBoolean("fixed"), dtype::setFixed); - applyIfNotEmpty(rs.getString("enumVals"), dtype::setEnumVals); - applyIfNotEmpty(rs.getString("ID"), dtype::setID); - applyIfNotEmpty(rs.getString("precision"), dtype::setPrecision); - applyIfNotEmpty(rs.getString("ucd"), dtype::setUCD); - applyIfNotEmpty(rs.getString("utype"), dtype::setUType); - applyIfNotEmpty(rs.getString("ref"), dtype::setRef); - applyIfNotEmpty(rs.getString("maxValue"), dtype::setMaxValue); - applyIfNotEmpty(rs.getString("minValue"), dtype::setMinValue); - applyIfNotEmpty(rs.getString("dataOptions"), dtype::setDataOptions); - applyIfNotEmpty(rs.getString("arraySize"), dtype::setArraySize); - applyIfNotEmpty(rs.getString("cellRenderer"), dtype::setCellRenderer); - applyIfNotEmpty(rs.getString("sortByCols"), dtype::setSortByCols); - - if (ignoreCols.contains(dtype.getKeyName())) { - dtype.setVisibility(DataType.Visibility.hide); - } + /** Applies non-empty metadata from info to dtype in place. */ + public static void applyInfoToDataType(DataType dtype, DataType info) { + if (info == null) return; + try { + applyInfo(dtype, col -> col.get().apply(info)); } catch (Exception e) { logger.warn(e); } } + /** supplies each column's value */ + private interface DdSource { Object valueOf(DdCol col) throws Exception; } + + private static void applyInfo(DataType dtype, DdSource source) throws Exception { + if (dtype == null) return; + for (DdCol col : DD_COLS) { + if (col.set() == null) continue; // no setter; don't apply updates + applyIfNotEmpty(source.valueOf(col), v -> col.set().accept(dtype, v)); + } + if (ignoreCols.contains(dtype.getKeyName())) { + dtype.setVisibility(DataType.Visibility.hide); + } + } + /** * Add a column to the end of given table, and populate the values with the given expression. * @param dbAdapter dbAdapter to use for the connection diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSource.java b/src/firefly/java/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSource.java index 8a6771c222..f12d261adc 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSource.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSource.java @@ -5,16 +5,19 @@ package edu.caltech.ipac.firefly.server.visualize.imagesources; -import edu.caltech.ipac.table.io.DsvTableIO; import edu.caltech.ipac.firefly.server.util.Logger; import edu.caltech.ipac.util.AppProperties; import edu.caltech.ipac.table.DataGroup; import edu.caltech.ipac.table.DataObject; import edu.caltech.ipac.util.download.FailedRequestException; -import org.apache.commons.csv.CSVFormat; +import edu.caltech.ipac.firefly.server.db.DuckDbReadable; +import edu.caltech.ipac.firefly.server.query.DataAccessException; +import edu.caltech.ipac.util.FormatUtil; +import edu.caltech.ipac.firefly.server.ServerContext; import java.io.File; -import java.io.FileInputStream; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -22,9 +25,7 @@ import java.util.List; import java.util.Map; import edu.caltech.ipac.firefly.server.visualize.imagesources.ImageMasterDataEntry.PARAMS; -import org.apache.commons.io.FileUtils; -import static edu.caltech.ipac.firefly.server.visualize.imagesources.ImageMasterData.makeJsonObj; /** * @author Trey Roby @@ -173,30 +174,16 @@ private HashMap getMappedColors(DataObject row){ DataGroup getDataFromMasterTable(String masterTableName) throws IOException, FailedRequestException { - InputStream inf= IrsaMasterDataSource.class.getResourceAsStream(masterTableName); - DataGroup dg = DsvTableIO.parse(inf, CSVFormat.DEFAULT); - return dg; - } - - public static void main(String[] args) throws Exception { - IrsaMasterDataSource s = new IrsaMasterDataSource(){ - @Override - DataGroup getDataFromMasterTable(String masterTableName) throws IOException, FailedRequestException { - FileInputStream inf = FileUtils.openInputStream(new File(masterTableName));// - DataGroup dg = DsvTableIO.parse(inf, CSVFormat.DEFAULT); - return dg; - } - }; - List dataList = s.createDataList("/hydra/cm/firefly/src/firefly/java/edu/caltech/ipac/firefly/resources/irsa-image-master-table.csv"); -// ImageMasterDataEntry o = (ImageMasterDataEntry) dataList.get(0); - for(ImageMasterDataEntry o:dataList){ - System.out.println(makeJsonObj(o.getDataMap())); - } - - ExternalMasterDataSource e = new ExternalMasterDataSource(); - List imageMasterData = e.getImageMasterData(); - for(ImageMasterDataEntry o:imageMasterData){ - System.out.println(makeJsonObj(o.getDataMap())); + // copy it out first under the working dir so DuckDB can read it. + File spill = File.createTempFile("master-", ".csv", ServerContext.getTempWorkDir()); + try (InputStream inf = IrsaMasterDataSource.class.getResourceAsStream(masterTableName)) { + if (inf == null) throw new IOException("Master table not found: " + masterTableName); + Files.copy(inf, spill.toPath(), StandardCopyOption.REPLACE_EXISTING); + return DuckDbReadable.read(FormatUtil.Format.CSV, spill.getAbsolutePath()); + } catch (DataAccessException e) { + throw new IOException("Unable to read master table: " + masterTableName, e); + } finally { + spill.delete(); } } diff --git a/src/firefly/java/edu/caltech/ipac/table/TableUtil.java b/src/firefly/java/edu/caltech/ipac/table/TableUtil.java index 1ef3e426da..d195457636 100644 --- a/src/firefly/java/edu/caltech/ipac/table/TableUtil.java +++ b/src/firefly/java/edu/caltech/ipac/table/TableUtil.java @@ -5,7 +5,9 @@ import edu.caltech.ipac.firefly.data.TableServerRequest; import edu.caltech.ipac.firefly.server.util.JsonToDataGroup; -import edu.caltech.ipac.table.io.DsvTableIO; +import edu.caltech.ipac.table.io.SpectrumMetaInspector; +import edu.caltech.ipac.firefly.server.query.DataAccessException; +import edu.caltech.ipac.firefly.server.db.DuckDbReadable; import edu.caltech.ipac.table.io.FITSTableReader; import edu.caltech.ipac.table.io.IpacTableReader; import edu.caltech.ipac.table.io.VoTableReader; @@ -59,7 +61,12 @@ public static DataGroup readAnyFormat(File inf, int tableIndex, TableServerReque return tables[0]; } else return null; } else if (format == FormatUtil.Format.CSV || format == FormatUtil.Format.TSV) { - return DsvTableIO.parse(inf, format, request); + try { + return DuckDbReadable.read(format, inf.getAbsolutePath(), + dg -> SpectrumMetaInspector.searchForSpectrum(dg, request)); + } catch (DataAccessException e) { + throw new IOException("Unable to read %s file: %s".formatted(format, inf), e); + } } else if (format == FormatUtil.Format.FITS ) { try { return FITSTableReader.readFitsTable(inf.getAbsolutePath(), request, tableIndex); diff --git a/src/firefly/java/edu/caltech/ipac/table/io/DsvTableIO.java b/src/firefly/java/edu/caltech/ipac/table/io/DsvTableIO.java index 468daa2c04..3317a14859 100644 --- a/src/firefly/java/edu/caltech/ipac/table/io/DsvTableIO.java +++ b/src/firefly/java/edu/caltech/ipac/table/io/DsvTableIO.java @@ -53,6 +53,7 @@ * LZ added another method in order to read file through an InputStream * */ +@Deprecated public class DsvTableIO { public static DataGroup parse(File inf, Format format) throws IOException { diff --git a/src/firefly/test/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSourceTest.java b/src/firefly/test/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSourceTest.java new file mode 100644 index 0000000000..b8128cf69b --- /dev/null +++ b/src/firefly/test/edu/caltech/ipac/firefly/server/visualize/imagesources/IrsaMasterDataSourceTest.java @@ -0,0 +1,54 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ +package edu.caltech.ipac.firefly.server.visualize.imagesources; + +import edu.caltech.ipac.firefly.ConfigTest; +import edu.caltech.ipac.firefly.server.ServerContext; +import edu.caltech.ipac.table.DataGroup; +import org.junit.Test; + +import java.io.File; +import java.util.List; + +import static org.junit.Assert.*; + +public class IrsaMasterDataSourceTest extends ConfigTest { + + private static final String MASTER_TABLE = "/edu/caltech/ipac/firefly/resources/irsa-image-master-table.csv"; + + @Test + public void testReadMasterTableFromClasspath() throws Exception { + DataGroup dg = new IrsaMasterDataSource().getDataFromMasterTable(MASTER_TABLE); + + assertNotNull("the master table is readable", dg); + assertTrue("it has rows", dg.size() > 0); + + // the columns the image ids are built from must stay text; a numeric guess would change the ids + for (String cname : List.of("missionId", "surveyKey", "wavebandId", "imageId")) { + assertNotNull("%s column exists".formatted(cname), dg.getDataDefintion(cname)); + assertEquals("%s is text".formatted(cname), String.class, dg.getDataDefintion(cname).getDataType()); + } + assertEquals("2MASS", dg.getData("missionId", 0)); + assertEquals("asky", dg.getData("surveyKey", 0)); + + File[] spills = ServerContext.getTempWorkDir().listFiles((d, n) -> n.startsWith("master-")); + assertEquals("the copy does not outlive the read", 0, spills == null ? 0 : spills.length); + } + + /** the master table is only useful if it still builds the entries the image source list is made of */ + @Test + public void testCreateDataList() throws Exception { + List entries = new IrsaMasterDataSource().createDataList(MASTER_TABLE); + + assertNotNull(entries); + assertTrue("entries were built", entries.size() > 0); + + ImageMasterDataEntry first = entries.get(0); + assertEquals("2MASS", first.getParamString(ImageMasterDataEntry.PARAMS.MISSION_ID)); + assertNotNull("every entry gets an imageId", first.getParamString(ImageMasterDataEntry.PARAMS.IMAGE_ID)); + for (ImageMasterDataEntry e : entries) { + assertNotNull("imageId is never null", e.getParamString(ImageMasterDataEntry.PARAMS.IMAGE_ID)); + } + } +} diff --git a/src/firefly/test/edu/caltech/ipac/table/DuckDbAdapterTest.java b/src/firefly/test/edu/caltech/ipac/table/DuckDbAdapterTest.java index f2d1edcaf1..b34748a628 100644 --- a/src/firefly/test/edu/caltech/ipac/table/DuckDbAdapterTest.java +++ b/src/firefly/test/edu/caltech/ipac/table/DuckDbAdapterTest.java @@ -13,6 +13,7 @@ import edu.caltech.ipac.firefly.server.db.DbMonitor; import edu.caltech.ipac.firefly.server.db.DuckDbAdapter; import edu.caltech.ipac.firefly.server.db.DuckDbReadable; +import edu.caltech.ipac.firefly.server.db.EmbeddedDbUtil; import edu.caltech.ipac.firefly.server.db.HsqlDbAdapter; import edu.caltech.ipac.firefly.server.query.DataAccessException; import edu.caltech.ipac.firefly.server.query.DecimationProcessor; @@ -21,26 +22,29 @@ import edu.caltech.ipac.firefly.server.query.tables.IpacTableFromSource; import edu.caltech.ipac.firefly.server.util.Logger; import edu.caltech.ipac.firefly.util.FileLoader; -import edu.caltech.ipac.table.io.DsvTableIO; -import edu.caltech.ipac.util.AppProperties; import edu.caltech.ipac.util.decimate.DecimateKey; -import org.apache.commons.csv.CSVFormat; import org.apache.logging.log4j.Level; import org.junit.After; import org.junit.Before; import org.junit.Test; import java.io.File; +import java.lang.reflect.Method; import java.nio.file.Files; -import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import static edu.caltech.ipac.firefly.data.TableServerRequest.TBL_FILE_TYPE; import static edu.caltech.ipac.firefly.server.db.DuckDbAdapter.*; import static edu.caltech.ipac.firefly.server.util.QueryUtil.SEARCH_REQUEST; +import static edu.caltech.ipac.util.FormatUtil.Format; import static org.junit.Assert.*; public class DuckDbAdapterTest extends ConfigTest { @@ -73,6 +77,285 @@ public void testParquetDbAdapter() throws DataAccessException { assertEquals(7, data.getDataDefinitions().length); } + /** + * test reading a source file directly into a DataGroup, without a dbFile + */ + @Test + public void testReadIntoDataGroup() throws DataAccessException { + File testFile = FileLoader.resolveFile(DuckDbAdapterTest.class, "/iris.parquet"); + + DataGroup info = DuckDbReadable.getInfo(Format.PARQUET, testFile.getAbsolutePath()); + DataGroup table = DuckDbReadable.read(Format.PARQUET, testFile.getAbsolutePath()); + + assertEquals("all of the rows are read", info.size(), table.size()); + assertEquals("same columns as getInfo", info.getDataDefinitions().length, table.getDataDefinitions().length); + assertEquals("no ROW_IDX/ROW_NUM added", 5, table.getDataDefinitions().length); + + // column info from the source is applied to the returned table + DataType sepalWidth = table.getDataDefintion("sepal.width"); + assertNotNull("sepal.width column exists", sepalWidth); + assertEquals(Double.class, sepalWidth.getDataType()); + assertEquals(3.5, (Double) table.getData("sepal.width", 0), 0.0001); + assertEquals("Setosa", table.getData("variety", 0)); + + // same table read via a detached adapter + DataGroup viaAdapter = DuckDbReadable.getDetachedAdapter(Format.PARQUET).read(testFile.getAbsolutePath()); + assertNull("not tied to a dbFile", DuckDbReadable.getDetachedAdapter(Format.PARQUET).getDbFile()); + assertEquals(table.size(), viaAdapter.size()); + } + + /** Ensures direct read and ingest paths produce the same table metadata. */ + @Test + public void testReadMatchesIngestPath() throws Exception { + String pq = voTableParquet(); + + // -- path A: ingest into a dbFile, then read it back out the way the app does + DuckDbReadable attached = DuckDbReadable.castInto(Format.PARQUET, new DuckDbAdapter( + new File(ServerContext.getWorkingDir(), "read-cmp.duckdb"))); + attached.initDbFile(); + attached.ingestDataDirectly(pq, null); + DataGroup viaIngest = attached.execQuery("select * from DATA", "DATA"); + attached.close(true); + + // -- path B: read straight into a DataGroup + DataGroup viaRead = DuckDbReadable.read(Format.PARQUET, pq); + + assertEquals("ingest adds ROW_IDX/ROW_NUM; read does not", + viaIngest.getDataDefinitions().length - 2, viaRead.getDataDefinitions().length); + + for (DataType col : viaRead.getDataDefinitions()) { + DataType ingested = viaIngest.getDataDefintion(col.getKeyName()); + assertNotNull("column %s is in both".formatted(col.getKeyName()), ingested); + assertEquals("column info for " + col.getKeyName(), colInfo(ingested), colInfo(col)); + } + + assertEquals("title", viaIngest.getTitle(), viaRead.getTitle()); + assertEquals("keywords", meta(viaIngest.getTableMeta().getKeywords()), meta(viaRead.getTableMeta().getKeywords())); + assertEquals("attributes", meta(viaIngest.getAttributeList()), meta(viaRead.getAttributeList())); + assertEquals("params", viaIngest.getParamInfos().size(), viaRead.getParamInfos().size()); + assertEquals("groups", viaIngest.getGroupInfos().size(), viaRead.getGroupInfos().size()); + assertEquals("resources", viaIngest.getResourceInfos().size(), viaRead.getResourceInfos().size()); + // the VOTable declares one of each, so these comparisons are 1 == 1 rather than 0 == 0 + assertEquals("params survive both paths", 1, viaRead.getParamInfos().size()); + assertEquals("groups survive both paths", 1, viaRead.getGroupInfos().size()); + assertEquals("links survive both paths", 1, viaRead.getLinkInfos().size()); + assertEquals("resources survive both paths", 1, viaRead.getResourceInfos().size()); + + assertEquals("row count", viaIngest.size(), viaRead.size()); + for (int r = 0; r < viaRead.size(); r++) { + for (DataType col : viaRead.getDataDefinitions()) { + assertEquals("%s[%d]".formatted(col.getKeyName(), r), + String.valueOf(viaIngest.getData(col.getKeyName(), r)), + String.valueOf(viaRead.getData(col.getKeyName(), r))); + } + } + } + + /** Parquet file with an embedded IVOA VOTable. */ + private static String voTableParquet() { + return FileLoader.resolveFile(DuckDbAdapterTest.class, "/iris-with-votable.parquet").getAbsolutePath(); + } + + /** Ensures schema-derived array info is preserved when absent from the VOTable. */ + @Test + public void testGetInfoUsesSourceSchema() throws DataAccessException { + String pq = voTableParquet(); + DataGroup info = DuckDbReadable.getInfo(Format.PARQUET, pq); + + assertEquals("columns come from the file", 6, info.getDataDefinitions().length); + assertEquals("row count", 150, info.size()); + + DataType flux = info.getDataDefintion("flux"); + assertNotNull("flux column exists", flux); + assertTrue("flux is an array in the file, though the VOTable does not say so", flux.isArrayType()); + assertEquals("the VOTable's meta is still applied", "Jy", flux.getUnits()); + + DataType sepalLength = info.getDataDefintion("sepal.length"); + assertNotNull("sepal.length column exists", sepalLength); + assertEquals(Double.class, sepalLength.getDataType()); + assertEquals("cm", sepalLength.getUnits()); + assertEquals("phys.size.length", sepalLength.getUCD()); + assertEquals("sepal length", sepalLength.getDesc()); + + // and read() sees the same columns it will decode the ResultSet with + DataGroup table = DuckDbReadable.read(Format.PARQUET, pq); + assertEquals(info.getDataDefinitions().length, table.getDataDefinitions().length); + assertTrue("flux stays an array", table.getDataDefintion("flux").isArrayType()); + } + + /** Ensures DD_COLS covers all supported DataType properties. */ + @Test + public void testDdColsCarriesEveryDataTypeProperty() throws Exception { + Set notCarried = Set.of( + "KeyName", // identifies the column; applying it would rename the column it is applied to + "DataType", // derived from TypeDesc, so that an unknown desc keeps the source's class + "PrefWidth"); // display only, never persisted + + DataType src = new DataType("acol", String.class); + Map expected = new LinkedHashMap<>(); + for (Method setter : DataType.class.getMethods()) { + String prop = setter.getName().startsWith("set") ? setter.getName().substring(3) : null; + if (prop == null || setter.getParameterCount() != 1 || notCarried.contains(prop)) continue; + Object val = sampleFor(setter.getParameterTypes()[0], prop); + if (val == null) continue; // no sample for this kind of property; nothing to assert + setter.invoke(src, val); + expected.put(prop, val); + } + assertTrue("found DataType properties to check", expected.size() > 15); + + DataType dest = new DataType("acol", String.class); + EmbeddedDbUtil.applyInfoToDataType(dest, src); + + List missing = new ArrayList<>(); + for (Map.Entry e : expected.entrySet()) { + Method getter = getterFor(e.getKey()); + assertNotNull("no getter for " + e.getKey(), getter); + if (!carried(e.getValue(), getter.invoke(dest))) missing.add(e.getKey()); + } + assertTrue("not carried by DD_COLS: %s -- add them to DD_COLS, or to notCarried above with a reason" + .formatted(missing), missing.isEmpty()); + } + + /** LinkInfo and friends have no equals(), so a carried collection is compared by size rather than by value */ + private static boolean carried(Object want, Object got) { + if (want instanceof Collection c) return got instanceof Collection g && g.size() == c.size(); + return want.equals(got); + } + + /** a distinctive, non-default value for a property of the given type, or null if we have no sample for it */ + private static Object sampleFor(Class type, String prop) { + if (type == String.class) return "val_" + prop; + if (type == int.class) return 42; + if (type == boolean.class) return true; + if (type == DataType.Visibility.class) return DataType.Visibility.hidden; + if (type == List.class) return List.of(new LinkInfo()); + return null; + } + + private static Method getterFor(String prop) { + for (String prefix : new String[]{"get", "is"}) { + try { return DataType.class.getMethod(prefix + prop); } catch (NoSuchMethodException ignored) {} + } + return null; + } + + /** Ensures CSV columns are correctly typed and values are preserved. */ + @Test + public void testReadAnyFormatCsv() throws Exception { + File csv = new File(ServerContext.getWorkingDir(), "readany.csv"); + Files.writeString(csv.toPath(), """ + ra,dec,name,nobs,flag + 5.5,14.02,alpha,3,true + -3.25,14.05,beta,7,false + """); + try { + DataGroup table = TableUtil.readAnyFormat(csv); + + assertEquals("rows", 2, table.size()); + assertEquals("columns", 5, table.getDataDefinitions().length); + assertEquals(Double.class, table.getDataDefintion("ra").getDataType()); + assertEquals(String.class, table.getDataDefintion("name").getDataType()); + assertEquals(Boolean.class, table.getDataDefintion("flag").getDataType()); + assertEquals(5.5, (Double) table.getData("ra", 0), 0.0001); + assertEquals("beta", table.getData("name", 1)); + assertEquals(3L, table.getData("nobs", 0)); + } finally { + csv.delete(); + } + } + + /** Ensures CSV comment lines are automatically detected. */ + @Test + public void testCommentLinesAreDetected() throws Exception { + File csv = new File(ServerContext.getWorkingDir(), "commented.csv"); + Files.writeString(csv.toPath(), """ + #Table1 + ra,dec,name + 1.5,2.5,alpha + #a comment in the middle + 3.5,4.5,beta + """); + try { + DataGroup table = DuckDbReadable.read(Format.CSV, csv.getAbsolutePath()); + + assertEquals("comment lines are not rows", 2, table.size()); + assertEquals("the header is still the header", 3, table.getDataDefinitions().length); + assertNotNull("ra column exists", table.getDataDefintion("ra")); + assertEquals(Double.class, table.getDataDefintion("ra").getDataType()); + assertEquals(3.5, (Double) table.getData("ra", 1), 0.0001); + assertEquals("beta", table.getData("name", 1)); + } finally { + csv.delete(); + } + } + + /** Ensures files under allowed directories are readable. */ + @Test + public void testReadFromSubdirOfAllowedDir() throws Exception { + File sub = new File(ServerContext.getWorkingDir(), "sub_a/sub_b"); + sub.mkdirs(); + File csv = new File(sub, "nested.csv"); + Files.writeString(csv.toPath(), "ra,dec\n1.5,2.5\n"); + try { + DataGroup table = DuckDbReadable.read(Format.CSV, csv.getAbsolutePath()); + assertEquals("a file under an allowed dir is readable", 1, table.size()); + assertEquals(1.5, (Double) table.getData("ra", 0), 0.0001); + } finally { + csv.delete(); + sub.delete(); + sub.getParentFile().delete(); + } + } + + /** Ensures remote sources are rejected. */ + @Test + public void testRemoteSourceIsRefused() { + try { + DuckDbReadable.read(Format.CSV, "https://no-such-host-xyz.example/x.csv"); + fail("a remote source should be refused while external access is off"); + } catch (DataAccessException expected) { + String msg = String.valueOf(expected.getMessage()) + expected.getCause(); + assertTrue("refused on configuration, not by a failed network call: " + msg, + msg.toLowerCase().contains("disabled") || msg.toLowerCase().contains("permission")); + } + } + + /** Ensures xtype survives the DuckDB ingest/read round-trip. */ + @Test + public void testXTypeSurvivesTheDdTable() throws Exception { + DataType ra = new DataType("ra", Double.class); + ra.setXType("adql:TIMESTAMP"); + ra.setUType("ss:my.utype"); + ra.setUnits("deg"); + DataGroup dg = new DataGroup("xtype test", List.of(ra)); + dg.add(new Object[]{1.5}); + + DuckDbAdapter db = new DuckDbAdapter(new File(ServerContext.getWorkingDir(), "xtype.duckdb")); + db.initDbFile(); + db.ingestData(() -> dg, db.getDataTable()); + DataGroup back = db.execQuery("select * from DATA", "DATA"); + db.close(true); + + DataType col = back.getDataDefintion("ra"); + assertNotNull("ra survived the ingest", col); + assertEquals("xtype survives the DD table", "adql:TIMESTAMP", col.getXType()); + assertEquals("utype still survives too", "ss:my.utype", col.getUType()); + assertEquals("deg", col.getUnits()); + } + + /** every bit of column info the two paths are expected to agree on */ + private static String colInfo(DataType dt) { + return "%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%d|%s|%s|%s|%s|%s|%s".formatted(dt.getKeyName(), dt.getDataType(), + dt.getTypeDesc(), dt.getLabel(), dt.getUnits(), dt.getUCD(), dt.getUType(), dt.getXType(), dt.getArraySize(), + dt.isArrayType(), dt.getVisibility(), dt.getWidth(), dt.getPrecision(), dt.getID(), dt.getFormat(), + dt.getNullString(), dt.getDesc(), dt.getLinkInfos().size()); + } + + private static String meta(List attribs) { + return attribs.stream().map(a -> "%s=%s(kw:%s)".formatted(a.getKey(), a.getValue(), a.isKeyword())) + .sorted().collect(Collectors.joining(", ")); + } + /** * test DuckDB decimate_key() function */ @@ -116,7 +399,7 @@ public void testDecimateKey() throws Exception { req.setInclColumns(String.format("\"ra\", \"dec\", decimate_key(\"ra\", \"dec\", %.15f, %.15f, %d, %d, %.15f, %.15f) as dkey", xMin, yMin, nXs, nYs, xUnit, yUnit)); var dgp = new SearchManager().getDataGroup(req); var dbData = dgp.getData(); - var csvData = DsvTableIO.parse(testFile, CSVFormat.DEFAULT); + var csvData = DuckDbReadable.read(Format.CSV, testFile.getAbsolutePath()); for (int i = 0; i < csvData.size(); i++) { DataObject row = csvData.get(i);