Java. Page Factory enhancement.

- Ability to use not only Webdriver to populate page object fields via
method
public static <T> T initElements(SearchContext context, Class<T>
pageClassToProxy).
This change was done in order to simplify page object instantiation and
population when there is the necessity to implement something that describes
widget or repeatable element set nested in a root element.
- the additional checking of field modifiers before it will
be populated. Field that is going to be filled should not  be static and
final. I found these issues some time ago. I tried to implement page
object with final fields that weren't annotated by @findby's. Also I found
that it tries to set proxy-WebElement as a value of a static WebElement-field.
Actually I think that both issues are minor but it would be good if whey
were fixed.

Author: TikhomirovSergey

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

@@ -17,7 +17,7 @@
 
 package org.openqa.selenium.support;
 
-import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.SearchContext;
 import org.openqa.selenium.support.pagefactory.DefaultElementLocatorFactory;
 import org.openqa.selenium.support.pagefactory.DefaultFieldDecorator;
 import org.openqa.selenium.support.pagefactory.ElementLocatorFactory;
@@ -26,6 +26,7 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Modifier;
 
 
 /**
@@ -52,30 +53,31 @@ public class PageFactory {
    * which takes a WebDriver instance as its only argument or falling back on a no-arg constructor.
    * An exception will be thrown if the class cannot be instantiated.
    *
-   * @param driver           The driver that will be used to look up the elements
+   * @param context          The org.openqa.selenium.SearchContext instance that will be used to
+   *                         look up the elements
    * @param pageClassToProxy A class which will be initialised.
    * @return An instantiated instance of the class with WebElement and List<WebElement>
    * fields proxied
    * @see FindBy
    * @see CacheLookup
    */
