You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2008/10/23 00:22:03 UTC

svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Author: jleroux
Date: Wed Oct 22 15:22:00 2008
New Revision: 707216

URL: http://svn.apache.org/viewvc?rev=707216&view=rev
Log:
Merged and modified Oscar Pablo's patch from "Data filtering in entity views" (https://issues.apache.org/jira/browse/OFBIZ-1232) - OFBIZ-1232
I put documentation in entitymodel.xsd, removed the need for an iterator, and strengthened makeViewFilterWhereClause.

Modified:
    ofbiz/trunk/framework/entity/dtd/entitymodel.xsd
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/DynamicViewEntity.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java

Modified: ofbiz/trunk/framework/entity/dtd/entitymodel.xsd
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/dtd/entitymodel.xsd?rev=707216&r1=707215&r2=707216&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/dtd/entitymodel.xsd (original)
+++ ofbiz/trunk/framework/entity/dtd/entitymodel.xsd Wed Oct 22 15:22:00 2008
@@ -245,6 +245,7 @@
                 <xs:element minOccurs="0" maxOccurs="unbounded" ref="alias-all"/>
                 <xs:element minOccurs="0" maxOccurs="unbounded" ref="alias"/>
                 <xs:element minOccurs="0" maxOccurs="unbounded" ref="view-link"/>
+                <xs:element minOccurs="0" maxOccurs="unbounded" ref="filter"/>
                 <xs:element minOccurs="0" maxOccurs="unbounded" ref="relation"/>
             </xs:sequence>
             <xs:attributeGroup ref="attlist.view-entity"/>
@@ -440,6 +441,26 @@
             </xs:simpleType>
         </xs:attribute>
     </xs:attributeGroup>
+    <xs:element name="filter">
+        <xs:annotation>
+            <xs:documentation>
+                Allows to filter a view using "entity-alias", "field-name", "operator" and "value"   
+                operator being one of (see EntityOperator.java for more details)
+                {not-in, greater, greaterThanEqualTo, less-equals, equals, or, greater-than, less-than-equal-to, like, in, not-equals,
+                greater-than-equal-to, and, not, lessThanEqualTo, not-equal, lessThan, greater-equals, greaterThan, not-like, less-than,
+                notEqual, between, less}
+            </xs:documentation>
+        </xs:annotation>
+        <xs:complexType>
+            <xs:attributeGroup ref="attlist.filter"/>
+        </xs:complexType>
+    </xs:element>
+    <xs:attributeGroup name="attlist.filter">
+        <xs:attribute type="xs:string" name="entity-alias" use="required"/>
+        <xs:attribute type="xs:string" name="field-name" use="required"/>
+        <xs:attribute type="xs:string" name="operator" use="required"/>
+        <xs:attribute type="xs:string" name="value" use="required"/>
+    </xs:attributeGroup>
 
     <xs:element name="extend-entity">
         <xs:complexType>

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=707216&r1=707215&r2=707216&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java Wed Oct 22 15:22:00 2008
@@ -26,10 +26,10 @@
 import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.sql.Blob;
+import java.sql.Clob;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
-import java.sql.Clob;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -53,6 +53,7 @@
 import org.ofbiz.entity.GenericNotImplementedException;
 import org.ofbiz.entity.GenericValue;
 import org.ofbiz.entity.condition.EntityConditionParam;
+import org.ofbiz.entity.condition.EntityOperator;
 import org.ofbiz.entity.condition.OrderByList;
 import org.ofbiz.entity.config.DatasourceInfo;
 import org.ofbiz.entity.datasource.GenericDAO;
@@ -355,11 +356,45 @@
                 throw new GenericModelException("The join-style " + joinStyle + " is not supported");
             }
 
+            makeViewFilterWhereClause(modelViewEntity, whereString);
+            
             if (whereString.length() > 0) {
                 return "(" + whereString.toString() + ")";
             }
         }
         return "";
