Java. Page Factory enhancement. #415

TikhomirovSergey · 2015-04-05T22:05:52Z

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. So if there is a class with constructors like follows:

public PageObject(Search context)

//or

public PageObject(WebElement rootElement)

and there is no necessity to implement customized decorating engines then it is possible to populete its WebElement-fields this way (only one step):

PageObject po = PageFactory.initElements(driver, PageObject,class) //then PageObject.class has 
//the default constructor or constructor with a SearchContext- or WebDriver- parameter


//or 
PageObject po = PageFactory.initElements(element, PageObject,class) //then PageObject.class has 
//the default constructor or constructor with a SearchContext- or WebElement- parameter

//and so on...

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. In both cases I caught IllegalModifierException. Actually I think that both issues are minor but it would be good if whey were fixed.

This change is

opensource21 · 2015-05-29T08:28:58Z

Hi I made some similar change at selophane. How ever the last days I found a use case where it's necessary to have the webdriver. So I changed it back. The background is, that if you create something like a button it would be great if there was a wait till a click on the button is finished. Therefor you need a WebDriver.
You can see my current solution in a pull request

TikhomirovSergey · 2015-05-29T10:01:55Z

@opensource21
There is nothing prevents to use Webdriver if this change will be accepted. Actually it is possible to use WebElement for the subelements populating now:

//excessive actions
WidgetObject wo = new WidgetObject(/*refToElement*/);
DefaultFieldDecorator dfd = new DefaultFieldDecorator(
new DefaultElementLocatorFactory(refToElement)); 
//excessive actions are finished
PageFactory.initElements(dfd, wo);

This should be usefull for cases and apps/services like... Google (just for example). The search form is the page by itself and the widget inside some element at Drive or GMail page. So if it is not necessary to made up and implement users customized decorator then this widget object will be created and populated in one action.

opensource21 · 2015-05-29T10:14:49Z

@TikhomirovSergey Yes you are right! Sorry for the trouble and thanks for the hints.

ddavison · 2015-09-30T15:04:13Z

there appears to be an issue with this travis build.

no ruby changes were detected, yet still failed. @lukeis? any idea

titusfortner · 2015-09-30T15:55:44Z

I'm not sure why it ran in the first place, but it looks like I might have changed something in the specs. I'll take a look.

@findby

- 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.

…page_factory_enhancements Conflicts: java/client/src/org/openqa/selenium/support/pagefactory/internal/LocatingElementHandler.java

Also: - PageFactory: some redundant code was removed

TikhomirovSergey · 2017-10-26T10:04:06Z

@mykola-mokhnach @shs96c
It seems that PR has been actualized.
Could you rewiew this PR?

Also I don't know why tests are not passed

mykola-mokhnach · 2017-10-26T20:13:59Z

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

+          if (!parameterClazz.isAssignableFrom(context.getClass()))
+            continue;
+          c.setAccessible(true);
+          return (T) c.newInstance(context);


pageClassToProxy.cast should address unchecked class cast warning

mykola-mokhnach · 2017-10-26T20:14:24Z

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

