SOLR-18106: remove SolrQueryRequest.getCloudDescriptor() #4114

epugh · 2026-02-07T12:56:14Z

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

Description

Deprecated method

Solution

Simple inline of code call.

Tests

All pass

Checklist

Copilot

Pull request overview

This PR aims to eliminate usage of the deprecated SolrQueryRequest#getCloudDescriptor() method by inlining the underlying call chain.

Changes:

Removes the deprecated getCloudDescriptor() default method from SolrQueryRequest.
Updates DelegatingSolrQueryRequest#getCloudDescriptor() to inline the call chain instead of delegating.
Replaces req.getCloudDescriptor() usage in HttpShardHandler with the inlined call chain.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java`	Deletes the deprecated `getCloudDescriptor()` method from the public request interface.
`solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java`	Stops overriding/delegating the removed interface method and inlines the call chain.
`solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java`	Replaces deprecated `req.getCloudDescriptor()` call with an inlined `req.getCore().getCoreDescriptor().getCloudDescriptor()` chain.

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

Copilot · 2026-02-07T13:06:14Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

  default CoreContainer getCoreContainer() {
    SolrCore core = getCore();
    return core == null ? null : core.getCoreContainer();
  }

-  /**
-   * @deprecated use getCore().getCoreDescriptor().getCloudDescriptor()
-   */
-  @Deprecated
-  default CloudDescriptor getCloudDescriptor() {
-    return getCore().getCoreDescriptor().getCloudDescriptor();
-  }
-
  /** The writer to use for this request, considering {@link CommonParams#WT}. Never null. */
  default QueryResponseWriter getResponseWriter() {


Removing SolrQueryRequest#getCloudDescriptor() is a source/binary incompatible API change on a public interface and also breaks current in-repo implementations that still override it (e.g., CoordinatorHttpSolrCall has an @OverRide getCloudDescriptor method). If the goal is only to eliminate deprecated call sites, keep the deprecated default method and update callers; if the goal is full removal, all remaining overrides/callers (including CoordinatorHttpSolrCall) must be updated in this PR.

thanks, I followed up on this.

Copilot · 2026-02-07T13:06:14Z

solr/core/src/java/org/apache/solr/request/DelegatingSolrQueryRequest.java


-  @Override
  public CloudDescriptor getCloudDescriptor() {
-    return delegate.getCloudDescriptor();
+    return delegate.getCore().getCoreDescriptor().getCloudDescriptor();
  }


DelegatingSolrQueryRequest still declares getCloudDescriptor(), but SolrQueryRequest no longer defines this method. This leaves a surprising extra public API on a class described as delegating the SolrQueryRequest interface. Either remove this method (and its CloudDescriptor import) or formally keep it as part of SolrQueryRequest (deprecated) so the delegation contract remains consistent.

I think I sorted this!

Copilot · 2026-02-07T13:06:15Z

solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java


    CoreDescriptor coreDescriptor = req.getCore().getCoreDescriptor();
-    CloudDescriptor cloudDescriptor = req.getCloudDescriptor();
+    CloudDescriptor cloudDescriptor = req.getCore().getCoreDescriptor().getCloudDescriptor();


This line duplicates the already-computed coreDescriptor and repeats req.getCore().getCoreDescriptor(). Prefer using the existing local variable (coreDescriptor.getCloudDescriptor()) to avoid redundant calls and keep the code consistent with the surrounding style.

Suggested change

CloudDescriptor cloudDescriptor = req.getCore().getCoreDescriptor().getCloudDescriptor();

CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor();

Copilot · 2026-02-07T13:06:15Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

  default CoreContainer getCoreContainer() {
    SolrCore core = getCore();
    return core == null ? null : core.getCoreContainer();
  }

-  /**
-   * @deprecated use getCore().getCoreDescriptor().getCloudDescriptor()
-   */
-  @Deprecated
-  default CloudDescriptor getCloudDescriptor() {
-    return getCore().getCoreDescriptor().getCloudDescriptor();
-  }
-
  /** The writer to use for this request, considering {@link CommonParams#WT}. Never null. */
  default QueryResponseWriter getResponseWriter() {


PR description says this is a "simple inline" of a deprecated call, but this diff also removes the deprecated method from SolrQueryRequest entirely. If method removal is intended, the PR description (and any deprecation/removal policy docs/release notes) should reflect that broader API change.

dsmiley · 2026-02-07T14:31:08Z

See JIRA issue

epugh · 2026-02-07T16:31:12Z

I'm going to reopen this so you can see the furthur commits, and then if we decide not to move forward then I'll remove the deprecation tag and close it...

epugh · 2026-02-10T20:45:40Z

@dsmiley thanks for the better title! Is there some sort of investigation that needs to be done before this can be merged? We can leave it open obviously for a few days, but is there anyone who we specifically need to chase to review this?

dsmiley · 2026-02-10T20:48:40Z

@patsonluk for your review please

patsonluk · 2026-02-11T02:15:17Z

Thanks @dsmiley ! I will take a look tomorrow. I see the concern you all discussed in https://issues.apache.org/jira/browse/SOLR-18106?focusedCommentId=18057106&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-18057106

Need to revisit to code and see whether the absence of modified CloudDescriptor would still work properly for coordinator node setup( I believe the core in the req for coordinator node would not have the correct info, but i don't quite remember all the details 😓 )

patsonluk · 2026-02-11T18:54:00Z

TL;DR:
Yes. We probably need some additional change in order to have CoordinatorHttpSolrCall to work with this change

Details:
By debugging, it's shown this change right now would cause some issues in HttpShardHandler#prepDistributed as suspected.

This is because CoordinatorHttpSolrCall only wraps the request by providing an overriding getCloudDescriptor with info of the actual core (with collection name, replica type and core shard ID replaced with the actual core info, instead of the synthetic core). Which means with this current change, getting CloudDescriptor from the CoreDescroptor would have returned the synthetic core with incorrect collection name (of the Synthetic core/collection)

Perhaps in CoordinatorHttpSolrCall, we will need to instead override getCore to return a wrapped SolrCore and CoreDescriptor that its getCloudDescriptor would return the modified cloud description from the coordinator code?

Not sure if I missed out any obvious details here 😓

epugh · 2026-02-11T19:03:23Z

TL;DR: Yes. We probably need some additional change in order to have CoordinatorHttpSolrCall to work with this change

Details: By debugging, it's shown this change right now would cause some issues in HttpShardHandler#prepDistributed as suspected.

This is because CoordinatorHttpSolrCall only wraps the request by providing an overriding getCloudDescriptor with info of the actual core (with collection name, replica type and core shard ID replaced with the actual core info, instead of the synthetic core). Which means with this current change, getting CloudDescriptor from the CoreDescroptor would have returned the synthetic core with incorrect collection name (of the Synthetic core/collection)

Perhaps in CoordinatorHttpSolrCall, we will need to instead override getCore to return a wrapped SolrCore and CoreDescriptor that its getCloudDescriptor would return the modified cloud description from the coordinator code?

Not sure if I missed out any obvious details here 😓

Can we make sure to add a test that fails with this change? I worry as I do refactoring, that without David's sharp eyes, something like this would be merged based on the tests passing ;-).

@patsonluk I hope you have some time to get the changes made, feel free to push to this branch. I'm really pushing hard to remove deprecated code from Solr, and this is one of them!

dsmiley · 2026-02-11T19:16:51Z

+100 to the test.

And I confess (again) that this deprecation of mine may be premature... there's no clear substitute right now. Your idea of returning a getCore with a custom core descriptor is a plausible solution!

inline deprecated call

9a20944

epugh added the no-changelog label Feb 7, 2026

epugh requested review from Copilot and dsmiley February 7, 2026 12:56

github-actions bot added the cat:search label Feb 7, 2026

Copilot started reviewing on behalf of epugh February 7, 2026 12:56 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

dsmiley closed this Feb 7, 2026

epugh added 2 commits February 7, 2026 10:40

Refactoring suggested by Claude, will post summary.

8b086b4

Remove unused variables

5331475

epugh reopened this Feb 7, 2026

github-actions bot added jetty-server cat:api labels Feb 7, 2026

epugh added 2 commits February 7, 2026 11:33

respond to copilot suggestion

aaa01c1

Merge remote-tracking branch 'upstream/main' into SOLR-18106

9430750

dsmiley changed the title ~~SOLR-18106: inline deprecated call~~ SOLR-18106: remove SolrQueryRequest.getCloudDescriptor() Feb 9, 2026

	CloudDescriptor cloudDescriptor = req.getCore().getCoreDescriptor().getCloudDescriptor();
	CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor();

SOLR-18106: remove SolrQueryRequest.getCloudDescriptor() #4114

Are you sure you want to change the base?

SOLR-18106: remove SolrQueryRequest.getCloudDescriptor() #4114

Conversation

epugh commented Feb 7, 2026

Description

Solution

Tests

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Feb 7, 2026

Uh oh!

epugh commented Feb 7, 2026

Uh oh!

epugh commented Feb 10, 2026

Uh oh!

dsmiley commented Feb 10, 2026

Uh oh!

patsonluk commented Feb 11, 2026

Uh oh!

patsonluk commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epugh commented Feb 11, 2026

Uh oh!

dsmiley commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

patsonluk commented Feb 11, 2026 •

edited

Loading