Model.java BIG issue in loadFromCursor

[BlackHornet]

I'm not sure if this is a compilation error or whatever, but when checking the decompiled loadFromCursor there is a HUGE mismatch between code and compilation.

this is java source:

public final void loadFromCursor(Cursor cursor) {
    /**
     * Obtain the columns ordered to fix issue #106 (https://github.com/pardom/ActiveAndroid/issues/106)
     * when the cursor have multiple columns with same name obtained from join tables.
     */
    List<String> columnsOrdered = new ArrayList<String>(Arrays.asList(cursor.getColumnNames()));
    for (Field field : mTableInfo.getFields()) {
        final String fieldName = mTableInfo.getColumnName(field);
        Class<?> fieldType = field.getType();
        final int columnIndex = columnsOrdered.indexOf(fieldName);

        if (columnIndex < 0) {
            continue;
        }

        field.setAccessible(true);

        try {
            boolean columnIsNull = cursor.isNull(columnIndex);
            TypeSerializer typeSerializer = Cache.getParserForType(fieldType);
            Object value = null;

            if (typeSerializer != null) {
                fieldType = typeSerializer.getSerializedType();
            }

            // TODO: Find a smarter way to do this? This if block is necessary because we
            // can't know the type until runtime.
            if (columnIsNull) {
                field = null;
            }
            else if (fieldType.equals(Byte.class) || fieldType.equals(byte.class)) {
                value = cursor.getInt(columnIndex);
            }
            else if (fieldType.equals(Short.class) || fieldType.equals(short.class)) {
                value = cursor.getInt(columnIndex);
            }
            else if (fieldType.equals(Integer.class) || fieldType.equals(int.class)) {
                value = cursor.getInt(columnIndex);
            }
            else if (fieldType.equals(Long.class) || fieldType.equals(long.class)) {
                value = cursor.getLong(columnIndex);
            }
            else if (fieldType.equals(Float.class) || fieldType.equals(float.class)) {
                value = cursor.getFloat(columnIndex);
            }
            else if (fieldType.equals(Double.class) || fieldType.equals(double.class)) {
                value = cursor.getDouble(columnIndex);
            }
            else if (fieldType.equals(Boolean.class) || fieldType.equals(boolean.class)) {
                value = cursor.getInt(columnIndex) != 0;
            }
            else if (fieldType.equals(Character.class) || fieldType.equals(char.class)) {
                value = cursor.getString(columnIndex).charAt(0);
            }
            else if (fieldType.equals(String.class)) {
                value = cursor.getString(columnIndex);
            }
            else if (fieldType.equals(Byte[].class) || fieldType.equals(byte[].class)) {
                value = cursor.getBlob(columnIndex);
            }
            else if (ReflectionUtils.isModel(fieldType)) {
                final long entityId = cursor.getLong(columnIndex);
                final Class<? extends Model> entityType = (Class<? extends Model>) fieldType;

                Model entity = Cache.getEntity(entityType, entityId);
                if (entity == null) {
                    entity = new Select().from(entityType).where(idName+"=?", entityId).executeSingle();
                }

                value = entity;
            }
            else if (ReflectionUtils.isSubclassOf(fieldType, Enum.class)) {
                @SuppressWarnings("rawtypes")
                final Class<? extends Enum> enumType = (Class<? extends Enum>) fieldType;
                value = Enum.valueOf(enumType, cursor.getString(columnIndex));
            }

            // Use a deserializer if one is available
            if (typeSerializer != null && !columnIsNull) {
                value = typeSerializer.deserialize(value);
            }

            // Set the field value
            if (value != null) {
                field.set(this, value);
            }
        }
        catch (IllegalArgumentException e) {
            Log.e(e.getClass().getName(), e);
        }
        catch (IllegalAccessException e) {
            Log.e(e.getClass().getName(), e);
        }
        catch (SecurityException e) {
            Log.e(e.getClass().getName(), e);
        }
    }

    if (mId != null) {
        Cache.addEntity(this);
    }
}

This is compiled Model.class (in my bytecode version 50.0 (Java 6)):

