Skip to content

Cat chat#11

Closed
codebyNorthsteep wants to merge 14 commits intomainfrom
catChat
Closed

Cat chat#11
codebyNorthsteep wants to merge 14 commits intomainfrom
catChat

Conversation

@codebyNorthsteep
Copy link

@codebyNorthsteep codebyNorthsteep commented Nov 13, 2025

Summary by CodeRabbit

  • New Features
    • Added chat messaging system enabling real-time message exchange with server
    • Added server connectivity controls (connect/disconnect buttons)
    • Integrated message display area with formatted timestamps
    • Redesigned interface with dedicated chat layout and styling
    • Rebranded application as "CatCode Messenger" with updated icon

BorderPane med 3 delar
Lagt till tillfälliga metoder i controller för vyn
…den ska sparas chatt-liknade

I controller - en observable som lyssnar på förändringar
Fokus på vem som ska hålla reda på vad
Övriga förändringar - nätverksanslutningar.
vidare uppdateringar för att anpassa efter nya listan, record och cellfactory.
Även lagt till att medelanden visas olika beroende på inkomande eller skickad
…er radarn" i klasser som berörs. Körs med fungerar ej helt som den ska.

Nytt test för att ta emot meddelanden från en fakead server.
@coderabbitai
Copy link

coderabbitai bot commented Nov 13, 2025

Walkthrough

This PR transforms a simple greeting application into a JavaFX-based chat messenger. It introduces a NtfyConnection abstraction for server communication, implements HTTP-based message transport, adds observable message handling in the model and controller, redesigns the UI with chat-oriented layout and styling, and includes integration tests using WireMock.

Changes

Cohort / File(s) Summary
Core Architecture & Abstraction
src/main/java/com/example/NtfyConnection.java, src/main/java/com/example/Subscription.java
Introduces contract interfaces for message subscription lifecycle and network communication abstraction.
Connection Implementation
src/main/java/com/example/NtfyConnectionImpl.java, src/main/java/com/example/NtfyMessageDto.java
Implements HTTP-based message sending/receiving via HttpClient; adds DTO record with JSON deserialization and formatted timestamp.
Model & Controller
src/main/java/com/example/HelloModel.java, src/main/java/com/example/HelloController.java
Replaces greeting API with observable message list; adds connection lifecycle management, UI binding, send/receive workflows, and button state toggling.
Application Setup
src/main/java/com/example/HelloFX.java, src/main/java/module-info.java
Updates window title/icon and scene dimensions; adds module requires for dotenv, http, jackson, and graphics.
UI & Styling
src/main/resources/com/example/hello-view.fxml, src/main/resources/css/style.css
Redesigns layout from StackPane to BorderPane with chat view, toolbar, message list, and input bar; defines comprehensive chat UI styling with bubbles, colors, and interactive states.
Testing
src/test/java/com/example/HelloModelTest.java, src/test/java/com/example/NtfyConnectionSpy.java
Adds WireMock-based integration tests for message sending/receiving and a spy implementation of NtfyConnection for test message simulation.
Configuration
pom.xml, .gitignore
Adds dependencies (jackson-databind, dotenv-java, wiremock, awaitility); configures Java 25 compiler with preview features; ignores .env file.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as HelloController
    participant Model as HelloModel
    participant Conn as NtfyConnectionImpl
    participant Server as NtfyServer

    Note over UI,Server: Send Message Flow
    User->>UI: Type message + click Send
    UI->>UI: updateSendButtonState()
    UI->>Model: sendMessage(text)
    Model->>Conn: send(message)
    Conn->>Server: POST /catChat
    Server-->>Conn: 200 OK
    Conn-->>Model: true

    Note over UI,Server: Receive Message Flow
    UI->>Model: receiveMessage()
    Model->>Conn: receive(consumer)
    Conn->>Server: GET /catChat/json (streaming)
    Server-->>Conn: JSON lines (events)
    Conn->>Conn: Parse event == "message"
    Conn->>Model: consumer.accept(NtfyMessageDto)
    Model->>Model: messages.add(dto)
    Model-->>UI: ListChangeListener fires
    UI->>UI: ListView updates
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • NtfyConnectionImpl async HTTP handling: Verify streaming HTTP GET logic, thread safety of consumer invocation on JavaFX thread, and Subscription lifecycle (e.g., cancellation semantics).
  • Observable binding coherence: Confirm HelloModel and HelloController coordinate message list updates, property binding, and subscription cleanup without leaks or race conditions.
  • Test infrastructure setup: Validate WireMock port resolution, Awaitility wait conditions, and spy message simulation correctness.
  • FXML to CSS alignment: Ensure layout constraints, styling classes, and chat UI visual hierarchy match across XML and CSS.
  • Module and dependency compatibility: Verify Java 25 preview features usage and new module requires are justified and not redundant.

