Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
HOST_NAME=https://ntfy.fungover.org
20 changes: 18 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,22 @@
<artifactId>javafx-fxml</artifactId>
<version>${javafx.version}</version>
</dependency>
<dependency>
<groupId>io.github.cdimascio</groupId>
<artifactId>dotenv-java</artifactId>
<version>3.2.0</version>
</dependency>
<dependency>
<groupId>tools.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>3.0.1</version>
</dependency>
Comment on lines +53 to +57
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect groupId for jackson-databind dependency.

The groupId tools.jackson.core appears to be incorrect. The standard Jackson groupId is com.fasterxml.jackson.core. This incorrect groupId will likely cause the dependency to fail to resolve from Maven Central.

Apply this diff to use the correct groupId:

         <dependency>
-            <groupId>tools.jackson.core</groupId>
+            <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
             <version>3.0.1</version>
         </dependency>

Additionally, verify that version 3.0.1 is appropriate for your Java version, as Jackson 3.x requires Java 17+.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<dependency>
<groupId>tools.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>3.0.1</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>3.0.1</version>
</dependency>
🤖 Prompt for AI Agents
In pom.xml around lines 53 to 57 the jackson-databind dependency uses the wrong
groupId "tools.jackson.core"; change the groupId to "com.fasterxml.jackson.core"
so Maven can resolve the artifact, and confirm the jackson-databind version
(3.0.1) is compatible with your project's Java runtime (Jackson 3.x requires
Java 17+) before committing the change.

<dependency>
<groupId>org.wiremock</groupId>
<artifactId>wiremock</artifactId>
<version>4.0.0-beta.15</version>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
Expand All @@ -55,7 +71,7 @@
<configuration>
<mainClass>com.example.HelloFX</mainClass>
<options>
<option>--enable-native-access=javafx.graphics</option>
<option>--enable-native-access=javafx.graphics</option>
</options>
<launcher>javafx</launcher>
<stripDebug>true</stripDebug>
Expand All @@ -65,4 +81,4 @@
</plugin>
</plugins>
</build>
</project>
</project>
50 changes: 48 additions & 2 deletions src/main/java/com/example/HelloController.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,68 @@
package com.example;

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.control.TextField;
import javafx.stage.FileChooser;
import javafx.stage.Stage;

import java.io.File;

/**
* Controller layer: mediates between the view (FXML) and the model.
*/
public class HelloController {

private final HelloModel model = new HelloModel();
private final HelloModel model = new HelloModel(new NtfyConnectionImpl());
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Direct instantiation breaks testability and violates dependency injection principles.

Creating new NtfyConnectionImpl() directly in the controller field initializer makes it impossible to:

  • Test the controller with a mock/spy connection
  • Configure the connection differently for different environments
  • Replace the implementation without modifying the controller

This is particularly problematic since tests like HelloModelTest use NtfyConnectionSpy but cannot test the controller's interaction with the model.

Consider one of these approaches:

Option 1: Constructor injection (preferred)

 public class HelloController {
 
-    private final HelloModel model = new HelloModel(new NtfyConnectionImpl());
+    private final HelloModel model;
+    
+    public HelloController() {
+        this(new NtfyConnectionImpl());
+    }
+    
+    // Package-private for testing
+    HelloController(NtfyConnection connection) {
+        this.model = new HelloModel(connection);
+    }

Option 2: Lazy initialization with factory method

 public class HelloController {
 
-    private final HelloModel model = new HelloModel(new NtfyConnectionImpl());
+    private HelloModel model;
     
