[
https://issues.apache.org/jira/browse/SOLR-10915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16056053#comment-16056053
]
Jason Gerlowski commented on SOLR-10915:
----------------------------------------
A few responses:
bq. Where a Builder class has been added, it doesn't have any explicit
visibility, while the existing Builder classes are explicitly marked public.
That's a good point/question. Most of the builders added in this patch were
for trivial SolrClient subclasses that were only used in that same Java file.
Because of that I didn't give visibility a ton of thought- I just left them as
package-scoped. But we could probably do something more thoughtful on a
case-by-case basis here if it's worth it.
bq. The new Builder classes do not have any fluent "with*" methods, only a
multiple-argument constructor.
The convention I've been trying to follow is that truly required arguments
should go in the {{Builder()}} ctor, to force users to provide them. Optional
ones I've been creating fluent {{withFoo}} methods for. The lack of added
{{withFoo}} methods in this patch is (I think) just a coincidence related to
the sort of SolrClient subclasses I ran into with this patch (created with a
very directed purpose, with limited visibility, for 1 or 2 Java files). If I
made any mistakes following that convention, or you would prefer deviating from
the convention for other reasons, I'd be fine with that too.
bq. The remaining non-deprecated constructor (before this patch) is likely
private or protected, and I don't think we have any reason to worry about
deprecating it before we change it to one that accepts a Builder.
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.
> 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]
