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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
target/
/.idea/
.env
*.iml
71 changes: 69 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,22 @@
<properties>
<maven.compiler.release>25</maven.compiler.release>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<junit.jupiter.version>6.0.0</junit.jupiter.version>
<junit.jupiter.version>5.9.2</junit.jupiter.version>
<assertj.core.version>3.27.6</assertj.core.version>
<mockito.version>5.20.0</mockito.version>
<javafx.version>25</javafx.version>
<testfx.version>4.0.16-alpha</testfx.version>
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 using a stable TestFX version.

The version 4.0.16-alpha is an alpha release, which may contain bugs or instability.


🌐 Web query:

What is the latest stable version of TestFX?

💡 Result:

The latest stable TestFX release is 4.0.18. [1][2]

Sources:
[1] TestFX GitHub README (shows 4.0.18).
[2] org.testfx listings / javadoc.io (latest versions: testfx-core/testfx-junit/testfx-junit5 = 4.0.18).


Update TestFX to the latest stable version 4.0.18.

The version 4.0.16-alpha is an alpha release and a newer stable version (4.0.18) is available. Update pom.xml line 18 to use 4.0.18 instead.

🤖 Prompt for AI Agents
In pom.xml around line 18, the TestFX dependency property is set to the alpha
release 4.0.16-alpha; update the value to the latest stable version by replacing
"4.0.16-alpha" with "4.0.18" so the project uses TestFX 4.0.18.

