From 8182c82f9d33ac41851ad7521df4c5090aa33fd5 Mon Sep 17 00:00:00 2001 From: elandau Date: Fri, 11 Dec 2015 09:52:16 -0800 Subject: [PATCH 1/5] New ConditionalBinder to simplify specifying multiple bindings for a single type --- .../governator/PropertiesPropertySource.java | 40 +- .../governator/conditional/Conditional.java | 15 + .../conditional/ConditionalBinder.java | 406 ++++++++++++++++++ .../conditional/ConditionalBinding.java | 25 ++ .../ConditionalBindingsTargetVisitor.java | 7 + .../conditional/ConditionalElement.java | 19 + .../conditional/ConditionalElementImpl.java | 84 ++++ .../conditional/ConditionalOnProfile.java | 41 ++ .../conditional/ConditionalOnProperty.java | 43 ++ .../governator/conditional/Indexer.java | 185 ++++++++ .../governator/conditional/Matcher.java | 14 + .../conditional/ConditionalBinderTest.java | 346 +++++++++++++++ 12 files changed, 1224 insertions(+), 1 deletion(-) create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinding.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBindingsTargetVisitor.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElement.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElementImpl.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/Indexer.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java create mode 100644 governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java diff --git a/governator-core/src/main/java/com/netflix/governator/PropertiesPropertySource.java b/governator-core/src/main/java/com/netflix/governator/PropertiesPropertySource.java index e0bc3d6c..7d8901c4 100644 --- a/governator-core/src/main/java/com/netflix/governator/PropertiesPropertySource.java +++ b/governator-core/src/main/java/com/netflix/governator/PropertiesPropertySource.java @@ -4,21 +4,35 @@ import javax.inject.Singleton; +import com.google.inject.Binder; import com.google.inject.Module; import com.google.inject.Provides; import com.netflix.governator.spi.PropertySource; -public class PropertiesPropertySource extends AbstractPropertySource { +public final class PropertiesPropertySource extends AbstractPropertySource implements Module { private Properties props; public PropertiesPropertySource(Properties props) { this.props = props; } + public PropertiesPropertySource() { + this(new Properties()); + } + public static PropertiesPropertySource from(Properties props) { return new PropertiesPropertySource(props); } + public PropertiesPropertySource setProperty(String key, String value) { + props.setProperty(key, value); + return this; + } + + public boolean hasProperty(String key, String value) { + return props.containsKey(key); + } + public static Module toModule(final Properties props) { return new SingletonModule() { @Provides @@ -38,4 +52,28 @@ public String get(String key) { public String get(String key, String defaultValue) { return props.getProperty(key, defaultValue); } + + @Override + public void configure(Binder binder) { + binder.bind(PropertySource.class).toInstance(this); + } + + @Override + public int hashCode() { + return getClass().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + + throw new RuntimeException("Only one PropertiesModule may be installed"); + } + + } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java new file mode 100644 index 00000000..d7047457 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java @@ -0,0 +1,15 @@ +package com.netflix.governator.conditional; + + +/** + * Contract for any conditional that may be applied to a conditional binding + * bound via {@link ConditionalBinder}. + */ +public interface Conditional> { + /** + * @return Class that can process this conditional. This class + * is instantiated by Guice and is therefore injectable. The class should be + * annotated as a {@literal @}Singleton + */ + Class> getMatcherClass(); +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java new file mode 100644 index 00000000..a7d8a81d --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java @@ -0,0 +1,406 @@ +package com.netflix.governator.conditional; + +import java.lang.annotation.Annotation; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.google.inject.Binder; +import com.google.inject.Binding; +import com.google.inject.ConfigurationException; +import com.google.inject.Inject; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.Module; +import com.google.inject.Provider; +import com.google.inject.Stage; +import com.google.inject.TypeLiteral; +import com.google.inject.binder.LinkedBindingBuilder; +import com.google.inject.internal.Errors; +import com.google.inject.spi.BindingTargetVisitor; +import com.google.inject.spi.Dependency; +import com.google.inject.spi.Message; +import com.google.inject.spi.ProviderInstanceBinding; +import com.google.inject.spi.ProviderWithExtensionVisitor; +import com.google.inject.spi.Toolable; + +/** + * An API to bind multiple conditional candidates separately, only to evaluate and + * resolve to one candidate when the type is actually injected. + * ConditionalBinder is intended for use in a Module + * + * 

+ * public class SnacksModule extends AbstractModule {
+ *   protected void configure() {
+ *     ConditionalBinder<Snack> conditionalbinder
+ *         = ConditionalBinder.newConditionalBinder(binder(), Snack.class);
+ *     multibinder.when(new ConditionalOnProperty("type", "twix")).toInstance(new Twix());
+ *     multibinder.when(new ConditionalOnProperty("type", "snickers")).toProvider(SnickersProvider.class);
+ *     multibinder.when(new ConditionalOnProperty("type", "skittles")).to(Skittles.class);
+ *     multibinder.whenNone().to(Carrots.class);
+ *   }
+ * }
+ * + *

With this binding when injecting {@code } all conditionals will be evaluated and only + * the single matched binding will be injected + * 


+ * class SnackMachine {
+ *   {@literal @}Inject
+ *   public SnackMachine(Snack snacks) { ... }
+ * }
+ * + *

Contributing conditional bindings from different modules is supported. For + * example, it is okay for both {@code CandyModule} and {@code ChipsModule} + * to create their own {@code ConditionalBinder}, and to each contribute + * conditional bindings to the set of candidate snacks. When a Snack is injected, it will + * use the one conditional bindings that matched. + * + * Exactly one conditional binding may be matched. An exception will be thrown + * when Snack is injected and no or multiple bindings' conditions are matched. + * + *

Conditionals are evaluated at injection time. If an element is bound to a + * provider, that provider's get method will be called each time Snack is + * injected (unless the binding is in Singleton scope, in which case Guice will cache + * the first call to get). + * + * Conditionals may only be used in Stage.DEVELOPMENT since running in Stage.PRODUCTION will + * result in every Singleton conditional candidate being instantiated eagerly. Any attempt + * to binding in Stage.PRODUCTION will result in a CreationException + * + * TODO: Strip all conditional bindings of their scope so they cannot be eager singletons. + * Alternatively, force lazy singleton behavior + * TODO: Discuss whether the conditional binder key should be singleton by default + */ +public abstract class ConditionalBinder { + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type}. + */ + public static ConditionalBinder newConditionalBinder(Binder binder, TypeLiteral type) { + return newRealConditionalBinder(binder, Key.get(type)); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type}. + */ + public static ConditionalBinder newConditionalBinder(Binder binder, Class type) { + return newRealConditionalBinder(binder, Key.get(type)); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type} with + * the qualifier {@code annotation} + */ + public static ConditionalBinder newConditionalBinder( + Binder binder, TypeLiteral type, Annotation annotation) { + return newRealConditionalBinder(binder, Key.get(type, annotation)); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type} with + * the qualifier {@code annotation} + */ + public static ConditionalBinder newConditionalBinder( + Binder binder, Class type, Annotation annotation) { + return newRealConditionalBinder(binder, Key.get(type, annotation)); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type} with + * the qualifier {@code annotation} + */ + public static ConditionalBinder newConditionalBinder(Binder binder, TypeLiteral type, + Class annotationType) { + return newRealConditionalBinder(binder, Key.get(type, annotationType)); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code key} where + * key may or may not have a qualifier. + */ + public static ConditionalBinder newConditionalBinder(Binder binder, Key key) { + return newRealConditionalBinder(binder, key); + } + + /** + * Returns a new ConditionalBinder that tracks all candidate instances of {@code type} with + * the qualifier {@code annotation} + */ + public static ConditionalBinder newConditionalBinder(Binder binder, Class type, + Class annotationType) { + return newRealConditionalBinder(binder, Key.get(type, annotationType)); + } + + static ConditionalBinder newRealConditionalBinder(Binder binder, Key key) { + if (binder.currentStage().equals(Stage.PRODUCTION)) { + throw new RuntimeException("ConditionalBinder may not be used in Stage.PRODUCTION. Use Stage.DEVELOPMENT."); + } + + binder = binder.skipSources(RealConditionalBinder.class, ConditionalBinder.class); + RealConditionalBinder result = new RealConditionalBinder(binder, key); + binder.install(result); + return result; + } + + /** + * Returns a binding builder used to add a new conditional candidate for the key. + * Each bound element must have a distinct value. Only the matching candidate will + * be instantiated and its provider cached when the key is first injected. + * + *

It is an error to call this method without also calling one of the + * {@code to} methods on the returned binding builder. + * + *

Scoping elements independently supported per binding. Use the {@code in} method + * to specify a binding scope. + */ + public abstract LinkedBindingBuilder whenMatch(Conditional obj); + + /** + * Returns a binding builder used to specify the candidate for the key when no other + * conditional bindings have been met. There can be only one default candidate. Only the + * matching candidate will be instantiated and its provider cached when the key is + * first injected. + * + *

It is an error to call this method without also calling one of the + * {@code to} methods on the returned binding builder. + * + *

Scoping elements independently supported per binding. Use the {@code in} method + * to specify a binding scope. + */ + public abstract LinkedBindingBuilder whenNoMatch(); + + static final class RealConditionalBinder extends ConditionalBinder + implements Module, ProviderWithExtensionVisitor, ConditionalBinding { + + private final TypeLiteral elementType; + private final String keyName; + private final Key primaryKey; + private ImmutableList> candidateBindings; + private Set> dependencies; + + private Provider matchedProvider; + private Binder binder; + + public RealConditionalBinder( + Binder binder, + Key key) { + this.binder = checkNotNull(binder, "binder"); + this.primaryKey = checkNotNull(key, "key"); + this.elementType = checkNotNull(key.getTypeLiteral(), "elementType"); + this.keyName = checkNotNull(ConditionalElementImpl.nameOf(key), "keyName"); + } + + @Override + public void configure(Binder binder) { + checkConfiguration(!isInitialized(), "ConditionalBinder was already initialized"); + binder.bind(primaryKey).toProvider(this); + } + + Key getKeyForNewItem(Conditional obj) { + checkConfiguration(!isInitialized(), "ConditionalBinder was already initialized"); + return Key.get(elementType, new ConditionalElementImpl(keyName, obj)); + } + + @Override + public LinkedBindingBuilder whenMatch(Conditional obj) { + return binder.bind(getKeyForNewItem(obj)); + } + + @Override + public LinkedBindingBuilder whenNoMatch() { + return binder.bind(getKeyForNewItem(null)); + } + + /** + * Invoked by Guice at Injector-creation time to prepare providers for each + * element in this set. + */ + @Toolable + @Inject + void initialize(Injector injector) { + List> candidateBindings = Lists.newArrayList(); + Binding matchedBinding = null; + Binding defaultBinding = null; + + Set index = Sets.newHashSet(); + Indexer indexer = new Indexer(injector); + List> dependencies = Lists.newArrayList(); + + for (Binding entry : injector.findBindingsByType(elementType)) { + if (keyMatches(entry.getKey())) { + @SuppressWarnings("unchecked") // protected by findBindingsByType() + Binding binding = (Binding) entry; + if (index.add(binding.acceptTargetVisitor(indexer))) { + candidateBindings.add(binding); + + Conditional condition = ((ConditionalElementImpl) entry.getKey().getAnnotation()).getCondition(); + if (condition == null) { + if (defaultBinding == null) { + defaultBinding = binding; + } + else { + throw newDuplicateBindingException(primaryKey, defaultBinding, binding); + } + } + else { + @SuppressWarnings("unchecked") + Class matcherType = condition.getMatcherClass(); + Matcher matcher = (Matcher)injector.getInstance(matcherType); + if (matcher.match(condition)) { + if (matchedBinding == null) { + matchedBinding = binding; + } + else { + throw newDuplicateBindingException(primaryKey, matchedBinding, binding); + } + } + } + dependencies.add(Dependency.get(binding.getKey())); + } + } + } + + if (matchedBinding == null) { + matchedBinding = defaultBinding; + } + + if (matchedBinding == null) { + throw newNoMatchingBindingException(primaryKey, candidateBindings); + } + + dependencies.add(Dependency.get(matchedBinding.getKey())); + this.matchedProvider = matchedBinding.getProvider(); + + this.candidateBindings = ImmutableList.copyOf(candidateBindings); + this.binder = null; + } + + private boolean keyMatches(Key key) { + return key.getTypeLiteral().equals(elementType) + && key.getAnnotation() instanceof ConditionalElement + && ((ConditionalElement) key.getAnnotation()).keyName().equals(keyName); + } + + @Override + public T get() { + checkConfiguration(isInitialized(), "ConditionalBinder is not initialized"); + return matchedProvider.get(); + } + + @Override + public Key getKey() { + return primaryKey; + } + + @Override + public List> getCandidateElements() { + if (isInitialized()) { + return (List>) (List) candidateBindings; // safe because bindings is immutable. + } + else { + throw new UnsupportedOperationException("getElements() not supported for module bindings"); + } + } + + @Override + public V acceptExtensionVisitor( + BindingTargetVisitor visitor, + ProviderInstanceBinding binding) { + if (visitor instanceof ConditionalBindingsTargetVisitor) { + return ((ConditionalBindingsTargetVisitor) visitor).visit(this); + } + else { + return visitor.visit(binding); + } + } + + private boolean isInitialized() { + return binder == null; + } + + @Override + public boolean containsElement(com.google.inject.spi.Element element) { + if (element instanceof Binding) { + Binding binding = (Binding) element; + return keyMatches(binding.getKey()) + || binding.getKey().equals(primaryKey); + } + else { + return false; + } + } + + @Override + public int hashCode() { + return primaryKey.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + RealConditionalBinder other = (RealConditionalBinder) obj; + return primaryKey.equals(other.primaryKey); + } + } + + static void checkConfiguration(boolean condition, String format, Object... args) { + if (condition) { + return; + } + + throw new ConfigurationException(ImmutableSet.of(new Message(Errors.format(format, args)))); + } + + private static ConfigurationException newDuplicateBindingException( + Key key, + Binding existingBindings, + Binding duplicateBinding) { + // When the value strings don't match, include them both as they may be useful for debugging + return new ConfigurationException(ImmutableSet.of(new Message(Errors.format( + "%s injection failed due to multiple matching conditionals:" + + "\n \"%s\"\n bound at %s" + + "\n \"%s\"\n bound at %s", + key, + duplicateBinding.getKey(), + duplicateBinding.getSource(), + existingBindings.getKey(), + existingBindings.getSource())))); + } + + private static ConfigurationException newNoMatchingBindingException( + Key key, + List> candidateBindings) { + + StringBuilder builder = new StringBuilder(); + builder.append(String.format("%s injection failed due to no matching conditional", key)); + + if (!candidateBindings.isEmpty()) { + for (Binding binding : candidateBindings) { + builder.append(String.format("\n \"%s\"\n bound at %s", + binding.getKey(), + binding.getSource())); + } + } + else { + builder.append("\n No to() bindings were specified "); + } + return new ConfigurationException(ImmutableSet.of(new Message(Errors.format(builder.toString())))); + } + + static T checkNotNull(T reference, String name) { + if (reference != null) { + return reference; + } + + NullPointerException npe = new NullPointerException(name); + throw new ConfigurationException(ImmutableSet.of(new Message(npe + .toString(), npe))); + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinding.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinding.java new file mode 100644 index 00000000..a2244af0 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinding.java @@ -0,0 +1,25 @@ +package com.netflix.governator.conditional; + +import java.util.List; + +import com.google.inject.Binding; +import com.google.inject.Key; +import com.google.inject.spi.Element; + +/** + * Binding object passed to a ConditionalBindingTargetVisitor when visiting + * Guice's bindings + */ +public interface ConditionalBinding { + /** + * @return Key for the main type for which there are conditional bindings + */ + Key getKey(); + + /** + * @return All candidate elements of which one should match + */ + List> getCandidateElements(); + + boolean containsElement(Element element); +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBindingsTargetVisitor.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBindingsTargetVisitor.java new file mode 100644 index 00000000..df4f06ec --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBindingsTargetVisitor.java @@ -0,0 +1,7 @@ +package com.netflix.governator.conditional; + +import com.google.inject.spi.BindingTargetVisitor; + +public interface ConditionalBindingsTargetVisitor extends BindingTargetVisitor { + V visit(ConditionalBinding conditionalBinding); +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElement.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElement.java new file mode 100644 index 00000000..8150f2e5 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElement.java @@ -0,0 +1,19 @@ +package com.netflix.governator.conditional; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; + +import com.google.inject.BindingAnnotation; + +/** + * Unique qualifier associated with conditional bindings. The unique qualifier + * is used to ensure that the bound element isn't bound to a real key that user + * code would inject. + */ +@Retention(RUNTIME) +@BindingAnnotation +@interface ConditionalElement { + String keyName(); + int uniqueId(); +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElementImpl.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElementImpl.java new file mode 100644 index 00000000..2999eb22 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalElementImpl.java @@ -0,0 +1,84 @@ +package com.netflix.governator.conditional; + +import java.lang.annotation.Annotation; +import java.util.concurrent.atomic.AtomicInteger; + +import com.google.inject.Key; +import com.google.inject.internal.Annotations; + +class ConditionalElementImpl implements ConditionalElement { + private static final AtomicInteger nextUniqueId = new AtomicInteger(1); + + private final int uniqueId; + private final String keyName; + private final Conditional condition; + + ConditionalElementImpl(String keyName, Conditional condition) { + this(keyName, condition, nextUniqueId.incrementAndGet()); + } + + ConditionalElementImpl(String keyName, Conditional condition, int uniqueId) { + this.uniqueId = uniqueId; + this.keyName = keyName; + this.condition = condition; + } + + @Override + public String keyName() { + return keyName; + } + + @Override + public int uniqueId() { + return uniqueId; + } + + public Conditional getCondition() { + return condition; + } + + @Override + public Class annotationType() { + return ConditionalElement.class; + } + + @Override + public String toString() { + return "@" + ConditionalElement.class.getName() + "(keyName=" + keyName + + ",uniqueId=" + uniqueId + ",condition=" + condition + ")"; + } + + @Override + public boolean equals(Object o) { + return o instanceof ConditionalElement + && ((ConditionalElement) o).keyName().equals(keyName()) + && ((ConditionalElement) o).uniqueId() == uniqueId(); + } + + @Override + public int hashCode() { + return ((127 * "keyName".hashCode()) ^ keyName.hashCode()) + + ((127 * "uniqueId".hashCode()) ^ uniqueId); + } + + /** + * Returns the name the binding should use. This is based on the annotation. + * If the annotation has an instance and is not a marker annotation, we ask + * the annotation for its toString. If it was a marker annotation or just an + * annotation type, we use the annotation's name. Otherwise, the name is the + * empty string. + */ + static String nameOf(Key key) { + Annotation annotation = key.getAnnotation(); + Class annotationType = key.getAnnotationType(); + if (annotation != null && !Annotations.isMarker(annotationType)) { + return key.getAnnotation().toString(); + } + else if (key.getAnnotationType() != null) { + return "@" + key.getAnnotationType().getName(); + } + else { + return ""; + } + } +} \ No newline at end of file diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java new file mode 100644 index 00000000..7a4bbd76 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java @@ -0,0 +1,41 @@ +package com.netflix.governator.conditional; + +import java.util.Set; + +import javax.inject.Singleton; + +import com.google.inject.Inject; +import com.netflix.governator.annotations.binding.Profiles; + +final public class ConditionalOnProfile implements Conditional { + private String profile; + + public ConditionalOnProfile(String profile) { + this.profile = profile; + } + + @Override + public Class> getMatcherClass() { + return ConditionalOnProfileMatcher.class; + } + + @Override + public String toString() { + return "ConditionalOnProfile[" + profile + "]"; + } + + @Singleton + final public static class ConditionalOnProfileMatcher implements Matcher { + @Inject(optional=true) + @Profiles + Set profiles; + + @Override + public boolean match(ConditionalOnProfile condition) { + if (profiles == null) { + return false; + } + return profiles.contains(condition.profile); + } + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java new file mode 100644 index 00000000..9de06b40 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java @@ -0,0 +1,43 @@ +package com.netflix.governator.conditional; + +import javax.inject.Singleton; + +import com.google.inject.Inject; +import com.netflix.governator.spi.PropertySource; + +/** + * Conditional that evaluates to true if the a property is set to a specific value + */ +public class ConditionalOnProperty implements Conditional { + private String value; + private String key; + + public ConditionalOnProperty(String key, String value) { + this.key = key; + this.value = value; + } + + @Override + public Class> getMatcherClass() { + return ConditionalOnPropertyMatcher.class; + } + + @Override + public String toString() { + return "ConditionalOnProperty[" + key + "=" + value + "]"; + } + + @Singleton + public static class ConditionalOnPropertyMatcher implements Matcher { + @Inject(optional=true) + PropertySource properties; + + @Override + public boolean match(ConditionalOnProperty condition) { + if (properties == null) { + return false; + } + return condition.value.equals(properties.get(condition.key, "")); + } + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Indexer.java b/governator-core/src/main/java/com/netflix/governator/conditional/Indexer.java new file mode 100644 index 00000000..f88ef259 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Indexer.java @@ -0,0 +1,185 @@ +package com.netflix.governator.conditional; + +/** + * Copyright (C) 2014 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. + */ + +import java.lang.annotation.Annotation; + +import com.google.common.base.Objects; +import com.google.inject.Binding; +import com.google.inject.Injector; +import com.google.inject.Scope; +import com.google.inject.Scopes; +import com.google.inject.TypeLiteral; +import com.google.inject.spi.BindingScopingVisitor; +import com.google.inject.spi.ConstructorBinding; +import com.google.inject.spi.ConvertedConstantBinding; +import com.google.inject.spi.DefaultBindingTargetVisitor; +import com.google.inject.spi.ExposedBinding; +import com.google.inject.spi.InstanceBinding; +import com.google.inject.spi.LinkedKeyBinding; +import com.google.inject.spi.ProviderBinding; +import com.google.inject.spi.ProviderInstanceBinding; +import com.google.inject.spi.ProviderKeyBinding; +import com.google.inject.spi.UntargettedBinding; + +/** + * Visits bindings to return a {@code IndexedBinding} that can be used to + * emulate the binding deduplication that Guice internally performs. + */ +class Indexer extends + DefaultBindingTargetVisitor implements + BindingScopingVisitor { + enum BindingType { + INSTANCE, PROVIDER_INSTANCE, PROVIDER_KEY, LINKED_KEY, UNTARGETTED, CONSTRUCTOR, CONSTANT, EXPOSED, PROVIDED_BY, + } + + static class IndexedBinding { + final String annotationName; + final TypeLiteral typeLiteral; + final Object scope; + final BindingType type; + final Object extraEquality; + + IndexedBinding(Binding binding, BindingType type, Object scope, + Object extraEquality) { + this.scope = scope; + this.type = type; + this.extraEquality = extraEquality; + this.typeLiteral = binding.getKey().getTypeLiteral(); + ConditionalElement annotation = (ConditionalElement) binding.getKey().getAnnotation(); + this.annotationName = annotation.keyName(); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof IndexedBinding)) { + return false; + } + IndexedBinding o = (IndexedBinding) obj; + return type == o.type && Objects.equal(scope, o.scope) + && typeLiteral.equals(o.typeLiteral) + && annotationName.equals(o.annotationName) + && Objects.equal(extraEquality, o.extraEquality); + } + + @Override + public int hashCode() { + return Objects.hashCode(type, scope, typeLiteral, annotationName, + extraEquality); + } + } + + final Injector injector; + + Indexer(Injector injector) { + this.injector = injector; + } + + boolean isIndexable(Binding binding) { + return binding.getKey().getAnnotation() instanceof ConditionalElement; + } + + private Object scope(Binding binding) { + return binding.acceptScopingVisitor(this); + } + + @Override + public Indexer.IndexedBinding visit( + ConstructorBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.CONSTRUCTOR, + scope(binding), binding.getConstructor()); + } + + @Override + public Indexer.IndexedBinding visit( + ConvertedConstantBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.CONSTANT, + scope(binding), binding.getValue()); + } + + @Override + public Indexer.IndexedBinding visit(ExposedBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.EXPOSED, + scope(binding), binding); + } + + @Override + public Indexer.IndexedBinding visit( + InstanceBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.INSTANCE, + scope(binding), binding.getInstance()); + } + + @Override + public Indexer.IndexedBinding visit( + LinkedKeyBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.LINKED_KEY, + scope(binding), binding.getLinkedKey()); + } + + @Override + public Indexer.IndexedBinding visit( + ProviderBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.PROVIDED_BY, + scope(binding), injector.getBinding(binding.getProvidedKey())); + } + + @Override + public Indexer.IndexedBinding visit( + ProviderInstanceBinding binding) { + return new Indexer.IndexedBinding(binding, + BindingType.PROVIDER_INSTANCE, scope(binding), + binding.getUserSuppliedProvider()); + } + + @Override + public Indexer.IndexedBinding visit( + ProviderKeyBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.PROVIDER_KEY, + scope(binding), binding.getProviderKey()); + } + + @Override + public Indexer.IndexedBinding visit( + UntargettedBinding binding) { + return new Indexer.IndexedBinding(binding, BindingType.UNTARGETTED, + scope(binding), null); + } + + private static final Object EAGER_SINGLETON = new Object(); + + @Override + public Object visitEagerSingleton() { + return EAGER_SINGLETON; + } + + @Override + public Object visitNoScoping() { + return Scopes.NO_SCOPE; + } + + @Override + public Object visitScope(Scope scope) { + return scope; + } + + @Override + public Object visitScopeAnnotation( + Class scopeAnnotation) { + return scopeAnnotation; + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java b/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java new file mode 100644 index 00000000..f2bb7fbb --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java @@ -0,0 +1,14 @@ +package com.netflix.governator.conditional; + +/** + * Matcher for a specific conditional. Matchers are created by Guice and + * are therefore injectable. + * @param + */ +public interface Matcher { + /** + * @param condition + * @return Evaluate the conditional and return true if matched + */ + boolean match(T condition); +} diff --git a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java new file mode 100644 index 00000000..1c6a42d0 --- /dev/null +++ b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java @@ -0,0 +1,346 @@ +package com.netflix.governator.conditional; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +import com.google.inject.AbstractModule; +import com.google.inject.CreationException; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.Stage; +import com.google.inject.name.Names; +import com.google.inject.util.Modules; +import com.netflix.governator.PropertiesPropertySource; + +public class ConditionalBinderTest { + @Rule + public TestName name = new TestName(); + + public static interface Foo { + public String getName(); + } + + public static class FooImpl implements Foo { + private final String name; + + public FooImpl(String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + } + + @Singleton + static class SingletonFoo implements Foo { + static int injectedCount = 0; + + @Inject + SingletonFoo() { + injectedCount++; + } + + @Override + public String getName() { + return "singleton"; + } + } + + static class NonSingletonFoo implements Foo { + static int injectedCount = 0; + + @Inject + NonSingletonFoo() { + injectedCount++; + } + + @Override + public String getName() { + return "nonsingleton"; + } + } + + @Before + public void before() { + SingletonFoo.injectedCount = 0; + NonSingletonFoo.injectedCount = 0; + } + + public static class ModuleGroup1 extends AbstractModule { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenMatch(new ConditionalOnProperty("group1", "a")) + .toInstance(new FooImpl("group1_a")); + binder.whenMatch(new ConditionalOnProperty("group1", "b")) + .toInstance(new FooImpl("group1_b")); + binder.whenNoMatch() + .toInstance(new FooImpl("group1_default")); + } + } + + public static class ModuleGroup2 extends AbstractModule { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class, Names.named("group2")); + + binder.whenMatch(new ConditionalOnProperty("group2", "a")) + .toInstance(new FooImpl("group2_a")); + binder.whenMatch(new ConditionalOnProperty("group2", "b")) + .toInstance(new FooImpl("group2_b")); + } + } + + public static class ModuleNoConditional extends AbstractModule { + @Override + protected void configure() { + bind(Key.get(Foo.class)).toInstance(new FooImpl("unconditional")); + } + } + + @Test + public void bindToMatchedOfMultipleConditionals() { + Injector injector = Guice.createInjector( + new PropertiesPropertySource() + .setProperty("group1", "a"), + new ModuleGroup1()); + + Foo foo1 = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo1.getName(), "group1_a"); + } + + @Test + public void simpleWithDefault() { + Injector injector = Guice.createInjector( + new PropertiesPropertySource(), + new ModuleGroup1()); + + Foo foo1 = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo1.getName(), "group1_default"); + } + + @Test + public void differentiateBetweenMultipleQualifiers() { + Injector injector = Guice.createInjector( + new PropertiesPropertySource() + .setProperty("group1", "a") + .setProperty("group2", "b"), + new ModuleGroup1(), + new ModuleGroup2()); + + Foo foo1 = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo1.getName(), "group1_a"); + + Foo foo2 = injector.getInstance(Key.get(Foo.class, Names.named("group2"))); + Assert.assertEquals(foo2.getName(), "group2_b"); + } + + @Test + public void overrideConditionalWithNonConditional() { + Injector injector = Guice.createInjector(Modules.override( + new PropertiesPropertySource() + .setProperty("group1", "a"), + new ModuleGroup1() + ) + .with( + new ModuleNoConditional())); + + Foo foo = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo.getName(), "unconditional"); + } + + @Test + public void bindToSingleConditionalWithNoDefault() { + Injector injector = Guice.createInjector( + new PropertiesPropertySource() + .setProperty("foo", "value"), + new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenMatch(new ConditionalOnProperty("foo", "value")) + .toInstance(new FooImpl("value")); + } + }); + + Foo foo = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo.getName(), "value"); + } + + @Test + public void bindToJustDefault() { + Injector injector = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenNoMatch() + .toInstance(new FooImpl("default")); + } + }); + + Foo foo = injector.getInstance(Key.get(Foo.class)); + Assert.assertEquals(foo.getName(), "default"); + } + + @Test + public void confirmSingletonConditionalBehavior() { + Injector injector = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + binder.whenNoMatch().to(SingletonFoo.class); + + bind(SingletonFoo.class); + } + }); + + Assert.assertEquals(0, SingletonFoo.injectedCount); + Foo foo1 = injector.getInstance(Foo.class); + Assert.assertEquals(1, SingletonFoo.injectedCount); + Foo foo2 = injector.getInstance(Foo.class); + Assert.assertEquals(1, SingletonFoo.injectedCount); + } + + @Test + public void confirmSingletonNotEagerlyCreated() { + Assert.assertEquals(0, SingletonFoo.injectedCount); + Assert.assertEquals(0, NonSingletonFoo.injectedCount); + + Injector injector = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + binder.whenMatch(new ConditionalOnProperty("foo", "1")).to(SingletonFoo.class); + binder.whenNoMatch().to(NonSingletonFoo.class); + } + }); + + Assert.assertEquals(0, SingletonFoo.injectedCount); + Assert.assertEquals(0, NonSingletonFoo.injectedCount); + Foo foo1 = injector.getInstance(Foo.class); + Foo foo2 = injector.getInstance(Foo.class); + Assert.assertEquals(0, SingletonFoo.injectedCount); + Assert.assertEquals(2, NonSingletonFoo.injectedCount); + Assert.assertEquals(foo1.getName(), "nonsingleton"); + } + + @Test(expected=CreationException.class) + public void failWithNoMatchedConditionals(){ + try { + Guice.createInjector(new ModuleGroup1(), new ModuleGroup2()); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + @Test(expected=CreationException.class) + public void failWithNoBindTo(){ + try { + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + } + }); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + @Test(expected=CreationException.class) + public void failWithDuplicateMatchedConditionals(){ + try { + Guice.createInjector( + new PropertiesPropertySource() + .setProperty("group1", "a"), + new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenMatch(new ConditionalOnProperty("group1", "a")) + .toInstance(new FooImpl("group1_a")); + binder.whenMatch(new ConditionalOnProperty("group1", "a")) + .toInstance(new FooImpl("group1_b")); + } + }); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + @Test(expected=CreationException.class) + public void failOnMultipleDefaults() { + try { + Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenNoMatch() + .toInstance(new FooImpl("default1")); + binder.whenNoMatch() + .toInstance(new FooImpl("default2")); + } + }); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + @Test(expected=CreationException.class) + public void conflictingConditionalAndNonConditional() { + try { + Guice.createInjector( + new PropertiesPropertySource() + .setProperty("group1", "a"), + new ModuleGroup1(), + new ModuleNoConditional()); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + @Test(expected=CreationException.class) + public void productionStageNotSupported() { + try { + Guice.createInjector(Stage.PRODUCTION, new AbstractModule() { + @Override + protected void configure() { + ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); + + binder.whenNoMatch().to(SingletonFoo.class); + binder.whenMatch(new ConditionalOnProperty("foo", "1")).to(NonSingletonFoo.class); + } + }); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + + } +} From 0fac88c0ec19282894ac9cf9500a41dab07b8ac8 Mon Sep 17 00:00:00 2001 From: elandau Date: Fri, 11 Dec 2015 12:57:08 -0800 Subject: [PATCH 2/5] fix javadoc --- .../netflix/governator/conditional/ConditionalBinder.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java index a7d8a81d..b68a8688 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java @@ -37,10 +37,10 @@ * protected void configure() { * ConditionalBinder<Snack> conditionalbinder * = ConditionalBinder.newConditionalBinder(binder(), Snack.class); - * multibinder.when(new ConditionalOnProperty("type", "twix")).toInstance(new Twix()); - * multibinder.when(new ConditionalOnProperty("type", "snickers")).toProvider(SnickersProvider.class); - * multibinder.when(new ConditionalOnProperty("type", "skittles")).to(Skittles.class); - * multibinder.whenNone().to(Carrots.class); + * conditionalbinder.when(new ConditionalOnProperty("type", "twix")).toInstance(new Twix()); + * conditionalbinder.when(new ConditionalOnProperty("type", "snickers")).toProvider(SnickersProvider.class); + * conditionalbinder.when(new ConditionalOnProperty("type", "skittles")).to(Skittles.class); + * conditionalbinder.whenNone().to(Carrots.class); * } * } * From b0e47d05aec95c2907a32852154e2d717b5f1ebd Mon Sep 17 00:00:00 2001 From: elandau Date: Mon, 14 Dec 2015 22:03:11 -0800 Subject: [PATCH 3/5] Simplify Conditional API by replacing the Matcher class with member injection of any dependency needed by the conditional directly into the conditional. Add AND, OR, NOT operations on conditionals. --- .../conditional/AbstractConditional.java | 14 ++++++ .../conditional/AndConditional.java | 21 +++++++++ .../governator/conditional/Conditional.java | 34 ++++++++++++--- .../conditional/ConditionalBinder.java | 8 ++-- .../conditional/ConditionalOnProfile.java | 32 +++++--------- .../conditional/ConditionalOnProperty.java | 34 ++++++--------- .../governator/conditional/Matcher.java | 14 ------ .../conditional/NotConditional.java | 19 ++++++++ .../governator/conditional/OrConditional.java | 21 +++++++++ .../governator/internal/ElementsEx.java | 1 - .../conditional/ConditionalTest.java | 43 +++++++++++++++++++ 11 files changed, 172 insertions(+), 69 deletions(-) create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java delete mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java create mode 100644 governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java new file mode 100644 index 00000000..6c66f270 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java @@ -0,0 +1,14 @@ +package com.netflix.governator.conditional; + +public abstract class AbstractConditional extends Conditional { + @Override + public Conditional and(Conditional conditional) { + return new AndConditional(this, conditional); + } + + @Override + public Conditional or(Conditional conditional) { + return new OrConditional(this, conditional); + } + +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java new file mode 100644 index 00000000..db7f820c --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java @@ -0,0 +1,21 @@ +package com.netflix.governator.conditional; + +/** + * Conditional equivalent to + * + * first && second + */ +public class AndConditional extends AbstractConditional { + private final Conditional first; + private final Conditional second; + + public AndConditional(Conditional first, Conditional second) { + this.first = first; + this.second = second; + } + + @Override + public boolean evaluate() { + return first.evaluate() && second.evaluate(); + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java index d7047457..1d2c18a1 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java @@ -1,15 +1,37 @@ package com.netflix.governator.conditional; - /** * Contract for any conditional that may be applied to a conditional binding * bound via {@link ConditionalBinder}. + * + * Dependencies needed by a concrete conditional should be injected using + * member injection. + * */ -public interface Conditional> { +public abstract class Conditional { + /** + * Evaluate whether the condition is true. evaluate() is only called once at injector + * creation time. + * @return True if conditional is true otherwise false + */ + public abstract boolean evaluate(); + + /** + * @param conditional + * @return Composite conditional that does a logical AND of this conditional with another + */ + public abstract Conditional and(Conditional conditional); + + /** + * @param conditional + * @return Composite conditional that does a logical OR of this conditional with another + */ + public abstract Conditional or(Conditional conditional); + /** - * @return Class that can process this conditional. This class - * is instantiated by Guice and is therefore injectable. The class should be - * annotated as a {@literal @}Singleton + * @return Create a conditional that is a logical NOT of the provided conditional */ - Class> getMatcherClass(); + public static Conditional not(Conditional conditional) { + return new NotConditional(conditional); + } } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java index b68a8688..8237b492 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java @@ -219,7 +219,7 @@ public LinkedBindingBuilder whenNoMatch() { */ @Toolable @Inject - void initialize(Injector injector) { + void initialize(final Injector injector) { List> candidateBindings = Lists.newArrayList(); Binding matchedBinding = null; Binding defaultBinding = null; @@ -245,10 +245,8 @@ void initialize(Injector injector) { } } else { - @SuppressWarnings("unchecked") - Class matcherType = condition.getMatcherClass(); - Matcher matcher = (Matcher)injector.getInstance(matcherType); - if (matcher.match(condition)) { + injector.injectMembers(condition); + if (condition.evaluate()) { if (matchedBinding == null) { matchedBinding = binding; } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java index 7a4bbd76..ee5ad174 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java @@ -2,40 +2,30 @@ import java.util.Set; -import javax.inject.Singleton; - import com.google.inject.Inject; import com.netflix.governator.annotations.binding.Profiles; -final public class ConditionalOnProfile implements Conditional { - private String profile; +final public class ConditionalOnProfile extends AbstractConditional { + private final String profile; + @Inject(optional=true) + @Profiles + Set profiles; + public ConditionalOnProfile(String profile) { this.profile = profile; } @Override - public Class> getMatcherClass() { - return ConditionalOnProfileMatcher.class; + public boolean evaluate() { + if (profiles == null) { + return false; + } + return profiles.contains(profile); } @Override public String toString() { return "ConditionalOnProfile[" + profile + "]"; } - - @Singleton - final public static class ConditionalOnProfileMatcher implements Matcher { - @Inject(optional=true) - @Profiles - Set profiles; - - @Override - public boolean match(ConditionalOnProfile condition) { - if (profiles == null) { - return false; - } - return profiles.contains(condition.profile); - } - } } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java index 9de06b40..f844c0d1 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java @@ -1,43 +1,33 @@ package com.netflix.governator.conditional; -import javax.inject.Singleton; - import com.google.inject.Inject; import com.netflix.governator.spi.PropertySource; /** * Conditional that evaluates to true if the a property is set to a specific value */ -public class ConditionalOnProperty implements Conditional { - private String value; - private String key; +public class ConditionalOnProperty extends AbstractConditional { + private final String value; + private final String key; + @Inject(optional=true) + PropertySource properties; + public ConditionalOnProperty(String key, String value) { this.key = key; this.value = value; } @Override - public Class> getMatcherClass() { - return ConditionalOnPropertyMatcher.class; + public boolean evaluate() { + if (properties == null) { + return false; + } + return value.equals(properties.get(key, "")); } - + @Override public String toString() { return "ConditionalOnProperty[" + key + "=" + value + "]"; } - - @Singleton - public static class ConditionalOnPropertyMatcher implements Matcher { - @Inject(optional=true) - PropertySource properties; - - @Override - public boolean match(ConditionalOnProperty condition) { - if (properties == null) { - return false; - } - return condition.value.equals(properties.get(condition.key, "")); - } - } } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java b/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java deleted file mode 100644 index f2bb7fbb..00000000 --- a/governator-core/src/main/java/com/netflix/governator/conditional/Matcher.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.netflix.governator.conditional; - -/** - * Matcher for a specific conditional. Matchers are created by Guice and - * are therefore injectable. - * @param - */ -public interface Matcher { - /** - * @param condition - * @return Evaluate the conditional and return true if matched - */ - boolean match(T condition); -} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java new file mode 100644 index 00000000..af61ea2e --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java @@ -0,0 +1,19 @@ +package com.netflix.governator.conditional; + +/** + * Conditional equivalent to + * + * !conditional + */ +public class NotConditional extends AbstractConditional { + private final Conditional conditional; + + public NotConditional(Conditional conditional) { + this.conditional = conditional; + } + + @Override + public boolean evaluate() { + return !conditional.evaluate(); + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java new file mode 100644 index 00000000..ad145175 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java @@ -0,0 +1,21 @@ +package com.netflix.governator.conditional; + +/** + * Conditional equivalent to + * + * first || second + */ +public class OrConditional extends AbstractConditional { + private final Conditional first; + private final Conditional second; + + public OrConditional(AbstractConditional first, Conditional second) { + this.first = first; + this.second = second; + } + + @Override + public boolean evaluate() { + return first.evaluate() || second.evaluate(); + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/internal/ElementsEx.java b/governator-core/src/main/java/com/netflix/governator/internal/ElementsEx.java index 6a06df53..5dc6649c 100644 --- a/governator-core/src/main/java/com/netflix/governator/internal/ElementsEx.java +++ b/governator-core/src/main/java/com/netflix/governator/internal/ElementsEx.java @@ -7,7 +7,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import com.google.inject.Binding; import com.google.inject.ImplementedBy; diff --git a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java new file mode 100644 index 00000000..b63cca42 --- /dev/null +++ b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java @@ -0,0 +1,43 @@ +package com.netflix.governator.conditional; + +import junit.framework.Assert; + +import org.junit.Test; + +public class ConditionalTest { + public static class True extends AbstractConditional { + @Override + public boolean evaluate() { + return true; + } + } + + public static class False extends AbstractConditional { + @Override + public boolean evaluate() { + return false; + } + } + + @Test + public void testOr() { + Assert.assertTrue(new True().or(new True()).evaluate()); + Assert.assertTrue(new True().or(new False()).evaluate()); + Assert.assertTrue(new False().or(new True()).evaluate()); + Assert.assertFalse(new False().or(new False()).evaluate()); + } + + @Test + public void testAnd() { + Assert.assertTrue(new True().and(new True()).evaluate()); + Assert.assertFalse(new True().and(new False()).evaluate()); + Assert.assertFalse(new False().and(new True()).evaluate()); + Assert.assertFalse(new False().and(new False()).evaluate()); + } + + @Test + public void testNot() { + Assert.assertFalse(Conditional.not(new True()).evaluate()); + Assert.assertTrue(Conditional.not(new False()).evaluate()); + } +} From c51ed37aa076264c9cfa4158395c2a49807d0b87 Mon Sep 17 00:00:00 2001 From: elandau Date: Tue, 15 Dec 2015 14:47:49 -0800 Subject: [PATCH 4/5] Change Conditional#matches to accept the Injector as an argument. It will then be the responsibility of a composite conditional (AndConditional, etc.) to pass the Injector to its children, giving them access to the injector. --- .../conditional/AndConditional.java | 6 ++-- .../governator/conditional/Conditional.java | 4 ++- .../conditional/ConditionalBinder.java | 27 +++++++++------- .../conditional/ConditionalOnProfile.java | 16 ++++++---- .../conditional/ConditionalOnProperty.java | 12 +++---- .../conditional/NotConditional.java | 6 ++-- .../governator/conditional/OrConditional.java | 6 ++-- .../conditional/ConditionalBinderTest.java | 32 +++++++++---------- .../conditional/ConditionalTest.java | 26 ++++++++------- 9 files changed, 76 insertions(+), 59 deletions(-) diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java index db7f820c..e065abdf 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java @@ -1,5 +1,7 @@ package com.netflix.governator.conditional; +import com.google.inject.Injector; + /** * Conditional equivalent to * @@ -15,7 +17,7 @@ public AndConditional(Conditional first, Conditional second) { } @Override - public boolean evaluate() { - return first.evaluate() && second.evaluate(); + public boolean matches(Injector injector) { + return first.matches(injector) && second.matches(injector); } } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java index 1d2c18a1..1807fd12 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java @@ -1,5 +1,7 @@ package com.netflix.governator.conditional; +import com.google.inject.Injector; + /** * Contract for any conditional that may be applied to a conditional binding * bound via {@link ConditionalBinder}. @@ -14,7 +16,7 @@ public abstract class Conditional { * creation time. * @return True if conditional is true otherwise false */ - public abstract boolean evaluate(); + public abstract boolean matches(Injector injector); /** * @param conditional diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java index 8237b492..1d31f9ea 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalBinder.java @@ -22,6 +22,7 @@ import com.google.inject.internal.Errors; import com.google.inject.spi.BindingTargetVisitor; import com.google.inject.spi.Dependency; +import com.google.inject.spi.HasDependencies; import com.google.inject.spi.Message; import com.google.inject.spi.ProviderInstanceBinding; import com.google.inject.spi.ProviderWithExtensionVisitor; @@ -37,10 +38,10 @@ * protected void configure() { * ConditionalBinder<Snack> conditionalbinder * = ConditionalBinder.newConditionalBinder(binder(), Snack.class); - * conditionalbinder.when(new ConditionalOnProperty("type", "twix")).toInstance(new Twix()); - * conditionalbinder.when(new ConditionalOnProperty("type", "snickers")).toProvider(SnickersProvider.class); - * conditionalbinder.when(new ConditionalOnProperty("type", "skittles")).to(Skittles.class); - * conditionalbinder.whenNone().to(Carrots.class); + * conditionalbinder.whenMatchBind(new ConditionalOnProperty("type", "twix")).toInstance(new Twix()); + * conditionalbinder.whenMatchBind(new ConditionalOnProperty("type", "snickers")).toProvider(SnickersProvider.class); + * conditionalbinder.whenMatchBind(new ConditionalOnProperty("type", "skittles")).to(Skittles.class); + * conditionalbinder.whenNoMatchBind().to(Carrots.class); * } * } * @@ -155,7 +156,7 @@ static ConditionalBinder newRealConditionalBinder(Binder binder, Key k *

Scoping elements independently supported per binding. Use the {@code in} method * to specify a binding scope. */ - public abstract LinkedBindingBuilder whenMatch(Conditional obj); + public abstract LinkedBindingBuilder whenMatchBind(Conditional obj); /** * Returns a binding builder used to specify the candidate for the key when no other @@ -169,10 +170,10 @@ static ConditionalBinder newRealConditionalBinder(Binder binder, Key k *

Scoping elements independently supported per binding. Use the {@code in} method * to specify a binding scope. */ - public abstract LinkedBindingBuilder whenNoMatch(); + public abstract LinkedBindingBuilder whenNoMatchBind(); static final class RealConditionalBinder extends ConditionalBinder - implements Module, ProviderWithExtensionVisitor, ConditionalBinding { + implements Module, ProviderWithExtensionVisitor, ConditionalBinding, HasDependencies { private final TypeLiteral elementType; private final String keyName; @@ -204,12 +205,12 @@ Key getKeyForNewItem(Conditional obj) { } @Override - public LinkedBindingBuilder whenMatch(Conditional obj) { + public LinkedBindingBuilder whenMatchBind(Conditional obj) { return binder.bind(getKeyForNewItem(obj)); } @Override - public LinkedBindingBuilder whenNoMatch() { + public LinkedBindingBuilder whenNoMatchBind() { return binder.bind(getKeyForNewItem(null)); } @@ -245,8 +246,7 @@ void initialize(final Injector injector) { } } else { - injector.injectMembers(condition); - if (condition.evaluate()) { + if (condition.matches(injector)) { if (matchedBinding == null) { matchedBinding = binding; } @@ -330,6 +330,11 @@ public boolean containsElement(com.google.inject.spi.Element element) { } } + @Override + public Set> getDependencies() { + return dependencies; + } + @Override public int hashCode() { return primaryKey.hashCode(); diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java index ee5ad174..ccb7b9b6 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java @@ -2,26 +2,28 @@ import java.util.Set; -import com.google.inject.Inject; +import com.google.inject.Binding; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.TypeLiteral; import com.netflix.governator.annotations.binding.Profiles; final public class ConditionalOnProfile extends AbstractConditional { + private static final TypeLiteral> STRING_SET_TYPE = new TypeLiteral>() {}; + private final String profile; - @Inject(optional=true) - @Profiles - Set profiles; - public ConditionalOnProfile(String profile) { this.profile = profile; } @Override - public boolean evaluate() { + public boolean matches(Injector injector) { + Binding> profiles = injector.getExistingBinding(Key.get(STRING_SET_TYPE, Profiles.class)); if (profiles == null) { return false; } - return profiles.contains(profile); + return profiles.getProvider().get().contains(profile); } @Override diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java index f844c0d1..a310cbbd 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java @@ -1,6 +1,8 @@ package com.netflix.governator.conditional; -import com.google.inject.Inject; +import com.google.inject.Binding; +import com.google.inject.Injector; +import com.google.inject.Key; import com.netflix.governator.spi.PropertySource; /** @@ -10,20 +12,18 @@ public class ConditionalOnProperty extends AbstractConditional { private final String value; private final String key; - @Inject(optional=true) - PropertySource properties; - public ConditionalOnProperty(String key, String value) { this.key = key; this.value = value; } @Override - public boolean evaluate() { + public boolean matches(Injector injector) { + Binding properties = injector.getExistingBinding(Key.get(PropertySource.class)); if (properties == null) { return false; } - return value.equals(properties.get(key, "")); + return value.equals(properties.getProvider().get().get(key, "")); } @Override diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java index af61ea2e..574d1e21 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java @@ -1,5 +1,7 @@ package com.netflix.governator.conditional; +import com.google.inject.Injector; + /** * Conditional equivalent to * @@ -13,7 +15,7 @@ public NotConditional(Conditional conditional) { } @Override - public boolean evaluate() { - return !conditional.evaluate(); + public boolean matches(Injector injector) { + return !conditional.matches(injector); } } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java index ad145175..aeefcee9 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java @@ -1,5 +1,7 @@ package com.netflix.governator.conditional; +import com.google.inject.Injector; + /** * Conditional equivalent to * @@ -15,7 +17,7 @@ public OrConditional(AbstractConditional first, Conditional second) { } @Override - public boolean evaluate() { - return first.evaluate() || second.evaluate(); + public boolean matches(Injector injector) { + return first.matches(injector) || second.matches(injector); } } diff --git a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java index 1c6a42d0..a968cbe2 100644 --- a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java +++ b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalBinderTest.java @@ -80,11 +80,11 @@ public static class ModuleGroup1 extends AbstractModule { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenMatch(new ConditionalOnProperty("group1", "a")) + binder.whenMatchBind(new ConditionalOnProperty("group1", "a")) .toInstance(new FooImpl("group1_a")); - binder.whenMatch(new ConditionalOnProperty("group1", "b")) + binder.whenMatchBind(new ConditionalOnProperty("group1", "b")) .toInstance(new FooImpl("group1_b")); - binder.whenNoMatch() + binder.whenNoMatchBind() .toInstance(new FooImpl("group1_default")); } } @@ -94,9 +94,9 @@ public static class ModuleGroup2 extends AbstractModule { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class, Names.named("group2")); - binder.whenMatch(new ConditionalOnProperty("group2", "a")) + binder.whenMatchBind(new ConditionalOnProperty("group2", "a")) .toInstance(new FooImpl("group2_a")); - binder.whenMatch(new ConditionalOnProperty("group2", "b")) + binder.whenMatchBind(new ConditionalOnProperty("group2", "b")) .toInstance(new FooImpl("group2_b")); } } @@ -169,7 +169,7 @@ public void bindToSingleConditionalWithNoDefault() { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenMatch(new ConditionalOnProperty("foo", "value")) + binder.whenMatchBind(new ConditionalOnProperty("foo", "value")) .toInstance(new FooImpl("value")); } }); @@ -185,7 +185,7 @@ public void bindToJustDefault() { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenNoMatch() + binder.whenNoMatchBind() .toInstance(new FooImpl("default")); } }); @@ -200,7 +200,7 @@ public void confirmSingletonConditionalBehavior() { @Override protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenNoMatch().to(SingletonFoo.class); + binder.whenNoMatchBind().to(SingletonFoo.class); bind(SingletonFoo.class); } @@ -222,8 +222,8 @@ public void confirmSingletonNotEagerlyCreated() { @Override protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenMatch(new ConditionalOnProperty("foo", "1")).to(SingletonFoo.class); - binder.whenNoMatch().to(NonSingletonFoo.class); + binder.whenMatchBind(new ConditionalOnProperty("foo", "1")).to(SingletonFoo.class); + binder.whenNoMatchBind().to(NonSingletonFoo.class); } }); @@ -275,9 +275,9 @@ public void failWithDuplicateMatchedConditionals(){ protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenMatch(new ConditionalOnProperty("group1", "a")) + binder.whenMatchBind(new ConditionalOnProperty("group1", "a")) .toInstance(new FooImpl("group1_a")); - binder.whenMatch(new ConditionalOnProperty("group1", "a")) + binder.whenMatchBind(new ConditionalOnProperty("group1", "a")) .toInstance(new FooImpl("group1_b")); } }); @@ -296,9 +296,9 @@ public void failOnMultipleDefaults() { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenNoMatch() + binder.whenNoMatchBind() .toInstance(new FooImpl("default1")); - binder.whenNoMatch() + binder.whenNoMatchBind() .toInstance(new FooImpl("default2")); } }); @@ -332,8 +332,8 @@ public void productionStageNotSupported() { protected void configure() { ConditionalBinder binder = ConditionalBinder.newConditionalBinder(binder(), Foo.class); - binder.whenNoMatch().to(SingletonFoo.class); - binder.whenMatch(new ConditionalOnProperty("foo", "1")).to(NonSingletonFoo.class); + binder.whenNoMatchBind().to(SingletonFoo.class); + binder.whenMatchBind(new ConditionalOnProperty("foo", "1")).to(NonSingletonFoo.class); } }); } diff --git a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java index b63cca42..0d156c25 100644 --- a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java +++ b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java @@ -4,40 +4,42 @@ import org.junit.Test; +import com.google.inject.Injector; + public class ConditionalTest { public static class True extends AbstractConditional { @Override - public boolean evaluate() { + public boolean matches(Injector injector) { return true; } } public static class False extends AbstractConditional { @Override - public boolean evaluate() { + public boolean matches(Injector injector) { return false; } } @Test public void testOr() { - Assert.assertTrue(new True().or(new True()).evaluate()); - Assert.assertTrue(new True().or(new False()).evaluate()); - Assert.assertTrue(new False().or(new True()).evaluate()); - Assert.assertFalse(new False().or(new False()).evaluate()); + Assert.assertTrue(new True().or(new True()).matches(null)); + Assert.assertTrue(new True().or(new False()).matches(null)); + Assert.assertTrue(new False().or(new True()).matches(null)); + Assert.assertFalse(new False().or(new False()).matches(null)); } @Test public void testAnd() { - Assert.assertTrue(new True().and(new True()).evaluate()); - Assert.assertFalse(new True().and(new False()).evaluate()); - Assert.assertFalse(new False().and(new True()).evaluate()); - Assert.assertFalse(new False().and(new False()).evaluate()); + Assert.assertTrue(new True().and(new True()).matches(null)); + Assert.assertFalse(new True().and(new False()).matches(null)); + Assert.assertFalse(new False().and(new True()).matches(null)); + Assert.assertFalse(new False().and(new False()).matches(null)); } @Test public void testNot() { - Assert.assertFalse(Conditional.not(new True()).evaluate()); - Assert.assertTrue(Conditional.not(new False()).evaluate()); + Assert.assertFalse(Conditional.not(new True()).matches(null)); + Assert.assertTrue(Conditional.not(new False()).matches(null)); } } From efac56b09effd2414be20e6f83087c80d6ad4a9f Mon Sep 17 00:00:00 2001 From: elandau Date: Tue, 15 Dec 2015 19:24:30 -0800 Subject: [PATCH 5/5] Move and, or, and not conditional wrappers into a separate Conditionals class. Rename 'and' to 'allOf', and 'or' to 'anyOf'. --- .../conditional/AbstractConditional.java | 14 ------ .../conditional/AllOfConditional.java | 27 ++++++++++++ .../conditional/AndConditional.java | 23 ---------- .../conditional/AnyOfConditional.java | 28 ++++++++++++ .../governator/conditional/Conditional.java | 23 +--------- .../conditional/ConditionalOnProfile.java | 2 +- .../conditional/ConditionalOnProperty.java | 2 +- .../governator/conditional/Conditionals.java | 44 +++++++++++++++++++ .../conditional/NotConditional.java | 2 +- .../governator/conditional/OrConditional.java | 23 ---------- .../conditional/ConditionalTest.java | 24 +++++----- 11 files changed, 116 insertions(+), 96 deletions(-) delete mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AllOfConditional.java delete mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/AnyOfConditional.java create mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/Conditionals.java delete mode 100644 governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java deleted file mode 100644 index 6c66f270..00000000 --- a/governator-core/src/main/java/com/netflix/governator/conditional/AbstractConditional.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.netflix.governator.conditional; - -public abstract class AbstractConditional extends Conditional { - @Override - public Conditional and(Conditional conditional) { - return new AndConditional(this, conditional); - } - - @Override - public Conditional or(Conditional conditional) { - return new OrConditional(this, conditional); - } - -} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AllOfConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AllOfConditional.java new file mode 100644 index 00000000..187bb6aa --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/AllOfConditional.java @@ -0,0 +1,27 @@ +package com.netflix.governator.conditional; + +import java.util.ArrayList; +import java.util.List; + +import com.google.inject.Injector; + +/** + * Conditional that is true if and only if all child conditionals are true + */ +public class AllOfConditional implements Conditional { + private final List children; + + public AllOfConditional(List children) { + this.children = new ArrayList<>(children); + } + + @Override + public boolean matches(Injector injector) { + for (Conditional conditional : children) { + if (!conditional.matches(injector)) { + return false; + } + } + return true; + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java deleted file mode 100644 index e065abdf..00000000 --- a/governator-core/src/main/java/com/netflix/governator/conditional/AndConditional.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.netflix.governator.conditional; - -import com.google.inject.Injector; - -/** - * Conditional equivalent to - * - * first && second - */ -public class AndConditional extends AbstractConditional { - private final Conditional first; - private final Conditional second; - - public AndConditional(Conditional first, Conditional second) { - this.first = first; - this.second = second; - } - - @Override - public boolean matches(Injector injector) { - return first.matches(injector) && second.matches(injector); - } -} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/AnyOfConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/AnyOfConditional.java new file mode 100644 index 00000000..c2ea521c --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/AnyOfConditional.java @@ -0,0 +1,28 @@ +package com.netflix.governator.conditional; + +import java.util.ArrayList; +import java.util.List; + +import com.google.inject.Injector; + +/** + * Conditional that is true if at least one of the child conditionals + * is true + */ +public class AnyOfConditional implements Conditional { + private final List children; + + public AnyOfConditional(List children) { + this.children = new ArrayList<>(children); + } + + @Override + public boolean matches(Injector injector) { + for (Conditional conditional : children) { + if (conditional.matches(injector)) { + return true; + } + } + return false; + } +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java index 1807fd12..17a385ab 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Conditional.java @@ -10,30 +10,11 @@ * member injection. * */ -public abstract class Conditional { +public interface Conditional { /** * Evaluate whether the condition is true. evaluate() is only called once at injector * creation time. * @return True if conditional is true otherwise false */ - public abstract boolean matches(Injector injector); - - /** - * @param conditional - * @return Composite conditional that does a logical AND of this conditional with another - */ - public abstract Conditional and(Conditional conditional); - - /** - * @param conditional - * @return Composite conditional that does a logical OR of this conditional with another - */ - public abstract Conditional or(Conditional conditional); - - /** - * @return Create a conditional that is a logical NOT of the provided conditional - */ - public static Conditional not(Conditional conditional) { - return new NotConditional(conditional); - } + boolean matches(Injector injector); } diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java index ccb7b9b6..9bac1a5c 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProfile.java @@ -8,7 +8,7 @@ import com.google.inject.TypeLiteral; import com.netflix.governator.annotations.binding.Profiles; -final public class ConditionalOnProfile extends AbstractConditional { +final public class ConditionalOnProfile implements Conditional { private static final TypeLiteral> STRING_SET_TYPE = new TypeLiteral>() {}; private final String profile; diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java index a310cbbd..2d297bbe 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/ConditionalOnProperty.java @@ -8,7 +8,7 @@ /** * Conditional that evaluates to true if the a property is set to a specific value */ -public class ConditionalOnProperty extends AbstractConditional { +public class ConditionalOnProperty implements Conditional { private final String value; private final String key; diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/Conditionals.java b/governator-core/src/main/java/com/netflix/governator/conditional/Conditionals.java new file mode 100644 index 00000000..6c08a002 --- /dev/null +++ b/governator-core/src/main/java/com/netflix/governator/conditional/Conditionals.java @@ -0,0 +1,44 @@ +package com.netflix.governator.conditional; + +import java.util.Arrays; + +import com.google.common.base.Preconditions; + +/** + * Utility class for creating conditional + */ +public abstract class Conditionals { + private Conditionals() { + } + + /** + * Create a conditional that matches to true if and only if all of the child conditionals + * are true + * + * @param conditional + * @return + */ + public static Conditional allOf(Conditional... conditional) { + Preconditions.checkNotNull(conditional, "Cannot have a conditional allOf(null)"); + return new AllOfConditional(Arrays.asList(conditional)); + } + + /** + * Create a conditional that matches to true if at least one of the child conditionals is + * true + * @param conditional + * @return + */ + public static Conditional anyOf(Conditional... conditional) { + Preconditions.checkNotNull(conditional, "Cannot have a conditional anyOf(null)"); + return new AnyOfConditional(Arrays.asList(conditional)); + } + + /** + * @return Create a conditional that is a logical NOT of the provided conditional + */ + public static Conditional not(Conditional conditional) { + return new NotConditional(conditional); + } + +} diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java index 574d1e21..459fce98 100644 --- a/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java +++ b/governator-core/src/main/java/com/netflix/governator/conditional/NotConditional.java @@ -7,7 +7,7 @@ * * !conditional */ -public class NotConditional extends AbstractConditional { +public class NotConditional implements Conditional { private final Conditional conditional; public NotConditional(Conditional conditional) { diff --git a/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java b/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java deleted file mode 100644 index aeefcee9..00000000 --- a/governator-core/src/main/java/com/netflix/governator/conditional/OrConditional.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.netflix.governator.conditional; - -import com.google.inject.Injector; - -/** - * Conditional equivalent to - * - * first || second - */ -public class OrConditional extends AbstractConditional { - private final Conditional first; - private final Conditional second; - - public OrConditional(AbstractConditional first, Conditional second) { - this.first = first; - this.second = second; - } - - @Override - public boolean matches(Injector injector) { - return first.matches(injector) || second.matches(injector); - } -} diff --git a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java index 0d156c25..a5811420 100644 --- a/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java +++ b/governator-core/src/test/java/com/netflix/governator/conditional/ConditionalTest.java @@ -7,14 +7,14 @@ import com.google.inject.Injector; public class ConditionalTest { - public static class True extends AbstractConditional { + public static class True implements Conditional { @Override public boolean matches(Injector injector) { return true; } } - public static class False extends AbstractConditional { + public static class False implements Conditional { @Override public boolean matches(Injector injector) { return false; @@ -23,23 +23,23 @@ public boolean matches(Injector injector) { @Test public void testOr() { - Assert.assertTrue(new True().or(new True()).matches(null)); - Assert.assertTrue(new True().or(new False()).matches(null)); - Assert.assertTrue(new False().or(new True()).matches(null)); - Assert.assertFalse(new False().or(new False()).matches(null)); + Assert.assertTrue(Conditionals.anyOf(new True(), new True()).matches(null)); + Assert.assertTrue(Conditionals.anyOf(new True(), new False()).matches(null)); + Assert.assertTrue(Conditionals.anyOf(new False(), new True()).matches(null)); + Assert.assertFalse(Conditionals.anyOf(new False(), new False()).matches(null)); } @Test public void testAnd() { - Assert.assertTrue(new True().and(new True()).matches(null)); - Assert.assertFalse(new True().and(new False()).matches(null)); - Assert.assertFalse(new False().and(new True()).matches(null)); - Assert.assertFalse(new False().and(new False()).matches(null)); + Assert.assertTrue(Conditionals.allOf(new True(), new True()).matches(null)); + Assert.assertFalse(Conditionals.allOf(new True(), new False()).matches(null)); + Assert.assertFalse(Conditionals.allOf(new False(), new True()).matches(null)); + Assert.assertFalse(Conditionals.allOf(new False(), new False()).matches(null)); } @Test public void testNot() { - Assert.assertFalse(Conditional.not(new True()).matches(null)); - Assert.assertTrue(Conditional.not(new False()).matches(null)); + Assert.assertFalse(Conditionals.not(new True()).matches(null)); + Assert.assertTrue(Conditionals.not(new False()).matches(null)); } }