[java] Restore HttpCommandExecutor client and httpClientFactory fields as deprecated

[diemol]

🔗 Related Issues

Follow-up to #18038, which broke the Appium Java client build (CI run).

💥 What does this PR do?

#18038 made HttpCommandExecutor.client protected and removed httpClientFactory without deprecating them first. Appium uses both, so current Appium releases would fail at runtime with Selenium 4.50.

This PR brings both fields back as deprecated, to be changed in 4.52:

• client is public again, and becomes protected in 4.52.
• httpClientFactory is back, and is removed in 4.52.

🔧 Implementation Notes

• The factory is set by the deprecated factory constructors and is null when the executor is created from an HttpClient.
• HttpCommandExecutorTest checks the field visibility and the factory value for each constructor.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: CI failure analysis, the fix, the unit test and this description
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• @joerg1985, this reaches the same end state as [java] Use one HttpClient.Factory inside one RemoteWebDriver instance #18038, through the deprecation policy.
• Appium will move off these fields before 4.52 (follow-up to fix: fall back to standard error mapping in ErrorCodesMobile.getExceptionType(String) appium/java-client#2449).

🔄 Types of changes

• Bug fix (backwards compatible)

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Restore deprecated HttpCommandExecutor fields for Appium compatibility

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Restore field access needed by existing Appium releases to avoid linkage failures.
• Retain the factory for factory-created executors while allowing client-created executors to have
 no factory.
• Add tests for field visibility and constructor-specific field values.

Diagram

graph TD
  F["Factory constructor"] --> H["HTTP factory"] --> C["HTTP client"] --> I["Shared initialization"] --> E["Restored fields"] --> A["Appium consumers"]
  D["Client constructor"] --> I
  F --> I

Loading

High-Level Assessment

Restoring the fields is the appropriate compatibility fix: adding getters or changing Appium alone would not prevent failures in already compiled Appium releases. Deprecation preserves a path to remove the fields after consumers migrate.

Files changed (2) +105 / -2

Bug fix (1) +30 / -2

HttpCommandExecutor.javaRestore deprecated client and factory fields +30/-2

Restore deprecated client and factory fields

• Makes client public again and restores the protected, nullable httpClientFactory field, marking both deprecated for removal. A shared private constructor retains the factory for factory-based construction and assigns null for direct-client construction.

java/src/org/openqa/selenium/remote/HttpCommandExecutor.java

Tests (1) +75 / -0

HttpCommandExecutorTest.javaTest field visibility and constructor-specific values +75/-0

Test field visibility and constructor-specific values

• Adds reflection checks for the public client and protected factory fields. Verifies that factory-based construction retains its factory and direct-client construction retains its client without a factory.

java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. New tests leave clients running ✓ Resolved

Description

keepsTheFactoryPassedToTheConstructor and hasNoFactoryWhenCreatedFromAClient create HTTP clients
without closing them. When these tests run, the default client’s executor is not shut down after the
assertions.

Code

java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[59]

+    HttpCommandExecutor executor = new HttpCommandExecutor(Map.of(), config, factory);

Evidence

The factory constructor creates a client, and the second test creates one directly; neither test
closes it. The default JDK client allocates an executor service during construction and shuts it
down in close().

java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[55-74]
java/src/org/openqa/selenium/remote/HttpCommandExecutor.java[157-165]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[93-108]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[575-603]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both new constructor tests create HTTP clients but leave them open. The default client owns an executor that is shut down by `close()`.
## Fix Focus Areas
- java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[55-74]
## Recommended Fix
Close the executor's client in a `finally` block in the factory-constructor test, and use try-with-resources for the explicitly created client in the other test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: 🧠 Deep: This push contains substantial, behavior-changing logic across CI workflows, Bazel test orchestration, JavaScript runtime/testing infrastructure, release tooling, and multiple language-specific tests, creating many independent paths where redundant review could catch subtle defects.

Tip of the day

💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit fb77ffd

Results up to commit 56d88a8 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. New tests leave clients running ✓ Resolved

Description

keepsTheFactoryPassedToTheConstructor and hasNoFactoryWhenCreatedFromAClient create HTTP clients
without closing them. When these tests run, the default client’s executor is not shut down after the
assertions.

Code

java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[59]

+    HttpCommandExecutor executor = new HttpCommandExecutor(Map.of(), config, factory);

Evidence

The factory constructor creates a client, and the second test creates one directly; neither test
closes it. The default JDK client allocates an executor service during construction and shuts it
down in close().

java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[55-74]
java/src/org/openqa/selenium/remote/HttpCommandExecutor.java[157-165]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[93-108]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[575-603]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both new constructor tests create HTTP clients but leave them open. The default client owns an executor that is shut down by `close()`.
## Fix Focus Areas
- java/test/org/openqa/selenium/remote/HttpCommandExecutorTest.java[55-74]
## Recommended Fix
Close the executor's client in a `finally` block in the factory-constructor test, and use try-with-resources for the explicitly created client in the other test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit fb77ffd