[hoanle0126]

Proposed changes

Fixes #1940

• Support decorating By and AppiumBy fields in AppiumFieldDecorator when annotated with Appium or Selenium locator annotations (@AndroidFindBy, @AndroidFindBys, @AndroidFindAll, @iOSXCUITFindBy, @iOSXCUITFindBys, @iOSXCUITFindAll, @FindBy, @FindBys, @FindAll).
• Return the single locator directly from DefaultElementByBuilder.buildMobileBy when only one locator is present (bys.length == 1) so single @AndroidFindBy / @iOSXCUITFindBy annotations resolve directly to AppiumBy (e.g. AppiumBy.id(...), AppiumBy.accessibilityId(...)) rather than wrapping a 1-element array in ByChained / ByAll.
• Leave unannotated By / AppiumBy fields untouched (null returned by decorate) so pre-initialized By constants on page objects are preserved.
• Add unit tests in ByFieldDecorationTest covering Android, iOS, browser/webview fallback, @HowToUseLocators, Widget sub-fields, pre-initialized By fields, and incompatible subtype validation.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update (if none of the other choices apply)

[mykola-mokhnach]

Review notes inline.

[hoanle0126]

Thanks @mykola-mokhnach for the thorough review! I have updated the PR in 36c20e8 to address all of your inline comments:

1. Lazy context resolution via ContentMappedBy & zero remote calls at initElements time:
   • By and ContentMappedBy fields now delegate directly to elementByBuilder.buildBy() once at decoration time (with no eager getCurrentContentType(...) call during initElements).
   • ContentMappedBy.findElement(SearchContext) and findElements(SearchContext) now call useContent(getCurrentContentType(context)), so decorated By / ContentMappedBy fields lazily follow context switches (NATIVE_APP <-> WEBVIEW) and use the standard ByIdOrName / By.ById fallback for any missing side.
2. Unchanged DefaultElementByBuilder.buildMobileBy for element fields:
   • Reverted DefaultElementByBuilder.buildMobileBy so WebElement, List, and Widget fields continue to receive ByChained / ByAll as before; single-element unwrapping is now isolated inside AppiumFieldDecorator.decorateBy when coercing to a narrower declared By subtype (such as AppiumBy).
3. Non-breaking handling of narrow By subtypes, pre-initialized fields, and final/static fields:
   • decorateBy skips static, final, and already-initialized By fields on the current page instance, and returns null (rather than throwing IllegalArgumentException) when a narrower declared By subtype does not match the built locator.
   • Unannotated ContentMappedBy fields now delegate directly to elementByBuilder.buildBy() so they receive the id/name fallback.
4. Expanded tests in ByFieldDecorationTest:
   • Added coverage for lazy context switching after initElements, zero getContext() / isBrowser() calls during initElements, missing-side ByIdOrName fallback in webview context, unannotated ContentMappedBy fields, pre-initialized/final/static By fields, @CacheLookup / @WithTimeout on By fields, and conflicting @FindBy + @FindBys / @FindAll annotations.

[mykola-mokhnach]

• AppiumFieldDecorator.java:117: elementByBuilder is now one instance shared by every decorate call (new field, previously a fresh DefaultElementByBuilder per locator). decorateBy mutates it with setAnnotated(field) and setAnnotated(null). If one decorator is used from two threads, for example parallel page initialisation, thread A's setAnnotated(null) in finally can run while thread B is mid-build. B then gets an NPE or builds a locator from the wrong field. Should this be one builder per decorateBy call, or synchronised?
• AppiumFieldDecorator.java:119: The two paths treat missing annotations differently. A ContentMappedBy-typed field with no annotations is always replaced with a ByIdOrName(fieldName) locator (see unannotatedContentMappedBy in the test), because that branch returns before the hasLocatorAnnotations check. A plain By field in the same state is left alone. A user who assigns the ContentMappedBy field in a constructor or setup method gets the value overwritten. Is that intended?
• AppiumFieldDecorator.java:130: unwrapSingleBy reads the private bys field of ByChained/ByAll by reflection and falls back silently on ReflectiveOperationException. Both classes live in this project, so a public accessor would avoid the reflection. A rename or refactor of bys would otherwise make the unwrap stop working with no signal, and coerceToType would return null.
• AppiumFieldDecorator.java:153: hasLocatorAnnotations hard-codes the list of supported annotations separately from the builder. An annotation added to the builder later but missing from this list leaves By fields null, because decorateBy returns early. Could the builder expose "has any locator annotation" so there is one source of truth?