+    }    
+
+    /** Filter a view using this syntax : 
+     * <filter entity-alias="<table_alias>" field-name="<field_name>" operator="<operator:equals, not-equals, like...>" value="<value_to_select>"/> 
+     * */
+    public static void makeViewFilterWhereClause(ModelViewEntity modelViewEntity, StringBuilder whereString) throws GenericEntityException {
+
+        for (ModelViewEntity.ModelFilter filter : modelViewEntity.getFilters()) {            
+            ModelEntity filterEntity = modelViewEntity.getMemberModelEntity(filter.getEntityAlias());
+            if (filterEntity == null) {
+                throw new GenericEntityException("Link entity not found with alias: " + filter.getEntityAlias() + " for entity: " + modelViewEntity.getEntityName());
+            }
+            
+            ModelField filterField = filterEntity.getField(filter.getFieldName());
+            if (filterField == null) {
+                throw new GenericEntityException("The field " + filter.getFieldName() + " does not appear to belong to entity " + modelViewEntity.getEntityName());                
+            }
+            if (whereString.length() > 0) {
+                whereString.append(" AND ");
+            }
+            whereString.append(filter.getEntityAlias());
+            whereString.append(".");
+            whereString.append(filterColName(filterField.getColName()));
+            
+            EntityOperator<?> entityOperator = EntityOperator.lookup(filter.getOperator());
+            if (entityOperator == null) {
+            	throw new GenericEntityException("Operator " + filter.getOperator() + " not supported in filter for entity: " + modelViewEntity.getEntityName());
+            }
+            whereString.append(" ").append(entityOperator.getCode()).append(" ");
+            
+            whereString.append("'" + filter.getValue().replaceAll("'", "''") + "'");
+        }
     }
 
     public static String makeOrderByClause(ModelEntity modelEntity, List<String> orderBy, DatasourceInfo datasourceInfo) throws GenericModelException {

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/DynamicViewEntity.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/DynamicViewEntity.java?rev=707216&r1=707215&r2=707216&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/DynamicViewEntity.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/DynamicViewEntity.java Wed Oct 22 15:22:00 2008
@@ -28,6 +28,7 @@
 import org.ofbiz.entity.model.ModelViewEntity.ComplexAliasMember;
 import org.ofbiz.entity.model.ModelViewEntity.ModelAlias;
 import org.ofbiz.entity.model.ModelViewEntity.ModelAliasAll;
+import org.ofbiz.entity.model.ModelViewEntity.ModelFilter;
 import org.ofbiz.entity.model.ModelViewEntity.ModelMemberEntity;
 import org.ofbiz.entity.model.ModelViewEntity.ModelViewLink;
 /**
@@ -63,6 +64,9 @@
     /** List of view links to define how entities are connected (or "joined") */
     protected List<ModelViewLink> viewLinks = new ArrayList<ModelViewLink>();
     
+    /** A List of criteria to filter view data */
+    protected List<ModelFilter> filters = new ArrayList<ModelFilter>();
+
     /** relations defining relationships between this entity and other entities */
     protected List<ModelRelation> relations = new ArrayList<ModelRelation>();
     
@@ -203,6 +207,15 @@
         addList.addAll(this.viewLinks);
     }
     
