-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encapsulate attributes for ExtensionContext #82
Comments
Implemented encapsulating methods in cf91ce4 |
Alternative suggestionExtend interface ExtensionContext {
Object get(Object key);
void store(Object key, Object value, Visibility visibility);
default void store(Object key, Object value) {
store(key, value, Visibility.DEFAULT);
}
} and enum Visibility {
DEFAULT, //visible only in this extension but inherited from ancestor extension contexts
EXTENSION, //visible only in this extension but in the full tree of extension contexts
LOCAL, //only visible in this extension and this extension context
GLOBAL //visible in all extensions and in the full tree of extension contexts
} This would then replace |
Team decision for the time being: interface ExtensionContext...
default Object get(Object key) {
return get(key, "");
}
Object get(Object key, String namespace);
default void put(Object key, Object value) {
put(key, value, "");
}
void put(Object key, Object value, String namespace);
Object getOrComputeIfAbsent(Object key, Function<Object, Object> defaultProvider, String namespace) {
Object value = get(key);
if (value == null) {
value = defaultProvider.apply(key);
put(key, value, visibility);
}
return value;
}
default Object getOrComputeIfAbsent(Object key, Function<Object, Object> defaultProvider) {
return getOrComputeIfAbsent(key, defaultProvider, "");
} Choose a suitable value for default namespace, e.g. have an object internally but only allow Strings from outside. |
I'm working on it. |
I suggest other changes in addition to the ones above:
Feedback? |
What I'm planning to do is implement it in master but leave the old attributes in place. Thus you can all see how it looks but no existing functionality must be touched. Any objections? |
For any new features, for which we do not yet know in which direction we should go, I would strongly recommend not to work in master. That's exactly what I am doing with the discovery mechanism. |
Ok. I'll only do in master on what we agreed yesterday.
|
Agreed changes are done in master. Suggestions from above must still be discussed. |
This commit refactors ExtensionContext and related classes so that the recently introduced remove() methods return the previous value stored under the associated key/namespace, analogous to the current behavior of removeAttribute(). Issue: #82
Implemented suggestion in https://github.com/junit-team/junit5/tree/issue82-store for review |
Merged suggestion after team decision in #128. Missing:
|
Resolved in e103a16 |
Encapsulate attributes for Test|ContainerExtensionContext:
The text was updated successfully, but these errors were encountered: