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 48afecb0a8..93abd96240 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 @@ -32,6 +32,7 @@ import org.phoebus.util.http.HttpRequestMultipartBody; import org.phoebus.util.http.QueryParamsHelper; +import javax.ws.rs.HttpMethod; import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import java.io.InputStream; @@ -158,7 +159,7 @@ private OlogHttpClient(String userName, String password) { @Override public LogEntry set(LogEntry log) throws LogbookException { - return save(log, null); + return save(log, null, HttpMethod.PUT); } /** @@ -171,7 +172,7 @@ public LogEntry set(LogEntry log) throws LogbookException { * @throws LogbookException E.g. due to invalid log entry data, or if attachment content type * cannot be determined. */ - private LogEntry save(LogEntry log, LogEntry inReplyTo) throws LogbookException { + private LogEntry save(LogEntry log, LogEntry inReplyTo, String method) throws LogbookException { try { javax.ws.rs.core.MultivaluedMap queryParams = new javax.ws.rs.core.MultivaluedHashMap<>(); queryParams.putSingle("markup", "commonmark"); @@ -183,24 +184,29 @@ 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(), attachment.getUniqueFilename()); + if(attachment.getFile() != null){ + httpRequestMultipartBody.addFilePart(attachment.getFile(), attachment.getUniqueFilename()); + } } + HttpRequest.BodyPublisher bodyPublisher = HttpRequest.BodyPublishers.ofByteArray(httpRequestMultipartBody.getBytes()); + HttpRequest request = HttpRequest.newBuilder() .uri(URI.create(Preferences.olog_url + OLOG_PREFIX + "/logs/multipart?" + QueryParamsHelper.mapToQueryParams(queryParams))) .header("Content-Type", httpRequestMultipartBody.getContentType()) .header(OLOG_CLIENT_INFO_HEADER, CLIENT_INFO) .header("Authorization", basicAuthenticationHeader) - .PUT(HttpRequest.BodyPublishers.ofByteArray(httpRequestMultipartBody.getBytes())) + .method(method, bodyPublisher) + //.PUT(HttpRequest.BodyPublishers.ofByteArray(httpRequestMultipartBody.getBytes())) .build(); HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); if(response.statusCode() == 401){ - LOGGER.log(Level.SEVERE, "Failed to create log entry: user not authenticated"); + LOGGER.log(Level.SEVERE, "Failed to create or update log entry: user not authenticated"); throw new LogbookException(Messages.SubmissionFailedInvalidCredentials); } else if (response.statusCode() >= 300) { - LOGGER.log(Level.SEVERE, "Failed to create log entry: " + response.body()); + LOGGER.log(Level.SEVERE, "Failed to create or update log entry: " + response.body()); throw new LogbookException(response.body()); } else { return OlogObjectMappers.logEntryDeserializer.readValue(response.body(), OlogLog.class); @@ -213,7 +219,7 @@ else if (response.statusCode() >= 300) { @Override public LogEntry reply(LogEntry log, LogEntry inReplyTo) throws LogbookException { - return save(log, inReplyTo); + return save(log, inReplyTo, HttpMethod.PUT); } /** @@ -305,32 +311,21 @@ public String getServiceUrl() { } /** - * Updates an existing {@link LogEntry}. Note that unlike the {@link #save(LogEntry, LogEntry)} API, - * this does not support attachments, i.e. it does not set up a multipart request to the service. + * Updates an existing {@link LogEntry}. + *

NOTE:

+ * The list of attachments in the {@link LogEntry} may contain additional attachments added by the user. Moreover, + * attachments listed in the {@link LogEntry} subject to change, but not listed in the {@link LogEntry} submitted + * to the service, will be removed. This makes it possible for a user to update an entry such that a + * potentially erroneous attachment is replaced by a correct one. * * @param logEntry - the updated log entry * @return The updated {@link LogEntry} */ @Override - public LogEntry update(LogEntry logEntry) { - - try { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create(Preferences.olog_url + OLOG_PREFIX + "/logs/" + logEntry.getId() + "?markup=commonmark")) - .header("Content-Type", CONTENT_TYPE_JSON) - .header("Authorization", basicAuthenticationHeader) - .POST(HttpRequest.BodyPublishers.ofString(OlogObjectMappers.logEntrySerializer.writeValueAsString(logEntry))) - .build(); - - HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); - - LogEntry updated = OlogObjectMappers.logEntryDeserializer.readValue(response.body(), OlogLog.class); - changeHandlers.forEach(h -> h.logEntryChanged(updated)); - return updated; - } catch (Exception e) { - LOGGER.log(Level.SEVERE, "Unable to update log entry id=" + logEntry.getId(), e); - return null; - } + public LogEntry update(LogEntry logEntry) throws LogbookException{ + LogEntry updated = save(logEntry, null, HttpMethod.POST); + changeHandlers.forEach(h -> h.logEntryChanged(updated)); + return updated; } @Override @@ -502,6 +497,25 @@ public InputStream getAttachment(Long logId, String attachmentName) { } } + @Override + public InputStream getAttachment(Long logId, Attachment attachment){ + try { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(Preferences.olog_url + OLOG_PREFIX + "/attachment/" + attachment.getId())) + .GET() + .build(); + HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); + if (response.statusCode() >= 300) { + LOGGER.log(Level.WARNING, "failed to obtain attachment: " + new String(response.body().readAllBytes())); + return null; + } + return response.body(); + } catch (Exception e) { + LOGGER.log(Level.WARNING, "failed to obtain attachment", e); + return null; + } + } + /** * @param id Unique log entry id * @return A {@link SearchResult} containing a list of {@link LogEntry} objects representing the diff --git a/app/logbook/olog/ui/doc/images/ContextMenuEdit.png b/app/logbook/olog/ui/doc/images/ContextMenuEdit.png new file mode 100644 index 0000000000..a30e987df4 Binary files /dev/null and b/app/logbook/olog/ui/doc/images/ContextMenuEdit.png differ diff --git a/app/logbook/olog/ui/doc/images/ContextMenuLogEntryTable.png b/app/logbook/olog/ui/doc/images/ContextMenuLogEntryTable.png index 403c9ec9be..1f05a29ce0 100644 Binary files a/app/logbook/olog/ui/doc/images/ContextMenuLogEntryTable.png and b/app/logbook/olog/ui/doc/images/ContextMenuLogEntryTable.png differ diff --git a/app/logbook/olog/ui/doc/index.rst b/app/logbook/olog/ui/doc/index.rst index e6d2cb8c84..106f945c8a 100644 --- a/app/logbook/olog/ui/doc/index.rst +++ b/app/logbook/olog/ui/doc/index.rst @@ -266,6 +266,18 @@ Please consider the following limitations of the log entry group feature: .. _preferences: +Updating an existing log entry +------------------------------ + +Existing log entries may be modified with respect to all parts of the log entry, except author and create/modify date. +A right click on an item in the list view will bring up the context menu from which the user can select Edit, see below. + +**NOTE**: when editing a log entry, the attachments view will show the attachments added when the entry was created +or modified. In this view users may add further attachments, but also remove the ones added previously. Removed attachments +will then be accessible only when inspecting the history of the entry. + +.. image:: images/ContextMenuEdit.png + Preferences ----------- diff --git a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/AttachmentsViewController.java b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/AttachmentsViewController.java index 8290b0dd53..0215e7e3f6 100644 --- a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/AttachmentsViewController.java +++ b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/AttachmentsViewController.java @@ -47,7 +47,12 @@ import org.phoebus.framework.spi.AppResourceDescriptor; import org.phoebus.framework.workbench.ApplicationService; import org.phoebus.logbook.Attachment; +import org.phoebus.logbook.LogClient; import org.phoebus.logbook.LogEntry; +import org.phoebus.logbook.LogService; +import org.phoebus.logbook.LogbookException; +import org.phoebus.logbook.LogbookPreferences; +import org.phoebus.olog.es.api.OlogHttpClient; import org.phoebus.ui.application.ApplicationLauncherService; import org.phoebus.ui.application.PhoebusApplication; import org.phoebus.ui.dialog.ExceptionDetailsErrorDialog; @@ -57,6 +62,7 @@ import java.awt.image.BufferedImage; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.nio.file.Files; import java.nio.file.StandardCopyOption; @@ -231,6 +237,12 @@ public void invalidateAttachmentList(LogEntry logEntry) { attachments.setAll(Collections.emptyList()); } + /** + * Sets the list of {@link Attachment}s when UI is created. This list is non-empty for instance if + * log entry is created from OPI (in which case the attschment is automatically added to a log entry), + * or when editing an existing entry containing attachments. + * @param attachmentsList List of {@link Attachment}s + */ public void setAttachments(Collection attachmentsList) { Platform.runLater(() -> { this.attachments.setAll(attachmentsList); @@ -278,26 +290,33 @@ private void showPreview(Attachment attachment) { * @param attachment The image {@link Attachment} selected by user. */ private void showImagePreview(Attachment attachment) { - if (attachment.getFile() != null && attachment.getFile().exists()) { - // Load image data off UI thread... - JobManager.schedule("Show image attachment", monitor -> { - try { - BufferedImage bufferedImage = ImageIO.read(attachment.getFile()); - // BufferedImage may be null due to lazy loading strategy. - if (bufferedImage == null) { - return; + JobManager.schedule("Show image attachment", monitor -> { + try { + BufferedImage bufferedImage = null; + if (attachment.getFile() != null && attachment.getFile().exists()) { // Attachment exists on disk + bufferedImage = ImageIO.read(attachment.getFile()); + } + else { // Attachment must be retrieved from service + LogClient logClient = LogService.getInstance().getLogFactories().get(LogbookPreferences.logbook_factory).getLogClient(); + InputStream inputStream = logClient.getAttachment(-1L, attachment); + if (inputStream != null) { + bufferedImage = ImageIO.read(inputStream); } - Platform.runLater(() -> { - Image image = SwingFXUtils.toFXImage(bufferedImage, null); - imagePreview.visibleProperty().setValue(true); - imagePreview.setImage(image); - }); - } catch (IOException ex) { - Logger.getLogger(AttachmentsViewController.class.getName()) - .log(Level.SEVERE, "Unable to load image file " + attachment.getFile().getAbsolutePath(), ex); } - }); - } + if (bufferedImage == null) { + return; + } + BufferedImage _bufferedImage = bufferedImage; + Platform.runLater(() -> { + Image image = SwingFXUtils.toFXImage(_bufferedImage, null); + imagePreview.visibleProperty().setValue(true); + imagePreview.setImage(image); + }); + } catch (Exception ex) { + Logger.getLogger(AttachmentsViewController.class.getName()) + .log(Level.SEVERE, "Unable to load image file " + attachment.getName(), ex); + } + }); } /** diff --git a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/OlogAttributeProvider.java b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/OlogAttributeProvider.java index c0e0b7e1c8..4121899338 100644 --- a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/OlogAttributeProvider.java +++ b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/OlogAttributeProvider.java @@ -18,17 +18,15 @@ package org.phoebus.logbook.olog.ui; +import org.apache.commons.io.FilenameUtils; import org.commonmark.ext.gfm.tables.TableBlock; import org.commonmark.renderer.html.AttributeProvider; - -import java.util.Map; -import java.util.logging.Level; -import java.util.logging.Logger; import org.phoebus.logbook.Attachment; -import java.util.List; -import org.apache.commons.io.FilenameUtils; import org.phoebus.olog.es.api.OlogHttpClient; +import java.util.List; +import java.util.Map; + /** * An {@link AttributeProvider} used to style elements of a log entry. Other types of * attribute processing may be added. @@ -39,18 +37,19 @@ public class OlogAttributeProvider implements AttributeProvider { private boolean preview = false; private List attachments; - public OlogAttributeProvider(String serviceUrl){ + public OlogAttributeProvider(String serviceUrl) { this.serviceUrl = serviceUrl; } /** * This is constructor for HTML preview feature - * @param serviceUrl Olog service url. - * @param preview A boolean flag set true for HTML preview feature parsing. + * + * @param serviceUrl Olog service url. + * @param preview A boolean flag set true for HTML preview feature parsing. * @param attachments A list of current attachments which can be parsed to - * find attachment file path from attachment id. + * find attachment file path from attachment id. */ - public OlogAttributeProvider(String serviceUrl, boolean preview, List attachments){ + public OlogAttributeProvider(String serviceUrl, boolean preview, List attachments) { this.serviceUrl = serviceUrl; this.preview = preview; this.attachments = attachments; @@ -80,12 +79,17 @@ public void setAttributes(org.commonmark.node.Node node, String s, Map { if (a.getFile() != null) { attachedFilesSize += getFileSize(a.getFile()); + filesToDeleteAfterSubmit.add(a.getFile()); } }); } attachmentsViewController.setAttachments(attachments); - filesToDeleteAfterSubmit.addAll(attachments.stream().map(Attachment::getFile).toList()); - removeButton.setGraphic(ImageCache.getImageView(ImageCache.class, "/icons/delete.png")); removeButton.disableProperty().bind(Bindings.isEmpty(attachmentsViewController.getSelectedAttachments())); diff --git a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogEntryEditorController.java b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogEntryEditorController.java index f5b0008876..15cee165d5 100644 --- a/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogEntryEditorController.java +++ b/app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogEntryEditorController.java @@ -309,7 +309,6 @@ public void initialize() { templateControls.visibleProperty().setValue(editMode.equals(EditMode.NEW_LOG_ENTRY)); attachmentsPane.managedProperty().bind(attachmentsPane.visibleProperty()); - attachmentsPane.visibleProperty().setValue(editMode.equals(EditMode.NEW_LOG_ENTRY)); // This could be configured in the fxml, but then these UI components would not be visible // in Scene Builder. @@ -734,13 +733,11 @@ public void submit() { ologLog.setLevel(selectedLevelProperty.get()); ologLog.setLogbooks(getSelectedLogbooks()); ologLog.setTags(getSelectedTags()); - if (editMode.equals(EditMode.NEW_LOG_ENTRY)) { - ologLog.setAttachments(attachmentsEditorController.getAttachments()); - } - else{ + ologLog.setAttachments(attachmentsEditorController.getAttachments()); + ologLog.setProperties(logPropertiesEditorController.getProperties()); + if (editMode.equals(EditMode.UPDATE_LOG_ENTRY)) { ologLog.setId(logEntry.getId()); } - ologLog.setProperties(logPropertiesEditorController.getProperties()); LogClient logClient = logFactory.getLogClient(new SimpleAuthenticationToken(usernameProperty.get(), passwordProperty.get()));