From dae0f414e866024ccfbc88fca41a6adf2c28b7b2 Mon Sep 17 00:00:00 2001 From: Paul Hammant Date: Wed, 15 Mar 2017 23:02:17 -0400 Subject: [PATCH 1/2] introduce backwards compatible abstraction for @FindBy PageFactory capability --- .../support/AbstractFindByBuilder.java | 133 ++++++++++++++++ .../src/org/openqa/selenium/support/BUCK | 2 + .../org/openqa/selenium/support/FindAll.java | 22 +++ .../org/openqa/selenium/support/FindBy.java | 21 +++ .../org/openqa/selenium/support/FindBys.java | 20 +++ .../selenium/support/PageFactoryFinder.java | 9 ++ .../pagefactory/AbstractAnnotations.java | 143 ------------------ .../support/pagefactory/Annotations.java | 30 ++-- .../support/pagefactory/AnnotationsTest.java | 41 +++++ 9 files changed, 266 insertions(+), 155 deletions(-) create mode 100644 java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java create mode 100644 java/client/src/org/openqa/selenium/support/PageFactoryFinder.java diff --git a/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java b/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java new file mode 100644 index 0000000000000..628c5a7124999 --- /dev/null +++ b/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java @@ -0,0 +1,133 @@ +package org.openqa.selenium.support; + +import org.openqa.selenium.By; + +import java.util.HashSet; +import java.util.Set; + +/** + * Created by paul on 3/8/17. + */ +public abstract class AbstractFindByBuilder { + + public abstract By buildIt(Object annotation); + + protected By buildByFromFindBy(FindBy findBy) { + assertValidFindBy(findBy); + + By ans = buildByFromShortFindBy(findBy); + if (ans == null) { + ans = buildByFromLongFindBy(findBy); + } + + return ans; + } + + protected By buildByFromShortFindBy(FindBy findBy) { + if (!"".equals(findBy.className())) + return By.className(findBy.className()); + + if (!"".equals(findBy.css())) + return By.cssSelector(findBy.css()); + + if (!"".equals(findBy.id())) + return By.id(findBy.id()); + + if (!"".equals(findBy.linkText())) + return By.linkText(findBy.linkText()); + + if (!"".equals(findBy.name())) + return By.name(findBy.name()); + + if (!"".equals(findBy.partialLinkText())) + return By.partialLinkText(findBy.partialLinkText()); + + if (!"".equals(findBy.tagName())) + return By.tagName(findBy.tagName()); + + if (!"".equals(findBy.xpath())) + return By.xpath(findBy.xpath()); + + // Fall through + return null; + } + protected By buildByFromLongFindBy(FindBy findBy) { + How how = findBy.how(); + String using = findBy.using(); + + switch (how) { + case CLASS_NAME: + return By.className(using); + + case CSS: + return By.cssSelector(using); + + case ID: + case UNSET: + return By.id(using); + + case ID_OR_NAME: + return new ByIdOrName(using); + + case LINK_TEXT: + return By.linkText(using); + + case NAME: + return By.name(using); + + case PARTIAL_LINK_TEXT: + return By.partialLinkText(using); + + case TAG_NAME: + return By.tagName(using); + + case XPATH: + return By.xpath(using); + + default: + // Note that this shouldn't happen (eg, the above matches all + // possible values for the How enum) + throw new IllegalArgumentException("Cannot determine how to locate element "); + } + } + protected void assertValidFindBys(FindBys findBys) { + for (FindBy findBy : findBys.value()) { + assertValidFindBy(findBy); + } + } + protected void assertValidFindBy(FindBy findBy) { + if (findBy.how() != null) { + if (findBy.using() == null) { + throw new IllegalArgumentException( + "If you set the 'how' property, you must also set 'using'"); + } + } + + Set finders = new HashSet<>(); + if (!"".equals(findBy.using())) finders.add("how: " + findBy.using()); + if (!"".equals(findBy.className())) finders.add("class name:" + findBy.className()); + if (!"".equals(findBy.css())) finders.add("css:" + findBy.css()); + if (!"".equals(findBy.id())) finders.add("id: " + findBy.id()); + if (!"".equals(findBy.linkText())) finders.add("link text: " + findBy.linkText()); + if (!"".equals(findBy.name())) finders.add("name: " + findBy.name()); + if (!"".equals(findBy.partialLinkText())) + finders.add("partial link text: " + findBy.partialLinkText()); + if (!"".equals(findBy.tagName())) finders.add("tag name: " + findBy.tagName()); + if (!"".equals(findBy.xpath())) finders.add("xpath: " + findBy.xpath()); + + // A zero count is okay: it means to look by name or id. + if (finders.size() > 1) { + throw new IllegalArgumentException( + String.format("You must specify at most one location strategy. Number found: %d (%s)", + finders.size(), finders.toString())); + } + } + + protected void assertValidFindAll(FindAll findBys) { + for (FindBy findBy : findBys.value()) { + assertValidFindBy(findBy); + } + } + + +} diff --git a/java/client/src/org/openqa/selenium/support/BUCK b/java/client/src/org/openqa/selenium/support/BUCK index 0b14225e36f09..3467f7c18b8f2 100644 --- a/java/client/src/org/openqa/selenium/support/BUCK +++ b/java/client/src/org/openqa/selenium/support/BUCK @@ -23,6 +23,7 @@ java_library(name = 'support', java_library(name = 'page-factory', srcs = [ + 'AbstractFindByBuilder.java', 'ByIdOrName.java', 'CacheLookup.java', 'FindAll.java', @@ -30,6 +31,7 @@ java_library(name = 'page-factory', 'FindBys.java', 'How.java', 'PageFactory.java', + 'PageFactoryFinder.java', ] + glob(['pagefactory/*.java', 'pagefactory/internal/*.java',]), deps = [ '//java/client/src/org/openqa/selenium:selenium', diff --git a/java/client/src/org/openqa/selenium/support/FindAll.java b/java/client/src/org/openqa/selenium/support/FindAll.java index c5cdd52716cd5..e6f26aad1f890 100644 --- a/java/client/src/org/openqa/selenium/support/FindAll.java +++ b/java/client/src/org/openqa/selenium/support/FindAll.java @@ -17,6 +17,10 @@ package org.openqa.selenium.support; +import org.openqa.selenium.By; +import org.openqa.selenium.support.pagefactory.ByAll; +import org.openqa.selenium.support.pagefactory.ByChained; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -38,6 +42,24 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) +@PageFactoryFinder(FindAll.FindByBuilder.class) public @interface FindAll { FindBy[] value(); + + public static class FindByBuilder extends AbstractFindByBuilder { + public By buildIt(Object annotation) { + FindAll findBys = (FindAll) annotation; + assertValidFindAll(findBys); + + FindBy[] findByArray = findBys.value(); + By[] byArray = new By[findByArray.length]; + for (int i = 0; i < findByArray.length; i++) { + byArray[i] = buildByFromFindBy(findByArray[i]); + } + + return new ByAll(byArray); + } + + } + } diff --git a/java/client/src/org/openqa/selenium/support/FindBy.java b/java/client/src/org/openqa/selenium/support/FindBy.java index 75e9520b7c8d7..18baceac47ef9 100644 --- a/java/client/src/org/openqa/selenium/support/FindBy.java +++ b/java/client/src/org/openqa/selenium/support/FindBy.java @@ -17,6 +17,9 @@ package org.openqa.selenium.support; +import org.openqa.selenium.By; +import org.openqa.selenium.support.pagefactory.ByChained; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -51,6 +54,7 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) +@PageFactoryFinder(FindBy.FindByBuilder.class) public @interface FindBy { How how() default How.UNSET; @@ -71,4 +75,21 @@ String partialLinkText() default ""; String xpath() default ""; + + public static class FindByBuilder extends AbstractFindByBuilder { + public By buildIt(Object annotation) { + FindBy findBy = (FindBy) annotation; + assertValidFindBy(findBy); + + By ans = buildByFromShortFindBy(findBy); + if (ans == null) { + ans = buildByFromLongFindBy(findBy); + } + + return ans; + + } + + } + } diff --git a/java/client/src/org/openqa/selenium/support/FindBys.java b/java/client/src/org/openqa/selenium/support/FindBys.java index eddfe66c57397..17f0b830d6dfc 100644 --- a/java/client/src/org/openqa/selenium/support/FindBys.java +++ b/java/client/src/org/openqa/selenium/support/FindBys.java @@ -17,6 +17,9 @@ package org.openqa.selenium.support; +import org.openqa.selenium.By; +import org.openqa.selenium.support.pagefactory.ByChained; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -37,6 +40,23 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) +@PageFactoryFinder(FindBys.FindByBuilder.class) public @interface FindBys { FindBy[] value(); + + public static class FindByBuilder extends AbstractFindByBuilder { + public By buildIt(Object annotation) { + FindBys findBys = (FindBys) annotation; + assertValidFindBys(findBys); + + FindBy[] findByArray = findBys.value(); + By[] byArray = new By[findByArray.length]; + for (int i = 0; i < findByArray.length; i++) { + byArray[i] = buildByFromFindBy(findByArray[i]); + } + + return new ByChained(byArray); + } + + } } diff --git a/java/client/src/org/openqa/selenium/support/PageFactoryFinder.java b/java/client/src/org/openqa/selenium/support/PageFactoryFinder.java new file mode 100644 index 0000000000000..3503c9aea5f6a --- /dev/null +++ b/java/client/src/org/openqa/selenium/support/PageFactoryFinder.java @@ -0,0 +1,9 @@ +package org.openqa.selenium.support; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +public @interface PageFactoryFinder { + Class value(); +} diff --git a/java/client/src/org/openqa/selenium/support/pagefactory/AbstractAnnotations.java b/java/client/src/org/openqa/selenium/support/pagefactory/AbstractAnnotations.java index c2f07aeba6c2c..143c184f7d9ed 100644 --- a/java/client/src/org/openqa/selenium/support/pagefactory/AbstractAnnotations.java +++ b/java/client/src/org/openqa/selenium/support/pagefactory/AbstractAnnotations.java @@ -51,147 +51,4 @@ public abstract class AbstractAnnotations { */ public abstract boolean isLookupCached(); - protected By buildByFromFindBys(FindBys findBys) { - assertValidFindBys(findBys); - - FindBy[] findByArray = findBys.value(); - By[] byArray = new By[findByArray.length]; - for (int i = 0; i < findByArray.length; i++) { - byArray[i] = buildByFromFindBy(findByArray[i]); - } - - return new ByChained(byArray); - } - - protected By buildBysFromFindByOneOf(FindAll findBys) { - assertValidFindAll(findBys); - - FindBy[] findByArray = findBys.value(); - By[] byArray = new By[findByArray.length]; - for (int i = 0; i < findByArray.length; i++) { - byArray[i] = buildByFromFindBy(findByArray[i]); - } - - return new ByAll(byArray); - } - - protected By buildByFromFindBy(FindBy findBy) { - assertValidFindBy(findBy); - - By ans = buildByFromShortFindBy(findBy); - if (ans == null) { - ans = buildByFromLongFindBy(findBy); - } - - return ans; - } - - protected By buildByFromLongFindBy(FindBy findBy) { - How how = findBy.how(); - String using = findBy.using(); - - switch (how) { - case CLASS_NAME: - return By.className(using); - - case CSS: - return By.cssSelector(using); - - case ID: - case UNSET: - return By.id(using); - - case ID_OR_NAME: - return new ByIdOrName(using); - - case LINK_TEXT: - return By.linkText(using); - - case NAME: - return By.name(using); - - case PARTIAL_LINK_TEXT: - return By.partialLinkText(using); - - case TAG_NAME: - return By.tagName(using); - - case XPATH: - return By.xpath(using); - - default: - // Note that this shouldn't happen (eg, the above matches all - // possible values for the How enum) - throw new IllegalArgumentException("Cannot determine how to locate element "); - } - } - - protected By buildByFromShortFindBy(FindBy findBy) { - if (!"".equals(findBy.className())) - return By.className(findBy.className()); - - if (!"".equals(findBy.css())) - return By.cssSelector(findBy.css()); - - if (!"".equals(findBy.id())) - return By.id(findBy.id()); - - if (!"".equals(findBy.linkText())) - return By.linkText(findBy.linkText()); - - if (!"".equals(findBy.name())) - return By.name(findBy.name()); - - if (!"".equals(findBy.partialLinkText())) - return By.partialLinkText(findBy.partialLinkText()); - - if (!"".equals(findBy.tagName())) - return By.tagName(findBy.tagName()); - - if (!"".equals(findBy.xpath())) - return By.xpath(findBy.xpath()); - - // Fall through - return null; - } - - private void assertValidFindBys(FindBys findBys) { - for (FindBy findBy : findBys.value()) { - assertValidFindBy(findBy); - } - } - - private void assertValidFindAll(FindAll findBys) { - for (FindBy findBy : findBys.value()) { - assertValidFindBy(findBy); - } - } - - private void assertValidFindBy(FindBy findBy) { - if (findBy.how() != null) { - if (findBy.using() == null) { - throw new IllegalArgumentException( - "If you set the 'how' property, you must also set 'using'"); - } - } - - Set finders = new HashSet<>(); - if (!"".equals(findBy.using())) finders.add("how: " + findBy.using()); - if (!"".equals(findBy.className())) finders.add("class name:" + findBy.className()); - if (!"".equals(findBy.css())) finders.add("css:" + findBy.css()); - if (!"".equals(findBy.id())) finders.add("id: " + findBy.id()); - if (!"".equals(findBy.linkText())) finders.add("link text: " + findBy.linkText()); - if (!"".equals(findBy.name())) finders.add("name: " + findBy.name()); - if (!"".equals(findBy.partialLinkText())) - finders.add("partial link text: " + findBy.partialLinkText()); - if (!"".equals(findBy.tagName())) finders.add("tag name: " + findBy.tagName()); - if (!"".equals(findBy.xpath())) finders.add("xpath: " + findBy.xpath()); - - // A zero count is okay: it means to look by name or id. - if (finders.size() > 1) { - throw new IllegalArgumentException( - String.format("You must specify at most one location strategy. Number found: %d (%s)", - finders.size(), finders.toString())); - } - } } diff --git a/java/client/src/org/openqa/selenium/support/pagefactory/Annotations.java b/java/client/src/org/openqa/selenium/support/pagefactory/Annotations.java index 42fffefb4e344..3d43c8b24413e 100644 --- a/java/client/src/org/openqa/selenium/support/pagefactory/Annotations.java +++ b/java/client/src/org/openqa/selenium/support/pagefactory/Annotations.java @@ -18,12 +18,15 @@ package org.openqa.selenium.support.pagefactory; import org.openqa.selenium.By; +import org.openqa.selenium.support.AbstractFindByBuilder; import org.openqa.selenium.support.ByIdOrName; import org.openqa.selenium.support.CacheLookup; import org.openqa.selenium.support.FindAll; import org.openqa.selenium.support.FindBy; import org.openqa.selenium.support.FindBys; +import org.openqa.selenium.support.PageFactoryFinder; +import java.lang.annotation.Annotation; import java.lang.reflect.Field; public class Annotations extends AbstractAnnotations { @@ -59,19 +62,22 @@ public By buildBy() { By ans = null; - FindBys findBys = field.getAnnotation(FindBys.class); - if (findBys != null) { - ans = buildByFromFindBys(findBys); - } - - FindAll findAll = field.getAnnotation(FindAll.class); - if (ans == null && findAll != null) { - ans = buildBysFromFindByOneOf(findAll); - } + for (Annotation annotation : field.getDeclaredAnnotations()) { + AbstractFindByBuilder builder = null; + if (annotation.annotationType().isAnnotationPresent(PageFactoryFinder.class)) { + try { + builder = annotation.annotationType() + .getAnnotation(PageFactoryFinder.class).value() + .newInstance(); + } catch (InstantiationException e) { + } catch (IllegalAccessException e) { + } + } + if (builder != null) { + ans = builder.buildIt(annotation); + break; + } - FindBy findBy = field.getAnnotation(FindBy.class); - if (ans == null && findBy != null) { - ans = buildByFromFindBy(findBy); } if (ans == null) { diff --git a/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java b/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java index c2f55cd49ee71..eacc91d57058c 100644 --- a/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java +++ b/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java @@ -26,13 +26,20 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.openqa.selenium.By; +import org.openqa.selenium.SearchContext; import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.AbstractFindByBuilder; import org.openqa.selenium.support.ByIdOrName; import org.openqa.selenium.support.FindAll; import org.openqa.selenium.support.FindBy; import org.openqa.selenium.support.FindBys; import org.openqa.selenium.support.How; +import org.openqa.selenium.support.PageFactoryFinder; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; import java.util.List; @RunWith(JUnit4.class) @@ -94,6 +101,34 @@ public class AnnotationsTest { @FindBy(using = "cheese") public WebElement findByUnsetHow_field; + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.FIELD, ElementType.TYPE}) + @PageFactoryFinder(FindByXXXX.FindByXXXXBuilder.class) + public @interface FindByXXXX { + + public static class FindByXXXXBuilder extends AbstractFindByBuilder { + + @Override + public By buildIt(Object annotation) { + return new By() { + @Override + public List findElements(SearchContext context) { + return null; + } + + @Override + public String toString() { + return "FindByXXXX's By"; + } + }; + } + } + + } + + @FindByXXXX() + public WebElement findBy_xxx; + @Test public void testDefault() throws Exception { assertThat(new Annotations(getClass().getField("default_field")).buildBy(), @@ -221,4 +256,10 @@ public void findByUnsetHowIsEquivalentToFindById() throws Exception { equalTo(By.id("cheese"))); } + @Test + public void findBySomethingOutsideSeleniumsWorld() throws Exception { + assertThat(new Annotations(getClass().getField("findBy_xxx")).buildBy().toString(), + equalTo("FindByXXXX's By")); + } + } From 581d92d3ff9326a477e33009f1509dc2c53abd3d Mon Sep 17 00:00:00 2001 From: Paul Hammant Date: Sun, 19 Mar 2017 20:23:19 -0400 Subject: [PATCH 2/2] standards, comments for @Findby abstraction work --- .../support/AbstractFindByBuilder.java | 24 +++++++++++++++---- .../support/pagefactory/AnnotationsTest.java | 8 ++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java b/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java index 628c5a7124999..9c1fd15a8be5f 100644 --- a/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java +++ b/java/client/src/org/openqa/selenium/support/AbstractFindByBuilder.java @@ -1,3 +1,20 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you 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. + package org.openqa.selenium.support; import org.openqa.selenium.By; @@ -5,9 +22,6 @@ import java.util.HashSet; import java.util.Set; -/** - * Created by paul on 3/8/17. - */ public abstract class AbstractFindByBuilder { public abstract By buildIt(Object annotation); @@ -51,6 +65,7 @@ protected By buildByFromShortFindBy(FindBy findBy) { // Fall through return null; } + protected By buildByFromLongFindBy(FindBy findBy) { How how = findBy.how(); String using = findBy.using(); @@ -90,11 +105,13 @@ protected By buildByFromLongFindBy(FindBy findBy) { throw new IllegalArgumentException("Cannot determine how to locate element "); } } + protected void assertValidFindBys(FindBys findBys) { for (FindBy findBy : findBys.value()) { assertValidFindBy(findBy); } } + protected void assertValidFindBy(FindBy findBy) { if (findBy.how() != null) { if (findBy.using() == null) { @@ -129,5 +146,4 @@ protected void assertValidFindAll(FindAll findBys) { } } - } diff --git a/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java b/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java index eacc91d57058c..5d61a7e816c5c 100644 --- a/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java +++ b/java/client/test/org/openqa/selenium/support/pagefactory/AnnotationsTest.java @@ -256,8 +256,14 @@ public void findByUnsetHowIsEquivalentToFindById() throws Exception { equalTo(By.id("cheese"))); } + /* + * Example of how teams making their own @FinyBy alikes would experience a general purpose + * capability. + * + * @See @FindByXXXX (above) + */ @Test - public void findBySomethingOutsideSeleniumsWorld() throws Exception { + public void findBySomethingElse() throws Exception { assertThat(new Annotations(getClass().getField("findBy_xxx")).buildBy().toString(), equalTo("FindByXXXX's By")); }