From bb5ebd71e10e2c2891b63522689b40d80e3b50e5 Mon Sep 17 00:00:00 2001 From: buczekp <buczekp> Date: Mon, 15 Feb 2010 09:05:24 +0000 Subject: [PATCH] [LMS-1385] fixed (wrong custom column numeric value shown for null value) SVN: 14727 --- .../renderer/AbstractPropertyColRenderer.java | 4 ++-- .../framework/AbstractColumnDefinition.java | 2 +- .../framework/CommonColumnDefinition.java | 2 +- .../framework/EntityPropertyColDef.java | 12 ++++------- .../GenericTableRowColumnDefinition.java | 2 +- .../specific/GridCustomColumnDefinition.java | 5 +++-- .../data/DataSetReportColumnDefinition.java | 2 +- .../calculator/GridExpressionUtils.java | 8 ++++++-- .../client/web/server/calculator/Row.java | 2 +- .../web/server/calculator/RowCalculator.java | 6 +++--- .../resultset/CachedResultSetManager.java | 20 ++++++++++++++----- .../generic/shared/basic/GridRowModel.java | 1 + .../shared/basic/IColumnDefinition.java | 4 ++-- .../shared/basic/dto/GridCustomColumn.java | 8 +++++++- .../server/calculator/RowCalculatorTest.java | 2 +- .../client/web/server/calculator/RowTest.java | 14 ++++++------- .../resultset/CachedResultSetManagerTest.java | 2 +- .../web/server/util/FilterUtilsTest.java | 2 +- .../web/server/util/TSVRendererTest.java | 2 +- 19 files changed, 59 insertions(+), 41 deletions(-) diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/model/renderer/AbstractPropertyColRenderer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/model/renderer/AbstractPropertyColRenderer.java index 5e4b46bd71f..7147548925d 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/model/renderer/AbstractPropertyColRenderer.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/model/renderer/AbstractPropertyColRenderer.java @@ -73,9 +73,9 @@ public abstract class AbstractPropertyColRenderer<T extends IEntityPropertiesHol return renderValue(entity); } - public Comparable<?> getComparableValue(GridRowModel<T> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel) { - return colDef.getComparableValue(rowModel); + return colDef.tryGetComparableValue(rowModel); } public boolean isLink() diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/AbstractColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/AbstractColumnDefinition.java index 41b66a1ae00..9d737b9ff92 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/AbstractColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/AbstractColumnDefinition.java @@ -72,7 +72,7 @@ public abstract class AbstractColumnDefinition<T> implements IColumnDefinitionUI return value != null ? value : ""; } - public Comparable<?> getComparableValue(GridRowModel<T> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel) { return getValue(rowModel); } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/CommonColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/CommonColumnDefinition.java index cbbb2d34982..a2034e20d6c 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/CommonColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/CommonColumnDefinition.java @@ -60,7 +60,7 @@ public class CommonColumnDefinition<T> extends AbstractColumnDefinition<T> } @Override - public Comparable<?> getComparableValue(GridRowModel<T> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel) { return columnDefinitionKind.getDescriptor().getComparableValue(rowModel); } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/EntityPropertyColDef.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/EntityPropertyColDef.java index b969a8a41f3..23ebed62279 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/EntityPropertyColDef.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/framework/EntityPropertyColDef.java @@ -35,10 +35,6 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PropertyType; public class EntityPropertyColDef<T extends IEntityPropertiesHolder> extends AbstractColumnDefinition<T> implements IsSerializable { - private static final Double DOUBLE_MIN_VALUE = new Double(-Double.MAX_VALUE); - - private static final Integer INTEGER_MIN_VALUE = new Integer(Integer.MIN_VALUE); - private static final int PROPERTY_COLUMN_WIDTH = 120; private static final String PROPERTY_PREFIX = "property-"; @@ -112,18 +108,18 @@ public class EntityPropertyColDef<T extends IEntityPropertiesHolder> extends } @Override - public Comparable<?> getComparableValue(GridRowModel<T> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel) { String valueAsString = tryGetValue(rowModel.getOriginalObject()); DataTypeCode dataType = getDataTypeCode(); switch (dataType) { case INTEGER: - return valueAsString == null ? INTEGER_MIN_VALUE : new Integer(valueAsString); + return valueAsString == null ? null : new Integer(valueAsString); case REAL: - return valueAsString == null ? DOUBLE_MIN_VALUE : new Double(valueAsString); + return valueAsString == null ? null : new Double(valueAsString); default: - return super.getComparableValue(rowModel); + return super.tryGetComparableValue(rowModel); } } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GenericTableRowColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GenericTableRowColumnDefinition.java index a03aee67f68..eff1fe538d5 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GenericTableRowColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GenericTableRowColumnDefinition.java @@ -44,7 +44,7 @@ public class GenericTableRowColumnDefinition implements IColumnDefinition<Generi this(null, null); } - public Comparable<?> getComparableValue(GridRowModel<GenericTableRow> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<GenericTableRow> rowModel) { return getCellValue(rowModel); } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GridCustomColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GridCustomColumnDefinition.java index f456e2c55f1..a00301beac1 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GridCustomColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/GridCustomColumnDefinition.java @@ -58,7 +58,7 @@ public class GridCustomColumnDefinition<T> implements IColumnDefinitionUI<T> return false; } - public Comparable<?> getComparableValue(GridRowModel<T> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel) { return getPrimitiveValue(rowModel).getComparableValue(); } @@ -72,7 +72,8 @@ public class GridCustomColumnDefinition<T> implements IColumnDefinitionUI<T> public String getValue(GridRowModel<T> rowModel) { - return getPrimitiveValue(rowModel).toString(); + String value = getPrimitiveValue(rowModel).toString(); + return value != null ? value : ""; } public String getHeader() diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/data/DataSetReportColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/data/DataSetReportColumnDefinition.java index 93825630e8f..2a795b7dddc 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/data/DataSetReportColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/columns/specific/data/DataSetReportColumnDefinition.java @@ -49,7 +49,7 @@ public class DataSetReportColumnDefinition implements IColumnDefinition<TableMod this.sessionID = sessionID; } - public Comparable<?> getComparableValue(GridRowModel<TableModelRow> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<TableModelRow> rowModel) { return getCellValue(rowModel); } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/GridExpressionUtils.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/GridExpressionUtils.java index 66dcd02922d..b0d24305aba 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/GridExpressionUtils.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/GridExpressionUtils.java @@ -128,8 +128,12 @@ public class GridExpressionUtils String columnId = customColumn.getCode(); RowCalculator<T> calculator = calculators.get(columnId); PrimitiveValue value = evalCustomColumn(rowData, customColumn, calculator); - customColumn.setDataType(DataTypeUtils.getCompatibleDataType(customColumn - .getDataType(), value.getDataType())); + // don't modify type if it is a null string + if (value.toString() == null) + { + customColumn.setDataType(DataTypeUtils.getCompatibleDataType(customColumn + .getDataType(), value.getDataType())); + } customColumnValues.put(columnId, value); } result.add(new GridRowModel<T>(rowData, customColumnValues)); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/Row.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/Row.java index 8f45ff5a73d..1679074e5f5 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/Row.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/Row.java @@ -72,7 +72,7 @@ final class Row<T> throw new IllegalArgumentException("Undefined column: " + columnID); } - return columnDefinition.getComparableValue(row); + return columnDefinition.tryGetComparableValue(row); } /** diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculator.java index 685def5e1a6..ae036c1dd68 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculator.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculator.java @@ -44,9 +44,9 @@ class RowCalculator<T> public RowCalculator(Set<IColumnDefinition<T>> availableColumns, String expression) { - this(availableColumns, expression, Collections.<ParameterWithValue>emptySet()); + this(availableColumns, expression, Collections.<ParameterWithValue> emptySet()); } - + public RowCalculator(Set<IColumnDefinition<T>> availableColumns, String expression, Set<ParameterWithValue> parameters) { @@ -73,7 +73,7 @@ class RowCalculator<T> return new PrimitiveValue((Double) value); } else { - return new PrimitiveValue(value.toString()); + return new PrimitiveValue(value == null ? (String) null : value.toString()); } } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManager.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManager.java index 2dac49fa4b6..f7d77beb66a 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManager.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManager.java @@ -257,9 +257,19 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se @SuppressWarnings("unchecked") public int compare(GridRowModel<T> o1, GridRowModel<T> o2) { - Comparable v1 = sortField.getComparableValue(o1); - Comparable v2 = sortField.getComparableValue(o2); - return v1.compareTo(v2); + Comparable v1 = sortField.tryGetComparableValue(o1); + Comparable v2 = sortField.tryGetComparableValue(o2); + // treat null as minimal value + if (v1 == null) + { + return -1; + } else if (v2 == null) + { + return 1; + } else + { + return v1.compareTo(v2); + } } }; return applySortDir(sortDir, comparator); @@ -335,11 +345,11 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se { K dataKey = cacheConfig.tryGetResultSetKey(); GridRowModels<T> data = fetchCachedData(dataKey); - if (data == null) // Really shoudn't happen, but these cases have been observed. + if (data == null) // Really shoudn't happen, but these cases have been observed. { return calculateResultSetAndSave(sessionToken, resultConfig, dataProvider); } - + if (mode == ResultSetFetchMode.FETCH_FROM_CACHE) { return filterLimitAndSort(resultConfig, data, dataKey); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/GridRowModel.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/GridRowModel.java index 9c1b5c9bf61..a2c941d30bf 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/GridRowModel.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/GridRowModel.java @@ -33,6 +33,7 @@ public class GridRowModel<T> implements IsSerializable { private T originalObject; + // TODO Now that we use GWT 2.0 it should be possible to use map here. // We would like to have a Map field, but we cannot do it because of a bug in // displaying serialization warnings in GWT 1.5. It was fixed in GWT 1.6 private List<GridCustomColumnValue> calculatedColumnValues; diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/IColumnDefinition.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/IColumnDefinition.java index 40a056c963e..8a8e4b7f4ac 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/IColumnDefinition.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/IColumnDefinition.java @@ -18,7 +18,6 @@ package ch.systemsx.cisd.openbis.generic.shared.basic; import com.google.gwt.user.client.rpc.IsSerializable; - /** * Describes table column's metadata. Has the ability to render cell values for the column given the * row model. @@ -30,7 +29,8 @@ public interface IColumnDefinition<T> extends IsSerializable /** extracts value for the cell of the represented column */ String getValue(GridRowModel<T> rowModel); - Comparable<?> getComparableValue(GridRowModel<T> rowModel); + /** tries to extract comparable value for the cell of the represented column */ + Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel); /** column's header */ String getHeader(); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/GridCustomColumn.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/GridCustomColumn.java index fc21d1036bd..a39a3c03b63 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/GridCustomColumn.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/GridCustomColumn.java @@ -16,7 +16,6 @@ package ch.systemsx.cisd.openbis.generic.shared.basic.dto; - /** * Stores information describing the grid custom column. * @@ -28,6 +27,7 @@ public class GridCustomColumn extends AbstractGridExpression private static final long serialVersionUID = ServiceVersionHolder.VERSION; private String code; + private DataTypeCode dataType; public String getCode() @@ -50,4 +50,10 @@ public class GridCustomColumn extends AbstractGridExpression return dataType; } + @Override + public String toString() + { + return code + "[" + dataType + "]"; + } + } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculatorTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculatorTest.java index e0f22846cd4..9e1bef58442 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculatorTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowCalculatorTest.java @@ -71,7 +71,7 @@ public class RowCalculatorTest extends AssertJUnit } @Override - public Comparable<?> getComparableValue(GridRowModel<Data> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<Data> rowModel) { return rowModel.getOriginalObject().getValue(); } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowTest.java index c707db2665a..14a2baa927a 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/calculator/RowTest.java @@ -121,10 +121,10 @@ public class RowTest extends AssertJUnit context.checking(new Expectations() { { - one(def1).getComparableValue(data1); + one(def1).tryGetComparableValue(data1); will(returnValue(getVal(data1))); - one(def2).getComparableValue(data2); + one(def2).tryGetComparableValue(data2); will(returnValue(getVal(data2))); } }); @@ -206,7 +206,7 @@ public class RowTest extends AssertJUnit one(def2).tryToGetProperty(PROPERTY_KEY); will(returnValue(PROPERTY_VALUE)); - one(def2).getComparableValue(data); + one(def2).tryGetComparableValue(data); will(returnValue(getVal(data))); } }); @@ -289,10 +289,10 @@ public class RowTest extends AssertJUnit context.checking(new Expectations() { { - one(def1).getComparableValue(data); + one(def1).tryGetComparableValue(data); will(returnValue(getVal(data))); - one(def2).getComparableValue(data); + one(def2).tryGetComparableValue(data); will(returnValue(2 * getVal(data))); } }); @@ -319,10 +319,10 @@ public class RowTest extends AssertJUnit context.checking(new Expectations() { { - one(def1).getComparableValue(data); + one(def1).tryGetComparableValue(data); will(returnValue(getVal(data))); - one(def2).getComparableValue(data); + one(def2).tryGetComparableValue(data); will(returnValue(2 * getVal(data))); } }); diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManagerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManagerTest.java index 7900b4e0ad8..36b8ec5b902 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManagerTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManagerTest.java @@ -333,7 +333,7 @@ public final class CachedResultSetManagerTest return identifier; } - public Comparable<?> getComparableValue(GridRowModel<String> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<String> rowModel) { return getValue(rowModel); } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/FilterUtilsTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/FilterUtilsTest.java index 7bd2ba4f5a7..40b48249eca 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/FilterUtilsTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/FilterUtilsTest.java @@ -78,7 +78,7 @@ public class FilterUtilsTest extends AssertJUnit } @Override - public Comparable<?> getComparableValue(GridRowModel<Data> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<Data> rowModel) { return rowModel.getOriginalObject().getValue(); } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/TSVRendererTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/TSVRendererTest.java index aff69868ed1..195793cb92c 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/TSVRendererTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/web/server/util/TSVRendererTest.java @@ -95,7 +95,7 @@ public class TSVRendererTest return rowModel.getOriginalObject()[colIx]; } - public Comparable<?> getComparableValue(GridRowModel<String[]> rowModel) + public Comparable<?> tryGetComparableValue(GridRowModel<String[]> rowModel) { return getValue(rowModel); } -- GitLab