Conversation
|
Copyright Validation Results ✅ Valid Files
✅ All files have valid copyright headers! |
There was a problem hiding this comment.
Pull request overview
This PR refactors request logging internals by moving InputStreamTee into RequestLoggerImpl, reflecting that it’s only used there and simplifying Polaris/thread-safety analysis.
Changes:
- Inline
InputStreamTeeas aprivate staticinner class inRequestLoggerImpl. - Remove the standalone
InputStreamTee.javaimplementation file. - Consolidate
java.ioimports inRequestLoggerImpl.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| marklogic-client-api/src/main/java/com/marklogic/client/impl/RequestLoggerImpl.java | Inlines InputStreamTee and updates imports. |
| marklogic-client-api/src/main/java/com/marklogic/client/impl/InputStreamTee.java | Removes the now-inlined class file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (sent >= max) return in.read(b, off, len); | ||
|
|
||
| return readTee(b, 0, in.read(b, off, len)); |
There was a problem hiding this comment.
In read(byte[] b, int off, int len), the call to readTee passes an offset of 0 instead of the provided 'off'. This causes the tee stream to write the wrong slice of the buffer (and can log stale bytes that precede 'off'). Pass 'off' through to readTee so tee.write uses the correct data.
| return readTee(b, 0, in.read(b, off, len)); | |
| return readTee(b, off, in.read(b, off, len)); |
| // Moved here in the 8.2 release to make it obvious it's only used by RequestLoggerImpl. It is only used by the | ||
| // copyContent method in RequestLoggerImpl. So while Polaris rightfully complains about the thread safety issues of | ||
| // this class, it is only used in a single thread context and so the thread safety issues are not a problem. | ||
| private static class InputStreamTee extends InputStream { | ||
| private InputStream in; |
There was a problem hiding this comment.
The newly added inner class uses tab indentation, while the rest of RequestLoggerImpl uses spaces. This makes the file inconsistent and can cause style/lint failures; please re-indent this block to match the existing spacing in the file.
No change here, just moving InputStreamTee into RequestLoggerImpl as that's the only place where it's used. And it makes it easier to see that the potential thread safety issues of InputStreamTee won't happen in practice.
No change here, just moving InputStreamTee into RequestLoggerImpl as that's the only place where it's used. And it makes it easier to see that the potential thread safety issues of InputStreamTee won't happen in practice.