From 9c4ec388b0d108ad12c4d2c90cf1654baf3e14b1 Mon Sep 17 00:00:00 2001 From: tatianag Date: Sat, 4 Mar 2017 21:00:29 -0800 Subject: [PATCH 1/3] DM-9590 XY plot is unrecoverable after it fails --- .../ipac/firefly/server/util/QueryUtil.java | 4 +- src/firefly/js/charts/dataTypes/XYColsCDT.js | 80 ++++++++++++------- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java b/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java index 11949886af..93b823eef0 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java @@ -623,7 +623,7 @@ public static DataAccessException createEndUserException(String msg, String deta * @param decimateInfo DecimateInfo object * @return decimated data group */ - public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) { + public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) throws DataAccessException { double xMax = Double.NEGATIVE_INFINITY, xMin = Double.POSITIVE_INFINITY, yMax = Double.NEGATIVE_INFINITY, yMin = Double.POSITIVE_INFINITY; @@ -633,7 +633,7 @@ public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) { if (!xValGetter.isValid() || !yValGetter.isValid()) { System.out.println("QueryUtil.doDecimation: invalid x or y column."); - return null; // TODO: handle null return in the caller? + throw new DataAccessException("Invalid column or expression"); } int maxPoints = decimateInfo.getMaxPoints() == 0 ? DECI_DEF_MAX_POINTS : decimateInfo.getMaxPoints(); diff --git a/src/firefly/js/charts/dataTypes/XYColsCDT.js b/src/firefly/js/charts/dataTypes/XYColsCDT.js index 1b58511121..7fcce5d9e4 100644 --- a/src/firefly/js/charts/dataTypes/XYColsCDT.js +++ b/src/firefly/js/charts/dataTypes/XYColsCDT.js @@ -219,20 +219,19 @@ function serverParamsChanged(oldParams, newParams, chartDataElement) { } else { // 'x', 'y', 'sortBy', 'xErr', 'xErrLow', 'xErrHigh', 'yErr', 'yErrLow', 'yErrHigh' // server call parameters are present in the data - const newOpts = omitBy({ - sortBy: newParams.sortColOrExpr, - x: newParams.x.columnOrExpr, - xErr: newParams.x.error, - xErrLow: newParams.x.errorLow, - xErrHigh: newParams.x.errorHigh, - y: newParams.y.columnOrExpr, - yErr: newParams.y.error, - yErrLow: newParams.y.errorLow, - yErrHigh: newParams.y.errorHigh - }, isUndefined); - return Object.keys(newOpts).some((o) => { - return newOpts[o] !== data[o]; - }); + const newOpts = getServerCallParameters(newParams, false); + if (data) { + // if data available, see if the new parameters are different from those used to obtain the data + return Object.keys(newOpts).some((o) => { + return newOpts[o] !== data[o]; + }); + } else { + // if data are not available, compare with the old parameters + const oldOpts = getServerCallParameters(oldParams, false); + return Object.keys(newOpts).some((o) => { + return newOpts[o] !== oldOpts[o]; + }); + } } } @@ -249,22 +248,39 @@ function isLargeTable(tblId) { // xyPlotParams.x.errorHigh || xyPlotParams.y.errorHigh); //} -function getServerCallParameters(xyPlotParams) { - if (!xyPlotParams) { return []; } +function getServerCallParameters(xyPlotParams, isLargeTable=true) { + if (isLargeTable) { + if (!xyPlotParams) { + return []; + } - if (xyPlotParams.zoom) { - var {xMin, xMax, yMin, yMax} = xyPlotParams.zoom; - } + if (xyPlotParams.zoom) { + var {xMin, xMax, yMin, yMax} = xyPlotParams.zoom; + } - let maxBins = 10000; - let xyRatio = xyPlotParams.xyRatio || 1.0; - if (xyPlotParams.nbins) { - const {x, y} = xyPlotParams.nbins; - maxBins = x*y; - xyRatio = x/y; + let maxBins = 10000; + let xyRatio = xyPlotParams.xyRatio || 1.0; + if (xyPlotParams.nbins) { + const {x, y} = xyPlotParams.nbins; + maxBins = x * y; + xyRatio = x / y; + } + // order should match the order of the parameters in serializeDecimateInfo + return [xyPlotParams.x.columnOrExpr, xyPlotParams.y.columnOrExpr, maxBins, xyRatio, xMin, xMax, yMin, yMax]; + } else { + // smaller (not decimated) table + return omitBy({ + sortBy: xyPlotParams.sortColOrExpr, + x: xyPlotParams.x.columnOrExpr, + xErr: xyPlotParams.x.error, + xErrLow: xyPlotParams.x.errorLow, + xErrHigh: xyPlotParams.x.errorHigh, + y: xyPlotParams.y.columnOrExpr, + yErr: xyPlotParams.y.error, + yErrLow: xyPlotParams.y.errorLow, + yErrHigh: xyPlotParams.y.errorHigh + }, isUndefined); } - // order should match the order of the parameters in serializeDecimateInfo - return [xyPlotParams.x.columnOrExpr, xyPlotParams.y.columnOrExpr, maxBins, xyRatio, xMin, xMax, yMin, yMax]; } export function getDataBoundaries(xyPlotData) { @@ -574,12 +590,20 @@ function fetchXYWithErrorsOrSort(dispatch, chartId, chartDataElementId) { function dispatchError(dispatch, chartId, chartDataElementId, reason) { const message = 'Failed to fetch XY plot data'; logError(`${message}: ${reason}`); + let reasonStr = `${reason}`.toLowerCase(); + if (reasonStr.match(/not supported/)) { + reasonStr = 'Unsupported feature requested. Please choose valid options.'; + } else if (reasonStr.match(/invalid column/)) { + reasonStr = 'Invalid column or expression. Please choose valid X and Y.'; + } else { + reasonStr = 'Please check console for more information.'; + } dispatch(chartDataUpdate( { chartId, chartDataElementId, isDataReady: true, - error: {message, reason}, + error: {message, reason: reasonStr}, data: undefined })); } From 636608d30891283d24814051907c88f8a063f391 Mon Sep 17 00:00:00 2001 From: tatianag Date: Fri, 10 Mar 2017 18:58:09 -0800 Subject: [PATCH 2/3] Fix error message and column field not being validated for histogram. --- .../server/query/HistogramProcessor.java | 5 +++- .../js/charts/dataTypes/HistogramCDT.js | 29 ++++++++++++------- src/firefly/js/charts/dataTypes/XYColsCDT.js | 4 +-- .../js/charts/ui/ColumnOrExpression.jsx | 27 ++++++++++++++++- src/firefly/js/charts/ui/HistogramOptions.jsx | 14 ++++++--- src/firefly/js/charts/ui/XYPlotOptions.jsx | 23 ++------------- 6 files changed, 63 insertions(+), 39 deletions(-) diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/query/HistogramProcessor.java b/src/firefly/java/edu/caltech/ipac/firefly/server/query/HistogramProcessor.java index cfa9859efd..d65f2471b2 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/query/HistogramProcessor.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/query/HistogramProcessor.java @@ -306,11 +306,14 @@ private boolean isInSelection( int idx, ArrayList list){ return false; } - private double[] getColumnData(DataGroup dg) { + private double[] getColumnData(DataGroup dg) throws DataAccessException { List objList = dg.values(); int nRow = objList.size(); DataType[] dataTypes=dg.getDataDefinitions(); DataObjectUtil.DoubleValueGetter dGetter = new DataObjectUtil.DoubleValueGetter(dataTypes, columnExpression); + if (!dGetter.isValid()) { + throw new DataAccessException("Invalid column or expression: "+columnExpression); + } double[] data = new double[nRow]; for (int i = 0; i < nRow; i++) { diff --git a/src/firefly/js/charts/dataTypes/HistogramCDT.js b/src/firefly/js/charts/dataTypes/HistogramCDT.js index afff713d1e..bd34ffc0b5 100644 --- a/src/firefly/js/charts/dataTypes/HistogramCDT.js +++ b/src/firefly/js/charts/dataTypes/HistogramCDT.js @@ -169,16 +169,25 @@ function fetchColData(dispatch, chartId, chartDataElementId) { } ).catch( (reason) => { - const message = 'Failed to fetch histogram data'; - logError(`${message}: ${reason}`); - dispatch(chartDataUpdate( - { - chartId, - chartDataElementId, - isDataReady: true, - error: {message, reason}, - data: undefined - })); + dispatchError(dispatch, chartId, chartDataElementId, reason); } ); } +function dispatchError(dispatch, chartId, chartDataElementId, reason) { + const message = 'Failed to fetch histogram data'; + logError(`${message}: ${reason}`); + let reasonStr = `${reason}`.toLowerCase(); + if (reasonStr.match(/invalid column/)) { + reasonStr = 'Non-existent column or invalid expression. Please use valid value.'; + } else { + reasonStr = 'Please contact Help Desk. Check browser console for more information.'; + } + dispatch(chartDataUpdate( + { + chartId, + chartDataElementId, + isDataReady: true, + error: {message, reason: reasonStr}, + data: undefined + })); +} \ No newline at end of file diff --git a/src/firefly/js/charts/dataTypes/XYColsCDT.js b/src/firefly/js/charts/dataTypes/XYColsCDT.js index 7fcce5d9e4..d80da0e546 100644 --- a/src/firefly/js/charts/dataTypes/XYColsCDT.js +++ b/src/firefly/js/charts/dataTypes/XYColsCDT.js @@ -594,9 +594,9 @@ function dispatchError(dispatch, chartId, chartDataElementId, reason) { if (reasonStr.match(/not supported/)) { reasonStr = 'Unsupported feature requested. Please choose valid options.'; } else if (reasonStr.match(/invalid column/)) { - reasonStr = 'Invalid column or expression. Please choose valid X and Y.'; + reasonStr = 'Non-existent column or invalid expression. Please choose valid X and Y.'; } else { - reasonStr = 'Please check console for more information.'; + reasonStr = 'Please contact Help Desk. Check browser console for more information.'; } dispatch(chartDataUpdate( { diff --git a/src/firefly/js/charts/ui/ColumnOrExpression.jsx b/src/firefly/js/charts/ui/ColumnOrExpression.jsx index 3c106aed1b..ebd6743046 100644 --- a/src/firefly/js/charts/ui/ColumnOrExpression.jsx +++ b/src/firefly/js/charts/ui/ColumnOrExpression.jsx @@ -4,6 +4,7 @@ import React, {PropTypes} from 'react'; import {get} from 'lodash'; +import {Expression} from '../../util/expr/Expression.js'; import {dispatchValueChange} from '../../fieldGroup/FieldGroupCntlr.js'; import {TextButton} from '../../ui/TextButton.jsx'; import {SuggestBoxInputField} from '../../ui/SuggestBoxInputField.jsx'; @@ -29,6 +30,24 @@ function parseSuggestboxContent(text) { return {token, priorContent}; } +export function getColValidator(colValStats, required=true) { + const colNames = colValStats.map((colVal) => {return colVal.name;}); + return (val) => { + let retval = {valid: true, message: ''}; + if (!val) { + if (required) { + return {valid: false, message: 'Can not be empty. Please provide value or expression'}; + } + } else if (colNames.indexOf(val) < 0) { + const expr = new Expression(val, colNames); + if (!expr.isValid()) { + retval = {valid: false, message: `${expr.getError().error}. Unable to parse ${val}.`}; + } + } + return retval; + }; +} + export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,labelWidth=30,tooltip,nullAllowed}) { // the suggestions are indexes in the colValStats array - it makes it easier to render then with labels @@ -55,12 +74,18 @@ export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,la val = colName; dispatchValueChange({fieldKey: fldPath, groupKey, value: colName, valid: true}); }; + const colValidator = getColValidator(colValStats,!nullAllowed); + const value = get(params, fldPath); + const {valid=true, message=''} = value ? colValidator(value) : {}; return (
el.value === algorithm) : algorithmOptions; return ( diff --git a/src/firefly/js/charts/ui/XYPlotOptions.jsx b/src/firefly/js/charts/ui/XYPlotOptions.jsx index 3a1a9cfc6b..017c783893 100644 --- a/src/firefly/js/charts/ui/XYPlotOptions.jsx +++ b/src/firefly/js/charts/ui/XYPlotOptions.jsx @@ -11,14 +11,13 @@ import {FieldGroup} from '../../ui/FieldGroup.jsx'; import FieldGroupUtils from '../../fieldGroup/FieldGroupUtils.js'; import {dispatchMultiValueChange, VALUE_CHANGE, MULTI_VALUE_CHANGE} from '../../fieldGroup/FieldGroupCntlr.js'; import Validate from '../../util/Validate.js'; -import {Expression} from '../../util/expr/Expression.js'; import {ValidationField} from '../../ui/ValidationField.jsx'; import {CheckboxGroupInputField} from '../../ui/CheckboxGroupInputField.jsx'; import {RadioGroupInputField} from '../../ui/RadioGroupInputField.jsx'; import {FieldGroupCollapsible} from '../../ui/panel/CollapsiblePanel.jsx'; import {plotParamsShape} from './XYPlot.jsx'; import {hideColSelectPopup} from './ColSelectView.jsx'; -import {ColumnOrExpression} from './ColumnOrExpression.jsx'; +import {ColumnOrExpression, getColValidator} from './ColumnOrExpression.jsx'; import {updateSet} from '../../util/WebUtil.js'; const DECI_ENABLE_SIZE = 5000; @@ -113,7 +112,7 @@ export function resultsSuccess(callback, flds, optionParameters) { x : { columnOrExpr : xName, error: xErr, label : xLabel, unit : xUnit, options : xOptions}, y : { columnOrExpr : yName, error: yErr, label : yLabel, unit : yUnit, options : yOptions}, tblId, - zoom, + zoom }, isUndefined); if (xErr || yErr) { @@ -155,24 +154,6 @@ export function setOptions(groupKey, xyPlotParams) { dispatchMultiValueChange(groupKey, flds); } -export function getColValidator(colValStats, required=true) { - const colNames = colValStats.map((colVal) => {return colVal.name;}); - return (val) => { - let retval = {valid: true, message: ''}; - if (!val) { - if (required) { - return {valid: false, message: 'Can not be empty. Please provide value or expression'}; - } - } else if (colNames.indexOf(val) < 0) { - const expr = new Expression(val, colNames); - if (!expr.isValid()) { - retval = {valid: false, message: `${expr.getError().error}. Unable to parse ${val}.`}; - } - } - return retval; - }; -} - /** * Reducer from field group component, * clears label, unit, and userSetBoundaries whenever x or y field changes, From 5b8c1fc525953150d62dcd202d48d8877a26b789 Mon Sep 17 00:00:00 2001 From: tatianag Date: Mon, 13 Mar 2017 09:17:52 -0700 Subject: [PATCH 3/3] - invalid initial value should produce warning - full height suggest box rendering should not cause flicker --- src/firefly/js/charts/ui/HistogramOptions.jsx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/firefly/js/charts/ui/HistogramOptions.jsx b/src/firefly/js/charts/ui/HistogramOptions.jsx index ffc8ca7e2f..044d6f9765 100644 --- a/src/firefly/js/charts/ui/HistogramOptions.jsx +++ b/src/firefly/js/charts/ui/HistogramOptions.jsx @@ -86,10 +86,16 @@ export class HistogramOptions extends React.Component { } }); // make sure column validator matches current columns - const {colValStats, groupKey} = this.props; + const {colValStats, groupKey, histogramParams} = this.props; if (colValStats) { const colValidator = getColValidator(colValStats); - dispatchValueChange({groupKey, fieldKey: 'columnOrExpr', validator: colValidator}); + var payload = {groupKey, fieldKey: 'columnOrExpr', validator: colValidator}; + const value = get(histogramParams, 'columnOrExpr'); + if (value) { + var {valid, message} = colValidator(value); + payload = Object.assign(payload, {value, valid, message}); + } + dispatchValueChange(payload); } this.iAmMounted= true; } @@ -142,8 +148,10 @@ export class HistogramOptions extends React.Component { const algorithm = get(histogramParams, 'algorithm', 'fixedSizeBins'); const m_algorithmOptions = fixedAlgorithm ? algorithmOptions.filter((el) => el.value === algorithm) : algorithmOptions; + // set minimum height to fit full height suggest box, + // to avoid width change due to scroll bar appearing when full height suggest box is rendered return ( -
+
{onOptionsSelected &&