Conversation
…r pattern, Singleton and exceptions
…metoderna i HelloController.java
WalkthroughThis PR adds chat network capabilities by introducing a ChatNetworkClient interface, NtfyHttpClient implementation for HTTP-based messaging, and subscription lifecycle management. It refactors the application with dependency injection, updates the UI with message lists and subscription controls, enhances HelloModel with thread-safe message storage and connection state tracking, and adds Maven dependencies for networking, JavaFX, and testing frameworks. Changes
Sequence DiagramssequenceDiagram
participant User
participant HelloController
participant HelloModel
participant NtfyHttpClient
participant NtfyServer
User->>HelloController: Click Subscribe
HelloController->>NtfyHttpClient: subscribe(baseUrl, topic, handler)
NtfyHttpClient->>NtfyServer: GET /topic/json (SSE stream)
NtfyServer-->>NtfyHttpClient: [stream of messages]
loop For each message
NtfyHttpClient->>NtfyHttpClient: Parse & filter (event=message)
NtfyHttpClient->>HelloController: handler.accept(message)
HelloController->>HelloController: Check duplicate & old messages
HelloController->>HelloModel: addMessage(NtfyMessage)
HelloModel-->>HelloController: runOnFX (thread-safe update)
end
User->>HelloController: Click Unsubscribe
HelloController->>NtfyHttpClient: subscription.close()
NtfyHttpClient-->>NtfyServer: Close stream
sequenceDiagram
participant User
participant HelloController
participant NtfyHttpClient
participant NtfyServer
User->>HelloController: Enter text & click Send
HelloController->>HelloController: Create NtfyMessage
HelloController->>NtfyHttpClient: send(baseUrl, message)
NtfyHttpClient->>NtfyServer: POST /topic (JSON payload)
NtfyServer-->>NtfyHttpClient: HTTP 200 OK
NtfyHttpClient-->>HelloController: return
Note over HelloController: Message sent successfully
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30–45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (6)
src/main/java/com/example/NtfyMessage.java (1)
7-12: Consider validation and nullability of record fields.The record allows all fields to be null, which could lead to issues downstream. Consider:
- Making
id,time,event, andtopicnon-null where appropriate- Adding validation or using
@NotNullannotations on the record componentsFor example:
@JsonIgnoreProperties(ignoreUnknown = true) public record NtfyMessage( - String id, - Long time, - String event, - String topic, + @NotNull String id, + long time, + @NotNull String event, + @NotNull String topic, String message) {Note:
timecould be a primitivelongif it should never be null, and the message field could remain nullable if that's the intended behavior.src/test/java/com/example/HelloModelTest.java (1)
15-32: Fix indentation inconsistency.The test logic is correct, but there's inconsistent indentation. Line 10 has proper indentation while line 17 has extra indentation.
Apply this diff to fix the indentation:
- @Test - void testAddMessage_AddsMessageToList() { - HelloModel model = new HelloModel(); - model.setTesting(true); //Aktivera testläge + @Test + void testAddMessage_AddsMessageToList() { + HelloModel model = new HelloModel(); + model.setTesting(true); //Aktivera testlägeAdditionally, consider translating the Swedish comment to English for consistency:
//Aktivera testläge→//Enable testing modesrc/main/java/com/example/HelloFX.java (1)
41-43: Add error handling for missing stylesheet.
Objects.requireNonNull()will throw aNullPointerExceptionwith no context if the stylesheet is missing. Consider providing a more descriptive error message.Apply this diff:
scene.getStylesheets().add( - Objects.requireNonNull(getClass().getResource("/com/example/styles.css")).toExternalForm() + Objects.requireNonNull( + getClass().getResource("/com/example/styles.css"), + "Could not find styles.css in resources" + ).toExternalForm() );src/main/resources/com/example/hello-view.fxml (1)
1-34: LGTM with internationalization consideration.The FXML layout is well-structured with appropriate controls and bindings. All necessary
fx:idattributes are defined for controller access.Consider internationalizing the Swedish text for a broader audience:
- "Meddelanden: " → "Messages: "
- "Skriv ett meddelande..." → "Write a message..."
- "Skicka" → "Send"
- "Prenumerera" → "Subscribe"
- "Avbryt prenumeration" → "Unsubscribe"
- "Ansluten: nej" → "Connected: no"
src/main/java/com/example/HelloController.java (2)
79-81: Improve error handling.Errors are only printed to
System.err, which provides poor user experience. Consider showing error messages in the UI.For example, add a label for error display in the FXML and update it when errors occur:
private void handleSendError(Exception e) { Platform.runLater(() -> { errorLabel.setText("Send failed: " + e.getMessage()); errorLabel.setVisible(true); }); }
56-67: Add input validation.The method checks for empty strings but doesn't validate message length or content. Consider adding validation for:
- Maximum message length
- Potentially harmful characters
- Whitespace-only messages
private void sendMessage() { - String messageText = messageTextField.getText(); + String messageText = messageTextField.getText().trim(); - if (!messageText.isEmpty()) { + if (!messageText.isEmpty() && messageText.length() <= 4096) { try { NtfyMessage message = createMessage(messageText); httpClient.send(hostName, message); messageTextField.clear(); } catch (Exception e) { handleSendError(e); } + } else if (messageText.length() > 4096) { + // Show error: message too long } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/resources/com/example/background.jpgis excluded by!**/*.jpg
📒 Files selected for processing (13)
.gitignore(1 hunks)pom.xml(4 hunks)src/main/java/com/example/ChatNetworkClient.java(1 hunks)src/main/java/com/example/HelloController.java(1 hunks)src/main/java/com/example/HelloFX.java(1 hunks)src/main/java/com/example/HelloModel.java(1 hunks)src/main/java/com/example/NtfyHttpClient.java(1 hunks)src/main/java/com/example/NtfyMessage.java(1 hunks)src/main/java/com/example/Subscription.java(1 hunks)src/main/java/module-info.java(1 hunks)src/main/resources/com/example/hello-view.fxml(1 hunks)src/main/resources/com/example/styles.css(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/main/resources/com/example/styles.css
[error] 2-2: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 3-3: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 4-4: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 8-8: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 9-9: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 11-11: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 15-15: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 16-16: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 17-17: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 18-18: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 19-19: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 22-22: Unexpected unknown pseudo-class odd
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 23-23: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 26-26: Unexpected unknown pseudo-class even
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 27-27: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 31-31: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 35-35: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
🔇 Additional comments (13)
.gitignore (1)
3-4: Good additions to .gitignore.Both entries align with best practices:
.envprevents accidental commits of environment configuration files that may contain secrets or local settings, which is important given the PR's introduction of environment configuration handling.*.imlprevents IntelliJ IDEA module files from being versioned, which is standard practice for Java development.src/main/java/module-info.java (1)
4-8: All dependency requirements verified and properly configured.The module requirements are well-established:
- jackson-databind version 2.17.0 is secure; latest version is 2.20.1, with all known CVEs affecting only older versions (pre-2.9.10, 2.10-2.12, 2.13.x)
- dotenv-java has no vulnerabilities detected
- java-dotenv 3.1.7 has no known vulnerabilities (your version 3.1.0 is also clean)
- All dependencies are properly declared in pom.xml with explicit versions
.envfiles are recommended to be removed or restricted from production systems, and your repository correctly includes.envin.gitignorewith no committed secretsThe module descriptor changes are correct and secure.
src/main/java/com/example/NtfyMessage.java (1)
14-18: LGTM with minor note.The
toString()implementation provides a sensible fallback. The@NotNullannotation correctly signals that this method never returns null.pom.xml (4)
14-14: LGTM: JUnit version corrected.The version has been changed to 5.9.2, which is a valid JUnit Jupiter version.
22-32: LGTM: JUnit dependencies properly split.The JUnit Jupiter dependency has been correctly split into
junit-jupiter-apiandjunit-jupiter-enginewith proper test scope.
95-105: LGTM: Resource configuration is appropriate.The resource filtering configuration correctly includes FXML, CSS, and image files, which aligns with the JavaFX application requirements.
72-81: Dependencies are secure and current.Based on the security vulnerability check:
- jackson-databind 2.17.0: Not affected by any known vulnerabilities. All returned advisories apply to older versions (up to 2.13.x); version 2.17.0 exceeds all vulnerable ranges.
- dotenv-java 3.1.0: No known vulnerabilities found.
Both versions are current and free from known security issues.
src/test/java/com/example/HelloModelTest.java (1)
8-12: LGTM: Test validates initial state.The test correctly verifies that a new
HelloModelinstance starts with an empty message list.src/main/java/com/example/HelloFX.java (1)
30-36: LGTM: Dependency injection properly implemented.The use of a controller factory to inject
HelloModel,ChatNetworkClient, andhostNameintoHelloControlleris a clean implementation of dependency injection.src/main/java/com/example/HelloController.java (2)
111-131: LGTM: Message filtering logic is sound.The duplicate detection and timestamp filtering appropriately prevent old and duplicate messages from being added to the list. The debug logging is helpful for troubleshooting.
33-37: LGTM: Constructor-based dependency injection.The constructor properly implements dependency injection, making the class testable and adhering to SOLID principles.
src/main/resources/com/example/styles.css (1)
1-36: CSS styling verified and correct.The CSS file uses valid JavaFX-specific properties and pseudo-classes. The static analysis warnings from Biome are false positives—it doesn't recognize JavaFX CSS syntax. The background image reference is valid;
background.jpgexists atsrc/main/resources/com/example/background.jpg.src/main/java/com/example/HelloModel.java (1)
18-39: Good separation of model state with testable hooksThe additions of:
ObservableList<NtfyMessage> messages,BooleanProperty connected,getMessages(),connectedProperty(), andsetTesting(boolean),give a clean, FX‑friendly model surface while keeping tests simple (no JavaFX toolkit required when
isTestingis true). Exposing the property asReadOnlyBooleanPropertyis also a nice touch to keep writes inside the model.No changes needed here from my side.
| <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> |
There was a problem hiding this comment.
🧩 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.
| <dependency> | ||
| <groupId>org.testng</groupId> | ||
| <artifactId>testng</artifactId> | ||
| <version>RELEASE</version> | ||
| <scope>compile</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Fix TestNG dependency configuration.
There are two critical issues:
RELEASEversion: This creates non-reproducible builds as it always fetches the latest versioncompilescope: Test frameworks should usetestscope, notcompile
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.
| 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(); | ||
| } |
There was a problem hiding this comment.
🧩 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
Subscriptionso everything usesChatNetworkClient.Subscription, and reference it explicitly (ChatNetworkClient.Subscription) in consumers.
- Remove the inner interface and change this signature to
- Once you pick one, ensure
NtfyHttpClient.subscribeand 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 -C2Length of output: 4355
Resolve critical Subscription type mismatch
Verification confirms a type conflict:
- Top-level
com.example.Subscriptionexists (src/main/java/com/example/Subscription.java) - Inner
ChatNetworkClient.Subscriptionexists (src/main/java/com/example/ChatNetworkClient.java:14) NtfyHttpClient.subscribe()returns the top-levelSubscription(line 85)ChatNetworkClient.subscribe()declares return type as unqualifiedSubscription(line 11), which resolves to the inner interface in this contextHelloControllerfield is typed explicitly asChatNetworkClient.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; changeChatNetworkClient.subscribe()to returncom.example.Subscription, and updateHelloController.subscriptionfield to the same type. - Option 2: Remove the top-level
Subscription; haveNtfyHttpClient.subscribe()returnChatNetworkClient.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.
| private final HelloModel model; | ||
| private final ChatNetworkClient httpClient; // Abstraktion (Dependency Inversion) | ||
| private final String hostName; | ||
| private ChatNetworkClient.Subscription subscription; | ||
| private long subscriptionStartTime; |
There was a problem hiding this comment.
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.
| private NtfyMessage createMessage(String text) { | ||
| return new NtfyMessage( | ||
| UUID.randomUUID().toString(), | ||
| System.currentTimeMillis(), | ||
| "message", | ||
| "mytopic", | ||
| text | ||
| ); | ||
| } |
There was a problem hiding this comment.
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.
| 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).
| 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 |
There was a problem hiding this comment.
🛠️ 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.
| 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); |
| // Ersätt den gamla runOnFX-metoden med denna: | ||
| private void runOnFX(Runnable task) { | ||
| if (isTesting) { // <-- Ny logik för tester | ||
| task.run(); | ||
| } else if (Platform.isFxApplicationThread()) { | ||
| task.run(); | ||
| } else { | ||
| try { | ||
| Platform.runLater(task); | ||
| } catch (IllegalThreadStateException notInitialized) { | ||
| task.run(); // Fallback för enhetstester (om JavaFX inte är initierat) | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| public void addMessage(NtfyMessage message) { | ||
| runOnFX(() -> messages.add(message)); | ||
| } | ||
|
|
||
| public void setConnected(boolean connected) { | ||
| runOnFX(() -> this.connected.set(connected)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Fix fallback handling in runOnFX – wrong exception type
The idea of runOnFX is solid, but the fallback path is currently broken:
} else {
try {
Platform.runLater(task);
} catch (IllegalThreadStateException notInitialized) {
task.run();
}
}Platform.runLaterthrowsIllegalStateExceptionif the JavaFX toolkit is not initialized, notIllegalThreadStateException. As written, the catch block will never handle that case, and tests or code paths without initialized JavaFX will still blow up.
Update the catch to handle IllegalStateException (or RuntimeException if you want to be broader), e.g.:
private void runOnFX(Runnable task) {
if (isTesting) {
task.run();
} else if (Platform.isFxApplicationThread()) {
task.run();
} else {
try {
Platform.runLater(task);
} catch (IllegalStateException notInitialized) {
// Fallback for unit tests (JavaFX not initialized)
task.run();
}
}
}This will make your test fallback actually work as intended.
🌐 Web query:
In JavaFX, what exception does `Platform.runLater` throw when the toolkit is not initialized?
💡 Result:
It throws java.lang.IllegalStateException (message: "Toolkit not initialized"). [1][2]
Sources:
[1] OpenJFX PlatformImpl.runLater source showing the throw.
[2] JavaFX Platform javadoc describing runLater must not be called before the FX runtime is initialized.
Fix fallback handling in runOnFX – wrong exception type
The fallback path has a critical flaw: Platform.runLater throws java.lang.IllegalStateException (not IllegalThreadStateException) when the JavaFX toolkit is not initialized. As written, the catch block will never execute, and the exception will propagate uncaught.
Update line 50 in src/main/java/com/example/HelloModel.java:
} catch (IllegalStateException notInitialized) {
task.run();
}🤖 Prompt for AI Agents
In src/main/java/com/example/HelloModel.java around lines 41 to 63, the catch
block for Platform.runLater is using the wrong exception type
(IllegalThreadStateException) so the fallback never triggers; change the catch
to catch java.lang.IllegalStateException instead and call task.run() there as
the fallback for uninitialized JavaFX.
| public void send(String baseUrl, NtfyMessage message) throws Exception{ | ||
| //Builder Pattern för HttpRequest | ||
| HttpRequest request = HttpRequest.newBuilder() | ||
| .POST(HttpRequest.BodyPublishers.ofString(message.message())) | ||
| .header("Content-Type", "application/json") | ||
| .uri(URI.create(baseUrl + "/" + message.topic())) | ||
| .build(); | ||
|
|
||
| //Debug-loggning | ||
| System.out.println("Skickar till: " + baseUrl + "/" + message.topic()); | ||
| System.out.println("Meddelande: " + message.message()); | ||
|
|
||
| //Synkront anrop (blockerande) | ||
| HttpResponse<Void> response = http.send( | ||
| request, | ||
| HttpResponse.BodyHandlers.discarding() | ||
| ); | ||
| //validera statuskoden | ||
| if (response.statusCode() >= 400) { | ||
| throw new IOException("Fel vid sändning: HTTP " + response.statusCode()); | ||
| } | ||
| System.out.println("Meddelandet skickat! Statuskod: " + response.statusCode()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify payload format and avoid blocking the FX thread in send
Two points here:
-
Blocking call on send
http.send(...)is synchronous. Ifsendis invoked from the JavaFX UI thread (e.g. button click handler), the UI will freeze until the HTTP call returns. For better UX, either:- Run
sendon a background thread /ExecutorService, or - Switch to
sendAsyncand notify the UI when the send completes or fails.
- Run
-
Payload vs Content‑Type
You set"Content-Type", "application/json"but sendmessage.message()directly as the body. Ifmessage.message()is plain text (as in typical ntfy usage), the header should likely betext/plain. If you really intend JSON, serialize the entireNtfyMessagewithObjectMapperinstead of sending justmessage().
Consider tightening the declared exception type (e.g. IOException) once you settle on the error‑handling strategy.
🌐 Web query:
ntfy.sh HTTP API: what Content-Type is recommended when posting plain text messages, and is JSON required or optional?
💡 Result:
Use Content-Type: text/plain for plain-text messages (or omit it — ntfy treats request bodies as plain text by default). JSON is optional — you only need to POST/PUT JSON to the ntfy root (https://ntfy.sh/) when you want to publish using the JSON message format (including a required "topic" field). [1][2]
Avoid blocking the FX thread and correct Content-Type header for plain-text messages
Two issues to address:
-
Blocking call on send
http.send(...)is synchronous. If invoked from the JavaFX UI thread (e.g., button click handler), the UI will freeze until the HTTP call completes. Either runsendon a background thread or switch tosendAsyncwith a callback to notify the UI. -
Content-Type header
ntfy.sh treats request bodies as plain text by default and recommendsContent-Type: text/plainfor plain-text messages. Your code sendsmessage.message()(plain text) with"Content-Type", "application/json", which is incorrect. Change the header totext/plain.
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyHttpClient.java around lines 24 to 46, the
method blocks the JavaFX UI thread by calling http.send(...) synchronously and
also sets an incorrect Content-Type for plain-text messages; change the header
from "application/json" to "text/plain" and make the HTTP call non-blocking
(either use http.sendAsync(...) and handle the CompletableFuture to validate
status and report errors back to the UI via Platform.runLater, or execute the
existing send call on a background thread/ExecutorService), ensure you still
check response.statusCode() >= 400 and propagate or surface errors to the UI
callback rather than throwing on the FX thread.
| public Subscription subscribe(String baseUrl, String topic, Consumer<NtfyMessage> messageHandler) { | ||
| AtomicBoolean isOpen = new AtomicBoolean(true); //Spåra prenumerationens status | ||
|
|
||
| http.sendAsync( | ||
| HttpRequest.newBuilder() | ||
| .GET() | ||
| .uri(URI.create(baseUrl + "/" + topic + "/json")) | ||
| .build(), | ||
| HttpResponse.BodyHandlers.ofLines() | ||
| ).thenAccept(response -> { | ||
| System.out.println("Prenumeration startad för topic: " + topic); //Debug-loggning | ||
|
|
||
| //Logga varje rad som mottas från servern | ||
| response.body() | ||
| .peek(line -> System.out.println("Mottagen rad från servern: " + line)) | ||
| .takeWhile(line -> isOpen.get()) //Avbryt strömmen om isOpen = false | ||
| .map(line-> { | ||
| try { | ||
| NtfyMessage message = mapper.readValue(line, NtfyMessage.class); | ||
| System.out.println("Parsat meddelande: " + message); //Debug-loggning | ||
| return message; | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| }) | ||
| .filter(message -> message.event().equals("message")) | ||
| .forEach(message -> { | ||
| if (isOpen.get()) { //Endast om prenumerationen är öppen | ||
| Platform.runLater(() -> messageHandler.accept(message)); | ||
| } | ||
| }); | ||
| }).exceptionally(error -> { | ||
| System.err.println("Fel vid prenumeration: " + error.getMessage()); | ||
| return null; | ||
| }); | ||
|
|
||
| //Command Pattern: Returnera ett Subscription-objekt | ||
| return new Subscription() { | ||
| @Override | ||
| public void close() throws IOException { | ||
| isOpen.set(false); //Stäng strömmen | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isOpen() { | ||
| return isOpen.get(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Improve subscription lifecycle: type alignment and actual cancellation
A few things to tighten up in subscribe:
-
Subscription type alignment
This method returns an unqualifiedSubscription, whileChatNetworkClient.subscribecurrently returnsChatNetworkClient.Subscription. Ifcom.example.Subscriptionalso exists, this can easily become a type mismatch. Once you resolve the duplication inChatNetworkClient, make sure this method’s return type matches exactly (and add@Override). -
Close should cancel the HTTP request, not just stop processing lines
close()only setsisOpentofalse. That stops the stream pipeline (takeWhile), but the underlying HTTP connection andCompletableFuturekeep running until the server closes the stream. Prefer:- Storing the
CompletableFuture<HttpResponse<Stream<String>>>fromsendAsync, and - Calling
future.cancel(true)and/or closing the body whenclose()is invoked.
- Storing the
-
Error handling and robustness
- JSON parse errors currently throw
RuntimeExceptionand will terminate the stream. For a chat client, it’s usually better to log and skip malformed lines. message.event().equals("message")will NPE ifevent()is null; a defensive check ("message".equals(message.event())) avoids that.
- JSON parse errors currently throw
These changes will give you a more predictable and controllable subscription lifecycle.
🏁 Script executed:
#!/bin/bash
# Check how Subscription is used with NtfyHttpClient.subscribe
rg -n "subscribe\\(" src/main/java/com/example -C3Length of output: 1949
---
🏁 Script executed:
```shell
#!/bin/bash
# Check NtfyHttpClient class declaration and what it implements
rg -n "^class NtfyHttpClient" src/main/java/com/example -A5
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for standalone Subscription interface definition
rg -n "^\s*interface Subscription|^\s*public interface Subscription" src/main/java/com/exampleLength of output: 195
🏁 Script executed:
#!/bin/bash
# Check if @Override annotation exists on the subscribe method
rg -n "@Override" src/main/java/com/example/NtfyHttpClient.java -B2 -A2Length of output: 523
🏁 Script executed:
#!/bin/bash
# Check full ChatNetworkClient interface to understand the relationship
cat src/main/java/com/example/ChatNetworkClient.javaLength of output: 545
---
🏁 Script executed:
```shell
#!/bin/bash
# Find NtfyHttpClient class with more flexible pattern
rg -n "class NtfyHttpClient" src/main/java/com/example -A3
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Check imports and full context of NtfyHttpClient
head -50 src/main/java/com/example/NtfyHttpClient.javaLength of output: 1902
🏁 Script executed:
#!/bin/bash
# Check what HelloController imports and uses
head -30 src/main/java/com/example/HelloController.javaLength of output: 1094
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full subscribe method implementation to verify cancellation issue
sed -n '48,96p' src/main/java/com/example/NtfyHttpClient.java
Length of output: 2133
🏁 Script executed:
#!/bin/bash
# Check if Subscription.java is ever used elsewhere to confirm it's unrelated to ChatNetworkClient
grep -r "implements Subscription\|extends Subscription" src/main/java/com/example --include="*.java"Length of output: 0
Fix subscription implementation: type mismatch, missing request cancellation, and error handling
Three issues require fixing in subscribe:
-
Return type mismatch
Method returns unqualifiedSubscription(resolves tocom.example.Subscription), butChatNetworkClientinterface declaresChatNetworkClient.Subscription. Add@Overrideand qualify the return type:ChatNetworkClient.Subscriptionor use fully qualified form in the anonymous class creation at line 85. -
Close does not cancel the HTTP request
close()only setsisOpentofalse. TheCompletableFuturefromsendAsync()(line 51) is not stored, so there's no way to cancel it. Store the future and callfuture.cancel(true)inclose()to actually terminate the HTTP connection. -
Error handling leaves stream vulnerable
- Line 73: JSON parse errors throw
RuntimeException, terminating the stream. Log and skip malformed lines instead. - Line 77:
message.event().equals("message")throws NPE ifevent()is null. Use"message".equals(message.event())for safety.
- Line 73: JSON parse errors throw
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyHttpClient.java around lines 48–96, the
subscribe implementation has three issues: the anonymous Subscription resolves
to the wrong type, the async request future isn’t stored/cancelled, and
parsing/null-safety error handling will kill the stream. Fix by annotating the
method with @Override and returning/creating ChatNetworkClient.Subscription
(fully qualified) for the anonymous class; capture the CompletableFuture
returned by http.sendAsync(...) into a local variable and call
future.cancel(true) inside close() after setting isOpen=false; replace the
RuntimeException on JSON parse failure with logging and skipping the malformed
line (do not rethrow) so the stream continues; and make the event check
null-safe by using a null-safe comparison (e.g.
"message".equals(message.event())).
| public interface Subscription extends Closeable { | ||
|
|
||
| @Override | ||
| void close() throws IOException; | ||
| boolean isOpen(); | ||
| } |
There was a problem hiding this comment.
Clarify interface location and remove redundant override.
Two observations:
- The
@Overrideannotation on line 9 is redundant sinceclose()is already declared inCloseable - The AI summary indicates this interface is defined as a nested interface inside
ChatNetworkClient, but this file defines it as a top-level interface
Apply this diff to remove the redundant annotation:
public interface Subscription extends Closeable {
- @Override
void close() throws IOException;
boolean isOpen();
}Additionally, verify whether this should be a nested interface in ChatNetworkClient or a standalone interface to avoid duplication.
📝 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.
| public interface Subscription extends Closeable { | |
| @Override | |
| void close() throws IOException; | |
| boolean isOpen(); | |
| } | |
| public interface Subscription extends Closeable { | |
| void close() throws IOException; | |
| boolean isOpen(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/Subscription.java around lines 6 to 11, remove the
redundant @Override annotation on the close() declaration (close() is already
declared by Closeable) and then reconcile the type location: search for a nested
Subscription interface inside ChatNetworkClient; if ChatNetworkClient already
defines Subscription, delete this duplicate top-level interface and update
references/imports to use the nested type (or conversely, move the nested
definition here and update usages to the new top-level type) so there is a
single canonical definition; ensure compilation by updating imports/usages and
running a quick build.
1 participant
First try
Summary by CodeRabbit
New Features
Tests