-      } catch (NoSuchMethodException e) {
-        return pageClassToProxy.newInstance();
+      Constructor<?>[] availableConstructors = pageClassToProxy.getDeclaredConstructors();
+      for (Constructor<?> c: availableConstructors){


a space is missing

mykola-mokhnach · 2017-10-26T20:14:58Z

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

+
+          Class<?>[] parameterTypes = c.getParameterTypes();
+          if (parameterTypes.length != 1)
+            continue;


it is a good practice to always enclose condition block into curly brackets, even if there is only one operator

mykola-mokhnach · 2017-10-26T20:16:32Z

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

  public Object invoke(Object object, Method method, Object[] objects) throws Throwable {
+
+    if ("toString".equals(method.getName())) {
+      return "Proxy element for: " + locator.toString();


String.format might be a better choice here

- some code style issues were fixed out

TikhomirovSergey · 2017-11-01T20:12:21Z

@mykola-mokhnach @shs96c I made the update. Please look at this once again.

mykola-mokhnach · 2017-11-01T20:29:52Z

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

    Field[] fields = proxyIn.getDeclaredFields();
    for (Field field : fields) {
+      int modifiers = field.getModifiers();
+      if (Modifier.isFinal(modifiers) || Modifier.isStatic(modifiers))


curly brackets would be good here as well

mykola-mokhnach · 2017-11-01T20:31:54Z

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

+            continue;
+          }
+          c.setAccessible(true);
+          return (T) c.newInstance(context);


did pageClassToProxy.cast fail?

Ok. I can improve it later.

- some code style issues were fixed out

CLAassistant · 2019-11-23T17:00:28Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

diemol · 2020-05-19T17:43:16Z

Apologies for the poor handling of this PR.

In general, we are discouraging the use of PageFactory, but in any case, lots of people use, so making these types of changes would break backward compatibility. Closing this PR.

andreastt added Z-awaiting review Archived: use GitHub review assignments C-java Java Bindings labels Apr 5, 2015

TikhomirovSergey mentioned this pull request Aug 26, 2015

PageFactory: Support instantiation of pages that depend on concrete WebDriver implementations #968

Closed

TikhomirovSergey force-pushed the page_factory_enhancements branch from 706caca to 38a1229 Compare August 27, 2015 20:23

ddavison added the A-needs decision TLC needs to discuss and agree label Aug 27, 2015

TikhomirovSergey mentioned this pull request Sep 26, 2015

Change PageFactory so that it support creating page object for part o… #1082

Closed

TikhomirovSergey force-pushed the page_factory_enhancements branch 2 times, most recently from 57b01a0 to 2e144a8 Compare September 30, 2015 14:37

TikhomirovSergey force-pushed the page_factory_enhancements branch from 4a314b8 to c8a8e13 Compare September 30, 2015 16:24

TikhomirovSergey force-pushed the page_factory_enhancements branch from fb266f6 to d079a12 Compare October 5, 2015 14:22

TikhomirovSergey added 2 commits October 24, 2017 23:52

Merge branch 'master' of https://github.com/SeleniumHQ/selenium into …

ecaf002

…page_factory_enhancements Conflicts: java/client/src/org/openqa/selenium/support/pagefactory/internal/LocatingElementHandler.java

Page factory enhancement: FIX of the broken merge.

9c74157

Also: - PageFactory: some redundant code was removed

mykola-mokhnach reviewed Oct 26, 2017

View reviewed changes

Page factory enhancement:

8b83f9a

- some code style issues were fixed out

mykola-mokhnach reviewed Nov 1, 2017

View reviewed changes

Page factory enhancement:

b9620df

- some code style issues were fixed out

diemol closed this May 19, 2020

mykola-mokhnach mentioned this pull request Oct 15, 2021

refactor!: Migrate to Selenium 4 appium/java-client#1531

Merged

4 tasks

Uh oh!

Java. Page Factory enhancement. #415

Java. Page Factory enhancement. #415

Uh oh!

Conversation

TikhomirovSergey commented Apr 5, 2015 • edited by juangj Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

opensource21 commented May 29, 2015

Uh oh!

TikhomirovSergey commented May 29, 2015

Uh oh!

opensource21 commented May 29, 2015

Uh oh!

ddavison commented Sep 30, 2015

Uh oh!

titusfortner commented Sep 30, 2015

Uh oh!

TikhomirovSergey commented Oct 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mykola-mokhnach Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Oct 26, 2017

Choose a reason for hiding this comment

Uh oh!

TikhomirovSergey commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mykola-mokhnach Nov 1, 2017

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Nov 1, 2017

Choose a reason for hiding this comment

Uh oh!

TikhomirovSergey Nov 1, 2017

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Nov 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diemol commented May 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

TikhomirovSergey commented Apr 5, 2015 •

edited by juangj

Loading

TikhomirovSergey commented Oct 26, 2017 •

edited

Loading

TikhomirovSergey commented Nov 1, 2017 •

edited

Loading

CLAassistant commented Nov 23, 2019 •

edited

Loading