Tidy up java code in o.a.s.common package by epugh · Pull Request #4669 · apache/solr

epugh · 2026-07-26T11:48:00Z

Description

Reviewing code in the org.apache.solr.common package for potential improvements.

Solution

Mostly driven by what IntelliJ is suggesting are weaknesses. Specifically, lots of unused methods. I found some gnarly extra params that don't get used in the JSON parsing stuff I'd love extra eyes on.

Tests

Existing tests, plus talking to Claude for reivew.

…d on interfaces.

Copilot

Pull request overview

This PR cleans up and simplifies code across org.apache.solr.common / org.apache.solr.common.util (SolrJ and a few core call sites), largely by removing unused helpers/overloads, tightening signatures, and doing small stylistic/doc tweaks.

Changes:

Removed a number of unused methods/classes and streamlined several APIs (notably JSON parsing/writing utilities and routing-rule construction).
Performed small correctness/style fixes (e.g., isEmpty() usage, boxing simplifications, typo fixes, minor javadocs).
Updated a few call sites/tests to match the refactors.

Reviewed changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
solr/solrj/src/java/org/apache/solr/common/util/XML.java	Removes unused overloads/imports; minor signature cleanup.
solr/solrj/src/java/org/apache/solr/common/util/Utils.java	Removes unused helpers/constants; centralizes JSONParser creation via `getJSONParser`.
solr/solrj/src/java/org/apache/solr/common/util/URLUtil.java	Minor javadoc wording tweak.
solr/solrj/src/java/org/apache/solr/common/util/Template.java	Deletes unused Template helper class.
solr/solrj/src/java/org/apache/solr/common/util/PropertiesUtil.java	Removes unused parsing helpers.
solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java	Removes redundant no-arg constructor.
solr/solrj/src/java/org/apache/solr/common/util/ObjectReleaseTracker.java	Removes unused default stack-filter list/imports.
solr/solrj/src/java/org/apache/solr/common/util/NamedList.java	Minor javadoc punctuation fix.
solr/solrj/src/java/org/apache/solr/common/util/JsonTextWriter.java	Removes unused “name” params and deletes unused mangling writer.
solr/solrj/src/java/org/apache/solr/common/util/JsonSchemaValidator.java	Removes stray semicolon in constructor.
solr/solrj/src/java/org/apache/solr/common/util/JsonRecordReader.java	Removes unused parameters, simplifies record parsing state flow.
solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java	Suppression cleanup + minor boxing simplifications.
solr/solrj/src/java/org/apache/solr/common/util/GlobPatternUtil.java	Uses `isEmpty()` for buffers.
solr/solrj/src/java/org/apache/solr/common/util/FastWriter.java	Array parameter style normalization.
solr/solrj/src/java/org/apache/solr/common/util/FastOutputStream.java	Array parameter style normalization.
solr/solrj/src/java/org/apache/solr/common/util/FastJavaBinDecoder.java	Literal cleanup + rely on autoboxing for returned Objects.
solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java	Removes unused shutdown helper.
solr/solrj/src/java/org/apache/solr/common/util/DOMUtil.java	Removes unused property-substitution helpers.
solr/solrj/src/java/org/apache/solr/common/util/DataInputInputStream.java	Fixes typos and removes stray semicolon.
solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java	Removes unused setter.
solr/solrj/src/java/org/apache/solr/common/util/CommonTestInjection.java	Removes unused “additional props” injection plumbing.
solr/solrj/src/java/org/apache/solr/common/util/CommandOperation.java	Uses `Utils.getJSONParser` for consistent parser flags.
solr/solrj/src/java/org/apache/solr/common/util/ByteUtils.java	Removes unused UTF8->String helper + minor casting cleanup.
solr/solrj/src/java/org/apache/solr/common/util/ByteArrayUtf8CharSequence.java	Removes unused getter; minor cleanup in conversion utility.
solr/solrj/src/java/org/apache/solr/common/ToleratedUpdateError.java	Minor cleanup (string return, enum style).
solr/solrj/src/java/org/apache/solr/common/SolrException.java	Removes MDC logging helpers; minor enum/constructor cleanup; adds TODO comment.
solr/solrj/src/java/org/apache/solr/common/SolrDocumentList.java	Minor toString simplification.
solr/solrj/src/java/org/apache/solr/common/params/UpdateParams.java	Interface constants cleanup (implicit `public static final`).
solr/solrj/src/java/org/apache/solr/common/params/TermVectorParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/TermsParams.java	Interface constants cleanup + minor formatting.
solr/solrj/src/java/org/apache/solr/common/params/StatsParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/SpellingParams.java	Interface constants cleanup + formatting.
solr/solrj/src/java/org/apache/solr/common/params/SpatialParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java	Removes unused primitive-field helpers and related javadoc refs.
solr/solrj/src/java/org/apache/solr/common/params/SimpleParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/MoreLikeThisParams.java	Interface constants cleanup + enum visibility cleanup.
solr/solrj/src/java/org/apache/solr/common/params/HighlightParams.java	Interface constants cleanup + formatting.
solr/solrj/src/java/org/apache/solr/common/params/GroupParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/FacetParams.java	Interface constants cleanup + enum visibility cleanup + minor javadoc tweak.
solr/solrj/src/java/org/apache/solr/common/params/ExpandParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/EventParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/DisMaxParams.java	Removes unused params and cleans interface declarations.
solr/solrj/src/java/org/apache/solr/common/params/CursorMarkParams.java	Interface constants cleanup.
solr/solrj/src/java/org/apache/solr/common/params/ConfigSetParams.java	Interface constants cleanup + enum visibility cleanup.
solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java	Removes unused constants + updates MEM_ALLOWED javadoc text.
solr/solrj/src/java/org/apache/solr/common/params/AnalysisParams.java	Interface constants cleanup + typo fixes.
solr/solrj/src/java/org/apache/solr/common/NavigableObject.java	Removes unused default overload.
solr/solrj/src/java/org/apache/solr/common/MapWriter.java	Removes deprecated EMPTY constant.
solr/solrj/src/java/org/apache/solr/common/LinkedHashMapWriter.java	Removes constructors/imports, leaving only no-arg ctor.
solr/solrj/src/java/org/apache/solr/common/ConditionalMapWriter.java	Deletes unused ConditionalMapWriter implementation.
solr/solrj/src/java/org/apache/solr/common/ConditionalKeyMapWriter.java	Removes dedupe helper and related import.
solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java	Updates RoutingRule construction to match new constructor.
solr/solrj/src/java/org/apache/solr/common/cloud/RoutingRule.java	Changes constructor signature to accept only the map.
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java	Removes unused logger/imports; fixes comment typo.
solr/solrj/src/java/org/apache/solr/common/cloud/HashBasedRouter.java	Changes `getId` signature and call pattern.
solr/solrj/src/java/org/apache/solr/common/cloud/CompositeIdRouter.java	Minor javadoc correction.
solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java	Minor javadoc punctuation fix.
solr/solrj/src/java/org/apache/solr/common/Callable.java	Interface method modifier cleanup.
solr/core/src/test/org/apache/solr/response/JSONWriterTest.java	Updates test expectations after JsonTextWriter method removal.
solr/core/src/java/org/apache/solr/cloud/overseer/SliceMutator.java	Updates RoutingRule construction to match new constructor.

