Search history: Data layer

[ashiagr]

Part of: #752

Description

This PR

• Adds a new table search_history (+ migration) to the database
  • Adds unique indexes to restrict duplicate search term/ podcast/ folder/ episode search history item insertion
  • Includes podcast/ folder/ episode search history item details as embedded objects (with minimum details to render corresponding search history item) so that search history can be displayed for them even when there's no corresponding local podcast/ folder/ episode in the database.
• Adds SearchHistoryDao for search history item operations
  • findAll
  • insert (or replace)
  • delete
  • deleteAll

Note: Targets a feature branch to allow modifying the search_history table until the feature is ready.

Testing Instructions

Tests

• Ensure that tests in SearchHistoryDaoTest cover important scenarios:
  • Single search history item insert and delete
  • Inserting same search term/ podcast/ folder/ episode search history item replaces the previous item
  • Multiple unique search term/ podcast/ folder/ episode search history items are inserted
  • Last inserted/ replaced search history item is at the top of the search history
  • Delete all search history

Migration

• Install the app from the main branch
• Install the app from this PR branch
• Go to database inspector and notice that search_history table is created

Fresh Install

• Clean install the app from this PR branch
• Go to database inspector and notice that search_history table is created

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md
• I have considered whether it makes sense to add tests for my changes
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
• Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

• with different themes
• with a landscape orientation
• with the device set to have a large display and font size
• for accessibility with TalkBack

[ashiagr]

These fields might change based on how an episode's recent search (logged when user taps an item from the episode search results) is displayed.

[mchowning]

Am I understanding correctly that the idea here is that each Search History Item would be for either a Podcast,Folder, or Episode, so the embedded fields for only one of those three would not be null in any given SearchHistoryItem?

If that's right, it might be worth considering at some point using a sealed class for this so we don't have so many nullable fields to deal with in our code. I'm not sure I'd do that at the db level necessarily (what you have here is nice and straightforward), but perhaps when we start using this part of the db, it would be worth creating a class for accessing the database that would convert all of the entries to/from an appropriate sealed class. Very rough pseudo code of what I'm thinking of:

pseudo code

sealed class SearchHistoryEntry(
    val modified: Long,
    /* ... */
) {

    class Podcast(
        modified: Long,
        val author: String,
        // ...
        ) : SearchHistoryEntry(modified /* ... */)

    class Folder(
        modified: Long,
        val color: Int
        // ...
    ): SearchHistoryEntry(modified /* ... */)

    class Episode(
        modified: Long,
        val publishedDate: Date,
        // ...
    ): SearchHistoryEntry(modified, /* ... */)
}

class SearchHistoryDbUtil(private val searchHistoryDao: SearchHistoryDao) {
    suspend fun insert(entry: SearchHistoryEntry) {
        val item: SearchHistoryItem = when (entry) {
            is SearchHistoryEntry.Episode -> // create SearchHistoryItem from SearchHistoryEntry.Episode
            is SearchHistoryEntry.Folder -> // create SearchHistoryItem from SearchHistoryEntry.Folder
            is SearchHistoryEntry.Podcast -> // create SearchHistoryItem from SearchHistoryEntry.Podcast
        }
        searchHistoryDao.insert(item)
    }

    suspend fun findAll(limit: Int = 10): List<SearchHistoryEntry> {
        val entries = searchHistoryDao.findAll(limit)
        return entries.map { it.toEntry() }
    }

    private fun SearchHistoryItem.toEntry(): SearchHistoryEntry {
        // create appropriate SearchHistoryEntry from SearchHistoryItem
    }
}

We may figure out that this is actually a bad idea once we start implementing this, but I just wanted to mention it as a possibility in case you might find that it would actually be useful as you start using the db.

[ashiagr]

Am I understanding correctly that the idea here is that each Search History Item would be for either a Podcast,Folder, or Episode, so the embedded fields for only one of those three would not be null in any given SearchHistoryItem?

That's correct.

If that's right, it might be worth considering at some point using a sealed class for this so we don't have so many nullable fields to deal with in our code. I'm not sure I'd do that at the db level necessarily (what you have here is nice and straightforward), but perhaps when we start using this part of the db, it would be worth creating a class for accessing the database that would convert all of the entries to/from an appropriate sealed class. Very rough pseudo code of what I'm thinking of:

I did try to add relations and different tables for each sub-entity and foreign key and all, but it looked complicated. So I didn't use it at the db level to not over-optimize it considering that embedded objects are small. I like your suggestion to use a sealed class at another point and will use it in another PR.

I might also add a query to keep a limited number of records in the table at a later point.

[mchowning]

Looks good! 👍

[mchowning]

Am I understanding correctly that the idea here is that each Search History Item would be for either a Podcast,Folder, or Episode, so the embedded fields for only one of those three would not be null in any given SearchHistoryItem?

If that's right, it might be worth considering at some point using a sealed class for this so we don't have so many nullable fields to deal with in our code. I'm not sure I'd do that at the db level necessarily (what you have here is nice and straightforward), but perhaps when we start using this part of the db, it would be worth creating a class for accessing the database that would convert all of the entries to/from an appropriate sealed class. Very rough pseudo code of what I'm thinking of:

pseudo code

sealed class SearchHistoryEntry(
    val modified: Long,
    /* ... */
) {

    class Podcast(
        modified: Long,
        val author: String,
        // ...
        ) : SearchHistoryEntry(modified /* ... */)

    class Folder(
        modified: Long,
        val color: Int
        // ...
    ): SearchHistoryEntry(modified /* ... */)

    class Episode(
        modified: Long,
        val publishedDate: Date,
        // ...
    ): SearchHistoryEntry(modified, /* ... */)
}

class SearchHistoryDbUtil(private val searchHistoryDao: SearchHistoryDao) {
    suspend fun insert(entry: SearchHistoryEntry) {
        val item: SearchHistoryItem = when (entry) {
            is SearchHistoryEntry.Episode -> // create SearchHistoryItem from SearchHistoryEntry.Episode
            is SearchHistoryEntry.Folder -> // create SearchHistoryItem from SearchHistoryEntry.Folder
            is SearchHistoryEntry.Podcast -> // create SearchHistoryItem from SearchHistoryEntry.Podcast
        }
        searchHistoryDao.insert(item)
    }

    suspend fun findAll(limit: Int = 10): List<SearchHistoryEntry> {
        val entries = searchHistoryDao.findAll(limit)
        return entries.map { it.toEntry() }
    }

    private fun SearchHistoryItem.toEntry(): SearchHistoryEntry {
        // create appropriate SearchHistoryEntry from SearchHistoryItem
    }
}

We may figure out that this is actually a bad idea once we start implementing this, but I just wanted to mention it as a possibility in case you might find that it would actually be useful as you start using the db.