Use names instead of ordinal values for encoding of enumerations in XML.

This change fixes issue #4. Change-Id: I020637c66359c8ae609188619257f20f4ef9ac21
enilink · Mar 24, 2016 · e07fb2b · e07fb2b
1 parent 0d1640c
commit e07fb2b
Show file tree

Hide file tree

Showing 20 changed files with 329 additions and 115 deletions.
diff --git a/llrp4j-core/META-INF/.gitignore b/llrp4j-core/META-INF/.gitignore
@@ -0,0 +1 @@
+/MANIFEST.MF
diff --git a/llrp4j-core/pom.xml b/llrp4j-core/pom.xml
@@ -19,7 +19,6 @@
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>
 				<artifactId>maven-jar-plugin</artifactId>
-				<version>2.6</version>
 				<executions>
 					<execution>
 						<goals>
@@ -40,7 +39,7 @@
 							net.enilink.llrp4j.types,
 							net.enilink.llrp4j.xml,
 							org.llrp.ltk.schema.core</Export-Package>
-						<Import-Package>org.slf4j;version="1.7.2"</Import-Package>
+						<Import-Package>org.slf4j;version="1.7.2",*</Import-Package>
 					</instructions>
 				</configuration>
 			</plugin>

diff --git a/llrp4j-core/src/main/java/net/enilink/llrp4j/XmlDecoder.java b/llrp4j-core/src/main/java/net/enilink/llrp4j/XmlDecoder.java
@@ -1,6 +1,5 @@
 package net.enilink.llrp4j;
 
-import static net.enilink.llrp4j.EncodingUtil.decodeEnum;
 import static net.enilink.llrp4j.EncodingUtil.firstUpper;
 import static net.enilink.llrp4j.EncodingUtil.indent;
 import static net.enilink.llrp4j.EncodingUtil.parameterType;
@@ -9,6 +8,7 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
 import java.util.ArrayList;
 import java.util.List;
