extended IDACall with handleDSRequest() override + DSRequest.addToCriteria() bug

[Blama]

Hi Isomorphic,

please see this DS-Request from the Server-log and what the IDAServlet does with it.
The suspicicous part is the FROM T_SETTINGS WHERE (('1'='1' AND '1'='1' AND '1'='1') AND (T_SETTINGS.TENANT_ID = 0 AND T_SETTINGS.TENANT_ID IS NOT NULL)).

I'm using the current nightly (v9.0p_2013-11-13/EVAL Deployment).

The server request comes in from the client with some criteria and is added a criterion for "TENANT_ID" by my IDACall (which I register in web.xml):

import java.io.IOException;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import com.isomorphic.criteria.AdvancedCriteria;
import com.isomorphic.criteria.Criterion;
import com.isomorphic.criteria.DefaultOperators;
import com.isomorphic.criteria.criterion.SimpleCriterion;
import com.isomorphic.datasource.DSRequest;
import com.isomorphic.datasource.DSResponse;
import com.isomorphic.datasource.DataSource;
import com.isomorphic.rpc.RPCManager;
import com.isomorphic.servlet.IDACall;
import com.isomorphic.servlet.RequestContext;
import com.lmscompany.lms.server.util.User;

public class LMSIDACall extends IDACall {
	private static final long serialVersionUID = 4543651445782711736L;

	/*
	 * See http://www.smartclient.com/smartgwtee/javadoc/com/smartgwt/client/docs/serverds/OperationBinding.html#requiresRole
	 */
	@Override
	public void processRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException,
			IOException {
		Integer i = User.getUserId(request);
		if (i != null) {
			try {
				RequestContext context = RequestContext.instance(this, request, response);
				RPCManager rpc = new RPCManager(request, response);
				rpc.setUserId(i.toString());
				// rpc.setAuthenticated(true);
				// rpc.setUserRoles((String) roles);

				// call processRPCTransaction() to iterate through all RPCRequests and
				// DSRequests and execute them
				processRPCTransaction(rpc, context);

			} catch (Throwable e) {
				handleError(response, e);
			}
		} else {
			super.processRequest(request, response);
		}
	}

	@SuppressWarnings("unchecked") 
	@Override
	public DSResponse handleDSRequest(DSRequest arg0, RPCManager arg1, RequestContext arg2) throws Exception {
		if (arg0.getOperationType().equals(DataSource.OP_ADD)) {
			arg0.setValues(arg0.getValues().put("TENANT_ID", User.getUserTenantId(arg0.getHttpServletRequest()))); 
		} else if (arg0.getOperationType().equals(DataSource.OP_FETCH)
				|| arg0.getOperationType().equals(DataSource.OP_UPDATE) || arg0.getOperationType().equals(DataSource.OP_REMOVE)) {
[B]			AdvancedCriteria oldAc = arg0.getAdvancedCriteria();
			AdvancedCriteria newAc = null;
			if (oldAc != null) {
				AdvancedCriteria tempAc = new AdvancedCriteria(DefaultOperators.And, new Criterion[] { new SimpleCriterion(
						"TENANT_ID", DefaultOperators.Equals, User.getUserTenantId(arg0.getHttpServletRequest())) });
				newAc = new AdvancedCriteria(DefaultOperators.And,
						new Criterion[] { oldAc.asCriterion(), tempAc.asCriterion() });
			} else
				newAc = new AdvancedCriteria(DefaultOperators.And, new Criterion[] { new SimpleCriterion("TENANT_ID",
						DefaultOperators.Equals, User.getUserTenantId(arg0.getHttpServletRequest())) });
			arg0.setAdvancedCriteria(newAc);[/B]

			// arg0.addToCriteria("TENANT_ID", "equal",
			// User.getUserTenantId(arg0.getHttpServletRequest()));
		}
		return super.handleDSRequest(arg0, arg1, arg2);
	}
}

The log:

