Shift contributions to use Room DB (#3324)

* Part of #3127
* Added Room Dependency
* Shifted ContributionsDao to use RoomDB

* Save and Fetch contributions via RoomDAO

* Bugfixes, fixed test cases, injected schedulers for ContributionsPresenter

* removed stetho

* Fixed ReviewHelperTest cases

* Fixed test cases in DeleteHelperTest

* Fetch all contributions [TODO add pagination to use this, maybe later in a seperate PR]

* Update Schema false in AppDatabase

* removed parameter from fetchControbutions

* Added logs for fetch contributions

* Fixed test case ContributionsPresenter

* Added an autogenerate primary key, submit save contributions on executor

* fixed getItemAtPosition

* MainActivity Config changes +=orientation

* BugFixes
* Make AppDataBase Singleton
* Set _id as autogenerate primary key [replacing the previously used filename, seems like they are not unique]
* Replace Execxutor Utils with Subscribers on Singles in UploadService
* BugFix, Upload Progress

* Remove un-nescessary null check on contributions in ContributionsListAdapter

* removed ContributionsListFragment [not-implemeted]

* Review suggested changes
* removed un-nescessary null checks
* provide ContributionsDao
* Minor bug fixes

* wip

* delete existing contributions table (from the existing db) on upgrade

* remove un-nescessary null checks in test classes

* shifted media to be a local variable in ReviewHelperTest

* removed captured folder

* Dispose composite disposables in UploadService

* replaced size check with isEmpty ContributionsPresenter

* transform saveContributions to a Completable

* Addressed comments in review
* Typo in Contributions
* ReasonBuilderTest (create media object instead of mocking)
* Use global Gson object instead of creating a new one in Converters

* Provide Gson to Converters from the CommonsApplicationComponent

* use static method instead of field instead of static field to provide GSON in Converters

* Modified gitignore to exclude captures/*

app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt

@@ -25,6 +25,14 @@ class TestCommonsApplication : Application() {
                     .build()
         }
         super.onCreate()
+        context=applicationContext
+    }
+
+    companion object{
+        private var context: Context?=null
+        fun getContext(): Context? {
+            return context
+        }
     }
 }
 

app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionDaoTest.kt

@@ -1,348 +0,0 @@
-package fr.free.nrw.commons.contributions
-
-import android.content.ContentProviderClient
-import android.content.ContentValues
-import android.database.MatrixCursor
-import android.database.sqlite.SQLiteDatabase
-import android.net.Uri
-import android.os.RemoteException
-import com.nhaarman.mockitokotlin2.*
-import fr.free.nrw.commons.BuildConfig
-import fr.free.nrw.commons.TestCommonsApplication
-import fr.free.nrw.commons.Utils
-import fr.free.nrw.commons.contributions.Contribution.*
-import fr.free.nrw.commons.contributions.ContributionDao.Table
-import fr.free.nrw.commons.contributions.ContributionsContentProvider.BASE_URI
-import fr.free.nrw.commons.contributions.ContributionsContentProvider.uriForId
-import org.junit.Assert.*
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
-import org.robolectric.annotation.Config
-import java.util.*
-
-@RunWith(RobolectricTestRunner::class)
-@Config(sdk = [21], application = TestCommonsApplication::class)
-class ContributionDaoTest {
-    private val localUri = "http://example.com/"
-    private val client: ContentProviderClient = mock()
-    private val database: SQLiteDatabase = mock()
-    private val captor = argumentCaptor<ContentValues>()
-
-    private lateinit var contentUri: Uri
-    private lateinit var testObject: ContributionDao
-
-    @Before
-    fun setUp() {
-        contentUri = uriForId(111)
-        testObject = ContributionDao { client }
-    }
-
-    @Test
-    fun createTable() {
-        Table.onCreate(database)
-        verify(database).execSQL(Table.CREATE_TABLE_STATEMENT)
-    }
-
-    @Test
-    fun deleteTable() {
-        Table.onDelete(database)
-
-        inOrder(database) {
-            verify(database).execSQL(Table.DROP_TABLE_STATEMENT)
-            verify(database).execSQL(Table.CREATE_TABLE_STATEMENT)
-        }
-    }
-
-    @Test
-    fun upgradeDatabase_v1_to_v2() {
-        Table.onUpdate(database, 1, 2)
-
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_DESCRIPTION_FIELD)
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_CREATOR_FIELD)
-        }
-    }
-
-    @Test
-    fun upgradeDatabase_v2_to_v3() {
-        Table.onUpdate(database, 2, 3)
-
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_MULTIPLE_FIELD)
-            verify<SQLiteDatabase>(database).execSQL(Table.SET_DEFAULT_MULTIPLE)
-        }
-    }
-
-    @Test
-    fun upgradeDatabase_v3_to_v4() {
-        Table.onUpdate(database, 3, 4)
-    }
-
-    @Test
-    fun upgradeDatabase_v4_to_v5() {
-        Table.onUpdate(database, 4, 5)
-    }
-
-    @Test
-    fun upgradeDatabase_v5_to_v6() {
-        Table.onUpdate(database, 5, 6)
-
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_WIDTH_FIELD)
-            verify<SQLiteDatabase>(database).execSQL(Table.SET_DEFAULT_WIDTH)
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_HEIGHT_FIELD)
-            verify<SQLiteDatabase>(database).execSQL(Table.SET_DEFAULT_HEIGHT)
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_LICENSE_FIELD)
-            verify<SQLiteDatabase>(database).execSQL(Table.SET_DEFAULT_LICENSE)
-        }
-    }
-
-    @Test
-    fun migrateTableVersionFrom_v6_to_v7() {
-        Table.onUpdate(database, 6, 7)
-        // Table has changed in version 7
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_WIKI_DATA_ENTITY_ID_FIELD)
-        }
-    }
-
-    @Test
-    fun migrateTableVersionFrom_v7_to_v8() {
-        Table.onUpdate(database, 7, 8)
-        // Table has changed in version 8
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_WIKI_DATA_ENTITY_ID_FIELD)
-        }
-    }
-
-    @Test
-    fun migrateTableVersionFrom_v8_to_v9() {
-        Table.onUpdate(database, 8, 9)
-        // Table changed in version 9
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_WIKI_DATA_ENTITY_ID_FIELD)
-        }
-    }
-
-    @Test
-    fun migrateTableVersionFrom_v9_to_v10() {
-        Table.onUpdate(database, 8, 9)
-        // Table changed in version 9
-        inOrder(database) {
-            verify<SQLiteDatabase>(database).execSQL(Table.ADD_WIKI_DATA_ENTITY_ID_FIELD)
-        }
-    }
-
-    @Test
-    fun saveNewContribution_nonNullFields() {
-        whenever(client.insert(isA(), isA())).thenReturn(contentUri)
-        val contribution = createContribution(true, null, null, null, null)
-
-        testObject.save(contribution)
-
-        assertEquals(contentUri, contribution.contentUri)
-        verify(client).insert(eq(BASE_URI), captor.capture())
-        captor.firstValue.let {
-            // Long fields
-            assertEquals(222L, it.getAsLong(Table.COLUMN_LENGTH))
-            assertEquals(321L, it.getAsLong(Table.COLUMN_TIMESTAMP))
-            assertEquals(333L, it.getAsLong(Table.COLUMN_TRANSFERRED))
-
-            // Integer fields
-            assertEquals(STATE_COMPLETED, it.getAsInteger(Table.COLUMN_STATE))
-            assertEquals(640, it.getAsInteger(Table.COLUMN_WIDTH))
-            assertEquals(480, it.getAsInteger(Table.COLUMN_HEIGHT))
-
-            // String fields
-            assertEquals(SOURCE_CAMERA, it.getAsString(Table.COLUMN_SOURCE))
-            assertEquals("desc", it.getAsString(Table.COLUMN_DESCRIPTION))
-            assertEquals("create", it.getAsString(Table.COLUMN_CREATOR))
-            assertEquals("007", it.getAsString(Table.COLUMN_LICENSE))
-        }
-    }
-
-    @Test
-    fun saveNewContribution_nullableFieldsAreNull() {
-        whenever(client.insert(isA(), isA())).thenReturn(contentUri)
-        val contribution = createContribution(true, null, null, null, null)
-
-        testObject.save(contribution)
-
-        assertEquals(contentUri, contribution.contentUri)
-        verify(client).insert(eq(BASE_URI), captor.capture())
-        captor.firstValue.let {
-            // Nullable fields are absent if null
-            assertFalse(it.containsKey(Table.COLUMN_LOCAL_URI))
-            assertFalse(it.containsKey(Table.COLUMN_IMAGE_URL))
-            assertFalse(it.containsKey(Table.COLUMN_UPLOADED))
-        }
-    }
-
-    @Test
-    fun saveNewContribution_nullableFieldsAreNonNull() {
-        whenever(client.insert(isA(), isA())).thenReturn(contentUri)
-        val contribution = createContribution(true, Uri.parse(localUri),
-                "image", Date(456L), null)
-
-        testObject.save(contribution)
-
-        assertEquals(contentUri, contribution.contentUri)
-        verify(client).insert(eq(BASE_URI), captor.capture())
-        captor.firstValue.let {
-            assertEquals(localUri, it.getAsString(Table.COLUMN_LOCAL_URI))
-            assertEquals("image", it.getAsString(Table.COLUMN_IMAGE_URL))
-            assertEquals(456L, it.getAsLong(Table.COLUMN_UPLOADED))
-        }
-    }
-
-    @Test
-    fun saveNewContribution_booleanEncodesTrue() {
-        whenever(client.insert(isA(), isA())).thenReturn(contentUri)
-        val contribution = createContribution(true, null, null, null, null)
-
-        testObject.save(contribution)
-
-        assertEquals(contentUri, contribution.contentUri)
-        verify(client).insert(eq(BASE_URI), captor.capture())
-
-        // Boolean true --> 1 for ths encoding scheme
-        assertEquals("Boolean true should be encoded as 1", 1,
-                captor.firstValue.getAsInteger(Table.COLUMN_MULTIPLE))
-    }
-
-    @Test
-    fun saveNewContribution_booleanEncodesFalse() {
-        whenever(client.insert(isA(), isA())).thenReturn(contentUri)
-        val contribution = createContribution(false, null, null, null, null)
-
-        testObject.save(contribution)
-
-        assertEquals(contentUri, contribution.contentUri)
-        verify(client).insert(eq(BASE_URI), captor.capture())
-
-        // Boolean true --> 1 for ths encoding scheme
-        assertEquals("Boolean false should be encoded as 0", 0,
-                captor.firstValue.getAsInteger(Table.COLUMN_MULTIPLE))
-    }
-
-    @Test
-    fun saveExistingContribution() {
-        val contribution = createContribution(false, null, null, null, null)
-        contribution.contentUri = contentUri
-
-        testObject.save(contribution)
-
-        verify(client).update(eq(contentUri), isA(), isNull(), isNull())
-    }
-
-    @Test(expected = RuntimeException::class)
-    fun saveTranslatesExceptions() {
-        whenever(client.insert(isA(), isA())).thenThrow(RemoteException(""))
-
-        testObject.save(createContribution(false, null, null, null, null))
-    }
-
-    @Test(expected = RuntimeException::class)
-    fun deleteTranslatesExceptions() {
-        whenever(client.delete(anyOrNull(), anyOrNull(), anyOrNull())).thenThrow(RemoteException(""))
-
-        val contribution = createContribution(false, null, null, null, null)
-        contribution.contentUri = contentUri
-        testObject.delete(contribution)
-    }
-
-    @Test(expected = RuntimeException::class)
-    fun exceptionThrownWhenAttemptingToDeleteUnsavedContribution() {
-        testObject.delete(createContribution(false, null, null, null, null))
-    }
-
-    @Test
-    fun deleteExistingContribution() {
-        val contribution = createContribution(false, null, null, null, null)
-        contribution.contentUri = contentUri
-
-        testObject.delete(contribution)
-
-        verify(client).delete(eq(contentUri), isNull(), isNull())
-    }
-
-    @Test
-    fun createFromCursor() {
-        val created = 321L
-        val uploaded = 456L
-        createCursor(created, uploaded, false, localUri).let { mc ->
-            testObject.fromCursor(mc).let {
-                assertEquals(uriForId(111), it.contentUri)
-                assertEquals("filePath", it.filename)
-                assertEquals(localUri, it.localUri.toString())
-                assertEquals("image", it.imageUrl)
-                assertEquals(created, it.dateCreated.time)
-                assertEquals(STATE_QUEUED, it.state)
-                assertEquals(222L, it.dataLength)
-                assertEquals(uploaded, it.dateUploaded?.time)
-                assertEquals(88L, it.transferred)
-                assertEquals(SOURCE_GALLERY, it.source)
-                assertEquals("desc", it.description)
-                assertEquals("create", it.creator)
-                assertEquals(640, it.width)
-                assertEquals(480, it.height)
-                assertEquals("007", it.license)
-            }
-        }
-    }
-
-    @Test
-    fun createFromCursor_nullableTimestamps() {
-        createCursor(0L, 0L, false, localUri).let { mc ->
-            testObject.fromCursor(mc).let {
-                assertNull(it.dateCreated)
-                assertNull(it.dateUploaded)
-            }
-        }
-    }
-
-    @Test
-    fun createFromCursor_nullableLocalUri() {
-        createCursor(0L, 0L, false, "").let { mc ->
-            testObject.fromCursor(mc).let {
-                assertNull(it.localUri)
-                assertNull(it.dateCreated)
-                assertNull(it.dateUploaded)
-            }
-        }
-    }
-
-    @Test
-    fun createFromCursor_booleanEncoding() {
-        val mcFalse = createCursor(0L, 0L, false, localUri)
-        assertFalse(testObject.fromCursor(mcFalse).multiple)
-
-        val mcHammer = createCursor(0L, 0L, true, localUri)
-        assertTrue(testObject.fromCursor(mcHammer).multiple)
-    }
-
-    private fun createCursor(created: Long, uploaded: Long, multiple: Boolean, localUri: String) =
-            MatrixCursor(Table.ALL_FIELDS, 1).apply {
-                addRow(listOf("111", "filePath", localUri, "image",
-                        created, STATE_QUEUED, 222L, uploaded, 88L, SOURCE_GALLERY, "desc",
-                        "create", if (multiple) 1 else 0, 640, 480, "007", "Q1"))
-                moveToFirst()
-            }
-
-    private fun createContribution(isMultiple: Boolean, localUri: Uri?, imageUrl: String?, dateUploaded: Date?, filename: String?): Contribution {
-        val contribution = Contribution(localUri, imageUrl, filename, "desc", 222L, Date(321L), dateUploaded,
-                "create", "edit", "coords").apply {
-            state = STATE_COMPLETED
-            transferred = 333L
-            source = SOURCE_CAMERA
-            license = "007"
-            multiple = isMultiple
-            width = 640
-            height = 480  // VGA should be enough for anyone, right?
-        }
-        contribution.wikiDataEntityId = "Q1"
-        return contribution
-    }
-}

app/src/test/kotlin/fr/free/nrw/commons/contributions/ContributionsPresenterTest.kt

@@ -1,11 +1,22 @@
 package fr.free.nrw.commons.contributions
 
 import android.database.Cursor
+import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.LiveData
+import androidx.lifecycle.MutableLiveData
 import androidx.loader.content.CursorLoader
 import androidx.loader.content.Loader
+import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.verify
+import com.nhaarman.mockitokotlin2.whenever
+import io.reactivex.Scheduler
+import io.reactivex.Single
+import io.reactivex.schedulers.TestScheduler
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
+import org.mockito.ArgumentMatchers
 import org.mockito.Mock
 import org.mockito.Mockito
 import org.mockito.MockitoAnnotations
@@ -15,11 +26,11 @@ import org.mockito.MockitoAnnotations
  */
 class ContributionsPresenterTest {
     @Mock
-    internal var repository: ContributionsRepository? = null
+    internal lateinit var repository: ContributionsRepository
     @Mock
-    internal var view: ContributionsContract.View? = null
+    internal lateinit var view: ContributionsContract.View
 
-    private var contributionsPresenter: ContributionsPresenter? = null
+    private lateinit var contributionsPresenter: ContributionsPresenter
 
     private lateinit var cursor: Cursor
 
@@ -27,6 +38,12 @@ class ContributionsPresenterTest {
 
     lateinit var loader: Loader<Cursor>
 
+    lateinit var liveData: LiveData<List<Contribution>>
+
+    @Rule @JvmField var instantTaskExecutorRule = InstantTaskExecutorRule()
+
+    lateinit var scheduler : Scheduler
+
     /**
      * initial setup
      */
@@ -34,21 +51,24 @@ class ContributionsPresenterTest {
     @Throws(Exception::class)
     fun setUp() {
         MockitoAnnotations.initMocks(this)
+        scheduler=TestScheduler()
         cursor = Mockito.mock(Cursor::class.java)
         contribution = Mockito.mock(Contribution::class.java)
-        contributionsPresenter = ContributionsPresenter(repository)
+        contributionsPresenter = ContributionsPresenter(repository,scheduler,scheduler)
         loader = Mockito.mock(CursorLoader::class.java)
-        contributionsPresenter?.onAttachView(view)
+        contributionsPresenter.onAttachView(view)
+        liveData=MutableLiveData()
     }
 
-
     /**
-     * Test presenter actions onGetContributionFromCursor
+     * Test fetch contributions
      */
     @Test
-    fun testGetContributionFromCursor() {
-        contributionsPresenter?.getContributionsFromCursor(cursor)
-        verify(repository)?.getContributionFromCursor(cursor)
+    fun testFetchContributions(){
+        whenever(repository.getString(ArgumentMatchers.anyString())).thenReturn("10")
+        whenever(repository.fetchContributions()).thenReturn(liveData)
+        contributionsPresenter.fetchContributions()
+        verify(repository).fetchContributions()
     }
 
     /**
@@ -56,55 +76,20 @@ class ContributionsPresenterTest {
      */
     @Test
     fun testDeleteContribution() {
-        contributionsPresenter?.deleteUpload(contribution)
-        verify(repository)?.deleteContributionFromDB(contribution)
+        whenever(repository.deleteContributionFromDB(ArgumentMatchers.any(Contribution::class.java))).thenReturn(Single.just(1))
+        contributionsPresenter.deleteUpload(contribution)
+        verify(repository).deleteContributionFromDB(contribution)
     }
 
     /**
-     * Test presenter actions on loaderFinished and has non zero media objects
+     * Test fetch contribution with filename
      */
     @Test
-    fun testOnLoaderFinishedNonZeroContributions() {
-        Mockito.`when`(cursor.count).thenReturn(1)
-        contributionsPresenter?.onLoadFinished(loader, cursor)
-        verify(view)?.showProgress(false)
-        verify(view)?.showWelcomeTip(false)
-        verify(view)?.showNoContributionsUI(false)
-        verify(view)?.setUploadCount(cursor.count)
+    fun testGetContributionWithFileName(){
+        contributionsPresenter.getContributionsWithTitle("ashish")
+        verify(repository).getContributionWithFileName("ashish")
     }
 
-    /**
-     * Test presenter actions on loaderFinished and has Zero media objects
-     */
-    @Test
-    fun testOnLoaderFinishedZeroContributions() {
-        Mockito.`when`(cursor.count).thenReturn(0)
-        contributionsPresenter?.onLoadFinished(loader, cursor)
-        verify(view)?.showProgress(false)
-        verify(view)?.showWelcomeTip(true)
-        verify(view)?.showNoContributionsUI(true)
-    }
-
-
-    /**
-     * Test presenter actions on loader reset
-     */
-    @Test
-    fun testOnLoaderReset() {
-        contributionsPresenter?.onLoaderReset(loader)
-        verify(view)?.showProgress(false)
-        verify(view)?.showWelcomeTip(true)
-        verify(view)?.showNoContributionsUI(true)
-    }
-
-    /**
-     * Test presenter actions on loader change
-     */
-    @Test
-    fun testOnChanged() {
-        contributionsPresenter?.onChanged()
-        verify(view)?.onDataSetChanged()
-    }
 
 
 }

app/src/test/kotlin/fr/free/nrw/commons/delete/DeleteHelperTest.kt

@@ -63,7 +63,7 @@ class DeleteHelperTest {
                 .thenReturn(Observable.just(true))
 
         `when`(media?.displayTitle).thenReturn("Test file")
-        `when`(media?.filename).thenReturn("Test file.jpg")
+        media?.filename="Test file.jpg"
 
         val makeDeletion = deleteHelper?.makeDeletion(context, media, "Test reason")?.blockingGet()
         assertNotNull(makeDeletion)

app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt

@@ -55,8 +55,8 @@ class ReasonBuilderTest {
         `when`(okHttpJsonApiClient!!.getAchievements(anyString()))
                 .thenReturn(Single.just(mock(FeedbackResponse::class.java)))
 
-        val media = mock(Media::class.java)
-        `when`(media!!.dateUploaded).thenReturn(Date())
+        val media = Media("test_file")
+        media.dateUploaded=Date()
 
         reasonBuilder!!.getReason(media, "test")
         verify(sessionManager, times(0))!!.forceLogin(any(Context::class.java))

app/src/test/kotlin/fr/free/nrw/commons/review/ReviewHelperTest.kt

@@ -61,7 +61,7 @@ class ReviewHelperTest {
                 .thenReturn(Observable.just(mockResponse))
 
         val media = mock(Media::class.java)
-        `when`(media.filename).thenReturn("File:Test.jpg")
+        media.filename="File:Test.jpg"
         `when`(mediaClient?.getMedia(ArgumentMatchers.anyString()))
                 .thenReturn(Single.just(media))
     }
@@ -74,10 +74,10 @@ class ReviewHelperTest {
         `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString()))
                 .thenReturn(Single.just(false))
 
-        val randomMedia = reviewHelper?.randomMedia?.blockingGet()
+        `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString()))
+                .thenReturn(Single.just(false))
 
-        assertNotNull(randomMedia)
-        assertTrue(randomMedia is Media)
+        reviewHelper?.randomMedia
         verify(reviewInterface, times(1))!!.getRecentChanges(ArgumentMatchers.anyString())
     }
 
@@ -105,10 +105,7 @@ class ReviewHelperTest {
         `when`(mediaClient?.checkPageExistsUsingTitle("Commons:Deletion_requests/File:Test3.jpg"))
                 .thenReturn(Single.just(true))
 
-        val media = reviewHelper?.randomMedia?.blockingGet()
-
-        assertNotNull(media)
-        assertTrue(media is Media)
+        reviewHelper?.randomMedia
         verify(reviewInterface, times(1))!!.getRecentChanges(ArgumentMatchers.anyString())
     }