Skip to content

[java] Remove deprecated GET /session/{sessionId}/se/files/{fileName} endpoint - #17982

Merged
diemol merged 3 commits into
trunkfrom
remove-deprecated-se-files-get
Sep 4, 2026
Merged

[java] Remove deprecated GET /session/{sessionId}/se/files/{fileName} endpoint#17982
diemol merged 3 commits into
trunkfrom
remove-deprecated-se-files-get

Conversation

@diemol

@diemol diemol commented Sep 4, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

Follow-up to #16844, which introduced this deprecation, and to the review discussion on #17968.

💥 What does this PR do?

Removes the deprecated GET /session/{sessionId}/se/files/{fileName} Grid Node endpoint and its client-side command (DriverCommand.GET_DOWNLOADED_FILE). #16844 (January 2026) marked it @Deprecated with a note to "Remove it in 4.41, 4.42 or 4.43." Trunk is now at 4.49.0-SNAPSHOT, six releases past that target.

What changed:

  • Node.java — removed the route registration for GET /session/{sessionId}/se/files/{fileName}.
  • LocalNode.java — removed the backward-compatibility dispatch branch and both extractFileName overloads that existed only to serve it.
  • DriverCommand.java — removed the @Deprecated GET_DOWNLOADED_FILE constant.
  • AbstractHttpCommandCodec.java — removed the corresponding command-codec entry and its now-unused import.
  • LocalNodeTest.java — removed extractsFileNameFromRequestUri, which only tested the removed method.

The three endpoints every client binding actually uses — GET /session/{sessionId}/se/files (list), POST /session/{sessionId}/se/files (download via JSON body), and DELETE /session/{sessionId}/se/files (delete) — are untouched.

🔧 Implementation Notes

Checked all four bindings (Python, Ruby, JavaScript, .NET): none of them ever built a request to the path-parameter GET form, they only use the three endpoints above (GET_DOWNLOADED_FILE was Java-only, unused since 4.40 per its own javadoc). So this removal has no cross-binding impact.

Verified with bazel test that NodeTest, LocalNodeTest, W3CHttpCommandCodecTest, and JdkHttpClientTest all still pass after the removal, and ran ./scripts/format.sh --pre-commit clean.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Claude Sonnet 5)
    • What was generated: an inventory of every reference to the deprecated endpoint across the repo (Java call sites, tests, docs, all four language bindings), and the resulting removal diff across the 5 files listed above.
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

None — this removes dead, already-deprecated code with an existing replacement (DOWNLOAD_FILE / POST) that all bindings already use.

🔄 Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)

… endpoint

Marked @deprecated in #16844 (Jan 2026) with a note to remove it in
Selenium 4.41, 4.42, or 4.43. Trunk is now at 4.49.0-SNAPSHOT, six
releases past that target, and none of the four client bindings ever
built a request to this path -- they only use the list/POST/DELETE
forms of /se/files.
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings labels Sep 4, 2026
diemol and others added 2 commits September 4, 2026 18:54
Spotbugs flagged it as an unused private method after the prior
commit removed the deprecated GET /se/files/{fileName} endpoint,
which was its only caller.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014kEM1tVUYCgzyhJ5xmseyF
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Removed route lacks regression test 📘 Rule violation ☼ Reliability
Description
The deprecated file-download route is removed without a test asserting that `GET
/session/{sessionId}/se/files/{fileName}` is rejected while the supported collection routes remain
available. This leaves the intended endpoint contract insufficiently protected against accidental
restoration or routing regressions.
Code

java/src/org/openqa/selenium/grid/node/Node.java[L174-176]

-            get("/session/{sessionId}/se/files/{fileName}")
-                .to(params -> new DownloadFile(this, sessionIdFrom(params)))
-                .with(spanDecorator("node.download_file")),
Evidence
PR Compliance ID 5 requires changed behavior to have appropriately scoped tests. The route is
removed in Node.java, while the PR deletes the helper-only filename extraction test and adds no
test asserting the removed route is unavailable.

AGENTS.md: Prefer Small Reliable Tests and Avoid Contract-Misrepresenting Mocks
java/src/org/openqa/selenium/grid/node/Node.java[171-176]
java/test/org/openqa/selenium/grid/node/NodeTest.java[808-908]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The removed `GET /session/{sessionId}/se/files/{fileName}` route has no replacement regression test verifying that requests to it are rejected.

## Issue Context
The existing download tests cover the supported list, POST download, and DELETE operations, but the behavior introduced by this PR—the removal of the path-parameter GET route—should also be covered by a small routing test.

## Fix Focus Areas
- java/test/org/openqa/selenium/grid/node/NodeTest.java[808-908]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is a localized but behavior-changing removal of a public Grid endpoint and client command, creating compatibility and routing risk that warrants a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread java/src/org/openqa/selenium/grid/node/Node.java
@diemol
diemol merged commit f3ccc60 into trunk Sep 4, 2026
48 checks passed
@diemol
diemol deleted the remove-deprecated-se-files-get branch September 4, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-grid Everything grid and server related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants