Conversation
WalkthroughAdds a JavaFX chat app "RuneChat": new MVC model with Ntfy connection interface and HTTP implementation, DTO and FX utilities, updated FXML and CSS, Maven/module updates (dotenv, Jackson, WireMock), .env ignored, and new unit/integration tests with a test spy. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Ctrl as HelloController
participant Model as HelloModel
participant Conn as NtfyConnectionImpl
participant Server as NtfyServer
rect `#e8f0ff`
note right of Ctrl: Send flow
User->>Ctrl: click Send
Ctrl->>Model: sendMessageAsync(text)
Model->>Model: validate message
Model->>Conn: send(message, callback)
Conn->>Server: POST /{topic}
alt success (2xx)
Server-->>Conn: 2xx
Conn->>Model: callback(true)
Model->>Ctrl: runOnFx(clear & focus)
else failure
Server-->>Conn: error
Conn->>Model: callback(false)
Model->>Ctrl: runOnFx(show alert)
end
end
rect `#e8ffe8`
note right of Model: Receive flow
Model->>Conn: receive(handler)
Conn->>Server: GET /{topic}/json (stream)
loop per JSON line
Server-->>Conn: JSON line
Conn->>Conn: parse -> NtfyMessageDto
Conn->>Model: handler(dto)
Model->>Model: runOnFx(append message)
Model->>Ctrl: UI auto-scroll/render
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (15)
src/main/java/com/example/Singelton.java (1)
5-5: Consider using conventional modifier order:private static final.Java convention typically orders modifiers as
private static finalrather thanprivate final static. While both are valid, following convention improves consistency.src/main/java/com/example/ManyParameters.java (2)
5-8: Empty constructor with unused parameters.The constructor accepts five parameters but doesn't use them—no fields are initialized or logic executed. If this is a placeholder for demonstration purposes, consider adding a comment to clarify intent. Otherwise, either implement the constructor body or remove unused parameters.
public class ManyParameters { + // TODO: Implement constructor body - assign parameters to fields public ManyParameters(String computerName, int timeout, String method, int size, byte[] data) { - + // Constructor body not yet implemented }
11-18: Method namedmain()is not a valid Java entry point.This method is named
mainbut lacks the standard signaturepublic static void main(String[] args), so it cannot serve as a program entry point. The current signaturestatic void main()will not be recognized by the JVM. Consider either:
- Renaming to something like
demonstrateBuilder()to avoid confusion- Changing to the standard main signature if intended as an entry point
- static void main() { + static void demonstrateBuilder() { ManyParametersBuilder builder = new ManyParametersBuilder(); builder .setComputerName("localhost") //Fluent API .setTimeout(10) .setSize(0) .createManyParameters(); }src/test/java/com/example/NtfyConnectionSpy.java (1)
5-22: Spy behavior fits tests; consider tightening threading semanticsThe spy does what the tests need (records the last message and simulates async callbacks), but a couple of tweaks would make it more robust:
handler(and optionallymessage) is accessed across threads (model thread vs test thread callingsimulateIncoming). Marking itvolatileor guarding access via synchronization would give you proper visibility guarantees.- Spawning a new
Threadon everysendis fine for these tests, but if usage grows it may be cheaper and more deterministic to either:
- invoke the callback synchronously, or
- reuse a small
ExecutorServiceinstead of creating raw threads.src/main/java/com/example/NtfyConnection.java (1)
5-12: Clarify contract semantics and clean up legacy commentsThe interface shape looks reasonable, but since this is the core abstraction it would help to:
- Add brief Javadoc describing:
- whether
sendmay call the callback synchronously vs always asynchronously,- whether the callback is guaranteed to be invoked exactly once and on which thread,
- whether
receiveregisters a long‑lived subscription and what threading context the handler is called on.- Remove or move the commented‑out legacy methods into version control history or a design doc; keeping them inline at the top level adds noise and can confuse readers about the “real” API.
src/main/java/com/example/ManyParametersBuilder.java (1)
3-37: Builder implementation is correct; minor API polish possibleThe builder is straightforward and correct: fluent setters and
createManyParameters()constructingManyParameterswith the stored values.If you want to align more with common builder conventions:
- Consider renaming
createManyParameters()tobuild()for familiarity.- If some fields are logically required (e.g.
computerName,method), you could validate them increateManyParameters()and throw an exception or document thatnullis allowed.src/test/java/com/example/HelloModelTest.java (3)
33-167: Send-path tests are solid; consider de-duplicating invalid-input casesThe send-related tests (
shouldSendMessageThroughConnection,shouldHandleMultipleConsecutiveSends, and the invalid-input tests) give good coverage of success, multiple sends, and failure paths.The three invalid-input tests (
shouldRejectBlankMessages,shouldFailWhenSendingEmptyText,shouldFailWhenSendingNullMessage) largely overlap, both in intent and structure. You could:
- Collapse them into a single parameterized test (
""," ",nulletc.), or- Extract a small helper that runs the “send and expect failure” pattern.
This will make future changes to validation rules easier to maintain and keep behavior across inputs consistent.
204-298: Reduce reliance onThread.sleepin receive-path tests to avoid flakinessThe receive tests (
shouldAddIncomingMessageToList,shouldDiscardNullIncomingMessage,shouldIgnoreMessagesWithBlankContent,shouldRejectAllInvalidIncomingMessages) all useThread.sleep(100)to “give the listener time to attach” before callingsimulateIncoming(...).This works but is inherently fragile:
- On slow CI, 100 ms might occasionally be insufficient.
- The tests become timing-sensitive and can be flaky.
You could make this deterministic by:
- Extending
NtfyConnectionSpywith aCountDownLatchthat iscountDown()’d inreceive(...), and- In tests, waiting on that latch before calling
simulateIncoming(...).That way you synchronize on the actual subscription event rather than guessing via
sleep.
300-320: WireMock integration test is good; optionally assert the success flag tooThe WireMock-based integration test nicely verifies that a POST to
/mytopicis issued with body"Hello World".If you want to tighten the contract a bit more, you could also assert that the callback’s
successflag istrue(e.g. via anAtomicBoolean) to verify thatHelloModelinterprets the HTTP interaction as a successful send, not just that it made the request.src/main/java/com/example/HelloController.java (1)
13-21: Consider injecting dependencies and double-checking timestamp unitsTwo small design points here:
new HelloModel(new NtfyConnectionImpl())hardwires the controller to a concrete HTTP implementation, which makes UI testing and configuration (e.g. topics/URLs) harder. Long-term, consider injecting either aHelloModelor anNtfyConnection(constructor, setter, or some factory) so the controller isn’t responsible for network wiring.- The cell factory formats time with
Instant.ofEpochMilli(msg.time()). IfNtfyMessageDto.time()represents Unix seconds (as many HTTP APIs do), you likely wantInstant.ofEpochSecond(...)instead. If it’s already milliseconds, the current code is fine—worth verifying against your DTO mapping.Also applies to: 39-39
src/main/java/com/example/NtfyConnectionImpl.java (2)
20-33: Guard against nullhostNamein the second constructorThe default constructor defends against a missing
HOST_NAMEviaObjects.requireNonNull, but theNtfyConnectionImpl(String hostName)constructor accepts null and will later fail atURI.create(...)with a less obvious error.You can align the behavior with the no-arg constructor:
public NtfyConnectionImpl(String hostName) { - this.hostName = hostName; + this.hostName = Objects.requireNonNull(hostName, "hostName must not be null");
57-86: Improvereceiveerror handling and response validation
receivecurrently assumes the response is a valid line-delimited JSON stream without checking the HTTP status and relies on a broad try/catch around stream processing. For robustness:
- Check the status code before processing the body; log and return early on non-2xx.
- Consider logging the status and perhaps the first line/body on error for easier debugging.
- Optionally wrap
response.body()in a try-with-resources to explicitly close theStream<String>when done, which is the recommended pattern forBodyHandlers.ofLines().For example, inside
thenAccept:.thenAccept(response -> { if (response.statusCode() < 200 || response.statusCode() >= 300) { System.err.println("Receive failed with status: " + response.statusCode()); return; } try (var lines = response.body()) { lines .map(line -> { /* parse to NtfyMessageDto */ }) .filter(Objects::nonNull) .forEach(messageHandler); } catch (Exception e) { System.err.println("Stream processing error: " + e.getMessage()); } })src/main/java/com/example/HelloModel.java (3)
22-35: Good use of FX thread utility and message validation
startReceiving+isValidMessagenicely centralize inbound validation and ensure that mutations of theObservableListhappen on the FX thread viarunOnFx, which should keep JavaFX happy even whenNtfyConnectioncalls back from a background thread.One optional enhancement would be to log or otherwise surface invalid messages inside
isValidMessageto aid debugging when the server sends unexpected payloads.
37-54: Align callback threading semantics insendMessageAsyncThe send flow is generally solid: you validate input, delegate to
connection.send, and clear the text field only when the send succeeds and the user hasn’t changed the message meanwhile.However, the
callbackis invoked on different threads depending on the path:
- For blank/invalid messages (Lines 39–41),
callback.accept(false)runs on the caller’s thread (typically FX).- For async completion (Lines 44–53),
callback.accept(success)runs on whatever threadNtfyConnectionuses (likely an HTTP client worker).Callers that update UI in the callback may therefore hit FX-thread violations. To make behavior predictable, consider always dispatching the callback on the FX thread, for example:
connection.send(msg, success -> { if (success) { runOnFx(() -> { if (msg.equals(messageToSend.get())) { messageToSend.set(""); } }); } runOnFx(() -> callback.accept(success)); });And similarly wrap the early
callback.accept(false)inrunOnFx.
56-70: Consider exposing an unmodifiable view ofmessagesReturning the backing
ObservableListdirectly is common in small apps, but it lets external code modify the list arbitrarily, bypassing your validation and FX-thread discipline.If you want stricter encapsulation, consider:
public ObservableList<NtfyMessageDto> getMessages() { return FXCollections.unmodifiableObservableList(messages); }You’d still update
messagesinternally while callers observe but cannot mutate it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignore(1 hunks)pom.xml(3 hunks)src/main/java/com/example/FxUtils.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/ManyParameters.java(1 hunks)src/main/java/com/example/ManyParametersBuilder.java(1 hunks)src/main/java/com/example/NtfyConnection.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.java(1 hunks)src/main/java/com/example/NtfyMessageDto.java(1 hunks)src/main/java/com/example/Singelton.java(1 hunks)src/main/java/module-info.java(1 hunks)src/main/resources/com/example/hello-view.fxml(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/HelloModel.java (1)
src/main/java/com/example/FxUtils.java (1)
FxUtils(5-19)
🔇 Additional comments (11)
src/main/java/com/example/Singelton.java (1)
5-13: Solid singleton implementation.The eager initialization approach is thread-safe and appropriate for this use case. The pattern is correctly implemented with a private constructor and public accessor method.
.gitignore (1)
3-3: LGTM!Adding
.envto.gitignoreis the correct approach when using dotenv for configuration management. This prevents sensitive environment variables from being committed to version control.src/main/java/com/example/HelloFX.java (1)
15-15: LGTM!The scene width increase from 640 to 1280 pixels appropriately accommodates the expanded UI layout introduced in the FXML file, which now includes a ListView, TextArea, and additional controls.
src/main/java/com/example/NtfyMessageDto.java (1)
1-7: LGTM!The record definition is clean and appropriate for a DTO. The
@JsonIgnoreProperties(ignoreUnknown = true)annotation provides good resilience against API changes or additional fields in the JSON response.src/main/resources/com/example/hello-view.fxml (1)
22-22: No action required—HelloController already implements the requested cell factory.HelloController implements a custom cell factory for messageView that overrides
updateItem()to format the display with timestamps and message content. The implementation properly formats output as[timestamp] messageusingtimeFormatter.format(Instant.ofEpochMilli(msg.time()))andmsg.message(), addressing the exact concern raised in the review comment.Likely an incorrect or invalid review comment.
src/main/java/module-info.java (1)
4-6: LGTM! Dependency scopes verified and align correctly.The module requirements are properly supported:
jackson-databindanddotenv-javaboth use default compile scope inpom.xml(no explicit<scope>tag) ✓java.net.httpis a built-in Java platform module (Java 9+), so nopom.xmlentry is required ✓All dependencies are correctly configured to support the new module declarations.
pom.xml (1)
48-57: The review comment is based on incorrect assumptions about Jackson groupId.The groupId
tools.jackson.coreis not non-standard or a typo—it is the correct groupId for Jackson 3.x. The official Jackson 2.x library uses groupIdcom.fasterxml.jackson.core, but Jackson intentionally changed coordinates for version 3.x.Regarding versions: The latest Jackson 3.x release is 3.0.0 (released Oct 3, 2025). The pom.xml specifies 3.0.1, which appears to be a patch release beyond the confirmed 3.0.0. Additionally, dotenv-java 3.2.0 is already the latest stable version, so no update is needed there.
Consider verifying that version 3.0.1 for Jackson 3.x is intentional and available from your Maven repository, but the groupId usage is correct as-is.
Likely an incorrect or invalid review comment.
src/main/java/com/example/HelloController.java (1)
31-56: UI bindings, formatting, and auto-scroll behavior look goodThe controller wiring is clean:
messageLabel,messageView, andmessageInputare all bound to the model in a straightforward way.- The
ListCellformatting[HH:mm:ss] messageis clear and easy to read.- Auto-scrolling via a
ListChangeListenerplusPlatform.runLateris an appropriate way to keep the view on the latest message.- Error handling in
sendMessagecorrectly marshals UI updates back onto the FX thread viaPlatform.runLater.Also applies to: 59-69
src/main/java/com/example/NtfyConnectionImpl.java (1)
36-54: Asynchronous send logic looks solidThe async
sendimplementation correctly sets a per-request timeout, maps 2xx status codes totrue, and reliably invokes the callback both on normal completion and on exceptions viaexceptionally. This is a reasonable, straightforward design for the sending side.src/main/java/com/example/HelloModel.java (2)
13-20: Clean separation of concerns in model state and connectionStoring the
NtfyConnectiondependency, an observablemessageslist, and a dedicatedmessageToSendproperty gives a clear boundary between transport and UI state, and starting the receive loop in the constructor is a reasonable lifecycle choice for this app.
72-74: Greeting change is straightforwardThe simplified
getGreeting()returning"Welcome to ChatApp"is fine and matches the new chat-focused domain.
| <dependency> | ||
| <groupId>org.wiremock</groupId> | ||
| <artifactId>wiremock</artifactId> | ||
| <version>4.0.0-beta.15</version> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
Beta version of WireMock in use.
You're using WireMock 4.0.0-beta.15, which is a pre-release version. While this is test-scoped, beta versions may have stability issues or breaking changes. Consider whether a stable release would be more appropriate for your testing needs.
🌐 Web query:
What is the latest stable version of WireMock for Maven?
💡 Result:
The latest stable WireMock for Maven is org.wiremock:wiremock version 3.13.1 (released June 11, 2025). [1][2]
Maven dependency:
org.wiremock
wiremock
3.13.1
test
Sources: [1] GitHub releases — WireMock 3.13.1. [2] Maven Central / MVNRepository entry for org.wiremock:wiremock 3.13.1.
Consider upgrading to stable WireMock 3.13.1 for improved stability.
You're using WireMock 4.0.0-beta.15, which is a pre-release version. The latest stable version is WireMock 3.13.1 (released June 11, 2025). While beta versions may be intentional for forward compatibility testing, they can introduce stability issues and breaking changes. Upgrading to the stable release would provide better reliability for your test suite.
🤖 Prompt for AI Agents
In pom.xml around lines 58 to 63, the dependency currently pins WireMock to the
pre-release version 4.0.0-beta.15; change the <version> element to 3.13.1 to use
the stable WireMock release (i.e., replace 4.0.0-beta.15 with 3.13.1), then run
the test suite (mvn -DskipTests=false test) and fix any test API differences if
they surface.
| public static void runOnFx(Runnable task) { | ||
| try { | ||
| if (Platform.isFxApplicationThread()) { | ||
| task.run(); | ||
| } else { | ||
| Platform.runLater(task); | ||
| } | ||
| } catch (Exception e) { | ||
| //fallback for headless environments | ||
| task.run(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Catch block defeats thread-safety guarantees and swallows exceptions.
The broad catch (Exception e) block has several issues:
- Thread-safety concern: The fallback
task.run()executes on the current thread, potentially bypassing FX thread requirements and causing threading issues if the task manipulates UI components. - Silent failure: The exception is caught and ignored without logging, making debugging difficult.
- Overly broad catch: Catching all exceptions may hide legitimate errors like
NullPointerExceptionif task is null.
If Platform is not initialized (e.g., in tests), consider either:
- Letting the exception propagate to make the issue visible
- Adding explicit Platform initialization checks
- Logging the exception before the fallback
Consider this safer approach:
public static void runOnFx(Runnable task) {
- try {
- if (Platform.isFxApplicationThread()) {
- task.run();
- } else {
- Platform.runLater(task);
- }
- } catch (Exception e) {
- //fallback for headless environments
- task.run();
+ if (task == null) {
+ throw new IllegalArgumentException("Task cannot be null");
+ }
+
+ if (Platform.isFxApplicationThread()) {
+ task.run();
+ } else {
+ Platform.runLater(task);
}
}If headless environment support is truly needed, document the threading implications clearly and consider using a test-specific implementation instead.
Committable suggestion skipped: line range outside the PR's diff.
| import io.github.cdimascio.dotenv.Dotenv; | ||
| import tools.jackson.databind.ObjectMapper; |
There was a problem hiding this comment.
❓ Verification inconclusive
Incorrect Jackson ObjectMapper import will break compilation
The import for ObjectMapper appears wrong; the standard Jackson databind class lives under com.fasterxml.jackson.databind, not tools.jackson.databind. As written, this will not compile once you add the Jackson dependency.
Consider:
-import tools.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectMapper;If unsure, double-check the Jackson databind package name in your IDE or the official Jackson documentation.
Incorrect Jackson ObjectMapper import will break compilation
The correct import is com.fasterxml.jackson.databind.ObjectMapper, not tools.jackson.databind.ObjectMapper. The current import will fail to resolve and prevent compilation.
-import tools.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectMapper;📝 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.
| import io.github.cdimascio.dotenv.Dotenv; | |
| import tools.jackson.databind.ObjectMapper; | |
| import io.github.cdimascio.dotenv.Dotenv; | |
| import com.fasterxml.jackson.databind.ObjectMapper; |
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around lines 3 to 4, the
Jackson ObjectMapper import is wrong (tools.jackson.databind.ObjectMapper);
replace it with the correct fully-qualified import
com.fasterxml.jackson.databind.ObjectMapper so the class resolves and the
project compiles, and remove the incorrect import if present.
| @@ -0,0 +1,14 @@ | |||
| package com.example; | |||
|
|
|||
| public class Singelton { | |||
There was a problem hiding this comment.
Fix the class name typo: "Singelton" → "Singleton".
The class name is misspelled. Correcting it now prevents propagation throughout the codebase.
Apply this diff to fix the typo:
-public class Singelton {
+public class Singleton {You'll also need to update the constructor and instance references:
- private final static Singelton instance = new Singelton();
+ private static final Singleton instance = new Singleton();
- private Singelton(){
+ private Singleton(){Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/example/Singelton.java around line 3 the class name is
misspelled "Singelton"; rename the class to "Singleton" and update the filename
to match (Singleton.java), then update the constructor name and all references
to the class and any static instance fields/usages across the codebase (e.g.,
getInstance(), variable declarations, imports) to the corrected "Singleton"
identifier so compilation and references remain consistent.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/main/java/com/example/NtfyConnectionImpl.java (1)
3-4: Incorrect Jackson import - already flagged.This import issue was identified in previous reviews and remains unresolved.
🧹 Nitpick comments (6)
src/main/java/com/example/NtfyConnection.java (2)
3-5: Remove unused imports.The imports for
CompletableFutureandTimeUnitare not used in this interface. OnlyConsumeris actually referenced.Apply this diff to remove unused imports:
-import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; import java.util.function.Consumer;
13-23: Consider removing or documenting stub default implementations.The default implementations provide hardcoded return values and a no-op
setCurrentTopic. While this allows the interface to compile, it creates a misleading contract where callers might assume these methods work, but they don't update state.Consider either:
- Removing the default keyword to force implementers to provide real implementations, or
- Adding clear Javadoc warnings that these are stubs
Option 1 (preferred) - Remove defaults and make them abstract:
- default String getCurrentTopic() { - return "mytopic"; - } + String getCurrentTopic(); - default void setCurrentTopic(String topic) { - - } + void setCurrentTopic(String topic); - default String getUserId() { - return "unknown"; - } + String getUserId();src/main/java/com/example/HelloController.java (3)
44-46: Remove unusedtimeFormatterfield.The
DateTimeFormatteris declared but never used in the controller. This appears to be leftover code.Apply this diff to remove the unused field:
- private final DateTimeFormatter timeFormatter = - DateTimeFormatter.ofPattern("HH:mm:ss") - .withZone(ZoneId.systemDefault()); -
71-79: Unnecessary null check forchangeTopicButton.The null check on line 71 is unnecessary since
changeTopicButtonis always present in the FXML (line 39 of hello-view.fxml). This suggests defensive programming that isn't needed.Consider removing the null check:
- if (changeTopicButton != null) { - changeTopicButton.disableProperty().bind(Bindings.createBooleanBinding( - () -> { - String text = topicInput.getText(); - return text == null || text.trim().isEmpty(); - }, - topicInput.textProperty() - )); - } + changeTopicButton.disableProperty().bind(Bindings.createBooleanBinding( + () -> { + String text = topicInput.getText(); + return text == null || text.trim().isEmpty(); + }, + topicInput.textProperty() + ));
128-143: Consider consolidating consecutivePlatform.runLatercalls.Lines 131-132 use two separate
Platform.runLatercalls for operations that could be combined into a single call, reducing overhead.Apply this diff to combine the calls:
if (success) { - Platform.runLater(() -> messageInput.clear()); - Platform.runLater(() -> messageInput.requestFocus()); + Platform.runLater(() -> { + messageInput.clear(); + messageInput.requestFocus(); + }); } else {src/main/java/com/example/HelloModel.java (1)
72-93: Replace console logging with proper logging framework.Lines 75 and 89 use
System.out.printlnfor logging. Consider using a proper logging framework like SLF4J for production code.Example using SLF4J:
private static final Logger logger = LoggerFactory.getLogger(HelloModel.class); // Then replace System.out.println calls: logger.debug("Nothing to send!"); logger.error("Failed to send message!");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/FxUtils.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/NtfyConnection.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.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/style.css(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/java/com/example/HelloModelTest.java
- src/main/java/com/example/FxUtils.java
- src/main/java/module-info.java
- src/main/java/com/example/HelloFX.java
- src/test/java/com/example/NtfyConnectionSpy.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/HelloModel.java (1)
src/main/java/com/example/FxUtils.java (1)
FxUtils(5-18)
🪛 Biome (2.1.2)
src/main/resources/com/example/style.css
[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] 5-5: 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] 6-6: 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] 12-12: 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] 13-13: 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] 20-20: 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] 25-25: 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: 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] 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] 28-28: 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] 29-29: 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] 33-33: 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] 34-34: 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] 38-38: 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] 39-39: 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] 40-40: 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] 41-41: 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] 42-42: 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] 43-43: 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] 44-44: 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] 47-47: Unexpected unknown pseudo-class focused
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 48-48: 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] 49-49: 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] 53-53: 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] 54-54: 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] 55-55: 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] 56-56: 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] 57-57: 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] 58-58: 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] 59-59: 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] 63-63: 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] 67-67: 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] 72-72: 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] 73-73: 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] 74-74: 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] 75-75: 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] 76-76: 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] 80-80: 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] 81-81: 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] 84-84: Unexpected unknown pseudo-class selected
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 85-85: 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] 90-90: 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] 91-91: 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] 92-92: 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] 92-92: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 93-93: 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] 94-94: 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] 95-95: 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] 99-99: 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] 100-100: 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] 101-101: 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] 105-105: 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] 106-106: 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] 107-107: 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] 112-112: 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] 113-113: 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] 117-117: 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] 118-118: 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] 119-119: 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] 120-120: 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] 121-121: 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] 122-122: 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] 123-123: 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] 126-126: Unexpected unknown pseudo-class focused
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 127-127: 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] 128-128: 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] 133-133: 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] 134-134: 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] 135-135: 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] 136-136: 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] 137-137: 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] 138-138: 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] 139-139: 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] 140-140: 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] 140-140: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 144-144: 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] 148-148: 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] 152-152: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 153-153: 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] 156-156: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 157-157: 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] 158-158: 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] 161-161: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 162-162: 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 (8)
src/main/resources/com/example/style.css (1)
1-163: Static analysis warnings are false positives for JavaFX CSS.Biome is a web CSS linter that doesn't recognize JavaFX's CSS dialect. All properties prefixed with
-fx-(e.g.,-fx-background-color,-fx-padding,-fx-text-fill) and pseudo-classes like:focused,:selected,:hover, and:disabledare valid in JavaFX and will function correctly at runtime.The stylesheet is well-structured, uses a cohesive color scheme, and appropriately styles the chat UI components with distinct sent/received message bubbles, input controls, and messaging layout.
To resolve the static analysis noise, configure your CI/linting setup to exclude JavaFX CSS files from Biome checks. You can do this via
.biomeignoreor in your linting configuration by pattern-matching**/*.cssfiles in resource directories, or by disabling CSS linting specifically for JavaFX resource paths. If you use a tool like ESLint or similar in parallel, ensure they're also configured to skip JavaFX stylesheets.src/main/resources/com/example/hello-view.fxml (1)
1-49: LGTM! Well-structured FXML layout.The FXML layout is properly structured with correct fx:id bindings that match the controller fields, appropriate event handlers, and clean separation of concerns using CSS classes for styling.
src/main/java/com/example/NtfyConnection.java (1)
7-11: LGTM! Clean interface design for async messaging.The abstract methods use appropriate callback patterns for asynchronous send/receive operations.
src/main/java/com/example/HelloController.java (3)
82-114: LGTM! Well-implemented custom cell factory.The cell factory properly handles null/empty cases, creates appropriate chat bubble UI elements, and uses CSS classes for styling. The conditional alignment based on sender ID is correctly implemented.
117-124: LGTM! Proper auto-scroll implementation.The listener correctly uses
Platform.runLaterfor thread-safe UI updates and properly checks for empty list before scrolling.
146-152: LGTM! Clean topic change implementation.The method properly validates input and delegates to the model layer.
src/main/java/com/example/HelloModel.java (2)
26-48: LGTM! Standard JavaFX property pattern.The property accessors follow the correct JavaFX conventions for observable properties.
95-100: LGTM! Proper thread-safe message handling.The method correctly uses
runOnFxto ensure messages are added to the observable list on the JavaFX Application Thread, and validates message content before processing.
| public HelloModel(NtfyConnection connection) { | ||
| this.connection = connection; | ||
| this.currentTopic.set(connection.getCurrentTopic()); | ||
| receiveMessage(); | ||
| } |
There was a problem hiding this comment.
Avoid starting async operations in the constructor.
Line 23 calls receiveMessage(), which starts an asynchronous operation, before the object is fully constructed. This can lead to race conditions where callbacks run while the object is still being initialized.
Consider refactoring to separate construction from initialization:
public HelloModel(NtfyConnection connection) {
this.connection = connection;
this.currentTopic.set(connection.getCurrentTopic());
- receiveMessage();
}
+
+ public void start() {
+ receiveMessage();
+ }Then call model.start() after construction completes in the controller.
📝 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 HelloModel(NtfyConnection connection) { | |
| this.connection = connection; | |
| this.currentTopic.set(connection.getCurrentTopic()); | |
| receiveMessage(); | |
| } | |
| public HelloModel(NtfyConnection connection) { | |
| this.connection = connection; | |
| this.currentTopic.set(connection.getCurrentTopic()); | |
| } | |
| public void start() { | |
| receiveMessage(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloModel.java around lines 20 to 24, remove the
call to receiveMessage() from the constructor to avoid starting async work
during object construction; instead add a public start() (or init()) method that
calls receiveMessage(), keep the constructor limited to assigning fields
(connection and currentTopic), and update all callers (controllers/creators of
HelloModel) to invoke model.start() immediately after construction completes.
| public void setCurrentTopic(String topic) { | ||
| if (topic != null && !topic.isBlank()) { | ||
| connection.setCurrentTopic(topic); | ||
| this.currentTopic.set(topic); | ||
| messages.clear(); | ||
| receiveMessage(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential duplicate message listeners when changing topics.
Line 55 calls receiveMessage() every time the topic changes, but there's no mechanism to stop the previous listener. This could result in multiple concurrent receive operations, leading to duplicate messages or resource leaks.
Consider implementing a mechanism to:
- Cancel/stop the previous receive operation before starting a new one
- Track the active receive operation (e.g., store the
CompletableFuture) - Ensure only one receive operation is active at a time
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloModel.java around lines 50 to 57, calling
receiveMessage() on every topic change can spawn multiple concurrent listeners;
store the active receive operation in a field (e.g., a CompletableFuture<?>
activeReceive), before starting a new receive check if activeReceive != null and
not done, then cancel it (activeReceive.cancel(true)) or signal it to stop and
wait/handle completion, set activeReceive to the new CompletableFuture returned
by receiveMessage(), and ensure receiveMessage() honors
interruption/cancellation and clears activeReceive on normal completion or error
so only one receive operation is active at a time.
| public NtfyConnectionImpl() { | ||
| Dotenv dotenv = Dotenv.load(); | ||
| this.hostName = Objects.requireNonNull(dotenv.get("HOST_NAME")); | ||
| this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID"); | ||
| this.currentTopic = dotenv.get("DEFAULT_TOPIC", "mytopic"); | ||
| } |
There was a problem hiding this comment.
Fix Objects.requireNonNull error message parameter.
On line 24, the second parameter to Objects.requireNonNull should be a descriptive error message, not a variable name. The current code would throw NullPointerException: USER_ID, which is unclear.
Apply this diff to fix the error message:
Dotenv dotenv = Dotenv.load();
this.hostName = Objects.requireNonNull(dotenv.get("HOST_NAME"));
- this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID");
+ this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID environment variable is required");
this.currentTopic = dotenv.get("DEFAULT_TOPIC", "mytopic");📝 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 NtfyConnectionImpl() { | |
| Dotenv dotenv = Dotenv.load(); | |
| this.hostName = Objects.requireNonNull(dotenv.get("HOST_NAME")); | |
| this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID"); | |
| this.currentTopic = dotenv.get("DEFAULT_TOPIC", "mytopic"); | |
| } | |
| public NtfyConnectionImpl() { | |
| Dotenv dotenv = Dotenv.load(); | |
| this.hostName = Objects.requireNonNull(dotenv.get("HOST_NAME")); | |
| this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID environment variable is required"); | |
| this.currentTopic = dotenv.get("DEFAULT_TOPIC", "mytopic"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around lines 21 to 26, the
second parameter to Objects.requireNonNull for USER_ID is currently just the
variable name and should be a descriptive error message; replace the second
argument with a clear message such as "Environment variable USER_ID must be set"
(or similar) so that a NullPointerException provides actionable information when
USER_ID is missing.
| @Override | ||
| public void send(String message, Consumer<Boolean> callback) { | ||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||
| .POST(HttpRequest.BodyPublishers.ofString(message)) | ||
| .header("Cache", "no") | ||
| .header("X-User-Id", userId) | ||
| .uri(URI.create(hostName + "/" + currentTopic)) | ||
| .build(); | ||
|
|
||
| http.sendAsync(httpRequest, HttpResponse.BodyHandlers.discarding()) | ||
| .thenApply(response -> response.statusCode() / 100 == 2) | ||
| .exceptionally(ex -> { | ||
| System.out.println("Error sending message: " + ex.getMessage()); | ||
| return false; | ||
| }) | ||
| .thenAccept(callback); | ||
| } |
There was a problem hiding this comment.
Multiple issues in the send method.
- Line 56: The header name
"Cache"should be"Cache-Control"with an appropriate value like"no-cache". - Lines 64, 89: Using
System.out.printlnfor error logging is not production-ready. Consider using a proper logging framework. - Missing timeout: HTTP requests should have timeouts configured to prevent indefinite hangs.
Apply this diff to fix the cache header:
HttpRequest httpRequest = HttpRequest.newBuilder()
.POST(HttpRequest.BodyPublishers.ofString(message))
- .header("Cache", "no")
+ .header("Cache-Control", "no-cache")
.header("X-User-Id", userId)
.uri(URI.create(hostName + "/" + currentTopic))
.build();For logging and timeouts, consider:
- Replace
System.out.printlnwith a proper logger (e.g., SLF4J) - Configure HTTP client with timeouts:
private final HttpClient http = HttpClient.newBuilder() .connectTimeout(Duration.ofSeconds(10)) .build();
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around lines 52-68, the
send method uses the wrong Cache header, prints errors to stdout, and has no
timeouts; change the header to "Cache-Control" with value "no-cache", replace
System.out.println(...) with a proper logger call (e.g., use an SLF4J Logger
instance and call logger.error(..., ex)), and ensure the HttpClient is built
with a connectTimeout (e.g., Duration.ofSeconds(10)) and the HttpRequest sets a
request timeout (e.g., .timeout(Duration.ofSeconds(10))); add necessary imports
and a private static final Logger field for SLF4J.
| @Override | ||
| public void receive(Consumer<NtfyMessageDto> messageHandler) { | ||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||
| .GET() | ||
| .uri(URI.create(hostName + "/" + currentTopic + "/json")) | ||
| .build(); | ||
|
|
||
| http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines()) | ||
| .thenAccept(response -> response.body() | ||
| .map(s -> { | ||
| try { | ||
| return mapper.readValue(s, NtfyMessageDto.class); | ||
| } catch (Exception e) { | ||
| System.out.println("Failed to parse message: " + e.getMessage()); | ||
| return null; | ||
| } | ||
| }) | ||
| .filter(Objects::nonNull) | ||
| .peek(System.out::println) | ||
| .forEach(messageHandler)); | ||
| } |
There was a problem hiding this comment.
Remove debug code and address lifecycle management.
- Line 88:
peek(System.out::println)appears to be debug code that should be removed from production. - Lifecycle concern: The
receive()method starts an async HTTP stream but provides no way to cancel or stop it. If called multiple times (e.g., when changing topics), this could create multiple concurrent streams.
Apply this diff to remove debug code:
.map(s -> {
try {
return mapper.readValue(s, NtfyMessageDto.class);
} catch (Exception e) {
System.out.println("Failed to parse message: " + e.getMessage());
return null;
}
})
.filter(Objects::nonNull)
- .peek(System.out::println)
.forEach(messageHandler));Consider adding a mechanism to manage the async operation lifecycle, such as storing the CompletableFuture and providing a way to cancel it when switching topics or shutting down.
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around lines 70 to 90,
remove the debug peek(System.out::println) call and add lifecycle management for
the async HTTP stream: stop printing messages to stdout, capture the
CompletableFuture returned by http.sendAsync in a field (e.g.,
currentReceiveFuture), expose a cancel/stop method (or cancel in receive()
before starting a new one) that calls future.cancel(true) and handles
InterruptedException/CompletionException, and ensure you propagate or log
parsing errors appropriately instead of swallowing them; also ensure the future
is nulled after completion or cancellation to avoid leaked references.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/test/java/com/example/HelloModelTest.java (7)
26-44: Also assert the success flag for valid sendsThis test verifies that the spy captured
"Hello World", but doesn’t assert that thesuccessvalue passed to the callback istrue. If the model ever calls the callback withfalsewhile still forwarding the message, this test would still pass. Consider capturing the boolean and asserting it explicitly to pin down the contract for successful sends.
46-64: AvoidThread.sleepin async test to reduce flakinessRelying on
Thread.sleep(100)after the latch makes the test timing-dependent and more fragile on slow CI machines. It’s more robust to assert the clearedmessageToSendinside the callback (or via an additional latch / listener that fires when the field becomes empty) instead of sleeping.
66-114: Good coverage for empty and null messages; could be parameterizedThe two tests nicely cover both
""andnullinputs yieldingsuccess == falseand no call to the spy. If you want to DRY them up, you could convert them into a single parameterized test over the two inputs, but they’re perfectly fine as separate cases too.
148-178: Sequential send behavior verified; consider asserting both messagesThe test does a good job confirming that both callbacks succeed and that the last sent message wins in the spy. If you ever extend
NtfyConnectionSpyto record all sent messages (e.g., a list), you could tighten coverage further by asserting that both"First"and"Second"were sent, not just the final one.
180-220: Exception-handling test semantics are a bit permissiveGiven the name “Should handle exceptions during send gracefully”, the test currently accepts two behaviors: an exception escaping the call, or a callback invocation with
success == false. For a crisper contract, you might want to pick one (likely “no exception escapes, callback reports failure”) and assert that explicitly instead of the current either/or logic.
252-302: Robust filtering of null/blank incoming messagesThese tests correctly ensure that
null, blank (" "), and empty ("") messages do not result in list additions, which is good defensive behavior. Minor readability nit only:noAddis a slight double-negative; a name likeadded(asserting it’sfalse) would be a bit clearer, but the current logic is fine.
342-369: Ensure WireMock expectations matchNtfyConnectionImpl’s payload formatThe integration test currently expects a POST to
/mytopicwith a raw request body matching"Hello World". IfNtfyConnectionImpluses a different payload format (e.g., JSON DTO, form-encoded body, etc.), this assertion will fail even though the HTTP integration is functionally correct. It’s worth double-checking the implementation and aligning the matcher (equalTo, JSON matcher, or similar) and path with the actual request shape, documenting that expected format here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/resources/com/example/style.css(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/main/resources/com/example/style.css
[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] 5-5: 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] 6-6: 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] 7-7: 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] 13-13: 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] 14-14: 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] 16-16: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 21-21: 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: 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] 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] 24-24: 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] 24-24: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 29-29: 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] 30-30: 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] 32-32: 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] 33-33: 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] 34-34: 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)
[error] 36-36: 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] 40-40: 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] 41-41: 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] 42-42: 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] 43-43: 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] 43-43: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 47-47: 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] 48-48: 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] 49-49: 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] 50-50: 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] 51-51: 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] 52-52: 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] 53-53: 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] 54-54: 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] 55-55: 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] 58-58: Unexpected unknown pseudo-class focused
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 59-59: 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] 60-60: 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] 64-64: 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] 65-65: 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] 66-66: 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] 67-67: 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] 68-68: 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] 69-69: 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] 70-70: 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] 71-71: 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] 72-72: 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] 73-73: 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] 73-73: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 77-77: 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] 78-78: 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] 79-79: 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] 83-83: 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] 88-88: 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] 89-89: 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] 90-90: 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] 91-91: 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] 92-92: 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] 96-96: 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] 97-97: 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] 100-100: Unexpected unknown pseudo-class selected
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 101-101: 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] 106-106: 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] 107-107: 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] 108-108: 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] 109-109: 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] 110-110: 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] 111-111: 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] 112-112: 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] 112-112: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 116-116: 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] 117-117: 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] 118-118: 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] 119-119: 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] 120-120: 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] 121-121: 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] 125-125: 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] 126-126: 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] 127-127: 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] 128-128: 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] 129-129: 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] 130-130: 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] 135-135: 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] 136-136: 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] 140-140: 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] 141-141: 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] 142-142: 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] 143-143: 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] 144-144: 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] 145-145: 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] 146-146: 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] 147-147: 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] 148-148: 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] 151-151: Unexpected unknown pseudo-class focused
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 152-152: 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] 153-153: 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] 158-158: 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] 159-159: 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] 160-160: 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] 161-161: 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] 162-162: 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] 163-163: 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] 164-164: 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] 165-165: 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] 166-166: 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] 167-167: 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] 168-168: 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] 168-168: Unexpected unknown function: dropshadow
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 172-172: 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] 173-173: 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] 174-174: 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] 178-178: 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] 182-182: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 183-183: 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] 186-186: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 187-187: 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] 188-188: 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] 189-189: 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] 190-190: 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] 193-193: Unexpected unknown pseudo-class vertical
See MDN web docs for more details.
(lint/correctness/noUnknownPseudoClass)
[error] 194-194: 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] 195-195: 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 (5)
src/main/resources/com/example/style.css (1)
1-196: All CSS classes are actively used; original concern verified as unfounded.The verification confirms that all CSS classes defined in the stylesheet are properly integrated:
- Direct FXML bindings: 11 base classes applied via
styleClassin the FXML layout- Programmatic application:
chat-bubble,chat-bubble-sent, andchat-bubble-receivedare dynamically applied inHelloController.java(lines 96, 103, 106) usingbubble.getStyleClass().add()- Descendant and pseudo-class selectors: Scrollbar,
:hover,:focused,:disabled, and:selectedselectors are correctly positioned for JavaFX's automatic state and hierarchy applicationNo dangling CSS classes exist. The stylesheet is well-integrated and complete.
src/test/java/com/example/HelloModelTest.java (4)
20-23: Headless FX initialization hook looks reasonableA no-op
@BeforeAllkeeps the suite headless-friendly as long asHelloModeldoesn’t touch the JavaFX toolkit directly. If that ever changes, this is the right place to plug in a proper FX bootstrap, so having the hook in place is useful.
116-146: Clear contract for failed sendsUsing a custom
NtfyConnectionthat always callscallback(false)and asserting thatmessageToSendis preserved clearly documents the desired behavior on failure. This gives a nice, focused regression test for “don’t clear the input on failed send”.
224-250: Receive happy-path test is solidUsing a
ListChangeListenerplus a latch to assert that a concreteNtfyMessageDtoends up ingetMessages()is a clean, race-free way to test the receive path. This nicely verifies that incoming messages are propagated into the observable list.
304-338: Order-preservation on received messages well coveredThis test nicely asserts both the expected size and the exact ordering of messages in
getMessages(), which is important if the implementation ever introduces sorting, filtering, or background threading. Good coverage for the receive pipeline.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/main/java/com/example/NtfyConnectionImpl.java (4)
3-4: Fix the incorrect Jackson import.Line 4 imports
tools.jackson.databind.ObjectMapper, which is incorrect and will prevent compilation. The correct import iscom.fasterxml.jackson.databind.ObjectMapper.import io.github.cdimascio.dotenv.Dotenv; -import tools.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectMapper;
24-30: Improve error message for missing environment variable.Line 28 passes
"USER_ID"as the error message toObjects.requireNonNull. If the environment variable is missing, this will throwNullPointerException: USER_ID, which isn't very helpful for debugging.Dotenv dotenv = Dotenv.load(); this.hostName = Objects.requireNonNull(dotenv.get("HOST_NAME")); - this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID"); + this.userId = Objects.requireNonNull(dotenv.get("USER_ID"), "USER_ID environment variable is required"); this.currentTopic = dotenv.get("DEFAULT_TOPIC", "mytopic");
66-82: Multiple issues in the send method.Three issues need to be addressed:
- Line 70: The header
"Cache"should be"Cache-Control"with value"no-cache"- Line 78:
System.out.printlnis not production-ready; use a proper logging framework- Missing timeouts: HTTP requests should have timeouts to prevent indefinite hangs
+import java.time.Duration; + @Override public void send(String message, Consumer<Boolean> callback) { HttpRequest httpRequest = HttpRequest.newBuilder() .POST(HttpRequest.BodyPublishers.ofString(message)) - .header("Cache", "no") + .header("Cache-Control", "no-cache") .header("X-User-Id", userId) .uri(URI.create(hostName + "/" + currentTopic)) + .timeout(Duration.ofSeconds(10)) .build();Also consider:
- Replacing
System.out.printlnwith a logger (e.g., SLF4J)- Adding a connect timeout to the HttpClient:
private final HttpClient http = HttpClient.newBuilder() .connectTimeout(Duration.ofSeconds(10)) .build();
88-108: Remove debug code and add lifecycle management.Multiple issues need to be addressed:
- Line 106:
peek(System.out::println)appears to be debug code and should be removed- Line 101: Error logging via
System.out.printlnis not production-ready- Lifecycle concern: No mechanism to cancel or stop the receive operation. If
receive()is called multiple times (e.g., when changing topics), multiple concurrent streams will be created.map(s -> { try { return mapper.readValue(s, NtfyMessageDto.class); } catch (Exception e) { - System.out.println("Failed to parse message: " + e.getMessage()); + // TODO: Use proper logger instead return null; } }) .filter(Objects::nonNull) - .peek(System.out::println) .forEach(messageHandler));Consider adding a field to track the active receive operation:
private CompletableFuture<?> activeReceive; public void receive(Consumer<NtfyMessageDto> messageHandler) { if (activeReceive != null && !activeReceive.isDone()) { activeReceive.cancel(true); } activeReceive = http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines()) // ... rest of implementation }src/main/java/com/example/HelloModel.java (2)
26-30: Avoid starting async operations in the constructor.Line 29 calls
receiveMessage()in the constructor, which starts an asynchronous operation before the object is fully constructed. This can lead to race conditions where callbacks access the object while it's still being initialized.Consider extracting initialization:
public HelloModel(NtfyConnection connection) { this.connection = connection; this.currentTopic.set(connection.getCurrentTopic()); - receiveMessage(); } + + public void start() { + receiveMessage(); + }Then update callers to invoke
model.start()after construction in HelloController.
56-63: Potential duplicate message listeners when changing topics.Line 61 calls
receiveMessage()every time the topic changes, but there's no mechanism to stop the previous listener. This creates multiple concurrent receive operations, which can lead to:
- Duplicate messages from old topics
- Resource leaks
- Unexpected behavior
Consider implementing lifecycle management:
+ private CompletableFuture<?> activeReceive; + public void setCurrentTopic(String topic) { if (topic != null && !topic.isBlank()) { + if (activeReceive != null && !activeReceive.isDone()) { + activeReceive.cancel(true); + } connection.setCurrentTopic(topic); this.currentTopic.set(topic); messages.clear(); - receiveMessage(); + activeReceive = receiveMessage(); } }Note: This requires
receiveMessage()to return theCompletableFuture, which would need changes inNtfyConnectionImpl.
🧹 Nitpick comments (4)
src/test/java/com/example/NtfyConnectionSpy.java (2)
10-11: Consider making fields private for better encapsulation.The
messageandhandlerfields are package-private, which allows any test class in the same package to modify them directly. While acceptable for test utilities, making them private with getter methods would better encapsulate the spy's internal state and make the test double more robust.Apply this diff:
- String message; - Consumer<NtfyMessageDto> handler; + private String message; + private Consumer<NtfyMessageDto> handler; + + public String getMessage() { + return message; + }
15-18: Consider using an executor orCompletableFuturefor async execution.Creating a raw
Threadfor each send operation works but has some drawbacks:
- Threads are relatively expensive resources
- No control over thread lifecycle (not daemon, no name)
- Exception handling relies on the callback not throwing
Consider using
CompletableFuture.runAsync()for cleaner async execution:@Override public void send(String message, Consumer<Boolean> callback) { this.message = message; - new Thread(() -> callback.accept(true)).start(); + CompletableFuture.runAsync(() -> callback.accept(true)); }This requires adding
import java.util.concurrent.CompletableFuture;at the top of the file.src/test/java/com/example/HelloModelTest.java (1)
201-241: Simplify exception handling expectations.The test accounts for two scenarios: exception propagating or callback receiving false. However, looking at HelloModel.sendMessageAsync (lines 78-99), there's no try-catch around
connection.send(), so exceptions would propagate uncaught. The current test logic withexceptionCaughtflag and conditional assertions is more complex than needed.Consider clarifying the expected behavior:
- If exceptions should be caught and converted to
callback.accept(false), add try-catch in HelloModel- If exceptions should propagate, simplify the test to expect the exception
// Act - try { - model.sendMessageAsync(success -> { - result[0] = success; - latch.countDown(); - }); - } catch (Exception e) { - exceptionCaught[0] = true; - latch.countDown(); - } + // Exception should propagate since HelloModel doesn't catch it + assertThatThrownBy(() -> { + model.sendMessageAsync(success -> { + result[0] = success; + latch.countDown(); + }); + }).isInstanceOf(RuntimeException.class) + .hasMessage("Simulated crash"); - boolean completed = latch.await(500, TimeUnit.MILLISECONDS); - - // Assert - assertThat(completed).isTrue(); - - if (exceptionCaught[0]) { - assertThat(exceptionCaught[0]).isTrue(); - } else { - assertThat(result[0]).isFalse(); - }src/main/java/com/example/HelloModel.java (1)
78-99: Replace console logging with proper logging framework.Lines 81 and 95 use
System.out.printlnfor logging, which is not suitable for production code. Consider using a proper logging framework like SLF4J.+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class HelloModel { + private static final Logger logger = LoggerFactory.getLogger(HelloModel.class); + // ... public void sendMessageAsync(Consumer<Boolean> callback) { String msg = messageToSend.get(); if (msg == null || msg.isBlank()) { - System.out.println("Nothing to send!"); + logger.debug("Nothing to send!"); callback.accept(false); return; } connection.send(msg, success -> { if (success) { runOnFx(() -> { if (msg.equals(messageToSend.get())) { messageToSend.set(""); } }); callback.accept(true); } else { - System.out.println("Failed to send message!"); + logger.error("Failed to send message!"); callback.accept(false); } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/example/FxUtils.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/NtfyConnection.java(1 hunks)src/main/java/com/example/NtfyConnectionImpl.java(1 hunks)src/main/java/com/example/NtfyMessageDto.java(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)src/test/java/com/example/NtfyConnectionSpy.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/example/FxUtils.java
- src/main/java/com/example/NtfyMessageDto.java
- src/main/java/com/example/NtfyConnection.java
- src/main/java/com/example/HelloFX.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/HelloModel.java (1)
src/main/java/com/example/FxUtils.java (1)
FxUtils(5-26)
🔇 Additional comments (19)
src/test/java/com/example/NtfyConnectionSpy.java (1)
21-30: LGTM!The
receive()andsimulateIncoming()methods provide a clean test API. The null check insimulateIncoming()properly prevents NPEs if the handler hasn't been set yet.src/test/java/com/example/HelloModelTest.java (9)
29-47: LGTM!Clean test that properly verifies message sending with async coordination using
CountDownLatch.
75-98: LGTM!Properly tests the empty message validation with correct async handling.
103-126: LGTM!Correctly validates null message handling.
131-161: LGTM!Excellent test of the failure path, confirming the message is retained when send fails.
166-196: LGTM!Correctly tests multiple sequential sends. Note that the spy only stores the last message, which is expected behavior for this test scenario.
246-272: LGTM!Properly tests message reception with correct use of
ListChangeListenerfor async verification.
277-330: LGTM!Excellent negative tests verifying that null, blank, and empty messages are properly filtered and not added to the list.
335-402: LGTM!Both tests provide excellent coverage:
- Order preservation test validates message list behavior
- Integration test with WireMock validates actual HTTP interaction
20-23: Headless JavaFX initialization approach is correct.Verified that HelloModel properly wraps all FX-dependent operations with
FxUtils.runOnFx(). In lines 79 and 87, modifications to theStringPropertyandObservableListare wrapped correctly. FxUtils catchesIllegalStateExceptionin headless mode and executes tasks synchronously (line 22), allowingListChangeListenercallbacks to fire immediately and tests to properly synchronize viaCountDownLatch. The implementation handles the headless scenario correctly.src/main/java/com/example/HelloController.java (5)
52-61: LGTM!Proper initialization of greeting and topic label with correct use of
Platform.runLaterfor focus management.
62-82: LGTM!Proper JavaFX bindings with good defensive null check for the optional
changeTopicButton.
120-128: LGTM!Auto-scroll implementation correctly uses
Platform.runLaterto ensure scrolling happens on the FX thread.
130-146: LGTM with minor note.The message sending logic is correct. Note that line 134 clears
messageInput, but since it's bidirectionally bound tomodel.messageToSendProperty()(line 64), and the model already clears that property on successful send (HelloModel line 90), the explicitclear()call is redundant but harmless. The defensive approach is acceptable.
148-156: LGTM!Topic change logic is straightforward and correct, with proper validation.
src/main/java/com/example/NtfyConnectionImpl.java (1)
32-59: LGTM!The alternative constructors and accessors are well-designed for testing scenarios.
src/main/java/com/example/HelloModel.java (3)
32-54: LGTM!Property accessors follow standard JavaFX patterns correctly.
65-76: LGTM!Simple utility methods with correct validation logic.
101-107: LGTM!Message reception logic correctly validates messages and uses
runOnFxfor thread-safe UI updates.
| private final DateTimeFormatter timeFormatter = | ||
| DateTimeFormatter.ofPattern("HH:mm:ss") | ||
| .withZone(ZoneId.systemDefault()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused timeFormatter field.
The timeFormatter field is declared and initialized but never used in the controller. The cell factory (lines 85-117) doesn't display timestamps, so this appears to be leftover from a previous implementation.
- private final DateTimeFormatter timeFormatter =
- DateTimeFormatter.ofPattern("HH:mm:ss")
- .withZone(ZoneId.systemDefault());
-
@FXML
private void initialize() {Also remove the unused imports:
-import java.time.ZoneId;
-import java.time.format.DateTimeFormatter;📝 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 final DateTimeFormatter timeFormatter = | |
| DateTimeFormatter.ofPattern("HH:mm:ss") | |
| .withZone(ZoneId.systemDefault()); | |
| @FXML | |
| private void initialize() { |
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 47 to 49, the
private field timeFormatter is declared and never used; remove that field
declaration and its initializer, and then remove any now-unused imports (e.g.,
java.time.DateTimeFormatter, java.time.ZoneId or related java.time imports) from
the top of the file so there are no unused import warnings.
| messageView.setCellFactory(lv -> new ListCell<>() { | ||
| @Override | ||
| protected void updateItem(NtfyMessageDto msg, boolean empty) { | ||
| super.updateItem(msg, empty); | ||
|
|
||
| if (empty || msg == null || msg.message() == null || msg.message().isBlank()) { | ||
| setText(null); | ||
| setGraphic(null); | ||
| } else { | ||
| // Skapa bubble-label | ||
| Label bubble = new Label(msg.message()); | ||
| bubble.setWrapText(true); | ||
| bubble.setMaxWidth(250); | ||
| bubble.setPadding(new Insets(10)); | ||
| bubble.getStyleClass().add("chat-bubble"); // Basstyle | ||
|
|
||
| HBox container = new HBox(bubble); | ||
| container.setPadding(new Insets(5)); | ||
|
|
||
| // Använd CSS-klasser för skickat/mottaget | ||
| if (model.getUserId().equals(msg.id())) { | ||
| bubble.getStyleClass().add("chat-bubble-sent"); | ||
| container.setAlignment(Pos.CENTER_RIGHT); | ||
| } else { | ||
| bubble.getStyleClass().add("chat-bubble-received"); | ||
| container.setAlignment(Pos.CENTER_LEFT); | ||
| } | ||
|
|
||
| setText(null); | ||
| setGraphic(container); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify sender identification logic.
Line 105 compares model.getUserId() with msg.id() to determine if the message was sent by the current user. However, it's unclear whether msg.id() represents the message ID or the sender/user ID. Looking at test data in HelloModelTest (e.g., line 252: new NtfyMessageDto("Test", 1, "message", "myroom", "Test")), the first parameter appears to be a message identifier rather than a user identifier.
If id() is the message ID (not sender ID), this comparison will never match, and all messages will be styled as "received" instead of properly distinguishing sent vs. received messages.
Verify the NtfyMessageDto structure and confirm which field contains the sender/user information:
🏁 Script executed:
#!/bin/bash
# Check the NtfyMessageDto record definition
rg -A 5 "record NtfyMessageDto"Length of output: 190
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where NtfyMessageDto is instantiated to understand what the id field contains
rg "new NtfyMessageDto" -B 2 -A 2 -n
Length of output: 1712
🏁 Script executed:
#!/bin/bash
# Check the HelloModel class to see what getUserId() returns and how it's used
rg -A 10 "getUserId\(\)" src/main/java/com/example/HelloModel.javaLength of output: 277
🏁 Script executed:
#!/bin/bash
# Look for any service/repository code that creates NtfyMessageDto
rg "NtfyMessageDto" -B 3 -A 3 -n --type java | head -50Length of output: 3933
🏁 Script executed:
#!/bin/bash
# Find the real NtfyConnection implementation (not the spy)
rg "class.*NtfyConnection" --type java -A 5Length of output: 1021
🏁 Script executed:
#!/bin/bash
# Look for where getUserId is set/initialized in the connection
rg "getUserId" --type java -B 3 -A 3 -n | head -60Length of output: 2147
🏁 Script executed:
#!/bin/bash
# Search for any parsing or mapping of Ntfy messages/responses
rg -i "ntfy\|parse\|message" src/main/java/com/example/ --type java -n | grep -i "parse\|json\|response\|send" | head -30Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the full NtfyConnectionImpl implementation, especially the receive method
cat src/main/java/com/example/NtfyConnectionImpl.javaLength of output: 3888
🏁 Script executed:
#!/bin/bash
# Search for where NtfyMessageDto is constructed
rg "new NtfyMessageDto\|NtfyMessageDto(" --type java -B 5 -A 2 -nLength of output: 120
---
🌐 Web query:
```web
Ntfy protocol JSON message format sender ID user identification
💡 Result:
Short answer: ntfy's JSON message format does not include a built‑in "sender" or "user" field. Messages (when published as JSON or received via /json or SSE) include id, time, topic, message, title, tags, priority, actions, click/attachment, etc., but not a publisher/sender property — identification of who published is handled by authentication (Basic auth or access tokens) at the HTTP layer, not as a JSON field in the message. See ntfy publish/JSON and subscribe docs and the auth/config docs for details. [1][2][3][4]
References
- ntfy publish (fields, headers, actions examples). [1]
- Publish-as-JSON (full list of supported JSON publish fields). [2]
- Subscribe API — JSON message format returned by /json and SSE (id, time, topic, message, title, tags, priority, attachment...). [3]
- Configuration / authentication (users, tokens, ACLs — how publishers are identified/authenticated). [4]
Fix sender identification logic — msg.id() returns message ID, not sender ID.
The comparison at line 105 (model.getUserId().equals(msg.id())) will never match. According to the Ntfy protocol, the JSON message format does not include sender/publisher information. The id field contains the message ID generated by the Ntfy server, not the user ID of who sent the message. Therefore, all messages will always be styled as "received" regardless of sender.
The current implementation cannot distinguish sent from received messages without additional logic. Consider either:
- Storing locally-sent messages separately and tracking them before receiving server responses
- Appending a sender identifier to the message payload before publishing (if the Ntfy server permits custom fields)
- Implementing a different mechanism to correlate outgoing and incoming messages
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java around lines 85 to 117, the
cell factory currently compares model.getUserId() to msg.id(), but msg.id() is
the server-generated message ID (not the sender), so the sent/received styling
never matches; fix by correlating incoming messages to locally-sent messages or
an explicit sender field: modify the message send path to attach a
client-generated identifier (e.g., a UUID or local delivery token) or sender tag
to the payload and persist that token in a local Set/Map when sending, then
change the cell factory to check that token or lookup in the local sent-message
store (instead of msg.id()) to decide sent vs received; alternatively, if you
cannot change payload, track locally-sent messages by their content+timestamp
and match incoming messages against that store, removing entries once matched.
| @Test | ||
| @DisplayName("Should clear message field after successful send.") | ||
| void shouldClearMessageFieldAfterSuccessfulSend() throws InterruptedException { | ||
| // Arrange | ||
| var spy = new NtfyConnectionSpy(); | ||
| var model = new HelloModel(spy); | ||
| model.setMessageToSend("Test message"); | ||
|
|
||
| CountDownLatch latch = new CountDownLatch(1); | ||
|
|
||
| // Act | ||
| model.sendMessageAsync(success -> latch.countDown()); | ||
| boolean completed = latch.await(1, TimeUnit.SECONDS); | ||
|
|
||
| // Assert | ||
| assertThat(completed).isTrue(); | ||
| Thread.sleep(100); | ||
| assertThat(model.getMessageToSend()).isEmpty(); | ||
| } |
There was a problem hiding this comment.
Replace arbitrary sleep with proper synchronization.
Line 68 uses Thread.sleep(100) to wait for the message field to clear, which can cause test flakiness. The clearing happens on the FX thread via runOnFx(), so the sleep is attempting to ensure that completes.
Consider using a more robust approach, such as:
- Adding a listener to
messageToSendProperty()with a second latch - Polling with a timeout
- Or accepting that the callback guarantee is sufficient (remove the clear check)
CountDownLatch latch = new CountDownLatch(1);
// Act
model.sendMessageAsync(success -> latch.countDown());
boolean completed = latch.await(1, TimeUnit.SECONDS);
// Assert
assertThat(completed).isTrue();
- Thread.sleep(100);
- assertThat(model.getMessageToSend()).isEmpty();
+ // The callback confirms send succeeded; clearing happens async on FX thread
+ // Consider adding a listener if you need to verify clearing explicitly📝 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.
| @Test | |
| @DisplayName("Should clear message field after successful send.") | |
| void shouldClearMessageFieldAfterSuccessfulSend() throws InterruptedException { | |
| // Arrange | |
| var spy = new NtfyConnectionSpy(); | |
| var model = new HelloModel(spy); | |
| model.setMessageToSend("Test message"); | |
| CountDownLatch latch = new CountDownLatch(1); | |
| // Act | |
| model.sendMessageAsync(success -> latch.countDown()); | |
| boolean completed = latch.await(1, TimeUnit.SECONDS); | |
| // Assert | |
| assertThat(completed).isTrue(); | |
| Thread.sleep(100); | |
| assertThat(model.getMessageToSend()).isEmpty(); | |
| } | |
| @Test | |
| @DisplayName("Should clear message field after successful send.") | |
| void shouldClearMessageFieldAfterSuccessfulSend() throws InterruptedException { | |
| // Arrange | |
| var spy = new NtfyConnectionSpy(); | |
| var model = new HelloModel(spy); | |
| model.setMessageToSend("Test message"); | |
| CountDownLatch latch = new CountDownLatch(1); | |
| // Act | |
| model.sendMessageAsync(success -> latch.countDown()); | |
| boolean completed = latch.await(1, TimeUnit.SECONDS); | |
| // Assert | |
| assertThat(completed).isTrue(); | |
| // The callback confirms send succeeded; clearing happens async on FX thread | |
| // Consider adding a listener if you need to verify clearing explicitly | |
| } |
🤖 Prompt for AI Agents
In src/test/java/com/example/HelloModelTest.java around lines 52 to 70, replace
the Thread.sleep(100) flake-prone wait with proper synchronization: attach a
listener to messageToSendProperty() (or use a second CountDownLatch) before
invoking sendMessageAsync, have the listener/countdown signal when the property
becomes empty, then await that latch with a timeout instead of sleeping;
alternatively remove the property check if the existing success callback already
guarantees the clear and adjust the assertion accordingly.
2 participants
fortsatt felsökning
Summary by CodeRabbit
New Features
New Functionality
Tests
Chores