=== 2013-11-13 14:15:34,821 [c-31] INFO  RequestContext - URL: '/lms/lms/sc/IDACall', User-Agent: 'Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0': Moz (Gecko) with Accept-Encoding header
=== 2013-11-13 14:15:34,825 [c-31] DEBUG XML - Parsed XML from (in memory stream): 3ms
=== 2013-11-13 14:15:34,827 [c-31] DEBUG RPCManager - Processing 1 requests.
=== 2013-11-13 14:15:34,827 [c-31] DEBUG DSRequest - Caching instance 300 of DS T_SETTINGS from DSRequest.getDataSource()
=== 2013-11-13 14:15:34,828 [c-31] DEBUG RPCManager - Request #1 (DSRequest) payload: {
    criteria:{
[B]        fieldName:"SHORTNAME",
        operator:"startsWith",
        value:"name_category"
[/B]    },
    operationConfig:{
        dataSource:"T_SETTINGS",
        operationType:"fetch"
    },
    sortBy:[
        "SHORTNAME"
    ],
    appID:"builtinApplication",
    operation:"T_SETTINGS_fetch",
    oldValues:{
        fieldName:"SHORTNAME",
        operator:"startsWith",
        value:"name_category"
    }
}
=== 2013-11-13 14:15:34,829 [c-31] DEBUG DeclarativeSecurity - Processing security checks for DataSource null, field null
=== 2013-11-13 14:15:34,829 [c-31] DEBUG DeclarativeSecurity - DataSource T_SETTINGS is not in the pre-checked list, processing...
=== 2013-11-13 14:15:34,829 [c-31] DEBUG AppBase - [builtinApplication.T_SETTINGS_fetch] No userTypes defined, allowing anyone access to all operations for this application
=== 2013-11-13 14:15:34,829 [c-31] DEBUG AppBase - [builtinApplication.T_SETTINGS_fetch] No public zero-argument method named '_T_SETTINGS_fetch' found, performing generic datasource operation
=== 2013-11-13 14:15:34,830 [c-31] INFO  SQLDataSource - [builtinApplication.T_SETTINGS_fetch] Performing fetch operation with
	[B]criteria: {criteria:[{criteria:[{value:"SHORTNAME",fieldName:"fieldName",operator:"equals"},{value:"startsWith",fieldName:"operator",operator:"equals"},{value:"name_category",fieldName:"value",operator:"equals"}],operator:"and"},{value:0,fieldName:"TENANT_ID",operator:"equals"}],operator:"and",_constructor:"AdvancedCriteria"}	values: {criteria:[{criteria:[{value:"SHORTNAME",fieldName:"fieldName",operator:"equals"},{value:"startsWith",fieldName:"operator",operator:"equals"},{value:"name_category",fieldName:"value",operator:"equals"}],operator:"and"},{value:0,fieldName:"TENANT_ID",operator:"equals"}],operator:"and",_constructor:"AdvancedCriteria"}[/B]
