diff --git a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogHttpClient.java b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogHttpClient.java index c29ebf886f..48afecb0a8 100644 --- a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogHttpClient.java +++ b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogHttpClient.java @@ -183,7 +183,7 @@ private LogEntry save(LogEntry log, LogEntry inReplyTo) throws LogbookException httpRequestMultipartBody.addTextPart("logEntry", OlogObjectMappers.logEntrySerializer.writeValueAsString(log), "application/json"); for (Attachment attachment : log.getAttachments()) { - httpRequestMultipartBody.addFilePart(attachment.getFile()); + httpRequestMultipartBody.addFilePart(attachment.getFile(), attachment.getUniqueFilename()); } HttpRequest request = HttpRequest.newBuilder() diff --git a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogAttachment.java b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogAttachment.java index 43d991fe7a..1b708f6904 100644 --- a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogAttachment.java +++ b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogAttachment.java @@ -29,8 +29,6 @@ public class OlogAttachment implements Attachment { protected Boolean thumbnail; - protected Long fileSize; - private File file; /** @@ -51,17 +49,15 @@ public OlogAttachment(String id) { this.id = id; } - /** - * @return the fileName - */ - public String getFileName() { + @Override + public String getUniqueFilename() { return fileName; } /** * @param fileName the fileName to set */ - public void setFileName(String fileName) { + public void setUniqueFilename(String fileName) { this.fileName = fileName; } diff --git a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogObjectMappers.java b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogObjectMappers.java index 618ae71dae..f1dc068ccc 100644 --- a/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogObjectMappers.java +++ b/app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/model/OlogObjectMappers.java @@ -1,34 +1,34 @@ /** - * + * */ package org.phoebus.olog.es.api.model; -import java.io.File; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.SerializerProvider; -import org.phoebus.logbook.Attachment; -import org.phoebus.logbook.Logbook; -import org.phoebus.logbook.Property; -import org.phoebus.logbook.Tag; - import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.Version; import com.fasterxml.jackson.core.JsonParser.Feature; +import com.fasterxml.jackson.core.Version; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.module.SimpleAbstractTypeResolver; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import org.phoebus.logbook.Attachment; +import org.phoebus.logbook.Logbook; +import org.phoebus.logbook.Property; +import org.phoebus.logbook.Tag; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; /** * A Utility class which provides {@link ObjectMapper}s for Olog-es entities. @@ -38,7 +38,7 @@ public class OlogObjectMappers { public static ObjectMapper logEntryDeserializer = new ObjectMapper().registerModule(new JavaTimeModule()); public static ObjectMapper logEntrySerializer = new ObjectMapper().registerModule(new JavaTimeModule()); - + static SimpleModule module = new SimpleModule("CustomModel", Version.unknownVersion()); static SimpleAbstractTypeResolver resolver = new SimpleAbstractTypeResolver(); @@ -50,17 +50,15 @@ static class PropertyDeserializer extends JsonDeserializer { @Override public OlogProperty deserialize(JsonParser jp, DeserializationContext ctxt) - throws IOException, JsonProcessingException { + throws IOException { JsonNode node = jp.getCodec().readTree(jp); // TODO throw error if either the property or attribute names are null String name = node.get("name").asText(); - String owner = node.get("owner").isNull()? "" : node.get("owner").asText(); - String state = node.get("state").isNull()? "" : node.get("state").asText(); - Map attributes = new HashMap(); + Map attributes = new HashMap<>(); node.get("attributes").iterator().forEachRemaining(n -> { attributes.put( n.get("name").asText(), - n.get("value").isNull()? "" : n.get("value").asText() + n.get("value").isNull() ? "" : n.get("value").asText() ); }); return new OlogProperty(name, attributes); @@ -87,7 +85,7 @@ public void serialize(Property value, JsonGenerator gen, SerializerProvider seri gen.writeStringField("value", entry.getValue() == null ? "" : entry.getValue()); gen.writeEndObject(); } catch (IOException e) { - e.printStackTrace(); + Logger.getLogger(OlogObjectMappers.class.getName()).log(Level.WARNING, "Failed to serialize property", e); } } ); @@ -105,13 +103,13 @@ static class AttachmentDeserializer extends JsonDeserializer { @Override public OlogAttachment deserialize(JsonParser jp, DeserializationContext ctxt) - throws IOException, JsonProcessingException { + throws IOException { JsonNode node = jp.getCodec().readTree(jp); String id = node.get("id").asText(); String filename = node.get("filename").asText(); String fileMetadataDescription = node.get("fileMetadataDescription").asText(); OlogAttachment a = new OlogAttachment(); - a.setFileName(filename); + a.setUniqueFilename(filename); a.setId(id); a.setContentType(fileMetadataDescription); return a; @@ -145,26 +143,26 @@ public OlogAttachment deserialize(JsonParser jp, DeserializationContext ctxt) logEntrySerializer.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); logEntrySerializer.configure(Feature.AUTO_CLOSE_SOURCE, true); } - + public interface AttachmentMixIn { @JsonProperty("filename") - public String getName(); + String getName(); @JsonProperty("file") - public File getFile(); + File getFile(); @JsonProperty("fileMetadataDescription") - public String getContentType(); + String getContentType(); @JsonProperty("filename") - public void setName(String name); + void setName(String name); @JsonProperty("file") - public void setFile(File file); + void setFile(File file); @JsonProperty("fileMetadataDescription") - public void setContentType(String contentType); + void setContentType(String contentType); } } diff --git a/app/logbook/olog/client/src/main/java/org/phoebus/olog/api/OlogClient.java b/app/logbook/olog/client/src/main/java/org/phoebus/olog/api/OlogClient.java index 4a135ebbf4..5fb775a397 100644 --- a/app/logbook/olog/client/src/main/java/org/phoebus/olog/api/OlogClient.java +++ b/app/logbook/olog/client/src/main/java/org/phoebus/olog/api/OlogClient.java @@ -14,7 +14,6 @@ import org.phoebus.logbook.SearchResult; import org.phoebus.logbook.Tag; import org.phoebus.security.store.SecureStore; -import org.phoebus.security.tokens.AuthenticationScope; import org.phoebus.security.tokens.ScopedAuthenticationToken; import org.phoebus.util.http.HttpRequestMultipartBody; import org.phoebus.util.http.QueryParamsHelper; @@ -374,7 +373,7 @@ public InputStream getAttachment(Long logId, Attachment attachment) { .build(); HttpResponse httpResponse = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofInputStream()); - if (httpResponse.statusCode() < 300){ + if (httpResponse.statusCode() < 300) { LOGGER.log(Level.WARNING, "Failed to get attachment"); throw new OlogException("Failed to get attachment"); } @@ -394,7 +393,7 @@ public InputStream getAttachment(Long logId, String attachmentName) { .build(); HttpResponse httpResponse = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofInputStream()); - if (httpResponse.statusCode() < 300){ + if (httpResponse.statusCode() < 300) { LOGGER.log(Level.WARNING, "Failed to get attachment"); throw new OlogException("Failed to get attachment"); } @@ -417,7 +416,7 @@ public Property call() throws Exception { .GET() .build(); HttpResponse httpResponse = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofString()); - if(httpResponse.statusCode() < 300){ + if (httpResponse.statusCode() < 300) { LOGGER.log(Level.WARNING, "Failed to get property " + property); throw new OlogException(httpResponse.statusCode(), httpResponse.body()); } @@ -480,7 +479,7 @@ public Collection call() { log.getAttachments().forEach(attachment -> { HttpRequestMultipartBody httpRequestMultipartBody = new HttpRequestMultipartBody(); - httpRequestMultipartBody.addFilePart(attachment.getFile()); + httpRequestMultipartBody.addFilePart(attachment.getFile(), attachment.getUniqueFilename()); HttpRequest attachmentRequest = HttpRequest.newBuilder() .uri(URI.create(Preferences.olog_url + "/attachments/" + createdLog.getId())) @@ -542,7 +541,7 @@ public LogEntry call() throws Exception { if (httpResponse.statusCode() < 300) return new OlogLog(OBJECT_MAPPER.readValue(httpResponse.body(), XmlLog.class)); else - throw new OlogException(httpResponse.statusCode(), httpResponse.body()); + throw new OlogException(httpResponse.statusCode(), httpResponse.body()); } } diff --git a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/SingleLogEntryDisplayController.java b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/SingleLogEntryDisplayController.java index c43d2d9a9d..68a9bb9869 100644 --- a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/SingleLogEntryDisplayController.java +++ b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/SingleLogEntryDisplayController.java @@ -249,7 +249,7 @@ private void fetchAndSetAttachments() { OlogAttachment fileAttachment = new OlogAttachment(); fileAttachment.setContentType(attachment.getContentType()); fileAttachment.setThumbnail(false); - fileAttachment.setFileName(attachment.getName()); + fileAttachment.setUniqueFilename(attachment.getName()); // Determine file extension, needed to support transition to Image Viewer app for image attachments String fileExtension = ""; int indexOfLastDot = attachment.getName().lastIndexOf('.'); diff --git a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/AttachmentsEditorController.java b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/AttachmentsEditorController.java index 7ac8f06303..d1cac86840 100644 --- a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/AttachmentsEditorController.java +++ b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/AttachmentsEditorController.java @@ -23,9 +23,8 @@ import javafx.beans.binding.Bindings; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleStringProperty; -import javafx.collections.FXCollections; -import javafx.collections.ObservableList; import javafx.collections.ListChangeListener; +import javafx.collections.ObservableList; import javafx.embed.swing.SwingFXUtils; import javafx.fxml.FXML; import javafx.scene.control.Alert; @@ -137,7 +136,7 @@ public void initialize() { Collection attachments = logEntry.getAttachments(); if (attachments != null && !attachments.isEmpty()) { attachments.forEach(a -> { - if(a.getFile() != null){ + if (a.getFile() != null) { attachedFilesSize += getFileSize(a.getFile()); } }); @@ -294,22 +293,23 @@ public void setTextArea(TextArea textArea) { } /** - * Add - * @param files + * Adds attachment files + * + * @param files A list of {@link File}s */ private void addFiles(List files) { Platform.runLater(() -> sizesErrorMessage.set(null)); if (!checkFileSizes(files)) { return; } - if(!checkForHeicFiles(files)){ + if (!checkForHeicFiles(files)) { return; } MimetypesFileTypeMap fileTypeMap = new MimetypesFileTypeMap(); for (File file : files) { OlogAttachment ologAttachment = new OlogAttachment(); ologAttachment.setFile(file); - ologAttachment.setFileName(file.getName()); + ologAttachment.setUniqueFilename(ologAttachment.getId() + "_" + file.getName()); String mimeType = fileTypeMap.getContentType(file.getName()); if (mimeType.startsWith("image")) { ologAttachment.setContentType("image"); @@ -342,7 +342,7 @@ private void addImage(Image image, String id) { OlogAttachment ologAttachment = new OlogAttachment(id); ologAttachment.setContentType("image"); ologAttachment.setFile(imageFile); - ologAttachment.setFileName(imageFile.getName()); + ologAttachment.setUniqueFilename(imageFile.getName()); attachmentsViewController.addAttachments(List.of(ologAttachment)); filesToDeleteAfterSubmit.add(ologAttachment.getFile()); } catch (IOException e) { @@ -361,7 +361,8 @@ public ObservableList getAttachments() { /** * Sets the file upload constraints information in the editor. - * @param maxFileSize Maximum size for a single file. + * + * @param maxFileSize Maximum size for a single file. * @param maxRequestSize Maximum total size of all attachments. */ public void setSizeLimits(String maxFileSize, String maxRequestSize) { @@ -409,19 +410,20 @@ private void showTotalSizeExceedsLimit() { /** * Clears list of {@link Attachment}s. */ - public void clearAttachments(){ + public void clearAttachments() { getAttachments().clear(); } /** * Checks if any of the attached files uses extension heic or heics, which are unsupported. If a heic file * is detected, an error dialog is shown. + * * @param files List of {@link File}s to check. * @return true if all is well, i.e. no heic files deyected, otherwise false. */ - private boolean checkForHeicFiles(List files){ + private boolean checkForHeicFiles(List files) { File file = detectHeicFiles(files); - if(file != null){ + if (file != null) { Alert alert = new Alert(AlertType.ERROR); alert.setHeaderText(MessageFormat.format(Messages.UnsupportedFileType, file.getAbsolutePath())); DialogHelper.positionDialog(alert, textArea, -200, -100); @@ -433,15 +435,16 @@ private boolean checkForHeicFiles(List files){ /** * Probes files for heic content, which is not supported. + * * @param files List of {@link File}s to check. * @return The first {@link File} in the list determined to have heic. If no such file * is detected, null is returned. */ - private File detectHeicFiles(List files){ - for(File file : files){ + private File detectHeicFiles(List files) { + for (File file : files) { try { String mimeType = MimeTypeDetector.determineMimeType(new FileInputStream(file)); - if(mimeType != null && mimeType.toLowerCase().contains("image/heic")){ + if (mimeType != null && mimeType.toLowerCase().contains("image/heic")) { return file; } } catch (IOException e) { diff --git a/app/logbook/olog/ui/src/test/java/org/phoebus/logbook/olog/ui/LogEntryDisplayDemo.java b/app/logbook/olog/ui/src/test/java/org/phoebus/logbook/olog/ui/LogEntryDisplayDemo.java index 7c1b1c44b8..810884af70 100644 --- a/app/logbook/olog/ui/src/test/java/org/phoebus/logbook/olog/ui/LogEntryDisplayDemo.java +++ b/app/logbook/olog/ui/src/test/java/org/phoebus/logbook/olog/ui/LogEntryDisplayDemo.java @@ -154,10 +154,10 @@ public void start(Stage primaryStage) throws Exception { ologLog.setProperties(Arrays.asList(track, experimentProperty)); List attachments = new ArrayList<>(); OlogAttachment attachment1 = new OlogAttachment("image_1.png"); - attachment1.setFileName("image_1.png"); + attachment1.setUniqueFilename("image_1.png"); attachment1.setContentType("image"); OlogAttachment attachment2 = new OlogAttachment("file_phoebus.txt"); - attachment2.setFileName("file_phoebus.txt"); + attachment2.setUniqueFilename("file_phoebus.txt"); attachment2.setContentType("text"); attachments.add(attachment1); attachments.add(attachment2); diff --git a/core/logbook/src/main/java/org/phoebus/logbook/Attachment.java b/core/logbook/src/main/java/org/phoebus/logbook/Attachment.java index 0e971a21b4..0a0b2e578c 100644 --- a/core/logbook/src/main/java/org/phoebus/logbook/Attachment.java +++ b/core/logbook/src/main/java/org/phoebus/logbook/Attachment.java @@ -18,16 +18,29 @@ default String getId(){ /** * In some cases the client must be able to set the (unique) id. - * @param id + * @param id A unique id for the {@link Attachment} */ default void setId(String id){} - public String getName(); + String getName(); - public File getFile(); + File getFile(); - public String getContentType(); + String getContentType(); - public Boolean getThumbnail(); + Boolean getThumbnail(); + /** + * Implementations must make sure this returns a string that: + *
    + *
  • Is unique between all attachments in a log entry submission.
    • + *
  • Preserves the file extension, if the original file defines it.
    • + *
+ * Note that this is practice not the same as the name of a file + * picked from the file system. + * @return A {@link String} unique between all attachments in a log entry submission. + */ + default String getUniqueFilename(){ + return null; + } } diff --git a/core/util/src/main/java/org/phoebus/util/http/HttpRequestMultipartBody.java b/core/util/src/main/java/org/phoebus/util/http/HttpRequestMultipartBody.java index 2eab012066..a5df781e02 100644 --- a/core/util/src/main/java/org/phoebus/util/http/HttpRequestMultipartBody.java +++ b/core/util/src/main/java/org/phoebus/util/http/HttpRequestMultipartBody.java @@ -63,9 +63,11 @@ public void addTextPart(String fieldName, String value, String contentType) { * application/octet-stream if probe fails. * * @param file A non-null file. + * @param uniqueFileName A file name that must be unique between all attachment files in a log entry submission. Note + * that a file extension - where applicable - must be present. * @throws RuntimeException if the file does not exist or cannot be read. */ - public void addFilePart(File file) { + public void addFilePart(File file, String uniqueFileName) { if (file == null){ throw new RuntimeException("File part must not be null"); } @@ -84,7 +86,7 @@ else if(!file.exists() || !file.canRead()){ } catch (IOException e) { Logger.getLogger(HttpRequestMultipartBody.class.getName()).log(Level.WARNING, "Unable to determine content type of file " + file.getAbsolutePath(), e); } - stringBuilder.append("\r\n--").append(boundary).append("\r\nContent-Disposition: form-data; name=\"").append("files").append("\"; filename=\"").append(file.getName()).append("\""); + stringBuilder.append("\r\n--").append(boundary).append("\r\nContent-Disposition: form-data; name=\"").append("files").append("\"; filename=\"").append(uniqueFileName).append("\""); stringBuilder.append("\r\nContent-Type: ").append(contentType).append("\r\n\r\n"); byteArrayOutputStream.writeBytes(stringBuilder.toString().getBytes(StandardCharsets.UTF_8)); diff --git a/core/util/src/test/java/org/phoebus/util/http/HttpRequestMultipartBodyTest.java b/core/util/src/test/java/org/phoebus/util/http/HttpRequestMultipartBodyTest.java index 2c68990f45..4147f1747b 100644 --- a/core/util/src/test/java/org/phoebus/util/http/HttpRequestMultipartBodyTest.java +++ b/core/util/src/test/java/org/phoebus/util/http/HttpRequestMultipartBodyTest.java @@ -37,7 +37,7 @@ public void testBody() throws IOException { fileOutputStream.flush(); fileOutputStream.close(); - httpRequestMultipartBody.addFilePart(file); + httpRequestMultipartBody.addFilePart(file, file.getName()); String body = new String(httpRequestMultipartBody.getBytes());