opendev/gerrit
Code Issues Proposed changes

Store Provider<T> in DynamicSet, DynamicMap

Browse Source 
Instead of storing the object instance T, store the Provider<T>.

This enables Guice to provide singletons wrapped in a Provider, or to
dynamically create the object on each use. Dynamic creation enables
plugins to make a new listener instance for each event that is fired,
permitting the listener to then use instance member fields during the
event. How this works is up to the plugin, but binding with @Singleton
will still be the recommended method for registering listeners.

Change-Id: I83b22f3ac2214f45c3c298937146843a806bea2f
This commit is contained in:
Shawn O. Pearce
2012-05-16 09:22:44 -07:00
parent 9637774986
commit dc5ef8f266
7 changed files with 76 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicMap.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.extensions.registration;
import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.Provider;
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Types;
@@ -34,9 +35,9 @@ import java.util.concurrent.ConcurrentMap;
* <p>
* Maps index their members by plugin name and export name.
* <p>
* DynamicMaps are always mapped as singletons in Guice, and only may contain
* singletons, as providers are resolved to an instance before the member is
* added to the map.
* DynamicMaps are always mapped as singletons in Guice. Maps store Providers
* internally, and resolve the provider to an instance on demand. This enables
* registrations to decide between singleton and non-singleton members.
*/
public abstract class DynamicMap<T> {
/**
@@ -82,10 +83,13 @@ public abstract class DynamicMap<T> {
.in(Scopes.SINGLETON);
}
final ConcurrentMap<NamePair, T> items;
final ConcurrentMap<NamePair, Provider<T>> items;
DynamicMap() {
items = new ConcurrentHashMap<NamePair, T>(16, 0.75f, 1);
items = new ConcurrentHashMap<NamePair, Provider<T>>(
16 /* initial size */,
0.75f /* load factor */,
1 /* concurrency level of 1, load/unload is single threaded */);
}
/**
@@ -95,9 +99,12 @@ public abstract class DynamicMap<T> {
* @param exportName name the plugin exports the item as.
* @return the implementation. Null if the plugin is not running, or if the
* plugin does not export this name.
* @throws ProvisionException if the registered provider is unable to obtain
* an instance of the requested implementation.
*/
public T get(String pluginName, String exportName) {
return items.get(new NamePair(pluginName, exportName));
Provider<T> p = items.get(new NamePair(pluginName, exportName));
return p != null ? p.get() : null;
}
/**
@@ -119,9 +126,9 @@ public abstract class DynamicMap<T> {
* @param pluginName name of the plugin.
* @return items exported by a plugin, keyed by the export name.
*/
public SortedMap<String, T> byPlugin(String pluginName) {
SortedMap<String, T> r = new TreeMap<String, T>();
for (Map.Entry<NamePair, T> e : items.entrySet()) {
public SortedMap<String, Provider<T>> byPlugin(String pluginName) {
SortedMap<String, Provider<T>> r = new TreeMap<String, Provider<T>>();
for (Map.Entry<NamePair, Provider<T>> e : items.entrySet()) {
if (e.getKey().pluginName.equals(pluginName)) {
r.put(e.getKey().exportName, e.getValue());
}
@@ -147,7 +154,8 @@ public abstract class DynamicMap<T> {
public boolean equals(Object other) {
if (other instanceof NamePair) {
NamePair np = (NamePair) other;
return pluginName.equals(np.pluginName) && exportName.equals(np.exportName);
return pluginName.equals(np.pluginName)
&& exportName.equals(np.exportName);
}
return false;
}

2
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicMapProvider.java
View File

@@ -38,7 +38,7 @@ class DynamicMapProvider<T> implements Provider<DynamicMap<T>> {
List<Binding<T>> bindings = injector.findBindingsByType(type);
if (bindings != null) {
for (Binding<T> b : bindings) {
m.put("gerrit", b.getKey(), b.getProvider().get());
m.put("gerrit", b.getKey(), b.getProvider());
}
}
return m;

52
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSet.java
View File

@@ -16,11 +16,13 @@ package com.google.gerrit.extensions.registration;
import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.Provider;
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
import com.google.inject.binder.LinkedBindingBuilder;
import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.name.Named;
import com.google.inject.util.Providers;
import com.google.inject.util.Types;
import java.util.Collection;
@@ -32,9 +34,9 @@ import java.util.concurrent.atomic.AtomicReference;
/**
* A set of members that can be modified as plugins reload.
* <p>
* DynamicSets are always mapped as singletons in Guice, and only may contain
* singletons, as providers are resolved to an instance before the member is
* added to the set.
* DynamicSets are always mapped as singletons in Guice. Sets store Providers
* internally, and resolve the provider to an instance on demand. This enables
* registrations to decide between singleton and non-singleton members.
*/
public class DynamicSet<T> implements Iterable<T> {
/**
@@ -125,22 +127,29 @@ public class DynamicSet<T> implements Iterable<T> {
return binder.bind(type).annotatedWith(name);
}
private final CopyOnWriteArrayList<AtomicReference<T>> items;
private final CopyOnWriteArrayList<AtomicReference<Provider<T>>> items;
DynamicSet(Collection<AtomicReference<T>> base) {
items = new CopyOnWriteArrayList<AtomicReference<T>>(base);
DynamicSet(Collection<AtomicReference<Provider<T>>> base) {
items = new CopyOnWriteArrayList<AtomicReference<Provider<T>>>(base);
}
@Override
public Iterator<T> iterator() {
final Iterator<AtomicReference<T>> itr = items.iterator();
final Iterator<AtomicReference<Provider<T>>> itr = items.iterator();
return new Iterator<T>() {
private T next;
@Override
public boolean hasNext() {
while (next == null && itr.hasNext()) {
next = itr.next().get();
Provider<T> p = itr.next().get();
if (p != null) {
try {
next = p.get();
} catch (RuntimeException e) {
// TODO Log failed member of DynamicSet.
}
}
}
return next != null;
}
@@ -169,7 +178,18 @@ public class DynamicSet<T> implements Iterable<T> {
* @return handle to remove the item at a later point in time.
*/
public RegistrationHandle add(final T item) {
final AtomicReference<T> ref = new AtomicReference<T>(item);
return add(Providers.of(item));
}
/**
* Add one new element to the set.
*
* @param item the item to add to the collection. Must not be null.
* @return handle to remove the item at a later point in time.
*/
public RegistrationHandle add(final Provider<T> item) {
final AtomicReference<Provider<T>> ref =
new AtomicReference<Provider<T>>(item);
items.add(ref);
return new RegistrationHandle() {
@Override
@@ -191,18 +211,20 @@ public class DynamicSet<T> implements Iterable<T> {
* @return a handle that can remove this item later, or hot-swap the item
* without it ever leaving the collection.
*/
public ReloadableRegistrationHandle<T> add(Key<T> key, T item) {
AtomicReference<T> ref = new AtomicReference<T>(item);
public ReloadableRegistrationHandle<T> add(Key<T> key, Provider<T> item) {
AtomicReference<Provider<T>> ref = new AtomicReference<Provider<T>>(item);
items.add(ref);
return new ReloadableHandle(ref, key, item);
}
private class ReloadableHandle implements ReloadableRegistrationHandle<T> {
private final AtomicReference<T> ref;
private final AtomicReference<Provider<T>> ref;
private final Key<T> key;
private final T item;
private final Provider<T> item;
ReloadableHandle(AtomicReference<T> ref, Key<T> key, T item) {
ReloadableHandle(AtomicReference<Provider<T>> ref,
Key<T> key,
Provider<T> item) {
this.ref = ref;
this.key = key;
this.item = item;
@@ -221,7 +243,7 @@ public class DynamicSet<T> implements Iterable<T> {
}
@Override
public ReloadableHandle replace(Key<T> newKey, T newItem) {
public ReloadableHandle replace(Key<T> newKey, Provider<T> newItem) {
if (ref.compareAndSet(item, newItem)) {
return new ReloadableHandle(ref, newKey, newItem);
}

10
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSetProvider.java
View File

@@ -39,7 +39,7 @@ class DynamicSetProvider<T> implements Provider<DynamicSet<T>> {
return new DynamicSet<T>(find(injector, type));
}
private static <T> List<AtomicReference<T>> find(
private static <T> List<AtomicReference<Provider<T>>> find(
Injector src,
TypeLiteral<T> type) {
List<Binding<T>> bindings = src.findBindingsByType(type);
@@ -47,10 +47,14 @@ class DynamicSetProvider<T> implements Provider<DynamicSet<T>> {
if (cnt == 0) {
return Collections.emptyList();
}
List<AtomicReference<T>> r = new ArrayList<AtomicReference<T>>(cnt);
List<AtomicReference<Provider<T>>> r = newList(cnt);
for (Binding<T> b : bindings) {
r.add(new AtomicReference<T>(b.getProvider().get()));
r.add(new AtomicReference<Provider<T>>(b.getProvider()));
}
return r;
}
private static <T> List<AtomicReference<Provider<T>>> newList(int cnt) {
return new ArrayList<AtomicReference<Provider<T>>>(cnt);
}
}

11
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicMapImpl.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.extensions.registration;
import com.google.gerrit.extensions.annotations.Export;
import com.google.inject.Key;
import com.google.inject.Provider;
/** <b>DO NOT USE</b> */
public class PrivateInternals_DynamicMapImpl<T> extends DynamicMap<T> {
@@ -32,7 +33,7 @@ public class PrivateInternals_DynamicMapImpl<T> extends DynamicMap<T> {
*/
public RegistrationHandle put(
String pluginName, String exportName,
final T item) {
final Provider<T> item) {
final NamePair key = new NamePair(pluginName, exportName);
items.put(key, item);
return new RegistrationHandle() {
@@ -57,7 +58,7 @@ public class PrivateInternals_DynamicMapImpl<T> extends DynamicMap<T> {
*/
public ReloadableRegistrationHandle<T> put(
String pluginName, Key<T> key,
T item) {
Provider<T> item) {
String exportName = ((Export) key.getAnnotation()).value();
NamePair np = new NamePair(pluginName, exportName);
items.put(np, item);
@@ -67,9 +68,9 @@ public class PrivateInternals_DynamicMapImpl<T> extends DynamicMap<T> {
private class ReloadableHandle implements ReloadableRegistrationHandle<T> {
private final NamePair np;
private final Key<T> key;
private final T item;
private final Provider<T> item;
ReloadableHandle(NamePair np, Key<T> key, T item) {
ReloadableHandle(NamePair np, Key<T> key, Provider<T> item) {
this.np = np;
this.key = key;
this.item = item;
@@ -86,7 +87,7 @@ public class PrivateInternals_DynamicMapImpl<T> extends DynamicMap<T> {
}
@Override
public ReloadableHandle replace(Key<T> newKey, T newItem) {
public ReloadableHandle replace(Key<T> newKey, Provider<T> newItem) {
if (items.replace(np, item, newItem)) {
return new ReloadableHandle(np, newKey, newItem);
}

3
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/ReloadableRegistrationHandle.java
View File

@@ -15,9 +15,10 @@
package com.google.gerrit.extensions.registration;
import com.google.inject.Key;
import com.google.inject.Provider;
public interface ReloadableRegistrationHandle<T> extends RegistrationHandle {
public Key<T> getKey();
public RegistrationHandle replace(Key<T> key, T item);
public RegistrationHandle replace(Key<T> key, Provider<T> item);
}