@@ -52,74 +52,109 @@ LlrpMessage parseMessage() throws Exception {
 
 		void parseProperties(Object o, Property[] properties) throws Exception {
 			QName name = next();
-
 			for (int i = 0; i < properties.length; i++) {
 				Property property = properties[i];
 
-				boolean parseProperty = false;
-				Class<?> elementClass = null;
+				if (logger.isDebugEnabled()) {
+					logger.debug(indent(depth, "decode " + property.field));
+					depth++;
+				}
+
+				boolean propertyWasRead = false;
 				if (property.isField) {
+					// this is a simple scalar field
 					String expectedName = firstUpper(property.field.getName());
 					if (name != null && name.getLocalPart().equals(expectedName)) {
-						parseProperty = true;
+						Object fieldValue = parseField(property.field, null);
+						if (property.required && fieldValue == null) {
+							throw new ParseException("Missing content in element " + name);
+						}
+						property.field.set(o, fieldValue);
+						propertyWasRead = true;
+					} else if (property.required) {
+						unexpected(name);
 					}
 				} else {
+					// this is a parameter object
+					boolean isList = List.class.isAssignableFrom(property.field.getType());
+					List<Object> valueList = null;
+					boolean required = property.required;
 					Class<?> expectedClass = propertyType(property.field);
-					elementClass = context.qnameToClass.get(name);
-					if (elementClass != null && expectedClass.isAssignableFrom(elementClass)) {
-						parseProperty = true;
-					}
-				}
-
-				if (parseProperty) {
-					if (logger.isDebugEnabled()) {
-						logger.debug(indent(depth, "decode " + property.field));
-						depth++;
-					}
 
-					Object fieldValue;
-					if (property.isField) {
-						fieldValue = parseField(property.field, null);
-						if (property.required && fieldValue == null) {
-							throw new ParseException("Missing content in element " + name);
+					Object fieldValue = null;
+					while (true) {
+						Class<?> elementClass = context.qnameToClass.get(name);
+						if (elementClass == null || !expectedClass.isAssignableFrom(elementClass)) {
+							// reset read flag in case of lists
+							propertyWasRead = false;
+							if (required) {
+								unexpected(name);
+							}
+							break;
 						}
-						property.field.set(o, fieldValue);
-					} else {
-						boolean isList = List.class.isAssignableFrom(property.field.getType());
-						fieldValue = parseParameter(elementClass, isList, property.required);
-						if (property.required && (fieldValue == null
+
+						propertyWasRead = true;
+						fieldValue = parseParameter(elementClass, isList, required);
+						if (required && (fieldValue == null
 								|| fieldValue instanceof List && ((List<?>) fieldValue).isEmpty())) {
 							unexpected(name);
 						}
-						if (isList && !(fieldValue instanceof List)) {
-							@SuppressWarnings("unchecked")
-							List<Object> list = (List<Object>) property.field.get(o);
-							if (list == null) {
-								list = new ArrayList<>();
-								property.field.set(o, list);
+						if (isList) {
+							if (valueList == null) {
+								valueList = new ArrayList<>();
+							}
+							if (fieldValue instanceof List) {
+								valueList.addAll((List<?>) fieldValue);
+							} else {
+								valueList.add(fieldValue);
 							}
-							list.add(fieldValue);
+							// at least one parameter value was already read
+							required = false;
+
+							end();
+							name = next();
 						} else {
-							property.field.set(o, fieldValue);
+							// read only one parameter
+							break;
 						}
 					}
-
-					if (logger.isDebugEnabled()) {
-						depth--;
-						logger.debug(indent(depth, "decoded " + property.field));
+					if (isList) {
+						fieldValue = valueList;
 					}
+					if (fieldValue != null) {
+						property.field.set(o, fieldValue);
+					}
+				}
 
+				if (logger.isDebugEnabled()) {
+					depth--;
+					logger.debug(indent(depth, "decoded " + property.field));
+				}
+
+				if (propertyWasRead) {
 					end();
 					name = next();
-				} else if (property.required) {
-					unexpected(name);
 				}
 			}
 			if (name != null) {
 				useCurrentAsNext();
 			}
 		}
 
+		private Object stringToEnum(Class<?> enumClass, boolean isList, String value) throws Exception {
+			Method valueOf = enumClass.getDeclaredMethod("valueOf", String.class);
+			if (isList) {
+				String[] elements = value.split("\\s*,\\s*");
+				List<Object> enumValues = new ArrayList<>(elements.length);
+				for (int i = 0; i < elements.length; i++) {
+					enumValues.add(valueOf.invoke(null, elements[i]));
+				}
+				return enumValues;
+			} else {
+				return valueOf.invoke(null, value);
+			}
+		}
+
 		private Object parseField(Field field, String value) throws Exception {
 			LlrpField annotation = field.getAnnotation(LlrpField.class);
 			FieldType type = annotation.type();
@@ -128,10 +163,12 @@ private Object parseField(Field field, String value) throws Exception {
 				value = parseStringValue();
 			}
 			if (value.length() > 0) {
-				Object javaValue = XmlTypes.fromString(type, annotation.format(), value);
 				Class<?> elementType = propertyType(field);
+				Object javaValue;
 				if (LlrpEnum.class.isAssignableFrom(elementType)) {
-					javaValue = decodeEnum(elementType, javaValue);
+					javaValue = stringToEnum(elementType, List.class.isAssignableFrom(field.getType()), value);
+				} else {
+					javaValue = XmlTypes.fromString(type, annotation.format(), value);
 				}
 				return javaValue;
 			}

diff --git a/llrp4j-core/src/main/java/net/enilink/llrp4j/XmlEncoder.java b/llrp4j-core/src/main/java/net/enilink/llrp4j/XmlEncoder.java
@@ -11,6 +11,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -132,12 +133,27 @@ private void encodeProperties(Object o, Property[] properties, XMLStreamWriter w
 		}
 	}
 
+	private String enumToString(Object value) {
+		if (value instanceof List<?>) {
+			StringBuilder sb = new StringBuilder();
+			for (Iterator<?> it = ((List<?>) value).iterator(); it.hasNext();) {
+				LlrpEnum element = (LlrpEnum) it.next();
+				sb.append(element.name());
+				if (it.hasNext()) {
+					sb.append(" ");
+				}
+			}
+			return sb.toString();
+		} else {
+			return ((LlrpEnum) value).name();
+		}
+	}
+
 	private String encodeField(Field field, Object value, String namespace, XMLStreamWriter writer) throws Exception {
 		LlrpField annotation = field.getAnnotation(LlrpField.class);
-		FieldType type = annotation.type();
 		if (value instanceof LlrpEnum
 				|| (value instanceof List<?> && LlrpEnum.class.isAssignableFrom(propertyType(field)))) {
-			value = encodeEnum(type, value);
+			value = enumToString(value);
 		}
 		String fieldName = firstUpper(field.getName());
 		String fieldValue = XmlTypes.toString(value, annotation.format());

diff --git a/llrp4j-core/src/main/java/net/enilink/llrp4j/types/LlrpEnum.java b/llrp4j-core/src/main/java/net/enilink/llrp4j/types/LlrpEnum.java
@@ -2,4 +2,6 @@
 
 public interface LlrpEnum {
 	public int value();
+
+	public String name();
 }
diff --git a/llrp4j-core/src/test/java/net/enilink/llrp4j/test/TestUtil.java b/llrp4j-core/src/test/java/net/enilink/llrp4j/test/TestUtil.java
@@ -74,46 +74,57 @@ public static Object createValue(Field f, Set<Class<?>> classesPool, Random rnd)
 			}
 		}
 
-		Object value = null;
-		if (c.isEnum()) {
-			Object[] constants = c.getEnumConstants();
-			value = constants[rnd.nextInt(constants.length)];
-		} else if (c.isInterface()) {
-			for (Class<?> candidate : classesPool) {
-				if (c.isAssignableFrom(candidate)) {
-					value = candidate.newInstance();
-					break;
+		int count = 1;
+		if (list != null) {
+			// add more than one element to list
+			count = 1 + rnd.nextInt(5);
+		}
+
+		while (true) {
+			Object value = null;
+			if (c.isEnum()) {
+				Object[] constants = c.getEnumConstants();
+				value = constants[rnd.nextInt(constants.length)];
+			} else if (c.isInterface()) {
+				for (Class<?> candidate : classesPool) {
+					if (c.isAssignableFrom(candidate)) {
+						value = candidate.newInstance();
+						break;
+					}
 				}
-			}
-		} else if (BitList.class.equals(c)) {
-			byte[] bytes = new byte[rnd.nextInt(5)];
-			rnd.nextBytes(bytes);
-			value = new BitList(bytes);
-		} else if (BigInteger.class.equals(c)) {
-			value = BigInteger.valueOf(rnd.nextLong());
-		} else if (c.isArray()) {
-			if (byte[].class.equals(c)) {
-				byte[] bytes = new byte[rnd.nextInt(5) + 1];
+			} else if (BitList.class.equals(c)) {
+				byte[] bytes = new byte[rnd.nextInt(5)];
 				rnd.nextBytes(bytes);
-				value = bytes;
+				value = new BitList(bytes);
+			} else if (BigInteger.class.equals(c)) {
+				value = BigInteger.valueOf(rnd.nextLong());
+			} else if (c.isArray()) {
+				if (byte[].class.equals(c)) {
+					byte[] bytes = new byte[rnd.nextInt(5) + 1];
+					rnd.nextBytes(bytes);
+					value = bytes;
+				} else {
+					value = Array.newInstance(c.getComponentType(), 0);
+				}
 			} else {
-				value = Array.newInstance(c.getComponentType(), 0);
+				value = c.newInstance();
+			}
+			if (value == null) {
+				throw new IllegalArgumentException("Cannot instantiate class: " + c);
+			}
+			if (!value.getClass().isArray()
+					&& !(value instanceof Number || value instanceof String || value instanceof Boolean)) {
+				mockObject(value, classesPool, rnd);
+			}
+			if (list != null) {
+				list.add(value);
+				if (count-- == 0) {
+					return list;
+				}
+			} else {
+				return value;
 			}
-		} else {
-			value = c.newInstance();
-		}
-		if (value == null) {
-			throw new IllegalArgumentException("Cannot instantiate class: " + c);
-		}
-		if (!value.getClass().isArray()
-				&& !(value instanceof Number || value instanceof String || value instanceof Boolean)) {
-			mockObject(value, classesPool, rnd);
-		}
-		if (list != null) {
-			list.add(value);
-			return list;
 		}
-		return value;
 	}
 
 	public static Class<?> responseType(Class<?> msgClass) {

diff --git a/llrp4j-generator/META-INF/.gitignore b/llrp4j-generator/META-INF/.gitignore
@@ -0,0 +1 @@
+/MANIFEST.MF
diff --git a/llrp4j-generator/META-INF/MANIFEST.MF b/llrp4j-generator/META-INF/MANIFEST.MF
diff --git a/llrp4j-generator/pom.xml b/llrp4j-generator/pom.xml
@@ -42,10 +42,7 @@
 				<configuration>
 					<instructions>
 						<Export-Package>net.enilink.llrp4j.generator</Export-Package>
-						<Import-Package>net.enilink.llrp4j,
-							net.enilink.llrp4j.annotations,
-							net.enilink.llrp4j.types,
-							org.slf4j;version="1.7.2"</Import-Package>
+						<Import-Package>org.slf4j;version="1.7.2", *</Import-Package>
 					</instructions>
 				</configuration>
 			</plugin>

diff --git a/llrp4j-impinj-module/META-INF/.gitignore b/llrp4j-impinj-module/META-INF/.gitignore
@@ -0,0 +1 @@
+/MANIFEST.MF
diff --git a/llrp4j-impinj-module/pom.xml b/llrp4j-impinj-module/pom.xml
@@ -86,14 +86,7 @@
 				<configuration>
 					<instructions>
 						<Export-Package>org.llrp.impinj.*</Export-Package>
-						<Import-Package>net.enilink.llrp4j,
-							net.enilink.llrp4j.annotations,
-							net.enilink.llrp4j.types,
-							org.llrp.enumerations,
-							org.llrp.interfaces,
-							org.llrp.ltk.schema.core,
-							org.llrp.messages,
-							org.llrp.parameters</Import-Package>
+						<Import-Package>*</Import-Package>
 					</instructions>
 				</configuration>
 			</plugin>

diff --git a/llrp4j-llrp-module/.classpath b/llrp4j-llrp-module/.classpath
@@ -17,6 +17,11 @@
 			<attribute name="maven.pomderived" value="true"/>
 		</attributes>
 	</classpathentry>
+	<classpathentry excluding="**" kind="src" output="target/test-classes" path="src/test/resources">
+		<attributes>
+			<attribute name="maven.pomderived" value="true"/>
+		</attributes>
+	</classpathentry>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.7">
 		<attributes>
 			<attribute name="maven.pomderived" value="true"/>

diff --git a/llrp4j-llrp-module/META-INF/.gitignore b/llrp4j-llrp-module/META-INF/.gitignore
@@ -0,0 +1 @@
+/MANIFEST.MF