Return-Path: Delivered-To: apmail-cxf-commits-archive@www.apache.org Received: (qmail 6799 invoked from network); 5 Mar 2011 22:38:45 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 5 Mar 2011 22:38:45 -0000 Received: (qmail 99252 invoked by uid 500); 5 Mar 2011 22:38:45 -0000 Delivered-To: apmail-cxf-commits-archive@cxf.apache.org Received: (qmail 99212 invoked by uid 500); 5 Mar 2011 22:38:45 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 99205 invoked by uid 99); 5 Mar 2011 22:38:45 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 05 Mar 2011 22:38:45 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 05 Mar 2011 22:38:43 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 961DC2388A68; Sat, 5 Mar 2011 22:38:23 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1078381 - in /cxf/trunk/rt/frontend/jaxrs/src: main/java/org/apache/cxf/jaxrs/ext/search/ test/java/org/apache/cxf/jaxrs/ext/search/ Date: Sat, 05 Mar 2011 22:38:23 -0000 To: commits@cxf.apache.org From: amichalec@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20110305223823.961DC2388A68@eris.apache.org> Author: amichalec Date: Sat Mar 5 22:38:23 2011 New Revision: 1078381 URL: http://svn.apache.org/viewvc?rev=1078381&view=rev Log: Fix: using primitive properties in type T of SimpleSearchCondtion is disallowed Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchCondition.java cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/FiqlParserTest.java cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchConditionTest.java Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchCondition.java URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchCondition.java?rev=1078381&r1=1078380&r2=1078381&view=diff ============================================================================== --- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchCondition.java (original) +++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchCondition.java Sat Mar 5 22:38:23 2011 @@ -32,7 +32,6 @@ import java.util.Set; * {@link #isMet(Object)} description. * * @param type of search condition. - * */ public class SimpleSearchCondition implements SearchCondition { @@ -47,9 +46,9 @@ public class SimpleSearchCondition im } private ConditionType joiningType = ConditionType.AND; private T condition; - + private List> scts; - + /** * Creates search condition with same operator (equality, inequality) applied in all comparison; see * {@link #isMet(Object)} for details of comparison. @@ -69,7 +68,7 @@ public class SimpleSearchCondition im } this.condition = condition; scts = createConditions(null, cType); - + } /** @@ -126,15 +125,17 @@ public class SimpleSearchCondition im } } - private List> createConditions(Map getters2operators, + private List> createConditions(Map getters2operators, ConditionType sharedType) { if (isPrimitive(condition)) { - return Collections.singletonList( - (SearchCondition)new PrimitiveSearchCondition(null, condition, sharedType, condition)); + return Collections.singletonList((SearchCondition)new PrimitiveSearchCondition(null, + condition, + sharedType, + condition)); } else { List> list = new ArrayList>(); Map get2val = getGettersAndValues(); - + for (String getter : get2val.keySet()) { ConditionType ct = getters2operators == null ? sharedType : getters2operators.get(getter); if (ct == null) { @@ -145,7 +146,7 @@ public class SimpleSearchCondition im continue; } list.add(new PrimitiveSearchCondition(getter, rval, ct, condition)); - + } if (list.isEmpty()) { throw new IllegalStateException("This search condition is empty and can not be used"); @@ -153,29 +154,32 @@ public class SimpleSearchCondition im return list; } } - + /** * Compares given object against template condition object. *