=== 2013-11-13 14:15:34,831 [c-31] INFO  SQLDataSource - [builtinApplication.T_SETTINGS_fetch] derived query: SELECT $defaultSelectClause FROM $defaultTableClause WHERE $defaultWhereClause ORDER BY $defaultOrderClause
=== 2013-11-13 14:15:34,832 [c-31] INFO  SQLDataSource - [builtinApplication.T_SETTINGS_fetch] 300: Executing SQL query on 'Oracle': [B]SELECT T_SETTINGS.CREATED_AT, T_SETTINGS.CREATED_BY, T_SETTINGS.DATATYPE, T_SETTINGS.DATEVALUE, T_SETTINGS.DATEVALUEDEFAULT, T_SETTINGS.DATEVALUE_USE, T_SETTINGS.DESCRIPTION, T_SETTINGS.ID, T_SETTINGS.INTEGERVALUE, T_SETTINGS.INTEGERVALUEDEFAULT, T_SETTINGS.INTEGERVALUE_USE, T_SETTINGS.MODIFIED_AT, T_SETTINGS.MODIFIED_BY, T_SETTINGS.NAME, T_SETTINGS.SHORTNAME, T_SETTINGS.TENANT_ID, T_SETTINGS.VARCHARVALUE, T_SETTINGS.VARCHARVALUEDEFAULT, T_SETTINGS.VARCHARVALUE_USE FROM T_SETTINGS [I][U]WHERE (('1'='1' AND '1'='1' AND '1'='1') AND (T_SETTINGS.TENANT_ID = 0 AND T_SETTINGS.TENANT_ID IS NOT NULL))[/U][/I] ORDER BY T_SETTINGS.SHORTNAME[/B]
=== 2013-11-13 14:15:34,832 [c-31] DEBUG PoolableSQLConnectionFactory - [builtinApplication.T_SETTINGS_fetch] Executing pingTest 'select 1 from dual' on connection 294722344
=== 2013-11-13 14:15:34,833 [c-31] DEBUG SQLConnectionManager - [builtinApplication.T_SETTINGS_fetch] Borrowed connection '294722344'
=== 2013-11-13 14:15:34,833 [c-31] DEBUG SQLTransaction - [builtinApplication.T_SETTINGS_fetch] Started new Oracle transaction "294722344"
=== 2013-11-13 14:15:34,833 [c-31] DEBUG SQLDriver - [builtinApplication.T_SETTINGS_fetch] About to execute SQL query in 'Oracle' using connection '294722344'
=== 2013-11-13 14:15:34,833 [c-31] INFO  SQLDriver - [builtinApplication.T_SETTINGS_fetch] Executing SQL query on 'Oracle': SELECT T_SETTINGS.CREATED_AT, T_SETTINGS.CREATED_BY, T_SETTINGS.DATATYPE, T_SETTINGS.DATEVALUE, T_SETTINGS.DATEVALUEDEFAULT, T_SETTINGS.DATEVALUE_USE, T_SETTINGS.DESCRIPTION, T_SETTINGS.ID, T_SETTINGS.INTEGERVALUE, T_SETTINGS.INTEGERVALUEDEFAULT, T_SETTINGS.INTEGERVALUE_USE, T_SETTINGS.MODIFIED_AT, T_SETTINGS.MODIFIED_BY, T_SETTINGS.NAME, T_SETTINGS.SHORTNAME, T_SETTINGS.TENANT_ID, T_SETTINGS.VARCHARVALUE, T_SETTINGS.VARCHARVALUEDEFAULT, T_SETTINGS.VARCHARVALUE_USE FROM T_SETTINGS WHERE (('1'='1' AND '1'='1' AND '1'='1') AND (T_SETTINGS.TENANT_ID = 0 AND T_SETTINGS.TENANT_ID IS NOT NULL)) ORDER BY T_SETTINGS.SHORTNAME
=== 2013-11-13 14:15:34,852 [c-31] INFO  DSResponse - [builtinApplication.T_SETTINGS_fetch] DSResponse: List with 27 items
=== 2013-11-13 14:15:34,853 [c-31] DEBUG RPCManager - Content type for RPC transaction: text/plain; charset=UTF-8
=== 2013-11-13 14:15:34,853 [c-31] DEBUG SQLTransaction - Committing Oracle transaction "294722344"
=== 2013-11-13 14:15:34,853 [c-31] DEBUG RPCManager - non-DMI response, dropExtraFields: false
=== 2013-11-13 14:15:34,858 [c-31] DEBUG SQLTransaction - Returning transactional connection for Oracle with hashcode "294722344"
=== 2013-11-13 14:15:34,858 [c-31] DEBUG SQLTransaction - Ending Oracle transaction "294722344"
=== 2013-11-13 14:15:34,859 [c-31] DEBUG SQLConnectionManager - About to close PoolableConnection with hashcode "294722344"
=== 2013-11-13 14:15:34,859 [c-31] DEBUG PoolableSQLConnectionFactory - Executing pingTest 'select 1 from dual' on connection 294722344
=== 2013-11-13 14:15:34,861 [c-31] INFO  Compression - /lms/lms/sc/IDACall: 9650 -> 901 bytes
=== 2013-11-13 14:15:34,957 [c-31] INFO  RequestContext - URL: '/lms/lms/sc/IDACall', User-Agent: 'Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0': Moz (Gecko) with Accept-Encoding header
=== 2013-11-13 14:15:34,961 [c-31] DEBUG XML - Parsed XML from (in memory stream): 3ms

