Skip to content

Rename SocketServer to Server Move HTTP request handling logic to a dedicated ConnectionHandler. #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fmazmz merged 8 commits into from
Feb 11, 2026
Merged

Rename SocketServer to Server Move HTTP request handling logic to a dedicated ConnectionHandler. #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dc7fd89
Rename SocketServer to Server
johanbriger Feb 10, 2026
c1f308b
Convert createSocket to an instance method named start().
johanbriger Feb 10, 2026
7e74b5a
resolve conflicts
fmazmz Feb 10, 2026
ac7ff05
Replace console prints with Logger in ConnectionHandler
johanbriger Feb 10, 2026
0090d65
Fixed typo in ConnectionHandler
johanbriger Feb 10, 2026
d3dd7b9
refactor Server and ConnectionHandler:
fmazmz Feb 11, 2026
26b916a
accept ConnectionHandlerFactory and not a specific implementation
fmazmz Feb 11, 2026
030eb5a
normalize handlerFactory name
fmazmz Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/main/java/org/juv25d/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
package org.juv25d;

import org.juv25d.logging.ServerLogging;
import org.juv25d.parser.HttpParser;

import java.util.logging.Logger;

public class App {
public static void main(String[] args) {
SocketServer.createSocket();
Logger logger = ServerLogging.getLogger();
HttpParser httpParser = new HttpParser();
DefaultConnectionHandlerFactory handlerFactory =
new DefaultConnectionHandlerFactory(httpParser, logger);

Server server = new Server(
logger,
handlerFactory
);

server.start();
Comment on lines -5 to +20

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactoring, app.java now only handles object creation and wiring, not business logic.

Testability: Each component can now be tested independently
Flexibility: Easy to swap implementations (e.g., MockLogger for testing)

When I add StaticFileHandler, should I pass it through the
ConnectionHandlerFactory, or inject it directly into
DefaultConnectionHandlerFactory? PR #36

Example:

DefaultConnectionHandlerFactory handlerFactory = 
    new DefaultConnectionHandlerFactory(httpParser, logger, staticFileHandler);

Would this be the right approach?

}
}
31 changes: 31 additions & 0 deletions src/main/java/org/juv25d/ConnectionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.juv25d;

import org.juv25d.parser.HttpParser;
import java.io.IOException;
import java.net.Socket;
import java.util.logging.Level;
import java.util.logging.Logger;

public class ConnectionHandler implements Runnable {
private final Socket socket;
private final HttpParser httpParser;
private final Logger logger;
Comment on lines +9 to +12

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of "final" - ensures immutability after construction.
Suggestion for my StaticFileHandler integration:
Would it make sense to add StaticFileHandler as a field here?

private final StaticFileHandler staticFileHandler;

This would allow ConnectionHandler to delegate GET requests
to StaticFileHandler.


public ConnectionHandler(Socket socket, HttpParser httpParser, Logger logger) {
this.socket = socket;
this.httpParser = httpParser;
this.logger = logger;
Comment on lines +14 to +17

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constructor follows DI pattern well.

Note:This is where I'll need to add StaticFileHandler
when integrating my PR #36 :

public ConnectionHandler(Socket socket, HttpParser httpParser, 
                        Logger logger, StaticFileHandler fileHandler) {
    this.socket = socket;
    this.httpParser = httpParser;
    this.logger = logger;
    this.staticFileHandler = fileHandler;
}

}

@Override
public void run() {
try (socket) {
HttpRequest request = httpParser.parse(socket.getInputStream());

logger.info("Handling request: " + request.method() + " " + request.path());

@annikaholmqvist94 annikaholmqvist94 Feb 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: This is where HTTP response logic should go

Currently, the request is parsed but no response is sent back.
This will cause the client to hang.

For my PR #36 integration, I propose adding:**

HttpRequest request = httpParser.parse(socket.getInputStream());

logger.info("Handling request: " + request.method() + " " + request.path());

// NEW CODE - Handle GET requests with StaticFileHandler
if (request.method().equalsIgnoreCase("GET")) {
    HttpResponse response = staticFileHandler.handle(request);
    HttpResponseWriter.write(socket.getOutputStream(), response);
} else {
    // Return 405 Method Not Allowed for non-GET
    HttpResponse response = createMethodNotAllowedResponse();
    HttpResponseWriter.write(socket.getOutputStream(), response);
}

Questions:

  1. Should response writing be handled here, or abstracted to another class?
  2. Should we add a ResponseWriter interface for flexibility?
  3. Is error handling sufficient, or should we return 500 on exceptions?


} catch (IOException e) {
logger.log(Level.SEVERE, "Error while handling request", e);
}
}
Comment thread
fmazmz marked this conversation as resolved.
}
7 changes: 7 additions & 0 deletions src/main/java/org/juv25d/ConnectionHandlerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.juv25d;