     @FXML
     private void initialize() {
+        if (model == null) {
+            model = createModel();
+        }
         // ... rest of initialization
+    }
+    
+    // Protected for testing/override
+    protected HelloModel createModel() {
+        return new HelloModel(new NtfyConnectionImpl());
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around line 18, the controller
directly instantiates new NtfyConnectionImpl in the field initializer which
breaks testability and DI; change to accept the dependency instead — add a
constructor that takes a NtfyConnection (or a NtfyConnectionFactory) and use
that to create or pass in the HelloModel, or alternatively provide a
package-visible/default constructor that delegates to a constructor accepting
the connection for production use; update instantiation sites (e.g., application
wiring) to pass new NtfyConnectionImpl(), and update tests to pass
NtfyConnectionSpy/mocks.


@FXML
public ListView<NtfyMessageDto> messageView;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Make messageView field private for proper encapsulation.

The messageView field is declared public but should be private. JavaFX's @FXML annotation works with private fields, so there's no need for public visibility. Public fields break encapsulation and expose internal implementation details.

Apply this diff:

     @FXML
-    public ListView<NtfyMessageDto> messageView;
+    private ListView<NtfyMessageDto> messageView;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public ListView<NtfyMessageDto> messageView;
@FXML
private ListView<NtfyMessageDto> messageView;
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around line 21, the
messageView field is declared public which breaks encapsulation; change its
visibility to private and annotate it with @FXML (if not already) so JavaFX can
inject it while keeping the field encapsulated; update any direct external
accesses to use getter/setter or controller methods if necessary.


@FXML
private Label messageLabel;

@FXML
private TextField messageInput;

@FXML
private void initialize() {
if (messageLabel != null) {
messageLabel.setText(model.getGreeting());
}
messageView.setItems(model.getMessages());


model.messageToSendProperty().bindBidirectional(messageInput.textProperty());


messageView.setCellFactory(lv -> new javafx.scene.control.ListCell<NtfyMessageDto>() {
@Override
protected void updateItem(NtfyMessageDto item, boolean empty) {
super.updateItem(item, empty);
if (empty || item == null) {
setText(null);
} else {
setText(item.message() != null ? item.message() : "(No message content)");
}
}
});
}

public void sendMessage(ActionEvent actionEvent) {
model.sendMessage();

messageInput.clear();
}

public void attachFile(ActionEvent actionEvent) {
FileChooser fileChooser = new FileChooser();
fileChooser.setTitle("Select File to Attach");
File file = fileChooser.showOpenDialog(new Stage());

if (file != null) {
model.sendFile(file);
}
}
Comment on lines +59 to 67
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the controller's window instead of creating a new Stage.

Line 62 creates a new Stage() for the FileChooser, which is unnecessary and may not provide the correct parent window relationship. The dialog should use the actual application window.

Apply this diff to get the owner window from an existing scene node:

     public void attachFile(ActionEvent actionEvent) {
         FileChooser fileChooser = new FileChooser();
         fileChooser.setTitle("Select File to Attach");
-        File file = fileChooser.showOpenDialog(new Stage());
+        Stage ownerStage = (Stage) messageInput.getScene().getWindow();
+        File file = fileChooser.showOpenDialog(ownerStage);
 
         if (file != null) {
             model.sendFile(file);
         }
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 59 to 67, the
FileChooser is opened with new Stage() which creates a separate window; instead
obtain the owner window from the controller's scene node and pass it to
showOpenDialog. Replace new Stage() by extracting the Window from the
ActionEvent source (cast event.getSource() to Node, call
getScene().getWindow()), guard against nulls, and use that Window as the owner
for fileChooser.showOpenDialog(window).

}
}
3 changes: 1 addition & 2 deletions src/main/java/com/example/HelloFX.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ public void start(Stage stage) throws Exception {
FXMLLoader fxmlLoader = new FXMLLoader(HelloFX.class.getResource("hello-view.fxml"));
Parent root = fxmlLoader.load();
Scene scene = new Scene(root, 640, 480);
stage.setTitle("Hello MVC");
stage.setTitle("JavaFX Chat App");
stage.setScene(scene);
stage.show();
}

public static void main(String[] args) {
launch();
}

}
72 changes: 70 additions & 2 deletions src/main/java/com/example/HelloModel.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,83 @@
package com.example;

import javafx.application.Platform;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

import java.io.File;

/**
* Model layer: encapsulates application data and business logic.
*/
public class HelloModel {

private final NtfyConnection connection;
private final ObservableList<NtfyMessageDto> messages = FXCollections.observableArrayList();
private final StringProperty messageToSend = new SimpleStringProperty();

public HelloModel(NtfyConnection connection) {
this.connection = connection;
startReceivingMessages();
}

public ObservableList<NtfyMessageDto> getMessages() {
return messages;
}

public String getMessageToSend() {
return messageToSend.get();
}

public StringProperty messageToSendProperty() {
return messageToSend;
}

public void setMessageToSend(String message) {
messageToSend.set(message);
}

/**
* 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 + ".";
return "Welcome to JavaFX Chat App " + javafxVersion + ", running on Java " + javaVersion + ".";
}

public boolean sendMessage() {
String message = getMessageToSend();
if (message == null || message.trim().isEmpty()) {
return false;
}
return connection.send(message.trim());
}

public boolean sendFile(File file) {
if (file == null || !file.exists()) {
return false;
}
return connection.sendFile(file);
}
Comment on lines +50 to +63
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Consider message clearing and return value semantics after send/sendFile.

Two small behavior points worth deciding explicitly:

  • After a successful sendMessage(), most chat UIs clear the input field. You could either clear messageToSend here on success or have the controller clear it based on the boolean result.
  • The boolean returned from sendMessage() / sendFile() currently reflects only basic validation and what NtfyConnection.send/sendFile return. Given the async nature of NtfyConnectionImpl, that boolean does not guarantee delivery—only that the call was accepted. Ensure the UI doesn’t over‑promise success based on it.

If you want to double‑check how the boolean is used, you can scan the controller:


🏁 Script executed:

#!/bin/bash
rg -n "sendMessage\(" src/main/java
rg -n "sendFile\(" src/main/java

Length of output: 695


I need to examine the controller methods more closely to verify how the return values are actually being handled.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get full context of sendMessage and sendFile methods in controller
rg -n -A10 "public void sendMessage\(" src/main/java/com/example/HelloController.java
rg -n -B5 -A15 "public void sendFile\(" src/main/java/com/example/HelloController.java

Length of output: 378


I can see that sendMessage() already clears the input field, but it does so unconditionally—even when model.sendMessage() returns false. Let me check the sendFile usage:


---

🏁 Script executed:

```shell
#!/bin/bash
# Get context around sendFile call at line 65
rg -n -B10 -A5 "model.sendFile" src/main/java/com/example/HelloController.java

Length of output: 383


Use the return value to gate input clearing in sendMessage() and provide feedback for sendFile().

Verification shows the boolean return values are being discarded entirely. In HelloController.sendMessage() (lines 53-57), messageInput.clear() happens unconditionally even when model.sendMessage() returns false—meaning the input clears even if validation fails. Similarly, sendFile() (line 65) ignores the return value with no user feedback on success/failure.

  • Only clear messageInput when model.sendMessage() returns true.
  • Consider providing feedback (toast/status) when sendFile() fails or succeeds.
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloModel.java around lines 50-63 and in
src/main/java/com/example/HelloController.java around lines 53-65: the boolean
returns from model.sendMessage() and model.sendFile(...) are currently ignored,
so messageInput.clear() runs unconditionally and file send has no feedback;
change HelloController.sendMessage() to call model.sendMessage() and only clear
the message input when it returns true, and change HelloController.sendFile(...)
to check the boolean result and surface success/failure to the user (e.g., show
a toast or status label) when sendFile returns false or true.


private void startReceivingMessages() {
connection.receive(this::addMessageToUI);
}

private void addMessageToUI(NtfyMessageDto message) {
// Check if we're on JavaFX application thread, if not use Platform.runLater
if (Platform.isFxApplicationThread()) {
messages.add(message);
} else {
Platform.runLater(() -> messages.add(message));
}
}

// Test helper method - package private for testing
void addTestMessage(NtfyMessageDto message) {
// Direct add for testing (bypasses Platform.runLater)
messages.add(message);
}
}
}
19 changes: 19 additions & 0 deletions src/main/java/com/example/ManyParameters.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.example;

public class ManyParameters {

public ManyParameters(String computerName, int timeout,
String method, int size, byte[] data) {

}
Comment on lines +5 to +8
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Constructor accepts parameters but doesn't store them.

The constructor receives 5 parameters but has an empty body. This makes the ManyParameters class effectively useless, as none of the data is stored. You should either:

  • Add private fields and assign them in the constructor, or
  • If this is intentional for demonstration, add a comment explaining why

Apply this diff to store the parameters:

+    private final String computerName;
+    private final int timeout;
+    private final String method;
+    private final int size;
+    private final byte[] data;
+
     public ManyParameters(String computerName, int timeout,
                           String method, int size, byte[] data) {
-
+        this.computerName = computerName;
+        this.timeout = timeout;
+        this.method = method;
+        this.size = size;
+        this.data = data;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public ManyParameters(String computerName, int timeout,
String method, int size, byte[] data) {
}
private final String computerName;
private final int timeout;
private final String method;
private final int size;
private final byte[] data;
public ManyParameters(String computerName, int timeout,
String method, int size, byte[] data) {
this.computerName = computerName;
this.timeout = timeout;
this.method = method;
this.size = size;
this.data = data;
}
🤖 Prompt for AI Agents
In src/main/java/com/example/ManyParameters.java around lines 5 to 8, the
constructor accepts five parameters but the body is empty so none of the values
are stored; add private fields for computerName (String), timeout (int), method
(String), size (int) and data (byte[]) to the class and assign each parameter to
the corresponding field in the constructor (or if intentional leave a comment
explaining why and mark constructor accordingly).



static void main() {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non-standard main method signature.

The method signature static void main() is incorrect for a Java entry point. The JVM requires static void main(String[] args). This method won't be recognized as an executable main method.

Apply this diff to fix the signature:

-    static void main() {
+    public static void main(String[] args) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static void main() {
public static void main(String[] args) {
🤖 Prompt for AI Agents
In src/main/java/com/example/ManyParameters.java around line 11, the method is
declared as `static void main()` which is not the JVM-recognized entry point.
Change the method signature to `static void main(String[] args)` so the JVM can
invoke it; if the parameter is unused, keep the name or mark it as unused (or
use `_args`) but do not remove it, and update any internal references
accordingly.

ManyParametersBuilder builder = new ManyParametersBuilder();
builder
.setComputerName("localhost") //Fluent API
.setTimeout(10)
.setSize(0)
.createManyParameters();
}
}
38 changes: 38 additions & 0 deletions src/main/java/com/example/ManyParametersBuilder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.example;

public class ManyParametersBuilder {
private String computerName;
private int timeout = 0;
private String method;
private int size = 0;
private byte[] data = null;

public ManyParametersBuilder setComputerName(String computerName) {
this.computerName = computerName;
return this;
}

public ManyParametersBuilder setTimeout(int timeout) {
this.timeout = timeout;
return this;
}

public ManyParametersBuilder setMethod(String method) {
this.method = method;
return this;
}

public ManyParametersBuilder setSize(int size) {
this.size = size;
return this;
}

public ManyParametersBuilder setData(byte[] data) {
this.data = data;
return this;
Comment on lines +30 to +32
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

No defensive copy of byte array parameter.

The builder stores the byte[] reference directly without making a defensive copy. This allows external code to mutate the array after passing it to the builder, potentially causing unexpected behavior.

Apply this diff to make a defensive copy:

     public ManyParametersBuilder setData(byte[] data) {
-        this.data = data;
+        this.data = data != null ? data.clone() : null;
         return this;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public ManyParametersBuilder setData(byte[] data) {
this.data = data;
return this;
public ManyParametersBuilder setData(byte[] data) {
this.data = data != null ? data.clone() : null;
return this;
}
🤖 Prompt for AI Agents
In src/main/java/com/example/ManyParametersBuilder.java around lines 30 to 32,
the setData(byte[] data) method assigns the incoming byte[] reference directly
which allows external mutation; change it to make a defensive copy (e.g., if
data is null set this.data = null else this.data = Arrays.copyOf(data,
data.length)) so the builder stores its own copy and does not expose internal
state to callers; ensure Arrays is imported or use data.clone() as an
alternative.

}

public ManyParameters createManyParameters() {
return new ManyParameters(computerName, timeout, method, size, data);
}
}
10 changes: 10 additions & 0 deletions src/main/java/com/example/NtfyConnection.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.example;

import java.io.File;
import java.util.function.Consumer;

public interface NtfyConnection {
boolean send(String message);
void receive(Consumer<NtfyMessageDto> messageHandler);
boolean sendFile(File file);
}
Loading
Loading