hongshaorou/openrocket
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
Kevin Ruland 2012-08-24 20:31:13 +00:00
parent f1e528d11f
commit ab4c14d3a0
2 changed files with 29 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
core/src/net/sf/openrocket/motor/Manufacturer.java
View File

@ -18,7 +18,7 @@ public class Manufacturer {
private static class ManufacturerList extends ConcurrentHashMap<String,Manufacturer> { private static class ManufacturerList extends ConcurrentHashMap<String,Manufacturer> {
void add( Manufacturer m ) { void add( Manufacturer m ) {
for( String s : m.getAllNames() ) { for( String s : m.getSearchNames() ) {
Manufacturer previousRegistered; Manufacturer previousRegistered;
if ( (previousRegistered = putIfAbsent( s, m )) != null ) { if ( (previousRegistered = putIfAbsent( s, m )) != null ) {
throw new IllegalStateException("Manufacturer name clash between " + throw new IllegalStateException("Manufacturer name clash between " +
@ -183,6 +183,9 @@ public class Manufacturer {
return allNames; return allNames;
} }
Set<String> getSearchNames() {
return searchNames;
}
/** /**
* Return the motor type that this manufacturer produces if it produces only one motor type. * 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. * @return the Manufacturer object corresponding the name.
*/ */
public static Manufacturer getManufacturer(String name) { public static Manufacturer getManufacturer(String name) {
Manufacturer m = manufacturers.get(name); String searchString = generateSearchString(name);
Manufacturer m = manufacturers.get(searchString);
if ( m != null ) { if ( m != null ) {
return m; return m;
} }
m = new Manufacturer(name.trim(), name.trim(), Motor.Type.UNKNOWN); m = new Manufacturer(name.trim(), name.trim(), Motor.Type.UNKNOWN);
Manufacturer oldManu = manufacturers.putIfAbsent(name, m); // We need some additional external synchronization here so we lock on the manufacturers.
return (oldManu != null) ? oldManu : m; synchronized( manufacturers ) {
Manufacturer retest = manufacturers.get(searchString);
if ( retest != null ) {
// it exists now.
return retest;
}
manufacturers.add(m);
}
return m;
} }
private static String generateSearchString(String str) {
private String generateSearchString(String str) {
return str.toLowerCase(Locale.getDefault()).replaceAll("[^a-zA-Z0-9]+", " ").trim(); return str.toLowerCase(Locale.getDefault()).replaceAll("[^a-zA-Z0-9]+", " ").trim();
} }

12
core/test/net/sf/openrocket/motor/ManufacturerTest.java
View File

@ -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 @Test
public void testNew() { public void testNew() {