</properties>
<dependencies>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<artifactId>junit-jupiter-api</artifactId>
<version>${junit.jupiter.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>${junit.jupiter.version}</version>
<scope>test</scope>
</dependency>
Expand All @@ -35,6 +42,18 @@
<version>${mockito.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testfx</groupId>
<artifactId>testfx-junit5</artifactId>
<version>${testfx.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testfx</groupId>
<artifactId>testfx-core</artifactId>
<version>${testfx.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.openjfx</groupId>
<artifactId>javafx-controls</artifactId>
Expand All @@ -45,8 +64,45 @@
<artifactId>javafx-fxml</artifactId>
<version>${javafx.version}</version>
</dependency>
<dependency>
<groupId>org.openjfx</groupId>
<artifactId>javafx-base</artifactId>
<version>${javafx.version}</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.17.0</version>
</dependency>
<dependency>
<groupId>io.github.cdimascio</groupId>
<artifactId>dotenv-java</artifactId>
<version>3.1.0</version>
</dependency>
<dependency>
<groupId>org.jetbrains</groupId>
<artifactId>annotations</artifactId>
<version>24.0.1</version>
</dependency>
<dependency>
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
<version>RELEASE</version>
<scope>compile</scope>
</dependency>
Comment on lines +87 to +92
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

Fix TestNG dependency configuration.

There are two critical issues:

  1. RELEASE version: This creates non-reproducible builds as it always fetches the latest version
  2. compile scope: Test frameworks should use test scope, not compile

Apply this diff:

 <dependency>
     <groupId>org.testng</groupId>
     <artifactId>testng</artifactId>
-    <version>RELEASE</version>
-    <scope>compile</scope>
+    <version>7.10.2</version>
+    <scope>test</scope>
 </dependency>

Additionally, verify if you actually need both JUnit Jupiter and TestNG in the same project, as this is unusual.

🤖 Prompt for AI Agents
In pom.xml around lines 87 to 92, the TestNG dependency is misconfigured: it
uses the non-reproducible version placeholder "RELEASE" and the wrong scope
"compile". Replace "RELEASE" with a specific released version number (e.g.,
7.7.0 or the project-approved TestNG version) and change the scope to "test" so
TestNG is only available for tests; then run a mvn dependency:tree or mvn -U
test to verify the resolved version. Also review whether TestNG is actually
needed alongside JUnit Jupiter and remove one if the project only uses a single
test framework.

</dependencies>
<build>
<resources>
<resource>
<directory>src/main/resources</directory>
<includes>
<include>**/*.fxml</include>
<include>**/*.css</include>
<include>**/*.jpg</include>
<include>**/*.png</include>
</includes>
</resource>
</resources>
<plugins>
<plugin>
<groupId>org.openjfx</groupId>
Expand All @@ -63,6 +119,17 @@
<noManPages>true</noManPages>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.2.5</version>
<configuration>
<includes>
<include>**/*Test.java</include>
<include>**/*IT.java</include>
</includes>
</configuration>
</plugin>
</plugins>
</build>
</project>
19 changes: 19 additions & 0 deletions src/main/java/com/example/ChatNetworkClient.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.example;

import java.io.Closeable;
import java.io.IOException;
import java.util.function.Consumer;

public interface ChatNetworkClient {
void send(String baseUrl, NtfyMessage message) throws Exception;

//Returnerar mitt egna Subscription-interface
Subscription subscribe(String baseUrl, String topic, Consumer<NtfyMessage> messageHandler);

//Inner interface för Subscription
interface Subscription extends Closeable {
@Override
void close() throws IOException;
boolean isOpen();
}
Comment on lines +7 to +18
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

🧩 Analysis chain

Resolve Subscription type duplication and align the API

You now have an inner ChatNetworkClient.Subscription and (per the PR summary) a separate top‑level com.example.Subscription. Returning the inner type here while NtfyHttpClient and controllers likely use the top‑level one can easily lead to compilation errors or awkward casts.

I recommend consolidating to a single Subscription type:

  • Either:
    • Remove the inner interface and change this signature to com.example.Subscription subscribe(...), or
    • Remove / adapt the top‑level Subscription so everything uses ChatNetworkClient.Subscription, and reference it explicitly (ChatNetworkClient.Subscription) in consumers.
  • Once you pick one, ensure NtfyHttpClient.subscribe and all fields in controllers/models use the same type.

Also consider narrowing send’s throws Exception to more specific checked exceptions (e.g. IOException), so callers know what to handle.


🏁 Script executed:

#!/bin/bash
# Quick check of Subscription usages and potential conflicts
rg -n "Subscription" src/main/java/com/example -C2

Length of output: 4355


Resolve critical Subscription type mismatch

Verification confirms a type conflict:

  • Top-level com.example.Subscription exists (src/main/java/com/example/Subscription.java)
  • Inner ChatNetworkClient.Subscription exists (src/main/java/com/example/ChatNetworkClient.java:14)
  • NtfyHttpClient.subscribe() returns the top-level Subscription (line 85)
  • ChatNetworkClient.subscribe() declares return type as unqualified Subscription (line 11), which resolves to the inner interface in this context
  • HelloController field is typed explicitly as ChatNetworkClient.Subscription (line 21), expecting the inner type

This creates a type mismatch: NtfyHttpClient cannot return a top-level Subscription when implementing ChatNetworkClient.subscribe(), which declares an inner type. HelloController will fail to assign the return value.

Action required: Choose one Subscription type and consolidate:

  • Option 1: Remove the inner ChatNetworkClient.Subscription; change ChatNetworkClient.subscribe() to return com.example.Subscription, and update HelloController.subscription field to the same type.
  • Option 2: Remove the top-level Subscription; have NtfyHttpClient.subscribe() return ChatNetworkClient.Subscription (or use explicit reference).

Also narrow send() from throws Exception to specific exceptions (e.g., IOException).

🤖 Prompt for AI Agents
In src/main/java/com/example/ChatNetworkClient.java around lines 7–18 there is a
type conflict between the inner interface ChatNetworkClient.Subscription and the
top-level com.example.Subscription referenced elsewhere
(NtfyHttpClient.subscribe() at line 85 and HelloController.subscription at line
21); pick one Subscription definition and reconcile all signatures: either (A)
delete the inner interface and change ChatNetworkClient.subscribe(...) return
type to com.example.Subscription and update HelloController.subscription to
com.example.Subscription, or (B) remove the top-level Subscription class and
change NtfyHttpClient.subscribe() to return ChatNetworkClient.Subscription (or
explicitly reference it), and update all imports/usages accordingly so they all
reference the same type; additionally, narrow the ChatNetworkClient.send(...)
signature to throw specific exceptions (e.g., IOException) instead of the broad
Exception and update implementations and callers to handle the narrower
exception types.

}
139 changes: 136 additions & 3 deletions src/main/java/com/example/HelloController.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,155 @@
package com.example;

import javafx.beans.binding.Bindings;
import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.control.TextField;
import javafx.scene.control.Button;

import java.io.IOException;
import java.util.UUID;

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

private final HelloModel model = new HelloModel();
private final HelloModel model;
private final ChatNetworkClient httpClient; // Abstraktion (Dependency Inversion)
private final String hostName;
private ChatNetworkClient.Subscription subscription;
private long subscriptionStartTime;
Comment on lines +18 to +22
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

Address thread safety concerns for subscription field.

The subscription field is accessed from multiple threads (UI thread and network callback thread) without synchronization. This could lead to race conditions.

Consider making the field volatile or using proper synchronization:

-    private ChatNetworkClient.Subscription subscription;
+    private volatile ChatNetworkClient.Subscription subscription;

Additionally, the subscriptionStartTime should also be volatile:

-    private long subscriptionStartTime;
+    private volatile long subscriptionStartTime;
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 18 to 22, the
subscription and subscriptionStartTime fields are accessed by multiple threads
without synchronization; make them thread-safe by declaring subscription as
volatile and subscriptionStartTime as volatile (or replace subscription with an
AtomicReference<ChatNetworkClient.Subscription> and subscriptionStartTime with
an AtomicLong) and ensure any compound operations on them are performed under
synchronization or via atomic compare-and-set to avoid race conditions.


@FXML
private Label messageLabel;
@FXML private Label messageLabel;

//Nya FXML-komponenter:
@FXML private ListView<NtfyMessage> messageListView; //Visar meddelanden från HelloModel
@FXML private TextField messageTextField; //Används för att skriva nya meddelanden
@FXML private Button sendButton; //Används för att skicka meddelanden
@FXML private Label connectionStatusLabel; // Status för anslutning

//Konstruktor för Dependency Injection
public HelloController(HelloModel model, ChatNetworkClient httpClient, String hostName) {
this.model = model;
this.httpClient = httpClient;
this.hostName = hostName;
}

@FXML
private void initialize() {
if (messageLabel != null) {
messageLabel.setText(model.getGreeting());
}
//Binder listView till meddelandelistan i modellen
messageListView.setItems(model.getMessages());

//Binder anslutningsstatus till etiketten
connectionStatusLabel.textProperty().bind(
Bindings.when(model.connectedProperty())
.then("Ansluten: ja")
.otherwise("Ansluten: nej")
);
}

@FXML
private void sendMessage() {
String messageText = messageTextField.getText();
if (!messageText.isEmpty()) {
try {
NtfyMessage message = createMessage(messageText);
httpClient.send(hostName, message);
messageTextField.clear();
} catch (Exception e) {
handleSendError(e);
}
}
}

private NtfyMessage createMessage(String text) {
return new NtfyMessage(
UUID.randomUUID().toString(),
System.currentTimeMillis(),
"message",
"mytopic",
text
);
}
Comment on lines +69 to +77
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

Fix time field to use seconds.

The time field in NtfyMessage should represent Unix timestamp in seconds (as per typical Ntfy behavior), but line 72 uses System.currentTimeMillis() which returns milliseconds.

Apply this diff:

 private NtfyMessage createMessage(String text) {
     return new NtfyMessage(
             UUID.randomUUID().toString(),
-            System.currentTimeMillis(),
+            System.currentTimeMillis() / 1000,
             "message",
             "mytopic",
             text
     );
 }

This matches the time comparison logic on line 117 which divides by 1000.

📝 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
private NtfyMessage createMessage(String text) {
return new NtfyMessage(
UUID.randomUUID().toString(),
System.currentTimeMillis(),
"message",
"mytopic",
text
);
}
private NtfyMessage createMessage(String text) {
return new NtfyMessage(
UUID.randomUUID().toString(),
System.currentTimeMillis() / 1000,
"message",
"mytopic",
text
);
}
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 69 to 77, the
NtfyMessage time field is set using System.currentTimeMillis() (milliseconds)
but should be a Unix timestamp in seconds; replace that value with seconds
(e.g., System.currentTimeMillis() / 1000L or Instant.now().getEpochSecond()) so
the time field matches the seconds-based logic used elsewhere (line 117).


private void handleSendError(Exception e) {
System.err.println("Fel vid sändning: " + e.getMessage());
}


@FXML
private void subscribeToTopic() {
unsubscribeFromCurrentTopic(); // Stäng den gamla prenumerationen
model.getMessages().clear(); // Rensa gamla meddelanden
startNewSubscription(); // Starta ny prenumeration
model.setConnected(true); // Uppdatera anslutningsstatus
}
Comment on lines +84 to +90
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 topic name configurable.

The topic "mytopic" is hard-coded in multiple places (lines 74, 106). Consider making it configurable, either through a TextField in the UI or as a constructor parameter.

For example, add a TextField to the FXML:

<TextField fx:id="topicTextField" promptText="Enter topic name..." text="mytopic" />

Then use it in the controller:

private void startNewSubscription() {
    String topic = topicTextField.getText();
    subscriptionStartTime = System.currentTimeMillis();
    subscription = httpClient.subscribe(hostName, topic, this::handleIncomingMessage);
}
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 74–106, the topic
name "mytopic" is hard-coded; make it configurable by adding a TextField in the
FXML with fx:id="topicTextField" (provide a default value like "mytopic"), add a
corresponding @FXML TextField topicTextField field in the controller, and
replace all hard-coded "mytopic" usages (lines ~74 and ~106) to read
topicTextField.getText() (with a null/empty fallback to "mytopic"); also ensure
startNewSubscription() uses this dynamic topic and update any constructor or
initialization logic to handle the new field.


private void unsubscribeFromCurrentTopic() {
if (subscription != null) {
try {
subscription.close();
} catch (IOException e) {
System.err.println("Kunde inte stänga gammal prenumeration: " + e.getMessage());
}
}
}

private void startNewSubscription() {
subscriptionStartTime = System.currentTimeMillis(); // Spara starttiden
subscription = httpClient.subscribe(
hostName,
"mytopic",
this::handleIncomingMessage
);
}

private void handleIncomingMessage(NtfyMessage message) {
System.out.println("Mottaget meddelande: " + message.id() + ", tid: " + message.time());
long messageTimeSeconds = message.time();
long currentTimeSeconds = System.currentTimeMillis() / 1000;

// Hoppa över gamla meddelanden
if (messageTimeSeconds < (subscriptionStartTime / 1000)) {
System.out.println("Hoppar över (gammalt meddelande).");
return;
}

// Hoppa över dubbletter
if (isDuplicateMessage(message)) {
System.out.println("Hoppar över (dublett).");
return;
}

// Lägg till meddelandet i modellen
System.out.println("Lägger till i modellen: " + message.message());
model.addMessage(message);
}

private boolean isDuplicateMessage(NtfyMessage message) {
return model.getMessages().stream()
.anyMatch(existingMessage -> existingMessage.id().equals(message.id()));
}


@FXML
private void unsubscribeFromTopic() {
if (subscription != null) {
closeSubscription();
model.setConnected(false);
subscription = null; // Rensa referensen
}
}

private void closeSubscription() {
try {
subscription.close();
} catch (IOException e) {
System.err.println("Fel vid avprenumeration: " + e.getMessage());
}
}
}
33 changes: 31 additions & 2 deletions src/main/java/com/example/HelloFX.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
package com.example;

import io.github.cdimascio.dotenv.Dotenv;
import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.stage.Stage;

import java.util.Objects;

public class HelloFX extends Application {

@Override
public void start(Stage stage) throws Exception {
FXMLLoader fxmlLoader = new FXMLLoader(HelloFX.class.getResource("hello-view.fxml"));
Dotenv dotenv = Dotenv.load(); //Ladda .env-filen

String hostName = dotenv.get("HOST_NAME");

//Validering (valfritt)
if (hostName == null) {
throw new IllegalStateException(
"Kunde inte läsa HOST_NAME från .env-filen. " +
"Kontrollera att filen finns i projektets rotmapp!"
);
}

System.out.println("Ansluter till ntfy på: " + hostName); //Debug-logg
Comment on lines +16 to +28
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

Add error handling for .env file loading.

The code validates HOST_NAME but doesn't handle the case where the .env file itself doesn't exist. Dotenv.load() might throw an exception.

Apply this diff to add proper error handling:

-        Dotenv dotenv = Dotenv.load(); //Ladda .env-filen
+        Dotenv dotenv = null;
+        try {
+            dotenv = Dotenv.load();
+        } catch (Exception e) {
+            throw new IllegalStateException(
+                "Could not load .env file. Ensure it exists in the project root: " + e.getMessage()
+            );
+        }
 
         String hostName = dotenv.get("HOST_NAME");
 
-        //Validering (valfritt)
         if (hostName == null) {
             throw new IllegalStateException(
-                    "Kunde inte läsa HOST_NAME från .env-filen. " +
-                    "Kontrollera att filen finns i projektets rotmapp!"
+                    "Could not read HOST_NAME from .env file. " +
+                    "Ensure the variable is defined in the .env file!"
             );
         }
 
-        System.out.println("Ansluter till ntfy på:  " + hostName); //Debug-logg
+        System.out.println("Connecting to ntfy at: " + hostName);

Note: I've also translated the Swedish comments to English for consistency.

📝 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
Dotenv dotenv = Dotenv.load(); //Ladda .env-filen
String hostName = dotenv.get("HOST_NAME");
//Validering (valfritt)
if (hostName == null) {
throw new IllegalStateException(
"Kunde inte läsa HOST_NAME från .env-filen. " +
"Kontrollera att filen finns i projektets rotmapp!"
);
}
System.out.println("Ansluter till ntfy på: " + hostName); //Debug-logg
Dotenv dotenv = null;
try {
dotenv = Dotenv.load();
} catch (Exception e) {
throw new IllegalStateException(
"Could not load .env file. Ensure it exists in the project root: " + e.getMessage()
);
}
String hostName = dotenv.get("HOST_NAME");
if (hostName == null) {
throw new IllegalStateException(
"Could not read HOST_NAME from .env file. " +
"Ensure the variable is defined in the .env file!"
);
}
System.out.println("Connecting to ntfy at: " + hostName);


HelloModel model = new HelloModel();
ChatNetworkClient httpClient = new NtfyHttpClient();

FXMLLoader fxmlLoader = new FXMLLoader(
HelloFX.class.getResource("/com/example/hello-view.fxml")
);
fxmlLoader.setControllerFactory(c -> new HelloController(model, httpClient, hostName));

Parent root = fxmlLoader.load();
Scene scene = new Scene(root, 640, 480);
stage.setTitle("Hello MVC");

scene.getStylesheets().add(
Objects.requireNonNull(getClass().getResource("/com/example/styles.css")).toExternalForm()
);

stage.setTitle("Chat App");
stage.setScene(scene);
stage.show();
}
Expand Down
Loading
Loading