From ab4c14d3a03da8431ddb7702c36864fbabe6c8cb Mon Sep 17 00:00:00 2001
From: Kevin Ruland <kruland@sunflower.com>
Date: Fri, 24 Aug 2012 20:31:13 +0000
Subject: [PATCH] Fix correctness and threading issue in new Manufacturer
 lookup mechanism.  It was incorrect because the Manufacturer objects should
 be registered under the searchNames.  The threading correctness is only an
 issue in the Manufacturer.get(String) method when the name does not exist. 
 This is handled by using external locking mechanism.  Added a unit test which
 exposed the problem looking for the manufacturer "Contrail_Rockets".

---
 .../net/sf/openrocket/motor/Manufacturer.java | 25 +++++++++++++------
 .../sf/openrocket/motor/ManufacturerTest.java | 12 +++++++++
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/core/src/net/sf/openrocket/motor/Manufacturer.java b/core/src/net/sf/openrocket/motor/Manufacturer.java
index 142b3d13c..5d562ed7c 100644
--- a/core/src/net/sf/openrocket/motor/Manufacturer.java
+++ b/core/src/net/sf/openrocket/motor/Manufacturer.java
@@ -18,7 +18,7 @@ public class Manufacturer {
 	private static class ManufacturerList extends ConcurrentHashMap<String,Manufacturer> {
 		
 		void add( Manufacturer m ) {
-			for( String s : m.getAllNames() ) {
+			for( String s : m.getSearchNames() ) {
 				Manufacturer previousRegistered;
 				if ( (previousRegistered = putIfAbsent( s, m )) != null ) {
 					throw new IllegalStateException("Manufacturer name clash between " +
@@ -183,6 +183,9 @@ public class Manufacturer {
 		return allNames;
 	}
 	
+	Set<String> getSearchNames() {
+		return searchNames;
+	}
 	
 	/**
 	 * Return the motor type that this manufacturer produces if it produces only one motor type.
@@ -231,21 +234,27 @@ public class Manufacturer {
 	 * @return		the Manufacturer object corresponding the name.
 	 */
 	public static Manufacturer getManufacturer(String name) {
-		Manufacturer m = manufacturers.get(name);
+		String searchString = generateSearchString(name);
+		Manufacturer m = manufacturers.get(searchString);
 		if ( m != null ) {
 			return m;
 		}
 
 		m = new Manufacturer(name.trim(), name.trim(), Motor.Type.UNKNOWN);
 
-		Manufacturer oldManu = manufacturers.putIfAbsent(name, m);
-		return  (oldManu != null) ? oldManu : m;
+		// We need some additional external synchronization here so we lock on the manufacturers.
+		synchronized( manufacturers ) {
+			Manufacturer retest = manufacturers.get(searchString);
+			if ( retest != null ) {
+				// it exists now.
+				return retest;
+			}
+			manufacturers.add(m);
+		}
+		return m;
 	}
 	
-	
-	
-	
-	private String generateSearchString(String str) {
+	private static String generateSearchString(String str) {
 		return str.toLowerCase(Locale.getDefault()).replaceAll("[^a-zA-Z0-9]+", " ").trim();
 	}
 	
diff --git a/core/test/net/sf/openrocket/motor/ManufacturerTest.java b/core/test/net/sf/openrocket/motor/ManufacturerTest.java
index 6870c5f2c..cd0ce25ef 100644
--- a/core/test/net/sf/openrocket/motor/ManufacturerTest.java
+++ b/core/test/net/sf/openrocket/motor/ManufacturerTest.java
@@ -24,6 +24,18 @@ public class ManufacturerTest {
 		
 	}
 	
+	public void testContrail() {
+		Manufacturer c1, c2;
+		
+		c1 = Manufacturer.getManufacturer("Contrail" );
+		
+		// Used in rsp files.
+		c2 = Manufacturer.getManufacturer("Contrail_Rockets");
+		
+		assertNotNull(c1);
+		assertEquals(c1,c2);
+	}
+	
 	@Test
 	public void testNew() {