From 2becc87128ba4fb13ddd1083cb50777c17c27a5a Mon Sep 17 00:00:00 2001 From: Toilal Date: Fri, 16 Sep 2016 16:04:22 +0200 Subject: [PATCH] Add factories annotations instead of getPriority() and getName() --- README.md | 6 +-- .../AbstractFactoryRegistryImpl.java | 50 +++++++++++++----- .../configuration/AlternativeNames.java | 16 ------ .../DefaultWebDriverFactories.java | 52 +++---------------- .../fluentlenium/configuration/Factory.java | 20 +++---- .../configuration/FactoryName.java | 17 ++++++ .../configuration/FactoryNames.java | 15 ++++++ .../configuration/FactoryPriority.java | 18 +++++++ .../MethodInvocationReflectionFactory.java | 15 ++---- .../ReflectiveCapabilitiesFactory.java | 24 ++++----- .../ReflectiveWebDriverFactory.java | 21 +++----- .../ReflectiveWebDriverFactoryTest.java | 6 +-- .../TestCapabilitiesFactory.java | 11 +--- .../configuration/WebDriversTest.java | 12 +---- 14 files changed, 126 insertions(+), 157 deletions(-) delete mode 100644 fluentlenium-core/src/main/java/org/fluentlenium/configuration/AlternativeNames.java create mode 100644 fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryName.java create mode 100644 fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryNames.java create mode 100644 fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryPriority.java diff --git a/README.md b/README.md index b56aab7b32..be75d68301 100644 --- a/README.md +++ b/README.md @@ -1211,6 +1211,7 @@ A ```WebDriverFactory``` implementation is responsible for creating new instance For instance, to run your tests on [BrowserStack](https://browserstack.com) ```java +@FactoryName("browserstack") public class BrowserStackWebDriverFactory implements WebDriverFactory { @Override public WebDriver newWebDriver() { @@ -1233,11 +1234,6 @@ public class BrowserStackWebDriverFactory implements WebDriverFactory { return new RemoteWebDriver(hubURL, caps); } - - @Override - public String[] getNames() { - return new String[0]; - } } ``` diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AbstractFactoryRegistryImpl.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AbstractFactoryRegistryImpl.java index 3650030fa8..f764f0fdda 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AbstractFactoryRegistryImpl.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AbstractFactoryRegistryImpl.java @@ -4,6 +4,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashMap; @@ -45,7 +46,13 @@ public synchronized T getDefault() { Collections.sort(factories, new Comparator() { @Override public int compare(T o1, T o2) { - return -Integer.compare(o1.getPriority(), o2.getPriority()); + FactoryPriority annotation1 = o1.getClass().getAnnotation(FactoryPriority.class); + int p1 = annotation1 == null ? 0 : annotation1.value(); + + FactoryPriority annotation2 = o2.getClass().getAnnotation(FactoryPriority.class); + int p2 = annotation2 == null ? 0 : annotation2.value(); + + return -Integer.compare(p1, p2); } }); List filteredFactories = new ArrayList<>(); @@ -97,23 +104,42 @@ public synchronized T get(String name) { /** * Register a new factory. *

- * It will also register the factory under names returned by {@link AlternativeNames#getAlternativeNames()} if - * it implements {@link AlternativeNames}. + * It will use {@link FactoryName} value as the default name. + *

+ * It will also register the factory under names returned by {@link FactoryNames#getNames()}} if + * it implements {@link FactoryNames}. * * @param factory factory to register */ public synchronized void register(T factory) { - if (factories.containsKey(factory.getName())) { - throw new ConfigurationException("A factory is already registered with this name: " + - factory.getName() + " (" + factories.get(factory.getName()) + ")"); + FactoryName annotation = factory.getClass().getAnnotation(FactoryName.class); + String annotationName = annotation == null ? null : annotation.value(); + + List names = new ArrayList<>(); + if (annotationName != null) { + names.add(annotationName); + } + if (factory instanceof FactoryNames) { + names.addAll(Arrays.asList(((FactoryNames) factory).getNames())); } - factories.put(factory.getName(), factory); - if (factory instanceof AlternativeNames) { - for (String alternativeName : ((AlternativeNames) factory).getAlternativeNames()) { - if (!factories.containsKey(alternativeName)) { - factories.put(alternativeName, factory); - } + boolean registered = false; + + if (names.size() == 0) { + throw new ConfigurationException("Factory " + factory.getClass().getName() + " has no name defined. Use @FactoryName annotation or implement FactoryNames."); + } + + for (String name : names) { + if (!registered) { + if (factories.containsKey(name)) { + throw new ConfigurationException("A factory is already registered with this name: " + + name + " (" + factories.get(name) + ")"); + } + factories.put(name, factory); + registered = true; + } + if (!factories.containsKey(name)) { + factories.put(name, factory); } } diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AlternativeNames.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AlternativeNames.java deleted file mode 100644 index 05256c977c..0000000000 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/AlternativeNames.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.fluentlenium.configuration; - -/** - * Add alternative names to an object. - *

- * {@link WebDriverFactory} implementations can also implement this interface to be registered in {@link WebDrivers} - * registry with those alternative names. - */ -public interface AlternativeNames { - /** - * Get the alternative names. - * - * @return array of alternative names - */ - String[] getAlternativeNames(); -} diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/DefaultWebDriverFactories.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/DefaultWebDriverFactories.java index c9e06953e9..e78154b14e 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/DefaultWebDriverFactories.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/DefaultWebDriverFactories.java @@ -12,81 +12,53 @@ import java.net.URL; public class DefaultWebDriverFactories { + @FactoryPriority(128) public static class FirefoxWebDriverFactory extends ReflectiveWebDriverFactory { public FirefoxWebDriverFactory() { super("firefox", "org.openqa.selenium.firefox.FirefoxDriver"); } - - @Override - public int getPriority() { - return 128; - } } + @FactoryPriority(127) public static class MarionetteWebDriverFactory extends ReflectiveWebDriverFactory { public MarionetteWebDriverFactory() { super("marionette", "org.openqa.selenium.firefox.MarionetteDriver"); } - - @Override - public int getPriority() { - return 127; - } } + @FactoryPriority(64) public static class ChromeWebDriverFactory extends ReflectiveWebDriverFactory { public ChromeWebDriverFactory() { super("chrome", "org.openqa.selenium.chrome.ChromeDriver"); } - - @Override - public int getPriority() { - return 64; - } } + @FactoryPriority(32) public static class InternetExplorerWebDriverFactory extends ReflectiveWebDriverFactory { public InternetExplorerWebDriverFactory() { super("ie", "org.openqa.selenium.ie.InternetExplorerDriver"); } - - @Override - public int getPriority() { - return 32; - } } + @FactoryPriority(31) public static class EdgeWebDriverFactory extends ReflectiveWebDriverFactory { public EdgeWebDriverFactory() { super("edge", "org.openqa.selenium.edge.EdgeDriver"); } - - @Override - public int getPriority() { - return 31; - } } + @FactoryPriority(16) public static class SafaryWebDriverFactory extends ReflectiveWebDriverFactory { public SafaryWebDriverFactory() { super("safari", "org.openqa.selenium.safari.SafariDriver"); } - - @Override - public int getPriority() { - return 16; - } } + @FactoryPriority(8) public static class PhantomJSWebDriverFactory extends ReflectiveWebDriverFactory { public PhantomJSWebDriverFactory() { super("phantomjs", "org.openqa.selenium.phantomjs.PhantomJSDriver"); } - - @Override - public int getPriority() { - return 8; - } } public static class RemoteWebDriverFactory extends ReflectiveWebDriverFactory { @@ -121,11 +93,6 @@ protected WebDriver newRemoteWebDriver(Object[] args) throws NoSuchMethodExcepti WebDriver webDriver = ReflectionUtils.getConstructor(webDriverClass, URL.class, Capabilities.class).newInstance(args); return new Augmenter().augment(webDriver); } - - @Override - public int getPriority() { - return 0; - } } public static class HtmlUnitWebDriverFactory extends ReflectiveWebDriverFactory { @@ -139,11 +106,6 @@ protected DesiredCapabilities newDefaultCapabilities() { desiredCapabilities.setJavascriptEnabled(true); return desiredCapabilities; } - - @Override - public int getPriority() { - return -128; - } } } diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/Factory.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/Factory.java index 4da8b60c74..06d365c5bb 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/Factory.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/Factory.java @@ -1,19 +1,11 @@ package org.fluentlenium.configuration; +/** + * Marker interface for factories. + * + * @see FactoryPriority + * @see FactoryName + */ public interface Factory { - /** - * Primary name of this factory. - *

- * To register it with alternative name, use {@link AlternativeNames}. - * - * @return Primary name - */ - String getName(); - /** - * Priority of the factory to be grabbed as default Factory. - * - * @return a priority index - */ - int getPriority(); } diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryName.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryName.java new file mode 100644 index 0000000000..a0fe853fc5 --- /dev/null +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryName.java @@ -0,0 +1,17 @@ +package org.fluentlenium.configuration; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Define names for a factory + */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +public @interface FactoryName { + String value(); +} diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryNames.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryNames.java new file mode 100644 index 0000000000..a98dec9f30 --- /dev/null +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryNames.java @@ -0,0 +1,15 @@ +package org.fluentlenium.configuration; + +/** + * Add names to a factory. + *

+ * {@link Factory} implementations can implement this interface to be registered in registry with those names. + */ +public interface FactoryNames { + /** + * Get the names. + * + * @return array of names + */ + String[] getNames(); +} diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryPriority.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryPriority.java new file mode 100644 index 0000000000..256d3494ae --- /dev/null +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/FactoryPriority.java @@ -0,0 +1,18 @@ +package org.fluentlenium.configuration; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + + +/** + * Defines the priority of the factory. + */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +public @interface FactoryPriority { + int value(); +} diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/MethodInvocationReflectionFactory.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/MethodInvocationReflectionFactory.java index 1d759a8c04..41ff2ff8b8 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/MethodInvocationReflectionFactory.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/MethodInvocationReflectionFactory.java @@ -6,7 +6,7 @@ import java.lang.reflect.Method; @IndexIgnore -public class MethodInvocationReflectionFactory implements CapabilitiesFactory, AlternativeNames { +public class MethodInvocationReflectionFactory implements CapabilitiesFactory, FactoryNames { private final Method method; private final Object instance; private final Object[] args; @@ -17,16 +17,6 @@ public MethodInvocationReflectionFactory(Method method, Object instance, Object. this.args = args; } - @Override - public String getName() { - return this.method.getDeclaringClass().getName() + "." + this.method.getName(); - } - - @Override - public int getPriority() { - return 0; - } - @Override public Capabilities newCapabilities() { try { @@ -39,8 +29,9 @@ public Capabilities newCapabilities() { } @Override - public String[] getAlternativeNames() { + public String[] getNames() { return new String[]{ + this.method.getDeclaringClass().getName() + "." + this.method.getName(), this.method.getDeclaringClass().getSimpleName() + "." + this.method.getName(), this.method.getName() }; diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveCapabilitiesFactory.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveCapabilitiesFactory.java index 92b32afbe6..54c77efee6 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveCapabilitiesFactory.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveCapabilitiesFactory.java @@ -6,11 +6,14 @@ import org.openqa.selenium.remote.DesiredCapabilities; import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; /** * A simple {@link CapabilitiesFactory} that create {@link Capabilities} instances using reflection. */ -public class ReflectiveCapabilitiesFactory implements CapabilitiesFactory, AlternativeNames, ReflectiveFactory { +public class ReflectiveCapabilitiesFactory implements CapabilitiesFactory, FactoryNames, ReflectiveFactory { private String name; private Object[] args; private String capabilitiesClassName; @@ -62,21 +65,12 @@ public Capabilities newCapabilities() { } } - @Override - public String getName() { - return name; - } - - @Override - public String[] getAlternativeNames() { + public String[] getNames() { + List names = new ArrayList<>(Arrays.asList(name)); if (capabilitiesClass != null) { - return new String[]{capabilitiesClass.getName(), capabilitiesClass.getSimpleName()}; + names.add(capabilitiesClass.getName()); + names.add(capabilitiesClass.getSimpleName()); } - return new String[0]; - } - - @Override - public int getPriority() { - return 0; + return names.toArray(new String[names.size()]); } } diff --git a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveWebDriverFactory.java b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveWebDriverFactory.java index 2469a09d44..cb84ea7f0b 100644 --- a/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveWebDriverFactory.java +++ b/fluentlenium-core/src/main/java/org/fluentlenium/configuration/ReflectiveWebDriverFactory.java @@ -9,11 +9,12 @@ import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; /** * A simple {@link WebDriverFactory} that create {@link WebDriver} instances using reflection. */ -public class ReflectiveWebDriverFactory implements WebDriverFactory, ReflectiveFactory, AlternativeNames { +public class ReflectiveWebDriverFactory implements WebDriverFactory, ReflectiveFactory, FactoryNames { protected String name; protected Object[] args; protected String webDriverClassName; @@ -85,20 +86,12 @@ protected WebDriver newInstance(Class webDriverClass, Confi } @Override - public String getName() { - return name; - } - - @Override - public String[] getAlternativeNames() { + public String[] getNames() { + List names = new ArrayList<>(Arrays.asList(name)); if (webDriverClass != null) { - return new String[]{webDriverClass.getName(), webDriverClass.getSimpleName()}; + names.add(webDriverClass.getName()); + names.add(webDriverClass.getSimpleName()); } - return new String[0]; - } - - @Override - public int getPriority() { - return 0; + return names.toArray(new String[names.size()]); } } diff --git a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/ReflectiveWebDriverFactoryTest.java b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/ReflectiveWebDriverFactoryTest.java index 7081588280..22b59e8c74 100644 --- a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/ReflectiveWebDriverFactoryTest.java +++ b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/ReflectiveWebDriverFactoryTest.java @@ -135,8 +135,7 @@ public void testHtmlUnitWebDriver() { assertThat(webDriverFactory.isAvailable()).isTrue(); assertThat(webDriverFactory.getWebDriverClass()).isSameAs(HtmlUnitDriver.class); - assertThat(webDriverFactory.getName()).isEqualTo("htmlunit"); - assertThat(webDriverFactory.getAlternativeNames()).containsExactly(HtmlUnitDriver.class.getName(), HtmlUnitDriver.class.getSimpleName()); + assertThat(webDriverFactory.getNames()).containsExactly("htmlunit", HtmlUnitDriver.class.getName(), HtmlUnitDriver.class.getSimpleName()); WebDriver webDriver = webDriverFactory.newWebDriver(null, null); try { @@ -153,8 +152,7 @@ public void testHtmlUnitWebDriverCapabilities() { assertThat(webDriverFactory.isAvailable()).isTrue(); assertThat(webDriverFactory.getWebDriverClass()).isSameAs(HtmlUnitDriver.class); - assertThat(webDriverFactory.getName()).isEqualTo("htmlunit"); - assertThat(webDriverFactory.getAlternativeNames()).containsExactly(HtmlUnitDriver.class.getName(), HtmlUnitDriver.class.getSimpleName()); + assertThat(webDriverFactory.getNames()).containsExactly("htmlunit", HtmlUnitDriver.class.getName(), HtmlUnitDriver.class.getSimpleName()); DesiredCapabilities desiredCapabilities = new DesiredCapabilities(); desiredCapabilities.setJavascriptEnabled(false); diff --git a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/TestCapabilitiesFactory.java b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/TestCapabilitiesFactory.java index e9aa540ee2..3ccebfe4fb 100644 --- a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/TestCapabilitiesFactory.java +++ b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/TestCapabilitiesFactory.java @@ -2,17 +2,8 @@ import org.openqa.selenium.Capabilities; +@FactoryName("test-capabilities-factory") public class TestCapabilitiesFactory implements CapabilitiesFactory { - @Override - public String getName() { - return "test-capabilities-factory"; - } - - @Override - public int getPriority() { - return 0; - } - @Override public Capabilities newCapabilities() { return new TestCapabilities(); diff --git a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/WebDriversTest.java b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/WebDriversTest.java index 4b4ac1ad4c..b9bda6b3e6 100644 --- a/fluentlenium-core/src/test/java/org/fluentlenium/configuration/WebDriversTest.java +++ b/fluentlenium-core/src/test/java/org/fluentlenium/configuration/WebDriversTest.java @@ -21,21 +21,13 @@ public static class CustomWebDriver extends HtmlUnitDriver { } + @FactoryPriority(2048) + @FactoryName("another") public static class AnotherFactory implements WebDriverFactory { @Override public WebDriver newWebDriver(Capabilities capabilities, ConfigurationProperties configuration) { return new CustomWebDriver(); } - - @Override - public String getName() { - return "another"; - } - - @Override - public int getPriority() { - return 2048; - } } @Before