You are viewing a plain text version of this content. The canonical link for it is here.
Posted to agila-dev@incubator.apache.org by ja...@mac.com on 2005/01/18 19:27:11 UTC
[PATCH] adding getTasksForInstance() and setTaskStatusForInstance() to TaskService
Attached is a patch (which also includes my previous patch from Nov
30th for adding property setting of Nodes in the workflow XML) which
adds 2 new methods to TaskService...
/**
* Returns a list of tasks for the given workflow instance
* which are at the specified status
*/
List getTasksForInstance(InstanceID instanceID, int status);
/**
* Updates the status of all tasks of the oldStatus on the given
workflow instance
* to the newStatus. This method can be used to cancel/complete all
open tasks, or
* re-open all complete tasks etc.
*
* @param instanceID
* @param oldStatus the current status of the tasks
* @param newStatus the new status of the tasks
* @return the number of rows updated
*/
int setTaskStatusForInstance(InstanceID instanceID, int oldStatus,
int newStatus);
This allows you to look up all of the tasks (or open/closed/cancelled
tasks) for a given workflow instance. (I've added a new TASK_CANCELLED
status too). It also lets you update the status of the tasks for an
instance - for example, to complete all outstanding tasks on a workflow
instance - or cancel them all - or re-open all the complete tasks or
whatever.
This allows you to, say, close an existing workflow instance and reopen
another one with ease, without leaving around lots of old, dead tasks.
This patch also includes some minor fixes, to use the name 'status' for
parameters consistently, rather than 'type' and to ensure that if the
status Task.TASK_ALL is used on some of the methods (like
getTasksForUser(), getTasksForGroup() and so forth) that the
status-filter is not applied.
Re: [PATCH] expression evaluation, timer service
Posted by Geir Magnusson Jr <ge...@4quarters.com>.
On Mar 9, 2005, at 5:18 AM, Glenn J Gonzales wrote:
>
> Geir Magnusson Jr wrote:
>
>> Comments. re the resolver, this looks mainly like a refactoring of
>> what was there in the jexlResolver (which is fine - we should make
>> this pluggable so other ELs can be used)
>>
>> However, I'm not sure that we need to be fault tolerant - suppose
>> you are expecting a string back - then your indication of error, "",
>> is a valid result, and you'll never know something went wrong. I
>> think that we should toss an exception rather than color the
>> interpretation of the returned result.
>
> That's right. I thought about this too but I saw that
> get/setBoundValue() in NodeContext interface doesn't declare thrown
> exceptions, and I thought the intention was to keep the evaluation
> "clean" (i.e., be fault-tolerant, since getBoundValue() returns the
> empty string when the evaluation causes an exception),
That's not fault tolerant, IMO - it really is confusing, as it doesn't
only return strings...
> because in NodeContextImpl it's the one that handles the exceptions
> from the evaluation itself. Letting these methods
> (get/setBoundValue()) declare thrown Exception(s) allows the invoking
> node to choose its own course of action,which may include a
> pass-through, or a call to some application-specific service that
> would know what to do.
Yep
>
>>
>> 2) Why do we need to surround expressions with ${}? Shouldn't we
>> just have the expression?
>
> I tried that before -- works, and probably makes better sense;
> however, it requires that you formulate expressions java-like (can get
> ugly), for example:
>
> <binding name="message" type="static" datatype="String" input="true"
> output="false"><![CDATA["Task for : " + user.name()]]></binding>
>
> It's similarly awkward in the case of a jdbc-based activity. An sql
> statement parameter may look like: "update customers set status = '" +
> cust.getStatus() + "', department = '" + cust.getDept() + "' where
> customer_id = " + cust.getId(), whereas if we make it EL-like we
> simply pass: update customers set status = '${cust.getStatus()}',
> department = '${cust.getDept()}' where customer_id = ${cust.getId()}
> (of course we can use prepared statements instead, but that would only
> be for jdbc-based activities of a specific functionality, not for a
> general jdbc-access activity where you can enter any sql along with
> other jdbc details).
>
> <binding name="sql" type="el" datatype="String" input="true"
> output="false"><![CDATA["update customers set status = '" +
> cust.getStatus() + "', department = '" + cust.getDept() + "' where
> customer_id = " + cust.getId()]]></binding>
>
> as opposed to:
>
> <binding name="sql" type="el" datatype="String" input="true"
> output="false"><![CDATA[update customers set status =
> '${cust.getStatus()}', department = '${cust.getDept()}' where
> customer_id = ${cust.getId()}]]></binding>
Hm.
>
>>>
>>> Index: org/apache/agila/impl/NodeContextImpl.java
>>> ===================================================================
>>> --- org/apache/agila/impl/NodeContextImpl.java (revision 156357)
>>> +++ org/apache/agila/impl/NodeContextImpl.java (working copy)
>>> @@ -25,6 +25,7 @@
>>> import org.apache.agila.services.notification.NotificationService;
>>> import org.apache.agila.services.task.TaskService;
>>> import org.apache.agila.services.TimerService;
>>> +import org.apache.agila.util.Evaluator;
>>> import org.apache.commons.jexl.Expression;
>>> import org.apache.commons.jexl.ExpressionFactory;
>>> import org.apache.commons.jexl.JexlContext;
>>> @@ -83,14 +84,8 @@
>>> return binding.getValue();
>>>
>>> case Binding.EL :
>>> - try {
>>> - return jexlResolver(binding.getValue(),
>>> this.getInstanceData());
>>> - }
>>> - catch(Exception e) {
>>> - // TODO somethig useful
>>> - e.printStackTrace();
>>> - return "";
>>> - }
>>> + return Evaluator.evaluate( binding.getValue(),
>>> this.getInstanceData() );
>>> +
>>> default :
>>> return binding.getValue();
>>> }
>>> @@ -111,17 +106,25 @@
>>>
>>> case Binding.EL :
>>> try {
>>> - /*
>>> - * Awful hack - need to fix so exprs can be used
>>> - * for now, assume a scalar reference and just
>>> set it
>>> - */
>>>
>>> - this.getInstanceData().put(binding.getName(),
>>> value);
>>> + if(value != null) {
>>> +
>>> this.getInstanceData().put(binding.getName(), Evaluator.evaluate(
>>> value.toString(), this.getInstanceData() ));
>>> + }
>>> + // else no op?
>>> }
>>
>>
>> This I don't get. IIRC the code correctly, here's the situation.
>> You have a binding
>>
>> suppose foo is the name, and "a.name()" is the expression value.
>>
>> I understand what it means to get the bound value of "foo" - you run
>> "a.name()" through the resolver, and get the value (or null)
>> depending on the state of the context, at that time.
>>
>> But to *put* the value for "foo"? I can't figure out why I'd do
>>
>> put("foo", "a.name()")
>>
>> if I wanted the expression to be evaluated at that moment, because I
>> could just do
>>
>> put("foo", a.name() )
>>
>> right?
>
> Yes, but that's just it -- if the node wanted to evaluate it at that
> time, then the value is put directly into the instance data, but if
> the node wants to pass it off for evaluation later, then maybe the
> expression would be what you want (a scenario would be if the value
> returned may change from one node execution to the next, like a
> balance account value -- dunno if that's a good example though :) ).
Right - but your patch evaluates the expression and puts the result in.
>
>>
>> One could argue that a node may want to work with an expression w/in
>> the context of that node's execution, and put the resulting value in
>> the context (which is what your proposed patch does), but I don't
>> think that's right. I think we lose too much functionality - the
>> ability to pass an expression onward so it can be evaluated at the
>> time of execution of another node.
>>
> Argh, I've interchanged the cases. The code above should have been for
> bindings with type "STATIC", because for these types of bindings, you
> want to set them to the actual (resolved) value of the expression,
> while for type "EL", you want to set it for evaluation later (code
> below). Given that the code above and directly below are interchanged
> for the cases, I think it fulfills the functionality you mentioned
> nicely; however, with regards to the "output" dynamics of the binding,
> this imposes some level of consistency, because if a node has an EL
> binding that declares an output, it is expected that the value it puts
> into the instance data will be an expression, and if it's of STATIC
> type, then the value will be the resolved value.
yes
>
>> if we do need to have nodes deal w/ expressions, we should just make
>> the expression evaluator available to it directly.
>>
> The evaluator can also be used directly since these are public static
> methods, but I think the node context should still be able to provide
> more general evaluation capability.
Remember, the evaluator should be pluggable - a node shouldn't assume
the kind of expressions, nor should it care. IOW, an activity
shouldn't have an import or reference to a given expression evaluator
class, but rather asked the node context for the appropriate one. Or
such.
>
> I have attached a redo.
Ok
>
> Cheers,
> Glenn
>
>>
>>
>>> catch(Exception e) {
>>> // TODO somethig useful
>>> e.printStackTrace();
>>> }
>>> +
>>> + break;
>>> +
>>> + case Binding.STATIC :
>>> +
>>> + if(value != null)
>>> this.getInstanceData().put(binding.getName(), value);
>>> +
>>> + break;
>>> +
>>> default :
>>> // TODO urg...
>>> }
>>
>
> Index: org/apache/agila/util/Evaluator.java
> ===================================================================
> --- org/apache/agila/util/Evaluator.java (revision 0)
> +++ org/apache/agila/util/Evaluator.java (revision 0)
> @@ -0,0 +1,106 @@
> +package org.apache.agila.util;
> +
> +import org.apache.commons.jexl.Expression;
> +import org.apache.commons.jexl.ExpressionFactory;
> +import org.apache.commons.jexl.JexlContext;
> +import org.apache.commons.jexl.JexlHelper;
> +
> +import java.util.Map;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Iterator;
> +
> +/**
> + * @author <a href="mailto:ggonzales@exist.com">Glenn J Gonzales.</a>
> + */
> +public class Evaluator {
> +
> + /**
> + * Reads a line of text and evaluates it using the data in
> <code>params</code>. Uses Commons-Jexl
> + * to execute the evaluation.
> + *
> + * @param line
> + * @param params
> + * @return If the text is a single EL-like expression, then the
> corresponding object value of the
> + * expression is returned, otherwise (i.e., if the text contains
> template text or more than one
> + * EL-like expression) this returns a concatenation of the string
> representation of the values.
> + */
> + public static Object evaluate(String line, Map params) throws
> Exception {
> +
> + if(line == null) return "";
> +
> + List store = new ArrayList();
> +
> + int indexExprStart = line.indexOf("${");
> +
> + if(indexExprStart != -1) {
> +
> + if(indexExprStart > 0) store.add(line.substring(0,
> indexExprStart));
> +
> + int indexExprEnd = line.indexOf("}", indexExprStart + 2);
> +
> + if(indexExprEnd != -1) {
> +
> + String expr = line.substring(indexExprStart + 2,
> indexExprEnd);
> +
> + Object value = resolve( expr, params );
> +
> + store.add(value);
> +
> + if(indexExprEnd != line.length() - 1) {
> + store.add( evaluate(line.substring(indexExprEnd +
> 1), params) );
> + }
> + }
> + else {
> + return evaluateStore(store);
> + }
> + }
> + else {
> + store.add(line);
> + }
> +
> + return evaluateStore(store);
> + }
> +
> + /**
> + * If <code>store</code> contains more than one element, then
> return the concatenation of the string
> + * representation of the objects; otherwise, return the lone
> object.
> + *
> + * @param store
> + * @return
> + */
> + private static Object evaluateStore(List store) {
> +
> + if(store.size() > 1) {
> +
> + StringBuffer buffer = new StringBuffer();
> +
> + for(Iterator contents = store.iterator();
> contents.hasNext(); ) {
> + buffer.append( "" + contents.next() );
> + }
> +
> + return buffer.toString();
> + }
> + else {
> + return store.get(0);
> + }
> + }
> +
> + /**
> + * Resolve the expression using the <code>params</code> data.
> + *
> + * @param expr
> + * @param params
> + * @return
> + */
> + private static Object resolve(String expr, Map params) throws
> Exception {
> +
> + Expression jexlExpr =
> ExpressionFactory.createExpression(expr);
> +
> + JexlContext jexlContext = JexlHelper.createContext();
> +
> + jexlContext.setVars(params);
> +
> + return jexlExpr.evaluate(jexlContext);
> + }
> +}
> Index: org/apache/agila/model/node/TestNotification.java
> ===================================================================
> --- org/apache/agila/model/node/TestNotification.java (revision
> 156357)
> +++ org/apache/agila/model/node/TestNotification.java (working copy)
> @@ -38,9 +38,17 @@
> */
> public Connection[] doEnd(NodeContext ctx) {
>
> - String val = (String) ctx.getBoundValue("message");
> + try {
> + String val = (String) ctx.getBoundValue("message");
> + ctx.getNotificationService().notify(new UserID(1), val);
> + }
> + catch (Exception e) {
> + // TODO log?
> + e.printStackTrace();
> + }
>
> - ctx.getNotificationService().notify(new UserID(1), val);
> + // becomes a no-op in case of an exception in evaluation
> +
> return this.getOutboundConnections();
> }
> }
> Index: org/apache/agila/model/node/SimpleDecision.java
> ===================================================================
> --- org/apache/agila/model/node/SimpleDecision.java (revision
> 156357)
> +++ org/apache/agila/model/node/SimpleDecision.java (working copy)
> @@ -27,7 +27,14 @@
> /*
> * get the value
> */
> - Object o = ctx.getBoundValue("value");
> + Object o = null;
> +
> + try {
> + o = ctx.getBoundValue("value");
> + }
> + catch (Exception e) {
> + e.printStackTrace();
> + }
>
> boolean b = false;
>
> Index: org/apache/agila/model/node/TestTaskNode.java
> ===================================================================
> --- org/apache/agila/model/node/TestTaskNode.java (revision
> 156357)
> +++ org/apache/agila/model/node/TestTaskNode.java (working copy)
> @@ -59,8 +59,15 @@
> if ("APPROVE".equals(name.toUpperCase())) {
> b = true;
> }
> - ctx.setBoundValue("approved", new Boolean(b));
>
> + try {
> + ctx.setBoundValue("approved", new Boolean(b));
> + }
> + catch (Exception e) {
> + // do output for now
> + e.printStackTrace();
> + }
> +
> return getOutboundConnections();
> }
>
> Index: org/apache/agila/model/node/HelloWorldActivity.java
> ===================================================================
> --- org/apache/agila/model/node/HelloWorldActivity.java (revision
> 156357)
> +++ org/apache/agila/model/node/HelloWorldActivity.java (working copy)
> @@ -30,8 +30,15 @@
> public Connection[] doEnd(NodeContext ctx) {
> System.out.println("HelloWorldActivity : hello world");
>
> - String val = (String) ctx.getBoundValue("message");
> + String val = null;
>
> + try {
> + val = (String) ctx.getBoundValue("message");
> + }
> + catch (Exception e) {
> + e.printStackTrace();
> + }
> +
> System.out.println("HelloWorldActivity : message = " + val);
>
> return getOutboundConnections();
> Index: org/apache/agila/example/LeaveApplicationTask.java
> ===================================================================
> --- org/apache/agila/example/LeaveApplicationTask.java (revision
> 156357)
> +++ org/apache/agila/example/LeaveApplicationTask.java (working copy)
> @@ -94,9 +94,16 @@
> * the bindings
> */
>
> - ctx.setBoundValue(NUMDAYS, taskData.numDays);
> - ctx.setBoundValue(REASON, taskData.reason);
> + try {
> + ctx.setBoundValue(NUMDAYS, taskData.numDays);
> + ctx.setBoundValue(REASON, taskData.reason);
> + }
> + catch (Exception e) {
> + // do output for now
> + e.printStackTrace();
> + }
>
> +
> return getOutboundConnections();
> }
> Index: org/apache/agila/model/NodeContext.java
> ===================================================================
> --- org/apache/agila/model/NodeContext.java (revision 156357)
> +++ org/apache/agila/model/NodeContext.java (working copy)
> @@ -31,8 +31,8 @@
> */
> public interface NodeContext {
>
> - public Object getBoundValue(String name);
> - public void setBoundValue(String name, Object value);
> + public Object getBoundValue(String name) throws Exception;
> + public void setBoundValue(String name, Object value) throws
> Exception;
>
> public Map getInstanceData();
> Index: org/apache/agila/impl/NodeContextImpl.java
> ===================================================================
> --- org/apache/agila/impl/NodeContextImpl.java (revision 156357)
> +++ org/apache/agila/impl/NodeContextImpl.java (working copy)
> @@ -25,6 +25,7 @@
> import org.apache.agila.services.notification.NotificationService;
> import org.apache.agila.services.task.TaskService;
> import org.apache.agila.services.TimerService;
> +import org.apache.agila.util.Evaluator;
> import org.apache.commons.jexl.Expression;
> import org.apache.commons.jexl.ExpressionFactory;
> import org.apache.commons.jexl.JexlContext;
> @@ -67,7 +68,7 @@
> * @param name
> * @return
> */
> - public Object getBoundValue(String name) {
> + public Object getBoundValue(String name) throws Exception {
>
> Map bm = node.getBindings();
>
> @@ -83,20 +84,14 @@
> return binding.getValue();
>
> case Binding.EL :
> - try {
> - return jexlResolver(binding.getValue(),
> this.getInstanceData());
> - }
> - catch(Exception e) {
> - // TODO somethig useful
> - e.printStackTrace();
> - return "";
> - }
> + return Evaluator.evaluate( binding.getValue(),
> this.getInstanceData() );
> +
> default :
> return binding.getValue();
> }
> }
>
> - public void setBoundValue(String name, Object value) {
> + public void setBoundValue(String name, Object value) throws
> Exception {
>
> Map bm = node.getBindings();
>
> @@ -109,19 +104,16 @@
>
> switch(binding.getType()) {
>
> + case Binding.STATIC :
> + this.getInstanceData().put( binding.getName(),
> Evaluator.evaluate( value.toString(), this.getInstanceData()) );
> +
> + break;
> +
> case Binding.EL :
> - try {
> - /*
> - * Awful hack - need to fix so exprs can be used
> - * for now, assume a scalar reference and just
> set it
> - */
> + if(value != null)
> this.getInstanceData().put(binding.getName(), value);
>
> - this.getInstanceData().put(binding.getName(),
> value);
> - }
> - catch(Exception e) {
> - // TODO somethig useful
> - e.printStackTrace();
> - }
> + break;
> +
> default :
> // TODO urg...
> }
--
Geir Magnusson Jr +1-203-665-6437
geir@gluecode.com
Re: [PATCH] expression evaluation, timer service
Posted by Glenn J Gonzales <gg...@exist.com>.
Hi Geir, great to hear from you! Heard you were busy with Geronimo :)
Geir Magnusson Jr wrote:
> Comments. re the resolver, this looks mainly like a refactoring of
> what was there in the jexlResolver (which is fine - we should make
> this pluggable so other ELs can be used)
>
> However, I'm not sure that we need to be fault tolerant - suppose you
> are expecting a string back - then your indication of error, "", is a
> valid result, and you'll never know something went wrong. I think
> that we should toss an exception rather than color the interpretation
> of the returned result.
That's right. I thought about this too but I saw that
get/setBoundValue() in NodeContext interface doesn't declare thrown
exceptions, and I thought the intention was to keep the evaluation
"clean" (i.e., be fault-tolerant, since getBoundValue() returns the
empty string when the evaluation causes an exception), because in
NodeContextImpl it's the one that handles the exceptions from the
evaluation itself. Letting these methods (get/setBoundValue()) declare
thrown Exception(s) allows the invoking node to choose its own course of
action,which may include a pass-through, or a call to some
application-specific service that would know what to do.
>
> 2) Why do we need to surround expressions with ${}? Shouldn't we just
> have the expression?
I tried that before -- works, and probably makes better sense; however,
it requires that you formulate expressions java-like (can get ugly), for
example:
<binding name="message" type="static" datatype="String" input="true"
output="false"><![CDATA["Task for : " + user.name()]]></binding>
It's similarly awkward in the case of a jdbc-based activity. An sql
statement parameter may look like: "update customers set status = '" +
cust.getStatus() + "', department = '" + cust.getDept() + "' where
customer_id = " + cust.getId(), whereas if we make it EL-like we simply
pass: update customers set status = '${cust.getStatus()}', department =
'${cust.getDept()}' where customer_id = ${cust.getId()} (of course we
can use prepared statements instead, but that would only be for
jdbc-based activities of a specific functionality, not for a general
jdbc-access activity where you can enter any sql along with other jdbc
details).
<binding name="sql" type="el" datatype="String" input="true"
output="false"><![CDATA["update customers set status = '" +
cust.getStatus() + "', department = '" + cust.getDept() + "' where
customer_id = " + cust.getId()]]></binding>
as opposed to:
<binding name="sql" type="el" datatype="String" input="true"
output="false"><![CDATA[update customers set status =
'${cust.getStatus()}', department = '${cust.getDept()}' where
customer_id = ${cust.getId()}]]></binding>
>>
>> Index: org/apache/agila/impl/NodeContextImpl.java
>> ===================================================================
>> --- org/apache/agila/impl/NodeContextImpl.java (revision 156357)
>> +++ org/apache/agila/impl/NodeContextImpl.java (working copy)
>> @@ -25,6 +25,7 @@
>> import org.apache.agila.services.notification.NotificationService;
>> import org.apache.agila.services.task.TaskService;
>> import org.apache.agila.services.TimerService;
>> +import org.apache.agila.util.Evaluator;
>> import org.apache.commons.jexl.Expression;
>> import org.apache.commons.jexl.ExpressionFactory;
>> import org.apache.commons.jexl.JexlContext;
>> @@ -83,14 +84,8 @@
>> return binding.getValue();
>>
>> case Binding.EL :
>> - try {
>> - return jexlResolver(binding.getValue(),
>> this.getInstanceData());
>> - }
>> - catch(Exception e) {
>> - // TODO somethig useful
>> - e.printStackTrace();
>> - return "";
>> - }
>> + return Evaluator.evaluate( binding.getValue(),
>> this.getInstanceData() );
>> +
>> default :
>> return binding.getValue();
>> }
>> @@ -111,17 +106,25 @@
>>
>> case Binding.EL :
>> try {
>> - /*
>> - * Awful hack - need to fix so exprs can be used
>> - * for now, assume a scalar reference and just
>> set it
>> - */
>>
>> - this.getInstanceData().put(binding.getName(),
>> value);
>> + if(value != null) {
>> +
>> this.getInstanceData().put(binding.getName(), Evaluator.evaluate(
>> value.toString(), this.getInstanceData() ));
>> + }
>> + // else no op?
>> }
>
>
> This I don't get. IIRC the code correctly, here's the situation. You
> have a binding
>
> suppose foo is the name, and "a.name()" is the expression value.
>
> I understand what it means to get the bound value of "foo" - you run
> "a.name()" through the resolver, and get the value (or null) depending
> on the state of the context, at that time.
>
> But to *put* the value for "foo"? I can't figure out why I'd do
>
> put("foo", "a.name()")
>
> if I wanted the expression to be evaluated at that moment, because I
> could just do
>
> put("foo", a.name() )
>
> right?
Yes, but that's just it -- if the node wanted to evaluate it at that
time, then the value is put directly into the instance data, but if the
node wants to pass it off for evaluation later, then maybe the
expression would be what you want (a scenario would be if the value
returned may change from one node execution to the next, like a balance
account value -- dunno if that's a good example though :) ).
>
> One could argue that a node may want to work with an expression w/in
> the context of that node's execution, and put the resulting value in
> the context (which is what your proposed patch does), but I don't
> think that's right. I think we lose too much functionality - the
> ability to pass an expression onward so it can be evaluated at the
> time of execution of another node.
>
Argh, I've interchanged the cases. The code above should have been for
bindings with type "STATIC", because for these types of bindings, you
want to set them to the actual (resolved) value of the expression, while
for type "EL", you want to set it for evaluation later (code below).
Given that the code above and directly below are interchanged for the
cases, I think it fulfills the functionality you mentioned nicely;
however, with regards to the "output" dynamics of the binding, this
imposes some level of consistency, because if a node has an EL binding
that declares an output, it is expected that the value it puts into the
instance data will be an expression, and if it's of STATIC type, then
the value will be the resolved value.
> if we do need to have nodes deal w/ expressions, we should just make
> the expression evaluator available to it directly.
>
The evaluator can also be used directly since these are public static
methods, but I think the node context should still be able to provide
more general evaluation capability.
I have attached a redo.
Cheers,
Glenn
>
>
>> catch(Exception e) {
>> // TODO somethig useful
>> e.printStackTrace();
>> }
>> +
>> + break;
>> +
>> + case Binding.STATIC :
>> +
>> + if(value != null)
>> this.getInstanceData().put(binding.getName(), value);
>> +
>> + break;
>> +
>> default :
>> // TODO urg...
>> }
>
Re: [PATCH] expression evaluation, timer service
Posted by Geir Magnusson Jr <ge...@4quarters.com>.
Comments. re the resolver, this looks mainly like a refactoring of
what was there in the jexlResolver (which is fine - we should make this
pluggable so other ELs can be used)
However, I'm not sure that we need to be fault tolerant - suppose you
are expecting a string back - then your indication of error, "", is a
valid result, and you'll never know something went wrong. I think that
we should toss an exception rather than color the interpretation of the
returned result.
2) Why do we need to surround expressions with ${}? Shouldn't we just
have the expression?
More inline. I don't like the expression/node context change for "put"
(see below) and love the timer suggestion. Will commit that.
geir
On Mar 8, 2005, at 4:07 AM, Glenn J Gonzales wrote:
> Hi,
>
> I'm submitting a patch for evaluation of expressions from parameter
> binding values. It uses commons-jexl and accepts a String expression.
> The code tries to be fault-tolerant by returning the empty string if
> no valid evaluation can be performed. Also here are adjustments to
> NodeContextImpl.java (get/setBoundValue). I've also included a patch
> for TimerService and TimerServiceImpl (set the message token id to the
> passed-in value; also changed the signature to allow the node
> associated with the token to be given data upon execution from the
> service).
>
> Cheers,
> Glenn
> Index: org/apache/agila/util/Evaluator.java
> ===================================================================
> --- org/apache/agila/util/Evaluator.java (revision 0)
> +++ org/apache/agila/util/Evaluator.java (revision 0)
> @@ -0,0 +1,118 @@
> +package org.apache.agila.util;
> +
> +import org.apache.commons.jexl.Expression;
> +import org.apache.commons.jexl.ExpressionFactory;
> +import org.apache.commons.jexl.JexlContext;
> +import org.apache.commons.jexl.JexlHelper;
> +
> +import java.util.Map;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Iterator;
> +
> +/**
> + * @author <a href="mailto:ggonzales@exist.com">Glenn J Gonzales.</a>
> + */
> +public class Evaluator {
> +
> + /**
> + * Reads a line of text and evaluates it using the data in
> <code>params</code>. Uses Commons-Jexl
> + * to execute the evaluation.
> + *
> + * @param line
> + * @param params
> + * @return If the text is a single EL-like expression, then the
> corresponding object value of the
> + * expression is returned, otherwise (i.e., if the text contains
> template text or more than one
> + * EL-like expression) this returns a concatenation of the string
> representation of the values.
> + */
> + public static Object evaluate(String line, Map params) {
> +
> + if(line == null) return "";
> +
> + List store = new ArrayList();
> +
> + int indexExprStart = line.indexOf("${");
> +
> + if(indexExprStart != -1) {
> +
> + if(indexExprStart > 0) store.add(line.substring(0,
> indexExprStart));
> +
> + int indexExprEnd = line.indexOf("}", indexExprStart + 2);
> +
> + if(indexExprEnd != -1) {
> +
> + String expr = line.substring(indexExprStart + 2,
> indexExprEnd);
> +
> + Object value = resolve( expr, params );
> +
> + store.add(value);
> +
> + if(indexExprEnd != line.length() - 1) {
> + Object returnObj =
> evaluate(line.substring(indexExprEnd + 1), params);
> + store.add( returnObj != null ? returnObj : "" );
> + }
> + }
> + else {
> + return evaluateStore(store);
> + }
> + }
> + else {
> + store.add(line);
> + }
> +
> + return evaluateStore(store);
> + }
> +
> + /**
> + * If <code>store</code> contains more than one element, then
> return the concatenation of the string
> + * representation of the objects; otherwise, return the lone
> object.
> + *
> + * @param store
> + * @return
> + */
> + private static Object evaluateStore(List store) {
> +
> + if(store.size() > 1) {
> +
> + StringBuffer buffer = new StringBuffer();
> +
> + for(Iterator contents = store.iterator();
> contents.hasNext(); ) {
> + buffer.append( contents.next().toString() );
> + }
> +
> + return buffer.toString();
> + }
> + else {
> + return store.get(0);
> + }
> + }
> +
> + /**
> + * Resolve the expression using the <code>params</code> data.
> + *
> + * @param expr
> + * @param params
> + * @return
> + */
> + private static Object resolve(String expr, Map params) {
> +
> + // make it fault tolerant
> + Object obj = new String("");
> +
> + try {
> + Expression jexlExpr =
> ExpressionFactory.createExpression(expr);
> +
> + JexlContext jexlContext = JexlHelper.createContext();
> +
> + jexlContext.setVars(params);
> +
> + obj = jexlExpr.evaluate(jexlContext);
> + }
> + catch (Exception e) {
> + // TODO log?
> + //e.printStackTrace();
> + }
> +
> + return obj;
> + }
> +}
> Index: org/apache/agila/impl/NodeContextImpl.java
> ===================================================================
> --- org/apache/agila/impl/NodeContextImpl.java (revision 156357)
> +++ org/apache/agila/impl/NodeContextImpl.java (working copy)
> @@ -25,6 +25,7 @@
> import org.apache.agila.services.notification.NotificationService;
> import org.apache.agila.services.task.TaskService;
> import org.apache.agila.services.TimerService;
> +import org.apache.agila.util.Evaluator;
> import org.apache.commons.jexl.Expression;
> import org.apache.commons.jexl.ExpressionFactory;
> import org.apache.commons.jexl.JexlContext;
> @@ -83,14 +84,8 @@
> return binding.getValue();
>
> case Binding.EL :
> - try {
> - return jexlResolver(binding.getValue(),
> this.getInstanceData());
> - }
> - catch(Exception e) {
> - // TODO somethig useful
> - e.printStackTrace();
> - return "";
> - }
> + return Evaluator.evaluate( binding.getValue(),
> this.getInstanceData() );
> +
> default :
> return binding.getValue();
> }
> @@ -111,17 +106,25 @@
>
> case Binding.EL :
> try {
> - /*
> - * Awful hack - need to fix so exprs can be used
> - * for now, assume a scalar reference and just
> set it
> - */
>
> - this.getInstanceData().put(binding.getName(),
> value);
> + if(value != null) {
> + this.getInstanceData().put(binding.getName(),
> Evaluator.evaluate( value.toString(), this.getInstanceData() ));
> + }
> + // else no op?
> }
This I don't get. IIRC the code correctly, here's the situation. You
have a binding
suppose foo is the name, and "a.name()" is the expression value.
I understand what it means to get the bound value of "foo" - you run
"a.name()" through the resolver, and get the value (or null) depending
on the state of the context, at that time.
But to *put* the value for "foo"? I can't figure out why I'd do
put("foo", "a.name()")
if I wanted the expression to be evaluated at that moment, because I
could just do
put("foo", a.name() )
right?
One could argue that a node may want to work with an expression w/in
the context of that node's execution, and put the resulting value in
the context (which is what your proposed patch does), but I don't think
that's right. I think we lose too much functionality - the ability to
pass an expression onward so it can be evaluated at the time of
execution of another node.
if we do need to have nodes deal w/ expressions, we should just make
the expression evaluator available to it directly.
> catch(Exception e) {
> // TODO somethig useful
> e.printStackTrace();
> }
> +
> + break;
> +
> + case Binding.STATIC :
> +
> + if(value != null)
> this.getInstanceData().put(binding.getName(), value);
> +
> + break;
> +
> default :
> // TODO urg...
> }Index: org/apache/agila/services/TimerService.java
> ===================================================================
> --- org/apache/agila/services/TimerService.java (revision 156357)
> +++ org/apache/agila/services/TimerService.java (working copy)
> @@ -18,6 +18,8 @@
>
> import org.apache.agila.engine.TokenID;
>
> +import java.io.Serializable;
> +
> /**
> *
> * @author <a href="mailto:geir@gluecode.com">Geir Magnusson Jr.</a>
> @@ -25,5 +27,5 @@
> */
> public interface TimerService {
>
> - public void setExecutionContinue(TokenID tokenID, int seconds);
> + public void setExecutionContinue(TokenID tokenID, int seconds,
> String dataKey, Serializable appData);
> }
> Index: org/apache/agila/impl/memory/TimerServiceImpl.java
> ===================================================================
> --- org/apache/agila/impl/memory/TimerServiceImpl.java (revision
> 156357)
> +++ org/apache/agila/impl/memory/TimerServiceImpl.java (working copy)
> @@ -23,6 +23,8 @@
> import org.apache.agila.impl.EngineMessageImpl;
> import EDU.oswego.cs.dl.util.concurrent.ClockDaemon;
>
> +import java.io.Serializable;
> +
> /**
> *
> * @author <a href="mailto:geir@gluecode.com">Geir Magnusson Jr.</a>
> @@ -37,7 +39,7 @@
> this.queue = qs;
> }
>
> - public void setExecutionContinue(final TokenID tokenID, int
> seconds) {
> + public void setExecutionContinue(final TokenID tokenID, int
> seconds, final String dataKey, final Serializable data) {
>
> daemon.executeAfterDelay(1000L * seconds,
>
> @@ -46,7 +48,12 @@
> public void run() {
> EngineMessage em = new EngineMessageImpl();
>
> - em.setCurrentTokenID(null);
> + //em.setCurrentTokenID(null);
> +
> + em.addAppData(dataKey, data);
> +
> + em.setCurrentTokenID(tokenID);
> +
> queue.enqueue(em);
> }
> }
--
Geir Magnusson Jr +1-203-665-6437
geir@gluecode.com
[PATCH] expression evaluation, timer service
Posted by Glenn J Gonzales <gg...@exist.com>.
Hi,
I'm submitting a patch for evaluation of expressions from parameter
binding values. It uses commons-jexl and accepts a String expression.
The code tries to be fault-tolerant by returning the empty string if no
valid evaluation can be performed. Also here are adjustments to
NodeContextImpl.java (get/setBoundValue). I've also included a patch for
TimerService and TimerServiceImpl (set the message token id to the
passed-in value; also changed the signature to allow the node associated
with the token to be given data upon execution from the service).
Cheers,
Glenn
Re: [PATCH] adding getTasksForInstance() and setTaskStatusForInstance() to TaskService
Posted by Geir Magnusson Jr <ge...@4quarters.com>.
applied
On Jan 18, 2005, at 2:27 PM, jastrachan@mac.com wrote:
> Attached is a patch (which also includes my previous patch from Nov
> 30th for adding property setting of Nodes in the workflow XML) which
> adds 2 new methods to TaskService...
>
>
> /**
> * Returns a list of tasks for the given workflow instance
> * which are at the specified status
> */
> List getTasksForInstance(InstanceID instanceID, int status);
>
> /**
> * Updates the status of all tasks of the oldStatus on the given
> workflow instance
> * to the newStatus. This method can be used to cancel/complete
> all open tasks, or
> * re-open all complete tasks etc.
> *
> * @param instanceID
> * @param oldStatus the current status of the tasks
> * @param newStatus the new status of the tasks
> * @return the number of rows updated
> */
> int setTaskStatusForInstance(InstanceID instanceID, int oldStatus,
> int newStatus);
>
>
>
> This allows you to look up all of the tasks (or open/closed/cancelled
> tasks) for a given workflow instance. (I've added a new TASK_CANCELLED
> status too). It also lets you update the status of the tasks for an
> instance - for example, to complete all outstanding tasks on a
> workflow instance - or cancel them all - or re-open all the complete
> tasks or whatever.
>
> This allows you to, say, close an existing workflow instance and
> reopen another one with ease, without leaving around lots of old, dead
> tasks.
>
> This patch also includes some minor fixes, to use the name 'status'
> for parameters consistently, rather than 'type' and to ensure that if
> the status Task.TASK_ALL is used on some of the methods (like
> getTasksForUser(), getTasksForGroup() and so forth) that the
> status-filter is not applied.
>
> <patch.txt>
>
> James
> -------
> http://radio.weblogs.com/0112098/
--
Geir Magnusson Jr +1-203-665-6437
geir@gluecode.com
[PATCH] ActorResolver.java for user/group-actor resolution, notification,
task services
Posted by Glenn J Gonzales <gg...@exist.com>.
I'm submitting a number of patches which includes an actor resolver
interface that is used to resolve actors to users and/or groups. This
also includes a patch for the notification and task services that uses
the resolver, and one for a utility class that starts up the Agila
engine and sets up the services according to a configuration file.
-- The actor resolver handles the mapping of actors to users and actors
to groups. I've written a simple implementation that uses a property file.
-- Added the following to TaskService:
/**
* Finds out if the task can be performed by the user.
*
* @param userID
* @return true if the task is not locked to another user, or if the
task has already been
* assigned to the given user; false otherwise.
*/
boolean isTaskDoable(TaskID taskID, UserID userID);
This allows the application to query the service if the user can still
do the task, for example, in the situation where the task has been
submitted to a group (the actor resolved to a group, so the members of
the group are able to view it in their task lists), and one of them
starts the task, and then another tries to do the task again.
-- Added the following to NotificationService:
/**
* Create a notification for the actor parameter. Actor may denote a
single user or a group.
* If group, notifications will be sent to each member in the group.
*
* @param actor
* @param message
* @return
*/
NotificationID[] notify(Actor actor, String message);
Currently the node creates a notification through:
NotificationService.notify(UserID userId, String message) but where to
get the user id? The only entity that the node has access to (and which
makes sense) is the actor, hence this method.
-- Also added the following to TaskService for the same reason:
/**
* Assigns a new task to the actor parameter. An actor may resolve
to an individual user
* or a group of users.
*
* @param tokenId
* @param message
* @param actor
* @param due
* @return
*/
TaskID assignTask(TokenID tokenId, String message, Actor actor, Date
due);
I also overloaded assignTask() to take a group id (changed
assignTaskToTeam()).
Cheers,
Glenn