+    public void addFilter(String entityAlias, String fieldName, String operator, String value) {
+        ModelFilter modelFilter = new ModelFilter(entityAlias, fieldName, operator, value);
+        this.filters.add(modelFilter);
+    }
+    
+    public void addAllFiltersToList(List<ModelFilter> addList) {
+        addList.addAll(this.filters);
+    }
+    
     public void addRelation(String type, String title, String relEntityName, List<ModelKeyMap> modelKeyMaps) {
         ModelRelation relation = new ModelRelation(type, title, relEntityName, null, modelKeyMaps);
         this.relations.add(relation);

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=707216&r1=707215&r2=707216&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Wed Oct 22 15:22:00 2008
@@ -80,6 +80,10 @@
 
     protected Map<String, Map<String, ModelConversion>> conversions = FastMap.newInstance();
 
+    /** A List of criteria to filter view data */
+    protected List<ModelFilter> filters = FastList.newInstance();
+
+    
     public ModelViewEntity(ModelReader reader, Element entityElement, UtilTimer utilTimer, ModelInfo def) {
         super(reader, entityElement, def);
 
@@ -116,6 +120,11 @@
             this.addViewLink(viewLink);
         }
 
+        for (Element filterElement : UtilXml.childElementList(entityElement, "filter")) {
+            ModelFilter filter = new ModelFilter(filterElement);
+            this.addFilter(filter);
+        }
+
         if (utilTimer != null) utilTimer.timerString("  createModelEntity: before relations");
         this.populateRelated(reader, entityElement);
 
@@ -144,6 +153,9 @@
         
         // view-links
         dynamicViewEntity.addAllViewLinksToList(this.viewLinks);
+
+        // filters
+        dynamicViewEntity.addAllFiltersToList(this.filters);
         
         // relations
         dynamicViewEntity.addAllRelationsToList(this.relations);
@@ -271,6 +283,21 @@
         return colNameString(Arrays.asList(flds), separator, afterLast, alias);
     }
 
+    /** List of filters to define how entities are filtered when data is showed */
+    public List<ModelFilter> getFilters() {
+        return this.filters;
+    }
+
+    public List<ModelFilter> getFiltersCopy() {
+        List<ModelFilter> newList = FastList.newInstance();
+        newList.addAll(this.filters);
+        return newList;
+    }
+
+    public void addFilter(ModelFilter filter) {
+        this.filters.add(filter);
+    }
+    
     public String colNameString(List<ModelField> flds, String separator, String afterLast, boolean alias) {
         StringBuilder returnString = new StringBuilder();
 
@@ -1001,6 +1028,45 @@
         }
     }
 
