Support Olog attachments with same original file name

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()

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;
     }
 

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<OlogProperty> {
 
         @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<String, String> attributes = new HashMap<String, String>();
+            Map<String, String> 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<OlogAttachment> {
 
         @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);
     }
 }

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<InputStream> 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<InputStream> 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<String> 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<LogEntry> 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());
         }
     }
 

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('.');

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<Attachment> 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<File> 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<Attachment> 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 <code>true</code> if all is well, i.e. no heic files deyected, otherwise <code>false</code>.
      */
-    private boolean checkForHeicFiles(List<File> files){
+    private boolean checkForHeicFiles(List<File> 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<File> 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, <code>null</code> is returned.
      */
-    private File detectHeicFiles(List<File> files){
-        for(File file : files){
+    private File detectHeicFiles(List<File> 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) {

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<Attachment> 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);

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:
+     * <ul>
+     *     <li>Is unique between all attachments in a log entry submission.</li>
+     *     <li>Preserves the file extension, if the original file defines it.</li>
+     * </ul>
+     * Note that this is practice <i>not</i> 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;
+    }
 }