Poem

🐰 A chat app hops to life, 🐱
No more greetings, just a message wave,
Observable lists and server subscription strife,
BorderPane layouts—what JavaFX gave!
With styles and tests, this rabbit does rave!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.31% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Cat chat' directly reflects the main feature introduced: a complete chat application with server connectivity, message handling, and UI components for cat-themed messaging.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (18)
src/main/java/com/example/HelloFX.java (1)

3-3: Remove unused import.

The Dotenv import is not used in this file.

Apply this diff:

-import io.github.cdimascio.dotenv.Dotenv;
src/main/java/com/example/Subscription.java (1)

6-13: Well-designed interface with minor refinements available.

The interface design is clean and appropriate for managing subscription lifecycle.

Optional improvements:

  1. The @Override annotation and redeclaration of close() is redundant since it's inherited from Closeable. You can remove it unless you need to add JavaDoc.

  2. Consider using English for comments to support international collaboration:

 public interface Subscription extends Closeable {
-    //Stoppar en subscription
-    @Override
-    void close() throws IOException;
-
-    //Meddelar om Subscription är öppen
+    // Returns true if the subscription is open
     boolean isOpen();
 }
src/main/java/com/example/NtfyMessageDto.java (2)

11-14: Remove commented-out code.

Commented-out code reduces readability. If you need this constructor later, it can be retrieved from version control.

Apply this diff:

 @JsonIgnoreProperties(ignoreUnknown = true)
 public record NtfyMessageDto(String id, Long time, String event, String topic, String message) {
-    //Convenience Konstruktor - som en genväg till record fält
-//    public NtfyMessageDto(String message) {
-//        this("thisID", System.currentTimeMillis()/1000, "message", "catChat", message);
-//        }
-

     public String formattedTime() {

17-23: Consider making timezone configurable.

The method hardcodes ZoneId.of("Europe/Stockholm"), which makes the timestamp display timezone-dependent and less portable.

Consider using the system default timezone or making it configurable:

     public String formattedTime() {
         if (time == null) return "";
         return DateTimeFormatter.ofPattern("HH:mm")
                 .format(Instant.ofEpochSecond(time)
-                        .atZone(ZoneId.of("Europe/Stockholm")));
+                        .atZone(ZoneId.systemDefault()));
     }

Alternatively, if the timezone must be Stockholm, consider documenting this requirement and the reasoning behind it.

src/test/java/com/example/NtfyConnectionSpy.java (1)

9-11: Make test spy fields explicitly private.

The fields message and consumer should be explicitly marked as private to follow best practices for encapsulation.

Apply this diff:

-    String message;
-    //funktionen som ska köras när ett meddelande kommer
-    Consumer<NtfyMessageDto> consumer;
+    private String message;
+    //funktionen som ska köras när ett meddelande kommer
+    private Consumer<NtfyMessageDto> consumer;
src/test/java/com/example/HelloModelTest.java (3)

25-35: Remove unnecessary setMessageToSend("") call.

Line 30 sets an empty message that is immediately overwritten by the sendMessage("Hello World") call on Line 32. This line appears to be a testing artifact and can be safely removed.

Apply this diff:

         var spy = new NtfyConnectionSpy();
         var model = new HelloModel(spy);
         //Act
-        model.setMessageToSend("");
-
         model.sendMessage("Hello World");

54-58: Remove commented-out code.

The commented Mockito code (Lines 54-58) should be removed to keep the test clean and maintainable.

Apply this diff:

                 .withBody(fakeMessage)));
 
