Additional changes to support new Java AMQP version code, some C++ tidy-up as well.

git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@489704 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 105 insertions, 70 deletions

diff --git a/gentools/src/org/apache/qpid/gentools/AmqpFieldMap.java b/gentools/src/org/apache/qpid/gentools/AmqpFieldMap.java
index e9f6ee5093..5053fba2aa 100644
--- a/gentools/src/org/apache/qpid/gentools/AmqpFieldMap.java
+++ b/gentools/src/org/apache/qpid/gentools/AmqpFieldMap.java
@@ -49,12 +49,21 @@ public class AmqpFieldMap extends TreeMap<String, AmqpField> implements VersionC
 		LanguageConverter converter)
 		throws AmqpTypeMappingException
 	{
+		// TODO: REVIEW THIS! There may be a bug here that affects C++ generation (only with >1 version)...
+		// If version == null (a common scenario) then the version map is built up on the
+		// basis of first found item, and ignores other version variations.
+		// This should probably be disallowed by throwing an NPE, as AmqpOrdinalFieldMap cannot
+		// represent these possibilities.
+		// *OR*
+		// Change the structure of AmqpOrdianlFieldMap to allow for the various combinations that
+		// will result from version variation - but that is what AmqpFieldMap is... :-$
 		AmqpOrdinalFieldMap ordinalFieldMap = new AmqpOrdinalFieldMap();
 		for (String thisFieldName: keySet())
 		{
 			AmqpField field = get(thisFieldName);
 			if (version == null || field.versionSet.contains(version))
 			{
+				// 1. Search for domain name in field domain map with version that matches
 				String domain = "";
 				boolean dFound = false;
 				for (String thisDomainName : field.domainMap.keySet())
@@ -69,6 +78,7 @@ public class AmqpFieldMap extends TreeMap<String, AmqpField> implements VersionC
 					}
 				}
 				
+				// 2. Search for ordinal in field ordianl map with version that matches
 				int ordinal = -1;
 				boolean oFound = false;
 				for (Integer thisOrdinal : field.ordinalMap.keySet())
diff --git a/gentools/src/org/apache/qpid/gentools/CppGenerator.java b/gentools/src/org/apache/qpid/gentools/CppGenerator.java
index 1e33e961e5..f1e7063800 100644
--- a/gentools/src/org/apache/qpid/gentools/CppGenerator.java
+++ b/gentools/src/org/apache/qpid/gentools/CppGenerator.java
@@ -31,8 +31,10 @@ public class CppGenerator extends Generator
 {
 	protected static final String versionNamespaceStartToken = "${version_namespace_start}";
 	protected static final String versionNamespaceEndToken = "${version_namespace_end}";
+	
+	// TODO: Move this to parent class
 	protected static final int FIELD_NAME = 0;
-	protected static final int FIELD_DOMAIN = 1;
+	protected static final int FIELD_CODE_TYPE = 1;
     
     /**
      * A complete list of C++ reserved words. The names of varous XML elements within the AMQP
@@ -1135,7 +1137,7 @@ public class CppGenerator extends Generator
         for (Integer thisOrdinal : ordinalFieldMap.keySet())
         {
             String[] fieldDomainPair = ordinalFieldMap.get(thisOrdinal);
-            sb.append(indent + fieldDomainPair[FIELD_DOMAIN] + " " + fieldDomainPair[FIELD_NAME] + ";" + cr);        	
+            sb.append(indent + fieldDomainPair[FIELD_CODE_TYPE] + " " + fieldDomainPair[FIELD_NAME] + ";" + cr);        	
         }
 		return sb.toString();
 	}
@@ -1152,7 +1154,7 @@ public class CppGenerator extends Generator
         for (Integer thisOrdinal : ordinalFieldMap.keySet())
         {
             String[] fieldDomainPair = ordinalFieldMap.get(thisOrdinal);
-			sb.append(indent + "inline " + setRef(fieldDomainPair[FIELD_DOMAIN]) + " get" +
+			sb.append(indent + "inline " + setRef(fieldDomainPair[FIELD_CODE_TYPE]) + " get" +
 				Utils.firstUpper(fieldDomainPair[FIELD_NAME]) + "() { return " +
 				fieldDomainPair[FIELD_NAME] + "; }" + cr);
         }
@@ -1172,7 +1174,7 @@ public class CppGenerator extends Generator
         for (Integer thisOrdinal : ordinalFieldMap.keySet())
         {
             String[] fieldDomainPair = ordinalFieldMap.get(thisOrdinal);
-            String cast = fieldDomainPair[FIELD_DOMAIN].compareTo("u_int8_t") == 0 ? "(int)" : "";
+            String cast = fieldDomainPair[FIELD_CODE_TYPE].compareTo("u_int8_t") == 0 ? "(int)" : "";
             sb.append(indent + "out << \"");
             if (!firstFlag)
                 sb.append("; ");
@@ -1197,7 +1199,7 @@ public class CppGenerator extends Generator
 			ordinal = oItr.next();
 			String[] fieldDomainPair = ordinalFieldMap.get(ordinal);
 			AmqpVersion thisVersion = version == null ? globalVersionSet.first() : version;
-			String domainType = getDomainType(fieldDomainPair[FIELD_DOMAIN], thisVersion);
+			String domainType = getDomainType(fieldDomainPair[FIELD_CODE_TYPE], thisVersion);
 			
 			// Defer bit types by adding them to an array. When the first subsequent non-bit
 			// type is encountered, then handle the bits. This allows consecutive bits to be
@@ -1256,7 +1258,7 @@ public class CppGenerator extends Generator
 			ordinal = oItr.next();
 			String[] fieldDomainPair = ordinalFieldMap.get(ordinal);
 			AmqpVersion thisVersion = version == null ? globalVersionSet.first() : version;
-			String domainType = getDomainType(fieldDomainPair[FIELD_DOMAIN], thisVersion);
+			String domainType = getDomainType(fieldDomainPair[FIELD_CODE_TYPE], thisVersion);
 			
 			// Defer bit types by adding them to an array. When the first subsequent non-bit
 			// type is encountered, then handle the bits. This allows consecutive bits to be
@@ -1324,7 +1326,7 @@ public class CppGenerator extends Generator
 			ordinal = oItr.next();
 			String[] fieldDomainPair = ordinalFieldMap.get(ordinal);
 			AmqpVersion thisVersion = version == null ? globalVersionSet.first() : version;
-			String domainType = getDomainType(fieldDomainPair[FIELD_DOMAIN], thisVersion);
+			String domainType = getDomainType(fieldDomainPair[FIELD_CODE_TYPE], thisVersion);
 			
 			// Defer bit types by adding them to an array. When the first subsequent non-bit
 			// type is encountered, then handle the bits. This allows consecutive bits to be
@@ -1388,7 +1390,7 @@ public class CppGenerator extends Generator
 		{
 			int ordinal = oItr.next();
 			String[] fieldDomainPair = ordinalFieldMap.get(ordinal);
-			sb.append(indent + (defineFlag ? setRef(fieldDomainPair[FIELD_DOMAIN]) + " " : "") +
+			sb.append(indent + (defineFlag ? setRef(fieldDomainPair[FIELD_CODE_TYPE]) + " " : "") +
 				fieldDomainPair[FIELD_NAME] + (initializerFlag ? "(" + fieldDomainPair[FIELD_NAME] + ")" : "") +
 				(oItr.hasNext() ? "," : "") + cr);
 		}
@@ -1415,7 +1417,7 @@ public class CppGenerator extends Generator
 				sb.append(indent);
 			}
 			sb.append(
-                (fieldTypeFlag ? setRef(field[FIELD_DOMAIN]) : "") +
+                (fieldTypeFlag ? setRef(field[FIELD_CODE_TYPE]) : "") +
 				(fieldNameFlag ? " " + field[FIELD_NAME] : "") +
 				(pItr.hasNext() ? "," + (fieldNameFlag ? cr : " ") : ""));
 			first = false;
diff --git a/gentools/src/org/apache/qpid/gentools/JavaGenerator.java b/gentools/src/org/apache/qpid/gentools/JavaGenerator.java
index 1067e385c4..36b5f2f6d1 100644
--- a/gentools/src/org/apache/qpid/gentools/JavaGenerator.java
+++ b/gentools/src/org/apache/qpid/gentools/JavaGenerator.java
@@ -30,6 +30,10 @@ import java.util.TreeMap;
 
 public class JavaGenerator extends Generator
 {
+	// TODO: Move this to parent class
+	protected static final int FIELD_NAME = 0;
+	protected static final int FIELD_CODE_TYPE = 1;
+	
 	private class DomainInfo
 	{
 		public String type;
@@ -301,15 +305,15 @@ public class JavaGenerator extends Generator
 			"buffer.putLong(#)", 							// encode expression
 			"# = buffer.getLong()")); 						// decode expression
 		typeMap.put("longstr", new DomainInfo(
-			"String",										// Java code type
-			"EncodingUtils.encodedLongStringLength(#)", 	// size
+			"byte[]",										// Java code type
+			"EncodingUtils.encodedLongstrLength(#)", 		// size
 			"EncodingUtils.writeLongStringBytes(buffer, #)", // encode expression
-			"# = EncodingUtils.readLongString(buffer)"));	// decode expression
+			"# = EncodingUtils.readLongstr(buffer)"));		// decode expression
 		typeMap.put("octet", new DomainInfo(
-			"char",											// Java code type
+			"short",										// Java code type
 			"1",											// size
-			"buffer.putChar(#)",								// encode expression
-			"# = buffer.getChar()")); 							// decode expression
+			"EncodingUtils.writeUnsignedByte(buffer, #)",	// encode expression
+			"# = buffer.getUnsigned()")); 					// decode expression
 		typeMap.put("short", new DomainInfo(
 			"int",											// Java code type
 			"2",											// size
@@ -428,20 +432,20 @@ public class JavaGenerator extends Generator
 	}
 	
 	protected void processTemplate(StringBuffer sb, AmqpClass thisClass, AmqpMethod method,
-			AmqpField field, String templateFileName, AmqpVersion version)
-			throws InvocationTargetException, IllegalAccessException, AmqpTypeMappingException
+		AmqpField field, String templateFileName, AmqpVersion version)
+		throws InvocationTargetException, IllegalAccessException, AmqpTypeMappingException
+	{
+		try { processAllLists(sb, thisClass, method, version); }
+		catch (AmqpTemplateException e)
 		{
-			try { processAllLists(sb, thisClass, method, version); }
-			catch (AmqpTemplateException e)
-			{
-				System.out.println("WARNING: " + templateFileName + ": " + e.getMessage());
-			}
-			try { processAllTokens(sb, thisClass, method, field, version); }
-			catch (AmqpTemplateException e)
-			{
-				System.out.println("WARNING: " + templateFileName + ": " + e.getMessage());
-			}
+			System.out.println("WARNING: " + templateFileName + ": " + e.getMessage());
 		}
+		try { processAllTokens(sb, thisClass, method, field, version); }
+		catch (AmqpTemplateException e)
+		{
+			System.out.println("WARNING: " + templateFileName + ": " + e.getMessage());
+		}
+	}
 
 	@Override
 	protected String processToken(String token, AmqpClass thisClass, AmqpMethod method, AmqpField field,
@@ -562,8 +566,10 @@ public class JavaGenerator extends Generator
 		}
 		else if (token.compareTo("${mb_field_parameter_list}") == 0)
 		{
-			// This is klunky... (cringe) TODO: Find a more elegant solution here sometime...
+			// <cringe> The code generated by this is ugly... It puts a comma on a line by itself!
+			// TODO: Find a more elegant solution here sometime...
 			codeSnippet = fieldMap.size() > 0 ? Utils.createSpaces(42) + "," + cr : "";
+			// </cringe>
 			codeSnippet += fieldMap.parseFieldMap(mbParamListGenerateMethod,
 				mbMangledParamListGenerateMethod, 42, 4, this);
 		}
@@ -731,51 +737,25 @@ public class JavaGenerator extends Generator
 		while (cItr.hasNext())
 		{
 			AmqpClass thisClass = model.classMap.get(cItr.next());
-			AmqpVersionSet firstVersionSet = thisClass.indexMap.get(thisClass.indexMap.firstKey());
-			boolean classVersionConsistentFlag = firstVersionSet.equals(globalVersionSet);
 			Iterator<String> mItr = thisClass.methodMap.keySet().iterator();
 			while (mItr.hasNext())
 			{
 				AmqpMethod method = thisClass.methodMap.get(mItr.next());
-				firstVersionSet = method.indexMap.get(method.indexMap.firstKey());
-				boolean methodVersionConsistentFlag = firstVersionSet.equals(globalVersionSet);
-				if (classVersionConsistentFlag && methodVersionConsistentFlag)
+				for (AmqpVersion version : globalVersionSet)
 				{
-					// Both class and method with consistent indeces for all known versions
-					int classIndex = thisClass.indexMap.firstKey();
-					int methodIndex = method.indexMap.firstKey();
-					sb.append(indent + "map.put(" + classIndex + "*1000 + " + methodIndex +
-						", " + thisClass.name + Utils.firstUpper(method.name) + "Body.class);" + cr);
-				}
-				else
-				{
-					// Non-consistent indeces for all known versions - version-specific code required
-					sb.append(cr);
-					
-					Iterator<AmqpVersion> vItr = globalVersionSet.iterator();
-					while (vItr.hasNext())
+					// Find class and method index for this version (if it exists)
+					try
 					{
-						boolean first = true;
-						AmqpVersion version = vItr.next();
-						
-						// Find class and method index for this version (if it exists)
-						try
-						{
-							int classIndex = findIndex(thisClass.indexMap, version);
-							int methodIndex = findIndex(method.indexMap, version);
-							sb.append(indent);
-							if (!first)
-								sb.append("else ");
-							sb.append("if ( major == " + version.getMajor() + " && minor == " +
-								version.getMinor() + " )" + cr);
-							sb.append(indent + tab + "map.put(" + classIndex + "*1000 + " +
-								methodIndex + ", " + thisClass.name + Utils.firstUpper(method.name) +
-								"Body.class);" + cr);
-							first = false;
-						}
-						catch (Exception e) {} // Ignore
+						int classIndex = findIndex(thisClass.indexMap, version);
+						int methodIndex = findIndex(method.indexMap, version);
+						sb.append(indent + "classIDMethodIDVersionBodyMap.put(" + cr);
+						sb.append(indent + tab + "createMapKey((short)" + classIndex +
+							", (short)" + methodIndex + ", (byte)" + version.getMajor() +
+							", (byte)" + version.getMinor() + "), " + cr);
+						sb.append(indent + tab + Utils.firstUpper(thisClass.name) +
+							Utils.firstUpper(method.name) + "Body.class);" + cr);
 					}
-					sb.append(cr);
+					catch (Exception e) {} // Ignore
 				}
 			}
 		}
diff --git a/gentools/templ.java/MethodBodyClass.tmpl b/gentools/templ.java/MethodBodyClass.tmpl
index 8ac6c4e00a..ad1ce36006 100644
--- a/gentools/templ.java/MethodBodyClass.tmpl
+++ b/gentools/templ.java/MethodBodyClass.tmpl
@@ -54,6 +54,8 @@ ${METHOD_ID_INIT}
 
     public int getClazz() { return classIdMap.get(major + "-" + minor); }
     public int getMethod() { return methodIdMap.get(major + "-" + minor); }
+    public static int getClazz(byte major, byte minor) { return classIdMap.get(major + "-" + minor); }
+    public static int getMethod(byte major, byte minor) { return methodIdMap.get(major + "-" + minor); }
     
     // Field methods		
 %{FLIST}    ${mb_field_get_method}
diff --git a/gentools/templ.java/MethodRegistryClass.tmpl b/gentools/templ.java/MethodRegistryClass.tmpl
index 4fb40055a5..0f15918f90 100644
--- a/gentools/templ.java/MethodRegistryClass.tmpl
+++ b/gentools/templ.java/MethodRegistryClass.tmpl
@@ -28,12 +28,53 @@
  
 package org.apache.qpid.framing;
 
-import java.util.Map;
+import java.util.HashMap;
+import java.lang.reflect.Constructor;
+import org.apache.log4j.Logger;
 
 class MainRegistry
 {
-    static void register(Map map, byte major, byte minor)
+    private static final Logger _log = Logger.getLogger(MainRegistry.class);
+	private static HashMap<Long, Class> classIDMethodIDVersionBodyMap = new HashMap<Long, Class>();
+	
+    static
     {
 %{CLIST}	${reg_map_put_method}
     }
+    
+    public static AMQMethodBody get(short classID, short methodID, byte major, byte minor)
+        throws AMQFrameDecodingException
+    {
+		Class bodyClass = classIDMethodIDVersionBodyMap.get(
+		    createMapKey(classID, methodID, major, minor));
+		if (bodyClass == null)
+		{
+	    	throw new AMQFrameDecodingException(_log,
+	    	    "Unable to find a suitable decoder for class " + classID + " and method " +
+	    	    methodID + " in AMQP version " + major + "-" + minor + ".");
+	    }
+	    try
+	    {
+	    	Constructor initFn = bodyClass.getConstructor(byte.class, byte.class);
+	        return (AMQMethodBody) initFn.newInstance(major, minor);
+	    }
+	    catch (Exception e)
+	    {
+	    	throw new AMQFrameDecodingException(_log,
+	    	    "Unable to instantiate body class for class " + classID + " and method " +
+	    	    methodID + " in AMQP version " + major + "-" + minor + " : " + e, e);
+	    }
+    }
+    
+    private static Long createMapKey(short classID, short methodID, byte major, byte minor)
+    {
+    	/**
+         *	Mapping of 4 components into a guaranteed unique key:
+         *  MSB                                     LSB
+         *  +----+----+----+----+----+----+-----+-----+
+         *  |    0    | classID |methodID |major|minor|
+         *  +----+----+----+----+----+----+-----+-----+
+         */
+    	return new Long(((long)classID << 32) + ((long)methodID << 16) + ((long)major << 8) + minor);
+    }
 }
diff --git a/gentools/templ.java/PropertyContentHeaderClass.tmpl b/gentools/templ.java/PropertyContentHeaderClass.tmpl
index 6cc2e55a33..3c147cf6b6 100644
--- a/gentools/templ.java/PropertyContentHeaderClass.tmpl
+++ b/gentools/templ.java/PropertyContentHeaderClass.tmpl
@@ -1,4 +1,4 @@
-&{${CLASS}PropertyContentHeader.java}
+&{${CLASS}ContentHeaderProperties.java}
 /*
  *
  * Licensed to the Apache Software Foundation (ASF) under one
@@ -31,7 +31,7 @@ package org.apache.qpid.framing;
 import org.apache.log4j.Logger;
 import org.apache.mina.common.ByteBuffer;
 
-class ${CLASS}ContentHeaderProperties implements ContentHeaderProperties
+public class ${CLASS}ContentHeaderProperties implements ContentHeaderProperties
 {
     private static final Logger logger = Logger.getLogger(BasicContentHeaderProperties.class);