AlexeiBarantsev: Use same annotation @findby for both elements and li…

…sts of elements

r14016

java/client/src/org/openqa/selenium/support/FindBy.java

@@ -24,7 +24,7 @@
 
 /**
  * Used to mark a field on a Page Object to indicate an alternative mechanism for locating the
- * element. Used in conjunction with
+ * element or a list of elements. Used in conjunction with
  * {@link org.openqa.selenium.support.PageFactory#proxyElement(org.openqa.selenium.WebDriver, Object, java.lang.reflect.Field)}
  * this allows users to quickly and easily create PageObjects.
  * 
@@ -39,6 +39,13 @@
  * @FindBy(id = "foobar") WebElement foobar;
  * @FindBy(how = How.ID, using = "foobar") WebElement foobar;
  * </pre>
+ * 
+ * and these two annotations point to the same list of elements:
+ * 
+ * <pre class="code">
+ * @FindBy(tagName = "a") List<WebElement> links;
+ * @FindBy(how = How.TAG_NAME, using = "a") WebElement links;
+ * </pre>
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.FIELD)

java/client/src/org/openqa/selenium/support/build.desc

@@ -11,7 +11,6 @@ java_library(name = "page-factory",
   srcs = [
     "ByIdOrName.java",
     "CacheLookup.java",
-    "FindAllBy.java",
     "FindBy.java",
     "FindBys.java",
     "How.java",

java/client/src/org/openqa/selenium/support/pagefactory/Annotations.java

@@ -20,16 +20,11 @@
 import org.openqa.selenium.By;
 import org.openqa.selenium.support.ByIdOrName;
 import org.openqa.selenium.support.CacheLookup;
-import org.openqa.selenium.support.FindAllBy;
 import org.openqa.selenium.support.FindBy;
 import org.openqa.selenium.support.FindBys;
 import org.openqa.selenium.support.How;
 
 import java.lang.reflect.Field;
-import java.lang.reflect.InvocationHandler;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -60,11 +55,6 @@ public By buildBy() {
       ans = buildByFromFindBy(findBy);
     }
 
-    FindAllBy findAllBy = field.getAnnotation(FindAllBy.class);
-    if (ans == null && findAllBy != null) {
-      ans = buildByFromFindBy(convert(findAllBy));
-    }
-
     if (ans == null) {
       ans = buildByFromDefault();
     }
@@ -76,26 +66,6 @@ public By buildBy() {
     return ans;
   }
 
-  private FindBy convert(final FindAllBy findAllBy) {
-    return (FindBy) Proxy.newProxyInstance(this.getClass().getClassLoader(),
-        new Class[] {FindBy.class}, new InvocationHandler() {
-      public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-        if ("annotationType".equals(method.getName())) {
-          return FindAllBy.class;
-        }
-
-        Method m2 = FindAllBy.class.getMethod(method.getName());
-
-        try {
-          return m2.invoke(findAllBy, args);
-        } catch (InvocationTargetException e) {
-          // Unwrap the underlying exception
-          throw e.getCause();
-        }
-      }
-    });
-  }
-
   protected By buildByFromDefault() {
     return new ByIdOrName(field.getName());
   }
@@ -192,18 +162,11 @@ protected By buildByFromShortFindBy(FindBy findBy) {
   }
 
   private void assertValidAnnotations() {
-    int count = 0;
-    if (field.getAnnotation(FindBys.class) != null) {
-      count += 1;
-    }
-    if (field.getAnnotation(FindBy.class) != null) {
-      count += 1;
-    }
-    if (field.getAnnotation(FindAllBy.class) != null) {
-      count += 1;
-    }
-    if (count > 1) {
-      throw new IllegalArgumentException("You may use only one of '@FindBy', '@FindBys' and '@FindAllBy' annotations");
+    FindBys findBys = field.getAnnotation(FindBys.class);
+    FindBy findBy = field.getAnnotation(FindBy.class);
+    if (findBys != null && findBy != null) {
+      throw new IllegalArgumentException("If you use a '@FindBys' annotation, "
+          + "you must not also use a '@FindBy' annotation");
     }
   }
 
@@ -240,4 +203,4 @@ private void assertValidFindBy(FindBy findBy) {
               finders.size(), finders.toString()));
     }
   }
-}
+}

java/client/src/org/openqa/selenium/support/pagefactory/DefaultElementLocator.java

@@ -27,15 +27,14 @@
 /**
  * The default element locator, which will lazily locate an element or an element list on a page. This class is
  * designed for use with the {@link org.openqa.selenium.support.PageFactory} and understands the
- * annotations {@link org.openqa.selenium.support.FindBy}, {@link org.openqa.selenium.support.FindAllBy} and
- * {@link org.openqa.selenium.support.CacheLookup}.
+ * annotations {@link org.openqa.selenium.support.FindBy} and {@link org.openqa.selenium.support.CacheLookup}.
  */
 public class DefaultElementLocator implements ElementLocator {
   private final WebDriver driver;
-  private final boolean cacheElement;
+  private final boolean shouldCache;
   private final By by;
   private WebElement cachedElement;
-  private List<WebElement> cachedElements;
+  private List<WebElement> cachedElementList;
 
   /**
    * Creates a new element locator.
@@ -46,20 +45,20 @@ public class DefaultElementLocator implements ElementLocator {
   public DefaultElementLocator(WebDriver driver, Field field) {
     this.driver = driver;
     Annotations annotations = new Annotations(field);
-    cacheElement = annotations.isLookupCached();
+    shouldCache = annotations.isLookupCached();
     by = annotations.buildBy();
   }
 
   /**
    * Find the element.
    */
   public WebElement findElement() {
-    if (cachedElement != null && cacheElement) {
+    if (cachedElement != null && shouldCache) {
       return cachedElement;
     }
 
     WebElement element = driver.findElement(by);
-    if (cacheElement) {
+    if (shouldCache) {
       cachedElement = element;
     }
 
@@ -70,13 +69,13 @@ public WebElement findElement() {
    * Find the element list.
    */
   public List<WebElement> findElements() {
-    if (cachedElements != null && cacheElement) {
-      return cachedElements;
+    if (cachedElementList != null && shouldCache) {
+      return cachedElementList;
     }
 
     List<WebElement> elements = driver.findElements(by);
-    if (cacheElement) {
-      cachedElements = elements;
+    if (shouldCache) {
+      cachedElementList = elements;
     }
 
     return elements;

java/client/src/org/openqa/selenium/support/pagefactory/DefaultFieldDecorator.java

@@ -69,6 +69,7 @@ protected WebElement proxyForLocator(ClassLoader loader, ElementLocator locator)
     return proxy;
   }
 
+  @SuppressWarnings("unchecked")
   protected List<WebElement> proxyForListLocator(ClassLoader loader, ElementLocator locator) {
     InvocationHandler handler = new LocatingElementListHandler(locator);
 

java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java

@@ -28,7 +28,6 @@
 import org.openqa.selenium.MockTestBase;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ByIdOrName;
-import org.openqa.selenium.support.FindAllBy;
 import org.openqa.selenium.support.FindBy;
 import org.openqa.selenium.support.FindBys;
 import org.openqa.selenium.support.How;
@@ -42,13 +41,13 @@ public class AnnotationsTest extends MockTestBase {
   @FindBy(how = How.NAME, using = "cheese")
   public WebElement longFindBy_field;
 
-  @FindAllBy(how = How.NAME, using = "cheese")
+  @FindBy(how = How.NAME, using = "cheese")
   public List<WebElement> longFindAllBy_field;
 
   @FindBy(name = "cheese")
   public WebElement shortFindBy_field;
 
-  @FindAllBy(name = "cheese")
+  @FindBy(name = "cheese")
   public List<WebElement> shortFindAllBy_field;
 
   @FindBys({@FindBy(how = How.NAME, using = "cheese"),
@@ -63,7 +62,7 @@ public class AnnotationsTest extends MockTestBase {
   @FindBy(id = "cheese", name = "fruit")
   public WebElement findByMultipleHows_field;
 
-  @FindAllBy(id = "cheese", name = "fruit")
+  @FindBy(id = "cheese", name = "fruit")
   public List<WebElement> findAllByMultipleHows_field;
 
   @FindBys({@FindBy(id = "cheese", name = "fruit"),

java/client/test/org/openqa/selenium/support/pagefactory/DefaultElementLocatorTest.java

@@ -26,7 +26,6 @@
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ByIdOrName;
 import org.openqa.selenium.support.CacheLookup;
-import org.openqa.selenium.support.FindAllBy;
 import org.openqa.selenium.support.FindBy;
 import org.openqa.selenium.support.How;
 
@@ -228,7 +227,7 @@ private static class Page {
     private WebElement byId;
 
     @SuppressWarnings("unused")
-    @FindAllBy(how = How.ID, using = "foo")
+    @FindBy(how = How.ID, using = "foo")
     private List<WebElement> listById;
   }
 }

java/client/test/org/openqa/selenium/support/pagefactory/UsingPageFactoryTest.java

@@ -25,13 +25,11 @@
 
 import java.util.List;
 
-import org.openqa.selenium.By;
 import org.openqa.selenium.JUnit4TestBase;
 import org.openqa.selenium.JavascriptEnabled;
 import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.CacheLookup;
-import org.openqa.selenium.support.FindAllBy;
 import org.openqa.selenium.support.FindBy;
 import org.openqa.selenium.support.PageFactory;
 
@@ -70,7 +68,7 @@ public static class Page {
     @FindBy(name = "someForm")
     WebElement formElement;
 
-    @FindAllBy(tagName = "a")
+    @FindBy(tagName = "a")
     @CacheLookup
     List<WebElement> links;
   }