-//        Consumer<NtfyMessageDto> fakeReceiver = Mockito.mock(Consumer.class);
-//        //con.receive(fakeReceiver);
-//
-//        ArgumentCaptor<NtfyMessageDto> captor = ArgumentCaptor.forClass(NtfyMessageDto.class);
-//        Subscription subscription = con.receive(fakeReceiver);
-
-//        //Act
-
         var model =new HelloModel(con);

79-89: Remove unnecessary setMessageToSend("") call.

Line 82 sets an empty message that is not used in the test. The sendMessage method on Line 85 directly sets the message content.

Apply this diff:

         var con = new NtfyConnectionImpl("http://localhost:" + wmRuntimeInfo.getHttpPort());
         var model = new HelloModel(con);
-        model.setMessageToSend("");
         stubFor(post("/catChat").willReturn(ok()));
src/main/java/com/example/HelloController.java (5)

17-18: Consider dependency injection for NtfyConnection.

The controller directly instantiates NtfyConnectionImpl, creating tight coupling that makes testing more difficult and reduces flexibility. Consider accepting the connection as a constructor parameter or using a factory pattern.

For example:

private final HelloModel model;

public HelloController() {
    this(new NtfyConnectionImpl());
}

// Package-private constructor for testing
HelloController(NtfyConnection connection) {
    this.model = new HelloModel(connection);
}

43-44: Address or remove TODO comments.

These TODO comments should either be implemented or removed before merging. If the functionality is planned for later, consider creating a tracked issue instead.


62-62: Remove commented-out code.

The commented call to model.receiveMessage() should be removed to keep the codebase clean.

Apply this diff:

         disconnectFromServer.setDisable(true);
 
-        //model.receiveMessage();
         //Styr hur varje meddelande ska visas i chatboxen

88-88: Hard-coded width calculation may cause layout issues.

The expression chatBox.getWidth()-20 uses a magic number and may not adapt well to different screen sizes or when the chat box is resized.

Consider binding the max width to the parent's width property:

hbox.maxWidthProperty().bind(chatBox.widthProperty().subtract(20));

90-97: String-based message type detection is fragile.

Using startsWith("User:") to determine message alignment is brittle and tightly couples the UI to message content format. If the prefix changes or is localized, this logic will break.

Consider adding a message direction/type property to NtfyMessageDto (e.g., an enum with INCOMING/OUTGOING values) to make the logic more robust:

if (item.direction() == MessageDirection.OUTGOING) {
    hbox.setAlignment(javafx.geometry.Pos.CENTER_RIGHT);
    label.getStyleClass().add("outgoing-message");
} else {
    hbox.setAlignment(javafx.geometry.Pos.CENTER_LEFT);
    label.getStyleClass().add("incoming-message");
}
src/main/java/com/example/HelloModel.java (3)

34-38: Remove commented-out subscription initialization.

Line 37 contains commented code that should be removed. The current design where the controller explicitly calls receiveMessage() appears to be the intended behavior.

Apply this diff:

     public HelloModel(NtfyConnection connection) {
 
         this.connection = connection;
-        //subscription = receiveMessage(); //subscription startar automatiskt när modellen skapas
     }

44-47: Clarify or remove unclear comment.

The comment //test on Line 44 doesn't provide meaningful information. Either clarify its purpose or remove it.


67-74: Simplify receiveMessage logic with early return.

The nested if statement can be simplified using an early return pattern for better readability.

Apply this diff:

     public Subscription receiveMessage() {
-if(subscription != null && subscription.isOpen()) {
-    return this.subscription;
-}
-    return subscription = connection.receive(messages::add);
-
-
+        if (subscription != null && subscription.isOpen()) {
+            return subscription;
+        }
+        subscription = connection.receive(messages::add);
+        return subscription;
     }
src/main/java/com/example/NtfyConnectionImpl.java (2)

24-27: Add descriptive message to requireNonNull.

Line 26 uses Objects.requireNonNull without a descriptive message. Adding a message improves debugging when the environment variable is missing.

