Change API of new OBDao class to be safer/cleaner
authorStefan Hühner <stefan.huehner@openbravo.com>
Tue, 08 Mar 2011 13:08:42 +0100
changeset 11087 7e9a7978463b
parent 11086 e713c25dd054
child 11088 ae2755c148fd
Change API of new OBDao class to be safer/cleaner
- getAllInstances, removed as just getFilteredCriteria().list()
- getOneInstance, removed as unsafe, most users can use existing uniqueResult
which fails with >1 result rows, and returns null or the value otherwise. This
fixes the unpredictable result is absence of an order by clause.
- getFilteredCriteria, remove version to disable client/org filter, can be
easily done with 1-2 lines extra in the caller when rally needed
- getFilteredCriteria: change api to use normal hibernate Criterion for
filtering which removes need for another class to declare the same concept
(Constraint).
modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/CustomQuerySelectorDatasource.java
modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/SelectorComponent.java
src/org/openbravo/dal/service/OBDao.java
--- a/modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/CustomQuerySelectorDatasource.java	Tue Mar 08 12:57:35 2011 +0100
+++ b/modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/CustomQuerySelectorDatasource.java	Tue Mar 08 13:08:42 2011 +0100
@@ -31,6 +31,7 @@
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.hibernate.Query;
+import org.hibernate.criterion.Expression;
 import org.openbravo.base.model.ModelProvider;
 import org.openbravo.base.model.domaintype.BigDecimalDomainType;
 import org.openbravo.base.model.domaintype.BooleanDomainType;
@@ -43,7 +44,6 @@
 import org.openbravo.dal.service.OBCriteria;
 import org.openbravo.dal.service.OBDal;
 import org.openbravo.dal.service.OBDao;
-import org.openbravo.dal.service.OBDao.Constraint;
 import org.openbravo.service.datasource.ReadOnlyDataSourceService;
 import org.openbravo.service.json.JsonConstants;
 import org.openbravo.service.json.JsonUtils;