Comments suppressed due to low confidence (2)

solr/solrj/src/java/org/apache/solr/common/cloud/RoutingRule.java:36

Changing the public constructor signature is a binary/source incompatible change for SolrJ users. Since routeKey was unused, consider restoring the old (String, Map) constructor as a deprecated forwarding overload to preserve backwards compatibility.

  public RoutingRule(Map<String, Object> propMap) {
    super(propMap);
    this.routeRangesStr = (String) propMap.get("routeRanges");
    String[] rangesArr = this.routeRangesStr.split(",");
    if (rangesArr != null && rangesArr.length > 0) {

solr/solrj/src/java/org/apache/solr/common/cloud/HashBasedRouter.java:69

Removing/changing the protected getId(SolrInputDocument, SolrParams) hook is a breaking change for downstream routers that override it. Re-introduce the old signature as a deprecated overload and have the new method delegate to it so existing subclass overrides continue to be invoked.

  protected String getId(SolrInputDocument sdoc) {
    Object idObj = sdoc.getFieldValue(ID); // blech
    String id = idObj != null ? idObj.toString() : "null"; // should only happen on client side
    return id;
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

        List<String> paths, // a List of segments from the split paths
        String fieldName, // the fieldName assoc with this path
-        boolean multiValued, // flag if this fieldName is multiValued or not
+        // flag if this fieldName is multiValued or not
        boolean record, // is this path a record or a field
        String path) {


+  // Can I be removed???  TODO
  private final Map<String, String> mdcContext;


 public class LinkedHashMapWriter<V> extends LinkedHashMap<String, V> implements MapWriter {

-  @SuppressForbidden(reason = "extends LinkedHashMap")
-  public LinkedHashMapWriter(int initialCapacity) {
-    super(initialCapacity);
-  }
-
  public LinkedHashMapWriter() {
    super();
  }


-
-  public static Predicate<CharSequence> dedupeKeyPredicate(Set<CharSequence> keys) {
-    return (k) -> keys.add(k);
-  }
 }


  }
-
-  @Deprecated // use SimpleOrderedMap.of()
-  MapWriter EMPTY = new MapWriterMap(Map.of());
 }


  /**
-   * Max query memory allocation value in mebibytes (float). If not set, or the value is &lt;= 0.0,
+   * Max query memory allocation value in megabytes (float). If not set, or the value is &lt;= 0.0,
   * there is no limit.


psalagnac · 2026-07-27T08:34:26Z

  public void testArrntvWriterOverridesAllWrites() {
    // List rather than Set because two not-overridden methods could share name but not signature
-    final List<String> methodsExpectedNotOverridden = new ArrayList<>(14);
+    final List<String> methodsExpectedNotOverridden = new ArrayList<>(13);


Minor: Why not just using the default constructor here? Mostly in test code, such a micro optimization does not make sense to me.
Plus the following lines add more than 13 items, so the array will have to grow anyway.

psalagnac · 2026-07-27T08:51:00Z

@@ -17,5 +17,5 @@
 package org.apache.solr.common;

 public interface Callable<T> {


I see a single internal class using this interface.
We could remove it and replace it by Java standard Consumer.

psalagnac · 2026-07-27T09:00:30Z

        List<String> paths, // a List of segments from the split paths
        String fieldName, // the fieldName assoc with this path
-        boolean multiValued, // flag if this fieldName is multiValued or not
+        // flag if this fieldName is multiValued or not


Comment to be removed

epugh added 4 commits July 24, 2026 14:28

Remove unused method param and excess logging.

9351344

Primarily removing extra static and final keywords that are not neede…

268fa62

…d on interfaces.

Unused classes in Solr now.

93ce94b

Remove unused methods, clean up java formatting, some typo fixes.

3b1f7d3

github-actions Bot added client:solrj tests cat:search cat:cloud labels Jul 26, 2026

epugh requested a review from Copilot July 26, 2026 12:42

Copilot started reviewing on behalf of epugh July 26, 2026 12:42 View session

Merge remote-tracking branch 'upstream/main' into review_oas_common_java

6dd27d5

epugh added the no-changelog label Jul 26, 2026

Copilot AI reviewed Jul 26, 2026

View reviewed changes

psalagnac reviewed Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Tidy up java code in o.a.s.common package - #4669

Tidy up java code in o.a.s.common package#4669
epugh wants to merge 5 commits into
apache:mainfrom
epugh:review_oas_common_java

epugh commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

psalagnac Jul 27, 2026

Uh oh!

psalagnac Jul 27, 2026

Uh oh!

psalagnac Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Can I be removed??? TODO
		private final Map<String, String> mdcContext;

		@@ -17,5 +17,5 @@
		package org.apache.solr.common;

		public interface Callable<T> {

Uh oh!

Conversation

epugh commented Jul 26, 2026

Description

Solution

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

psalagnac Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

psalagnac Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

psalagnac Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants