SOLR-18248 : Migration of Tasks V2 APIs to JAX-RS Constructs

[jaykay12]

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

Description

Migrating 2 out of the 3 V2 APIs of Tasks, from existing homegrown annotations to JAX-RS Standard constructs & annotations

1. List Tasks API
2. Task Status API

Solution

1. Have exposed 1 new interface: ListActiveTasksApi which serves the /lists/task as well as /list/task/<taskId> endpoints.

2. ListActiveTasks is the main class which takes on the responsibility for v2 path as well as also for the v1 api path via ActiveTasksListHandler

3. 2 new DTO for List Task API

4. 1 new DTO for Task Status API

5. Removed the now deprecated files: ListActiveTasksAPI & ActiveTasksListComponent from the codebase.

No Coding Agent used.

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide
• I have added a changelog entry for my change

[epugh]

;-) Look slike you have some debugging?

[jaykay12]

yeah, started with firstly wiring this api to give any random response, & now using this to compare my V2 response with the original V2 response, 😄 thus, kept a different api endpoint for now to do response comparision.

will surely update this during self-review before raising for formal review 💯

[epugh]

Thanks for getting started on this, I went ahead and put some comments in that may be too early to be added ;-)

[epugh]

So, I think we would revamp this api... if you look at the desired api, ListActiveTasks would just list all tasks. If you wanted a specfific task, then it would be a idfferent end point with a /taskUUID.

[jaykay12]

yes, have updated this Api class for now, handling the taskUUID as a path param now, it was queryParam in the V1 as well as homegrown v2.

[epugh]

We are trying to move to more strongly typed responses so taht we don't have Map everywhere. Go ahead and specify out the return properties!

[jaykay12]

Yes, have done so.

[epugh]

One thing we are trying to do is move the V1 API logic into the V2 code base, and have the V1 call the V2 version, this way, as these apis evolve, we know that v1 and v2 stay in sync till we get rid of V1. I believe that ideally we would want ActiveTasksListComponent to delegate to the same code as this V2 api. Now, maybe it's so simple that both can just call core getCancellableQueryTracker().getActiveQueriesGenerated. In other V2 apis, you can see the code live in the V2 and then V1 calls it...

[jaykay12]

understood this part, have made suitable changes.

[epugh]

not quite sure I see the differences in the two paths yet?

[jaykay12]

yea, i am also checking this up.
I have tested this endpoint by running on cloud example which we have in examples, it worked fine. It should work fine for standalone as well, will remove the current conditional branching done once that's verified.

[jaykay12]

Thanks for getting started on this, I went ahead and put some comments in that may be too early to be added ;-)

thank you so much @epugh for the prompt reviews & initial guidance, much appreciated.

I will keep addressing your comments as we go, I kept this PR as draft for now as per our community guidelines, but yea thanks again for reviewing this at such initial state, so that i can course correct myself promptly. ✨

[jaykay12]

self review ✅
tests added ✅

[epugh]

this reads much nicer!

[epugh]

code comments addressed ✅

local verification of v1 & v2 APIs - ToBeAttached ❗

Here is an example of sort of the testing I am looking for. In the reference PR I found a couple of commands that exercised the code externally, just to get more confidence. If you do the equivalent with some curl commands, that would give me more confidence. Plus, I'll run them and learn some stuff! #4320 (comment)

[epugh]

@jaykay12 oh, we need to update the Reference Guide for any changes we've made in the V2 outputs as well ;-). If you hate doing that, I can do that ;-).

[jaykay12]

@epugh

Facing issues with the approach related to Nested Interfaces, API is not getting registered into Jersey & when trying to call V2 API it is throwing 404 error.

Post this comment, I am reverting back those changes. Not removing the code changes entirely, but commenting them for now.

Commit - d161485 - points to the code that is causing issue & what works fine.
Commit - 12e0a9c - commenting the remaining files so, that gradle build succeeds.

[jaykay12]

Test Evidences

How I verified that these changes work via local solr run

Started solr in local via these commands:

./gradlew dev 
export SOLR_OPTS="-Dsolr.security.allow.urls.enabled=false"
solr/packaging/build/dev/bin/solr start

Ran this dummy shard, which creates slow query tasks & thus, I can verify the V1 & V2 APIs. Took help of Gemini to figure this testing out

python3 -c "
import socket, time
server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
server.bind(('127.0.0.1', 9999))
server.listen(1)
print('📡 Raw socket listening on port 9999... Run your Solr query now!')

while True:
    conn, addr = server.accept()
    print('📥 Solr connected! Freezing connection for 2 seconds...')
    time.sleep(2)
    conn.close()
    print('🔌 Connection released.')
"

Actual Verification

Fired these URLs to create solr query tasks:

1. http://localhost:8983/solr/techproducts/select?q=*:*&shards=127.0.0.1:9999/solr/techproducts&canCancel=true&queryUUID=my-delayed-task-1
2. http://localhost:8983/solr/techproducts/select?q=*:*&shards=127.0.0.1:9999/solr/techproducts&canCancel=true&queryUUID=my-delayed-task-2
3. http://localhost:8983/solr/techproducts/select?q=*:*&shards=127.0.0.1:9999/solr/techproducts&canCancel=true&queryUUID=my-delayed-task-7

List Task APIs Endpoints:

Version	Main Branch	Feature Branch
V1 API	http://localhost:8983/solr/techproducts/tasks/list	http://localhost:8983/solr/techproducts/tasks/list
V1 Response
V2 API	http://localhost:8983/v2/collections/techproducts/tasks/list	http://localhost:8983/v2/collections/techproducts/tasks
V2 Response

Get Task Status APIs Endpoints:

Version	Main Branch	Feature Branch
V1 API	http://localhost:8983/solr/techproducts/tasks/list?taskUUID=my-delayed-task-2	http://localhost:8983/solr/techproducts/tasks/list?taskUUID=my-delayed-task-2
V1 Response
V2 API	http://localhost:8983/v2/collections/techproducts/tasks/list?taskUUID=my-delayed-task-2	http://localhost:8983/v2/collections/techproducts/tasks/my-delayed-task-2
V2 Response

[epugh]

@jaykay12 I got some AI help.... And there are some things that we need to improve. I'm going to try and summarize here, and then share the AI report below. I've gone through and edited/confirmed what the AI flagged!

1. For the V1 API, we need to return the old format "id: 25, status: inactive" for the status, even though it's terrible, so that we don't introduce a breaking change! I think I actually advocated with you to deal wiht it in v1, but now I am realizing we can't ;-(.

2. In the V1 approach we collected tasks from all shards by fanning out the checks, but we have dropped that in this PR. "Distributed regression — cross-shard task aggregation is silently dropped" is what is in the AI report below. It turns out that we never had a unit test that covered this situation, so I think we need to start with that.

Critical Issues

1. Distributed regression — cross-shard task aggregation is silently dropped

The deleted ActiveTasksListComponent had a handleResponses() method that aggregated task results across all shards in a distributed query. The new ActiveTask only queries the local core:

// Only checks this shard's CancellableQueryTracker
solrQueryRequest.getCore().getCancellableQueryTracker().isQueryIdActive(taskID)

In a distributed collection, a task running on shard 2 will be invisible to a client hitting shard 1's coordinator. The new implementation silently drops this cross-shard fan-out.

2. TaskStatus enum missing @JsonValue — serializes as uppercase

public enum TaskStatus {
    ACTIVE("active"),
    INACTIVE("inactive");
    private final String value;
    public String getValue() { return this.value; }
}

Without @JsonValue on getValue(), Jackson serializes by enum constant name: "ACTIVE" / "INACTIVE". The value field is unused at the wire level unless @JsonValue is added:

@JsonValue
public String getValue() { return this.value; }

3. V1 response format breaking change

Old ActiveTasksListComponent.handleResponses() returned a formatted string:

"id: 5, status: active"

New ActiveTasksListHandler.handleRequestBody() returns a boolean:

boolean taskStatus = taskStatusResponse.taskStatus.equals(ACTIVE);
rsp.add("taskStatus", taskStatus);  // true/false, not a string

This breaks existing v1 clients that parse the old string format. The new integration tests also cast to String and call .contains("active") — if this is actually returning a boolean, those tests would throw ClassCastException.

---

Design Issues

4. New tests may not be exercising the task status path

testCheckSpecificQueryStatus_Active and testCheckSpecificQueryStatus_Inactive set params.set("taskUUID", "5"), but ActiveTasksListHandler checks req.getParams().get(TASK_CHECK_UUID, null) where TASK_CHECK_UUID comes from CommonParams. Verify these are the same parameter name — if not, the tests fall through to the list-all-tasks path and queryResponse.get("taskStatus") will be null, not "active"/"inactive".

5. @JsonProperty names are camelCase, not kebab-case

Solr's JAX-RS model convention uses kebab-case property names (e.g. "task-id", "task-query"). The new models use camelCase:

@JsonProperty public String taskID;     // "taskID" on the wire
@JsonProperty public String taskQuery;  // "taskQuery" on the wire

Compare to e.g. SchemaDesignerResponse which uses @JsonProperty("configSet") with explicit strings. These should at minimum have explicit @JsonProperty("taskID") to make the contract clear, or be changed to @JsonProperty("task-id") to follow convention.

---

Minor Issues

7. Unnecessary no-arg constructor in ActiveTaskDetails

public ActiveTaskDetails() {}  // Jackson doesn't need this — fields are public

Remove it; the parameterized constructor is sufficient for test/internal use, and Jackson can access public fields directly.

8. Unrelated change in HealthCheckHandler.java

The removal of the Javadoc line about NodeHealth delegation appears unrelated to this PR. Worth either moving to a separate commit/PR or explaining why it's included here.

9. TasksApi interface — @StoreApiParameters on a @GET endpoint

Verify that @StoreApiParameters is appropriate for these read-only endpoints. It's used on both methods in TasksApi — check whether this annotation has side effects that are undesirable for a pure-read listing operation.

---

Test Coverage

• ActiveTaskTest.java covers the happy path for both methods with mocked deps — good baseline.
• Missing: test for listAllActiveTasks() when no tasks are active (empty iterator).
• Missing: test verifying the v1 handleRequestBody path actually returns the right format.
• The new TestTaskManagement tests (items 3 and 4 above) need review to confirm they're hitting the intended code paths.

---

Summary Table

Issue	Severity
Distributed cross-shard aggregation removed	High
@JsonValue missing on TaskStatus enum	High
V1 boolean vs. string response format break	High
New integration tests may use wrong param name	High
@JsonProperty names — camelCase vs. convention	Medium
Unrelated HealthCheckHandler Javadoc change	Low
Unnecessary ActiveTaskDetails() constructor	Low