Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

StoreRoom #338

Merged
merged 12 commits into from
May 30, 2018
Merged

StoreRoom #338

merged 12 commits into from
May 30, 2018

Conversation

digitalbuddha
Copy link
Contributor

@digitalbuddha digitalbuddha commented May 25, 2018

closes #215

This PR is a working implementation of a Store that can use a RoomPersister. While not in its final form I wanted to open the pr to get feedback. I'll annotate the important parts.

Example usage (will move to readme)
example usage:

@Entity
data class User(
        @PrimaryKey(autoGenerate = true)
        var uid: Int = 0,
        val name: String)

@Dao
interface UserDao {
    @Query("SELECT name FROM user")
    fun loadAll(): Flowable<List<String>>

    @Insert
    fun insertAll(vararg users: User)

}

@Database(entities = arrayOf(User::class), version = 1)
abstract class AppDatabase : RoomDatabase() {
    abstract fun userDao(): UserDao
}

val db = Room.databaseBuilder(SampleApp.appContext!!, AppDatabase::class.java, "db").build()


val store = RoomInternalStore(
        Fetcher<User, String> { Single.just(User(name = "Mike")) },
        object : RoomPersister<User, List<String>, String> {

            override fun read(key: String): Observable<List<String>> {
                return db.userDao().loadAll().toObservable()
            }

            override fun write(key: String, user: User) {
                db.userDao().insertAll(user)
            } 
        })

}

private fun RoomSample() {
var foo = store.get("")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see SampleRoomStore.kt for the implementation of this store, biggest takeaway is the ability to treat each store subscription as an endless stream backed by a Room Query. Now anytime you subscribe to get if someone calls store.fresh or if another get call busts through the cache, we will emit to all of the previous get/fresh subscribers. Store becomes truly reactive when backed by room



// File: User.java
@Entity
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple implementation of a RoomDB as a proof of concept


val db = Room.databaseBuilder(SampleApp.appContext!!, AppDatabase::class.java, "db").build()

val persister = object : RoomPersister<User, List<String>, String>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new persister needed to be created that is exposed as an observable rather than a single. The RoomPersister takes a third param as you can have the freedom to have different queries for reading and writing. In this example writing is always done by adding a single user to the DB while read returns the list of all users. Another use case is writing a User but only reading a part of the user



//store
class SampleRoomStore(fetcher: Fetcher<User, String>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing crazy here besides a few interface changes that lead to a RoomInternalStore. TODO: create builders for instantiation




public interface BasePersister {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now have RoomPersister and Persister I made a marker interface for use with StoreUtil for things like checking stale policy or clearing the persister

@@ -58,4 +59,50 @@ private CacheFactory() {
.build();
}
}

public static <Key, Parsed> Cache<Key, Observable<Parsed>> createRoomCache(MemoryPolicy memoryPolicy) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed a new cache implementation that holds observables rather than singles to keep the "live query" setup

Copy link
Contributor

@alexio alexio May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can keep it simple and have a method level generic for the singles/maybes/observables.

private static <Key, Value> Cache<Key, Value> createBaseCache(MemoryPolicy memoryPolicy)

* @param <Parsed> data type after parsing
* <p>
*/
public class RoomInternalStore<Raw, Parsed, Key> implements RoomStore<Parsed, Key> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RoomInternalStore is the meat of it. Notice that we don't need stream or getRefreshing as both of those can be achieved by Room without the need of the helpers. Update a query that a RoomPersister is listening to and get will refresh on its own. If not preferable users can transform the get back into a single

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most logic in this class stayed the same but changed to use Observable rather than Single

*
* @param <Raw> data type before parsing
*/
public abstract class RoomPersister<Raw, Parsed, Key> implements RoomDiskRead<Parsed, Key>, RoomDiskWrite<Raw, Key>, BasePersister {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsers are unnecessary since the DB read query can be a "parsed" version of whatever the written "raw" data is

@digitalbuddha
Copy link
Contributor Author

I'll migrate tests as well to make sure the Room impl works as well as the regular one. I'll also mark this implementation as experimental

apply plugin: 'com.getkeepsafe.dexcount'

apply plugin: 'kotlin-kapt'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woohoo!

buildTools : '27.0.3',
kotlin : '1.1.2-5',

// UI libs.
supportLibs : '25.1.1',
supportLibs : '26.1.0',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be 27 as well...


var appContext: Context? = null
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format and remove EL for this file..

implementation 'com.nytimes.android:cache3:3.0.1'
implementation 'com.nytimes.android:middleware3:3.0.1'
implementation 'com.nytimes.android:filesystem3:3.0.1'
implementation project(':store')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might need to flip before checking in....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do once I can push the new snapshot thanks!

@brianPlummer
Copy link
Contributor

LGTM besides the nitpicks :)

@digitalbuddha
Copy link
Contributor Author

Added prelimanary tests, marked RoomStore as Experimental and added tests, bump rev up to 3.1.0

Pr is ready to merge IMO.

Follow up from me will be a blog post, more tests and a cleanup of the example that is currently living in SampleApp

@tiwiz wake up and review my code :-D

@digitalbuddha digitalbuddha changed the title Room Store StoreRoom May 28, 2018
.subscribe { again() }
}

private fun again() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to more descriptive name?

abstract fun userDao(): UserDao
}

val db = Room.databaseBuilder(SampleApp.appContext!!, AppDatabase::class.java, "db").build()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reimplement to avoid using the !! operator?
Can be as simple as creating a builder that just takes a context.

}

companion object {
var appContext: Context? = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not great, lets avoid having objects create a static reference to themselves?

Copy link
Contributor

@alexio alexio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code quality request changes.

@tiwiz tiwiz merged commit 6ac40cd into nytimes:feature/rx2 May 30, 2018
}

configurations.all {
resolutionStrategy.force 'com.android.support:support-v4:26.1.0'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we're still using support lib v26.1.0 while there is latest one v27.1.1. Any plan to upgrade?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spike: replace file system with Room/sql
6 participants