import java.net.Socket;

public interface ConnectionHandlerFactory {
Runnable create(Socket socket);
}
Comment on lines +5 to +7

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of Factory pattern

This interface allows for different ConnectionHandler implementations:

  • DefaultConnectionHandlerFactory (current)
  • MockConnectionHandlerFactory (for testing)

Suggestion:
Consider making the return type more specific:

ConnectionHandler create(Socket socket);

Instead of Runnable, to expose ConnectionHandler-specific methods
if needed in the future.

Integration note (PR #36):I'll modify DefaultConnectionHandlerFactory
to pass StaticFileHandler to ConnectionHandler, rather than creating
a new factory implementation

21 changes: 21 additions & 0 deletions src/main/java/org/juv25d/DefaultConnectionHandlerFactory.java
View file
Open in desktop

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Factory pattern implementation looks clean

Integration question:
For StaticFileHandler integration would these options suit?:

Option 1:

Modify this factory

private final StaticFileHandler staticFileHandler;

public DefaultConnectionHandlerFactory(HttpParser httpParser, 
                                      Logger logger,
                                      StaticFileHandler fileHandler) {
    this.httpParser = httpParser;
    this.logger = logger;
    this.staticFileHandler = fileHandler;
}

@Override
public Runnable create(Socket socket) {
    return new ConnectionHandler(socket, httpParser, logger, staticFileHandler);
}

option 2:

Create a new factory

public class StaticFileConnectionHandlerFactory implements ConnectionHandlerFactory {
    // ... custom implementation
}

Which approach aligns better with the architecture?

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.juv25d;

import org.juv25d.parser.HttpParser;

import java.net.Socket;
import java.util.logging.Logger;

public class DefaultConnectionHandlerFactory implements ConnectionHandlerFactory{
private final HttpParser httpParser;
private final Logger logger;

public DefaultConnectionHandlerFactory(HttpParser httpParser, Logger logger) {
this.httpParser = httpParser;
this.logger = logger;
}

@Override
public Runnable create(Socket socket) {
return new ConnectionHandler(socket, httpParser, logger);
}
}
32 changes: 32 additions & 0 deletions src/main/java/org/juv25d/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.juv25d;

import java.io.IOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.logging.Logger;

public class Server {
private static final int PORT = 3000;

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

Consider making PORT configurable:

private final int port;

public Server(Logger logger, ConnectionHandlerFactory factory, int port) {
    this.logger = logger;
    this.handlerFactory = factory;
    this.port = port;
}

This would allow:

  • Testing on different ports
  • Running multiple servers
  • Environment-specific configuration

But since we are only creating one server this is only a suggestion for further improvement:)

private final Logger logger;
private final ConnectionHandlerFactory handlerFactory;

public Server(Logger logger, ConnectionHandlerFactory handlerFactory) {
this.logger = logger;
this.handlerFactory = handlerFactory;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public void start() {
try (ServerSocket serverSocket = new ServerSocket(PORT, 64)) {
logger.info("Server started at port: " + serverSocket.getLocalPort());

while (true) {
Socket socket = serverSocket.accept();
Runnable handler = handlerFactory.create(socket);
Thread.ofVirtual().start(handler);
}

} catch (IOException e) {
throw new RuntimeException("Server error", e);
}
}
Comment on lines +18 to +31

@annikaholmqvist94 annikaholmqvist94 Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent use of Virtual Threads

  • No thread pool management needed
  • Perfect for I/O-bound tasks like HTTP

Question:
Should we add shutdown handling?

private volatile boolean running = true;

public void stop() {
    running = false;
}

while (running) {
    Socket socket = serverSocket.accept();
    // ...
}

}
55 changes: 0 additions & 55 deletions src/main/java/org/juv25d/SocketServer.java
View file
Open in desktop

This file was deleted.