For me it seems that I'm either completely misusing AdvancedCriteria.asCriterion() or there is a bug in the method leading to the triple '1'='1'. But I don't think I'm misusing the API as this use is suggested in the AdvancedCriteria's constructor javadoc (http://www.smartclient.com/smartgwte...erion[]%29).

Thank you & best regards,
Blama

[Blama]

My T_SETTINGS.ds.xml for the sake of completeness:

<DataSource dbName="Oracle" tableName="T_SETTINGS" ID="T_SETTINGS" serverType="sql">
	<fields>
		<field primaryKey="true" hidden="true" name="ID" type="sequence"></field>
		<field hidden="true" name="TENANT_ID" title="TENANT_ID" type="integer"></field>
		<field foreignKey="V_USER_CREATED_BY.ID" name="CREATED_BY" title="Erstellt von" type="integer"></field>
		<field name="CREATED_AT" title="Erstellt am" type="date"></field>
		<field foreignKey="V_USER_MODIFIED_BY.ID" name="MODIFIED_BY" title="Geändert von" type="integer"></field>
		<field name="MODIFIED_AT" title="Geändert am" type="date"></field>
		<field name="SHORTNAME" title="Optionskurzname (technisch)" length="30" type="text" escapeHTML="true"></field>
		<field name="NAME" title="Optionsname" length="60" type="text" escapeHTML="true" canEdit="false"></field>
		<field name="DESCRIPTION" title="Beschreibung" length="80" type="text" escapeHTML="true" canEdit="false"></field>
		<field name="DATATYPE" title="DATATYPE" length="20" type="text" escapeHTML="true" canEdit="false"></field>
		<field name="VARCHARVALUE" title="Wert (Text)" length="20" type="text" escapeHTML="true"></field>
		<field name="VARCHARVALUEDEFAULT" title="Defaultwert (Text)" length="20" type="text" escapeHTML="true" canEdit="false"></field>
		<field name="VARCHARVALUE_USE" title="VARCHARVALUE_USE" length="20" type="text" escapeHTML="true" canEdit="false"></field>
		<field name="INTEGERVALUE" title="Wert (Zahl)" type="integer"></field>
		<field name="INTEGERVALUEDEFAULT" title="Defaultwert (Zahl)" type="integer" canEdit="false"></field>
		<field name="INTEGERVALUE_USE" title="INTEGERVALUE_USE" type="integer" canEdit="false"></field>
		<field name="DATEVALUE" title="Wert (Datum)" type="date"></field>
		<field name="DATEVALUEDEFAULT" title="Defaultwert (Datum)" type="date" canEdit="false"></field>
		<field name="DATEVALUE_USE" title="DATEVALUE_USE" type="date" canEdit="false"></field>
	</fields>
	<operationBindings>
		<operationBinding operationType="add">
			<values fieldName="CREATED_BY" value="$session.authenticatedUserID" />
			<values fieldName="CREATED_AT" value="$currentDate" />
			<values fieldName="MODIFIED_BY" value="$session.authenticatedUserID" />
			<values fieldName="MODIFIED_AT" value="$currentDate" />
			<values fieldName="TENANT_ID" value="$session.authenticatedUserTenantID" />
		</operationBinding>
		<operationBinding operationType="update">
			<values fieldName="MODIFIED_BY" value="$session.authenticatedUserID" />
			<values fieldName="MODIFIED_AT" value="$currentDate" />
			<values fieldName="TENANT_ID" value="$session.authenticatedUserTenantID" />
		</operationBinding>
	</operationBindings>
</DataSource>

[Isomorphic]

The root of the problem seems to be here:

criteria:{
fieldName:"SHORTNAME",
operator:"startsWith",
value:"name_category"
},

This is clearly intended to be AdvancedCriteria, but it lacks the required _constructor:"AdvancedCriteria" attribute, so it is treated as simple Criteria.

So you actually need a client-side fix to avoid this bad criteria being submitted. But you haven't shown how this request ends up being sent.

[Blama]

Hello Isomorphic,

you are (once again :) perfectly correct:

My client-side code was:

Criteria c = new Criteria() {
	{
		addCriteria(new Criterion("SHORTNAME", OperatorId.STARTS_WITH, "name_category"));
	}};
settingsDS.fetchData(c);

My new code is:

settingsDS.fetchData(new AdvancedCriteria("SHORTNAME", OperatorId.STARTS_WITH, "name_category"));

Is the following statement correct?
"When using AdvancedCriteria, work with Criterion inside the AdvancedCriteria. Use Criteria only for simple equality checks."
Does the constructor of the AdvancedCriteria in my code do that (create an empty add-AdvancedCriteria, then add a Criterion to it?

Thank you for your help & best regards,
Blama

[Isomorphic]

That statement is correct, even so, the framework could automatically up-convert your Criteria to AdvancedCriteria in this instance, and we'll add that behavior at some point in the future so that even this (slightly odd :) series of calls works.

[Isomorphic]

Just a note that we're actually going to do this a bit differently - Criteria.addCriteria(criteria) will be changed to throw an error if the passed criteria is an AdvancedCriteria (or Criterion) rather than essentially corrupting the criteria as it currently does.

We're doing it this way because we don't want to have the Criteria internally turn into AdvancedCriteria and yet still have "instanceof AdvancedCriteria" return false.

[Isomorphic]

We've made these changes. Next smartgwt nightly build will throw an exception on method Criteria.addCriteria(criteria) if the passed criteria is an AdvancedCriteria or Criterion. Use Criteria.asAdvancedCriteria() to convert to AdvancedCriteria instead.

Regards
Isomorphic Software