[hoanle0126]

Thanks @mykola-mokhnach for the follow-up review! Updated in 342f11a8 (and rebased onto master):

1. Per-call DefaultElementByBuilder & single source of truth for locator annotations:
   • Removed the shared elementByBuilder field from AppiumFieldDecorator; decorateBy now instantiates a fresh DefaultElementByBuilder per call so concurrent page initializations across threads cannot race on setAnnotated.
   • Moved hasLocatorAnnotations() into DefaultElementByBuilder so the supported locator annotations live in one place.
2. Consistent handling of unannotated By and ContentMappedBy fields (and reverted PageFactory changes):
   • decorateBy now checks builder.hasLocatorAnnotations() first for all By subtypes (including ContentMappedBy), returning null whenever no locator annotations are present. Unannotated or constructor/field-initialized By and ContentMappedBy fields are left untouched regardless of how initElements or decorate is invoked, and PageFactory is reverted to master.
3. Non-mutating, explicit-preserving ContentMappedBy resolution:
   • currentContent in ContentMappedBy is now @Nullable (excluded from @EqualsAndHashCode) and findElement / findElements resolve currentContent != null ? currentContent : getCurrentContentType(context) into a local variable. Callers that explicitly set the content type via useContent(...) (such as AppiumElementLocator) are respected without extra remote context round-trips or overriding nested element lookups, while hashCode / equals remain invariant.
4. Replaced reflection in unwrapSingleBy:
   • Added getBys() accessors to ByChained and ByAll and used them directly in unwrapSingleBy.

[mykola-mokhnach]

• Annotated By fields with an initial value are overwritten. @FindBy(id="x") private By loc = By.id("y"); is replaced at initElements. Only unannotated fields are preserved (preInitializedBy in the test). Is overwriting intended now that the earlier "already initialised" guard is gone? The earlier thread on this was resolved based on that guard.

[mykola-mokhnach]

The default changes from always NATIVE_MOBILE_SPECIFIC to auto-detection when no content type is set. Code that builds new ContentMappedBy(map) and passes it to driver.findElement used to get the native locator. On a context that is neither HasBrowserCheck nor SupportsContextSwitching (a plain RemoteWebDriver or a mock), it now gets the web one. toString() still reports the native entry when nothing is forced, so logs can show a locator that isn't the one used. The class Javadoc doesn't mention the new behavior.

[mykola-mokhnach]

With @EqualsAndHashCode.Exclude on currentContent, two instances over the same map but forced to different content types now compare equal and share a hash. A Set or Map keyed by By would conflate locators that resolve to different elements. Is that acceptable, or should equality use only the forced type, if any?

[mykola-mokhnach]

If the field type is a narrow By subtype and no built locator fits, the field stays null and initElements succeeds. Examples: @AndroidFindBy(xpath=...) AppiumBy, an AppiumBy field with only @FindBy, or an AppiumBy field with several annotations. (unwrapSingleBy only flattens single-element chains.) The first use is an NPE with no hint of the cause, while conflicting annotations do throw IllegalArgumentException. The test pins the null result but nothing documents it. A WARN log or a Javadoc note on the decorator would help.

[mykola-mokhnach]

this does not look very reliable to me. how can we make sure another annotation will be added to this list and not forgotten?