SOLR-18374: remove NamedList.NamedListEntry, scope MapWriterMap to its package - #4763
Open
serhiy-bzhezytskyy wants to merge 1 commit into
Conversation
…s package Two deprecated members, two different remedies - the annotations themselves say so. NamedListEntry adds nothing to AbstractMap.SimpleEntry but a constructor, so it is replaced at all four production sites at once. They have to move together: three of them create the array (DebugComponent, HighlightComponent, TermVectorComponent) and the fourth, SolrPluginUtils.copyNamedListIntoArrayByDocPosInResponse, is the single filler that stores into all three. Widening the array's component type is safe by construction here, and that was measured rather than assumed: planting the mismatch on purpose - array of SimpleEntry while the filler still stored NamedListEntry - left DistributedDebugComponentTest at 5/5, because NamedListEntry IS-A SimpleEntry. A throwing control at the same place failed all 5, so the path is genuinely exercised and the green result was substantive. After the change the runtime component type and the stored type match exactly at all three creators; the declared type stays Map.Entry<String, Object>[], so removeNulls is untouched. MapWriterMap is NOT removed. Its own annotation reads "May keep but use package scope", and it has exactly one caller in the tree, NavigableObject.wrap, in the same package - so the class and its constructor drop public and keep working. The @OverRide methods stay public because they implement MapWriter. This is a source- and binary-incompatible change for out-of-tree code that constructed it directly, which is why the changelog says scoped rather than removed. Verified: compileJava (solrj, core, core tests), ecjLintMain, spotlessJavaCheck, renderJavadoc, and DistributedDebugComponentTest, CustomHighlightComponentTest, DistributedTermsComponentTest, TestDistributedSearch - 8 tests, 0 failures, counted from the JUnit XML rather than trusted from BUILD SUCCESSFUL. SOLR-18373 (apache#4761) also touches NamedList.java, in a different region - simulated the merge by applying this diff on top of a scratch merge of apache#4761 into origin/main; it applies cleanly. AI-assisted (Claude Sonnet 5)
Contributor
Author
|
cc @dsmiley -- removes AI-assisted (Claude Sonnet 5) |
dsmiley
self-requested a review
August 20, 2026 00:45
dsmiley
approved these changes
Aug 20, 2026
Contributor
There was a problem hiding this comment.
TBH; these PRs you are working on, the changelog isn't necessary. Removing deprecated things generally don't need changelogs unless the thing in question is believed to be highly visible/used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-18374
Two members, two remedies, per each one's own deprecation note.
NamedList.NamedListEntryis fully removed — it added nothing overAbstractMap.SimpleEntry, so all 4 production sites move together (3 array creators, 1 filler that stores into them; widening the array's component type is safe by construction, confirmed by a planted mismatch that left tests green and a throwing control that failed them).MapWriterMapis NOT removed — its own note says "may keep but use package scope" — so the class and constructor droppublic; its one caller (NavigableObject.wrap) is already in the same package.4 tests across 4 changed classes, 0 failures. Compile + ecjLint + spotless + renderJavadoc clean on solrj and core.
SOLR-18373 (#4761) also touches NamedList.java, in a different region -- simulated the merge order and confirmed this patch still applies cleanly on top of it.
AI-assisted (Claude Sonnet 5)