You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Marcin Skladaniec <ma...@ish.com.au> on 2007/11/07 23:16:52 UTC

sorting and null in the path

Hi
Quite some time ago I mentioned that sorting using Ordering fails when  
there is a "null in the path". In the past we have made some  
workarounds (mostly not allowing user to sort on certain properties),  
but now we have to move forward. I put together a simple patch which  
does not affect the way cayenne works, but just adds extra  
functionality.

Ordering has now an additional field "allowNullsInThePath". When this  
field is set to true in the case of "null in the path" exception null  
is returned altogether, but the execution of the sort is not terminated.

I know that this patch is simplistic^2. I'm open to any suggestions on  
how to actually make it good. We do really need this functionality.

Marcin
PS: the patch:


Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/ObjPathResolvingException.java
===================================================================
--- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/ObjPathResolvingException.java	(revision 0)
+++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/ObjPathResolvingException.java	(revision 0)
@@ -0,0 +1,56 @@
+/*****************************************************************
+ *   Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ ****************************************************************/
+package org.apache.cayenne;
+
+/**
+ * A runtime exception thrown when <code>PropertyUtils.getProperty()</ 
code> finds that there
+ * is a null value in the resolved path.
+ *
+ * @author Marcin Skladaniec
+ */
+public class ObjPathResolvingException extends  
CayenneRuntimeException {
+
+    /**
+     * Creates new FaultFailureException without detail message.
+     */
+    public ObjPathResolvingException() {
+        super();
+    }
+
+    /**
+     * Constructs an FaultFailureException with the specified detail  
message.
+     *
+     * @param msg the detail message.
+     */
+    public ObjPathResolvingException(String msg) {
+        super(msg);
+    }
+
+    /**
+     * Constructs an FaultFailureException that wraps a  
<code>Throwable</code> thrown
+     * elsewhere.
+     */
+    public ObjPathResolvingException(Throwable th) {
+        super(th);
+    }
+
+    public ObjPathResolvingException(String msg, Throwable th) {
+        super(msg, th);
+    }
+}
Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/query/Ordering.java
===================================================================
--- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/query/Ordering.java	(revision 592920)
+++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/query/Ordering.java	(working copy)
@@ -28,6 +28,7 @@

  import org.apache.commons.collections.ComparatorUtils;
  import org.apache.cayenne.exp.Expression;
+import org.apache.cayenne.exp.ExpressionException;
  import org.apache.cayenne.util.ConversionUtil;
  import org.apache.cayenne.util.Util;
  import org.apache.cayenne.util.XMLEncoder;
@@ -58,6 +59,7 @@
      protected transient Expression sortSpec;
      protected boolean ascending;
      protected boolean caseInsensitive;
