27 Oct
2012
27 Oct
'12
7:13 p.m.
Hi Andreas,
On Oct 26, 2012, at 2:07 PM, Andreas Jonsson <aj(a)member.fsf.org> wrote:
Hi Everyone,
I would like you to vote on merging the feature-execution-context
branches of commons and platform before the release of 4.3M2:
https://github.com/xwiki/xwiki-commons/compare/master...feature-execution-c…
https://github.com/xwiki/xwiki-platform/compare/master...feature-execution-…
Explanation of the feature:
The execution context is a simple map that binds a string to an object.
This can be compared to how variables are handled by scripting languages
such as bash, where an assignment brings the variable into existance:
$ my_variable="some value"
In the case of the execution context, the syntax is:
context.setProperty("my_variable", "some value");
This feature is about adding property declarations, where a property can
be associated with attributes that controls how the execution context
and execution context manager handles the property. The general idea
can, once again, be compared to bash and how it is possible to declare
variables there:
$ declare -ru my_variable="read only, forced upper case"
Of course, the set of attributes that are interesting to us is different
from bash. Currently the feature branch have support for declaring
properties with these attributes:
* Final - The value may not be updated within the the execution
context. Default: false.
* Inherited - The property will be inherited from the current
context when replacing the execution context within the current scope.
Default: false.
* Cloned value - Also clone the value when the
execution context is
cloned. Default: false.
ATM I believe that all our properties are "cloned" since when we clone the
context they are there. Won't this cause a backward compat issue? Shouldn't it be
true by default?
* Type - The class of the value, for typechecking when setting
the value. Default: null (unchecked).
* Non-null - The value may not be null, checked when setting the
value. Default: false.
Example declaration:
ExecutionContextProperty property = new
ExecutionContextProperty("my_variable");
property.setValue("some value");
property.setType(String.class);
property.setInherited(true);
context.declareProperty(property);
The API is very verbose just to declare one property… IMO it would be nice to have
something more compact.
As I've already commented I'd personally have preferred that the
ExecutionContextProperty and properties in general be immutable and set with one call to
context.setProperty(key, ECP) (i.e. no need to declare IMO). I find it more
straightforward and prevents forgetting to call declareProperty, which becomes really
critical to get the right behavior you want.
The property value may be updated and the property may
be removed and
redeclared, unless declared 'final':
context.setProperty("my_variable", "some new value");
context.removeProperty("my_variable");
In general immutability is better in term of performance/implementation.
Additional attributes may be added later. This
feature is also
backwards compliant, in that implicit declaration by just setting a
value is allowed. (Although we might want to make explicit declarations
mandatory in the future.)
Within the scope of a request, or the life time of a thread executing
some administrative task (e.g., the lucene index updater) the basic life
cycle of the execution context is the following:
@Inject
Execution execution;
No need for this injection in your code below :)
@Inject
ExecutionContextManager ecm;
ExecutionContext context = new ExecutionContext();
ecm.initialize(context);
try {
// Do work
} finally {
ecm.removeContext();
}
Within the life cycle of the "root" execution context, we may push a new
execution context, which may either be a clean context, or a clone of
the current context:
// Pushing a clone
ExecutionContext context = ecm.clone(execution.getContext());
<brainstorming mode>
Actually I wonder why we have both Execution and ECM. Shouldn't we have just one?
Also I wonder why we have to call ecm.clone() instead of implementing clone() in
ExecutionContext so that we would have:
ExecutionContext context = execution.getContext();
execution.pushContext(context.clone())
And to create an EC:
ExecutionContext ec = execution.createContext();
Ok maybe we would have a hard time with backward compat with this but for the sake of the
discussion, would that be something better?
</brainstorming mode>
execution.pushContext(context);
try {
// Do work
} finally {
execution.popContext();
}
// Pushing a clean context
ExecutionContext context = new ExecutionContext();
execution.pushContext(context);
try {
// Do work
} finally {
execution.popContext();
}
Component authors that needs to place a value in the execution context
provides an initializer that declares the variable and sets an initial
value.
The attributes 'final', 'cloned value', and 'inherited' lets
component
authors control how the value is managed during the lifecycle of the
root execution context.
The attributes 'type' and 'non-null' provides some runtime assertions to
catch some errors earlier.
So to summarize, this feature:
1. is a convenient mechanism for managing properties in the execution
context,
2. provides some validation of the property values,
3. improves performance slightly by avoiding unnecessary cloning.
For more information, see the proposal thread:
http://xwiki.475771.n2.nabble.com/PROPOSAL-Execution-context-property-decla…
+0 from me after we agree on the items above.
I'd really like to get feedback from other committers before you push this since
it's a really critical change (the Execution/EC is key and is going to become even
more and more important as we move away from XWikiContext).
Thanks
-Vincent