- * For primitive type T like String, Number (precisely, from type T located in subpackage of - * "java.lang.*") given object is directly compared with template object. Comparison for - * {@link ConditionType#EQUALS} requires correct implementation of {@link Object#equals(Object)}, using - * inequalities requires type T implementing {@link Comparable}. + * For built-in type T like String, Number (precisely, from type T located in subpackage of "java.lang.*") + * given object is directly compared with template object. Comparison for {@link ConditionType#EQUALS} + * requires correct implementation of {@link Object#equals(Object)}, using inequalities requires type T + * implementing {@link Comparable}. *

- * For other types comparison of given object against template object is done using these getters; - * returned "is met" value is conjunction ('and' operator) of comparisons per each getter. Getters - * of template object that return null or throw exception are not used in comparison, in extreme if all - * getters are excluded it means every given pojo object matches. If - * {@link #SimpleSearchCondition(ConditionType, Object) constructor with shared operator} was used, then - * getters are compared using the same operator. If {@link #SimpleSearchCondition(Map, Object) constructor - * with map of operators} was used then for every getter specified operator is used (getters for missing - * mapping are ignored). The way that comparison per getter is done depends on operator type per getter - - * comparison for {@link ConditionType#EQUALS} requires correct implementation of + * For other types the comparison of given object against template object is done using its + * getters; Value returned by {@linkplain #isMet(Object)} operation is conjunction ('and' + * operator) of comparisons of each getter accessible in object of type T. Getters of template object + * that return null or throw exception are not used in comparison. If type T contains getters that return + * primitive not-nullable types (as int, float etc) exception will be thrown. Finally, if all getters + * return nulls (are excluded) it is interpreted as no filter (match every pojo). + *

+ * If {@link #SimpleSearchCondition(ConditionType, Object) constructor with shared operator} was used, + * then getters are compared using the same operator. If {@link #SimpleSearchCondition(Map, Object) + * constructor with map of operators} was used then for every getter specified operator is used (getters + * for missing mapping are ignored). The way that comparison per-getter is done depending on operator type + * per getter - comparison for {@link ConditionType#EQUALS} requires correct implementation of * {@link Object#equals(Object)}, using inequalities requires that getter type implements * {@link Comparable}. *

- * For equality comparison and String type in template object (either being primitive or getter from - * complex type) it is allowed to used asterisk at the beginning or at the end of text as wild card (zero + * For equality comparison and String type in template object (either being built-in or getter from client + * provided type) it is allowed to used asterisk at the beginning or at the end of text as wild card (zero * or more of any characters) e.g. "foo*", "*foo" or "*foo*". Inner asterisks are not interpreted as wild * cards. *

@@ -189,7 +193,7 @@ public class SimpleSearchCondition im * * class Entity { * public String getName() {... - * public int getLevel() {... + * public Integer getLevel() {... * public String getMessage() {... * } * @@ -234,14 +238,25 @@ public class SimpleSearchCondition im * @return template (condition) object getters mapped to their non-null values */ private Map getGettersAndValues() { - Map getters2values = new LinkedHashMap(); Beanspector beanspector = new Beanspector(condition); for (String getter : beanspector.getGettersNames()) { + try { + if (beanspector.getAccessorType(getter).isPrimitive()) { + String beanType = beanspector.getBean().getClass().getCanonicalName(); + throw new IllegalArgumentException("Type '" + beanType + "' has property '" + getter + + "' of primitive type and " + + "cannot be used as a condition"); + } + } catch (IllegalArgumentException e) { + throw e; + } catch (Exception e) { + throw new IllegalArgumentException(e); + } Object value = getValue(beanspector, getter, condition); getters2values.put(getter, value); } - //we do not need compare class objects + // we do not need compare class objects getters2values.keySet().remove("class"); return getters2values; } @@ -258,7 +273,6 @@ public class SimpleSearchCondition im return pojo.getClass().getName().startsWith("java.lang"); } - public List findAll(Collection pojos) { List result = new ArrayList(); for (T pojo : pojos) { @@ -272,7 +286,7 @@ public class SimpleSearchCondition im public String toSQL(String table, String... columns) { return SearchUtils.toSQL(this, table, columns); } - + public PrimitiveStatement getStatement() { if (scts.size() == 1) { return scts.get(0).getStatement(); @@ -284,6 +298,5 @@ public class SimpleSearchCondition im public void accept(SearchConditionVisitor visitor) { visitor.visit(this); } - - + } Modified: cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/FiqlParserTest.java URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/FiqlParserTest.java?rev=1078381&r1=1078380&r2=1078381&view=diff ============================================================================== --- cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/FiqlParserTest.java (original) +++ cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/FiqlParserTest.java Sat Mar 5 22:38:23 2011 @@ -289,11 +289,11 @@ public class FiqlParserTest extends Asse this.name = name; } - public int getLevel() { + public Integer getLevel() { return level; } - public void setLevel(int level) { + public void setLevel(Integer level) { this.level = level; } Modified: cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java?rev=1078381&r1=1078380&r2=1078381&view=diff ============================================================================== --- cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java (original) +++ cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java Sat Mar 5 22:38:23 2011 @@ -21,11 +21,11 @@ package org.apache.cxf.jaxrs.ext.search; import java.util.ArrayList; import java.util.List; -import org.apache.cxf.jaxrs.resources.Book; import org.apache.cxf.message.Message; import org.apache.cxf.message.MessageImpl; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; public class SearchContextImplTest extends Assert { @@ -67,4 +67,48 @@ public class SearchContextImplTest exten assertEquals(1, found.size()); assertEquals(new Book("CXF Rocks", 125L), found.get(0)); } + + @Ignore + public static class Book { + private String name; + private long id; + + public Book() { + } + + public Book(String name, long id) { + this.name = name; + this.id = id; + } + + public void setName(String n) { + name = n; + } + + public String getName() { + return name; + } + + public void setId(Long i) { + id = i; + } + public Long getId() { + return id; + } + + public int hashCode() { + return name.hashCode() * 37 + new Long(id).hashCode(); + } + + public boolean equals(Object o) { + if (!(o instanceof Book)) { + return false; + } + Book other = (Book)o; + + return other.name.equals(name) && other.id == id; + + } + } + } Modified: cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchConditionTest.java URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchConditionTest.java?rev=1078381&r1=1078380&r2=1078381&view=diff ============================================================================== --- cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchConditionTest.java (original) +++ cxf/trunk/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/ext/search/SimpleSearchConditionTest.java Sat Mar 5 22:38:23 2011 @@ -332,6 +332,11 @@ public class SimpleSearchConditionTest { assertTrue(ssc.isMet("fooba*rbaz")); assertFalse(ssc.isMet("foobarbaz")); } + + @Test(expected = IllegalArgumentException.class) + public void testPrimitivePropertyOfCondition() { + new SimpleSearchCondition(ConditionType.EQUALS, new PrimitiveProp(123)); + } static class SingleAttr { private String foo; @@ -368,4 +373,22 @@ public class SimpleSearchConditionTest { return bar; } } + + static class PrimitiveProp { + private Integer foo; + + public PrimitiveProp(Integer foo) { + super(); + this.foo = foo; + } + + //primitives getter will be detected + public int getFoo() { + return foo; + } + + public void setFoo(int foo) { + this.foo = foo; + } + } }