Merge pull request #3439 from rcordovano/develop

Fixes to ExtractedContentViewer for handling node lookup contents correctly

KeywordSearch/src/org/sleuthkit/autopsy/keywordsearch/ExtractedContentViewer.java

@@ -305,10 +305,9 @@ public class ExtractedContentViewer implements DataContentViewer {
         }
 
         /*
-         * If the lookup of the node contains either a keyword hit artifact or
+         * If the lookup of the node contains either a keyword hit artifact or a
-         * one to many credit card account artifacts from a credit card account
+         * credit card account artifact from a credit card account numbers
-         * numbers search, then there must be indexed text that produced the
+         * search, then there must be indexed text that produced the hit(s).
-         * hit(s).
          */
         BlackboardArtifact artifact = node.getLookup().lookup(BlackboardArtifact.class);
         if (artifact != null) {
@@ -323,53 +322,58 @@ public class ExtractedContentViewer implements DataContentViewer {
                     }
                 } catch (TskCoreException ex) {
                     /*
-                     * If there is an error, log it and return true. The reason
+                     * If there was an error checking the account type, fall
-                     * for returning true is so that the user will have an
+                     * back to the check below for the file associated with the
-                     * opportunity to see an error message in the panel when
+                     * account (if there is one).
-                     * this query fails again when setNode is called, instead of
-                     * having an unexpectedly disabled content viewer with no
-                     * other feedback.
                      */
                     logger.log(Level.SEVERE, "Error getting TSK_ACCOUNT_TYPE attribute from artifact " + artifact.getArtifactID(), ex);
-                    return true;
                 }
             }
         }
 
+        /*
+         * If the lookup of the node contains a file, check to see if there is
+         * indexed text for the file. Note that there should be a file in the
+         * lookup of all nodes except artifact nodes that are associated with a
+         * data source instead of a file.
+         */
+        AbstractFile file = node.getLookup().lookup(AbstractFile.class);
+        if (file != null && solrHasContent(file.getId())) {
+            return true;
+        }
+
         /*
          * If the lookup of the node contains an artifact that is neither a
-         * keyword hit artifact nor a credit card account artifact, check to see
+         * keyword hit artifact nor a credit card account artifact, and the
-         * if there is indexed text for the artifact.
+         * artifact is not associated with a file, check to see if there is
+         * indexed text for the artifact.
          */
         if (artifact != null) {
             return solrHasContent(artifact.getArtifactID());
         }
 
-        /*
-         * If the lookup of the node contains no artifacts but does contain a
-         * file, check to see if there is indexed text for the file.
-         */
-        AbstractFile file = node.getLookup().lookup(AbstractFile.class);
-        if (file != null) {
-            return solrHasContent(file.getId());
-        }
-
-        /*
-         * If the lookup of the node contains neither ad hoc search results, nor
-         * artifacts, nor a file, there is no indexed text.
-         */
         return false;
     }
 
     @Override
     public int isPreferred(Node node) {
-        BlackboardArtifact art = node.getLookup().lookup(BlackboardArtifact.class);
+        BlackboardArtifact artifact = node.getLookup().lookup(BlackboardArtifact.class);
-
+        if (artifact == null) {
-        if (art == null) {
             return 4;
-        } else if (art.getArtifactTypeID() == BlackboardArtifact.ARTIFACT_TYPE.TSK_KEYWORD_HIT.getTypeID()
+        } else if (artifact.getArtifactTypeID() == BlackboardArtifact.ARTIFACT_TYPE.TSK_KEYWORD_HIT.getTypeID()) {
-                || art.getArtifactTypeID() == BlackboardArtifact.ARTIFACT_TYPE.TSK_ACCOUNT.getTypeID()) {
             return 6;
+        } else if (artifact.getArtifactTypeID() == BlackboardArtifact.ARTIFACT_TYPE.TSK_ACCOUNT.getTypeID()) {
+            try {
+                BlackboardAttribute attribute = artifact.getAttribute(TSK_ACCOUNT_TYPE);
+                if (attribute != null && Account.Type.CREDIT_CARD.getTypeName().equals(attribute.getValueString())) {
+                    return 6;
+                } else {
+                    return 4;
+                }
+            } catch (TskCoreException ex) {
+                logger.log(Level.SEVERE, "Error getting TSK_ACCOUNT_TYPE attribute from artifact " + artifact.getArtifactID(), ex);
+                return 4;
+            }
         } else {
             return 4;
         }

ruleset.xml

@@ -62,7 +62,8 @@
   <!-- Commented out because it was flagged some of our header / copyright comments
   <rule ref="rulesets/java/comments.xml/CommentSize"/> -->
   <rule ref="rulesets/java/comments.xml/CommentContent"/>
-  <rule ref="rulesets/java/comments.xml/CommentDefaultAccessModifier"/>
+  <!-- Commented out because we use default (package) access often and commenting it does not make sense for us
+  <rule ref="rulesets/java/comments.xml/CommentDefaultAccessModifier"/> -->
   <!--
     Commented out because they are controversial and we want basics right now.
   <rule ref="rulesets/java/controversial.xml/NullAssignment"/>
@@ -212,13 +213,16 @@
   <rule ref="rulesets/java/naming.xml/NoPackage"/>
   <rule ref="rulesets/java/naming.xml/MethodWithSameNameAsEnclosingClass"/>
   <rule ref="rulesets/java/naming.xml/ShortVariable"/>
-  <rule ref="rulesets/java/naming.xml/LongVariable"/>
+  <!-- Commented out because clarity trumps brevity, developers can use their own judgement
+  <rule ref="rulesets/java/naming.xml/LongVariable"/>  -->
   <rule ref="rulesets/java/naming.xml/ShortMethodName"/>
   <rule ref="rulesets/java/naming.xml/BooleanGetMethodName"/>
   <rule ref="rulesets/java/naming.xml/PackageCase"/>
+  <!-- Commented out because this is a nice micro-refinement, but too much of our code does not do this, developers can, but don't have to ignore this practice for now
   <rule ref="rulesets/java/optimizations.xml/LocalVariableCouldBeFinal"/>
-  <rule ref="rulesets/java/optimizations.xml/MethodArgumentCouldBeFinal"/>
+  <rule ref="rulesets/java/optimizations.xml/MethodArgumentCouldBeFinal"/>  -->
-  <rule ref="rulesets/java/optimizations.xml/AvoidInstantiatingObjectsInLoops"/>
+  <!-- Commented out because this is not typical coding practice and it seems of dubious value, e.g., https://stackoverflow.com/questions/17340421/pmd-avoid-instantiating-new-objects-inside-loops/17458503
+  <rule ref="rulesets/java/optimizations.xml/AvoidInstantiatingObjectsInLoops"/>   -->
   <rule ref="rulesets/java/optimizations.xml/UseArrayListInsteadOfVector"/>
   <rule ref="rulesets/java/optimizations.xml/SimplifyStartsWith"/>
   <rule ref="rulesets/java/optimizations.xml/UseStringBufferForStringAppends"/>