Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DatabaseStorageModule not working correctly #3506

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import com.github.spotbugs.snom.SpotBugsTask

import com.github.spotbugs.snom.Confidence
import com.github.spotbugs.snom.Effort
import com.github.spotbugs.snom.SpotBugsTask

apply plugin: 'com.android.application'
apply plugin: 'kotlin-android'
Expand Down Expand Up @@ -91,6 +92,10 @@ android {
}
}

sourceSets {
androidTest.assets.srcDirs += files("$projectDir/schemas".toString())
}

testInstrumentationRunnerArgument "TEST_SERVER_URL", "${NC_TEST_SERVER_BASEURL}"
testInstrumentationRunnerArgument "TEST_SERVER_USERNAME", "${NC_TEST_SERVER_USERNAME}"
testInstrumentationRunnerArgument "TEST_SERVER_PASSWORD", "${NC_TEST_SERVER_PASSWORD}"
Expand Down Expand Up @@ -239,6 +244,7 @@ dependencies {
implementation "androidx.room:room-rxjava2:${roomVersion}"
kapt "androidx.room:room-compiler:${roomVersion}"
implementation "androidx.room:room-ktx:${roomVersion}"
androidTestImplementation "androidx.room:room-testing:2.6.1"

implementation "org.parceler:parceler-api:$parcelerVersion"
implementation 'eu.davidea:flexible-adapter:5.1.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"formatVersion": 1,
"database": {
"version": 9,
"identityHash": "666fcc4bbbdf3ff121b8f1ace8fcbcb8",
"identityHash": "250a3a56f3943f0d72f7ca0aac08fd1e",
rapterjet2004 marked this conversation as resolved.
Show resolved Hide resolved
"entities": [
{
"tableName": "User",
Expand Down Expand Up @@ -92,7 +92,7 @@
},
{
"tableName": "ArbitraryStorage",
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountIdentifier` INTEGER NOT NULL, `key` TEXT NOT NULL, `object` TEXT, `value` TEXT, PRIMARY KEY(`accountIdentifier`, `key`))",
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountIdentifier` INTEGER NOT NULL, `key` TEXT NOT NULL, `object` TEXT NOT NULL, `value` TEXT, PRIMARY KEY(`accountIdentifier`, `key`))",
"fields": [
{
"fieldPath": "accountIdentifier",
Expand All @@ -110,7 +110,7 @@
"fieldPath": "storageObject",
"columnName": "object",
"affinity": "TEXT",
"notNull": false
"notNull": true
},
{
"fieldPath": "value",
Expand All @@ -123,7 +123,8 @@
"autoGenerate": false,
"columnNames": [
"accountIdentifier",
"key"
"key",
"object"
]
},
"indices": [],
Expand All @@ -133,7 +134,7 @@
"views": [],
"setupQueries": [
"CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
"INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '666fcc4bbbdf3ff121b8f1ace8fcbcb8')"
"INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '250a3a56f3943f0d72f7ca0aac08fd1e')"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Nextcloud Talk application
*
* @author Julius Linus
* Copyright (C) 2023 Julius Linus <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package com.nextcloud.talk.migrations

import androidx.room.Room
import androidx.room.testing.MigrationTestHelper
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
import androidx.test.platform.app.InstrumentationRegistry
import com.nextcloud.talk.data.source.local.Migrations
import com.nextcloud.talk.data.source.local.TalkDatabase
import org.junit.Rule
import org.junit.Test
import java.io.IOException

class MigrationsTest {

@get:Rule
val helper: MigrationTestHelper = MigrationTestHelper(
InstrumentationRegistry.getInstrumentation(),
TalkDatabase::class.java.canonicalName!!,
FrameworkSQLiteOpenHelperFactory()
)

@Test
@Throws(IOException::class)
fun migrateAll() {
// Create earliest version of the database.
helper.createDatabase(TEST_DB, 8).apply {
close()
}

// Open latest version of the database. Room validates the schema
// once all migrations execute.
Room.databaseBuilder(
InstrumentationRegistry.getInstrumentation().targetContext,
TalkDatabase::class.java,
TEST_DB
).addMigrations(
Migrations.MIGRATION_6_8,
Migrations.MIGRATION_7_8,
Migrations.MIGRATION_8_9,
Migrations.MIGRATION_9_10
).build().apply {
openHelper.writableDatabase.close()
}
}

companion object {
private const val TEST_DB = "migration-test"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import com.nextcloud.talk.data.storage.model.ArbitraryStorage
import io.reactivex.Maybe

class ArbitraryStorageManager(private val arbitraryStoragesRepository: ArbitraryStoragesRepository) {
fun storeStorageSetting(accountIdentifier: Long, key: String, value: String?, objectString: String?) {
fun storeStorageSetting(accountIdentifier: Long, key: String, value: String?, objectString: String) {
arbitraryStoragesRepository.saveArbitraryStorage(ArbitraryStorage(accountIdentifier, key, objectString, value))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ class ConversationInfoActivity :

override fun onResume() {
super.onResume()

if (databaseStorageModule == null) {
databaseStorageModule = DatabaseStorageModule(conversationUser, conversationToken)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ object Migrations {
}
}

val MIGRATION_9_10 = object : Migration(9, 10) {
override fun migrate(db: SupportSQLiteDatabase) {
Log.i("Migrations", "Migrating 9 to 10")
migrateToTriplePrimaryKeyArbitraryStorage(db)
}
}

fun migrateToRoom(db: SupportSQLiteDatabase) {
db.execSQL(
"CREATE TABLE User_new (" +
Expand Down Expand Up @@ -124,4 +131,29 @@ object Migrations {
// Change the table name to the correct one
db.execSQL("ALTER TABLE ArbitraryStorage_dualPK RENAME TO ArbitraryStorage")
}

fun migrateToTriplePrimaryKeyArbitraryStorage(db: SupportSQLiteDatabase) {
db.execSQL(
"CREATE TABLE ArbitraryStorage_triplePK (" +
rapterjet2004 marked this conversation as resolved.
Show resolved Hide resolved
"accountIdentifier INTEGER NOT NULL, " +
"value TEXT, " +
"\"key\" TEXT NOT NULL, " +
"object TEXT NOT NULL, " +
"PRIMARY KEY(accountIdentifier, \"key\", object)" +
")"
)
// Copy the data
db.execSQL(
"INSERT INTO ArbitraryStorage_triplePK (" +
"accountIdentifier, \"key\", object, value) " +
"SELECT " +
"accountIdentifier, \"key\", object, value " +
"FROM ArbitraryStorage"
)
// Remove the old table
db.execSQL("DROP TABLE ArbitraryStorage")

// Change the table name to the correct one
db.execSQL("ALTER TABLE ArbitraryStorage_triplePK RENAME TO ArbitraryStorage")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import java.util.Locale

@Database(
entities = [UserEntity::class, ArbitraryStorageEntity::class],
version = 9,
version = 10,
exportSchema = true
)
@TypeConverters(
Expand Down Expand Up @@ -94,9 +94,14 @@ abstract class TalkDatabase : RoomDatabase() {

return Room
.databaseBuilder(context.applicationContext, TalkDatabase::class.java, dbName)
// comment out openHelperFactory to view the database entries in Android Studio for debugging
// NOTE: comment out openHelperFactory to view the database entries in Android Studio for debugging
.openHelperFactory(factory)
.addMigrations(Migrations.MIGRATION_6_8, Migrations.MIGRATION_7_8, Migrations.MIGRATION_8_9)
.addMigrations(
Migrations.MIGRATION_6_8,
Migrations.MIGRATION_7_8,
Migrations.MIGRATION_8_9,
Migrations.MIGRATION_9_10
)
.allowMainThreadQueries()
.addCallback(
object : RoomDatabase.Callback() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ class ArbitraryStoragesRepositoryImpl(private val arbitraryStoragesDao: Arbitrar
): Maybe<ArbitraryStorage> {
return arbitraryStoragesDao
.getStorageSetting(accountIdentifier, key, objectString)
.map { ArbitraryStorageMapper.toModel(it) }
.map {
ArbitraryStorageMapper.toModel(it)
}
}

override fun getAll(): Maybe<List<ArbitraryStorageEntity>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ import kotlinx.parcelize.Parcelize
data class ArbitraryStorage(
var accountIdentifier: Long,
var key: String,
var storageObject: String? = null,
var storageObject: String,
var value: String? = null
) : Parcelable
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import androidx.room.Entity
import kotlinx.parcelize.Parcelize

@Parcelize
@Entity(tableName = "ArbitraryStorage", primaryKeys = ["accountIdentifier", "key"])
@Entity(tableName = "ArbitraryStorage", primaryKeys = ["accountIdentifier", "key", "object"])
data class ArbitraryStorageEntity(
@ColumnInfo(name = "accountIdentifier")
var accountIdentifier: Long = 0,
Expand All @@ -35,7 +35,7 @@ data class ArbitraryStorageEntity(
var key: String = "",

@ColumnInfo(name = "object")
var storageObject: String? = null,
var storageObject: String = "",

@ColumnInfo(name = "value")
var value: String? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,4 +251,10 @@ public String getString(String key, String defaultVal) {
public void setMessageExpiration(int messageExpiration) {
this.messageExpiration = messageExpiration;
}

@androidx.annotation.NonNull
public String toString() {
return "Conversation token: " + conversationToken
+ "\nAccount Number: " + accountIdentifier;
}
}
Loading