Hid BinderImpl. Redesigned SoureProvider API.

git-svn-id: https://google-guice.googlecode.com/svn/trunk@195 d779f126-a31b-0410-b53b-1d3aecad763e

11 files changed, 193 insertions, 192 deletions

diff --git a/build.properties b/build.properties
index f1cfe8b1..e6a1b113 100644
--- a/build.properties
+++ b/build.properties
@@ -5,4 +5,4 @@ test.dir=test
 build.dir=build
 javadoc.packagenames=com.google.inject,com.google.inject.spi,\
   com.google.inject.matcher,com.google.inject.servlet,com.google.inject.name,\
-  com.google.inject.tools.jmx
+  com.google.inject.tools.jmx,com.google.inject.binder
diff --git a/src/com/google/inject/AbstractModule.java b/src/com/google/inject/AbstractModule.java
index 24b6cce7..289fb3d6 100644
--- a/src/com/google/inject/AbstractModule.java
+++ b/src/com/google/inject/AbstractModule.java
@@ -21,6 +21,7 @@ import com.google.inject.binder.ConstantBindingBuilder;
 import com.google.inject.binder.LinkedBindingBuilder;
 import com.google.inject.matcher.Matcher;
 import com.google.inject.util.Objects;
+import com.google.inject.spi.SourceProviders;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import org.aopalliance.intercept.MethodInterceptor;
@@ -29,7 +30,7 @@ import org.aopalliance.intercept.MethodInterceptor;
  * A support class for {@link Module Modules} which reduces repetition and
  * results in a more readable configuration. Simply extends this class,
  * implement {@link #configure()}, and call the inherited methods which mirror
- * those found in {@link BinderImpl}. For example:
+ * those found in {@link Binder}. For example:
  *
  * <pre>
  * import static com.google.inject.Names.named;
@@ -48,9 +49,13 @@ import org.aopalliance.intercept.MethodInterceptor;
  */
 public abstract class AbstractModule implements Module {
 
+  static {
+    SourceProviders.skip(AbstractModule.class);
+  }
+
   Binder builder;
 
-  public final synchronized void configure(BinderImpl builder) {
+  public final synchronized void configure(Binder builder) {
     try {
       if (this.builder != null) {
         throw new IllegalStateException("Re-entry is not allowed.");
@@ -66,7 +71,7 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * Configures a {@link BinderImpl} via the exposed methods.
+   * Configures a {@link Binder} via the exposed methods.
    */
   protected abstract void configure();
 
@@ -78,7 +83,7 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * @see BinderImpl#bindScope(Class, Scope)
+   * @see Binder#bindScope(Class, Scope)
    */
   protected void scope(Class<? extends Annotation> scopeAnnotation,
       Scope scope) {
@@ -86,14 +91,14 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * @see BinderImpl#bind(Key)
+   * @see Binder#bind(Key)
    */
   protected <T> BindingBuilder<T> bind(Key<T> key) {
     return builder.bind(key);
   }
 
   /**
-   * @see BinderImpl#bind(TypeLiteral)
+   * @see Binder#bind(TypeLiteral)
    */
   protected <T> BindingBuilder<T> bind(
       TypeLiteral<T> typeLiteral) {
@@ -101,21 +106,21 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * @see BinderImpl#bind(Class)
+   * @see Binder#bind(Class)
    */
   protected <T> BindingBuilder<T> bind(Class<T> clazz) {
     return builder.bind(clazz);
   }
 
   /**
-   * @see BinderImpl#link(Key)
+   * @see Binder#link(Key)
    */
   protected <T> LinkedBindingBuilder<T> link(Key<T> key) {
     return builder.link(key);
   }
 
   /**
-   * @see BinderImpl#bindConstant(Class)
+   * @see Binder#bindConstant(Class)
    */
   protected ConstantBindingBuilder bindConstant(
       Class<? extends Annotation> annotationType) {
@@ -123,7 +128,7 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * @see BinderImpl#bindConstant(java.lang.annotation.Annotation)
+   * @see Binder#bindConstant(java.lang.annotation.Annotation)
    */
   protected ConstantBindingBuilder bindConstant(
       Annotation annotation) {
@@ -131,21 +136,21 @@ public abstract class AbstractModule implements Module {
   }
 
   /**
-   * @see BinderImpl#install(Module)
+   * @see Binder#install(Module)
    */
   protected void install(Module module) {
     builder.install(module);
   }
 
   /**
-   * @see BinderImpl#requestStaticInjection(Class[])
+   * @see Binder#requestStaticInjection(Class[])
    */
   protected void requestStaticInjection(Class<?>... types) {
     builder.requestStaticInjection(types);
   }
 
   /**
-   * @see BinderImpl#bindInterceptor(com.google.inject.matcher.Matcher,
+   * @see Binder#bindInterceptor(com.google.inject.matcher.Matcher,
    *  com.google.inject.matcher.Matcher,
    *  org.aopalliance.intercept.MethodInterceptor[])
    */
diff --git a/src/com/google/inject/BinderImpl.java b/src/com/google/inject/BinderImpl.java
index 1f3ea6c7..c6ec0ec3 100644
--- a/src/com/google/inject/BinderImpl.java
+++ b/src/com/google/inject/BinderImpl.java
@@ -21,7 +21,7 @@ import com.google.inject.Key.AnnotationStrategy;
 import static com.google.inject.Scopes.CONTAINER;
 import com.google.inject.matcher.Matcher;
 import com.google.inject.spi.Message;
-import com.google.inject.spi.SourceConsumer;
+import com.google.inject.spi.SourceProviders;
 import static com.google.inject.util.Objects.nonNull;
 import com.google.inject.util.Stopwatch;
 import java.lang.annotation.Annotation;
@@ -50,7 +50,11 @@ import org.aopalliance.intercept.MethodInterceptor;
  *
  * @author crazybob@google.com (Bob Lee)
  */
-public final class BinderImpl extends SourceConsumer implements Binder {
+class BinderImpl implements Binder {
+
+  static {
+    SourceProviders.skip(BinderImpl.class);
+  }
 
   private static final Logger logger
       = Logger.getLogger(BinderImpl.class.getName());
@@ -141,13 +145,11 @@ public final class BinderImpl extends SourceConsumer implements Binder {
 
   public void bindInterceptor(Matcher<? super Class<?>> classMatcher,
       Matcher<? super Method> methodMatcher, MethodInterceptor... interceptors) {
-    ensureNotCreated();
     proxyFactoryBuilder.intercept(classMatcher, methodMatcher, interceptors);
   }
 
   public void bindScope(Class<? extends Annotation> annotationType,
       Scope scope) {
-    ensureNotCreated();
     Scope existing = scopes.get(nonNull(annotationType, "annotation type"));
     if (existing != null) {
       addError(source(), ErrorMessages.DUPLICATE_SCOPES, existing,
@@ -159,7 +161,6 @@ public final class BinderImpl extends SourceConsumer implements Binder {
   }
 
   public <T> BindingBuilderImpl<T> bind(Key<T> key) {
-    ensureNotCreated();
     BindingBuilderImpl<T> builder = new BindingBuilderImpl<T>(this, key, source());
     bindingBuilders.add(builder);
     return builder;
@@ -174,7 +175,6 @@ public final class BinderImpl extends SourceConsumer implements Binder {
   }
 
   public <T> LinkedBindingBuilderImpl<T> link(Key<T> key) {
-    ensureNotCreated();
     LinkedBindingBuilderImpl<T> builder =
         new LinkedBindingBuilderImpl<T>(this, key).from(source());
     linkedBindingBuilders.add(builder);
@@ -192,13 +192,11 @@ public final class BinderImpl extends SourceConsumer implements Binder {
   }
 
   public ConstantBindingBuilderImpl bindConstant(Annotation annotation) {
-    ensureNotCreated();
     return bind(source(), Key.strategyFor(annotation));
   }
 
   public ConstantBindingBuilderImpl bindConstant(
       Class<? extends Annotation> annotationType) {
-    ensureNotCreated();
     return bind(source(), Key.strategyFor(annotationType));
   }
 
@@ -243,12 +241,10 @@ public final class BinderImpl extends SourceConsumer implements Binder {
    *     expectation is that the application will log this exception and exit.
    * @throws IllegalStateException if called more than once
    */
-  public synchronized Container createContainer()
-      throws CreationException {
+  Container createContainer() throws CreationException {
     stopwatch.resetAndLog(logger, "Configuration");
 
     // Create the container.
-    ensureNotCreated();
     Map<Key<?>, Binding<?>> bindings = new HashMap<Key<?>, Binding<?>>();
     container = new ContainerImpl(
         proxyFactoryBuilder.create(), bindings, scopes);
@@ -409,16 +405,10 @@ public final class BinderImpl extends SourceConsumer implements Binder {
   }
 
   /**
-   * Currently we only support creating one Container instance per builder. If
-   * we want to support creating more than one container per builder, we should
-   * move to a "factory factory" model where we create a factory instance per
-   * Container. Right now, one factory instance would be shared across all the
-   * containers, which means container-scoped objects would be shared, etc.
+   * Gets the current source.
    */
-  private void ensureNotCreated() {
-    if (container != null) {
-      throw new IllegalStateException("Container already created.");
-    }
+  Object source() {
+    return SourceProviders.defaultSource();    
   }
 
   ErrorHandler configurationErrorHandler = new AbstractErrorHandler() {
diff --git a/src/com/google/inject/Container.java b/src/com/google/inject/Container.java
index 8f077b2f..96e9ca41 100644
--- a/src/com/google/inject/Container.java
+++ b/src/com/google/inject/Container.java
@@ -30,7 +30,7 @@ import java.util.Map;
  * {@code int}, the container will look for a binding to {@code Integer}.
  *
  * @author crazybob@google.com (Bob Lee)
- * @see BinderImpl
+ * @see Binder
  */
 public interface Container {
 
diff --git a/src/com/google/inject/Module.java b/src/com/google/inject/Module.java
index e1919108..12c0382f 100644
--- a/src/com/google/inject/Module.java
+++ b/src/com/google/inject/Module.java
@@ -18,7 +18,7 @@ package com.google.inject;
 
 /**
  * A module contributes a set of configurations, typically interface bindings,
- * to a {@link BinderImpl} which will later create a {@link Container}.
+ * to a {@link Binder} which will later be used to create a {@link Container}.
  * Implementing this interface is the standard means for encapsulating and
  * reusing configuration logic. Your Module classes can use a more streamlined
  * syntax by extending {@link AbstractModule} rather than implementing this
@@ -33,5 +33,5 @@ public interface Module {
    * that the resulting {@link Container} will include this module properly set
    * up.
    */
-  void configure(BinderImpl builder);
+  void configure(Binder binder);
 }
diff --git a/src/com/google/inject/name/Names.java b/src/com/google/inject/name/Names.java
index 28ed13a2..ea2f8431 100644
--- a/src/com/google/inject/name/Names.java
+++ b/src/com/google/inject/name/Names.java
@@ -17,9 +17,8 @@
 package com.google.inject.name;
 
 import com.google.inject.Binder;
-import com.google.inject.BinderImpl;
 import com.google.inject.Key;
-import com.google.inject.spi.DefaultSourceProvider;
+import com.google.inject.spi.SourceProviders;
 import com.google.inject.spi.SourceProvider;
 import java.util.Map;
 import java.util.Properties;
@@ -33,6 +32,10 @@ public class Names {
 
   private Names() {}
 
+  static {
+    SourceProviders.skip(Names.class);
+  }
+
   /**
    * Creates a {@link Named} annotation with {@code name} as the value.
    */
@@ -43,35 +46,49 @@ public class Names {
   /**
    * Creates a constant binding to {@code @Named(key)} for each property.
    */
-  public static void bindProperties(BinderImpl binder,
-      Map<String, String> properties) {
-    skipNames(binder);
-    for (Map.Entry<String, String> entry : properties.entrySet()) {
-      String key = entry.getKey();
-      String value = entry.getValue();
-      ((Binder) binder).bind(
-          Key.get(String.class, new NamedImpl(key))).to(value);
-    }
+  public static void bindProperties(final Binder binder,
+      final Map<String, String> properties) {
+    SourceProviders.withDefault(
+        new SimpleSourceProvider(SourceProviders.defaultSource()),
+        new Runnable() {
+          public void run() {
+            for (Map.Entry<String, String> entry : properties.entrySet()) {
+              String key = entry.getKey();
+              String value = entry.getValue();
+              binder.bind(Key.get(String.class, new NamedImpl(key))).to(value);
+            }
+          }
+        });
   }
 
   /**
    * Creates a constant binding to {@code @Named(key)} for each property.
    */
-  public static void bindProperties(BinderImpl binder,
-      Properties properties) {
-    skipNames(binder);
-    for (Map.Entry<Object, Object> entry : properties.entrySet()) {
-      String key = (String) entry.getKey();
-      String value = (String) entry.getValue();
-      ((Binder) binder).bind(
-          Key.get(String.class, new NamedImpl(key))).to(value);
-    }
+  public static void bindProperties(final Binder binder,
+      final Properties properties) {
+    SourceProviders.withDefault(
+        new SimpleSourceProvider(SourceProviders.defaultSource()),
+        new Runnable() {
+          public void run() {
+            for (Map.Entry<Object, Object> entry : properties.entrySet()) {
+              String key = (String) entry.getKey();
+              String value = (String) entry.getValue();
+              binder.bind(Key.get(String.class, new NamedImpl(key))).to(value);
+            }
+          }
+        });
   }
 
-  private static void skipNames(BinderImpl builder) {
-    SourceProvider sourceProvider = builder.getSourceProvider();
-    if (sourceProvider instanceof DefaultSourceProvider) {
-      ((DefaultSourceProvider) sourceProvider).skip(Names.class);
+  static class SimpleSourceProvider implements SourceProvider {
+
+    final Object source;
+
+    SimpleSourceProvider(Object source) {
+      this.source = source;
+    }
+
+    public Object source() {
+      return source;
     }
   }
 }
diff --git a/src/com/google/inject/package-info.java b/src/com/google/inject/package-info.java
index 4bca06f5..ebc78e57 100644
--- a/src/com/google/inject/package-info.java
+++ b/src/com/google/inject/package-info.java
@@ -34,7 +34,7 @@
  *     instructions for how Guice should handle injection -- for a particular
  *     set of interfaces.
  *
- * <dt>{@link BinderImpl}
+ * <dt>{@link com.google.inject.Binder}
  * <dd>The object that Guice passes into your {@link com.google.inject.Module}
  *     to collect these bindings.
  *
diff --git a/src/com/google/inject/spi/DefaultSourceProvider.java b/src/com/google/inject/spi/DefaultSourceProvider.java
deleted file mode 100644
index f6f8d7e6..00000000
--- a/src/com/google/inject/spi/DefaultSourceProvider.java
+++ /dev/null
@@ -1,62 +0,0 @@
-/**
- * Copyright (C) 2006 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.google.inject.spi;
-
-import com.google.inject.AbstractModule;
-import com.google.inject.BinderImpl;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
-
-/**
- * A source provider which returns {@code Binder}'s caller's {@code
- * StackTraceElement}.
- * 
- * @author crazybob@google.com (Bob Lee)
- */
-public class DefaultSourceProvider implements SourceProvider {
-
-  final Set<String> skippedClassNames = new HashSet<String>(Arrays.asList(
-      BinderImpl.class.getName(),
-      AbstractModule.class.getName(),
-      DefaultSourceProvider.class.getName(),
-      SourceConsumer.class.getName()
-  ));
-
-  /**
-   * Instructs the provider to skip the given class in the stack trace when
-   * determining the source. Use this to keep the container builder from
-   * logging utility methods as the sources of bindings (i.e. it will skip to
-   * the utility methods' callers instead).
-   *
-   * <p>Skipping only takes place after this method is called.
-   */
-  public void skip(Class<?> clazz) {
-    skippedClassNames.add(clazz.getName());
-  }
-
-  public Object source() {
-    // Search up the stack until we find a class outside of this one.
-    for (final StackTraceElement element : new Throwable().getStackTrace()) {
-      String className = element.getClassName();
-      if (!skippedClassNames.contains(className)) {
-        return element;
-      }
-    }
-    throw new AssertionError();
-  }
-}
diff --git a/src/com/google/inject/spi/SourceConsumer.java b/src/com/google/inject/spi/SourceConsumer.java
deleted file mode 100644
index 90d02838..00000000
--- a/src/com/google/inject/spi/SourceConsumer.java
+++ /dev/null
@@ -1,62 +0,0 @@
-/**
- * Copyright (C) 2006 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.google.inject.spi;
-
-/**
- * Support for classes which use source objects.
- *
- * @author crazybob@google.com (Bob Lee)
- */
-public class SourceConsumer {
-
-  SourceProvider sourceProvider = new DefaultSourceProvider();
-
-  /**
-   * Returns the current source.
-   */
-  protected Object source() {
-    return sourceProvider.source();
-  }
-
-  /**
-   * Gets the current source provider.
-   */
-  public SourceProvider getSourceProvider() {
-    return sourceProvider;
-  }
-
-  /**
-   * Sets the current source provider.
-   */
-  public void setSourceProvider(SourceProvider sourceProvider) {
-    this.sourceProvider = sourceProvider;
-  }
-
-  /**
-   * Sets the source provider, runs the given command, and restores the
-   * previous source provider.
-   */
-  public void withSourceProvider(SourceProvider sourceProvider, Runnable r) {
-    SourceProvider previous = this.sourceProvider;
-    try {
-      this.sourceProvider = sourceProvider;
-      r.run();
-    } finally {
-      this.sourceProvider = previous;
-    }
-  }
-}
diff --git a/src/com/google/inject/spi/SourceProviders.java b/src/com/google/inject/spi/SourceProviders.java
new file mode 100644
index 00000000..e52e7d8b
--- /dev/null
+++ b/src/com/google/inject/spi/SourceProviders.java
@@ -0,0 +1,110 @@
+/**
+ * Copyright (C) 2006 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.inject.spi;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.Collections;
+
+/**
+ * Provides access to the default {@link SourceProvider} implementation and
+ * common controls for certain implementations.
+ * 
+ * @author crazybob@google.com (Bob Lee)
+ */
+public class SourceProviders {
+
+  private SourceProviders() {}
+
+  static final SourceProvider DEFAULT_INSTANCE = new StacktraceSourceProvider();
+
+  static Set<String> skippedClassNames = Collections.emptySet();
+
+  static {
+    skip(SourceProviders.class);
+    skip(StacktraceSourceProvider.class);
+  }
+
+  /**
+   * Instructs stacktrace-based providers to skip the given class in the stack
+   * trace when
+   * determining the source. Use this to keep the container builder from
+   * logging utility methods as the sources of bindings (i.e. it will skip to
+   * the utility methods' callers instead).
+   *
+   * <p>Skipping only takes place after this method is called.
+   */
+  public synchronized static void skip(Class<?> clazz) {
+    Set<String> copy = new HashSet<String>();
+    copy.addAll(skippedClassNames);
+    copy.add(clazz.getName());
+    skippedClassNames = Collections.unmodifiableSet(copy);
+  }
+
+  /**
+   * Gets the set of class names which should be skipped by stacktrace-based
+   * providers.
+   */
+  public synchronized static Set<String> getSkippedClassNames() {
+    return skippedClassNames;
+  }
+
+  static ThreadLocal<SourceProvider[]> localSourceProvider =
+      new ThreadLocal<SourceProvider[]>() {
+    protected SourceProvider[] initialValue() {
+      return new SourceProvider[] { DEFAULT_INSTANCE };
+    }
+  };
+
+  /**
+   * Returns the current source obtained from the default provider.
+   */
+  public static Object defaultSource() {
+    return localSourceProvider.get()[0].source();
+  }
+
+  /**
+   * Sets the default source provider, runs the given command, and then
+   * restores the previous default source provider.
+   */
+  public static void withDefault(
+      SourceProvider sourceProvider, Runnable r) {
+    // We use a holder so we perform only 1 thread local access instead of 3.
+    SourceProvider[] holder = localSourceProvider.get();
+    SourceProvider previous = holder[0];
+    try {
+      holder[0] = sourceProvider;
+      r.run();
+    } finally {
+      holder[0] = previous;
+    }
+  }
+
+  static class StacktraceSourceProvider implements SourceProvider {
+    public Object source() {
+      // Search up the stack until we find a class outside of this one.
+      Set<String> skippedClassNames = getSkippedClassNames();
+      for (final StackTraceElement element : new Throwable().getStackTrace()) {
+        String className = element.getClassName();
+        if (!skippedClassNames.contains(className)) {
+          return element;
+        }
+      }
+      throw new AssertionError();
+    }
+  }
+}
diff --git a/test/com/google/inject/servlet/ServletTest.java b/test/com/google/inject/servlet/ServletTest.java
index 7d9a72fc..ec12ea6a 100644
--- a/test/com/google/inject/servlet/ServletTest.java
+++ b/test/com/google/inject/servlet/ServletTest.java
@@ -16,10 +16,11 @@
 
 package com.google.inject.servlet;
 
-import com.google.inject.BinderImpl;
 import com.google.inject.Container;
 import com.google.inject.CreationException;
 import com.google.inject.Key;
+import com.google.inject.Guice;
+import com.google.inject.AbstractModule;
 import java.io.IOException;
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -170,11 +171,13 @@ public class ServletTest extends TestCase {
   }
 
   private Container createContainer() throws CreationException {
-    BinderImpl builder = new BinderImpl();
-    builder.install(new ServletModule());
-    builder.bind(InSession.class);
-    builder.bind(InRequest.class);
-    return builder.createContainer();
+    return Guice.createContainer(new AbstractModule() {
+      protected void configure() {
+        install(new ServletModule());
+        bind(InSession.class);
+        bind(InRequest.class);
+      }
+    });
   }
 
   @SessionScoped