[
https://issues.apache.org/jira/browse/SOLR-10915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16056177#comment-16056177
]
Anshum Gupta commented on SOLR-10915:
-------------------------------------
Thanks Jason. The patch looks good overall but I think we should only have
client constructors that accept the builder, and nothing else - even private
constructors. Everything that is needed is a part of the builder object, and so
can be extracted. That certainly means more work, but I think it is worth it.
e.g.
{code:java}
// L209 in the patch
protected CloudSolrClient(Builder builder) {
this(builder.zkHosts, builder.zkChroot, builder.solrUrls,
builder.httpClient, builder.loadBalancedSolrClient,
builder.lbClientBuilder, builder.shardLeadersOnly,
builder.directUpdatesToLeadersOnly, builder.stateProvider);
}
{code}
We should just move the implementation of that private constructor in here.
I don't personally have a strong opinion around {{withFoo}} methods Vs those
being a part of the constructor. As long as that list is restricted to < 3
params (almost an arbitrary number that's not too high), I don't have a problem.
bq. That seems reasonable to me, but I also see an argument that methods marked
as protected are a part of our API and we'd need to give more warning. Either
way would be fine with me.
We aren't 'removing' anything here but just deprecating so we should be fine.
> Make SolrClient classes extendable without code duplication
> -----------------------------------------------------------
>
> Key: SOLR-10915
> URL: https://issues.apache.org/jira/browse/SOLR-10915
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
> Components: clients - java
> Reporter: Anshum Gupta
> Assignee: Anshum Gupta
> Priority: Blocker
> Fix For: master (7.0)
>
> Attachments: SOLR-10915.patch.with-deprecations,
> SOLR-10915.patch.without-deprecations
>
>
> SolrClient used to be easily extendable but our move to Builder pattern has
> made it impossible to extend those classes without code duplication.
> As an example, if we wanted to extend HttpSolrClient we would also want to
> extend the Builder. The problem is that the constructor of the main class,
> does not accept the builder object but explicit parameters, and the inherited
> class doesn't have access to any of those values from the Builder object as
> they are all private.
> Generally, we'd want to have the client object constructor accept the
> Builder, instead of explicit values, and also make the Builder values
> protected so the inherited classes could use them.
> I'm marking this as a blocker for 7.0 as this is an important piece that
> needs to be fixed before the major release, specially now that we know that
> this problem exists.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