-  public static <T> T initElements(WebDriver driver, Class<T> pageClassToProxy) {
-    T page = instantiatePage(driver, pageClassToProxy);
-    initElements(driver, page);
+  public static <T> T initElements(SearchContext context, Class<T> pageClassToProxy) {
+    T page = instantiatePage(context, pageClassToProxy);
+    initElements(context, page);
     return page;
   }
 
   /**
-   * As {@link org.openqa.selenium.support.PageFactory#initElements(org.openqa.selenium.WebDriver,
+   * As {@link org.openqa.selenium.support.PageFactory#initElements(org.openqa.selenium.SearchContext,
    * Class)} but will only replace the fields of an already instantiated Page Object.
    *
-   * @param driver The driver that will be used to look up the elements
+   * @param context The org.openqa.selenium.SearchContext instance that will be used to look up the elements
    * @param page   The object with WebElement and List&lt;WebElement&gt; fields that
    *               should be proxied.
    */
-  public static void initElements(WebDriver driver, Object page) {
-    final WebDriver driverRef = driver;
-    initElements(new DefaultElementLocatorFactory(driverRef), page);
+  public static void initElements(SearchContext context, Object page) {
+    final SearchContext searchContextRef = context;
+    initElements(new DefaultElementLocatorFactory(searchContextRef), page);
   }
 
   /**
@@ -109,6 +111,10 @@ public static void initElements(FieldDecorator decorator, Object page) {
   private static void proxyFields(FieldDecorator decorator, Object page, Class<?> proxyIn) {
     Field[] fields = proxyIn.getDeclaredFields();
     for (Field field : fields) {
+      int modifiers = field.getModifiers();
+      if (Modifier.isFinal(modifiers) || Modifier.isStatic(modifiers))
+        continue;
+      
       Object value = decorator.decorate(page.getClass().getClassLoader(), field);
       if (value != null) {
         try {
@@ -121,14 +127,24 @@ private static void proxyFields(FieldDecorator decorator, Object page, Class<?>
     }
   }
 
-  private static <T> T instantiatePage(WebDriver driver, Class<T> pageClassToProxy) {
+  @SuppressWarnings("unchecked")
+  private static <T> T instantiatePage(SearchContext context, Class<T> pageClassToProxy) {
     try {
-      try {
-        Constructor<T> constructor = pageClassToProxy.getConstructor(WebDriver.class);
-        return constructor.newInstance(driver);
-      } catch (NoSuchMethodException e) {
-        return pageClassToProxy.newInstance();
+      Constructor<?>[] availableConstructors = pageClassToProxy.getDeclaredConstructors();
+      for (Constructor<?> c: availableConstructors){
+
+          Class<?>[] parameterTypes = c.getParameterTypes();
+          if (parameterTypes.length != 1)
+            continue;
+
+          Class<?> parameterClazz = parameterTypes[0];
+          if (!parameterClazz.isAssignableFrom(context.getClass()))
+            continue;
+          c.setAccessible(true);
+          return (T) c.newInstance(context);
       }
+
+      return pageClassToProxy.newInstance();
     } catch (InstantiationException e) {
       throw new RuntimeException(e);
     } catch (IllegalAccessException e) {

java/client/src/org/openqa/selenium/support/pagefactory/internal/LocatingElementHandler.java

@@ -32,14 +32,16 @@ public LocatingElementHandler(ElementLocator locator) {
   }
 
   public Object invoke(Object object, Method method, Object[] objects) throws Throwable {
+        
+    if ("toString".equals(method.getName())) {
+      return "Proxy element for: " + locator.toString();
+    }
+    
     WebElement element;
     try {
       element = locator.findElement();
     } catch (NoSuchElementException e) {
-      if ("toString".equals(method.getName())) {
-        return "Proxy element for: " + locator.toString();
-      }
-      else throw e;
+      throw e;
     }
 
     if ("getWrappedElement".equals(method.getName())) {

java/client/src/org/openqa/selenium/support/pagefactory/internal/LocatingElementListHandler.java

@@ -32,6 +32,10 @@ public LocatingElementListHandler(ElementLocator locator) {
   }
 
   public Object invoke(Object object, Method method, Object[] objects) throws Throwable {
+    if ("toString".equals(method.getName())) {
+      return "Proxy list of elements for: " + locator.toString();
+    }
+    
     List<WebElement> elements = locator.findElements();
 
     try {

java/client/test/org/openqa/selenium/support/PageFactoryTest.java

@@ -32,18 +32,23 @@
 import org.openqa.selenium.TimeoutException;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
+import org.openqa.selenium.remote.RemoteWebElement;
 import org.openqa.selenium.support.pagefactory.FieldDecorator;
 import org.openqa.selenium.support.ui.ExpectedConditions;
 import org.openqa.selenium.support.ui.TickingClock;
 import org.openqa.selenium.support.ui.Wait;
 import org.openqa.selenium.support.ui.WebDriverWait;
+import org.testng.Assert;
 
 import java.lang.reflect.Field;
 import java.util.List;
 
 public class PageFactoryTest {
 
   private WebDriver driver;
+  
+  @FindBy 
+  private WebElement proxiedField;
 
   @Test
   public void shouldProxyElementsInAnInstantiatedPage() {
@@ -66,6 +71,62 @@ public void shouldInsertProxiesForPublicWebElements() {
     assertThat(page.list, is(notNullValue()));
   }
 
+  @Test
+  public void shouldInsertProxiesForPublicWebElements2() {
+    RemoteWebElement e = null;
+    PublicPage page = PageFactory.initElements(e , PublicPage.class);
+
+    assertThat(page.q, is(notNullValue()));
+    assertThat(page.list, is(notNullValue()));
+  }
+
+  @Test
+  public void shouldInsertProxiesForPublicWebElementsWhenClassHasConstructorWithWebElementParameter() {
+    RemoteWebElement e = mock(RemoteWebElement.class);
+    NestedWidget page = PageFactory.initElements(e , NestedWidget.class);
+
+    assertThat(page.q, is(notNullValue()));
+    assertThat(page.list, is(notNullValue()));
+  }
+  
+  @Test
+  public void shouldInsertProxiesForPublicWebElementsWhenProxyOfElementIsPassedThroughConsctructor() {
+    WebDriver d = mock(WebDriver.class);
+    PageFactory.initElements(d, this);
+    
+    try{
+      NestedWidget page = PageFactory.initElements(proxiedField , NestedWidget.class);
+      assertThat(page.q, is(notNullValue()));
+      assertThat(page.list, is(notNullValue()));
+    }
+    finally{
+      proxiedField = null;
+    }
+  }
+
+  @Test
+  public void shouldNotInstantiatePageObjectWhenThereIsNoRelevantConstructor() {
+    try{
+    WebDriver d = mock(WebDriver.class);
+    @SuppressWarnings("unused")
+    NestedWidget page = PageFactory.initElements(d , NestedWidget.class);
+    fail("Should not instantiate class because it has no relevant constructors");
+    }
+    catch (Exception e){
+      Assert.assertEquals(e.getClass(), RuntimeException.class);
+      Assert.assertEquals(e.getCause().getClass(), InstantiationException.class);
+    }
+  }
+
+  @Test
+  public void shouldNotInsertProxiesForPublicStaticAndFinalFields() {
+    PageObjectWithStaticAndFinalFields p = new PageObjectWithStaticAndFinalFields();
+    PageFactory.initElements(driver , p);
+
+    assertThat(PageObjectWithStaticAndFinalFields.staticField, is(nullValue()));
+    assertThat(p.finalField, is(equalTo(p.finalField)));
+  }
+
   @Test
   public void shouldProxyElementsFromParentClassesToo() {
     ChildPage page = new ChildPage();
@@ -210,6 +271,28 @@ public static class PublicPage {
     public WebElement rendered;
   }
 
+  public static class PageObjectWithStaticAndFinalFields{
+    @FindBy(name = "q")
+    public static WebElement staticField;
+
+    @FindBy(name = "q")
+    public final WebElement finalField = mock(WebElement.class);
+  }
+
+  public static class NestedWidget {
+
+    @FindBy(name = "q")
+    public WebElement q;
+
+    @FindBy(name = "q")
+    public List<WebElement> list;
+
+    public WebElement rendered;
+
+    NestedWidget(WebElement e){
+    }
+  }
+
   public static class ChildPage extends PublicPage {
 
     public WebElement submit;