From: kruland2607 <kruland2607@180e2498-e6e9-4542-8430-84ac67f01cd8>
Date: Fri, 24 Aug 2012 20:31:13 +0000 (+0000)
Subject: Fix correctness and threading issue in new Manufacturer lookup mechanism.  It was... 
X-Git-Tag: upstream/12.09^2~24
X-Git-Url: https://git.gag.com/?p=debian%2Fopenrocket;a=commitdiff_plain;h=0e2104d1ddd05ac08568195d27ca6d3dbde25de9

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".

git-svn-id: https://openrocket.svn.sourceforge.net/svnroot/openrocket/trunk@1000 180e2498-e6e9-4542-8430-84ac67f01cd8
---

diff --git a/core/src/net/sf/openrocket/motor/Manufacturer.java b/core/src/net/sf/openrocket/motor/Manufacturer.java
index 142b3d13..5d562ed7 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 6870c5f2..cd0ce25e 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() {