From 1fde8bbc6e44f72fac8dc92bdecf302cd9c6f91d Mon Sep 17 00:00:00 2001 From: AmandaRiu Date: Thu, 20 Jun 2019 13:04:24 -0600 Subject: [PATCH 1/3] Revert foreign key constraint added in a recent PR --- .../notifications/NotificationSqlUtilsTest.kt | 217 +++++++++--------- .../fluxc/persistence/NotificationSqlUtils.kt | 5 - 2 files changed, 113 insertions(+), 109 deletions(-) diff --git a/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt b/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt index e75d8b2f4f..59c22007d3 100644 --- a/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt +++ b/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt @@ -17,7 +17,6 @@ import org.wordpress.android.fluxc.model.notification.NoteIdSet import org.wordpress.android.fluxc.network.rest.wpcom.notifications.NotificationApiResponse import org.wordpress.android.fluxc.persistence.NotificationSqlUtils import org.wordpress.android.fluxc.persistence.NotificationSqlUtils.NotificationModelBuilder -import org.wordpress.android.fluxc.persistence.SiteSqlUtils import org.wordpress.android.fluxc.tools.FormattableContentMapper import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -26,33 +25,27 @@ import kotlin.test.assertNull @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner::class) class NotificationSqlUtilsTest { - // All notifications require a matching [SiteModel] in the database in order to save - // to the database due to a foreign key constraint - private val site = SiteModel().apply { - name = "Unit Test Site" - url = "https://www.mytestsite.com" - } - @Before fun setUp() { val appContext = RuntimeEnvironment.application.applicationContext - val config = SingleStoreWellSqlConfigForTests( - appContext, - listOf(NotificationModelBuilder::class.java, SiteModel::class.java), - "") + val config = SingleStoreWellSqlConfigForTests(appContext, listOf(NotificationModelBuilder::class.java)) WellSql.init(config) config.reset() - - // Save the test [SiteModel] to the db - saveSiteToDb(site) } @Test fun testInsertOrUpdateNotifications() { - // Test inserting notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) - val notesList = notificationSqlUtils.getNotifications() + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val notesList = apiResponse.notes?.map { + NotificationApiResponse.notificationResponseToNotificationModel(it, 0) + } ?: emptyList() + + // Test inserting notifications + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Test updating notifications val newNote = notesList[0].copy(noteId = -1, remoteNoteId = 333) @@ -68,7 +61,14 @@ class NotificationSqlUtilsTest { fun testGetNotifications() { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val notesList = apiResponse.notes?.map { + NotificationApiResponse.notificationResponseToNotificationModel(it, 0) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Get notifications val notifications = notificationSqlUtils.getNotifications(SelectQuery.ORDER_DESCENDING) @@ -79,11 +79,20 @@ class NotificationSqlUtilsTest { fun testGetNotificationsForSite() { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } + val notesList = apiResponse.notes?.map { + val siteId = NotificationApiResponse.getRemoteSiteId(it) + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Get notifications val notifications = notificationSqlUtils.getNotificationsForSite(site, SelectQuery.ORDER_DESCENDING) - assertEquals(6, notifications.size) + assertEquals(3, notifications.size) } @Test @@ -93,6 +102,7 @@ class NotificationSqlUtilsTest { val jsonString = UnitTestUtils .getStringFromResourceFile(this.javaClass, "notifications/store-order-notification.json") val apiResponse = NotificationTestUtils.parseNotificationApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 141286411 } val notesList = listOf(NotificationApiResponse.notificationResponseToNotificationModel(apiResponse, site.id)) val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } assertEquals(1, inserted) @@ -105,7 +115,7 @@ class NotificationSqlUtilsTest { val note = notifications[0] assertEquals(note.title, "New Order") assertEquals(note.noteHash, 2064099309) - assertEquals(note.localSiteId, site.id) + assertEquals(note.localSiteId, 141286411) assertEquals(note.remoteNoteId, 3604874081) assertEquals(note.type, NotificationModel.Kind.STORE_ORDER) assertEquals(note.read, true) @@ -172,6 +182,7 @@ class NotificationSqlUtilsTest { val jsonString = UnitTestUtils .getStringFromResourceFile(this.javaClass, "notifications/store-review-notification.json") val apiResponse = NotificationTestUtils.parseNotificationApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } val notesList = listOf(NotificationApiResponse.notificationResponseToNotificationModel(apiResponse, site.id)) val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } assertEquals(1, inserted) @@ -186,7 +197,7 @@ class NotificationSqlUtilsTest { val note = notifications[0] assertEquals(note.noteHash, 1543255567) assertEquals(note.title, "Product Review") - assertEquals(note.localSiteId, site.id) + assertEquals(note.localSiteId, 153482281) assertEquals(note.remoteNoteId, 3617558725) assertEquals(note.type, NotificationModel.Kind.COMMENT) assertEquals(note.read, true) @@ -205,7 +216,14 @@ class NotificationSqlUtilsTest { fun testGetNotifications_filterBy() { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val notesList = apiResponse.notes?.map { + NotificationApiResponse.notificationResponseToNotificationModel(it, 0) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Get notifications of type "store_order" val newOrderNotifications = notificationSqlUtils.getNotifications( @@ -226,43 +244,40 @@ class NotificationSqlUtilsTest { @Test fun testGetNotificationsForSite_filterBy() { - // Insert notifications for main test site + // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) - - // Insert notifications for additional test site - val site2 = SiteModel().apply { - name = "Test Site 2" - url = "https://someothertestsite.com" - } - saveSiteToDb(site2) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site2) - - // Verify we have 12 total notifications in the db - val allNotifs = notificationSqlUtils.getNotifications() - assertEquals(12, allNotifs.size) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } + val notesList = apiResponse.notes?.map { + val siteId = NotificationApiResponse.getRemoteSiteId(it) + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Get notifications of type "store_order". // - // Note: FOUR store_order notifications were inserted into the db, but they belong to two - // different sites so only 2 should be returned. + // Note: TWO store_order notifications were inserted into the db, but they belong to two + // different sites so only 1 should be returned. val newOrderNotifications = notificationSqlUtils.getNotificationsForSite( - site2, + site, filterByType = listOf(NotificationModel.Kind.STORE_ORDER.toString())) - assertEquals(2, newOrderNotifications.size) + assertEquals(1, newOrderNotifications.size) // Get notifications of subtype "store_review" val storeReviewNotifications = notificationSqlUtils.getNotificationsForSite( - site2, + site, filterBySubtype = listOf(NotificationModel.Subkind.STORE_REVIEW.toString())) assertEquals(2, storeReviewNotifications.size) // Get notifications of type "store_order" or subtype "store_review" val combinedNotifications = notificationSqlUtils.getNotificationsForSite( - site2, + site, filterByType = listOf(NotificationModel.Kind.STORE_ORDER.toString()), filterBySubtype = listOf(NotificationModel.Subkind.STORE_REVIEW.toString())) - assertEquals(4, combinedNotifications.size) + assertEquals(3, combinedNotifications.size) } @Test @@ -271,16 +286,25 @@ class NotificationSqlUtilsTest { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } + val notesList = apiResponse.notes?.map { + val siteId = NotificationApiResponse.getRemoteSiteId(it) + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Fetch a single notification using the noteIdSet val idSet = NoteIdSet(-1, noteId, site.id) val notification = notificationSqlUtils.getNotificationByIdSet(idSet) assertNotNull(notification) - with(notification) { - assertEquals(remoteNoteId, noteId) - assertEquals(localSiteId, site.id) + notification?.let { + assertEquals(it.remoteNoteId, noteId) + assertEquals(it.localSiteId, site.id) } } @@ -290,15 +314,24 @@ class NotificationSqlUtilsTest { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } + val notesList = apiResponse.notes?.map { + val siteId = NotificationApiResponse.getRemoteSiteId(it) + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Fetch a single notification using the remoteNoteId val notification = notificationSqlUtils.getNotificationByRemoteId(noteId) assertNotNull(notification) - with(notification) { - assertEquals(remoteNoteId, noteId) - assertEquals(localSiteId, site.id) + notification?.let { + assertEquals(it.remoteNoteId, noteId) + assertEquals(it.localSiteId, site.id) } } @@ -306,7 +339,14 @@ class NotificationSqlUtilsTest { fun testGetNotificationCount() { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val notesList = apiResponse.notes?.map { + NotificationApiResponse.notificationResponseToNotificationModel(it, 0) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Get notifications val count = notificationSqlUtils.getNotificationsCount() @@ -316,8 +356,16 @@ class NotificationSqlUtilsTest { @Test fun testHasUnreadNotifications() { val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val notesList = apiResponse.notes?.map { + NotificationApiResponse.notificationResponseToNotificationModel(it, 0) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) + val site = SiteModel().apply { id = 0 } val hasUnread = notificationSqlUtils.hasUnreadNotificationsForSite(site) assertEquals(hasUnread, true) } @@ -328,7 +376,16 @@ class NotificationSqlUtilsTest { // Insert notifications val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) + val jsonString = UnitTestUtils + .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") + val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) + val site = SiteModel().apply { id = 153482281 } + val notesList = apiResponse.notes?.map { + val siteId = NotificationApiResponse.getRemoteSiteId(it) + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + } ?: emptyList() + val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } + assertEquals(6, inserted) // Fetch a single notification val notification = notificationSqlUtils.getNotificationByRemoteId(noteId) @@ -341,52 +398,4 @@ class NotificationSqlUtilsTest { // Verify notification not in database assertNull(notificationSqlUtils.getNotificationByRemoteId(noteId)) } - - @Test - fun testDeleteSiteCascadesToNotificationTable() { - // Assemble: - // - Insert notifications for main test site - // - Insert notifications for additional test site - // - Verify we have 12 total notifications in the db - val notificationSqlUtils = NotificationSqlUtils(FormattableContentMapper(Gson())) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site) - - val site2 = SiteModel().apply { - name = "Test Site 2" - url = "https://someothertestsite.com" - } - saveSiteToDb(site2) - generateAndSaveTestNotificationsToDb(notificationSqlUtils, site2) - - val allNotifs = notificationSqlUtils.getNotifications() - assertEquals(12, allNotifs.size) - - // Act: - // - Delete the site from the database - SiteSqlUtils.deleteSite(site2) - - // Assert: - // - Verify all the notifications for site2 have also been deleted from the database - val savedNotifs = notificationSqlUtils.getNotifications() - assertEquals(6, savedNotifs.size) - } - - private fun generateAndSaveTestNotificationsToDb(sqlUtils: NotificationSqlUtils, siteModel: SiteModel) { - val jsonString = UnitTestUtils - .getStringFromResourceFile(this.javaClass, "notifications/notifications-api-response.json") - val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) - val notesList = apiResponse.notes?.map { - NotificationApiResponse.notificationResponseToNotificationModel(it, siteModel.id) - } ?: emptyList() - val inserted = notesList.sumBy { sqlUtils.insertOrUpdateNotification(it) } - assertEquals(6, inserted) - } - - private fun saveSiteToDb(siteModel: SiteModel) { - val siteRowsAffected = SiteSqlUtils.insertOrUpdateSite(siteModel) - assertEquals(1, siteRowsAffected) - val savedSite = SiteSqlUtils.getSitesByNameOrUrlMatching(siteModel.name).firstOrNull() - assertNotNull(savedSite) - assertEquals(siteModel.name, savedSite.name) - } } diff --git a/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/NotificationSqlUtils.kt b/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/NotificationSqlUtils.kt index 58b6097471..55cf659b57 100644 --- a/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/NotificationSqlUtils.kt +++ b/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/NotificationSqlUtils.kt @@ -8,7 +8,6 @@ import com.yarolegovich.wellsql.WellSql import com.yarolegovich.wellsql.core.Identifiable import com.yarolegovich.wellsql.core.annotation.Column import com.yarolegovich.wellsql.core.annotation.PrimaryKey -import com.yarolegovich.wellsql.core.annotation.RawConstraints import com.yarolegovich.wellsql.core.annotation.Table import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.notification.NoteIdSet @@ -208,10 +207,6 @@ class NotificationSqlUtils @Inject constructor(private val formattableContentMap } @Table(name = "NotificationModel") - @RawConstraints( - "FOREIGN KEY(LOCAL_SITE_ID) REFERENCES SiteModel(_id) ON DELETE CASCADE", - "UNIQUE (REMOTE_NOTE_ID, LOCAL_SITE_ID) ON CONFLICT REPLACE" - ) data class NotificationModelBuilder( @PrimaryKey @Column private var mId: Int = -1, @Column var remoteNoteId: Long, From 9680ce81d65e908e0ca5e1f3338ce043544d0f27 Mon Sep 17 00:00:00 2001 From: AmandaRiu Date: Thu, 20 Jun 2019 13:05:30 -0600 Subject: [PATCH 2/3] Add migration strategy to onUpgrade to remove foreign key constraint --- .../android/fluxc/persistence/WellSqlConfig.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.java b/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.java index 1d5549f1b7..7cb88ecff5 100644 --- a/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.java +++ b/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.java @@ -44,7 +44,7 @@ public WellSqlConfig(Context context, @AddOn String... addOns) { @Override public int getDbVersion() { - return 70; + return 71; } @Override @@ -529,6 +529,14 @@ public void onUpgrade(SQLiteDatabase db, WellTableManager helper, int oldVersion AppLog.d(T.DB, "Migrating to version " + (oldVersion + 1)); migrateAddOn(ADDON_WOOCOMMERCE, db, oldVersion); oldVersion++; + case 70: + AppLog.d(T.DB, "Migrating to version " + (oldVersion + 1)); + db.execSQL("DROP TABLE IF EXISTS NotificationModel"); + db.execSQL("CREATE TABLE NotificationModel (_id INTEGER PRIMARY KEY AUTOINCREMENT," + + "REMOTE_NOTE_ID INTEGER,LOCAL_SITE_ID INTEGER,NOTE_HASH INTEGER,TYPE TEXT," + + "SUBTYPE TEXT,READ INTEGER,ICON TEXT,NOTICON TEXT,TIMESTAMP TEXT,URL TEXT," + + "TITLE TEXT,FORMATTABLE_BODY TEXT,FORMATTABLE_SUBJECT TEXT,FORMATTABLE_META TEXT)"); + oldVersion++; } db.setTransactionSuccessful(); db.endTransaction(); From f0001723caa65e6263ae95ecb493ad54911b88be Mon Sep 17 00:00:00 2001 From: AmandaRiu Date: Thu, 20 Jun 2019 13:08:20 -0600 Subject: [PATCH 3/3] Fix lint warnings and update a test to properly use correct site id --- .../notifications/NotificationSqlUtilsTest.kt | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt b/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt index 59c22007d3..10854beb36 100644 --- a/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt +++ b/example/src/test/java/org/wordpress/android/fluxc/notifications/NotificationSqlUtilsTest.kt @@ -302,10 +302,8 @@ class NotificationSqlUtilsTest { val notification = notificationSqlUtils.getNotificationByIdSet(idSet) assertNotNull(notification) - notification?.let { - assertEquals(it.remoteNoteId, noteId) - assertEquals(it.localSiteId, site.id) - } + assertEquals(notification.remoteNoteId, noteId) + assertEquals(notification.localSiteId, site.id) } @Test @@ -329,10 +327,8 @@ class NotificationSqlUtilsTest { val notification = notificationSqlUtils.getNotificationByRemoteId(noteId) assertNotNull(notification) - notification?.let { - assertEquals(it.remoteNoteId, noteId) - assertEquals(it.localSiteId, site.id) - } + assertEquals(notification.remoteNoteId, noteId) + assertEquals(notification.localSiteId, site.id) } @Test @@ -381,8 +377,8 @@ class NotificationSqlUtilsTest { val apiResponse = NotificationTestUtils.parseNotificationsApiResponseFromJsonString(jsonString) val site = SiteModel().apply { id = 153482281 } val notesList = apiResponse.notes?.map { - val siteId = NotificationApiResponse.getRemoteSiteId(it) - NotificationApiResponse.notificationResponseToNotificationModel(it, siteId!!.toInt()) + val siteId = site.id + NotificationApiResponse.notificationResponseToNotificationModel(it, siteId) } ?: emptyList() val inserted = notesList.sumBy { notificationSqlUtils.insertOrUpdateNotification(it) } assertEquals(6, inserted)