Database support for ordering chapters like the source

app/src/main/java/eu/kanade/tachiyomi/data/database/DbOpenHelper.kt

@@ -5,7 +5,8 @@ import android.database.sqlite.SQLiteDatabase
 import android.database.sqlite.SQLiteOpenHelper
 import eu.kanade.tachiyomi.data.database.tables.*
 
-class DbOpenHelper(context: Context) : SQLiteOpenHelper(context, DbOpenHelper.DATABASE_NAME, null, DbOpenHelper.DATABASE_VERSION) {
+class DbOpenHelper(context: Context)
+: SQLiteOpenHelper(context, DATABASE_NAME, null, DATABASE_VERSION) {
 
     companion object {
         /**
@@ -16,7 +17,7 @@ class DbOpenHelper(context: Context) : SQLiteOpenHelper(context, DbOpenHelper.DA
         /**
          * Version of the database.
          */
-        const val DATABASE_VERSION = 1
+        const val DATABASE_VERSION = 2
     }
 
     override fun onCreate(db: SQLiteDatabase) = with(db) {
@@ -33,7 +34,9 @@ class DbOpenHelper(context: Context) : SQLiteOpenHelper(context, DbOpenHelper.DA
     }
 
     override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
-
+        if (oldVersion < 2) {
+            db.execSQL(ChapterTable.getSourceOrderUpdateQuery())
+        }
     }
 
     override fun onConfigure(db: SQLiteDatabase) {

app/src/main/java/eu/kanade/tachiyomi/data/database/models/Chapter.java

@@ -41,6 +41,9 @@ public class Chapter implements Serializable {
     @StorIOSQLiteColumn(name = ChapterTable.COLUMN_CHAPTER_NUMBER)
     public float chapter_number;
 
+    @StorIOSQLiteColumn(name = ChapterTable.COLUMN_SOURCE_ORDER)
+    public int source_order;
+
     public int status;
 
     private transient List<Page> pages;

app/src/main/java/eu/kanade/tachiyomi/data/database/queries/ChapterQueries.kt

@@ -1,20 +1,16 @@
 package eu.kanade.tachiyomi.data.database.queries
 
-import android.util.Pair
 import com.pushtorefresh.storio.sqlite.operations.get.PreparedGetObject
 import com.pushtorefresh.storio.sqlite.queries.Query
 import com.pushtorefresh.storio.sqlite.queries.RawQuery
 import eu.kanade.tachiyomi.data.database.DbProvider
-import eu.kanade.tachiyomi.data.database.inTransaction
 import eu.kanade.tachiyomi.data.database.models.Chapter
 import eu.kanade.tachiyomi.data.database.models.Manga
 import eu.kanade.tachiyomi.data.database.models.MangaChapter
 import eu.kanade.tachiyomi.data.database.resolvers.ChapterProgressPutResolver
+import eu.kanade.tachiyomi.data.database.resolvers.ChapterSourceOrderPutResolver
 import eu.kanade.tachiyomi.data.database.resolvers.MangaChapterGetResolver
 import eu.kanade.tachiyomi.data.database.tables.ChapterTable
-import eu.kanade.tachiyomi.data.source.base.Source
-import eu.kanade.tachiyomi.util.ChapterRecognition
-import rx.Observable
 import java.util.*
 
 interface ChapterQueries : DbProvider {
@@ -92,67 +88,23 @@ interface ChapterQueries : DbProvider {
 
     fun insertChapters(chapters: List<Chapter>) = db.put().objects(chapters).prepare()
 
-    // TODO this logic shouldn't be here
-    // Add new chapters or delete if the source deletes them
-    open fun insertOrRemoveChapters(manga: Manga, sourceChapters: List<Chapter>, source: Source): Observable<Pair<Int, Int>> {
-        val dbChapters = getChapters(manga).executeAsBlocking()
-
-        val newChapters = Observable.from(sourceChapters)
-                .filter { it !in dbChapters }
-                .doOnNext { c ->
-                    c.manga_id = manga.id
-                    source.parseChapterNumber(c)
-                    ChapterRecognition.parseChapterNumber(c, manga)
-                }.toList()
-
-        val deletedChapters = Observable.from(dbChapters)
-                .filter { it !in sourceChapters }
-                .toList()
-
-        return Observable.zip(newChapters, deletedChapters) { toAdd, toDelete ->
-            var added = 0
-            var deleted = 0
-            var readded = 0
-
-            db.inTransaction {
-                val deletedReadChapterNumbers = TreeSet<Float>()
-                if (!toDelete.isEmpty()) {
-                    for (c in toDelete) {
-                        if (c.read) {
-                            deletedReadChapterNumbers.add(c.chapter_number)
-                        }
-                    }
-                    deleted = deleteChapters(toDelete).executeAsBlocking().results().size
-                }
-
-                if (!toAdd.isEmpty()) {
-                    // Set the date fetch for new items in reverse order to allow another sorting method.
-                    // Sources MUST return the chapters from most to less recent, which is common.
-                    var now = Date().time
-
-                    for (i in toAdd.indices.reversed()) {
-                        val c = toAdd[i]
-                        c.date_fetch = now++
-                        // Try to mark already read chapters as read when the source deletes them
-                        if (c.chapter_number != -1f && c.chapter_number in deletedReadChapterNumbers) {
-                            c.read = true
-                            readded++
-                        }
-                    }
-                    added = insertChapters(toAdd).executeAsBlocking().numberOfInserts()
-                }
-            }
-            Pair.create(added - readded, deleted - readded)
-        }
-    }
-
     fun deleteChapter(chapter: Chapter) = db.delete().`object`(chapter).prepare()
 
     fun deleteChapters(chapters: List<Chapter>) = db.delete().objects(chapters).prepare()
 
     fun updateChapterProgress(chapter: Chapter) = db.put()
             .`object`(chapter)
-            .withPutResolver(ChapterProgressPutResolver.instance)
+            .withPutResolver(ChapterProgressPutResolver())
+            .prepare()
+
+    fun updateChaptersProgress(chapters: List<Chapter>) = db.put()
+            .objects(chapters)
+            .withPutResolver(ChapterProgressPutResolver())
+            .prepare()
+
+    fun fixChaptersSourceOrder(chapters: List<Chapter>) = db.put()
+            .objects(chapters)
+            .withPutResolver(ChapterSourceOrderPutResolver())
             .prepare()
 
 }

app/src/main/java/eu/kanade/tachiyomi/data/database/resolvers/ChapterProgressPutResolver.kt

@@ -11,10 +11,6 @@ import eu.kanade.tachiyomi.data.database.tables.ChapterTable
 
 class ChapterProgressPutResolver : PutResolver<Chapter>() {
 
-    companion object {
-        val instance = ChapterProgressPutResolver()
-    }
-
     override fun performPut(db: StorIOSQLite, chapter: Chapter) = db.inTransactionReturn {
         val updateQuery = mapToUpdateQuery(chapter)
         val contentValues = mapToContentValues(chapter)

app/src/main/java/eu/kanade/tachiyomi/data/database/resolvers/ChapterSourceOrderPutResolver.kt

@@ -0,0 +1,32 @@
+package eu.kanade.tachiyomi.data.database.resolvers
+
+import android.content.ContentValues
+import com.pushtorefresh.storio.sqlite.StorIOSQLite
+import com.pushtorefresh.storio.sqlite.operations.put.PutResolver
+import com.pushtorefresh.storio.sqlite.operations.put.PutResult
+import com.pushtorefresh.storio.sqlite.queries.UpdateQuery
+import eu.kanade.tachiyomi.data.database.inTransactionReturn
+import eu.kanade.tachiyomi.data.database.models.Chapter
+import eu.kanade.tachiyomi.data.database.tables.ChapterTable
+
+class ChapterSourceOrderPutResolver : PutResolver<Chapter>() {
+
+    override fun performPut(db: StorIOSQLite, chapter: Chapter) = db.inTransactionReturn {
+        val updateQuery = mapToUpdateQuery(chapter)
+        val contentValues = mapToContentValues(chapter)
+
+        val numberOfRowsUpdated = db.internal().update(updateQuery, contentValues)
+        PutResult.newUpdateResult(numberOfRowsUpdated, updateQuery.table())
+    }
+
+    fun mapToUpdateQuery(chapter: Chapter) = UpdateQuery.builder()
+            .table(ChapterTable.TABLE)
+            .where("${ChapterTable.COLUMN_URL} = ? AND ${ChapterTable.COLUMN_MANGA_ID} = ?")
+            .whereArgs(chapter.url, chapter.manga_id)
+            .build()
+
+    fun mapToContentValues(chapter: Chapter) = ContentValues(1).apply {
+        put(ChapterTable.COLUMN_SOURCE_ORDER, chapter.source_order)
+    }
+
+}

app/src/main/java/eu/kanade/tachiyomi/data/database/tables/ChapterTable.java

@@ -34,6 +34,9 @@ public final class ChapterTable {
 	@NonNull
 	public static final String COLUMN_CHAPTER_NUMBER = "chapter_number";
 
+	@NonNull
+	public static final String COLUMN_SOURCE_ORDER = "source_order";
+
 	private ChapterTable() throws InstantiationException {
 		throw new InstantiationException("This class is not for instantiation");
 	}
@@ -48,6 +51,7 @@ public final class ChapterTable {
 				+ COLUMN_READ + " BOOLEAN NOT NULL, "
 				+ COLUMN_LAST_PAGE_READ + " INT NOT NULL, "
 				+ COLUMN_CHAPTER_NUMBER + " FLOAT NOT NULL, "
+				+ COLUMN_SOURCE_ORDER + " INTEGER NOT NULL, "
 				+ COLUMN_DATE_FETCH + " LONG NOT NULL, "
 				+ COLUMN_DATE_UPLOAD + " LONG NOT NULL, "
 				+ "FOREIGN KEY(" + COLUMN_MANGA_ID + ") REFERENCES " + MangaTable.TABLE + "(" + MangaTable.COLUMN_ID + ") "
@@ -55,9 +59,15 @@ public final class ChapterTable {
 				+ ");";
 	}
 
+	@NonNull
 	public static String getCreateMangaIdIndexQuery() {
 		return "CREATE INDEX " + TABLE + "_" + COLUMN_MANGA_ID + "_index ON " + TABLE + "(" + COLUMN_MANGA_ID + ");";
 
 	}
+
+	@NonNull
+	public static String getSourceOrderUpdateQuery() {
+		return "ALTER TABLE " + TABLE + " ADD COLUMN " + COLUMN_SOURCE_ORDER + " INTEGER DEFAULT 0";
+	}
 	
 }

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateService.kt

@@ -8,7 +8,6 @@ import android.content.Intent
 import android.os.IBinder
 import android.os.PowerManager
 import android.support.v4.app.NotificationCompat
-import android.util.Pair
 import com.github.pwittchen.reactivenetwork.library.ConnectivityStatus
 import com.github.pwittchen.reactivenetwork.library.ReactiveNetwork
 import eu.kanade.tachiyomi.App
@@ -292,7 +291,7 @@ class LibraryUpdateService : Service() {
         val source = sourceManager.get(manga.source)
         return source!!
                 .pullChaptersFromNetwork(manga.url)
-                .flatMap { db.insertOrRemoveChapters(manga, it, source) }
+                .map { syncChaptersWithSource(db, it, manga, source) }
     }
 
     /**

app/src/main/java/eu/kanade/tachiyomi/ui/manga/chapter/ChaptersPresenter.kt

@@ -1,7 +1,6 @@
 package eu.kanade.tachiyomi.ui.manga.chapter
 
 import android.os.Bundle
-import android.util.Pair
 import eu.kanade.tachiyomi.data.database.DatabaseHelper
 import eu.kanade.tachiyomi.data.database.models.Chapter
 import eu.kanade.tachiyomi.data.database.models.Manga
@@ -15,6 +14,7 @@ import eu.kanade.tachiyomi.ui.base.presenter.BasePresenter
 import eu.kanade.tachiyomi.ui.manga.MangaEvent
 import eu.kanade.tachiyomi.ui.manga.info.ChapterCountEvent
 import eu.kanade.tachiyomi.util.SharedData
+import eu.kanade.tachiyomi.util.syncChaptersWithSource
 import rx.Observable
 import rx.android.schedulers.AndroidSchedulers
 import rx.schedulers.Schedulers
@@ -98,7 +98,7 @@ class ChaptersPresenter : BasePresenter<ChaptersFragment>() {
     fun getOnlineChaptersObs(): Observable<Pair<Int, Int>> {
         return source.pullChaptersFromNetwork(manga.url)
                 .subscribeOn(Schedulers.io())
-                .flatMap { chapters -> db.insertOrRemoveChapters(manga, chapters, source) }
+                .map { syncChaptersWithSource(db, it, manga, source) }
                 .observeOn(AndroidSchedulers.mainThread())
     }
 
@@ -170,7 +170,7 @@ class ChaptersPresenter : BasePresenter<ChaptersFragment>() {
                     }
                 }
                 .toList()
-                .flatMap { db.insertChapters(it).asRxObservable() }
+                .flatMap { db.updateChaptersProgress(it).asRxObservable() }
                 .subscribeOn(Schedulers.io())
                 .subscribe()
     }
@@ -180,7 +180,7 @@ class ChaptersPresenter : BasePresenter<ChaptersFragment>() {
                 .filter { it.chapter_number > -1 && it.chapter_number < selected.chapter_number }
                 .doOnNext { it.read = true }
                 .toList()
-                .flatMap { db.insertChapters(it).asRxObservable() }
+                .flatMap { db.updateChaptersProgress(it).asRxObservable() }
                 .subscribe()
     }
 

app/src/main/java/eu/kanade/tachiyomi/util/ChapterSourceSync.kt

@@ -0,0 +1,83 @@
+package eu.kanade.tachiyomi.util
+
+import eu.kanade.tachiyomi.data.database.DatabaseHelper
+import eu.kanade.tachiyomi.data.database.models.Chapter
+import eu.kanade.tachiyomi.data.database.models.Manga
+import eu.kanade.tachiyomi.data.source.base.Source
+import java.util.*
+
+/**
+ * Helper method for syncing the list of chapters from the source with the ones from the database.
+ *
+ * @param db the database.
+ * @param sourceChapters a list of chapters from the source.
+ * @param manga the manga of the chapters.
+ * @param source the source of the chapters.
+ * @return a pair of new insertions and deletions.
+ */
+fun syncChaptersWithSource(db: DatabaseHelper,
+                           sourceChapters: List<Chapter>,
+                           manga: Manga,
+                           source: Source) : Pair<Int, Int> {
+
+    // Chapters from db.
+    val dbChapters = db.getChapters(manga).executeAsBlocking()
+
+    // Fix manga id and order in source.
+    sourceChapters.forEachIndexed { i, chapter ->
+        chapter.manga_id = manga.id
+        chapter.source_order = i
+    }
+
+    // Chapters from the source not in db.
+    val toAdd = sourceChapters.filterNot { it in dbChapters }
+
+    // Recognize number for new chapters.
+    toAdd.forEach {
+        source.parseChapterNumber(it)
+        ChapterRecognition.parseChapterNumber(it, manga)
+    }
+
+    // Chapters from the db not in the source.
+    val toDelete = dbChapters.filterNot { it in sourceChapters }
+
+    // Amount of chapters added and deleted.
+    var added = 0
+    var deleted = 0
+
+    // Amount of chapters readded (different url but the same chapter number).
+    var readded = 0
+
+    db.inTransaction {
+        val deletedReadChapterNumbers = TreeSet<Float>()
+        if (!toDelete.isEmpty()) {
+            for (c in toDelete) {
+                if (c.read) {
+                    deletedReadChapterNumbers.add(c.chapter_number)
+                }
+            }
+            deleted = db.deleteChapters(toDelete).executeAsBlocking().results().size
+        }
+
+        if (!toAdd.isEmpty()) {
+            // Set the date fetch for new items in reverse order to allow another sorting method.
+            // Sources MUST return the chapters from most to less recent, which is common.
+            var now = Date().time
+
+            for (i in toAdd.indices.reversed()) {
+                val c = toAdd[i]
+                c.date_fetch = now++
+                // Try to mark already read chapters as read when the source deletes them
+                if (c.chapter_number != -1f && c.chapter_number in deletedReadChapterNumbers) {
+                    c.read = true
+                    readded++
+                }
+            }
+            added = db.insertChapters(toAdd).executeAsBlocking().numberOfInserts()
+        }
+
+        // Fix order in source.
+        db.fixChaptersSourceOrder(sourceChapters).executeAsBlocking()
+    }
+    return Pair(added - readded, deleted - readded)
+}

app/src/test/java/eu/kanade/tachiyomi/data/library/LibraryUpdateServiceTest.java

@@ -2,7 +2,6 @@ package eu.kanade.tachiyomi.data.library;
 
 import android.content.Context;
 import android.os.Build;
-import android.util.Pair;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -21,14 +20,9 @@ import eu.kanade.tachiyomi.data.database.models.Manga;
 import eu.kanade.tachiyomi.data.source.base.Source;
 import rx.Observable;
 
-import static org.mockito.Matchers.any;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.anyListOf;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @Config(constants = BuildConfig.class, sdk = Build.VERSION_CODES.LOLLIPOP)
@@ -62,45 +56,39 @@ public class LibraryUpdateServiceTest {
 
     @Test
     public void testUpdateManga() {
-        Manga manga = Manga.create("manga1");
-        List<Chapter> chapters = createChapters("/chapter1", "/chapter2");
+        Manga manga = createManga("/manga1").get(0);
+        manga.id = 1L;
+        service.db.insertManga(manga).executeAsBlocking();
+
+        List<Chapter> sourceChapters = createChapters("/chapter1", "/chapter2");
 
-        when(source.pullChaptersFromNetwork(manga.url)).thenReturn(Observable.just(chapters));
-        when(service.db.insertOrRemoveChapters(manga, chapters, source))
-                .thenReturn(Observable.just(Pair.create(2, 0)));
+        when(source.pullChaptersFromNetwork(manga.url)).thenReturn(Observable.just(sourceChapters));
 
         service.updateManga(manga).subscribe();
 
-        verify(service.db).insertOrRemoveChapters(manga, chapters, source);
+        assertThat(service.db.getChapters(manga).executeAsBlocking()).hasSize(2);
     }
 
     @Test
     public void testContinuesUpdatingWhenAMangaFails() {
-        Manga manga1 = Manga.create("manga1");
-        Manga manga2 = Manga.create("manga2");
-        Manga manga3 = Manga.create("manga3");
-
-        List<Manga> favManga = createManga("manga1", "manga2", "manga3");
+        List<Manga> favManga = createManga("/manga1", "/manga2", "/manga3");
+        service.db.insertMangas(favManga).executeAsBlocking();
+        favManga = service.db.getFavoriteMangas().executeAsBlocking();
 
         List<Chapter> chapters = createChapters("/chapter1", "/chapter2");
         List<Chapter> chapters3 = createChapters("/achapter1", "/achapter2");
 
-        when(service.db.getFavoriteMangas().executeAsBlocking()).thenReturn(favManga);
-
         // One of the updates will fail
-        when(source.pullChaptersFromNetwork("manga1")).thenReturn(Observable.just(chapters));
-        when(source.pullChaptersFromNetwork("manga2")).thenReturn(Observable.<List<Chapter>>error(new Exception()));
-        when(source.pullChaptersFromNetwork("manga3")).thenReturn(Observable.just(chapters3));
-
-        when(service.db.insertOrRemoveChapters(manga1, chapters, source)).thenReturn(Observable.just(Pair.create(2, 0)));
-        when(service.db.insertOrRemoveChapters(manga3, chapters, source)).thenReturn(Observable.just(Pair.create(2, 0)));
+        when(source.pullChaptersFromNetwork("/manga1")).thenReturn(Observable.just(chapters));
+        when(source.pullChaptersFromNetwork("/manga2")).thenReturn(Observable.<List<Chapter>>error(new Exception()));
+        when(source.pullChaptersFromNetwork("/manga3")).thenReturn(Observable.just(chapters3));
 
         service.updateMangaList(service.getMangaToUpdate(null)).subscribe();
 
         // There are 3 network attempts and 2 insertions (1 request failed)
-        verify(source, times(3)).pullChaptersFromNetwork((String)any());
-        verify(service.db, times(2)).insertOrRemoveChapters((Manga)any(), anyListOf(Chapter.class), (Source)any());
-        verify(service.db, never()).insertOrRemoveChapters(eq(manga2), anyListOf(Chapter.class), (Source)any());
+        assertThat(service.db.getChapters(favManga.get(0)).executeAsBlocking()).hasSize(2);
+        assertThat(service.db.getChapters(favManga.get(1)).executeAsBlocking()).hasSize(0);
+        assertThat(service.db.getChapters(favManga.get(2)).executeAsBlocking()).hasSize(2);
     }
 
     private List<Chapter> createChapters(String... urls) {
@@ -108,6 +96,7 @@ public class LibraryUpdateServiceTest {
         for (String url : urls) {
             Chapter c = Chapter.create();
             c.url = url;
+            c.name = url.substring(1);
             list.add(c);
         }
         return list;
@@ -117,6 +106,8 @@ public class LibraryUpdateServiceTest {
         List<Manga> list = new ArrayList<>();
         for (String url : urls) {
             Manga m = Manga.create(url);
+            m.title = url.substring(1);
+            m.favorite = true;
             list.add(m);
         }
         return list;

app/src/test/java/eu/kanade/tachiyomi/injection/module/TestDataModule.kt

@@ -1,17 +1,12 @@
 package eu.kanade.tachiyomi.injection.module
 
 import android.app.Application
-import eu.kanade.tachiyomi.data.database.DatabaseHelper
 import eu.kanade.tachiyomi.data.network.NetworkHelper
 import eu.kanade.tachiyomi.data.source.SourceManager
 import org.mockito.Mockito
 
 class TestDataModule : DataModule() {
 
-    override fun provideDatabaseHelper(app: Application): DatabaseHelper {
-        return Mockito.mock(DatabaseHelper::class.java, Mockito.RETURNS_DEEP_STUBS)
-    }
-
     override fun provideNetworkHelper(app: Application): NetworkHelper {
         return Mockito.mock(NetworkHelper::class.java)
     }