Hi Andreas, On Oct 26, 2012, at 2:07 PM, Andreas Jonsson <[email protected]> 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-context-metadata > https://github.com/xwiki/xwiki-platform/compare/master...feature-execution-context-metadata > > 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-declarations-and-property-metadata-attributes-td7581766.html +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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
