dsmiley commented on code in PR #2881:
URL: https://github.com/apache/solr/pull/2881#discussion_r1855522803


##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -1128,9 +1131,9 @@ protected SolrCore(
       initIndex(prev != null, reload);
 
       initWriters();
-      qParserPlugins.init(QParserPlugin.standardPlugins, this);
-      valueSourceParsers.init(ValueSourceParser.standardValueSourceParsers, 
this);
-      transformerFactories.init(TransformerFactory.defaultFactories, this);
+      qParserPlugins.init(QParserPlugins.standardPlugins, this);

Review Comment:
   These 3 registries only exist for SolrCore registration.  You could move 
them to the same package and make them not public.
   
   But longer term (or soon if you are to do the work), I'd rather our built-in 
plugins be auto-discovered and registered via embracing 
java.util.ServiceLoader.  Then we would have no static registries. -- CC @janhoy



##########
solr/solrj/src/java/org/apache/solr/common/MapWriters.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.common;
+
+import java.util.Collections;
+
+public class MapWriters {

Review Comment:
   just for EMPTY feels sad... 
   Instead, I could imagine MapWriter.empty() that references an EMPTY constant 
declared on MapWriterMap



##########
solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java:
##########
@@ -38,14 +36,24 @@
  * @lucene.experimental
  */
 public abstract class DocRouter {
-  public static final String DEFAULT_NAME = CompositeIdRouter.NAME;
-  public static final DocRouter DEFAULT;
+  /**
+   * @deprecated Use {@link DocRouters#DEFAULT_NAME} instead.
+   */
+  @Deprecated(since = "9.8", forRemoval = true)
+  public static final String DEFAULT_NAME = DocRouters.DEFAULT_NAME;

Review Comment:
   WDYT about keeping this as a constant string?  Then almost nobody would have 
a need to reference the new DocRouters.



##########
solr/core/src/java/org/apache/solr/search/FunctionQParser.java:
##########
@@ -398,7 +398,7 @@ protected ValueSource parseValueSource(int flags) throws 
SyntaxError {
     if ((ch >= '0' && ch <= '9') || ch == '.' || ch == '+' || ch == '-') {
       Number num = sp.getNumber();
       if (num instanceof Long) {
-        valueSource = new 
ValueSourceParser.LongConstValueSource(num.longValue());
+        valueSource = new 
ValueSourceParsers.LongConstValueSource(num.longValue());

Review Comment:
   it appears inconsistent with the lower two lines; can you static import 
this?  Or move to top level if the below ones are (consistency)



##########
solr/core/src/java/org/apache/solr/search/ValueSourceParser.java:
##########
@@ -128,1329 +44,19 @@ public abstract class ValueSourceParser implements 
NamedListInitializedPlugin {
   /** Parse the user input into a ValueSource. */
   public abstract ValueSource parse(FunctionQParser fp) throws SyntaxError;
 
-  /** standard functions supported by default, filled in static class 
initialization */
-  private static final Map<String, ValueSourceParser> standardVSParsers = new 
HashMap<>();
-
-  /** standard functions supported by default */
-  public static final Map<String, ValueSourceParser> 
standardValueSourceParsers =
-      Collections.unmodifiableMap(standardVSParsers);
-
   /**
-   * Adds a new parser for the name and returns any existing one that was 
overridden. This is not
-   * thread safe.
+   * standard functions supported by default
+   *
+   * @deprecated Use {@link ValueSourceParsers#standardValueSourceParsers} 
instead.
    */
-  private static ValueSourceParser addParser(String name, ValueSourceParser p) 
{
-    return standardVSParsers.put(name, p);
-  }
+  @Deprecated(since = "9.8", forRemoval = true)
+  public static final Map<String, ValueSourceParser> 
standardValueSourceParsers =

Review Comment:
   no need to deprecate



##########
solr/core/src/java/org/apache/solr/search/QParserPlugin.java:
##########
@@ -43,57 +32,11 @@ public abstract class QParserPlugin implements 
NamedListInitializedPlugin, SolrI
    * {@link QParserPlugin} has own instance of standardPlugins. This leads to 
cyclic dependencies of
    * static fields and to case when NAME field is not yet initialized. This 
result to NPE during
    * initialization. For every plugin, listed here, NAME field has to be final 
and static.
+   *
+   * @deprecated Use {@link QParserPlugins#standardPlugins} instead.
    */
-  public static final Map<String, QParserPlugin> standardPlugins;
-
-  static {
-    HashMap<String, QParserPlugin> map = new HashMap<>(30, 1);
-    map.put(LuceneQParserPlugin.NAME, new LuceneQParserPlugin());
-    map.put(FunctionQParserPlugin.NAME, new FunctionQParserPlugin());
-    map.put(PrefixQParserPlugin.NAME, new PrefixQParserPlugin());
-    map.put(BoostQParserPlugin.NAME, new BoostQParserPlugin());
-    map.put(DisMaxQParserPlugin.NAME, new DisMaxQParserPlugin());
-    map.put(ExtendedDismaxQParserPlugin.NAME, new 
ExtendedDismaxQParserPlugin());
-    map.put(FieldQParserPlugin.NAME, new FieldQParserPlugin());
-    map.put(RawQParserPlugin.NAME, new RawQParserPlugin());
-    map.put(TermQParserPlugin.NAME, new TermQParserPlugin());
-    map.put(TermsQParserPlugin.NAME, new TermsQParserPlugin());
-    map.put(NestedQParserPlugin.NAME, new NestedQParserPlugin());
-    map.put(FunctionRangeQParserPlugin.NAME, new FunctionRangeQParserPlugin());
-    map.put(SpatialFilterQParserPlugin.NAME, new SpatialFilterQParserPlugin());
-    map.put(SpatialBoxQParserPlugin.NAME, new SpatialBoxQParserPlugin());
-    map.put(JoinQParserPlugin.NAME, new JoinQParserPlugin());
-    map.put(SurroundQParserPlugin.NAME, new SurroundQParserPlugin());
-    map.put(SwitchQParserPlugin.NAME, new SwitchQParserPlugin());
-    map.put(MaxScoreQParserPlugin.NAME, new MaxScoreQParserPlugin());
-    map.put(BlockJoinParentQParserPlugin.NAME, new 
BlockJoinParentQParserPlugin());
-    map.put(BlockJoinChildQParserPlugin.NAME, new 
BlockJoinChildQParserPlugin());
-    map.put(FiltersQParserPlugin.NAME, new FiltersQParserPlugin());
-    map.put(CollapsingQParserPlugin.NAME, new CollapsingQParserPlugin());
-    map.put(SimpleQParserPlugin.NAME, new SimpleQParserPlugin());
-    map.put(ComplexPhraseQParserPlugin.NAME, new ComplexPhraseQParserPlugin());
-    map.put(ReRankQParserPlugin.NAME, new ReRankQParserPlugin());
-    map.put(ExportQParserPlugin.NAME, new ExportQParserPlugin());
-    map.put(MLTQParserPlugin.NAME, new MLTQParserPlugin());
-    map.put(MLTContentQParserPlugin.NAME, new MLTContentQParserPlugin());
-    map.put(HashQParserPlugin.NAME, new HashQParserPlugin());
-    map.put(GraphQParserPlugin.NAME, new GraphQParserPlugin());
-    map.put(XmlQParserPlugin.NAME, new XmlQParserPlugin());
-    map.put(GraphTermsQParserPlugin.NAME, new GraphTermsQParserPlugin());
-    map.put(IGainTermsQParserPlugin.NAME, new IGainTermsQParserPlugin());
-    map.put(TextLogisticRegressionQParserPlugin.NAME, new 
TextLogisticRegressionQParserPlugin());
-    map.put(SignificantTermsQParserPlugin.NAME, new 
SignificantTermsQParserPlugin());
-    map.put(PayloadScoreQParserPlugin.NAME, new PayloadScoreQParserPlugin());
-    map.put(PayloadCheckQParserPlugin.NAME, new PayloadCheckQParserPlugin());
-    map.put(BoolQParserPlugin.NAME, new BoolQParserPlugin());
-    map.put(MinHashQParserPlugin.NAME, new MinHashQParserPlugin());
-    map.put(HashRangeQParserPlugin.NAME, new HashRangeQParserPlugin());
-    map.put(RankQParserPlugin.NAME, new RankQParserPlugin());
-    map.put(KnnQParserPlugin.NAME, new KnnQParserPlugin());
-    map.put(VectorSimilarityQParserPlugin.NAME, new 
VectorSimilarityQParserPlugin());
-
-    standardPlugins = Collections.unmodifiableMap(map);
-  }
+  @Deprecated(since = "9.8", forRemoval = true)
+  public static final Map<String, QParserPlugin> standardPlugins = 
QParserPlugins.standardPlugins;

Review Comment:
   no need to deprecate



##########
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java:
##########
@@ -105,18 +104,10 @@ default boolean mayModifyValue() {
     }
   }
 
-  public static final Map<String, TransformerFactory> defaultFactories = new 
HashMap<>(9, 1.0f);
-
-  static {
-    defaultFactories.put("explain", new ExplainAugmenterFactory());
-    defaultFactories.put("value", new ValueAugmenterFactory());
-    defaultFactories.put("docid", new DocIdAugmenterFactory());
-    defaultFactories.put("shard", new ShardAugmenterFactory());
-    defaultFactories.put("child", new ChildDocTransformerFactory());
-    defaultFactories.put("subquery", new SubQueryAugmenterFactory());
-    defaultFactories.put("json", new RawValueTransformerFactory("json"));
-    defaultFactories.put("xml", new RawValueTransformerFactory("xml"));
-    defaultFactories.put("geo", new GeoTransformerFactory());
-    defaultFactories.put("core", new CoreAugmenterFactory());
-  }
+  /**
+   * @deprecated Use {@link TransformerFactories#defaultFactories} instead.
+   */
+  @Deprecated(since = "9.8", forRemoval = true)

Review Comment:
   No need to deprecate this; it's way too internal.   (I've worked on Solr 
long enough to make this judgement)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to