@@ -351,7 +351,7 @@
     // If sortByClause is empty set default sort options.
     if (sortByClause.length() == 0) {
       OBCriteria<SelectorField> selFieldsCrit = OBDao.getFilteredCriteria(SelectorField.class,
-          new Constraint(SelectorField.PROPERTY_OBUISELSELECTOR, sel), new Constraint(
+          Expression.eq(SelectorField.PROPERTY_OBUISELSELECTOR, sel), Expression.eq(
               SelectorField.PROPERTY_SHOWINGRID, true));
       selFieldsCrit.addOrderBy(SelectorField.PROPERTY_SORTNO, true);
       for (SelectorField selField : selFieldsCrit.list()) {
--- a/modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/SelectorComponent.java	Tue Mar 08 12:57:35 2011 +0100
+++ b/modules/org.openbravo.userinterface.selector/src/org/openbravo/userinterface/selector/SelectorComponent.java	Tue Mar 08 13:08:42 2011 +0100
@@ -29,6 +29,8 @@
 import javax.inject.Inject;
 
 import org.apache.log4j.Logger;
+import org.hibernate.criterion.Criterion;
+import org.hibernate.criterion.Expression;
 import org.openbravo.base.model.Entity;
 import org.openbravo.base.model.ModelProvider;
 import org.openbravo.base.model.Property;
@@ -49,7 +51,6 @@
 import org.openbravo.dal.service.OBDal;
 import org.openbravo.dal.service.OBDao;
 import org.openbravo.dal.service.OBQuery;
-import org.openbravo.dal.service.OBDao.Constraint;
 import org.openbravo.data.Sqlc;
 import org.openbravo.model.ad.module.Module;
 import org.openbravo.model.ad.ui.Field;
@@ -343,8 +344,8 @@
    */
   public String getShowSelectorGrid() {
     if (OBDao.getFilteredCriteria(SelectorField.class,
-        new Constraint(SelectorField.PROPERTY_OBUISELSELECTOR, getSelector()),
-        new Constraint(SelectorField.PROPERTY_SHOWINGRID, true)).count() > 0) {
+        Expression.eq(SelectorField.PROPERTY_OBUISELSELECTOR, getSelector()),
+        Expression.eq(SelectorField.PROPERTY_SHOWINGRID, true)).count() > 0) {
       return Boolean.TRUE.toString();
     }
     return Boolean.FALSE.toString();
@@ -541,14 +542,12 @@
 
     OBContext.setAdminMode();
     try {
-      final Constraint selectorConstraint = new Constraint(SelectorField.PROPERTY_OBUISELSELECTOR,
+      final Criterion selectorConstraint = Expression.eq(SelectorField.PROPERTY_OBUISELSELECTOR,
           getSelector());
-      final Constraint isOutFieldConstraint = new Constraint(SelectorField.PROPERTY_ISOUTFIELD,
-          true);
-      final Constraint hasSuffixConstraint = new Constraint(SelectorField.PROPERTY_SUFFIX, null,
-          OBDao.Operator.NOT_EQUAL_OPERATOR);
-      List<SelectorField> fields = OBDao.getAllInstances(SelectorField.class, selectorConstraint,
-          isOutFieldConstraint, hasSuffixConstraint);
+      final Criterion isOutFieldConstraint = Expression.eq(SelectorField.PROPERTY_ISOUTFIELD, true);
+      final Criterion hasSuffixConstraint = Expression.isNotNull(SelectorField.PROPERTY_SUFFIX);
+      List<SelectorField> fields = OBDao.getFilteredCriteria(SelectorField.class,
+          selectorConstraint, isOutFieldConstraint, hasSuffixConstraint).list();
       for (final SelectorField field : fields) {
         hiddenInputs.put(columnName + field.getSuffix(), getElementString.replaceAll("@id@",
             columnName + field.getSuffix()));
--- a/src/org/openbravo/dal/service/OBDao.java	Tue Mar 08 12:57:35 2011 +0100
+++ b/src/org/openbravo/dal/service/OBDao.java	Tue Mar 08 13:08:42 2011 +0100
@@ -19,12 +19,11 @@
 package org.openbravo.dal.service;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.StringTokenizer;
 
 import org.apache.commons.lang.StringUtils;
-import org.hibernate.criterion.Expression;
+import org.hibernate.criterion.Criterion;
 import org.openbravo.base.structure.BaseOBObject;
 
 /**
@@ -40,214 +39,20 @@
    * 
    * @param clazz
    *          Class (entity).
-   * @param setFilterClient
-   *          If true then only objects from readable clients are returned, if false then objects
-   *          from all clients are returned
-   * @param setFilterOrg
-   *          If true then when querying (for example call list()) a filter on readable
-   *          organizations is added to the query, if false then this is not done
    * @param constraints
-   *          Constraint. Property, value and operator.
+   *          List of hibernate Criterion instances which are used as filters
    * @return An OBCriteria object with the constraints.
    */
   public static <T extends BaseOBObject> OBCriteria<T> getFilteredCriteria(Class<T> clazz,
-      boolean setFilterClient, boolean setFilterOrg, Constraint... constraints) {
+      Criterion... constraints) {
     OBCriteria<T> obc = OBDal.getInstance().createCriteria(clazz);
-    obc.setFilterOnReadableClients(setFilterClient);
-    obc.setFilterOnReadableOrganization(setFilterOrg);
-    addContrainsToCriteria(obc, constraints);
+    for (Criterion c : constraints) {
+      obc.add(c);
+    }
     return obc;
   }
 
   /**
-   * Implementation of {@link OBDao#getFilteredCriteria(Class, boolean, boolean, Constraint...)}
-   * enabling the filter by readable clients and organizations.
-   */
-  public static <T extends BaseOBObject> OBCriteria<T> getFilteredCriteria(Class<T> clazz,
-      Constraint... constraints) {
-    return getFilteredCriteria(clazz, true, true, constraints);
-  }
-
-  /**
-   * Returns a List object with the instances of the given OBObject Class filtered by the given
-   * array of constraints. The default client and organization filters can be disabled using the
-   * corresponding boolean parameters.
-   * 
-   * @param clazz
-   *          Class (entity).
-   * @param setFilterClient
-   *          If true then only objects from readable clients are returned, if false then objects
-   *          from all clients are returned
-   * @param setFilterOrg
-   *          If true then when querying (for example call list()) a filter on readable
-   *          organizations is added to the query, if false then this is not done
-   * @param constraints
-   *          Constraint. Property, value and operator.
-   * @return An List object with the objects of Class clazz filtered by the given constraints.
-   */
-  public static <T extends BaseOBObject> List<T> getAllInstances(Class<T> clazz,
-      boolean setClientFilter, boolean setOrganizationFilter, Constraint... constraints) {
-    return getFilteredCriteria(clazz, setClientFilter, setOrganizationFilter, constraints).list();
-  }
-
-  /**
-   * Implementation of {@link OBDao#getAllInstances(Class, boolean, boolean, Constraint...)}
-   * enabling the filter by readable clients and organizations.
-   */
-  public static <T extends BaseOBObject> List<T> getAllInstances(Class<T> clazz,
-      Constraint... constraints) {
-    return getFilteredCriteria(clazz, true, true, constraints).list();
-  }
-
-  /**
-   * Returns the first object of the given OBObject Class filtered by the given array of
-   * constraints. The default client and organization filters can be disabled using the
-   * corresponding boolean parameters.
-   * 
-   * @param clazz
-   *          Class (entity).
-   * @param setFilterClient
-   *          If true then only objects from readable clients are returned, if false then objects
-   *          from all clients are returned
-   * @param setFilterOrg
-   *          If true then when querying (for example call list()) a filter on readable
-   *          organizations is added to the query, if false then this is not done
-   * @param constraints
-   *          Constraint. Property, value and operator.
-   * @return An List object with the objects of Class clazz filtered by the given constraints.
-   */
-  public static <T extends BaseOBObject> T getOneInstance(Class<T> clazz, boolean setClientFilter,
-      boolean setOrganizationFilter, Constraint... constraints) {
-    OBCriteria<T> criteria = getFilteredCriteria(clazz, setClientFilter, setOrganizationFilter,
-        constraints);
-    criteria.setMaxResults(1);
-    if (criteria.list().isEmpty()) {
-      return null;
-    }
-    return criteria.list().get(0);
-  }
-
-  /**
-   * Implementation of {@link OBDao#getOneInstance(Class, boolean, boolean, Constraint...)} enabling
-   * the filter by readable clients and organizations.
-   */
-  public static <T extends BaseOBObject> T getOneInstance(Class<T> clazz, Constraint... constraints) {
-    return getOneInstance(clazz, true, true, constraints);
-  }
-
-  public static <T extends BaseOBObject> void addContrainsToCriteria(OBCriteria<T> obc,
-      Constraint... constraints) {
-    for (Constraint constraint : constraints) {
-      if (constraint.getValue() == null && Operator.EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.isNull(constraint.getProperty()));
-      } else if (constraint.getValue() == null
-          && Operator.NOT_EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.isNotNull(constraint.getProperty()));
-      } else if (Operator.EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.eq(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.NOT_EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.ne(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.LESS_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.lt(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.GREATER_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.gt(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.LESS_EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.le(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.GREATER_EQUAL_OPERATOR.equals(constraint.getOperator())) {
-        obc.add(Expression.ge(constraint.getProperty(), constraint.getValue()));
-      } else if (Operator.IN_OPERATOR.equals(constraint.getOperator())) {
-        Object value = constraint.getValue();
-        if (value instanceof Collection) {
-          obc.add(Expression.in(constraint.getProperty(), (Collection<?>) value));
-        } else if (value instanceof Object[]) {
-          obc.add(Expression.in(constraint.getProperty(), (Object[]) constraint.getValue()));
-        }
-      } else {
-        obc.add(Expression.eq(constraint.getProperty(), constraint.getValue()));
-      }
-    }
-
-  }
-
-  /**
-   * Each instance of this class contains a Constraint to be added to the OBCriteria object used on
-   * {@link OBDao#getFilteredCriteria(Class, boolean, boolean, Constraint...)}.
-   * 
-   * @author gorkaion
-   * 
-   */
-  public static class Constraint {
-    private String property;
-    private Object value;
-    // '==', '!=', '<=', '>=', '<', '>'
-    private Operator operator;
-
-    /**
-     * Initializes a Constraint to be added to an OBCriteria. The Operator is defaulted to
-     * {@link OBDao.Operator EQUAL_OPERATOR}.
-     * 
-     * <br>
-     * A <i>null</i> value creates a "isNull" Expression for the given property
-     * 
-     * @param property
-     *          String with the property to filter by the OBCriteria.
-     * @param value
-     *          Object with the filter the property is filtered by.
-     */
-    public Constraint(String property, Object value) {
-      this.property = property;
-      this.value = value;
-      this.operator = Operator.EQUAL_OPERATOR;
-    }
-
-    /**
-     * Initializes a Constraint to be added to an OBCriteria.
-     * 
-     * <br>
-     * A <i>null</i> value and {@link OBDao.Operator EQUAL_OPERATOR} operator creates a "isNull"
-     * expression for the given property.
-     * 
-     * <br>
-     * A <i>null</i> value and {@link OBDao.Operator NOT_EQUAL_OPERATOR} operator creates a
-     * "isNotNull" expression for the given property.
-     * 
-     * @param property
-     *          String with the property to filter by the OBCriteria.
-     * @param value
-     *          Object with the filter the property is filtered by.
-     * @param operator
-     *          Operator that defines the expression type to be applied for this Constraint.
-     */
-    public Constraint(String property, Object value, Operator operator) {
-      this.property = property;
-      this.value = value;
-      this.operator = operator;
-    }
-
-    public String getProperty() {
-      return property;
-    }
-
-    public Object getValue() {
-      return value;
-    }
-
-    public Operator getOperator() {
-      return operator;
-    }
-  }
-
-  /**
-   * Valid operators to apply on a Constraint to filter an OBCriteria.
-   * 
-   * @author gorkaion
-   * 
-   */
-  public static enum Operator {
-    EQUAL_OPERATOR, NOT_EQUAL_OPERATOR, LESS_EQUAL_OPERATOR, GREATER_EQUAL_OPERATOR, LESS_OPERATOR, GREATER_OPERATOR, IN_OPERATOR
-  }
-
-  /**
    * Returns a List of BaseOBOBjects of the Property identified by the property from the
    * BaseOBObject obj. This method enables the activeFilter so inactive BaseOBObjects are not
    * included on the returned List.