SOLR-18139: Migrate to List.of() and forbid Collections.emptyList()

[epugh]

https://issues.apache.org/jira/browse/SOLR-18139

Description

Add Collections.emptyList() to forbidden API, and migrate.

Solution

Mostly find/replace and then a bunch of manual fixes. However, I do have failing tests because it turns out they are NOT the exact same. For example, this reproducing test fails:

./gradlew :solr:core:test --tests "org.apache.solr.cloud.TestConfigSetsAPI.testSingleFileUntrustedV2" "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=FB0721039D6B10C7 -Ptests.useSecurityManager=true -Ptests.file.encoding=UTF-8

I asked Claude for help:

The problem is in UploadConfigSet.java. When cleanup=false, filesToDelete is set to List.of(), but the code later calls filesToDelete.remove(filePath) for every zip entry. Here's the key behavioral difference:

• Collections.emptyList().remove(obj) → returns false silently ✅ (the iterator loop body never fires on an empty list, so iterator.remove() is never called)
• List.of().remove(obj) → throws UnsupportedOperationException ❌ (ImmutableCollections.AbstractImmutableList directly overrides remove(Object) to call uoe())

Therefore the fix is instead of using filesToDelete = List.of(); we have to use filesToDelete = new ArrayList<>();.

Here is the longer description:

What Changed and Why It Broke

The bug is in UploadConfigSet.java. Here's the problematic code:

    if (overwritesExisting && cleanup) {
      filesToDelete = configSetService.getAllConfigFiles(configSetName);
    } else {
      filesToDelete = new ArrayList<>();  // was List.of()
    }

    try (ZipInputStream zis = new ZipInputStream(requestBody, StandardCharsets.UTF_8)) {
      ...
      while ((zipEntry = zis.getNextEntry()) != null) {
        String filePath = zipEntry.getName();
        filesToDelete.remove(filePath);  // ← mutates the list!
        ...
      }
    }

The Critical Behavioral Difference

Collections.emptyList() and List.of() both claim to be "unmodifiable," but they differ in a subtle and important way when it comes to remove(Object o):

Operation	Collections.emptyList()	List.of()
remove(Object o)	Returns false silently ✅	Throws UnsupportedOperationException ❌

Why? Collections.emptyList() returns a Collections.EmptyList which inherits remove(Object o) from AbstractCollection. That method iterates the collection looking for the element—but since the list is empty, the iterator loop body never runs, so iterator.remove() is never called and no exception is thrown. It just returns false.

List.of() returns an ImmutableCollections.List0 whose parent class AbstractImmutableList directly overrides remove(Object o) to immediately call uoe() (throw UnsupportedOperationException), regardless of whether the list is empty.

Why Only This Code Path Triggered It

The filesToDelete.remove(filePath) call runs for every entry in the uploaded zip file. In the test, cleanup=false, so filesToDelete was set to the empty list — and as soon as the first zip entry was processed, remove() on List.of() threw the exception, returning a 500 to the client.

The Fix

