Fix the issue #106 (#106)

When use the cursor method getColumnIndex(String columnName) and have multiple columns with same name, the android implementation not return the first index, so the model entity is filled with wrong data. Create a custom search of index and replace the usages. Also add a junit test to check the issue and the fix.
pardom-zz · May 28, 2014 · c97998d · c97998d
1 parent 12c6937
commit c97998d
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 2 deletions.
diff --git a/src/com/activeandroid/Model.java b/src/com/activeandroid/Model.java
@@ -28,6 +28,8 @@
 import com.activeandroid.util.ReflectionUtils;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 @SuppressWarnings("unchecked")
@@ -176,10 +178,15 @@ public static <T extends Model> T load(Class<T> type, long id) {
 	// Model population
 
 	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 = cursor.getColumnIndex(fieldName);
+			final int columnIndex = columnsOrdered.indexOf(fieldName);
 
 			if (columnIndex < 0) {
 				continue;

diff --git a/src/com/activeandroid/util/SQLiteUtils.java b/src/com/activeandroid/util/SQLiteUtils.java
@@ -27,8 +27,11 @@
 import com.activeandroid.annotation.Column.ConflictAction;
 import com.activeandroid.serializer.TypeSerializer;
 
+import java.lang.Long;
+import java.lang.String;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -327,8 +330,13 @@ public static <T extends Model> List<T> processCursor(Class<? extends Model> typ
 			Constructor<?> entityConstructor = type.getConstructor();
 
 			if (cursor.moveToFirst()) {
+                /**
+                 * 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()));
 				do {
-					Model entity = Cache.getEntity(type, cursor.getLong(cursor.getColumnIndex(idName)));
+					Model entity = Cache.getEntity(type, cursor.getLong(columnsOrdered.indexOf(idName)));
 					if (entity == null) {
 						entity = (T) entityConstructor.newInstance();
 					}

diff --git a/tests/src/com/activeandroid/test/ModelTest.java b/tests/src/com/activeandroid/test/ModelTest.java
@@ -19,11 +19,15 @@
 import com.activeandroid.Cache;
 import com.activeandroid.Model;
 import com.activeandroid.TableInfo;
+import com.activeandroid.annotation.Column;
 import com.activeandroid.annotation.Table;
 import com.activeandroid.query.Select;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Date;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -167,11 +171,97 @@ public void testBooleanColumnType() {
         assertNull( new Select().from(MockModel.class).where("booleanField = ?", 1).executeSingle() );
 
         assertNull( new Select().from(MockModel.class).where("booleanField = ?", true).executeSingle() );
+    }
+
+	/**
+     * Test to check the join of two (or more) tables with some fields in common when not use a projection on select.
+     * Test the issue #106 (https://github.com/pardom/ActiveAndroid/issues/106)
+     */
+    public void testJoinWithSameNames(){
+        //create a parent entity and store
+        ParentJoinMockModel parent = new ParentJoinMockModel();
+        parent.booleanField = true;
+        parent.dateField = new Date();
+        parent.doubleField = 2.0;
+        parent.intField = 1;
+        parent.save();
+
+        //the values to assign to child
+        Date dateValue = new Date();
+        double doubleValue = 30.0;
+        int intValue = 3;
+
+        //create two child entities, relate with parent and save
+        ChildMockModel child1 = new ChildMockModel();
+        child1.booleanField = false;
+        child1.dateField = dateValue;
+        child1.doubleField = doubleValue;
+        child1.intField = intValue;
+        child1.parent = parent;
+        child1.save();
+
+        ChildMockModel child2 = new ChildMockModel();
+        child2.booleanField = false;
+        child2.dateField = dateValue;
+        child2.doubleField = doubleValue;
+        child2.intField = intValue;
+        child2.parent = parent;
+        child2.save();
+
+        //Store the ids assigned to child entities when persists
+        List<Long> ids = new ArrayList<Long>();
+        ids.add(child1.getId());
+        ids.add(child2.getId());
+
+        //make the query with a join
+        List<ChildMockModel> result = new Select().from(ChildMockModel.class).
+                join(ParentJoinMockModel.class).on("ParentJoinMockModel.Id = ChildMockModel.parent").execute();
+
+        //check result
+        assertNotNull(result);
+        assertEquals(result.size(), 2);
+        for(ChildMockModel currentModel : result){
+            assertFalse(currentModel.booleanField);
+            assertEquals(currentModel.intField, intValue);
+            assertEquals(currentModel.doubleField, doubleValue);
+            assertTrue(ids.contains(currentModel.getId()));
+        }
+
     }
 
 	/**
 	 * Mock model as we need 2 different model classes.
 	 */
 	@Table(name = "AnotherMockTable")
 	public static class AnotherMockModel extends Model {}
+
+    /**
+     * Mock model to test joins with same names.
+     * It's a copy from MockModel.
+     */
+    @Table(name = "ParentJoinMockModel")
+    public static class ParentJoinMockModel extends Model {
+        @Column
+        public Date dateField;
+
+        @Column
+        public double doubleField;
+
+        @Column
+        public int intField;
+
+        @Column
+        public boolean booleanField;
+    }
+
+    /**
+     * Mock model to test joins with same names.
+     * Extends from ParentJoinMockModel to have the same columns.
+     * Have a relationship with ParentJoinMockModel to make te join query.
+     */
+    @Table(name = "ChildMockModel")
+    public static class ChildMockModel extends ParentJoinMockModel {
+        @Column
+        ParentJoinMockModel parent;
+    }
 }