-
Notifications
You must be signed in to change notification settings - Fork 67
Model #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Model #14
Changes from 8 commits
ba9d641
87e581f
906b4b2
404b1f1
16b1396
5de28bf
61b6033
bf7e2bf
8e888be
8f79299
64ef5aa
d5c2cd2
91d4a26
1cfcb6b
b665727
234a4e0
120da00
29fefae
4c8aaf9
a731882
e07b438
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| target/ | ||
| /.idea/ | ||
| .env |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,22 +1,71 @@ | ||||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import javafx.application.Platform; | ||||||||||||||||||||||||
| import javafx.event.ActionEvent; | ||||||||||||||||||||||||
| import javafx.fxml.FXML; | ||||||||||||||||||||||||
| import javafx.scene.control.Label; | ||||||||||||||||||||||||
| import javafx.scene.control.*; | ||||||||||||||||||||||||
| import javafx.scene.control.ListCell; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import java.time.Instant; | ||||||||||||||||||||||||
| import java.time.ZoneId; | ||||||||||||||||||||||||
| import java.time.format.DateTimeFormatter; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Controller layer: mediates between the view (FXML) and the model. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| public class HelloController { | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private final HelloModel model = new HelloModel(); | ||||||||||||||||||||||||
| private final HelloModel model = new HelloModel(new NtfyConnectionImpl()); | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tight coupling makes testing difficult. The Apply this diff to enable dependency injection: + private HelloModel model;
+
+ public HelloController() {
+ this(new HelloModel(new NtfyConnectionImpl()));
+ }
+
+ // Package-private constructor for testing
+ HelloController(HelloModel model) {
+ this.model = model;
+ }
- private final HelloModel model = new HelloModel(new NtfyConnectionImpl());Alternatively, use a factory pattern or dependency injection framework. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @FXML | ||||||||||||||||||||||||
| private Label messageLabel; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @FXML | ||||||||||||||||||||||||
| private ListView<NtfyMessageDto> messageView; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @FXML | ||||||||||||||||||||||||
| private TextArea messageInput; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Formatter för tid | ||||||||||||||||||||||||
| private final DateTimeFormatter timeFormatter = | ||||||||||||||||||||||||
| DateTimeFormatter.ofPattern("HH:mm:ss") | ||||||||||||||||||||||||
| .withZone(ZoneId.systemDefault()); | ||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use consistent language in code comments. The code contains Swedish comments (e.g., "Formatter för tid", "Visa hälsning", "Koppla ListView till modellens meddelanden") while the rest of the codebase uses English. Use English consistently throughout for better maintainability and collaboration. Also applies to: 33-34, 36-37, 39-40, 42-43, 56-57 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @FXML | ||||||||||||||||||||||||
| private void initialize() { | ||||||||||||||||||||||||
| if (messageLabel != null) { | ||||||||||||||||||||||||
| messageLabel.setText(model.getGreeting()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| // Visa hälsning | ||||||||||||||||||||||||
| messageLabel.setText(model.getGreeting()); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Koppla ListView till modellens meddelanden | ||||||||||||||||||||||||
| messageView.setItems(model.getMessages()); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Bind TextArea till modellens property | ||||||||||||||||||||||||
| messageInput.textProperty().bindBidirectional(model.messageToSendProperty()); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Snyggare visning av meddelanden | ||||||||||||||||||||||||
| messageView.setCellFactory(lv -> new ListCell<>() { | ||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||
| protected void updateItem(NtfyMessageDto msg, boolean empty) { | ||||||||||||||||||||||||
| super.updateItem(msg, empty); | ||||||||||||||||||||||||
| if (empty || msg == null) { | ||||||||||||||||||||||||
| setText(null); | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| setText("[" + timeFormatter.format(Instant.ofEpochMilli(msg.time())) + "] " | ||||||||||||||||||||||||
| + msg.message()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Scrolla automatiskt ner till senaste meddelandet | ||||||||||||||||||||||||
| model.getMessages().addListener((javafx.collections.ListChangeListener<NtfyMessageDto>) change -> { | ||||||||||||||||||||||||
| Platform.runLater(() -> { | ||||||||||||||||||||||||
| if (!messageView.getItems().isEmpty()) { | ||||||||||||||||||||||||
| messageView.scrollTo(messageView.getItems().size() - 1); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @FXML | ||||||||||||||||||||||||
| private void sendMessage(ActionEvent actionEvent) { | ||||||||||||||||||||||||
| model.sendMessage(); | ||||||||||||||||||||||||
| messageInput.clear(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,74 @@ | ||
| package com.example; | ||
|
|
||
| /** | ||
| * Model layer: encapsulates application data and business logic. | ||
| */ | ||
| import javafx.application.Platform; | ||
| import javafx.beans.property.SimpleStringProperty; | ||
| import javafx.beans.property.StringProperty; | ||
| import javafx.collections.FXCollections; | ||
| import javafx.collections.ObservableList; | ||
| import java.util.function.Consumer; | ||
|
|
||
| public class HelloModel { | ||
| /** | ||
| * Returns a greeting based on the current Java and JavaFX versions. | ||
| */ | ||
|
|
||
| private final NtfyConnection connection; | ||
| private final ObservableList<NtfyMessageDto> messages = FXCollections.observableArrayList(); | ||
| private final StringProperty messageToSend = new SimpleStringProperty(); | ||
|
|
||
| public HelloModel(NtfyConnection connection) { | ||
| this.connection = connection; | ||
| receiveMessage(); | ||
| } | ||
|
|
||
| public ObservableList<NtfyMessageDto> getMessages() { | ||
| return messages; | ||
| } | ||
|
|
||
| public String getMessageToSend() { | ||
| return messageToSend.get(); | ||
| } | ||
|
|
||
| public StringProperty messageToSendProperty() { | ||
| return messageToSend; | ||
| } | ||
|
|
||
| public void setMessageToSend(String message) { | ||
| messageToSend.set(message); | ||
| } | ||
|
|
||
| public String getGreeting() { | ||
| String javaVersion = System.getProperty("java.version"); | ||
| String javafxVersion = System.getProperty("javafx.version"); | ||
| return "Hello, JavaFX " + javafxVersion + ", running on Java " + javaVersion + "."; | ||
| return "Welcome to ChatApp, made in JavaFX " + javafxVersion + ", running on Java " + javaVersion + "."; | ||
| } | ||
|
|
||
| public void sendMessage() { | ||
| String msg = messageToSend.get(); | ||
| if (msg == null || msg.isBlank()) { | ||
| return; | ||
| } | ||
| connection.send(msg); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| public void sendMessageAsync(Consumer<Boolean> callback) { | ||
| String msg = messageToSend.get(); | ||
| if (msg == null || msg.isBlank()) { | ||
| callback.accept(false); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| boolean success = connection.send(msg); | ||
| callback.accept(success); | ||
| } catch (Exception e) { | ||
| callback.accept(false); | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private void receiveMessage() { | ||
| connection.receive(message -> { | ||
| if (message == null || message.message() == null || message.message().isBlank()) { | ||
| return; | ||
| } | ||
| Platform.runLater(() -> messages.add(message)); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public class ManyParameters { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ManyParameters(String computerName, int timeout, | ||||||||||||||||||||||||||
| String method, int size, byte[] data) { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+5
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty constructor body - incomplete implementation. The constructor accepts five parameters but doesn't store or use them. Since the class declares no fields, constructed instances will have no state. This makes the builder pattern ineffective, as Add fields and assignments: public class ManyParameters {
+ private final String computerName;
+ private final int timeout;
+ private final String method;
+ private final int size;
+ private final byte[] data;
public ManyParameters(String computerName, int timeout,
String method, int size, byte[] data) {
-
+ this.computerName = computerName;
+ this.timeout = timeout;
+ this.method = method;
+ this.size = size;
+ this.data = data;
}
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| static void main() { | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-standard main() signature. This method cannot serve as a Java application entry point because it's package-private and lacks the Apply this diff to fix the signature for a proper entry point: - static void main() {
+ public static void main(String[] args) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| ManyParametersBuilder builder = new ManyParametersBuilder(); | ||||||||||||||||||||||||||
| builder | ||||||||||||||||||||||||||
| .setComputerName("localhost") //Fluent API | ||||||||||||||||||||||||||
| .setTimeout(10) | ||||||||||||||||||||||||||
| .setSize(0) | ||||||||||||||||||||||||||
| .createManyParameters(); | ||||||||||||||||||||||||||
|
Comment on lines
+12
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused object creation. The Apply this diff to capture the result: - ManyParametersBuilder builder = new ManyParametersBuilder();
- builder
+ ManyParameters params = new ManyParametersBuilder()
.setComputerName("localhost") //Fluent API
.setTimeout(10)
.setSize(0)
.createManyParameters();
+ // Use params...📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public class ManyParametersBuilder { | ||||||||||||||||||||||
| private String computerName; | ||||||||||||||||||||||
| private int timeout = 0; | ||||||||||||||||||||||
| private String method; | ||||||||||||||||||||||
| private int size = 0; | ||||||||||||||||||||||
| private byte[] data = null; | ||||||||||||||||||||||
|
Comment on lines
+4
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent null initialization. Only Apply this diff for consistency: private String computerName;
private int timeout = 0;
private String method;
private int size = 0;
- private byte[] data = null;
+ private byte[] data;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParametersBuilder setComputerName(String computerName) { | ||||||||||||||||||||||
| this.computerName = computerName; | ||||||||||||||||||||||
| return this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParametersBuilder setTimeout(int timeout) { | ||||||||||||||||||||||
| this.timeout = timeout; | ||||||||||||||||||||||
| return this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParametersBuilder setMethod(String method) { | ||||||||||||||||||||||
| this.method = method; | ||||||||||||||||||||||
| return this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParametersBuilder setSize(int size) { | ||||||||||||||||||||||
| this.size = size; | ||||||||||||||||||||||
| return this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParametersBuilder setData(byte[] data) { | ||||||||||||||||||||||
| this.data = data; | ||||||||||||||||||||||
| return this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public ManyParameters createManyParameters() { | ||||||||||||||||||||||
| return new ManyParameters(computerName, timeout, method, size, data); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.example; | ||
|
|
||
| import java.util.function.Consumer; | ||
|
|
||
| public interface NtfyConnection { | ||
|
|
||
| public boolean send(String message); | ||
|
|
||
| public void receive(Consumer<NtfyMessageDto> messageHandler); | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||
| package com.example; | ||||||
|
|
||||||
| import io.github.cdimascio.dotenv.Dotenv; | ||||||
| import tools.jackson.databind.ObjectMapper; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix ObjectMapper import package
-import tools.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectMapper;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| import java.io.IOException; | ||||||
| import java.net.URI; | ||||||
| import java.net.http.HttpClient; | ||||||
| import java.net.http.HttpRequest; | ||||||
| import java.net.http.HttpResponse; | ||||||
| import java.util.Objects; | ||||||
| import java.util.function.Consumer; | ||||||
|
|
||||||
| public class NtfyConnectionImpl implements NtfyConnection { | ||||||
|
|
||||||
| private final HttpClient http = HttpClient.newHttpClient(); | ||||||
| private final String hostName; | ||||||
| private final ObjectMapper mapper = new ObjectMapper(); | ||||||
|
|
||||||
| public NtfyConnectionImpl() { | ||||||
| Dotenv dotenv = Dotenv.load(); | ||||||
| hostName = Objects.requireNonNull(dotenv.get("HOST_NAME")); | ||||||
| } | ||||||
|
|
||||||
| public NtfyConnectionImpl(String hostName) { | ||||||
| this.hostName = hostName; | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public boolean send(String message) { | ||||||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||||||
| .POST(HttpRequest.BodyPublishers.ofString(message)) | ||||||
| .header("Cache", "no") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify HTTP header name. The header 🌐 Web query: 💡 Result: Use Cache-Control. The standard header for controlling/ disabling caching is Cache-Control (HTTP/1.1). To prevent caching use directives such as:
For HTTP/1.0 backwards-compatibility also include:
Sources: RFC 7234 (HTTP caching), MDN, OWASP. [1][2][3] References ::: 🏁 Script executed: #!/bin/bash
# Verify the file and examine the code context
if [ -f "src/main/java/com/example/NtfyConnectionImpl.java" ]; then
echo "=== File found. Context around line 33 ==="
sed -n '25,40p' "src/main/java/com/example/NtfyConnectionImpl.java"
else
echo "File not found at expected path. Searching for NtfyConnectionImpl.java..."
find . -name "NtfyConnectionImpl.java" -type f 2>/dev/null | head -5
fiLength of output: 697 Replace non-standard "Cache" header with standard "Cache-Control". The header 🤖 Prompt for AI Agents |
||||||
| .uri(URI.create(hostName + "/mytopic")) | ||||||
| .build(); | ||||||
| try { | ||||||
| //Todo: handle long blocking send requests to not freeze the JavaFX thread | ||||||
| //1. Use thread send message? | ||||||
| //2. Use async? | ||||||
| var reponse = http.send(httpRequest, HttpResponse.BodyHandlers.discarding()); | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||
| return true; | ||||||
| } catch (IOException e) { | ||||||
| System.out.println("Error sending message"); | ||||||
| } catch (InterruptedException e) { | ||||||
| System.out.println("Interruped sending message"); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo in error message. Line 45 has a typo: Apply this diff: - System.out.println("Interruped sending message");
+ System.out.println("Interrupted sending message");📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| } | ||||||
| return false; | ||||||
| } | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||
|
|
||||||
| @Override | ||||||
| public void receive(Consumer<NtfyMessageDto> messageHandler) { | ||||||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||||||
| .GET() | ||||||
| .uri(URI.create(hostName + "/mytopic/json")) | ||||||
| .build(); | ||||||
|
|
||||||
| http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines()) | ||||||
| .thenAccept(response -> response.body() | ||||||
| .map(s -> mapper.readValue(s, NtfyMessageDto.class)) | ||||||
| .forEach(messageHandler)); | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package com.example; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
|
|
||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record NtfyMessageDto(String id, long time, String event, String topic, String message) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package com.example; | ||
|
|
||
| public class Singelton { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix the typo in the class name. The class name is misspelled as "Singelton" instead of "Singleton". Apply this diff to correct the spelling: -public class Singelton {
+public class Singleton {
- private final static Singelton instance = new Singelton();
+ private final static Singleton instance = new Singleton();
- private Singelton(){
+ private Singleton(){
}
- public static Singelton getInstance(){
+ public static Singleton getInstance(){
return instance;
}
}
🤖 Prompt for AI Agents |
||
|
|
||
| private final static Singelton instance = new Singelton(); | ||
|
|
||
| private Singelton(){ | ||
|
|
||
| } | ||
|
|
||
| public static Singelton getInstance(){ | ||
| return instance; | ||
| } | ||
| } | ||
|
Comment on lines
+3
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify the purpose of this singleton class. This singleton class is not used in any of the provided files and its purpose in the messaging feature is unclear. If it's not needed, consider removing it to avoid dead code. Run the following script to check for usages of this class: 🏁 Script executed: #!/bin/bash
# Search for usages of Singelton class
rg -nP '\bSingelton\b' --type=java -C3Length of output: 827 Remove unused singleton class and fix spelling error. The verification confirms this class is dead code—no usages exist outside its own definition file. Additionally, the class name has a spelling error ("Singelton" instead of "Singleton"). Either remove the class or correct the spelling and add it to the messaging feature if it's needed. 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Consider using a stable WireMock version.
The project uses WireMock version 4.0.0-beta.15, which is a beta release and may have stability or compatibility issues.
🌐 Web query:
💡 Result:
The latest stable WireMock release is 3.13.1 (released June 13, 2025). [1][2]
Sources:
[1] WireMock blog — "WireMock 3.13.1 Released" (June 13, 2025).
[2] GitHub release — wiremock/wiremock tag 3.13.1.
Use WireMock 3.13.1, the latest stable release.
The project currently uses WireMock 4.0.0-beta.15, a beta release that may have stability or compatibility issues. The latest stable version is 3.13.1, which should be preferred for production dependencies.
🤖 Prompt for AI Agents