Skip to content

SOLR-18360: un-deprecate HttpJettySolrClient.addListenerFactory - #4780

Open
serhiy-bzhezytskyy wants to merge 1 commit into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-18360-undeprecate-addlistenerfactory
Open

SOLR-18360: un-deprecate HttpJettySolrClient.addListenerFactory#4780
serhiy-bzhezytskyy wants to merge 1 commit into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-18360-undeprecate-addlistenerfactory

Conversation

@serhiy-bzhezytskyy

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18360

Un-deprecates HttpJettySolrClient.addListenerFactory(HttpListenerFactory) instead of removing it. PKIAuthenticationPlugin needs it to attach a listener to clients that are already built and already referenced elsewhere by the time security wiring runs:

// HttpShardHandlerFactory's constructor:
this.defaultClient = new HttpJettySolrClient.Builder()...build();
this.loadbalancer = new LBJettySolrClient.Builder(defaultClient).build();  // captures defaultClient here

// CoreContainer.setupHttpClientForAuthPlugin, called later (and again on security.json hot-reload):
shardHandlerFactory.setSecurityBuilder(pkiAuthenticationSecurityBuilder);  // -> defaultClient.addListenerFactory(...)

Rebuilding defaultClient via the Builder instead would leave loadbalancer pointing at the old, listener-less client -- the security listener would silently never fire on the path that actually routes inter-node requests.

Migrated the one call site that was pure construction-time convenience (HttpShardHandlerFactory's own defaultClient) to Builder.addListenerFactory. No changelog (un-deprecation, not a removal).

50 tests, 0 failures.

AI-assisted (Claude Sonnet 5)

…istenerFactory)

The ticket asked to remove this instance method in favor of the
Builder-only equivalent, but PKIAuthenticationPlugin (implementing
HttpClientBuilderPlugin) attaches a listener to already-built clients
across HttpShardHandlerFactory/UpdateShardHandler/HttpSolrClientProvider
from CoreContainer.setupHttpClientForAuthPlugin -- a path that can
re-fire on security.json hot-reload, on clients that other long-lived
objects (e.g. HttpShardHandlerFactory's own loadbalancer) already
reference. Rebuilding via the Builder instead would desync those
references. Migrated the one call site that WAS just construction-time
convenience (HttpShardHandlerFactory's own defaultClient) to the Builder
method; left the instance method for the case it actually serves.
@serhiy-bzhezytskyy

Copy link
Copy Markdown
Contributor Author

@epugh flagging this un-deprecation (not a removal) -- see PR description for why the instance method is still needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant