-
Notifications
You must be signed in to change notification settings - Fork 0
Add file upload support and improved message rendering #3
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
Changes from all commits
c092456
e449cda
5ae767a
d707e55
65ab3cf
547ac25
46aeeb7
04907ae
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,9 +1,21 @@ | ||
| package com.example; | ||
| import javafx.collections.FXCollections; | ||
| import javafx.collections.ObservableList; | ||
| import javafx.event.ActionEvent; | ||
| import javafx.fxml.FXML; | ||
| import javafx.geometry.Insets; | ||
| import javafx.scene.control.*; | ||
| import javafx.scene.control.Button; | ||
| import javafx.scene.control.Label; | ||
| import javafx.scene.control.TextField; | ||
| import javafx.scene.image.Image; | ||
| import javafx.scene.image.ImageView; | ||
| import javafx.scene.layout.HBox; | ||
| import javafx.stage.FileChooser; | ||
|
|
||
| import java.awt.*; | ||
| import java.io.File; | ||
| import java.net.URI; | ||
|
|
||
| /** | ||
| * Controller layer: mediates between the view (FXML) and the model. | ||
|
|
@@ -12,61 +24,164 @@ public class HelloController { | |
|
|
||
| private final HelloModel model = new HelloModel(new NtfyConnectionImpl()); | ||
| public ListView<NtfyMessageDto> messageView; | ||
| private boolean lastActionWasFile = false; | ||
|
|
||
| @FXML | ||
| private Label messageLabel; | ||
|
|
||
| @FXML | ||
| private TextField messageField; | ||
|
|
||
| @FXML | ||
| private void attachFile() { | ||
| FileChooser chooser = new FileChooser(); | ||
| chooser.getExtensionFilters().add( | ||
| new FileChooser.ExtensionFilter("Images", "*.png", "*.jpg", "*.jpeg", "*.gif", "*.txt") | ||
| ); | ||
|
|
||
| File file = chooser.showOpenDialog(messageField.getScene().getWindow()); | ||
| if (file != null) { | ||
| lastActionWasFile = true; | ||
| model.sendFile(file); | ||
| } | ||
| } | ||
|
|
||
| @FXML | ||
| private void initialize() { | ||
|
|
||
| if (messageLabel != null) { | ||
| messageLabel.setText(model.getGreeting()); | ||
| } | ||
|
|
||
| messageView.setItems(model.getMessages()); | ||
|
|
||
| // quickfix :D prevent listview cell selection which forces a re-render and | ||
| // causes image messages to flicker or disappear when clicking or scrolling. | ||
| messageView.setSelectionModel(new NoSelectionModel<>()); | ||
|
|
||
|
|
||
| messageView.setCellFactory(list -> new ListCell<>() { | ||
| private final Label messageLabel = new Label(); | ||
| private final HBox bubble = new HBox(messageLabel); | ||
|
|
||
| private final Label textLabel = new Label(); | ||
| private final HBox textBubble = new HBox(textLabel); | ||
|
|
||
| { | ||
| bubble.setPadding(new Insets(5, 10, 5, 10)); | ||
| bubble.setMaxWidth(200); | ||
| messageLabel.setWrapText(true); | ||
| bubble.getStyleClass().add("chat-bubble"); | ||
| textBubble.setPadding(new Insets(5, 10, 5, 10)); | ||
| textBubble.setMaxWidth(200); | ||
| textLabel.setWrapText(true); | ||
| textBubble.getStyleClass().add("chat-bubble"); | ||
| } | ||
|
|
||
| @Override | ||
| protected void updateItem(NtfyMessageDto item, boolean empty) { | ||
| super.updateItem(item, empty); | ||
|
|
||
| if (empty || item == null) { | ||
| setText(null); | ||
| setGraphic(null); | ||
| } else { | ||
| // Format tid + text | ||
| java.time.LocalTime time = java.time.Instant.ofEpochSecond(item.time()) | ||
| .atZone(java.time.ZoneId.systemDefault()) | ||
| .toLocalTime(); | ||
| String formattedTime = time.format(java.time.format.DateTimeFormatter.ofPattern("HH:mm")); | ||
|
|
||
| messageLabel.setText(formattedTime + "\n" + item.message()); | ||
| setGraphic(bubble); | ||
| return; | ||
| } | ||
|
|
||
| if (item.attachment() != null && | ||
| item.attachment().type() != null && | ||
| item.attachment().type().startsWith("image") && | ||
| item.attachment().url() != null) { | ||
|
|
||
| try { | ||
| Image image = new Image(item.attachment().url(), 200, 0, true, true); | ||
| ImageView imageView = new ImageView(image); | ||
| imageView.setPreserveRatio(true); | ||
|
|
||
| HBox imageBubble = new HBox(imageView); | ||
| imageBubble.setPadding(new Insets(5, 10, 5, 10)); | ||
| imageBubble.setMaxWidth(200); | ||
| imageBubble.getStyleClass().add("chat-bubble"); | ||
|
|
||
| setText(null); | ||
| setGraphic(imageBubble); | ||
| } catch (Exception e) { | ||
| Label err = new Label("[Image failed to load]"); | ||
| HBox bubble = new HBox(err); | ||
| bubble.setPadding(new Insets(5, 10, 5, 10)); | ||
| bubble.getStyleClass().add("chat-bubble"); | ||
| setGraphic(bubble); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (item.attachment() != null && item.attachment().url() != null) { | ||
|
|
||
| String fileName = item.attachment().name(); | ||
| String fileUrl = item.attachment().url(); | ||
|
|
||
| Label icon = new Label("📄"); | ||
| icon.setStyle("-fx-font-size: 20px;"); | ||
|
|
||
| Label fileLabel = new Label(fileName); | ||
| fileLabel.setStyle("-fx-text-fill: white; -fx-font-size: 14px;"); | ||
|
|
||
| Button openBtn = new Button("Open file"); | ||
| openBtn.setOnAction(e -> { | ||
| System.out.println("Opening: " + fileUrl); | ||
| new Thread(() -> { | ||
| try { | ||
| URI uri = new URI(fileUrl); | ||
| Desktop.getDesktop().browse(uri); | ||
| } catch (Exception ex) { | ||
| ex.printStackTrace(); | ||
| } | ||
| }).start(); | ||
|
Comment on lines
+124
to
+133
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. Validate URLs before opening in browser. The code opens attachment URLs directly in the browser without validation. This could potentially expose users to malicious links if the server is compromised or sends malicious URLs. Apply this diff to add basic URL validation: Button openBtn = new Button("Open file");
openBtn.setOnAction(e -> {
System.out.println("Opening: " + fileUrl);
+ if (!isValidUrl(fileUrl)) {
+ System.err.println("Invalid or unsafe URL: " + fileUrl);
+ return;
+ }
new Thread(() -> {
try {
URI uri = new URI(fileUrl);
Desktop.getDesktop().browse(uri);
} catch (Exception ex) {
ex.printStackTrace();
}
}).start();
});Add a validation helper method: private boolean isValidUrl(String url) {
if (url == null || url.isEmpty()) return false;
try {
URI uri = new URI(url);
String scheme = uri.getScheme();
return "http".equals(scheme) || "https".equals(scheme);
} catch (Exception e) {
return false;
}
}🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
|
|
||
| HBox fileBox = new HBox(10, icon, fileLabel, openBtn); | ||
| fileBox.setPadding(new Insets(5, 10, 5, 10)); | ||
| fileBox.setMaxWidth(200); | ||
| fileBox.getStyleClass().add("chat-bubble"); | ||
|
|
||
| setText(null); | ||
| setGraphic(fileBox); | ||
| return; | ||
| } | ||
|
|
||
|
|
||
| java.time.LocalTime time = java.time.Instant.ofEpochSecond(item.time()) | ||
| .atZone(java.time.ZoneId.systemDefault()) | ||
| .toLocalTime(); | ||
| String formattedTime = time.format(java.time.format.DateTimeFormatter.ofPattern("HH:mm")); | ||
|
|
||
| Label msgLabel = new Label(formattedTime + "\n" + item.message()); | ||
| msgLabel.setWrapText(true); | ||
| msgLabel.setMaxWidth(250); | ||
|
|
||
| HBox msgBubble = new HBox(msgLabel); | ||
| msgBubble.setPadding(new Insets(5, 10, 5, 10)); | ||
| msgBubble.setMaxWidth(300); | ||
| msgBubble.getStyleClass().add("chat-bubble"); | ||
|
|
||
| setText(null); | ||
| setGraphic(msgBubble); | ||
| } | ||
|
|
||
| }); | ||
|
|
||
| model.messageToSendProperty().bind(messageField.textProperty()); | ||
|
|
||
| } | ||
|
|
||
|
|
||
| public void sendMessage(ActionEvent actionEvent) { | ||
| String message = messageField.getText(); | ||
| if(message == null || message.isBlank()){ | ||
| if(!lastActionWasFile && (message == null || message.isBlank())){ | ||
| showTemporaryAlert("You must write something before sending!"); | ||
| return; | ||
| } | ||
|
|
||
| model.sendMessage(); | ||
| if (message != null && !message.isBlank()) { | ||
| model.sendMessage(); | ||
| } | ||
|
|
||
| messageField.clear(); | ||
| lastActionWasFile = false; | ||
| } | ||
|
|
||
| private void showTemporaryAlert(String alertMessage) { | ||
|
|
@@ -83,4 +198,38 @@ private void showTemporaryAlert(String alertMessage) { | |
| } catch (InterruptedException e) {} | ||
| }).start(); | ||
| } | ||
|
|
||
| private static class NoSelectionModel<T> extends MultipleSelectionModel<T> { | ||
|
|
||
| /* | ||
| Quickfix to prevent the ListView from selecting cells when the user clicks on them. | ||
| Otherwise it triggers full re render of the listcells when which causes message bubbles | ||
| or images to flicker or dissapears on click or scroll | ||
| */ | ||
|
|
||
| @Override | ||
| public ObservableList<Integer> getSelectedIndices() { | ||
| return FXCollections.emptyObservableList(); | ||
| } | ||
|
|
||
| @Override | ||
| public ObservableList<T> getSelectedItems() { | ||
| return FXCollections.emptyObservableList(); | ||
| } | ||
|
|
||
| @Override public void selectIndices(int index, int... indices) { } | ||
| @Override public void selectAll() { } | ||
| @Override public void clearAndSelect(int index) { } | ||
| @Override public void select(int index) { } | ||
| @Override public void select(T obj) { } | ||
| @Override public void clearSelection(int index) { } | ||
| @Override public void clearSelection() { } | ||
| @Override public boolean isSelected(int index) { return false; } | ||
| @Override public boolean isEmpty() { return true; } | ||
| @Override public void selectPrevious() { } | ||
| @Override public void selectNext() { } | ||
| @Override public void selectFirst() { } | ||
| @Override public void selectLast() { } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |||||||||||||
| import javafx.collections.FXCollections; | ||||||||||||||
| import javafx.collections.ObservableList; | ||||||||||||||
|
|
||||||||||||||
| import java.io.File; | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Model layer: encapsulates application data and business logic. | ||||||||||||||
|
|
@@ -52,4 +54,8 @@ public void sendMessage() { | |||||||||||||
| public void receiveMessage() { | ||||||||||||||
| connection.receive(m->Platform.runLater(()->messages.add(m))); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public void sendFile(File file) { | ||||||||||||||
| connection.sendFile(file); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+58
to
+60
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. Handle the boolean return value from The Consider either:
Apply this diff to propagate the return value: - public void sendFile(File file) {
- connection.sendFile(file);
+ public boolean sendFile(File file) {
+ return connection.sendFile(file);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.github.cdimascio.dotenv.Dotenv; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import tools.jackson.databind.ObjectMapper; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.File; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.URI; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.net.http.HttpClient; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,7 +30,7 @@ public NtfyConnectionImpl(String hostName){ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public boolean send(String message) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .POST(HttpRequest.BodyPublishers.ofString(message)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(hostName + "/mytopic")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(hostName + "/adam")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: handle long blocking send requests to not freeze the JavaFX thread | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -46,17 +48,53 @@ public boolean send(String message) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void receive(Consumer<NtfyMessageDto> messageHandler) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String startId = "8TuugOLkvDz1"; // just to make the app wont load 10000000 messages | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Remove or use the unused The Apply this diff to use the variable consistently: - String startId = "8TuugOLkvDz1"; // just to make the app wont load 10000000 messages
+ String startId = "3hPbr2dcIUiU"; // just to make the app wont load 10000000 messages
HttpRequest httpRequest = HttpRequest.newBuilder()
.GET()
- .uri(URI.create(hostName + "/adam/json?since=3hPbr2dcIUiU"))
+ .uri(URI.create(hostName + "/adam/json?since=" + startId))
.build();Or remove it if not needed: - String startId = "8TuugOLkvDz1"; // just to make the app wont load 10000000 messages
HttpRequest httpRequest = HttpRequest.newBuilder()
.GET()
.uri(URI.create(hostName + "/adam/json?since=3hPbr2dcIUiU"))
.build();
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpRequest httpRequest = HttpRequest.newBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .GET() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(hostName + "/mytopic/json?since=wBuD2KGEaAe0")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(hostName + "/adam/json?since=3hPbr2dcIUiU")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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(messageHandler)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .peek(s -> System.out.println(s)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(s -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return mapper.readValue(s, NtfyMessageDto.class); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| System.out.println("JSON parse fail: " + s); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(msg -> msg != null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(msg -> "message".equals(msg.event())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .forEach(messageHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public boolean sendFile(File file){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String mime = java.nio.file.Files.probeContentType(file.toPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if(mime == null) mime = "application/octet-stream"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| long size = file.length(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Remove unused The Apply this diff: if(mime == null) mime = "application/octet-stream";
- long size = file.length();
-
HttpRequest request = HttpRequest.newBuilder()🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HttpRequest request = HttpRequest.newBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .uri(URI.create(hostName + "/adam")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .header("Filename", file.getName()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .header("Content-Type", mime) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .PUT(HttpRequest.BodyPublishers.ofFile(file.toPath())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| http.sendAsync(request, HttpResponse.BodyHandlers.discarding()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e.printStackTrace(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
99
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. Critical: Line 92 uses
Apply this diff to use synchronous sending: - http.sendAsync(request, HttpResponse.BodyHandlers.discarding());
+ var response = http.send(request, HttpResponse.BodyHandlers.discarding());
+ return response.statusCode() >= 200 && response.statusCode() < 300;
- return true;
} catch (Exception e) {
e.printStackTrace();
return false;
}Or handle the CompletableFuture properly: - public boolean sendFile(File file){
+ public CompletableFuture<Boolean> sendFile(File file){
try {
// ... mime detection code ...
- http.sendAsync(request, HttpResponse.BodyHandlers.discarding());
- return true;
+ return http.sendAsync(request, HttpResponse.BodyHandlers.discarding())
+ .thenApply(response -> response.statusCode() >= 200 && response.statusCode() < 300);
} catch (Exception e) {
e.printStackTrace();
- return false;
+ return CompletableFuture.completedFuture(false);
}
}📝 Committable suggestion
Suggested change
🤖 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.
Misleading file filter label and missing error feedback.
Line 39 labels the filter as "Images" but includes
*.txtfiles. Additionally, there's no user feedback ifmodel.sendFile(file)fails (note that the return value issue was flagged in HelloModel.java review).Apply this diff to fix the label:
Or separate the filters:
📝 Committable suggestion
🤖 Prompt for AI Agents