#660 - findPagedList() does not handle raw sql order by

src/main/java/com/avaje/ebean/OrderBy.java

@@ -221,24 +221,39 @@ public final class OrderBy<T> implements Serializable {
 
     private boolean ascending;
 
+    private String nulls;
+
+    private String highLow;
+
     public Property(String property, boolean ascending) {
       this.property = property;
       this.ascending = ascending;
     }
 
+    public Property(String property, boolean ascending, String nulls, String highLow) {
+      this.property = property;
+      this.ascending = ascending;
+      this.nulls = nulls;
+      this.highLow = highLow;
+    }
+
     /**
      * Return a copy of this Property with the path trimmed.
      */
     public Property copyWithTrim(String path) {
-      return new Property(property.substring(path.length() + 1), ascending);
+      return new Property(property.substring(path.length() + 1), ascending, nulls, highLow);
     }
 
+    @Override
     public int hashCode() {
       int hc = property.hashCode();
       hc = hc * 31 + (ascending ? 0 : 1);
+      hc = hc * 31 + (nulls == null ? 0 : nulls.hashCode());
+      hc = hc * 31 + (highLow == null ? 0 : highLow.hashCode());
       return hc;
     }
-    
+
+    @Override
     public boolean equals(Object obj) {
       if (obj == this) {
         return true;
@@ -246,10 +261,11 @@ public final class OrderBy<T> implements Serializable {
       if (!(obj instanceof Property)) {
         return false;
       }
-      
       Property e = (Property) obj;
-      return e.ascending == ascending 
-          && e.property.equals(property);
+      if (ascending != e.ascending) return false;
+      if (!property.equals(e.property)) return false;
+      if (nulls != null ? !nulls.equals(e.nulls) : e.nulls != null) return false;
+      return highLow != null ? highLow.equals(e.highLow) : e.highLow == null;
     }
 
     public String toString() {
@@ -257,10 +273,20 @@ public final class OrderBy<T> implements Serializable {
     }
 
     public String toStringFormat() {
-      if (ascending) {
-        return property;
+      if (nulls == null) {
+        if (ascending) {
+          return property;
+        } else {
+          return property + " desc";
+        }
       } else {
-        return property + " desc";
+        StringBuilder sb = new StringBuilder();
+        sb.append(property);
+        if (!ascending) {
+          sb.append(" ").append("desc");
+        }
+        sb.append(" ").append(nulls).append(" ").append(highLow);
+        return  sb.toString();
       }
     }
 
@@ -282,7 +308,7 @@ public final class OrderBy<T> implements Serializable {
      * Return a copy of this property.
      */
     public Property copy() {
-      return new Property(property, ascending);
+      return new Property(property, ascending, nulls, highLow);
     }
 
     /**
@@ -323,7 +349,6 @@ public final class OrderBy<T> implements Serializable {
 
     String[] chunks = orderByClause.split(",");
     for (int i = 0; i < chunks.length; i++) {
-
       String[] pairs = chunks[i].split(" ");
       Property p = parseProperty(pairs);
       if (p != null) {
@@ -353,8 +378,12 @@ public final class OrderBy<T> implements Serializable {
       boolean asc = isAscending(wordList.get(1));
       return new Property(wordList.get(0), asc);
     }
-    String m = "Expecting a max of 2 words in [" + Arrays.toString(pairs)
-        + "] but got " + wordList.size();
+    if (wordList.size() == 4) {
+      // nulls high or nulls low as 3rd and 4th
+      boolean asc = isAscending(wordList.get(1));
+      return new Property(wordList.get(0), asc, wordList.get(2), wordList.get(3));
+    }
+    String m = "Expecting a 1, 2 or 4 words in [" + Arrays.toString(pairs) + "] but got " + wordList;
     throw new RuntimeException(m);
   }
 

src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -3,6 +3,7 @@ package com.avaje.ebeaninternal.server.deploy;
 import com.avaje.ebean.OrderBy;
 import com.avaje.ebean.PersistenceContextScope;
 import com.avaje.ebean.Query;
+import com.avaje.ebean.RawSql;
 import com.avaje.ebean.SqlUpdate;
 import com.avaje.ebean.Transaction;
 import com.avaje.ebean.ValuePair;
@@ -2719,6 +2720,10 @@ public class BeanDescriptor<T> implements MetaBeanInfo, BeanType<T> {
     if (idProperty != null && !idProperty.isEmbedded()) {
       OrderBy<T> orderBy = query.getOrderBy();
       if (orderBy == null || orderBy.isEmpty()) {
+        RawSql rawSql = query.getRawSql();
+        if (rawSql != null) {
+          query.order(rawSql.getSql().getOrderBy());
+        }
         query.order().asc(idProperty.getName());
       } else if (!orderBy.containsProperty(idProperty.getName())){
         query.order().asc(idProperty.getName());

src/test/java/com/avaje/ebean/PropertyTest.java

@@ -0,0 +1,43 @@
+package com.avaje.ebean;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class PropertyTest {
+
+  @Test
+  public void equals() throws Exception {
+
+    assertEquals(prop("foo", true), prop("foo", true));
+  }
+
+  @Test
+  public void diff_basic() throws Exception {
+
+    assertNotEquals(prop("foo", true), prop("bar", true));
+    assertNotEquals(prop("foo", true), prop("foo", false));
+    assertNotEquals(prop("foo", false), prop("foo", true));
+  }
+
+  @Test
+  public void diff_nulls() throws Exception {
+
+    assertEquals(prop("foo", true, "nulls", "high"), prop("foo", true, "nulls", "high"));
+    assertEquals(prop("foo", true, "nulls", "low"), prop("foo", true, "nulls", "low"));
+
+    assertNotEquals(prop("foo", true), prop("foo", true, "nulls", "high"));
+    assertNotEquals(prop("foo", true, "nulls", "high"), prop("foo", true));
+    assertNotEquals(prop("foo", true, "nulls", "high"), prop("foo", true, "nulls", "low"));
+    assertNotEquals(prop("foo", true, "nulls", "low"), prop("foo", true, "nulls", "high"));
+  }
+
+  private OrderBy.Property prop(String name, boolean asc) {
+    return new OrderBy.Property(name, asc, null, null);
+  }
+
+  private OrderBy.Property prop(String name, boolean asc, String nulls, String highLow) {
+    return new OrderBy.Property(name, asc, nulls, highLow);
+  }
+
+}

src/test/java/com/avaje/tests/rawsql/TestRawSqlOrmQuery.java

@@ -3,6 +3,7 @@ package com.avaje.tests.rawsql;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
+import com.avaje.tests.model.basic.Order;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -17,6 +18,8 @@ import com.avaje.ebean.RawSqlBuilder;
 import com.avaje.tests.model.basic.Customer;
 import com.avaje.tests.model.basic.ResetBasicData;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 public class TestRawSqlOrmQuery extends BaseTestCase {
 
   @Test
@@ -107,4 +110,71 @@ public class TestRawSqlOrmQuery extends BaseTestCase {
     }
 
   }
+
+  @Test
+  public void testPaging_with_existingRawSqlOrderBy_expect_id_appendToOrderBy() {
+
+    ResetBasicData.reset();
+
+    RawSql rawSql = RawSqlBuilder.parse("select o.id, o.order_date, o.ship_date from o_order o order by o.ship_date desc nulls last")
+        .columnMapping("o.id", "id")
+        .columnMapping("o.order_date", "orderDate")
+        .columnMapping("o.ship_date", "shipDate")
+        .create();
+
+    Query<Order> query = Ebean.find(Order.class);
+    query.setUseCache(false).setUseQueryCache(false);
+    query.setRawSql(rawSql);
+
+    query.setMaxRows(100);
+    query.findList();
+
+    assertThat(query.getGeneratedSql()).contains("order by o.ship_date desc nulls last, o.id limit 100");
+  }
+
+  @Test
+  public void testPaging_when_setOrderBy_expect_id_appendToOrderBy() {
+
+    ResetBasicData.reset();
+
+    RawSql rawSql = RawSqlBuilder.parse("select o.id, o.order_date, o.ship_date from o_order o order by o.ship_date desc nulls last")
+        .columnMapping("o.id", "id")
+        .columnMapping("o.order_date", "orderDate")
+        .columnMapping("o.ship_date", "shipDate")
+        .create();
+
+    Query<Order> query = Ebean.find(Order.class);
+    query.setUseCache(false).setUseQueryCache(false);
+    query.setRawSql(rawSql);
+
+    query.setMaxRows(100);
+    query.order("coalesce(shipDate, now()) desc");
+    query.findList();
+
+    assertThat(query.getGeneratedSql()).contains("order by coalesce(o.ship_date, now()) desc, o.id limit 100");
+  }
+
+  @Test
+  public void testPaging_when_setOrderBy_containsId_expect_leaveAsIs() {
+
+    ResetBasicData.reset();
+
+    RawSql rawSql = RawSqlBuilder.parse("select o.id, o.order_date, o.ship_date from o_order o order by o.ship_date desc nulls last")
+        .columnMapping("o.id", "id")
+        .columnMapping("o.order_date", "orderDate")
+        .columnMapping("o.ship_date", "shipDate")
+        .create();
+
+    Query<Order> query = Ebean.find(Order.class);
+    query.setUseCache(false).setUseQueryCache(false);
+    query.setRawSql(rawSql);
+
+    query.setMaxRows(100);
+    query.order("id desc");
+    PagedList<Order> pagedList = query.findPagedList();
+    pagedList.getList();
+    pagedList.getTotalRowCount();
+
+    assertThat(query.getGeneratedSql()).contains("order by o.id desc limit 100");
+  }
 }

src/test/java/com/avaje/tests/unitinternal/TestOrderByParse.java

@@ -44,6 +44,28 @@ public class TestOrderByParse extends BaseTestCase {
     assertFalse(o1.containsProperty("junk"));
   }
 
+  @Test
+  public void parseNullsHigh() {
+
+    OrderBy<Object> o1 = new OrderBy<Object>("id desc nulls high");
+    assertTrue(o1.getProperties().size() == 1);
+    assertTrue(o1.getProperties().get(0).getProperty().equals("id"));
+    assertTrue(!o1.getProperties().get(0).isAscending());
+    assertTrue(o1.toStringFormat().equals("id desc nulls high"));
+  }
+
+  @Test
+  public void parseNullsHigh_with_second() {
+
+    OrderBy<Object> o1 = new OrderBy<Object>("id desc nulls high, name");
+    assertTrue(o1.getProperties().size() == 2);
+    assertTrue(o1.getProperties().get(0).getProperty().equals("id"));
+    assertTrue(!o1.getProperties().get(0).isAscending());
+    assertTrue(o1.toStringFormat().equals("id desc nulls high, name"));
+    assertTrue(o1.getProperties().get(1).getProperty().equals("name"));
+    assertTrue(o1.getProperties().get(1).isAscending());
+  }
+
   @Test
   public void testParsingTwo() {