-
Notifications
You must be signed in to change notification settings - Fork 66
Färdig JavaFX chat app med tester #9
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,27 @@ | ||
| package com.example; | ||
|
|
||
| import javafx.fxml.FXML; | ||
| import javafx.scene.control.Label; | ||
| import javafx.scene.control.TextArea; | ||
| import javafx.scene.control.TextField; | ||
|
|
||
| /** | ||
| * Controller layer: mediates between the view (FXML) and the model. | ||
| */ | ||
| public class HelloController { | ||
| @FXML private TextArea chatArea; | ||
| @FXML private TextField inputField; | ||
|
|
||
| private final HelloModel model = new HelloModel(); | ||
| private HelloModel model; | ||
|
|
||
| @FXML | ||
| private Label messageLabel; | ||
| public void initialize() { | ||
| model = new HelloModel("javafx-chat"); // Du kan byta topic-namnet | ||
| model.listen(msg -> chatArea.appendText(msg + "\n")); | ||
| } | ||
|
|
||
| @FXML | ||
| private void initialize() { | ||
| if (messageLabel != null) { | ||
| messageLabel.setText(model.getGreeting()); | ||
| public void onSendButtonClick() { | ||
| String text = inputField.getText().trim(); | ||
| if (!text.isEmpty()) { | ||
| model.sendMessage(text); | ||
| inputField.clear(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,52 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Model layer: encapsulates application data and business logic. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.URI; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.http.HttpClient; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.http.HttpRequest; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.http.HttpResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.concurrent.CompletableFuture; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class HelloModel { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Returns a greeting based on the current Java and JavaFX versions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String getGreeting() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String javaVersion = System.getProperty("java.version"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String javafxVersion = System.getProperty("javafx.version"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "Hello, JavaFX " + javafxVersion + ", running on Java " + javaVersion + "."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final HttpClient client = HttpClient.newHttpClient(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final String topic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final String backendUrl; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public HelloModel(String topic) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.topic = topic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.backendUrl = System.getenv("BACKEND_URL"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (backendUrl == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new IllegalStateException("BACKEND_URL is not set!"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void sendMessage(String message) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String json = "{\"message\": \"" + message + "\"}"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String url = backendUrl + "/" + topic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpRequest request = HttpRequest.newBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(url)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .header("Content-Type", "application/json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .POST(HttpRequest.BodyPublishers.ofString(json)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.sendAsync(request, HttpResponse.BodyHandlers.ofString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: JSON injection vulnerability and missing error handling. Line 23 constructs JSON by string concatenation without escaping user input. If Additionally, line 32 ignores the HTTP response and provides no error handling for network failures. Apply this diff to use proper JSON handling and add basic error handling: public void sendMessage(String message) {
- String json = "{\"message\": \"" + message + "\"}";
+ // Escape quotes, backslashes, and control characters
+ String escapedMessage = message
+ .replace("\\", "\\\\")
+ .replace("\"", "\\\"")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
+ .replace("\t", "\\t");
+ String json = "{\"message\": \"" + escapedMessage + "\"}";
String url = backendUrl + "/" + topic;
HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create(url))
.header("Content-Type", "application/json")
.POST(HttpRequest.BodyPublishers.ofString(json))
.build();
- client.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+ client.sendAsync(request, HttpResponse.BodyHandlers.ofString())
+ .exceptionally(e -> {
+ System.err.println("Failed to send message: " + e.getMessage());
+ return null;
+ });
}Better solution: Use a proper JSON library like Jackson or Gson instead of manual string concatenation. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public CompletableFuture<Void> listen(MessageHandler handler) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String url = backendUrl + "/" + topic + "/json"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpRequest request = HttpRequest.newBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(url)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return client.sendAsync(request, HttpResponse.BodyHandlers.ofLines()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .thenAccept(response -> response.body().forEach(line -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (line.contains("\"message\"")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| handler.onMessage(line); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CRITICAL: Naive JSON parsing and missing error handling. Line 43 uses Additionally, there's no error handling for connection failures, timeouts, or malformed responses. Consider using a proper JSON parser. Here's a basic improvement with error handling: public CompletableFuture<Void> listen(MessageHandler handler) {
String url = backendUrl + "/" + topic + "/json";
HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create(url))
.build();
return client.sendAsync(request, HttpResponse.BodyHandlers.ofLines())
.thenAccept(response -> response.body().forEach(line -> {
- if (line.contains("\"message\"")) {
+ // Basic JSON parsing - consider using a proper JSON library
+ if (line.trim().matches(".*\"message\"\\s*:\\s*\".*\".*")) {
handler.onMessage(line);
}
- }));
+ }))
+ .exceptionally(e -> {
+ System.err.println("Failed to listen for messages: " + e.getMessage());
+ return null;
+ });
}Recommended: Use a JSON library (Jackson/Gson) to properly parse the response and extract the message field value. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public interface MessageHandler { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void onMessage(String message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| module hellofx { | ||
| requires javafx.controls; | ||
| requires javafx.fxml; | ||
| requires java.net.http; | ||
|
|
||
| opens com.example to javafx.fxml; | ||
| exports com.example; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,26 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <?import javafx.scene.layout.StackPane?> | ||
| <?import javafx.scene.control.Label?> | ||
| <?import javafx.scene.control.*?> | ||
| <?import javafx.scene.layout.*?> | ||
| <?import javafx.geometry.Insets?> | ||
|
|
||
| <StackPane xmlns="http://javafx.com/javafx" xmlns:fx="http://javafx.com/fxml" fx:controller="com.example.HelloController"> | ||
| <children> | ||
| <Label fx:id="messageLabel" text="Hello, JavaFX!" /> | ||
| </children> | ||
| </StackPane> | ||
| <BorderPane xmlns:fx="http://javafx.com/fxml" | ||
| fx:controller="com.example.HelloController"> | ||
|
|
||
| <center> | ||
| <TextArea fx:id="chatArea" | ||
| editable="false" | ||
| prefHeight="400" | ||
| prefWidth="600"/> | ||
| </center> | ||
|
|
||
| <bottom> | ||
| <!-- ✅ Rätt sätt att skriva padding i JavaFX 25 --> | ||
| <HBox spacing="10"> | ||
| <padding> | ||
| <Insets top="10" right="10" bottom="10" left="10"/> | ||
| </padding> | ||
| <TextField fx:id="inputField" HBox.hgrow="ALWAYS"/> | ||
| <Button text="Send" onAction="#onSendButtonClick"/> | ||
| </HBox> | ||
| </bottom> | ||
| </BorderPane> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package com.example; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| public class HelloModelTest { | ||
|
|
||
| @Test | ||
| void testModelInitialization() { | ||
| // Försök skapa modellen, hantera fel ifall BACKEND_URL saknas | ||
| try { | ||
| HelloModel model = new HelloModel("test-topic"); | ||
| assertNotNull(model, "Model should be created successfully"); | ||
| } catch (IllegalStateException e) { | ||
| // Om miljövariabeln saknas ska felet ha rätt meddelande | ||
| assertTrue(e.getMessage().contains("BACKEND_URL"), "Exception should mention BACKEND_URL"); | ||
| } | ||
| } | ||
|
Comment on lines
+9
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test always passes and doesn't verify model behavior. The try-catch structure makes this test pass in both success and failure scenarios. Additionally, creating a model with a missing Apply this diff to properly test model initialization: @Test
void testModelInitialization() {
- // Försök skapa modellen, hantera fel ifall BACKEND_URL saknas
- try {
- HelloModel model = new HelloModel("test-topic");
- assertNotNull(model, "Model should be created successfully");
- } catch (IllegalStateException e) {
- // Om miljövariabeln saknas ska felet ha rätt meddelande
- assertTrue(e.getMessage().contains("BACKEND_URL"), "Exception should mention BACKEND_URL");
- }
+ // Test requires BACKEND_URL to be set in test environment
+ String originalUrl = System.getenv("BACKEND_URL");
+ if (originalUrl == null || originalUrl.isBlank()) {
+ // If BACKEND_URL is not set, verify exception is thrown
+ assertThrows(IllegalStateException.class,
+ () -> new HelloModel("test-topic"),
+ "Model should throw IllegalStateException when BACKEND_URL is not set");
+ } else {
+ // If BACKEND_URL is set, verify model is created
+ HelloModel model = new HelloModel("test-topic");
+ assertNotNull(model, "Model should be created successfully when BACKEND_URL is set");
+ }
}🤖 Prompt for AI Agents |
||
|
|
||
| @Test | ||
| void testBackendUrlDefault() { | ||
| // Testar att miljövariabeln hanteras | ||
| String backendUrl = System.getenv("BACKEND_URL"); | ||
| if (backendUrl == null || backendUrl.isBlank()) { | ||
| backendUrl = "https://ntfy.sh"; | ||
| } | ||
| assertTrue(backendUrl.startsWith("https://"), "BACKEND_URL should start with https://"); | ||
| } | ||
|
Comment on lines
+20
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test doesn't verify HelloModel behavior. This test checks environment variable logic but doesn't test how Consider removing this test or rewriting it to verify that 🤖 Prompt for AI Agents |
||
|
|
||
| @Test | ||
| void testSendMessageFormatting() { | ||
| String userMessage = "Hej världen!"; | ||
| String formatted = "{\"message\": \"" + userMessage + "\"}"; | ||
| assertTrue(formatted.contains("Hej världen!")); | ||
| assertTrue(formatted.contains("{")); | ||
| assertTrue(formatted.contains("}")); | ||
| } | ||
|
Comment on lines
+30
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test doesn't call HelloModel methods. This test validates string concatenation logic within the test itself rather than testing The test should verify the actual HTTP request sent by 🤖 Prompt for AI Agents |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRITICAL: Raw JSON displayed in chat area instead of parsed messages.
Line 16 appends the raw message from the listener to
chatArea. Based onHelloModel.listen()(line 44 inHelloModel.java), this will be a raw JSON line from the HTTP response, not the parsed message text. Users will see JSON strings like{"message":"Hello"}instead of just"Hello".Apply this diff to parse the message before displaying:
@FXML public void initialize() { model = new HelloModel("javafx-chat"); // Du kan byta topic-namnet - model.listen(msg -> chatArea.appendText(msg + "\n")); + model.listen(msg -> { + // Parse JSON to extract message text + // Basic parsing - consider using a JSON library for production + int start = msg.indexOf("\"message\""); + if (start != -1) { + start = msg.indexOf("\"", start + 9) + 1; + int end = msg.indexOf("\"", start); + if (end != -1) { + String messageText = msg.substring(start, end); + chatArea.appendText(messageText + "\n"); + } + } + }); }Better solution: Extract the message parsing logic to
HelloModelso theMessageHandler.onMessage()receives the parsed message text, not raw JSON.📝 Committable suggestion
🤖 Prompt for AI Agents