From a500ef01e65d4bc4c46c3db8e526b9e36d7af4c9 Mon Sep 17 00:00:00 2001 From: millmanorama Date: Sun, 10 Mar 2019 15:32:02 +0100 Subject: [PATCH] fix issues found in code review --- .../autopsy/timeline/TimeLineController.java | 1 - .../timeline/actions/AddManualEvent.java | 33 ++++++++++------- .../timeline/actions/Bundle.properties-MERGED | 3 ++ .../timeline/utils/TimelineDBUtils.java | 35 ++++++++++++++----- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/timeline/TimeLineController.java b/Core/src/org/sleuthkit/autopsy/timeline/TimeLineController.java index 0d4e3fb8bb..190f0ad9e9 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/TimeLineController.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/TimeLineController.java @@ -33,7 +33,6 @@ import java.util.Optional; import java.util.TimeZone; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; import java.util.logging.Level; import javafx.application.Platform; import javafx.beans.Observable; diff --git a/Core/src/org/sleuthkit/autopsy/timeline/actions/AddManualEvent.java b/Core/src/org/sleuthkit/autopsy/timeline/actions/AddManualEvent.java index 609f4081de..559342067a 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/actions/AddManualEvent.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/actions/AddManualEvent.java @@ -21,7 +21,6 @@ package org.sleuthkit.autopsy.timeline.actions; import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneId; -import java.util.Arrays; import static java.util.Arrays.asList; import java.util.List; import java.util.Objects; @@ -75,7 +74,7 @@ import org.sleuthkit.datamodel.timeline.EventType; public class AddManualEvent extends Action { private final static Logger logger = Logger.getLogger(AddManualEvent.class.getName()); - private static final String MANUAL_CREATION = "Manual Creation"; + private static final String MANUAL_CREATION = "Manual Creation"; //NON-NLS private static final Image ADD_EVENT_IMAGE = new Image("/org/sleuthkit/autopsy/timeline/images/add.png", 16, 16, true, true, true); // NON-NLS private final TimeLineController controller; @@ -118,7 +117,7 @@ public class AddManualEvent extends Action { setLongText(Bundle.AddManualEvent_longText()); setEventHandler(actionEvent -> { - //shoe the dialog and if it completed normally add the event. + //show the dialog and if it completed normally add the event. new EventCreationDialog(controller, epochMillis).showAndWait().ifPresent(this::addEvent); }); } @@ -158,11 +157,11 @@ public class AddManualEvent extends Action { try { sleuthkitCase.getBlackboard().postArtifact(artifact, source); } catch (Blackboard.BlackboardException ex) { - logger.log(Level.SEVERE, "Error posting artifact to the blackboard.", ex); + logger.log(Level.SEVERE, "Error posting artifact to the blackboard.", ex); //NON-NLS new Alert(Alert.AlertType.ERROR, Bundle.AddManualEvent_postArtifactFailed(), ButtonType.OK).showAndWait(); } } catch (TskCoreException ex) { - logger.log(Level.SEVERE, "Error creatig new artifact.", ex); + logger.log(Level.SEVERE, "Error creatig new artifact.", ex); //NON-NLS new Alert(Alert.AlertType.ERROR, Bundle.AddManualEvent_createArtifactFailed(), ButtonType.OK).showAndWait(); } } @@ -175,7 +174,7 @@ public class AddManualEvent extends Action { EventCreationDialog(TimeLineController controller, Long epochMillis) { this.eventCreationDialogPane = new EventCreationDialogPane(controller, epochMillis); - setTitle("Add Event"); + setTitle(Bundle.AddManualEvent_text()); setDialogPane(eventCreationDialogPane); //We can't do these steps until after the dialog is shown or we get an error. @@ -215,7 +214,7 @@ public class AddManualEvent extends Action { EventCreationDialogPane(TimeLineController controller, Long epochMillis) { this.controller = controller; - FXMLConstructor.construct(this, "EventCreationDialog.fxml"); + FXMLConstructor.construct(this, "EventCreationDialog.fxml"); //NON-NLS if (epochMillis == null) { timePicker.setLocalDateTime(LocalDateTime.now()); } else { @@ -226,7 +225,7 @@ public class AddManualEvent extends Action { @FXML @NbBundle.Messages("AddManualEvent.EventCreationDialogPane.initialize.dataSourcesError=Error getting datasources in case.") void initialize() { - assert descriptionTextField != null : "fx:id=\"descriptionTextField\" was not injected: check your FXML file 'EventCreationDialog.fxml'."; + assert descriptionTextField != null : "fx:id=\"descriptionTextField\" was not injected: check your FXML file 'EventCreationDialog.fxml'.";//NON-NLS timeZoneChooser.getItems().setAll(timeZoneList); timeZoneChooser.getSelectionModel().select(TimeZoneUtils.createTimeZoneString(TimeLineController.getTimeZone())); @@ -241,9 +240,12 @@ public class AddManualEvent extends Action { return dataSource.getName() + "(ID: " + dataSource.getId() + ")"; } + /** + * This method should never get called. + */ @Override public DataSource fromString(String string) { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + throw new UnsupportedOperationException("Not supported yet."); //NON-NLS } }); } catch (TskCoreException ex) { @@ -255,10 +257,17 @@ public class AddManualEvent extends Action { /** * Install/Configure the ValidationSupport. */ + @NbBundle.Messages({ + "AddManualEvent.validation.description=Description is required.", + "AddManualEvent.validation.datetime=Invalid datetime", + "AddManualEvent.validation.timezone=Invalid time zone",}) void installValidation() { - validationSupport.registerValidator(descriptionTextField, false, Validator.createEmptyValidator("Description is required")); - validationSupport.registerValidator(timePicker, false, Validator.createPredicateValidator(Objects::nonNull, "Invalid datetime")); - validationSupport.registerValidator(timeZoneChooser, false, Validator.createPredicateValidator((String zone) -> timeZoneList.contains(zone.trim()), "Invalid time zone")); + validationSupport.registerValidator(descriptionTextField, false, + Validator.createEmptyValidator(Bundle.AddManualEvent_validation_description())); + validationSupport.registerValidator(timePicker, false, + Validator.createPredicateValidator(Objects::nonNull, Bundle.AddManualEvent_validation_description())); + validationSupport.registerValidator(timeZoneChooser, false, + Validator.createPredicateValidator((String zone) -> timeZoneList.contains(zone.trim()), Bundle.AddManualEvent_validation_timezone())); validationSupport.initInitialDecoration(); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/actions/Bundle.properties-MERGED b/Core/src/org/sleuthkit/autopsy/timeline/actions/Bundle.properties-MERGED index fcd8b62ceb..11501087e7 100755 --- a/Core/src/org/sleuthkit/autopsy/timeline/actions/Bundle.properties-MERGED +++ b/Core/src/org/sleuthkit/autopsy/timeline/actions/Bundle.properties-MERGED @@ -3,6 +3,9 @@ AddManualEvent.EventCreationDialogPane.initialize.dataSourcesError=Error getting AddManualEvent.longText=Manually add an event to the timeline. AddManualEvent.postArtifactFailed=Failed to post artifact to blackboard. AddManualEvent.text=Add Event +AddManualEvent.validation.datetime=Invalid datetime +AddManualEvent.validation.description=Description is required. +AddManualEvent.validation.timezone=Invalid time zone # {0} - action accelerator keys Back.longText=Back: {0}\nGo back to the last view settings. Back.text=Back diff --git a/Core/src/org/sleuthkit/autopsy/timeline/utils/TimelineDBUtils.java b/Core/src/org/sleuthkit/autopsy/timeline/utils/TimelineDBUtils.java index 8570759ef9..5c67852ae5 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/utils/TimelineDBUtils.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/utils/TimelineDBUtils.java @@ -1,7 +1,7 @@ /* * Autopsy Forensic Browser * - * Copyright 2018 Basis Technology Corp. + * Copyright 2018-2019 Basis Technology Corp. * Contact: carrier sleuthkit org * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -27,25 +27,41 @@ import org.sleuthkit.datamodel.TskCoreException; import org.sleuthkit.datamodel.TskData; /** - * + * Utilities to help work with databases. */ public class TimelineDBUtils { private final SleuthkitCase sleuthkitCase; + /** + * Create a TimelineDBUtils for the givecn SleuthlitCase. + * + * @param sleuthkitCase The case to make utilities for. The type of the + * underlying database is used in the implementation of + * these utilities. + */ public TimelineDBUtils(SleuthkitCase sleuthkitCase) { this.sleuthkitCase = sleuthkitCase; } - public String csvAggFunction(String args) { + /** + * Wrap the given columnName in the appropiate sql function to get a comma + * seperated list in the result. + * + * @param columnName + * + * @return An sql expression that will produce a comma seperated list of + * values from the given column in the result. + */ + public String csvAggFunction(String columnName) { return (sleuthkitCase.getDatabaseType() == TskData.DbType.POSTGRESQL ? "string_agg" : "group_concat") - + "(Cast (" + args + " AS VARCHAR) , '" + "," + "')"; + + "(Cast (" + columnName + " AS VARCHAR) , ',')"; //NON-NLS } /** - * take the result of a group_concat SQLite operation and split it into a - * set of X using the mapper to to convert from string to X If groupConcat - * is empty, null, or all whitespace, returns an empty list. + * Take the result of a group_concat / string_agg sql operation and split it + * into a set of X using the mapper to convert from string to X. If + * groupConcat is empty, null, or all whitespace, returns an empty list. * * @param the type of elements to return * @param groupConcat a string containing the group_concat result ( a comma @@ -55,13 +71,14 @@ public class TimelineDBUtils { * @return a Set of X, each element mapped from one element of the original * comma delimited string * - * @throws org.sleuthkit.datamodel.TskCoreException + * @throws org.sleuthkit.datamodel.TskCoreException If the mapper throws a + * TskCoreException */ public static List unGroupConcat(String groupConcat, CheckedFunction mapper) throws TskCoreException { if (isBlank(groupConcat)) { return Collections.emptyList(); - } + } List result = new ArrayList<>(); for (String s : groupConcat.split(",")) {