Apply this diff:

     public NtfyConnectionImpl() {
         Dotenv dotenv = Dotenv.load();
-        hostName = Objects.requireNonNull(dotenv.get("HOST_NAME"));
+        hostName = Objects.requireNonNull(dotenv.get("HOST_NAME"), "HOST_NAME environment variable is required");
     }

46-50: Consider using a logging framework instead of console output.

Lines 46-50 print errors directly to System.out, which makes it difficult to control log levels and integrate with logging infrastructure in production.

Consider using a logging framework like SLF4J or java.util.logging for better error handling and observability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21c51e8 and 6d1a71d.

⛔ Files ignored due to path filters (2)
  • src/main/resources/cat_pattern.png is excluded by !**/*.png
  • src/main/resources/coolCat.png is excluded by !**/*.png
📒 Files selected for processing (14)
  • .gitignore (1 hunks)
  • pom.xml (3 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/main/java/com/example/Subscription.java (1 hunks)
  • src/main/java/module-info.java (1 hunks)
  • src/main/resources/com/example/hello-view.fxml (1 hunks)
  • src/main/resources/css/style.css (1 hunks)
  • src/test/java/com/example/HelloModelTest.java (1 hunks)
  • src/test/java/com/example/NtfyConnectionSpy.java (1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/main/resources/css/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] 10-10: Unknown property is not allowed.

See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.

(lint/correctness/noUnknownProperty)


[error] 11-11: Unknown property is not allowed.

See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.

(lint/correctness/noUnknownProperty)


[error] 15-15: Unknown property is not allowed.

See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.

(lint/correctness/noUnknownProperty)


[error] 16-16: Unknown property is not allowed.

See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.

(lint/correctness/noUnknownProperty)


[error] 17-17: Unknown property is not allowed.

See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.

(lint/correctness/noUnknownProperty)


[error] 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] 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] 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] 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] 37-37: 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] 46-46: 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: 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] 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] 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] 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] 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] 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] 81-81: Unexpected unknown function: dropshadow

Use a known function instead.
See MDN web docs for more details.

(lint/correctness/noUnknownFunction)


[error] 84-85: Unexpected unknown pseudo-class focused

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] 87-87: 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] 98-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] 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] 102-102: 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] 103-103: 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] 104-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] 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)

🔇 Additional comments (15)
.gitignore (1)

3-3: LGTM! Appropriate addition for environment configuration.

Adding .env to .gitignore is a good practice to prevent sensitive environment variables from being committed to the repository.

pom.xml (3)

26-30: LGTM! Appropriate dependency for environment configuration.

The dotenv-java dependency supports the .env file handling added to the project.


65-70: LGTM! Appropriate test dependency.

Awaitility is a good choice for testing asynchronous behavior in the chat application.


37-42: Verify WireMock version choice against project compatibility needs.

The latest stable WireMock version is 3.13.1 (released June 12, 2025), while the dependency uses 4.0.0-beta.15. Using a beta version of the next major release introduces potential instability, though the test scope limits the impact. Confirm this version was intentionally chosen for 4.0.0 compatibility testing or consider downgrading to the stable 3.13.1.

src/main/java/com/example/HelloFX.java (2)

18-19: LGTM! Appropriate UI updates for chat messenger.

The scene dimensions and title reflect the application's new chat-focused purpose.


20-20: No changes needed. The icon resource exists at src/main/resources/coolCat.png, so getResourceAsStream will successfully load it. The code as-is will not throw a NullPointerException.

Likely an incorrect or invalid review comment.

src/main/resources/css/style.css (1)

1-108: LGTM! Comprehensive chat UI styling.

The CSS provides well-structured styling for the chat messenger UI with appropriate visual hierarchy, color scheme, and interactive states.

Note: The static analysis warnings from Biome are false positives. Biome is a web CSS linter and doesn't recognize JavaFX-specific CSS properties (prefixed with -fx-), functions like dropshadow(), or pseudo-classes like :focused. All of these are valid in JavaFX CSS.

src/main/resources/com/example/hello-view.fxml (1)

6-31: LGTM! Well-structured chat UI layout.

