feature/FilterPlugin

[LinusWestling]

Closes #15

@coderabbitai ignore

Summary by CodeRabbit

• New Features

  • Introduced an HTTP filter pipeline and chainable filters for request/response interception
  • Added an extensible plugin mechanism as the final request handler
  • Added an HTTP request parser and request/response data types
  • Added a logging filter that records HTTP method and path

• Documentation

  • Guides for creating and registering filters and plugins, and pipeline execution flow

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a filter/plugin pipeline, HTTP parsing, and integrates them into the socket server: new Pipeline, Filter/FilterChain, Plugin interfaces and examples, HttpParser/HttpRequest/HttpResponse scaffolding, and SocketServer updated to parse requests and run the filter chain before delegating to a plugin.

Changes

Cohort / File(s)	Summary
Pipeline & Server src/main/java/org/example/Pipeline.java, src/main/java/org/example/SocketServer.java, src/main/java/org/example/App.java	Adds Pipeline to register filters and a plugin; SocketServer.handleClient signature changed to accept a Pipeline, now parses requests, builds a FilterChain and executes it. App instantiates Pipeline, registers LoggingFilter and HelloPlugin (server start left commented).
Filter framework src/main/java/org/example/filter/Filter.java, src/main/java/org/example/filter/FilterChain.java, src/main/java/org/example/filter/LoggingFilter.java, src/main/java/org/example/filter/README.md	New Filter interface, FilterChain implementation that iterates filters then calls Plugin, a LoggingFilter implementation, and documentation describing filter lifecycle and registration.
HTTP layer src/main/java/org/example/http/HttpParser.java, src/main/java/org/example/http/HttpRequest.java, src/main/java/org/example/http/HttpResponse.java	Adds HttpParser to parse request line, headers, and body; HttpRequest record for request data; HttpResponse placeholder class for response construction.
Plugin system src/main/java/org/example/plugin/Plugin.java, src/main/java/org/example/plugin/HelloPlugin.java, src/main/java/org/example/plugin/README.md	Adds Plugin interface and HelloPlugin stub (no-op response body commented); README documents plugin responsibilities and integration.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Server as SocketServer
    participant Parser as HttpParser
    participant Pipeline as Pipeline
    participant Chain as FilterChain
    participant Filter as LoggingFilter
    participant Plugin as HelloPlugin
    participant Response as HttpResponse

    Client->>Server: TCP connection with HTTP bytes
    Server->>Parser: parse(InputStream)
    Parser-->>Server: HttpRequest
    Server->>Pipeline: pipeline.createChain()
    Server->>Chain: doFilter(HttpRequest, HttpResponse)
    Chain->>Filter: doFilter(req, res, chain)
    Filter->>Filter: log method & path
    Filter->>Chain: chain.doFilter(req, res)
    Chain->>Plugin: handle(req, res)
    Plugin->>Response: set status/body (placeholder)
    Chain-->>Server: chain complete
    Server->>Client: write response bytes / flush

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped a path of code today,
Filters checked the light and way,
A plugin waved, the chain ran through,
Tiny logs and "Hello" too —
Hopped by a happy rabbit!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'feature/FilterPlugin' is partially related to the changeset; it indicates this is about filters and plugins, but is vague and doesn't clearly summarize the main architectural changes being introduced.	Consider a more descriptive title like 'Implement filter and plugin architecture' or 'Add FilterChain, Filter, and Plugin interfaces with Pipeline orchestration' to better summarize the key changes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR successfully implements all core requirements from issue #15: Filter interface with doFilter signature, Plugin interface with handle signature, Pipeline registration system, FilterChain orchestration, comprehensive documentation, and working example (LoggingFilter and HelloPlugin).
Out of Scope Changes check	✅ Passed	All changes are directly aligned with issue #15 objectives; no out-of-scope modifications detected. The PR introduces filter/plugin architecture, interfaces, implementations, and documentation as specified.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/FilterPlugin

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/org/example/SocketServer.java (1)

23-26: ⚠️ Potential issue | 🔴 Critical

Compilation error: handleClient call arity mismatch.

Line 25 calls handleClient(socket) with one argument, but the method signature on line 33 requires two: handleClient(Socket socket, Pipeline pipeline). This will not compile.

createSocket needs to accept a Pipeline parameter and forward it:

Proposed fix