+    protected boolean allowNullsInThePath;

      /**
       * Orders a given list of objects, using a List of Orderings  
applied according the
@@ -103,6 +105,15 @@
              this.sortSpec = null;
          }
      }
+
+
+    public void setAllowNullsInThePath(boolean allowNullsInThePath) {
+        this.allowNullsInThePath = allowNullsInThePath;
+    }
+
+    public boolean getAllowNullsInThePath() {
+        return allowNullsInThePath;
+    }

      /**
       * Returns sortSpec string representation.
@@ -173,8 +184,29 @@
       */
      public int compare(Object o1, Object o2) {
          Expression exp = getSortSpec();
-        Object value1 = exp.evaluate(o1);
-        Object value2 = exp.evaluate(o2);
+		Object value1 = null;
+		Object value2 = null;
+		try {
+        	value1 = exp.evaluate(o1);
+		} catch (ExpressionException e) {
+			if (allowNullsInThePath && e.getCause() != null && e.getCause()  
instanceof org.apache.cayenne.ObjPathResolvingException) {
+				//do nothing, we expect this
+			} else {
+				//rethrow
+				throw e;
+			}
+		}
+		
+		try {
+        	value2 = exp.evaluate(o2);
+		} catch (ExpressionException e) {
+			if (allowNullsInThePath && e.getCause() != null && e.getCause()  
instanceof org.apache.cayenne.ObjPathResolvingException) {
+				//do nothing, we expect this
+			} else {
+				//rethrow
+				throw e;
+			}
+		}

          // nulls first policy... maybe make this configurable as  
some DB do
          if (value1 == null) {
Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/reflect/PropertyUtils.java
===================================================================
--- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/reflect/PropertyUtils.java	(revision 592920)
+++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/ 
cayenne/reflect/PropertyUtils.java	(working copy)
@@ -29,6 +29,7 @@
  import java.util.StringTokenizer;

  import org.apache.cayenne.CayenneRuntimeException;
+import org.apache.cayenne.ObjPathResolvingException;
  import org.apache.cayenne.map.Entity;
  import org.apache.cayenne.util.Util;

@@ -72,8 +73,7 @@

                  if (value == null) {
                      // null value in the middle....
-                    throw new CayenneRuntimeException(
-                            "Null value in the middle of the path");
+                    throw new ObjPathResolvingException("Null value  
in the middle of the path, failed on"+nestedPropertyName +" from  
"+object);
                  }

                  value = getSimpleProperty(value, pathSegment);

  

Re: sorting and null in the path

Posted by Andrus Adamchik <an...@objectstyle.org>.
My other purely semantic suggestion would be to move  
ObjPathResolvingException out of the root package. Maybe to the  
'reflect' package? And maybe call it something more user friendly  
(not sure what exactly ... NullPathException, ???)

Andrus

On Nov 8, 2007, at 12:44 AM, Kevin Menard wrote:

> Ahh, I hadn't even noticed the domain.  Cool.
>
> I generally like the idea.  Although, it would be nicer to have a  
> bit more
> control.  E.g., discard nulls or group them all together at the end.
>
> As a suggested code cleanup, the following:
>
> e.getCause() != null && e.getCause()
> instanceof org.apache.cayenne.ObjPathResolvingException
>
> Could be simplified to just the instanceof operation (it's null  
> safe and
> returns false if the argument is null).
>
> -- 
> Kevin
>
>
> On 11/7/07 5:28 PM, "Aristedes Maniatis" <ar...@maniatis.org> wrote:
>
>>
>> On 08/11/2007, at 9:22 AM, Kevin Menard wrote:
>>
>>> Please submit the patch through JIRA.  By submitting through JIRA
>>> you can
>>> click a button that grants the ASF a non-exclusive license to use
>>> the code.
>>> This is a prerequisite for the code to get committed.
>>
>>
>> I'll take care of all that with Marcin and get him to sign a CLA. We
>> should be expecting more contributions from Marcin in the future.
>>
>> I'll also work with him to clean up the code, the naming and comment
>> it a bit, but what do you think of the concept? An extra switch in
>> Ordering which allows us to ignore paths that don't full resolve? Is
>> there a better way?
>>
>> Ari
>
>
>


Re: sorting and null in the path

Posted by Kevin Menard <km...@servprise.com>.
Ahh, I hadn't even noticed the domain.  Cool.

I generally like the idea.  Although, it would be nicer to have a bit more
control.  E.g., discard nulls or group them all together at the end.

As a suggested code cleanup, the following:

e.getCause() != null && e.getCause()
instanceof org.apache.cayenne.ObjPathResolvingException

Could be simplified to just the instanceof operation (it's null safe and
returns false if the argument is null).

-- 
Kevin


On 11/7/07 5:28 PM, "Aristedes Maniatis" <ar...@maniatis.org> wrote:

> 
> On 08/11/2007, at 9:22 AM, Kevin Menard wrote:
> 
>> Please submit the patch through JIRA.  By submitting through JIRA
>> you can
>> click a button that grants the ASF a non-exclusive license to use
>> the code.
>> This is a prerequisite for the code to get committed.
> 
> 
> I'll take care of all that with Marcin and get him to sign a CLA. We
> should be expecting more contributions from Marcin in the future.
> 
> I'll also work with him to clean up the code, the naming and comment
> it a bit, but what do you think of the concept? An extra switch in
> Ordering which allows us to ignore paths that don't full resolve? Is
> there a better way?
> 
> Ari



Re: sorting and null in the path

Posted by Aristedes Maniatis <ar...@maniatis.org>.
On 08/11/2007, at 9:22 AM, Kevin Menard wrote:

> Please submit the patch through JIRA.  By submitting through JIRA  
> you can
> click a button that grants the ASF a non-exclusive license to use  
> the code.
> This is a prerequisite for the code to get committed.


I'll take care of all that with Marcin and get him to sign a CLA. We  
should be expecting more contributions from Marcin in the future.

I'll also work with him to clean up the code, the naming and comment  
it a bit, but what do you think of the concept? An extra switch in  
Ordering which allows us to ignore paths that don't full resolve? Is  
there a better way?

Ari


-------------------------->
Aristedes Maniatis
phone +61 2 9660 9700
PGP fingerprint 08 57 20 4B 80 69 59 E2  A9 BF 2D 48 C2 20 0C C8



Re: sorting and null in the path

Posted by Kevin Menard <km...@servprise.com>.
Hi Marcin,

Please submit the patch through JIRA.  By submitting through JIRA you can
click a button that grants the ASF a non-exclusive license to use the code.
This is a prerequisite for the code to get committed.

-- 
Kevin


On 11/7/07 5:16 PM, "Marcin Skladaniec" <ma...@ish.com.au> wrote:

> Hi
> Quite some time ago I mentioned that sorting using Ordering fails when
> there is a "null in the path". In the past we have made some
> workarounds (mostly not allowing user to sort on certain properties),
> but now we have to move forward. I put together a simple patch which
> does not affect the way cayenne works, but just adds extra
> functionality.
> 
> Ordering has now an additional field "allowNullsInThePath". When this
> field is set to true in the case of "null in the path" exception null
> is returned altogether, but the execution of the sort is not terminated.
> 
> I know that this patch is simplistic^2. I'm open to any suggestions on
> how to actually make it good. We do really need this functionality.
> 
> Marcin
> PS: the patch:
> 
> 
> Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/ObjPathResolvingException.java
> ===================================================================
> --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/ObjPathResolvingException.java (revision 0)
> +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/ObjPathResolvingException.java (revision 0)
> @@ -0,0 +1,56 @@
> +/*****************************************************************
> + *   Licensed to the Apache Software Foundation (ASF) under one
> + *  or more contributor license agreements.  See the NOTICE file
> + *  distributed with this work for additional information
> + *  regarding copyright ownership.  The ASF licenses this file
> + *  to you under the Apache License, Version 2.0 (the
> + *  "License"); you may not use this file except in compliance
> + *  with the License.  You may obtain a copy of the License at
> + *
> + *    http://www.apache.org/licenses/LICENSE-2.0
> + *
> + *  Unless required by applicable law or agreed to in writing,
> + *  software distributed under the License is distributed on an
> + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + *  KIND, either express or implied.  See the License for the
> + *  specific language governing permissions and limitations
> + *  under the License.
> + ****************************************************************/
> +package org.apache.cayenne;
> +
> +/**
> + * A runtime exception thrown when <code>PropertyUtils.getProperty()</
> code> finds that there
> + * is a null value in the resolved path.
> + *
> + * @author Marcin Skladaniec
> + */
> +public class ObjPathResolvingException extends
> CayenneRuntimeException {
> +
> +    /**
> +     * Creates new FaultFailureException without detail message.
> +     */
> +    public ObjPathResolvingException() {
> +        super();
> +    }
> +
> +    /**
> +     * Constructs an FaultFailureException with the specified detail
> message.
> +     *
> +     * @param msg the detail message.
> +     */
> +    public ObjPathResolvingException(String msg) {
> +        super(msg);
> +    }
> +
> +    /**
> +     * Constructs an FaultFailureException that wraps a
> <code>Throwable</code> thrown
> +     * elsewhere.
> +     */
> +    public ObjPathResolvingException(Throwable th) {
> +        super(th);
> +    }
> +
> +    public ObjPathResolvingException(String msg, Throwable th) {
> +        super(msg, th);
> +    }
> +}
> Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/query/Ordering.java
> ===================================================================
> --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/query/Ordering.java (revision 592920)
> +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/query/Ordering.java (working copy)
> @@ -28,6 +28,7 @@
> 
>   import org.apache.commons.collections.ComparatorUtils;
>   import org.apache.cayenne.exp.Expression;
> +import org.apache.cayenne.exp.ExpressionException;
>   import org.apache.cayenne.util.ConversionUtil;
>   import org.apache.cayenne.util.Util;
>   import org.apache.cayenne.util.XMLEncoder;
> @@ -58,6 +59,7 @@
>       protected transient Expression sortSpec;
>       protected boolean ascending;
>       protected boolean caseInsensitive;
> +    protected boolean allowNullsInThePath;
> 
>       /**
>        * Orders a given list of objects, using a List of Orderings
> applied according the
> @@ -103,6 +105,15 @@
>               this.sortSpec = null;
>           }
>       }
> +
> +
> +    public void setAllowNullsInThePath(boolean allowNullsInThePath) {
> +        this.allowNullsInThePath = allowNullsInThePath;
> +    }
> +
> +    public boolean getAllowNullsInThePath() {
> +        return allowNullsInThePath;
> +    }
> 
>       /**
>        * Returns sortSpec string representation.
> @@ -173,8 +184,29 @@
>        */
>       public int compare(Object o1, Object o2) {
>           Expression exp = getSortSpec();
> -        Object value1 = exp.evaluate(o1);
> -        Object value2 = exp.evaluate(o2);
> +  Object value1 = null;
> +  Object value2 = null;
> +  try {
> +         value1 = exp.evaluate(o1);
> +  } catch (ExpressionException e) {
> +   if (allowNullsInThePath && e.getCause() != null && e.getCause()
> instanceof org.apache.cayenne.ObjPathResolvingException) {
> +    //do nothing, we expect this
> +   } else {
> +    //rethrow
> +    throw e;
> +   }
> +  }
> +  
> +  try {
> +         value2 = exp.evaluate(o2);
> +  } catch (ExpressionException e) {
> +   if (allowNullsInThePath && e.getCause() != null && e.getCause()
> instanceof org.apache.cayenne.ObjPathResolvingException) {
> +    //do nothing, we expect this
> +   } else {
> +    //rethrow
> +    throw e;
> +   }
> +  }
> 
>           // nulls first policy... maybe make this configurable as
> some DB do
>           if (value1 == null) {
> Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/reflect/PropertyUtils.java
> ===================================================================
> --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/reflect/PropertyUtils.java (revision 592920)
> +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
> cayenne/reflect/PropertyUtils.java (working copy)
> @@ -29,6 +29,7 @@
>   import java.util.StringTokenizer;
> 
>   import org.apache.cayenne.CayenneRuntimeException;
> +import org.apache.cayenne.ObjPathResolvingException;
>   import org.apache.cayenne.map.Entity;
>   import org.apache.cayenne.util.Util;
> 
> @@ -72,8 +73,7 @@
> 
>                   if (value == null) {
>                       // null value in the middle....
> -                    throw new CayenneRuntimeException(
> -                            "Null value in the middle of the path");
> +                    throw new ObjPathResolvingException("Null value
> in the middle of the path, failed on"+nestedPropertyName +" from
> "+object);
>                   }
> 
>                   value = getSimpleProperty(value, pathSegment);
> 
>   

--