+    public static class ModelFilter implements Serializable {
+        protected String entityAlias = "";
+        protected String fieldName = "";
+        protected String operator = "";
+        protected String value = "";
+
+        protected ModelFilter() {}
+
+        public ModelFilter(Element filterElement) {
+            this.entityAlias = UtilXml.checkEmpty(filterElement.getAttribute("entity-alias"));
+            this.fieldName = UtilXml.checkEmpty(filterElement.getAttribute("field-name"));
+            this.operator = UtilXml.checkEmpty(filterElement.getAttribute("operator"));
+            this.value = UtilXml.checkEmpty(filterElement.getAttribute("value"));
+        }
+
+        public ModelFilter(String entityAlias, String fieldName, String operator, String value) {
+            this.entityAlias = entityAlias;
+            this.fieldName = fieldName;
+            this.operator = operator;
+            this.value = value;
+        }
+
+        public String getEntityAlias() {
+            return this.entityAlias;
+        }
+
+        public String getFieldName() {
+            return this.fieldName;
+        }
+
+        public String getOperator() {
+            return this.operator;
+        }
+
+        public String getValue() {
+            return this.value;
+        }
+    }
+
     public class ModelConversion implements Serializable {
         protected String aliasName;
         protected ModelEntity fromModelEntity;



Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Great, let us (the ones not a the conference) know please

Thanks

Jacques

From: "Adam Heath" <do...@brainfood.com>
> Jacques Le Roux wrote:
>> Reverted in revision: 707830
>> 
>> Obviously Adam's solution would be far better.
> 
> I'd like to talk about this and other changes I'd like to do, at the
> upcoming conference.
>

Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by BJ Freeman <bj...@free-man.net>.
I hope a summary of the discussion gets fed back here so those not going
can be kept in the loop.
:D

Adam Heath sent the following on 10/25/2008 11:58 AM:
> Jacques Le Roux wrote:
>> Reverted in revision: 707830
>>
>> Obviously Adam's solution would be far better.
> 
> I'd like to talk about this and other changes I'd like to do, at the
> upcoming conference.
> 
> 

Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by Adam Heath <do...@brainfood.com>.
Jacques Le Roux wrote:
> Reverted in revision: 707830
> 
> Obviously Adam's solution would be far better.

I'd like to talk about this and other changes I'd like to do, at the
upcoming conference.

Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Reverted in revision: 707830

Obviously Adam's solution would be far better.

Jacques

From: "David E Jones" <jo...@hotwaxmedia.com>
>
> Jacques,
>
> It might be good to revert temporarily until the solution is refined,  especially the design aspects of it.
>
> -David
>
>
> On Oct 24, 2008, at 7:48 AM, Jacques Le Roux wrote:
>
>> Hi Adam,
>>
>> I was not aware of all the context. From this thread http://lists.ofbiz.org/pipermail/dev/2004-March/004381.html I see now what 
>> you
>> mean.
>> I agree that Oscar's solution is missing operators (your solution  with EntityCondition allows operators) and that I should have 
>> used
>> prepared statements for.
>> Should I revert right now before people notice and begin to use it ?  Or could we extend this syntax with your solution  ?
>>
>> Jacques
>>
>> From: "Adam Heath" <do...@brainfood.com>
>>> One substantial comment below, bad code.
>>>
>>> Nice feature otherwise, but it doesn't support conditions, only  constant
>>> values.
>>>
>>> This really should be done using the EntityCondition framework, so  that
>>> the sql generating code can be reused and not duplicated.
>>>
>>> jleroux@apache.org wrote:
>>>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ SqlJdbcUtil.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=707216&r1=707215&r2=707216&view=diff
>>>> = = = = = = = = = = ====================================================================
>>>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ SqlJdbcUtil.java (original)
>>>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ SqlJdbcUtil.java Wed Oct 22 15:22:00 2008
>>>> +    public static void makeViewFilterWhereClause(ModelViewEntity  modelViewEntity, StringBuilder whereString) throws
>>>> GenericEntityException {
>>>> +
>>>> +        for (ModelViewEntity.ModelFilter filter :  modelViewEntity.getFilters()) {
>>>> +            ModelEntity filterEntity =  modelViewEntity.getMemberModelEntity(filter.getEntityAlias());
>>>> +            if (filterEntity == null) {
>>>> +                throw new GenericEntityException("Link entity not  found with alias: " + filter.getEntityAlias() + " for 
>>>> entity:
>>>> " + modelViewEntity.getEntityName());
>>>> +            }
>>>> +
>>>> +            ModelField filterField =  filterEntity.getField(filter.getFieldName());
>>>> +            if (filterField == null) {
>>>> +                throw new GenericEntityException("The field " +  filter.getFieldName() + " does not appear to belong to entity 
>>>> "
>>>> + modelViewEntity.getEntityName());
>>>> +            }
>>>> +            if (whereString.length() > 0) {
>>>> +                whereString.append(" AND ");
>>>> +            }
>>>> +            whereString.append(filter.getEntityAlias());
>>>> +            whereString.append(".");
>>>> +             whereString.append(filterColName(filterField.getColName()));
>>>> +
>>>> +            EntityOperator<?> entityOperator =  EntityOperator.lookup(filter.getOperator());
>>>> +            if (entityOperator == null) {
>>>> +            throw new GenericEntityException("Operator " +  filter.getOperator() + " not supported in filter for entity: " +
>>>> modelViewEntity.getEntityName());
>>>
>>> Tabbing is wrong on this line.
>>>
>>>> +            }
>>>> +            whereString.append("  ").append(entityOperator.getCode()).append(" ");
>>>> +
>>>> +            whereString.append("'" +  filter.getValue().replaceAll("'", "''") + "'");
>>>> +        }
>>>>     }
>>>>
>>>
>>> ICK!  Danger Will Robinson, Danger!
>>>
>>> Do NOT use string concatenation.  Use prepared statements.  Plus, the
>>> quoting used is not portable.
>>>
>>
> 


Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by David E Jones <jo...@hotwaxmedia.com>.
Jacques,

It might be good to revert temporarily until the solution is refined,  
especially the design aspects of it.

-David


On Oct 24, 2008, at 7:48 AM, Jacques Le Roux wrote:

> Hi Adam,
>
> I was not aware of all the context. From this thread http://lists.ofbiz.org/pipermail/dev/2004-March/004381.html 
>  I see now what you
> mean.
> I agree that Oscar's solution is missing operators (your solution  
> with EntityCondition allows operators) and that I should have used
> prepared statements for.
> Should I revert right now before people notice and begin to use it ?  
> Or could we extend this syntax with your solution  ?
>
> Jacques
>
> From: "Adam Heath" <do...@brainfood.com>
>> One substantial comment below, bad code.
>>
>> Nice feature otherwise, but it doesn't support conditions, only  
>> constant
>> values.
>>
>> This really should be done using the EntityCondition framework, so  
>> that
>> the sql generating code can be reused and not duplicated.
>>
>> jleroux@apache.org wrote:
>>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ 
>>> SqlJdbcUtil.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=707216&r1=707215&r2=707216&view=diff
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ 
>>> SqlJdbcUtil.java (original)
>>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ 
>>> SqlJdbcUtil.java Wed Oct 22 15:22:00 2008
>>> +    public static void makeViewFilterWhereClause(ModelViewEntity  
>>> modelViewEntity, StringBuilder whereString) throws
>>> GenericEntityException {
>>> +
>>> +        for (ModelViewEntity.ModelFilter filter :  
>>> modelViewEntity.getFilters()) {
>>> +            ModelEntity filterEntity =  
>>> modelViewEntity.getMemberModelEntity(filter.getEntityAlias());
>>> +            if (filterEntity == null) {
>>> +                throw new GenericEntityException("Link entity not  
>>> found with alias: " + filter.getEntityAlias() + " for entity:
>>> " + modelViewEntity.getEntityName());
>>> +            }
>>> +
>>> +            ModelField filterField =  
>>> filterEntity.getField(filter.getFieldName());
>>> +            if (filterField == null) {
>>> +                throw new GenericEntityException("The field " +  
>>> filter.getFieldName() + " does not appear to belong to entity "
>>> + modelViewEntity.getEntityName());
>>> +            }
>>> +            if (whereString.length() > 0) {
>>> +                whereString.append(" AND ");
>>> +            }
>>> +            whereString.append(filter.getEntityAlias());
>>> +            whereString.append(".");
>>> +             
>>> whereString.append(filterColName(filterField.getColName()));
>>> +
>>> +            EntityOperator<?> entityOperator =  
>>> EntityOperator.lookup(filter.getOperator());
>>> +            if (entityOperator == null) {
>>> +            throw new GenericEntityException("Operator " +  
>>> filter.getOperator() + " not supported in filter for entity: " +
>>> modelViewEntity.getEntityName());
>>
>> Tabbing is wrong on this line.
>>
>>> +            }
>>> +            whereString.append("  
>>> ").append(entityOperator.getCode()).append(" ");
>>> +
>>> +            whereString.append("'" +  
>>> filter.getValue().replaceAll("'", "''") + "'");
>>> +        }
>>>     }
>>>
>>
>> ICK!  Danger Will Robinson, Danger!
>>
>> Do NOT use string concatenation.  Use prepared statements.  Plus, the
>> quoting used is not portable.
>>
>


Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Adam,

I was not aware of all the context. From this thread http://lists.ofbiz.org/pipermail/dev/2004-March/004381.html I see now what you
mean.
I agree that Oscar's solution is missing operators (your solution with EntityCondition allows operators) and that I should have used
prepared statements for.
Should I revert right now before people notice and begin to use it ? Or could we extend this syntax with your solution  ?

Jacques

From: "Adam Heath" <do...@brainfood.com>
> One substantial comment below, bad code.
>
> Nice feature otherwise, but it doesn't support conditions, only constant
> values.
>
> This really should be done using the EntityCondition framework, so that
> the sql generating code can be reused and not duplicated.
>
> jleroux@apache.org wrote:
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=707216&r1=707215&r2=707216&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java (original)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java Wed Oct 22 15:22:00 2008
>> +    public static void makeViewFilterWhereClause(ModelViewEntity modelViewEntity, StringBuilder whereString) throws
>> GenericEntityException {
>> +
>> +        for (ModelViewEntity.ModelFilter filter : modelViewEntity.getFilters()) {
>> +            ModelEntity filterEntity = modelViewEntity.getMemberModelEntity(filter.getEntityAlias());
>> +            if (filterEntity == null) {
>> +                throw new GenericEntityException("Link entity not found with alias: " + filter.getEntityAlias() + " for entity:
>> " + modelViewEntity.getEntityName());
>> +            }
>> +
>> +            ModelField filterField = filterEntity.getField(filter.getFieldName());
>> +            if (filterField == null) {
>> +                throw new GenericEntityException("The field " + filter.getFieldName() + " does not appear to belong to entity "
>> + modelViewEntity.getEntityName());
>> +            }
>> +            if (whereString.length() > 0) {
>> +                whereString.append(" AND ");
>> +            }
>> +            whereString.append(filter.getEntityAlias());
>> +            whereString.append(".");
>> +            whereString.append(filterColName(filterField.getColName()));
>> +
>> +            EntityOperator<?> entityOperator = EntityOperator.lookup(filter.getOperator());
>> +            if (entityOperator == null) {
>> +            throw new GenericEntityException("Operator " + filter.getOperator() + " not supported in filter for entity: " +
>> modelViewEntity.getEntityName());
>
> Tabbing is wrong on this line.
>
>> +            }
>> +            whereString.append(" ").append(entityOperator.getCode()).append(" ");
>> +
>> +            whereString.append("'" + filter.getValue().replaceAll("'", "''") + "'");
>> +        }
>>      }
>>
>
> ICK!  Danger Will Robinson, Danger!
>
> Do NOT use string concatenation.  Use prepared statements.  Plus, the
> quoting used is not portable.
>


Re: svn commit: r707216 - in /ofbiz/trunk/framework/entity: dtd/entitymodel.xsd src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java src/org/ofbiz/entity/model/DynamicViewEntity.java src/org/ofbiz/entity/model/ModelViewEntity.java

Posted by Adam Heath <do...@brainfood.com>.
One substantial comment below, bad code.

Nice feature otherwise, but it doesn't support conditions, only constant
values.

This really should be done using the EntityCondition framework, so that
the sql generating code can be reused and not duplicated.

jleroux@apache.org wrote:
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=707216&r1=707215&r2=707216&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java Wed Oct 22 15:22:00 2008
> +    public static void makeViewFilterWhereClause(ModelViewEntity modelViewEntity, StringBuilder whereString) throws GenericEntityException {
> +
> +        for (ModelViewEntity.ModelFilter filter : modelViewEntity.getFilters()) {            
> +            ModelEntity filterEntity = modelViewEntity.getMemberModelEntity(filter.getEntityAlias());
> +            if (filterEntity == null) {
> +                throw new GenericEntityException("Link entity not found with alias: " + filter.getEntityAlias() + " for entity: " + modelViewEntity.getEntityName());
> +            }
> +            
> +            ModelField filterField = filterEntity.getField(filter.getFieldName());
> +            if (filterField == null) {
> +                throw new GenericEntityException("The field " + filter.getFieldName() + " does not appear to belong to entity " + modelViewEntity.getEntityName());                
> +            }
> +            if (whereString.length() > 0) {
> +                whereString.append(" AND ");
> +            }
> +            whereString.append(filter.getEntityAlias());
> +            whereString.append(".");
> +            whereString.append(filterColName(filterField.getColName()));
> +            
> +            EntityOperator<?> entityOperator = EntityOperator.lookup(filter.getOperator());
> +            if (entityOperator == null) {
> +            	throw new GenericEntityException("Operator " + filter.getOperator() + " not supported in filter for entity: " + modelViewEntity.getEntityName());

Tabbing is wrong on this line.

> +            }
> +            whereString.append(" ").append(entityOperator.getCode()).append(" ");
> +            
> +            whereString.append("'" + filter.getValue().replaceAll("'", "''") + "'");
> +        }
>      }
>  

ICK!  Danger Will Robinson, Danger!

Do NOT use string concatenation.  Use prepared statements.  Plus, the
quoting used is not portable.