The BorderPane-based layout appropriately organizes the chat interface with clear separation between server controls (top), message display (center), and message input (bottom). All necessary controls are properly wired with fx:id attributes for controller access.

src/main/java/com/example/NtfyMessageDto.java (1)

9-10: LGTM! Appropriate use of record for DTO.

The @JsonIgnoreProperties(ignoreUnknown = true) annotation is a good defensive practice for handling API responses with extra fields.

src/main/java/com/example/NtfyConnection.java (1)

7-17: LGTM! Clean interface design.

The interface provides a clear contract for message communication with appropriate method signatures for sending messages, sending files, and receiving messages via a consumer pattern.

src/test/java/com/example/NtfyConnectionSpy.java (1)

15-51: Test spy implementation is well-structured.

The spy correctly captures sent messages, stores consumers, and provides simulation capabilities for testing. The inner Subscription class properly manages lifecycle state.

src/test/java/com/example/HelloModelTest.java (1)

92-101: Test correctly validates observable list behavior.

The test appropriately uses the spy to simulate incoming messages and verifies they appear in the model's observable list.

src/main/java/com/example/HelloController.java (1)

110-147: Connection management and UI state handling look good.

The methods correctly manage the subscription lifecycle, update button states, and handle message sending with appropriate validation.

src/main/java/com/example/NtfyConnectionImpl.java (2)

76-86: Subscription implementation correctly manages lifecycle.

The anonymous Subscription class appropriately cancels the asynchronous HTTP connection and tracks its state.


54-57: File sending not yet implemented.

The sendFile method is currently stubbed and always returns false. This is acceptable if file sending is planned for a future iteration.

Consider adding a TODO comment or creating a tracked issue for this feature if it's planned for future implementation.

Comment on lines +21 to +25
<dependency>
<groupId>tools.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>3.0.1</version>
</dependency>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect groupId for Jackson dependency.

The groupId tools.jackson.core is incorrect. The correct groupId for Jackson is com.fasterxml.jackson.core.

Apply this diff to fix the dependency:

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

Note: Jackson 3.x is still in alpha/beta. Using the stable 2.x version (2.18.2 is the latest stable) is recommended unless you specifically need 3.x features.

📝 Committable suggestion

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

Suggested change
<dependency>
<groupId>tools.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>3.0.1</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.18.2</version>
</dependency>
🤖 Prompt for AI Agents
In pom.xml around lines 21 to 25, the Jackson dependency uses the wrong groupId
"tools.jackson.core" and an unstable 3.0.1 version; change the groupId to
"com.fasterxml.jackson.core" and update the version to a stable 2.x release (for
example 2.18.2) unless you intentionally need Jackson 3.x, ensuring the
dependency block references the correct groupId and recommended stable version.

Comment on lines +89 to +97
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>25</source>
<target>25</target>
<compilerArgs>--enable-preview</compilerArgs>
</configuration>
</plugin>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the need for preview features.

The compiler is configured to use Java 25 with --enable-preview. Preview features are experimental and may change in future releases. Ensure this is intentional and that the codebase actually uses preview features.


🏁 Script executed:

#!/bin/bash
# Search for common Java preview features in the codebase
echo "Checking for record patterns, pattern matching, or other preview features..."
rg -n "instanceof.*\(|record.*\{|sealed|permits" --type java

Length of output: 222


🏁 Script executed:

#!/bin/bash
# More comprehensive search for Java preview features
echo "=== Searching for switch pattern matching ==="
rg -n "switch.*{.*case.*->|case.*when" --type java -A 3

echo "=== Searching for sealed classes ==="
rg -n "sealed\s+class|sealed\s+interface" --type java

echo "=== Searching for text blocks ==="
rg -n '"""' --type java

echo "=== Searching for virtual threads or structured concurrency ==="
rg -n "Thread\.ofVirtual|ExecutorService|StructuredTaskScope" --type java

echo "=== Listing all .java files to get scope ==="
fd "\.java$" --type f | wc -l

Length of output: 502


🌐 Web query:

Java 25 preview features JEP

💡 Result:

Preview JEPs in JDK 25

  • JEP 470 — PEM Encodings of Cryptographic Objects (Preview). [1][2]
  • JEP 502 — Stable Values (Preview). [1][3]
  • JEP 505 — Structured Concurrency (Fifth Preview). [1][3]
  • JEP 507 — Primitive Types in patterns, instanceof, and switch (Third Preview). [1][3]

Other notable non-preview experimental/incubator JEPs in JDK 25:

  • JEP 508 — Vector API (Incubator). [1][3]
  • JEP 509 — JFR CPU-Time Profiling (Experimental). [1][3]

Sources: JDK 25 release notes and OpenJDK/Oracle announcements. [1][2][3]


Remove the --enable-preview flag from maven-compiler-plugin configuration.

Verification confirms the codebase does not use any Java 25 preview features. The only preview-related element found is a record class (NtfyMessageDto.java), but records are a standard feature since Java 16 and do not require preview enablement. Text blocks in tests are also standard since Java 15. No actual preview features (JEP 470, 502, 505, 507) are used. Remove <compilerArgs>--enable-preview</compilerArgs> from lines 89-97 in pom.xml.

🤖 Prompt for AI Agents
In pom.xml around lines 89 to 97 the maven-compiler-plugin is configured with a
compilerArgs element that enables Java preview features; remove the
<compilerArgs>--enable-preview</compilerArgs> entry from the plugin
configuration so the compiler no longer runs with preview enabled, leaving
source/target as-is.

Comment on lines +76 to 83
public void stopSubscription() {
if (subscription != null && subscription.isOpen())
try{
subscription.close();
} catch(IOException e) {
System.out.println("Error closing subscription" + e.getMessage());
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix error message formatting.

Line 81 has a missing space in the error message concatenation, resulting in "Error closing subscription" instead of "Error closing subscription: ".

Apply this diff:

         if (subscription != null && subscription.isOpen())
             try{
             subscription.close();
         } catch(IOException e) {
-                System.out.println("Error closing subscription" + e.getMessage());
+                System.out.println("Error closing subscription: " + e.getMessage());
             }
     }
📝 Committable suggestion

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

Suggested change
public void stopSubscription() {
if (subscription != null && subscription.isOpen())
try{
subscription.close();
} catch(IOException e) {
System.out.println("Error closing subscription" + e.getMessage());
}
}
public void stopSubscription() {
if (subscription != null && subscription.isOpen())
try{
subscription.close();
} catch(IOException e) {
System.out.println("Error closing subscription: " + e.getMessage());
}
}
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloModel.java around lines 76 to 83, the error
message concatenation in the catch prints "Error closing subscription" and the
exception message without a separator; update the catch to print a colon and
space before the exception text (i.e., include ": " between the fixed message
and e.getMessage()) so the output reads "Error closing subscription: <message>";
ensure the rest of the try/catch stays intact.


import io.github.cdimascio.dotenv.Dotenv;
import javafx.application.Platform;
import tools.jackson.databind.ObjectMapper;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect Jackson import will cause compilation failure.

Line 5 imports tools.jackson.databind.ObjectMapper, but the standard Jackson library uses com.fasterxml.jackson.databind.ObjectMapper. This incorrect import will prevent the code from compiling unless you're using a non-standard Jackson fork.

Apply this diff:

-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.

Suggested change
import tools.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectMapper;
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around line 5, the import
uses tools.jackson.databind.ObjectMapper which is incorrect; replace it with the
standard Jackson import com.fasterxml.jackson.databind.ObjectMapper to fix the
compilation error and ensure the correct ObjectMapper class is referenced.

String inputMessage = Objects.requireNonNull(message);
HttpRequest httpRequest = HttpRequest.newBuilder()
.POST(HttpRequest.BodyPublishers.ofString(inputMessage))
.header("Cache", "no")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incorrect HTTP header name.

Line 40 uses "Cache" as the header name, but the correct HTTP header for cache control is "Cache-Control". The current header will be ignored by HTTP clients and servers.

Apply this diff:

         HttpRequest httpRequest = HttpRequest.newBuilder()
                 .POST(HttpRequest.BodyPublishers.ofString(inputMessage))
-                .header("Cache", "no")
+                .header("Cache-Control", "no-cache")
                 .uri(URI.create(hostName + "/catChat"))
                 .build();

Note: Also changed the value to "no-cache" which is a valid Cache-Control directive.

📝 Committable suggestion

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

Suggested change
.header("Cache", "no")
HttpRequest httpRequest = HttpRequest.newBuilder()
.POST(HttpRequest.BodyPublishers.ofString(inputMessage))
.header("Cache-Control", "no-cache")
.uri(URI.create(hostName + "/catChat"))
.build();
🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around line 40, the HTTP
header is incorrectly named "Cache" with value "no"; change it to the standard
"Cache-Control" header and set its value to the valid directive "no-cache" so
clients/servers recognize and honor the cache policy.

Comment on lines +70 to +75
var connected = http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines())
.thenAccept(response -> response.body()
.map(s -> mapper.readValue(s, NtfyMessageDto.class))
.filter(message -> message.event().equals("message"))
.peek(System.out::println)
.forEach(message -> Platform.runLater(()-> messageHandler.accept(message))));
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Unchecked exception handling required in stream.

