Skip to content

refactor: remove dead AbstractBody.writeNotRead() - #3196

Merged
predic8 merged 1 commit into
masterfrom
refactor/remove-dead-writenotread
Sep 4, 2026
Merged

refactor: remove dead AbstractBody.writeNotRead()#3196
predic8 merged 1 commit into
masterfrom
refactor/remove-dead-writenotread

Conversation

@predic8

@predic8 predic8 commented Sep 2, 2026

Copy link
Copy Markdown
Member

What

Removes AbstractBody.writeNotRead(), all six overrides and the test stub. Deletion only, 79 lines.

Why

Nothing has called it since 721428ca2 (2020-10-13, "introduction of MessageObserver.bodyChunk()").

Before that commit AbstractBody.write() chose between the two paths:

if (hasRelevantObservers()) {
    writeNotRead(out);
} else {
    writeStreamed(out);
    wasStreamed = true;
}

That commit dropped the branch — writeStreamed() handles both cases now — and write() has
dispatched only to writeStreamed() or writeAlreadyRead() ever since. writeNotRead() stayed
behind as an abstract method that every AbstractBody subclass had to implement, in three cases
with a full body that could never run.

Removed

File
http/AbstractBody.java the abstract declaration
http/Body.java override (22 lines)
http/ChunkedBody.java override (20 lines)
transport/http2/StreamInfo.java Http2Body override (20 lines)
http/XmlDomBody.java override, delegated to writeAlreadyRead()
http/EmptyBody.java empty override
interceptor/sse/ServerSentEventsDemoStreamInterceptor.java StreamingBody empty override
http/BodyTest.java stub in the anonymous AbstractBody

No helper is orphaned: StreamInfo's createByteArray() and removeDataFrame() are still called
from readLocal(), and Body's BUFFER_SIZE / ChunkedBody's readChunkSize(), readByteArray()
and readTrailer() are all still used by the live paths.

Verification

  • mvn -o -DskipTests install — BUILD SUCCESS across the whole reactor, so no caller existed in any
    module.
  • com.predic8.membrane.core.http — 293/293 green.
  • com.predic8.membrane.core.transport — 126/126 green (1 pre-existing skip).
  • No remaining reference to the name anywhere in the repo, and no reflective lookup of it.

ServerSentEventsDemoStreamInterceptor has no unit test (it is covered by
distribution/tutorials/misc/Server-Sent-Events.yaml); its StreamingBody overrides write()
directly, so it never reached any of these methods, and only an empty body was deleted.

No behaviour change — the removed code was unreachable.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Simplified body-writing behavior by removing the unused “not read” write path.
    • Body types now rely on their standard streaming or already-read handling when sending content.
    • Empty bodies use the default handling instead of silently ignoring this operation.

Nothing has called writeNotRead() since 721428c (2020-10-13, "introduction of
MessageObserver.bodyChunk()"). Before that commit, AbstractBody.write() picked
between writeNotRead() and writeStreamed() on hasRelevantObservers(); it dropped
the branch and now dispatches only to writeStreamed() or writeAlreadyRead(),
leaving writeNotRead() orphaned as an abstract method every AbstractBody
subclass still had to implement.

Removes the declaration, all six overrides (Body, ChunkedBody, XmlDomBody,
EmptyBody, StreamInfo.Http2Body, ServerSentEventsDemoStreamInterceptor's
StreamingBody) and the stub in BodyTest. No helper becomes unused: StreamInfo's
createByteArray() and removeDataFrame() are still called from readLocal(), and
XmlDomBody's writeNotRead() only delegated to writeAlreadyRead().

No behaviour change - the removed code was unreachable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 47a09602-2ba4-492c-afde-eaf69b4406af

📥 Commits

Reviewing files that changed from the base of the PR and between cfe7c76 and 7b72cf2.

📒 Files selected for processing (8)
  • core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java
  • core/src/main/java/com/predic8/membrane/core/http/Body.java
  • core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java
  • core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java
  • core/src/main/java/com/predic8/membrane/core/http/XmlDomBody.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/sse/ServerSentEventsDemoStreamInterceptor.java
  • core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java
  • core/src/test/java/com/predic8/membrane/core/http/BodyTest.java
💤 Files with no reviewable changes (8)
  • core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java
  • core/src/main/java/com/predic8/membrane/core/http/XmlDomBody.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/sse/ServerSentEventsDemoStreamInterceptor.java
  • core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java
  • core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java
  • core/src/main/java/com/predic8/membrane/core/http/Body.java
  • core/src/test/java/com/predic8/membrane/core/http/BodyTest.java
  • core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The change removes writeNotRead from AbstractBody and its overrides in body implementations, HTTP/2 stream handling, and test setup. Remaining write paths are unchanged. EmptyBody now uses the inherited implementation.

Changes

Body write-path simplification

Layer / File(s) Summary
Remove unread-body write overrides
core/src/main/java/com/predic8/membrane/core/http/*Body.java, core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java, core/src/test/java/com/predic8/membrane/core/http/BodyTest.java
AbstractBody no longer declares writeNotRead. Concrete overrides and the test helper override were removed. EmptyBody now inherits the default implementation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7b72c

This deletion-only refactor removes an unreachable method and its overrides without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit hops past code so neat
writeNotRead leaves its seat
The other paths still stream along
Tests shed one override song
Ears twitch, and builds march on

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the unused AbstractBody.writeNotRead() method.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/remove-dead-writenotread

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@predic8
predic8 requested a review from rrayst September 3, 2026 07:11
@predic8 predic8 added this to the 7.6.0 milestone Sep 4, 2026
@predic8
predic8 merged commit e588fd7 into master Sep 4, 2026
4 of 6 checks passed
@predic8
predic8 deleted the refactor/remove-dead-writenotread branch September 4, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants