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

[hoanle0126]

Thanks @mykola-mokhnach! Addressed all 5 points in b5dd09d:

• Restored the pre-initialized field guard via PageFactory.isCurrentPageFieldInitialized(Field) (backed by the active page object stack so re-entrant initElements calls match the declaring class without exposing the page instance, and preserving annotated pre-initialized By / ContentMappedBy fields).
• Removed hasLocatorAnnotations() completely in favor of calling buildMobileNativeBy() and buildDefaultBy() directly.
• Included currentContent in ContentMappedBy equals/hashCode, updated toString() when unpinned, and documented the auto-detection behavior in Javadoc.
• Added LOG.warn(...) and Javadoc on AppiumFieldDecorator when a built locator cannot be coerced to a narrow By subtype.

[mykola-mokhnach]

Inline comment only; no general findings this round.

[mykola-mokhnach]

@valfirst could you please take look?