Line 72 calls mapper.readValue() which throws a checked IOException, but this exception is not handled within the stream pipeline. This will cause a compilation error. Additionally, there's no error handling if the HTTP request fails.

Wrap the mapping operation in a try-catch block or extract it to a helper method:

         var connected = http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines())
                 .thenAccept(response -> response.body()
-                        .map(s -> mapper.readValue(s, NtfyMessageDto.class))
+                        .map(s -> {
+                            try {
+                                return mapper.readValue(s, NtfyMessageDto.class);
+                            } catch (Exception e) {
+                                System.err.println("Error parsing message: " + e.getMessage());
+                                return null;
+                            }
+                        })
+                        .filter(Objects::nonNull)
                         .filter(message -> message.event().equals("message"))
                         .peek(System.out::println)
-                        .forEach(message -> Platform.runLater(()-> messageHandler.accept(message))));
+                        .forEach(message -> Platform.runLater(() -> messageHandler.accept(message))));

Also consider adding error handling for the HTTP request itself using .exceptionally() on the CompletableFuture.

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

🤖 Prompt for AI Agents
In src/main/java/com/example/NtfyConnectionImpl.java around lines 70 to 75, the
stream calls mapper.readValue(...) which throws a checked IOException and the
CompletableFuture lacks HTTP error handling; extract the mapping into a helper
method that catches IOException (returning Optional.empty() or null for invalid
lines) or wraps it in an unchecked exception, then use that helper inside the
stream to avoid compilation errors; additionally attach an .exceptionally(...)
or .whenComplete(...) to the sendAsync CompletableFuture to log/handle
HTTP-level failures so the app can recover or report errors instead of silently
failing.

Comment on lines +4 to +7
requires io.github.cdimascio.dotenv.java;
requires java.net.http;
requires tools.jackson.databind;
requires javafx.graphics;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect Jackson module name.

The module name tools.jackson.databind is incorrect. The correct module name is com.fasterxml.jackson.databind.

Apply this diff:

     requires io.github.cdimascio.dotenv.java;
     requires java.net.http;
-    requires tools.jackson.databind;
+    requires com.fasterxml.jackson.databind;
     requires javafx.graphics;

This issue is related to the incorrect groupId in pom.xml. Both need to be corrected together.

📝 Committable suggestion

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

Suggested change
requires io.github.cdimascio.dotenv.java;
requires java.net.http;
requires tools.jackson.databind;
requires javafx.graphics;
requires io.github.cdimascio.dotenv.java;
requires java.net.http;
requires com.fasterxml.jackson.databind;
requires javafx.graphics;
🤖 Prompt for AI Agents
In src/main/java/module-info.java around lines 4 to 7, the declared module
`tools.jackson.databind` is incorrect; replace it with
`com.fasterxml.jackson.databind`. Also update the project's pom.xml to use the
correct Jackson groupId/artifact coordinates (com.fasterxml.jackson) so the
module name matches the dependency; ensure module-info requires
`com.fasterxml.jackson.databind` and the pom dependency entries use the matching
com.fasterxml.jackson groupId and appropriate artifactId/version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant