GP-155 - Help - fixed intermittent help build issue caused by map keys

using display text instead of the unique help item ID

Closes #2443

Ghidra/Framework/Help/src/main/java/help/OverlayHelpTree.java

@@ -42,13 +42,11 @@ public class OverlayHelpTree {
 
 	public OverlayHelpTree(TOCItemProvider tocItemProvider, LinkDatabase linkDatabase) {
 		this.linkDatabase = linkDatabase;
-		for (TOCItemExternal external : tocItemProvider.getTOCItemExternalsByDisplayMapping()
+		for (TOCItemExternal external : tocItemProvider.getExternalTocItemsById().values()) {
-				.values()) {
 			addExternalTOCItem(external);
 		}
 
-		for (TOCItemDefinition definition : tocItemProvider.getTOCItemDefinitionsByIDMapping()
+		for (TOCItemDefinition definition : tocItemProvider.getTocDefinitionsByID().values()) {
-				.values()) {
 			addSourceTOCItem(definition);
 		}
 	}
@@ -171,8 +169,16 @@ public class OverlayHelpTree {
 		OverlayNode newRootNode = new OverlayNode(null, rootItem);
 		buildChildren(newRootNode);
 
+		//
+		// The parent to children map is cleared as nodes are created.   The map is populated by
+		// adding any references to the 'parent' key as they are loaded from the help files.
+		// As we build nodes, starting at the root, will will create child nodes for those that
+		// reference the 'parent' key.   If the map is empty, then it means we never built a
+		// node for the 'parent' key, which means we never found a help file containing the
+		// definition for that key.
+		//
 		if (!parentToChildrenMap.isEmpty()) {
-			throw new RuntimeException("Unresolved definitions in tree!");
+			throw new RuntimeException("Unresolved definitions in tree! - " + parentToChildrenMap);
 		}
 		rootNode = newRootNode;
 		return true;

Ghidra/Framework/Help/src/main/java/help/TOCItemProvider.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,18 +15,25 @@
  */
 package help;
 
+import java.util.Map;
+
 import help.validator.model.TOCItemDefinition;
 import help.validator.model.TOCItemExternal;
 
-import java.util.Map;
-
 /**
- * An interface that allows us to perform dependency injection in the testing
+ * An interface that allows us to perform dependency injection in the testing environment
- * environment.
  */
 public interface TOCItemProvider {
 
-	public Map<String, TOCItemExternal> getTOCItemExternalsByDisplayMapping();
+	/**
+	 * Returns all external TOC items referenced by this provider
+	 * @return the items
+	 */
+	public Map<String, TOCItemExternal> getExternalTocItemsById();
 
-	public Map<String, TOCItemDefinition> getTOCItemDefinitionsByIDMapping();
+	/**
+	 * Returns all TOC items defined by this provider
+	 * @return the items
+	 */
+	public Map<String, TOCItemDefinition> getTocDefinitionsByID();
 }

Ghidra/Framework/Help/src/main/java/help/validator/LinkDatabase.java

@@ -108,7 +108,7 @@ public class LinkDatabase {
 	}
 
 	private void collectTOCItemDefinitions(TOCItemProvider tocProvider) {
-		Map<String, TOCItemDefinition> map = tocProvider.getTOCItemDefinitionsByIDMapping();
+		Map<String, TOCItemDefinition> map = tocProvider.getTocDefinitionsByID();
 		Set<Entry<String, TOCItemDefinition>> entrySet = map.entrySet();
 		for (Entry<String, TOCItemDefinition> entry : entrySet) {
 			String key = entry.getKey();
@@ -124,7 +124,7 @@ public class LinkDatabase {
 	}
 
 	private void collectTOCItemExternals(TOCItemProvider tocProvider) {
-		Map<String, TOCItemExternal> map = tocProvider.getTOCItemExternalsByDisplayMapping();
+		Map<String, TOCItemExternal> map = tocProvider.getExternalTocItemsById();
 		for (TOCItemExternal tocItem : map.values()) {
 			mapOfIDsToTOCExternals.put(tocItem.getIDAttribute(), tocItem);
 		}

Ghidra/Framework/Help/src/main/java/help/validator/location/HelpModuleCollection.java

@@ -32,22 +32,22 @@ import help.TOCItemProvider;
 import help.validator.model.*;
 
 /**
- * A class that is meant to hold a single help <b>input</b> directory and 0 or more 
+ * A class that is meant to hold a single help <b>input</b> directory and 0 or more
  * <b>external, pre-built</b> help sources (i.e., jar file or directory).
  * <p>
  * <pre>
  * 						Note
  * 						Note
  * 						Note
- * 
+ *
  *  This class is a bit conceptually muddled.  Our build system is reflected in this class in that
- *  we currently build one help module at a time.  Thus, any dependencies of that module being 
+ *  we currently build one help module at a time.  Thus, any dependencies of that module being
  *  built can be passed into this "collection" at build time.   We used to build multiple help
- *  modules at once, resolving dependencies for all of the input modules after we built each 
+ *  modules at once, resolving dependencies for all of the input modules after we built each
  *  module.  This class will need to be tweaked in order to go back to a build system with
  *  multiple input builds.
- * 
+ *
- * </pre> 
+ * </pre>
  */
 public class HelpModuleCollection implements TOCItemProvider {
 
@@ -62,6 +62,8 @@ public class HelpModuleCollection implements TOCItemProvider {
 	/**
 	 * Creates a help module collection that contains only a singe help module from a help
 	 * directory, not a pre-built help jar.
+	 * @param dir the directory containing help
+	 * @return the help collection
 	 */
 	public static HelpModuleCollection fromHelpDirectory(File dir) {
 		return new HelpModuleCollection(toHelpLocations(Collections.singleton(dir)));
@@ -70,6 +72,8 @@ public class HelpModuleCollection implements TOCItemProvider {
 	/**
 	 * Creates a help module collection that assumes zero or more pre-built help jar files and
 	 * one help directory that is an input into the help building process.
+	 * @param files the files from which to get help
+	 * @return the help collection
 	 */
 	public static HelpModuleCollection fromFiles(Collection<File> files) {
 		return new HelpModuleCollection(toHelpLocations(files));
@@ -78,6 +82,8 @@ public class HelpModuleCollection implements TOCItemProvider {
 	/**
 	 * Creates a help module collection that assumes zero or more pre-built help jar files and
 	 * one help directory that is an input into the help building process.
+	 * @param locations the locations from which to get help
+	 * @return the help collection
 	 */
 	public static HelpModuleCollection fromHelpLocations(Collection<HelpModuleLocation> locations) {
 		return new HelpModuleCollection(locations);
@@ -240,7 +246,7 @@ public class HelpModuleCollection implements TOCItemProvider {
 	}
 
 	@Override
-	public Map<String, TOCItemDefinition> getTOCItemDefinitionsByIDMapping() {
+	public Map<String, TOCItemDefinition> getTocDefinitionsByID() {
 		Map<String, TOCItemDefinition> map = new HashMap<>();
 		GhidraTOCFile TOC = inputHelp.getSourceTOCFile();
 		map.putAll(TOC.getTOCDefinitionByIDMapping());
@@ -248,7 +254,7 @@ public class HelpModuleCollection implements TOCItemProvider {
 	}
 
 	@Override
-	public Map<String, TOCItemExternal> getTOCItemExternalsByDisplayMapping() {
+	public Map<String, TOCItemExternal> getExternalTocItemsById() {
 		Map<String, TOCItemExternal> map = new HashMap<>();
 
 		if (externalHelpSets.isEmpty()) {
@@ -282,18 +288,17 @@ public class HelpModuleCollection implements TOCItemProvider {
 			if (parent != null) {
 				CustomTreeItemDecorator dec = (CustomTreeItemDecorator) parent.getUserObject();
 				if (dec != null) {
-					parentItem = mapByDisplay.get(dec.getDisplayText());
+					parentItem = mapByDisplay.get(dec.getTocID());
 				}
 			}
 
 			ID targetID = item.getID();
-
 			String displayText = item.getDisplayText();
-			String tocID = item.getTocID();
+			String tocId = item.getTocID();
 			String target = targetID == null ? null : targetID.getIDString();
-			TOCItemExternal external = new TOCItemExternal(parentItem, tocPath, tocID, displayText,
+			TOCItemExternal external = new TOCItemExternal(parentItem, tocPath, tocId, displayText,
 				target, item.getName(), -1);
-			mapByDisplay.put(displayText, external);
+			mapByDisplay.put(tocId, external);
 		}
 
 		@SuppressWarnings("rawtypes")
@@ -304,7 +309,10 @@ public class HelpModuleCollection implements TOCItemProvider {
 		}
 	}
 
-	/** Input TOC items are those that we are building for the input help module of this collection */
+	/**
+	 * Input TOC items are those that we are building for the input help module of this collection
+	 * @return the items
+	 */
 	public Collection<TOCItem> getInputTOCItems() {
 		Collection<TOCItem> items = new ArrayList<>();
 		GhidraTOCFile TOC = inputHelp.getSourceTOCFile();

Ghidra/Framework/Help/src/test/java/help/OverlayHelpTreeTest.java

@@ -43,24 +43,24 @@ public class OverlayHelpTreeTest {
 		// in a help <TOCITEM> that lives inside of a pre-built jar file.
 		//
 		/*
-		
+
 		 	Example makeup we will create:
-		
+
 			  	PreBuild_TOC.xml
-		
+
 			  		<tocitem id="root" target="fake">
 			  			<tocitem id="child_1" target="fake" />
 			  		</tocitem>
-		
+
-		
+
 			 	TOC_Source.xml
-		
+
 			 		<tocref id="root">
 			 			<tocref="child_1">
 			 				<tocdef id="child_2" target="fake" />
 			 			</tocref>
 			 		</tocref>
-		
+
 		 */
 
 		TOCItemExternal root = externalItem("root");
@@ -92,26 +92,26 @@ public class OverlayHelpTreeTest {
 	public void testSourceTOCFileThatDependsAnotherTOCSourceFile() {
 
 		/*
-		
+
 		 The first source file defines attributes that the second file references.
-		
+
 		 Example makeup we will create:
-		
+
 		  	TOC_Source.xml
-		
+
 		  		<tocdef id="root" target="fake">
 		  			<tocdef id="child_1" target="fake" />
 		  		</tocdef>
-		
+
-		
+
 		 	Another TOC_Source.xml
-		
+
 		 		<tocref id="root">
 		 			<tocref="child_1">
 		 				<tocdef id="child_2" target="fake" />
 		 			</tocref>
 		 		</tocref>
-		
+
 		*/
 
 		Path toc_1 = Paths.get("/fake/path_1/TOC_Source.xml");
@@ -147,34 +147,34 @@ public class OverlayHelpTreeTest {
 		// in a help <TOCITEM> that lives inside of multiple pre-built jar files.
 		//
 		/*
-		
+
 		 	Example makeup we will create:
-		
+
 			  	PreBuild_TOC.xml
-		
+
 			  		<tocitem id="root" target="fake">
 			  			<tocitem id="child_1" target="fake">
 			  				<tocitem="prebuilt_a_child" target="fake" />
 			  			</tocitem>
 			  		</tocitem>
-		
+
 				Another PreBuild_TOC.xml
-		
+
 			  		<tocitem id="root" target="fake">
 			  			<tocitem id="child_1" target="fake">
 			  				<tocitem="prebuilt_b_child" target="fake" />
 			  			</tocitem>
 			  		</tocitem>
-		
+
-		
+
 			 	TOC_Source.xml
-		
+
 			 		<tocref id="root">
 			 			<tocref="child_1">
 			 				<tocdef id="child_2" target="fake" />
 			 			</tocref>
 			 		</tocref>
-		
+
 		 */
 
 		TOCItemExternal root_a = externalItem("root");
@@ -219,37 +219,37 @@ public class OverlayHelpTreeTest {
 	public void testSourceTOCFileThatHasNodeWithSameTextAttributeAsOneOfItsExternalModluleDependencies() {
 
 		/*
-		
+
 		 The first source file defines attributes that the second file references.   Both files
 		 will have multiple nodes that coincidentally share 'text' attribute values.
-		
+
 		 Note: the 'id' attributes have to be unique; the 'text' attributes do not have to be unique
-		
+
 		 Example makeup we will create:
-		
+
 		  	PreBuild_TOC.xml
-		
+
 		  		<tocitem id="root" target="fake">
 		  			<tocitem id="child_1_1" text="Child 1" target="fake" />
 		  		</tocitem>
-		
+
 			Another PreBuild_TOC.xml
-		
+
 		  		<tocitem id="root" target="fake">
 		  			<tocitem id="child_2_1" text=Child 1" target="fake" />
 		  			<tocitem id="child_2_2" text=Child 2" target="fake" />
 		  		</tocitem>
-		
+
-		
+
 		 	Another TOC_Source.xml
-		
+
 		 		<tocref id="root">
 		 			<tocref="child_1_1">
 		 				<tocdef id="child_2_1a" text="Child 1a" target="fake" />
 		 			</tocref>
 		 			<tocdef id="child_3_2" text="Child 2" target="fake" />
 		 		</tocref>
-		
+
 		*/
 
 		TOCItemExternal root_a = externalItem("root");
@@ -432,12 +432,12 @@ public class OverlayHelpTreeTest {
 		}
 
 		@Override
-		public Map<String, TOCItemExternal> getTOCItemExternalsByDisplayMapping() {
+		public Map<String, TOCItemExternal> getExternalTocItemsById() {
 			return externals;
 		}
 
 		@Override
-		public Map<String, TOCItemDefinition> getTOCItemDefinitionsByIDMapping() {
+		public Map<String, TOCItemDefinition> getTocDefinitionsByID() {
 			return definitions;
 		}