-static void createSocket() {
+static void createSocket(Pipeline pipeline) {
     int port = 3000;
 
     try (ServerSocket serverSocket = new ServerSocket(port, 64)) {
 
         System.out.println("Server started at port: " + serverSocket.getLocalPort());
 
         while (true) {
             Socket socket = serverSocket.accept();
-            Thread.ofVirtual().start(() -> handleClient(socket));
+            Thread.ofVirtual().start(() -> handleClient(socket, pipeline));
         }
 
     } catch (IOException e) {
         throw new RuntimeException(e);
     }
 }

🤖 Fix all issues with AI agents

In `@src/main/java/org/example/App.java`:
- Around line 7-16: SocketServer.createSocket() currently blocks in its accept
loop, so construct and configure the Pipeline (new Pipeline(),
pipeline.addFilter(...), pipeline.setPlugin(...)) before starting the server and
pass that Pipeline into the server start method; change
SocketServer.createSocket() to accept a Pipeline parameter (e.g.,
createSocket(Pipeline pipeline)) and ensure it forwards that pipeline into
handleClient (or into startServer) so handleClient uses the configured Pipeline
instead of leaving pipeline setup unreachable.

In `@src/main/java/org/example/filter/FilterChain.java`:
- Around line 20-27: The doFilter method in FilterChain calls plugin.handle(...)
unguarded which will NPE if plugin is null; update FilterChain to either enforce
a non-null plugin in the constructor/setter (e.g., validate in
Pipeline.setPlugin or FilterChain constructor) or add a null-check in
FilterChain.doFilter that throws a clear IllegalStateException (or returns a 500
response) when plugin is missing before calling plugin.handle; also fix the
inline comment to say "pass requests through all the filters" instead of "pass
https". Reference: FilterChain.doFilter, plugin, Pipeline.setPlugin.

In `@src/main/java/org/example/filter/README.md`:
- Around line 27-38: Update the README paths: change the package path suggestion
from "src/main/java/.../filters/" to use the singular "filter" (e.g.,
src/main/java/.../filter/) and correct the App file path from
"src/org.example/App.java" to the proper filesystem path
"src/main/java/org/example/App.java"; keep the references to implementing the
Filter interface, implementing doFilter, and registering the filter with
Pipeline (e.g., Pipeline pipeline = new Pipeline(); pipeline.addFilter(new
LoggingFilter());) unchanged.

In `@src/main/java/org/example/http/HttpParser.java`:
- Around line 68-84: The readLine method reads raw bytes and casts each byte to
char ((char) b), which corrupts multi-byte UTF-8 sequences; update readLine in
HttpParser to accumulate raw bytes (e.g., into a ByteArrayOutputStream or byte
buffer) until CR/LF, then convert the full byte sequence to a String using the
correct Charset (StandardCharsets.UTF_8) instead of per-byte casting; ensure the
same CR/LF handling (skip '\r', stop at '\n') is preserved and return null on
EOF with empty buffer as before.

In `@src/main/java/org/example/http/HttpResponse.java`:
- Around line 3-5: HttpResponse is currently empty and must be implemented so
HelloPlugin.handle() and SocketServer can compile; add fields for int status,
Map<String,String> headers, and String body with getters/setters (e.g.,
setStatus, setBody, setHeader/addHeader, getHeaders), ensure default status 200
and sensible defaults for headers, and implement a serialization method (e.g.,
toByteArray() or writeTo(OutputStream)) that builds an HTTP response
string/bytes including status line, headers (including Content-Length), a blank
line, and the body so SocketServer can write it to the socket; reference
HttpResponse class and HelloPlugin.handle() for expected method names and
SocketServer for the serializer usage.

In `@src/main/java/org/example/Pipeline.java`:
- Around line 22-24: The createChain method currently passes the mutable filters
list and a possibly-null plugin into new FilterChain, causing NPEs and
concurrent-mutation problems; change createChain to pass a defensive copy of
filters (e.g., new ArrayList<>(filters)) and ensure plugin is non-null by
substituting a default no-op implementation (e.g., a NoOpPlugin that implements
Plugin and does nothing) when plugin == null (or alternatively throw a clear
IllegalStateException); update references to createChain, FilterChain,
setPlugin, addFilter, filters, and plugin accordingly.

In `@src/main/java/org/example/plugin/README.md`:
- Line 22: The README path is inconsistent: it refers to
"src/main/java/.../plugins/" but the code uses package org.example.plugin;
update the README text to reference "src/main/java/.../plugin/" (singular) and
any example package paths to match org.example.plugin so readers create classes
under the correct package; search for occurrences of "plugins/" in the README
and replace them with "plugin/" to keep the documentation aligned with the
package name.
- Around line 48-52: Update the README example: correct the file path to use
src/main/java/org/example/App.java and fix the Java instantiation in the snippet
by calling the constructor with parentheses when registering the plugin (replace
pipeline.setPlugin(new HelloPlugin) with pipeline.setPlugin(new HelloPlugin()));
ensure the snippet references Pipeline and pipeline.setPlugin as shown and that
HelloPlugin() is used as the instantiated class.

In `@src/main/java/org/example/SocketServer.java`:
- Around line 38-48: HttpResponse is a stub but SocketServer and plugins expect
setStatus(int), setBody(String) and toBytes() to produce a real HTTP response;
implement HttpResponse with private fields (int statusCode, String body,
Map<String,String> headers), provide public setStatus(int) and setBody(String)
(and optionally setHeader), and implement toBytes() to serialize a valid
HTTP/1.1 response: build the status line ("HTTP/1.1 {statusCode}
{reasonPhrase}"), ensure required headers (Content-Length, Content-Type default
"text/plain; charset=utf-8", plus any headers map entries), join headers with
CRLF, add a blank CRLF then the body, and return the whole string encoded as
UTF-8 bytes; keep reasonable defaults (200/OK) and include a small helper to map
common status codes to reason phrases so SocketServer and filter plugins
compiling calls to res.setStatus()/res.setBody()/res.toBytes() succeed.

🧹 Nitpick comments (3)

src/main/java/org/example/http/HttpRequest.java (1)

5-12: Record with mutable Map and byte[] fields is not deeply immutable.

Java records auto-generate equals/hashCode using field references. For byte[], this means content-based equality won't work. More importantly, if a caller retains a reference to the headers map or body array, they can mutate the record's state after construction — risky when the same request flows through multiple filters.

Consider a compact constructor that makes defensive copies:

♻️ Defensive-copy constructor

 public record HttpRequest(
         String method,
         String path,
         String queryString,
         String httpVersion,
         Map<String, String> headers,
         byte[] body
-) {}
+) {
+    public HttpRequest {
+        headers = headers != null ? Map.copyOf(headers) : Map.of();
+        body = body != null ? body.clone() : new byte[0];
+    }
+}

