Hello Juan Hernandez,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/23624
to review the following change.
Change subject: restapi: Don't use method names to infer types
......................................................................
restapi: Don't use method names to infer types
Currently when we randomly populate objects in order to run tests, we
infer the types of objects contained in lists from the names of the
methods. For example, lets say that we have an object that has a
"getNics()" method that returns a list of "Nics" objects. We use the
word after the "get" prefix to infer the type. In this case we infer
that the type will be "Nics". But doing this has the dissadvantage that
we force ourselves to use method names (and thus tag names in the XML
representation) that match the type names. There was no reasonable
alternative in Java 6, but in Java 7 we can obtain the type of the
elements of the list directly. This is what we do in this patch.
Change-Id: Iadc9827c3885d7b9e39611b006a1b02eefa5c9f0
Signed-off-by: Juan Hernandez <[email protected]>
---
M
backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/MappingTestHelper.java
1 file changed, 3 insertions(+), 64 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/24/23624/1
diff --git
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/MappingTestHelper.java
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/MappingTestHelper.java
index 761c3e4..08ae5c8 100644
---
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/MappingTestHelper.java
+++
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/MappingTestHelper.java
@@ -1,6 +1,7 @@
package org.ovirt.engine.api.restapi.types;
import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.GregorianCalendar;
@@ -15,13 +16,6 @@
public class MappingTestHelper {
- private static final String USAGES = "Usages";
- private static final String SLAVE_SINGLE = "HostNIC";
- private static final String SLAVES_PLURAL = "Slaves";
- private static final String FLOPPY_SINGLE = "Floppy";
- private static final String FLOPPIES_PLURAL = "Floppies";
- private static final String NIC_SINGLE = "NIC";
- private static final String NICS_PLURAL = "Nics";
private static final String SET_ROOT = "set";
private static final String GET_ROOT = "get";
@@ -141,10 +135,8 @@
@SuppressWarnings("unchecked")
private static void fill(Method method, Object model, List<Class<?>> seen,
int level) throws Exception {
- // List<T> type parameter removed by erasure, hence we attempt to
- // infer from method name
- String elementType = method.getName().substring(GET_ROOT.length());
- Class<?> childType = getChildType(model, elementType);
+ ParameterizedType returnType = (ParameterizedType)
method.getGenericReturnType();
+ Class<?> childType = (Class<?>) returnType.getActualTypeArguments()[0];
if (level == 1 || unseen(childType, seen)) {
List<Object> list = (List<Object>) method.invoke(model);
Object child = null;
@@ -205,59 +197,6 @@
private static boolean returnsList(Method m) {
return List.class.equals(m.getReturnType());
- }
-
- private static Class<?> getChildType(Object model, String elementType)
throws ClassNotFoundException {
- if (isSpecialType(elementType)) {
- return handleSpecialType(elementType);
- } else {
- return coPackaged(model, elementType);
- }
- }
-
- private static Class<?> coPackaged(Object model, String elementType)
throws ClassNotFoundException {
- String packageRoot = model.getClass().getPackage().getName() + ".";
- try {
- return Class.forName(packageRoot + singular(elementType));
- } catch (ClassNotFoundException cnf) {
- try {
- return Class.forName(packageRoot + elementType);
- } catch (ClassNotFoundException cnfe) {
- // try inner class
- return Class.forName(model.getClass().getName() + "$" +
elementType);
- }
- }
- }
-
- private static boolean isSpecialType(String elementType) {
- // Right now there's only one special case ('Usages'), but this
- // was assigned a method for prospective future special cases.
- return elementType.equals(USAGES);
- }
-
- private static Class<String> handleSpecialType(String elementType) {
- // Consider special case: "Usages" contains a list of Strings, not
'Usage' elements.
- if (elementType.equals(USAGES)) {
- return String.class;
- } // else... other special cases in the future.
- return String.class; // default which should never be reached.
- }
-
- private static String singular(String s) {
- return isSingularSpecialCase(s) ? handleSingularSpecialCase(s) :
- s.endsWith("s") ? s.substring(0, s.length() - 1) : s;
- }
-
- private static boolean isSingularSpecialCase(String s) {
- return s.equals(NICS_PLURAL) || s.equals(FLOPPIES_PLURAL) ||
s.equals(SLAVES_PLURAL);
- }
-
- private static String handleSingularSpecialCase(String s) {
- // 'Nics' is plural of 'NIC' (uppercase)
- // 'Floppies' is plural of 'Floppy' (not 'Floppie')
- // 'Slaves' is plural of 'HostNIC' (we don't have a 'Slave' entity)
- return s.equals(NICS_PLURAL) ? NIC_SINGLE : s.equals(FLOPPIES_PLURAL)
? FLOPPY_SINGLE
- : s.equals(SLAVES_PLURAL) ? SLAVE_SINGLE : s;
}
private static boolean unseen(Class<?> type, List<Class<?>> seen) {
--
To view, visit http://gerrit.ovirt.org/23624
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadc9827c3885d7b9e39611b006a1b02eefa5c9f0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
