Bug 432297 - add setGlobal to IEclipseContext
Summary: add setGlobal to IEclipseContext
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Runtime (show other bugs)
Version: 4.4   Edit
Hardware: PC Mac OS X
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: platform-runtime-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 10:26 EDT by Steven Spungin CLA
Modified: 2014-07-14 16:18 EDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Spungin CLA 2014-04-08 10:26:24 EDT
In response to the feedback and direction of Bug 421790 - [EclipseContexts] add boolean isModifiable() to IEclipseContext, I now suggest adding additional interface methods instead.

setGlobal(name or class) and isGlobal(name or class)

1. The implementation has a negligent performance and memory hit.
2. Contexts not using method will have no impact.
3. The clear nature of the method's API makes it useful, predictable, and easy to manage
4. It has functionality not available in the current API
4. The name setGlobal describes exactly what programming languages intend as a global variable.

In a nutshell, once setGlobal is called, the context branch is set as global for a given key, and current or future children that set, get, or modify the key will resolve to the setGlobal's caller context.  All current objects set in the branch will migrate to the caller, and an exception will be thrown if the branch already contains more than 1 object set for the given key.

If a child calls setGlobal on an already global branch, it is a NOP.
If an ancestor calls setGlobal, it becomes the new global master of the data.

The patch has a unit test that clearly shows the use cases.  Here is the most significant:

IEclipseContext grandparent = new EclipseContext(null);
IEclipseContext parent = grandparent.createChild();
IEclipseContext son = parent.createChild();
IEclipseContext daughter = parent.createChild();

String obj = "foo";
son.set(theKey, obj);
daughter.set(theKey, obj);

// The son and daughter are merged into the grandparent.  
// Note: that usually setGlobal will be call first; and a merge would never happen as all future sets would resolve to grandparent.
grandparent.setGlobal(theKey);

assertTrue(grandparent.get(theKey) == obj);
assertTrue(parent.get(theKey) == obj);
assertTrue(son.get(theKey) == obj);
assertTrue(daughter.get(theKey) == obj);

parent.set(theKey, theOtherValue);
assertTrue(grandparent.get(theKey) == theOtherValue);
assertTrue(parent.get(theKey) == theOtherValue);
assertTrue(son.get(theKey) == theOtherValue);
assertTrue(daughter.get(theKey) == theOtherValue);

son.set(theKey, theValue);
assertTrue(grandparent.get(theKey) == theValue);
assertTrue(parent.get(theKey) == theValue);
assertTrue(son.get(theKey) == theValue);
assertTrue(daughter.get(theKey) == theValue);
Comment 1 Steven Spungin CLA 2014-04-08 10:44:45 EDT
https://git.eclipse.org/r/#/c/24651/
Comment 2 Paul Webster CLA 2014-04-08 11:31:36 EDT
This seems to be only slightly different than modify/declareModifiable.  We can't have new API that's only slightly semantically different than existing API.

PW
Comment 3 Steven Spungin CLA 2014-04-08 12:42:26 EDT

 (In reply to Paul Webster from comment #2)
> We can't have new API that's only slightly semantically different than existing
> API.
> 
> PW

Why?

I really don't understand 'slightly different' when there is a complete inability to adjust the current implementation or API, which is both not consistant and unclear.

And semantically it is not the same; isModifiable allows middleman contexts to interfere, and allows multiple values and modifiable declarations to exist throughout the branch.  It's really a mess to me, but if that's one's cup of tea, they can keep on using isModifiable.

This is a bit more than 'slightly different' to me;  In this circumstance I think there is only 'different'.  Global means global: One and only 1 value to be shared by the branch.  I don't think isModifiable has ANYTHING to do with that.

This API is no doubt simpler to use for every instance in which I have previously used isModifiable, and I feel uneasy using the former in the future given how bizarre my experience has been with it.

I am interested in other developers view of this, specifically related 
1. Their uses of isModifiable and how setGlobal would either make their code clearer and more dependable, or break it.

2. Given these 2 approaches, which one would they prefer.

Please consider this before giving the idea the axe.  Thanks!
Comment 4 Paul Webster CLA 2014-04-08 12:46:10 EDT
(In reply to Steven Spungin from comment #3)
> 
>  (In reply to Paul Webster from comment #2)
> > We can't have new API that's only slightly semantically different than existing
> > API.
> > 
> > PW
> 
> Why?
> 
> I really don't understand 'slightly different' when there is a complete
> inability to adjust the current implementation or API, which is both not
> consistant and unclear.

Because we have to fix and live with the API we have, not add extra API on the side.

> 
> And semantically it is not the same; isModifiable allows middleman contexts
> to interfere, 

This is an implementation problem, coupled with it being vague API.  The suggestion on the bug (walk up the parent tree and set the value where it's set until we hit the context where it's marked as modifiable) fixes that.

PW
Comment 5 Steven Spungin CLA 2014-04-08 12:59:47 EDT
> This is an implementation problem, coupled with it being vague API.  The
> suggestion on the bug (walk up the parent tree and set the value where it's
> set until we hit the context where it's marked as modifiable) fixes that.
> 
> PW

It doesn't solve these issues:

Calling modifiable twice gives exception second time.

Grandparent was modifiable until middleman declared modifiable and now nephews and nieces don't get the new value.

Middleman calling set vs. modify; These have 2 very different implications in the current implementation.  They are not unified as in setGlobal.  In fact, with setGlobal, you can decide to make a context key global, while leaving all the children set(...) calls without changing them to modify.  A huge improvement.
Comment 6 Paul Webster CLA 2014-04-08 13:02:26 EDT
(In reply to Steven Spungin from comment #5)
> > This is an implementation problem, coupled with it being vague API.  The
> > suggestion on the bug (walk up the parent tree and set the value where it's
> > set until we hit the context where it's marked as modifiable) fixes that.
> > 
> > PW
> 
> It doesn't solve these issues:
> 
> Calling modifiable twice gives exception second time.

that was something also to be fixed, I thought.

Yes, calling modify on a parent down the chain will isolate those children.  Don't do that (Eclipse4 goes out of its way to *not* protect against these kinds of usecases.  Consumers and users have to be careful)

PW