public final void loadFromCursor(Cursor cursor) {
    ArrayList columnsOrdered = new ArrayList(Arrays.asList(cursor.getColumnNames()));
    Iterator i$ = this.mTableInfo.getFields().iterator();

    while(true) {
        Field field;
        Class fieldType;
        int columnIndex;
        do {
            if(!i$.hasNext()) {
                if(this.mId != null) {
                    Cache.addEntity(this);
                }

                return;
            }

            field = (Field)i$.next();
            String fieldName = this.mTableInfo.getColumnName(field);
            fieldType = field.getType();
            columnIndex = columnsOrdered.indexOf(fieldName);
        } while(columnIndex < 0);

        field.setAccessible(true);

        try {
            boolean e = cursor.isNull(columnIndex);
            TypeSerializer typeSerializer = Cache.getParserForType(fieldType);
            Object value = null;
            if(typeSerializer != null) {
                fieldType = typeSerializer.getSerializedType();
            }

            if(e) {
                value = null;
            } else if(!fieldType.equals(Byte.class) && !fieldType.equals(Byte.TYPE)) {
                if(!fieldType.equals(Short.class) && !fieldType.equals(Short.TYPE)) {
                    if(!fieldType.equals(Integer.class) && !fieldType.equals(Integer.TYPE)) {
                        if(!fieldType.equals(Long.class) && !fieldType.equals(Long.TYPE)) {
                            if(!fieldType.equals(Float.class) && !fieldType.equals(Float.TYPE)) {
                                if(!fieldType.equals(Double.class) && !fieldType.equals(Double.TYPE)) {
                                    if(!fieldType.equals(Boolean.class) && !fieldType.equals(Boolean.TYPE)) {
                                        if(!fieldType.equals(Character.class) && !fieldType.equals(Character.TYPE)) {
                                            if(fieldType.equals(String.class)) {
                                                value = cursor.getString(columnIndex);
                                            } else if(!fieldType.equals(Byte[].class) && !fieldType.equals(byte[].class)) {
                                                if(ReflectionUtils.isModel(fieldType)) {
                                                    long enumType = cursor.getLong(columnIndex);
                                                    Model entity = Cache.getEntity(fieldType, enumType);
                                                    if(entity == null) {
                                                        entity = (new Select()).from(fieldType).where(this.idName + "=?", new Object[]{Long.valueOf(enumType)}).executeSingle();
                                                    }

                                                    value = entity;
                                                } else if(ReflectionUtils.isSubclassOf(fieldType, Enum.class)) {
                                                    value = Enum.valueOf(fieldType, cursor.getString(columnIndex));
                                                }
                                            } else {
                                                value = cursor.getBlob(columnIndex);
                                            }
                                        } else {
                                            value = Character.valueOf(cursor.getString(columnIndex).charAt(0));
                                        }
                                    } else {
                                        value = Boolean.valueOf(cursor.getInt(columnIndex) != 0);
                                    }
                                } else {
                                    value = Double.valueOf(cursor.getDouble(columnIndex));
                                }
                            } else {
                                value = Float.valueOf(cursor.getFloat(columnIndex));
                            }
                        } else {
                            value = Long.valueOf(cursor.getLong(columnIndex));
                        }
                    } else {
                        value = Integer.valueOf(cursor.getInt(columnIndex));
                    }
                } else {
                    value = Integer.valueOf(cursor.getInt(columnIndex));
                }
            } else {
                value = Integer.valueOf(cursor.getInt(columnIndex));
            }

            if(typeSerializer != null && !e) {
                value = typeSerializer.deserialize(value);
            }

            if (value != null) {
                field.set(this, value);
            }
        } catch (IllegalArgumentException var15) {
            Log.e(var15.getClass().getName(), var15);
        } catch (IllegalAccessException var16) {
            Log.e(var16.getClass().getName(), var16);
        } catch (SecurityException var17) {
            Log.e(var17.getClass().getName(), var17);
        }
    }
}

be aware of the total crappy while(true)...do-while with return if mId != null

In my situation where I don't use any joins, i have to comment

        if (columnIndex < 0) {
            continue;
        }

to fix the compilation issue...after commenting these line the compilation looks fine again! I'm not sure if this a local issue with the used Java compiler, but in case this has to be checked if #106 was applied wrong here.

In addition there is a HUGE error in that code, where existing entities field won't set NULL if the field value in the cursor is NULL:

        if (columnIndex < 0) {
            continue;
        }
[...]
        // Set the field value
        if (value != null) {
            field.set(this, value);
        }

This WON'T change anything, instead it should be:

        if (columnIsNull) {
            value = null;
        }
[...]
        // Set the field value
        field.set(this, value);

This ensures if a field changes to NULL this is propagated to the entity too.

[jlhonora]

As for your last point, shouldn't it be

if (field != null) {
    field.set(this, value);
}

Your proposed change wouldn't work because field is null if columnIsNull == true.

[BlackHornet]

no, I set value = null if columnIsNull

And for your point: in that case it would crash earlier anyway in

field.setAccessible(true);

[jlhonora]

That is not the case:

for (Field field : mTableInfo.getFields()) {
    final String fieldName = mTableInfo.getColumnName(field);
    Class<?> fieldType = field.getType();
    final int columnIndex = columnsOrdered.indexOf(fieldName);

    if (columnIndex < 0) {
        continue;
    }

    field.setAccessible(true);

    try {
        boolean columnIsNull = cursor.isNull(columnIndex);
        TypeSerializer typeSerializer = Cache.getParserForType(fieldType);
        Object value = null;

        if (typeSerializer != null) {
            fieldType = typeSerializer.getSerializedType();
        }

        if (columnIsNull) {
            field = null; // NOTE: Field is set as null
        }

[...]

        // Set the field value
        if (field != null) { // NOTE: This was changed
            field.set(this, value);
        }
    }
}

The comments with NOTE are mine.

[BlackHornet]

But as I said...instead it should be:

        if (columnIsNull) {
            value = null;
        }
[...]
        // Set the field value
        field.set(this, value);

:)

It don't make any sense to set field null if the cursor VALUE for a field is null...