Hi Andreas,

It would be great if you could explain your real use case and the reason you 
wish to introduce this (I guess you have a use case where you need this :)).

See below.

On Oct 12, 2012, at 9:27 AM, Andreas Jonsson <[email protected]> wrote:

> Hi everyone,
> 
> I would like to propose to introduce a possibility to associate
> properties in the execution context with metadata by declaring them.  We
> introduce the class 'ExecutionContextProperty' that will store the
> metadata attributes, in addition to the actual property value.
> 
>  /**
>   * Declare a property in the execution context.
>   *
>   * @param key The key that names the property
>   * @param property The property to declare.
>   */
>  public void declareProperty(String key, ExecutionContextProperty property)

Does this mean the current ExecutionContext.setProperty() method would be 
deprecated/removed?

Why not use instead: setProperty(String key, ECP property)?

> The metadata governs how the property should be treated when managing
> the execution contexts.  For instance, these attributes could be supported:
> 
> * Read-only  - The value may not be updated within the the execution
> context.

How do you ensure this? So getProperty() would continue to return the value and 
not an ECP instance right?

So when the user calls declareProperty() with the same key as an existing key 
it would be refused if it's been declared Readonly?

Now imagine that I pass an ECP instance with a value of List. How do you 
prevent some from doing: getProperty("mylistkey").add(somenewelement)?

> * Inherited  - The property will be inherited from the current context
> when replacing the execution context within the scope of a request.

I don't understand this one, we create a new Execution Context for each Request.

> * Clone      - Also clone the value when the execution context is cloned.

What are the cases when we don't want to clone a property? I don't see any, for 
me we always want to clone them and in practice we would instead need to change 
our current signature to: 

setProperty(String key, Cloneable value)

> * Type       - The class of the value, for typechecking when replacing
> the value.

Can you explain more? Does it mean you wish that getProperty() returns a typed 
value? We can already do this by using generics.

> * Non-null   - The value may not be null
> 
> So, the arguments for this proposal are at least two:
> 
> 1. Catch errors early and simplify debugging.

Can you explain more this point, I don't see the benefit over what we can 
already do.

> 2. Convenience of having properties automatically maintained across
> execution contexts within the same request cycle.

This is probably related to the inherit item which I don't understand.

> 
> What do you think?

Honestly I don't see the need for this.

What we lack IMO is proper Execution Context cloning and for this to happen we 
need to ensure we only cloneable items in the EC. As for the other "types" I 
don't really see a strong need but maybe I've misunderstood a few things (hence 
my questions above).

> I would like to implement and merge this ASAP.

We need to be very careful because the EC is a key class and introducing this 
means we're changing an important public API and we cannot go back easily 
(except through some deprecation cycles). IMO, seen the importance this should 
be a VOTE instead of a PROPOSAL.

We would really need some feedback from several devs before going on with this 
IMO. ATM I'd like to understand it more before voting.

Thanks
-Vincent

PS: Didn't get the time to read your implementation code yet

> Best Regards,
> 
> /Andreas

_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to