go forward through cursor in message restore

Daniel Gultsch created 6 years ago

We have seen some weird CursorIndexNotFoundException that we were unable to reproduce.
We assume that going forward (moveToNext()) through the cursor instead of (moveToPrevious() fixes that issue

Change summary

src/main/java/eu/siacs/conversations/entities/Message.java            | 40 
src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java | 49 
2 files changed, 20 insertions(+), 69 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Message.java

@@ -3,7 +3,6 @@ package eu.siacs.conversations.entities;
 import android.content.ContentValues;
 import android.database.Cursor;
 import android.graphics.Color;
-import android.support.annotation.ColorInt;
 import android.text.SpannableStringBuilder;
 import android.util.Log;
 
@@ -181,35 +180,11 @@ public class Message extends AbstractEntity implements AvatarService.Avatarable
 	}
 
 	public static Message fromCursor(Cursor cursor, Conversation conversation) {
-		Jid jid;
-		try {
-			String value = cursor.getString(cursor.getColumnIndex(COUNTERPART));
-			if (value != null) {
-				jid = Jid.of(value);
-			} else {
-				jid = null;
-			}
-		} catch (IllegalArgumentException e) {
-			jid = null;
-		} catch (IllegalStateException e) {
-			return null; // message too long?
-		}
-		Jid trueCounterpart;
-		try {
-			String value = cursor.getString(cursor.getColumnIndex(TRUE_COUNTERPART));
-			if (value != null) {
-				trueCounterpart = Jid.of(value);
-			} else {
-				trueCounterpart = null;
-			}
-		} catch (IllegalArgumentException e) {
-			trueCounterpart = null;
-		}
 		return new Message(conversation,
 				cursor.getString(cursor.getColumnIndex(UUID)),
 				cursor.getString(cursor.getColumnIndex(CONVERSATION)),
-				jid,
-				trueCounterpart,
+				fromString(cursor.getString(cursor.getColumnIndex(COUNTERPART))),
+				fromString(cursor.getString(cursor.getColumnIndex(TRUE_COUNTERPART))),
 				cursor.getString(cursor.getColumnIndex(BODY)),
 				cursor.getLong(cursor.getColumnIndex(TIME_SENT)),
 				cursor.getInt(cursor.getColumnIndex(ENCRYPTION)),
@@ -229,6 +204,17 @@ public class Message extends AbstractEntity implements AvatarService.Avatarable
 				cursor.getInt(cursor.getColumnIndex(DELETED)) > 0);
 	}
 
+	private static Jid fromString(String value) {
+		try {
+			if (value != null) {
+				return Jid.of(value);
+			}
+		} catch (IllegalArgumentException e) {
+			return null;
+		}
+		return null;
+	}
+
 	public static Message createStatusMessage(Conversation conversation, String body) {
 		final Message message = new Message(conversation);
 		message.setType(Message.TYPE_STATUS);

src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java

@@ -28,10 +28,8 @@ import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -751,14 +749,15 @@ public class DatabaseBackend extends SQLiteOpenHelper {
                     null, null, Message.TIME_SENT + " DESC",
                     String.valueOf(limit));
         }
-        if (cursor.getCount() > 0) {
-            cursor.moveToLast();
-            do {
-                Message message = Message.fromCursor(cursor, conversation);
+        while (cursor.moveToNext()) {
+            try {
+                final Message message = Message.fromCursor(cursor, conversation);
                 if (message != null) {
-                    list.add(message);
+                    list.add(0, message);
                 }
-            } while (cursor.moveToPrevious());
+            } catch (Exception e) {
+                Log.e(Config.LOGTAG,"unable to restore message");
+            }
         }
         cursor.close();
         return list;
@@ -771,40 +770,6 @@ public class DatabaseBackend extends SQLiteOpenHelper {
         return db.rawQuery(SQL, new String[]{FtsUtils.toMatchString(term)});
     }
 
-    public Iterable<Message> getMessagesIterable(final Conversation conversation) {
-        return () -> {
-            class MessageIterator implements Iterator<Message> {
-                private SQLiteDatabase db = getReadableDatabase();
-                private String[] selectionArgs = {conversation.getUuid()};
-                private Cursor cursor = db.query(Message.TABLENAME, null, Message.CONVERSATION
-                        + "=?", selectionArgs, null, null, Message.TIME_SENT
-                        + " ASC", null);
-
-                private MessageIterator() {
-                    cursor.moveToFirst();
-                }
-
-                @Override
-                public boolean hasNext() {
-                    return !cursor.isAfterLast();
-                }
-
-                @Override
-                public Message next() {
-                    Message message = Message.fromCursor(cursor, conversation);
-                    cursor.moveToNext();
-                    return message;
-                }
-
-                @Override
-                public void remove() {
-                    throw new UnsupportedOperationException();
-                }
-            }
-            return new MessageIterator();
-        };
-    }
-
     public List<String> markFileAsDeleted(final File file, final boolean internal) {
         SQLiteDatabase db = this.getReadableDatabase();
         String selection;