Changed List.of() to new ArrayList<>() since the list needs to be mutable (items are removed from it as they're found in the zip). An empty ArrayList has the same no-op behavior of remove() when the element isn't present, but without throwing.

Tests

Existing

[epugh]

I just fixed the test org.apache.solr.search.join.BlockJoinNestedVectorsQParserTest.parentRetrievalFloat_topKWithChildTransformerWithFilter_shouldReturnBestChild with Claude help in afc083f:

Now I have the complete picture. Both QueryUtils.parseFilterQueries and AbstractVectorQParserBase.getLocalParamTags were changed from Collections.emptyList() to List.of() in the first commit. The combination breaks getFilterQuery in two ways:

1. globalFQs = parseFilterQueries(req) → now returns List.of() (immutable) when there are no fq params
2. The if (null != excludedGlobalFQTags) guard was designed for when getLocalParamTags returned null, but now it always returns List.of() — so the guard is always true and globalFQs.removeAll(...) is always called on the immutable List.of()

The fix needs two things in AbstractVectorQParserBase.getFilterQuery:

1. Ensure globalFQs is mutable from the start (new ArrayList<>())
2. Fix the null guard to a proper isEmpty() check

What Changed and Why It Broke

The first commit changed two files that interact in AbstractVectorQParserBase.getFilterQuery:

Change 1: QueryUtils.parseFilterQueries

-    return Collections.emptyList();
+    return List.of();

When a request has no fq parameters, globalFQs is now an immutable List.of().

Change 2: AbstractVectorQParserBase.getLocalParamTags

-      return Collections.emptyList();
+      return List.of();

When no excludeTags are in the local params, excludedGlobalFQTags is now List.of() instead of null.

The Explosion in getFilterQuery

The old code was written with the assumption that getLocalParamTags returned null when no tags were specified — hence the guard if (null != excludedGlobalFQTags). But getLocalParamTags was already returning Collections.emptyList() (not null!), so this null check was always true even before.

This was safe before because Collections.emptyList().removeAll(anything) is a harmless no-op — AbstractCollection.removeAll() iterates this looking for elements to remove, and since the list is empty, the loop body never fires. List.of().removeAll(anything) however immediately throws UnsupportedOperationException via AbstractImmutableCollection.removeAll() — even with an empty argument.

The Fix (two changes)

-      List<Query> globalFQs = QueryUtils.parseFilterQueries(req);
+      List<Query> globalFQs = new ArrayList<>(QueryUtils.parseFilterQueries(req));
 
       if (!includedGlobalFQTags.isEmpty()) {
         globalFQs = new ArrayList<>(QueryUtils.getTaggedQueries(req, includedGlobalFQTags));
       }
-      if (null != excludedGlobalFQTags) {
+      if (!excludedGlobalFQTags.isEmpty()) {
         globalFQs.removeAll(QueryUtils.getTaggedQueries(req, excludedGlobalFQTags));
       }

1. new ArrayList<>(...) wrapping: ensures globalFQs is always mutable, so removeAll can always be called on it even when parseFilterQueries returns an empty immutable list.
2. null != ... → !...isEmpty(): fixes the stale null-guard to its correct semantic meaning — "only remove excluded FQs if there are actually exclude tags specified." This is also a correctness improvement since skipping the getTaggedQueries call when there are no exclude tags is a minor optimization.

[Copilot]

Pull request overview

Migrates Solr’s Java codebase away from Collections.emptyList() to List.of() and updates forbidden-apis to ban Collections.emptyList() usage (SOLR-18139), with a targeted fix where an empty list must remain mutable during configset upload.

Changes:

• Add java.util.Collections#emptyList() to Solr’s forbidden APIs list (default message: “Use List.of()”).
• Replace many Collections.emptyList() / Collections.<T>emptyList() call sites with List.of() across core, solrj, modules, tests, and benchmarks.
• Fix a behavioral difference by using a mutable list (new ArrayList<>()) where removals occur during configset upload.

Reviewed changes

Copilot reviewed 129 out of 129 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
solr/test-framework/src/java/org/apache/solr/handler/component/TrackingShardHandlerFactory.java	Replace empty list return with List.of()
solr/solrj/src/test/org/apache/solr/common/util/TestFastJavabinDecoder.java	Replace default empty list with List.of()
solr/solrj/src/test/org/apache/solr/common/SolrDocumentTest.java	Replace Collections.emptyList() with List.of() in test
solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java	Return List.of() for null input instead of Collections.emptyList()
solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java	Replace emptyList() static import usage with List.of()
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java	Replace empty duplicates list with List.of()
solr/solrj/src/java/org/apache/solr/common/cloud/HashBasedRouter.java	Use List.of() for empty search slices
solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java	Use List.of() when partitions == 0
solr/solrj/src/java/org/apache/solr/common/cloud/CompositeIdRouter.java	Use List.of() when partitions == 0
solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java	Initialize empty fields list with List.of()
solr/solrj/src/java/org/apache/solr/client/solrj/response/FacetField.java	Use List.of() when values are null
solr/solrj/src/java/org/apache/solr/client/solrj/response/ClusteringResponse.java	Use List.of() for default empty lists
solr/solrj/src/java/org/apache/solr/client/solrj/response/Cluster.java	Replace empty list defaults with List.of()
solr/solrj/src/java/org/apache/solr/client/solrj/request/SolrQuery.java	Use List.of() when sorts are null
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java	Use List.of() when collection is null
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java	Replace empty operations lists with List.of()
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/DefaultZkCredentialsInjector.java	Return List.of() for no credentials
solr/solrj-streaming/src/test/org/apache/solr/client/solrj/io/stream/eval/MovingAverageEvaluatorTest.java	Assert against List.of() instead of Collections.emptyList()
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/eval/ListCacheEvaluator.java	Return List.of() when cache is missing
solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrTable.java	Replace multiple empty list args with List.of()
solr/modules/ltr/src/test/org/apache/solr/ltr/model/TestWrapperModel.java	Replace empty lists with List.of() in tests
solr/modules/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java	Use List.of() for empty feature/norm lists
solr/modules/langid/src/java/org/apache/solr/update/processor/LangDetectLanguageIdentifierUpdateProcessor.java	Return List.of() on exceptions
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java	Initialize redirect URIs with List.of()
solr/modules/extraction/src/java/org/apache/solr/handler/extraction/ExtractionMetadata.java	Return List.of() when metadata missing
solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java	Replace empty path list args with List.of()
solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java	Replace empty list argument with List.of()
solr/core/src/test/org/apache/solr/update/PeerSyncTest.java	Replace empty versions list with List.of()
solr/core/src/test/org/apache/solr/security/PathBasedCertPrincipalResolverTest.java	Replace empty values list with List.of()
solr/core/src/test/org/apache/solr/security/AllowListUrlCheckerTest.java	Replace empty allowlist inputs with List.of()
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKGEquiv.java	Replace expected empty lists with List.of()
solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetSKG.java	Replace expected empty list with List.of()
solr/core/src/test/org/apache/solr/search/ThinCache.java	Replace empty warmup list with List.of()
solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java	Replace absent params empty list with List.of()
solr/core/src/test/org/apache/solr/search/SignificantTermsQParserPluginTest.java	Replace empty component list with List.of()
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java	Replace empty languages list with List.of()
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java	Replace empty languages list with List.of()
solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java	Replace empty languages list with List.of()
solr/core/src/test/org/apache/solr/handler/component/PhrasesIdentificationComponentTest.java	Replace expected empty list with List.of()
solr/core/src/test/org/apache/solr/handler/component/ParallelHttpShardHandlerTest.java	Return List.of() from shutdownNow
solr/core/src/test/org/apache/solr/handler/admin/api/V2NodeAPIMappingTest.java	Return List.of() when body is null
solr/core/src/test/org/apache/solr/handler/admin/ZookeeperStatusHandlerTest.java	Pass List.of() for empty ZK response
solr/core/src/test/org/apache/solr/handler/admin/V2ApiMappingTest.java	Return List.of() when body is null
solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java	Return List.of() when payload is null
solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java	Return List.of() for empty commands
solr/core/src/test/org/apache/solr/handler/V2UpdateAPIMappingTest.java	Return List.of() for empty commands
solr/core/src/test/org/apache/solr/handler/V2ClusterAPIMappingTest.java	Return List.of() when body is null
solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerDiskOverFlow.java	Replace expected empty list with List.of()
solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCoreSnapshots.java	Return List.of() on IndexNotFoundException
solr/core/src/test/org/apache/solr/core/TestConfLoadPerf.java	Replace ctor empty list arg with List.of()
solr/core/src/test/org/apache/solr/cloud/VMParamsZkACLAndCredentialsProvidersTest.java	Replace empty credentials lists with List.of()
solr/core/src/test/org/apache/solr/cloud/UnloadDistributedZkTest.java	Use List.of() for null collection slices
solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java	Return List.of() in mocked retry loop
solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java	Use List.of() as default collection for response paths
solr/core/src/test/org/apache/solr/cli/PostToolTest.java	Replace robots empty list with List.of()
solr/core/src/java/org/apache/solr/util/ModuleUtils.java	Use List.of() default for module list sysprop/env
solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java	Return List.of() for no child resources
solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessorChain.java	Return List.of() when processor is null
solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java	Return List.of() when template resolution missing
solr/core/src/java/org/apache/solr/update/processor/FieldMutatingUpdateProcessorFactory.java	Replace empty selector collections with List.of()
solr/core/src/java/org/apache/solr/update/processor/DocBasedVersionConstraintsProcessorFactory.java	Initialize empty param names with List.of()
solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java	Set empty child list value using List.of()
solr/core/src/java/org/apache/solr/update/processor/AddSchemaFieldsUpdateProcessorFactory.java	Initialize typeMappings with List.of()
solr/core/src/java/org/apache/solr/spelling/WordBreakSolrSpellChecker.java	Replace empty suggestion lists with List.of()
solr/core/src/java/org/apache/solr/spelling/SuggestQueryConverter.java	Return List.of() when original is null
solr/core/src/java/org/apache/solr/spelling/SpellingQueryConverter.java	Return List.of() when original is null
solr/core/src/java/org/apache/solr/spelling/DirectSolrSpellChecker.java	Replace empty suggestion lists with List.of()
solr/core/src/java/org/apache/solr/spelling/ConjunctionSolrSpellChecker.java	Replace empty suggestion lists with List.of()
solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java	Replace empty suggestion lists with List.of()
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java	Replace empty collections/commands list defaults with List.of()
solr/core/src/java/org/apache/solr/security/cert/PathBasedCertResolverBase.java	Default filter values list to List.of()
solr/core/src/java/org/apache/solr/security/AllowListUrlChecker.java	Use List.of() for allow-all checker construction
solr/core/src/java/org/apache/solr/search/vector/AbstractVectorQParserBase.java	Ensure mutable global FQ list; use List.of() for tag defaults
solr/core/src/java/org/apache/solr/search/grouping/distributed/command/TopGroupsFieldCommand.java	Return List.of() when no groups
solr/core/src/java/org/apache/solr/search/grouping/distributed/command/SearchGroupsFieldCommand.java	Default empty topGroups to List.of()
solr/core/src/java/org/apache/solr/search/function/distance/GeoDistValueSourceParser.java	Pass List.of() to super for empty sources
solr/core/src/java/org/apache/solr/search/facet/SlotAcc.java	Return List.of() when no sweep structs
solr/core/src/java/org/apache/solr/search/facet/FacetMerger.java	Replace empty list returns with List.of()
solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessor.java	Replace empty list cast fallback with List.of()
solr/core/src/java/org/apache/solr/search/SortedIntDocSet.java	Return List.of() for child resources
solr/core/src/java/org/apache/solr/search/SortSpecParsing.java	Build empty SortSpec using List.of()
solr/core/src/java/org/apache/solr/search/QueryUtils.java	Return List.of() when no filter queries
solr/core/src/java/org/apache/solr/search/DocSlice.java	Return List.of() for child resources
solr/core/src/java/org/apache/solr/search/BitDocSet.java	Return List.of() for child resources
solr/core/src/java/org/apache/solr/schema/PointField.java	Return List.of() when field unused
solr/core/src/java/org/apache/solr/schema/IndexSchema.java	Return List.of() when no copy sources
solr/core/src/java/org/apache/solr/schema/FieldType.java	Use List.of() for no created fields
solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java	Return List.of() on null shape
solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java	Default empty commands list to List.of()
solr/core/src/java/org/apache/solr/request/SimpleFacets.java	Replace empty tag list default with List.of()
solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java	Use List.of() when no path elements
solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java	Return List.of() when no field values
solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java	Return List.of() for empty iterator result
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettingsDAO.java	Store default languages list as List.of()
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettings.java	Default/set languages to List.of() when null
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java	Use List.of() for empty collections/languages/sample values
solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java	Return List.of() when no diffs
solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java	Use mutable ArrayList instead of immutable empty list for deletions
solr/core/src/java/org/apache/solr/handler/component/StatsInfo.java	Return List.of() when no tagged stats
solr/core/src/java/org/apache/solr/handler/component/StatsField.java	Replace empty tag/exclude lists with List.of()
solr/core/src/java/org/apache/solr/handler/component/StandaloneReplicaSource.java	Return List.of() for non-cloud slice names
solr/core/src/java/org/apache/solr/handler/component/ShardHandlerFactory.java	Use List.of() for default PluginInfo children
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java	Replace empty versions/ids list defaults with List.of()
solr/core/src/java/org/apache/solr/handler/component/RangeFacetProcessor.java	Initialize empty range facet requests with List.of()
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java	Replace empty elevated/excluded ID lists with List.of()
solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java	Replace empty lists with List.of() throughout pivot parsing
solr/core/src/java/org/apache/solr/handler/component/PivotFacetFieldValueCollection.java	Return List.of() when no refinable values
solr/core/src/java/org/apache/solr/handler/component/PivotFacet.java	Replace empty refinements/pivots lists with List.of()
solr/core/src/java/org/apache/solr/handler/component/PhrasesIdentificationComponent.java	Return List.of() for empty details payload
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java	Replace empty facet lists/tags with List.of()
solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java	Return List.of() when slice missing
solr/core/src/java/org/apache/solr/handler/admin/api/ReplicationAPIBase.java	Use List.of() when file list is absent
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java	Use List.of() for null paramNames
solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java	Build ResponseBuilder with List.of()
solr/core/src/java/org/apache/solr/handler/IndexFetcher.java	Use List.of() for “no files” cases/returns
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java	Return List.of() on missing ZK nodes
solr/core/src/java/org/apache/solr/core/snapshots/CollectionSnapshotMetaData.java	Default replica snapshots list to List.of()
solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java	Return List.of() when config string empty
solr/core/src/java/org/apache/solr/core/SolrCore.java	Return List.of() for empty plugin lists
solr/core/src/java/org/apache/solr/core/SolrConfig.java	Return List.of() when no plugin infos
solr/core/src/java/org/apache/solr/core/PluginInfo.java	Use List.of() for null/empty children lists
solr/core/src/java/org/apache/solr/core/NodeConfig.java	Default allowUrls list to List.of()
solr/core/src/java/org/apache/solr/core/CoreSorter.java	Replace empty list returns with List.of() and update comment wording
solr/core/src/java/org/apache/solr/cluster/placement/plugins/OrderedNodePlacementPlugin.java	Replace Collections::emptyList with List::of
solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java	Replace empty action lists with List.of()
solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java	Replace empty action list with List.of()
solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java	Return List.of() on missing configs znode
solr/benchmark/src/java/org/apache/solr/bench/generators/RandomDataHistogram.java	Replace empty list comparisons with List.of()
gradle/validation/forbidden-apis/java.solr.txt	Forbid Collections.emptyList() and steer to List.of()

---

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

[Copilot]

Collections.emptyList() is now forbidden, but there are still remaining call sites using the generic form Collections.<T>emptyList() (e.g., solr/core/src/test/org/apache/solr/legacy/TestLegacyNumericUtils.java around the inverse-range assertions). With this forbidden-apis rule enabled, those remaining usages will fail the build; please migrate them to List.of() (or another appropriate empty list) as part of this PR.

[epugh]

Good catch, so turns out this needs to be in defaults.all.txt.

[epugh]

unit tests all now pass locally.

[dsmiley]

I reviewed every single line. Phew!
Just one thing to change.

Thanks for doing this, and I look forward to the saem for emptyMap & emptySet

[dsmiley]

copy isn't needed anymore

[dsmiley]

I love seeing the conciseness cut downs on lines, as seen here

[dsmiley]

you know emptyMap is the next target Eric :-)
In fact you could reasonably increase the scope of the JIRA issue to cover the others.