src/main/java/org/example/plugin/HelloPlugin.java (1)

8-16: Blocked on HttpResponse implementation — setStatus and setBody don't exist yet.

The developer-acknowledged TODO on Line 13 confirms this. Once HttpResponse is fleshed out (see review comment on that file), this will work. No further issues with the plugin logic itself.

Also, minor style: missing space before { on Line 8 (implements Plugin{).

src/main/java/org/example/filter/FilterChain.java (1)

15-18: Defensively copy the filters list to prevent external mutation.

If the caller (e.g., Pipeline.createChain()) passes a mutable list and later modifies it, the chain's iteration could be corrupted mid-request.

♻️ Proposed fix

     public FilterChain(List<Filter> filters, Plugin plugin) {
-        this.filters = filters;
+        this.filters = List.copyOf(filters);
         this.plugin = plugin;
     }

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/main/java/org/example/plugin/HelloPlugin.java`:
- Around line 8-20: HelloPlugin currently does nothing because HttpResponse is
not initialized and handle() is commented out; update the pipeline so
HelloPlugin.handle(HttpRequest req, HttpResponse res) can set a real response:
ensure SocketServer (or the code that constructs/dispatches to Plugin.handle)
initializes an HttpResponse instance before calling Plugin.handle, and flesh out
HttpResponse with setStatus(int) and setBody(String) (and any minimal fields
like statusCode/body) so HelloPlugin can call res.setStatus(200) and
res.setBody("Hello from plugin!"); also fix the minor style by adding a space in
the class declaration (change "implements Plugin{" to "implements Plugin {").

In `@src/main/java/org/example/plugin/README.md`:
- Around line 31-45: The RouterPlugin example's handle method signature doesn't
match the Plugin interface (which declares throws IOException); update the
RouterPlugin class by changing the handle(HttpRequest req, HttpResponse res)
method to declare throws IOException so its signature matches the Plugin
interface (i.e., modify the RouterPlugin.handle method to include "throws
IOException") and ensure imports/existing code remain consistent.

In `@src/main/java/org/example/SocketServer.java`:
- Around line 25-26: Pipeline is being instantiated per connection with no
filters or plugin, causing FilterChain.doFilter to call a null plugin and throw
NPE; fix by creating and configuring a single Pipeline (adding required filters
and setting the plugin) before entering the accept loop and reuse that
configured Pipeline for each connection (or provide a PipelineFactory used by
SocketServer.accept to build a fully-configured Pipeline per connection); update
code references where Pipeline is created near Thread.ofVirtual().start(() ->
handleClient(socket, pipeline)) and ensure handleClient receives a non-null,
configured Pipeline (or factory) so FilterChain.plugin is always initialized.

🧹 Nitpick comments (2)

src/main/java/org/example/SocketServer.java (2)

34-56: No error response on parse failure — client hangs.

If HttpParser.parse() throws an IOException (e.g. malformed request), the catch block on Line 54 only prints a stack trace. The client receives no response (not even a 400 Bad Request). Consider sending a minimal error response before closing the socket.

Sketch

         } catch (IOException e) {
-            e.printStackTrace();
+            e.printStackTrace();
+            try {
+                OutputStream out = socket.getOutputStream();
+                out.write("HTTP/1.1 400 Bad Request\r\nContent-Length: 0\r\n\r\n".getBytes());
+                out.flush();
+            } catch (IOException ignored) { }
         }

---

51-52: Debug logging duplicates LoggingFilter responsibility.

These System.out.println calls log the request method and path, which is exactly what a LoggingFilter in the pipeline should handle. Once the pipeline is wired correctly, this becomes redundant and should be removed to avoid double-logging.

[kristinaxm]

The parser code is already in main from the previous PR, so Git will only apply the new changes here. Rebasing on main after the first PR merges will keep the history clean, I think.. :P

[kristinaxm]

.

[kristinaxm]

@VonAdamo one final approval, not to the moon, launch it to another galaxy 😄🚀 I’m officially done looking at this