From 7f47248bfda8f1d722e4d879c9902bed22fb3131 Mon Sep 17 00:00:00 2001
From: buczekp <buczekp>
Date: Thu, 4 Mar 2010 12:14:59 +0000
Subject: [PATCH] [LMS-1412] fixed problems with null values and sorting values
 of different types in custom columns

SVN: 15046
---
 .../specific/GridCustomColumnDefinition.java  |  2 +-
 .../calculator/GridExpressionUtils.java       |  2 +-
 .../web/server/calculator/RowCalculator.java  |  6 +-
 .../resultset/CachedResultSetManager.java     | 21 ++---
 .../generic/shared/basic/PrimitiveValue.java  | 77 +++++++++++++------
 5 files changed, 70 insertions(+), 38 deletions(-)

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 a00301beac1..c48eeb2a639 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
@@ -60,7 +60,7 @@ public class GridCustomColumnDefinition<T> implements IColumnDefinitionUI<T>
 
     public Comparable<?> tryGetComparableValue(GridRowModel<T> rowModel)
     {
-        return getPrimitiveValue(rowModel).getComparableValue();
+        return getPrimitiveValue(rowModel);
     }
 
     private PrimitiveValue getPrimitiveValue(GridRowModel<T> 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 5bdaf65151b..23388c5c3b5 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
@@ -136,7 +136,7 @@ public class GridExpressionUtils
                 RowCalculator<T> calculator = calculators.get(columnId);
                 PrimitiveValue value =
                         evalCustomColumn(rowData, customColumn, calculator, errorMessagesAreLong);
-                if (value.toString() != null)
+                if (value != PrimitiveValue.NULL)
                 {
                     customColumn.setDataType(DataTypeUtils.getCompatibleDataType(customColumn
                             .getDataType(), value.getDataType()));
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 ae036c1dd68..0ddbc4ceff0 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
@@ -65,6 +65,10 @@ class RowCalculator<T>
     public PrimitiveValue getTypedResult()
     {
         Object value = evaluator.eval();
+        if (value == null)
+        {
+            return PrimitiveValue.NULL;
+        }
         if (value instanceof Long)
         {
             return new PrimitiveValue((Long) value);
@@ -73,7 +77,7 @@ class RowCalculator<T>
             return new PrimitiveValue((Double) value);
         } else
         {
-            return new PrimitiveValue(value == null ? (String) null : value.toString());
+            return new PrimitiveValue(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 ec6133854c7..e460eb64746 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
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.collections.comparators.NullComparator;
 import org.apache.commons.collections.comparators.ReverseComparator;
 import org.apache.commons.lang.text.StrMatcher;
 import org.apache.commons.lang.text.StrTokenizer;
@@ -248,31 +249,25 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
         Collections.sort(data, comparator);
     }
 
+    @SuppressWarnings("unchecked")
     private static <T> Comparator<GridRowModel<T>> createComparator(final SortDir sortDir,
             final IColumnDefinition<T> sortField)
     {
         Comparator<GridRowModel<T>> comparator = new Comparator<GridRowModel<T>>()
             {
 
-                @SuppressWarnings("unchecked")
                 public int compare(GridRowModel<T> o1, GridRowModel<T> o2)
                 {
                     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);
-                    }
+                    // NullComparator wrapper is dealing with nulls (see below)
+                    return v1.compareTo(v2);
                 }
+
             };
-        return applySortDir(sortDir, comparator);
+
+        // null values will be treated as smallestte
+        return applySortDir(sortDir, new NullComparator(comparator, false));
     }
 
     @SuppressWarnings("unchecked")
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/PrimitiveValue.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/PrimitiveValue.java
index 581e1844d2a..68f6b1b4072 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/PrimitiveValue.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/PrimitiveValue.java
@@ -21,50 +21,86 @@ import com.google.gwt.user.client.rpc.IsSerializable;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataTypeCode;
 
 /**
- * Stores one primitive value: Double, Long or String.
+ * Stores one primitive value: Double, Long or String (null is represented as "" -
+ * {@link PrimitiveValue#NULL}).
  * <p>
- * Such a type is needed becaus GWT does not support serialization fields of Object or Serializable
+ * Such a type is needed because GWT does not support serialization fields of Object or Serializable
  * type.
  * </p>
  * 
  * @author Tomasz Pylak
  */
-public class PrimitiveValue implements IsSerializable
+public class PrimitiveValue implements IsSerializable, Comparable<PrimitiveValue>
 {
+    public static PrimitiveValue NULL = new PrimitiveValue("");
+
     private Double doubleValueOrNull;
 
     private Long longValueOrNull;
 
     private String stringValueOrNull;
 
+    private DataTypeCode dataTypeCodeOrNull;
+
     public PrimitiveValue(Double value)
     {
+        assert value != null;
         doubleValueOrNull = value;
+        dataTypeCodeOrNull = DataTypeCode.REAL;
     }
 
     public PrimitiveValue(Long value)
     {
+        assert value != null;
         longValueOrNull = value;
+        dataTypeCodeOrNull = DataTypeCode.INTEGER;
     }
 
     public PrimitiveValue(String value)
     {
+        assert value != null;
         stringValueOrNull = value;
+        dataTypeCodeOrNull = DataTypeCode.VARCHAR;
     }
-    
+
     public DataTypeCode getDataType()
+    {
+        return dataTypeCodeOrNull;
+    }
+
+    @Override
+    public String toString()
     {
         if (doubleValueOrNull != null)
         {
-            return DataTypeCode.REAL;
+            return doubleValueOrNull.toString();
+        } else if (longValueOrNull != null)
+        {
+            return longValueOrNull.toString();
+        } else
+        {
+            return stringValueOrNull;
         }
-        if (longValueOrNull != null)
+    }
+
+    @SuppressWarnings("unchecked")
+    public int compareTo(PrimitiveValue o)
+    {
+        Integer thisTypeOrdinal = getComparableDataTypeOrdinal();
+        Integer thatTypeOrdinal = o.getComparableDataTypeOrdinal();
+        int typeComparisonResult = thisTypeOrdinal.compareTo(thatTypeOrdinal);
+        if (typeComparisonResult != 0)
+        {
+            return typeComparisonResult;
+        } else
         {
-            return DataTypeCode.INTEGER;
+            Comparable v1 = getComparableValue();
+            Comparable v2 = o.getComparableValue();
+            return v1.compareTo(v2);
         }
-        return DataTypeCode.VARCHAR;
     }
-    
+
+    // Exposed for testing
     public Comparable<?> getComparableValue()
     {
         if (doubleValueOrNull != null)
@@ -72,29 +108,26 @@ public class PrimitiveValue implements IsSerializable
             return doubleValueOrNull;
         } else if (longValueOrNull != null)
         {
-            return longValueOrNull;
+            return new Double(longValueOrNull);
         } else
         {
-            return stringValueOrNull;
+            return stringValueOrNull != null ? stringValueOrNull : "";
         }
     }
 
-    @Override
-    public String toString()
+    private Integer getComparableDataTypeOrdinal()
     {
-        if (doubleValueOrNull != null)
-        {
-            return doubleValueOrNull.toString();
-        } else if (longValueOrNull != null)
+        // strings should be always smaller compared to integers and reals
+        switch (getDataType())
         {
-            return longValueOrNull.toString();
-        } else
-        {
-            return stringValueOrNull;
+            case INTEGER:
+            case REAL:
+                return 1;
+            default:
+                return 0;
         }
     }
 
-    // GWT only
     @SuppressWarnings("unused")
     private PrimitiveValue()
     {
-- 
GitLab