Hi,
2012-10-16 11:03, Vincent Massol skrev:
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 :)).
I want to put authorization information in the execution context, and I
want to trust that this information is carried over to any new execution
context that is activated in the scope of the current request. It is
also great if the object that holds the information cannot be replaced.
So, if you look at the code you will see that I have implemented a
rather aggressive enforcment of the combination of the attributes
read-only and inherited.
But also, the need for the inherited property already exists for
managing the properties "xwikicontext" and "velocityContext". Just
look
at the TODO and FIXME in DefaultExecutionContextManager in the master
branch.
But the general idea is just to be able to associate any kind of
metadata with each property. It can be compared to how interpreted
languages handle variables. Take for instance bash, where you can just
set a variable to a value, wich implicitly declares the variable. But
you can also explicitly declare a variable, and in the declaration you
can provide flags that sets various attributes of the variable. (e.g,
declare -r variable='readonly value'.)
See below.
On Oct 12, 2012, at 9:27 AM, Andreas Jonsson <aj(a)member.fsf.org> 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)?
Setting and declaring a property are completely different concepts. In
a purely declarative model, properties would have to be declared before
being set. But what I'm proposing is more in line with what bash does
with variables. Setting a value on an undefined property will
implicitly declare it with default values for all attributes. You can
also provide an initial value when declaring the property (which of
course is required for read-only properties).
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?
Yes, getProperty returns the value of the property. I have not added
any method to obtain the declared ECP instance as I don't see any need
for it right now.
Of course, it is only the reference to the value that is guaranteed to
be read-only by the execution context. Depending on the application, it
may be desirable to make the object immutable, but that is not up to the
execution context to enforce.
So when the user calls declareProperty() with the same key as an existing key it would be
refused if it's been declared Readonly?
A property may only be declared once in an execution context, so it will
be refused if the property is already declared. It doesn't matter if it
was an explicit or implicit declaration. Attempting to declare an
already existing property is an indication that something is wrong, so
raising an exception seems appropriate.
If the property is read-only, then also setProperty will refuse to
replace the value.
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.
No, you are mistaken. We allow the creation and activation of several
execution contexts per request. That's why we have the kludges in
DefaultExecutionContextManager for copying and cloning "xwikicontext"
and "velocityContext".
* 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)
I do not want to clone my authorization data. It wouldn't hurt cloning
it, but it would serve no purpose. But I can also imagine that some
applications might want to put objects that are expensive to clone in
the execution context.
* 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.
No, it just means that we can perform run-time type checking when
setting the value of a declared property. It would catch some errors
earlier, but not as early as compile time.
* 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.
You can let the execution context provide some basic validation when
setting it. If you mark a value read-only, you will not need to worry
about if any component suddenly replaces or remove it in the middle of
the request.
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.
Yes it is. ;)
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.
The proposal adds two methods and one class, while beeing backwards
compliant with current code. To me it seems that we have plenty of time
to change our minds before releasing 4.3-rc1 if I were to merge this
with master right now. Or is our policy that we have to commit to any
API that we have added in a milestone release?
I will propose a vote for it later in any event.
Best Regards,
/Andreas
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
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs