Skip to content

Commit

Permalink
Remove unused type parameter from DriverProvider.getDriver method
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 348551278
  • Loading branch information
galganif authored and arcs-c3po committed Dec 22, 2020
1 parent b29be67 commit d31b8c6
Show file tree
Hide file tree
Showing 15 changed files with 44 additions and 105 deletions.
3 changes: 1 addition & 2 deletions java/arcs/core/storage/DirectStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,7 @@ class DirectStore<Data : CrdtData, Op : CrdtOperation, T> /* internal */ constru
val driver = CrdtException.requireNotNull(
driverFactory.getDriver(
options.storageKey,
crdtType.crdtModelDataClass,
options.type
crdtType.crdtModelDataClass
) as? Driver<Data>
) { "No driver exists to support storage key ${options.storageKey}" }

Expand Down
9 changes: 3 additions & 6 deletions java/arcs/core/storage/DriverFactory.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package arcs.core.storage

import arcs.core.type.Type
import kotlin.reflect.KClass

interface DriverFactory : ExternalStorageOps {
Expand All @@ -9,8 +8,7 @@ interface DriverFactory : ExternalStorageOps {
*/
suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data>?

/**
Expand Down Expand Up @@ -59,6 +57,5 @@ interface DriverFactory : ExternalStorageOps {
* Fetches a [Driver] of type [Data] given its [storageKey].
*/
suspend inline fun <reified Data : Any> DriverFactory.getDriver(
storageKey: StorageKey,
type: Type
): Driver<Data>? = getDriver(storageKey, Data::class, type)
storageKey: StorageKey
): Driver<Data>? = getDriver(storageKey, Data::class)
4 changes: 1 addition & 3 deletions java/arcs/core/storage/DriverProvider.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package arcs.core.storage

import arcs.core.type.Type
import kotlin.reflect.KClass

/** Provider of information on the [Driver] and characteristics of the storage behind it. */
Expand All @@ -11,8 +10,7 @@ interface DriverProvider {
/** Gets a [Driver] for the given [storageKey] and type [Data] (declared by [dataClass]). */
suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data>

/**
Expand Down
6 changes: 2 additions & 4 deletions java/arcs/core/storage/FixedDriverFactory.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package arcs.core.storage

import arcs.core.common.collectExceptions
import arcs.core.type.Type
import kotlin.reflect.KClass

/**
Expand All @@ -22,11 +21,10 @@ class FixedDriverFactory(

override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data>? {
return providerForStorageKey(storageKey)
?.getDriver(storageKey, dataClass, type)
?.getDriver(storageKey, dataClass)
}

override suspend fun removeAllEntities() {
Expand Down
4 changes: 1 addition & 3 deletions java/arcs/core/storage/driver/Database.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.database.ReferenceWithVersion
import arcs.core.storage.keys.DatabaseStorageKey
import arcs.core.type.Type
import arcs.core.util.Random
import arcs.core.util.TaggedLog
import kotlin.reflect.KClass
Expand Down Expand Up @@ -64,8 +63,7 @@ object DatabaseDriverProvider : DriverProvider {

override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> {
val databaseKey = requireNotNull(storageKey as? DatabaseStorageKey) {
"Unsupported StorageKey: $storageKey for DatabaseDriverProvider"
Expand Down
4 changes: 1 addition & 3 deletions java/arcs/core/storage/driver/RamDiskDriverProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import arcs.core.storage.DriverProvider
import arcs.core.storage.StorageKey
import arcs.core.storage.driver.volatiles.VolatileDriverImpl
import arcs.core.storage.keys.RamDiskStorageKey
import arcs.core.type.Type
import kotlin.reflect.KClass

/**
Expand All @@ -22,8 +21,7 @@ class RamDiskDriverProvider : DriverProvider {

override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> {
require(willSupport(storageKey)) {
"This provider does not support StorageKey: $storageKey"
Expand Down
5 changes: 1 addition & 4 deletions java/arcs/core/storage/driver/VolatileDriverProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import arcs.core.storage.driver.volatiles.VolatileDriverImpl
import arcs.core.storage.driver.volatiles.VolatileMemory
import arcs.core.storage.driver.volatiles.VolatileMemoryImpl
import arcs.core.storage.keys.VolatileStorageKey
import arcs.core.type.Type
import arcs.core.util.TaggedLog
import arcs.core.util.guardedBy
import java.lang.IllegalStateException
import kotlin.reflect.KClass
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
Expand Down Expand Up @@ -62,8 +60,7 @@ class VolatileDriverProvider : DriverProvider {
*/
override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> {
require(storageKey is VolatileStorageKey) {
"Expected VolatileStorageKey, got ${storageKey::class.simpleName}"
Expand Down
4 changes: 1 addition & 3 deletions java/arcs/core/storage/driver/testutil/SlowRamDisk.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import arcs.core.storage.driver.volatiles.VolatileEntry
import arcs.core.storage.driver.volatiles.VolatileMemory
import arcs.core.storage.driver.volatiles.VolatileMemoryImpl
import arcs.core.storage.keys.RamDiskStorageKey
import arcs.core.type.Type
import kotlin.reflect.KClass

/**
Expand All @@ -27,8 +26,7 @@ class SlowRamDiskDriverProvider(

override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> = VolatileDriverImpl.create(storageKey, memory)

override suspend fun removeAllEntities() = Unit
Expand Down
4 changes: 1 addition & 3 deletions java/arcs/core/storage/testutil/DriverMocks.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@ package arcs.core.storage.testutil
import arcs.core.storage.Driver
import arcs.core.storage.DriverProvider
import arcs.core.storage.StorageKey
import arcs.core.type.Type
import kotlin.reflect.KClass

class MockDriverProvider : DriverProvider {
override fun willSupport(storageKey: StorageKey): Boolean = true

override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> = MockDriver(storageKey)

override suspend fun removeAllEntities() = Unit
Expand Down
4 changes: 1 addition & 3 deletions java/arcs/core/storage/testutil/FakeDriverProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package arcs.core.storage.testutil
import arcs.core.storage.Driver
import arcs.core.storage.DriverProvider
import arcs.core.storage.StorageKey
import arcs.core.type.Type
import kotlin.reflect.KClass

/**
Expand All @@ -30,8 +29,7 @@ class FakeDriverProvider(
@Suppress("UNCHECKED_CAST")
override suspend fun <Data : Any> getDriver(
storageKey: StorageKey,
dataClass: KClass<Data>,
type: Type
dataClass: KClass<Data>
): Driver<Data> {
require(storageKey in storageKeys)
return drivers[storageKey] as Driver<Data>
Expand Down
10 changes: 4 additions & 6 deletions javatests/arcs/core/storage/FixedDriverFactoryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import arcs.core.storage.testutil.DummyStorageKey
import arcs.core.storage.testutil.FakeDriverProvider
import arcs.core.testutil.CallbackChoreographer
import arcs.core.testutil.assertSuspendingThrows
import arcs.core.type.Type
import com.google.common.truth.Truth.assertThat
import com.nhaarman.mockitokotlin2.mock
import kotlinx.atomicfu.atomic
Expand Down Expand Up @@ -39,7 +38,6 @@ class FixedDriverFactoryTest {
storageKey3 to mockDriver3
)

private val mockType: Type = mock {}
private val fakeClass = CrdtData::class

@Test
Expand All @@ -64,16 +62,16 @@ class FixedDriverFactoryTest {
fun getDriver_withMatch_returnsFirstMatching() = runBlockingTest {
val factory = FixedDriverFactory(listOf(provider1, provider2, provider3))

assertThat(factory.getDriver(storageKey1, fakeClass, mockType)).isEqualTo(mockDriver1)
assertThat(factory.getDriver(storageKey2, fakeClass, mockType)).isEqualTo(mockDriver2)
assertThat(factory.getDriver(storageKey3, fakeClass, mockType)).isEqualTo(mockDriver3)
assertThat(factory.getDriver(storageKey1, fakeClass)).isEqualTo(mockDriver1)
assertThat(factory.getDriver(storageKey2, fakeClass)).isEqualTo(mockDriver2)
assertThat(factory.getDriver(storageKey3, fakeClass)).isEqualTo(mockDriver3)
}

@Test
fun getDriver_withoutMatch_returnsNull() = runBlockingTest {
val factory = FixedDriverFactory(listOf(provider1))

assertThat(factory.getDriver(storageKey2, fakeClass, mockType)).isNull()
assertThat(factory.getDriver(storageKey2, fakeClass)).isNull()
}

@Test
Expand Down
7 changes: 2 additions & 5 deletions javatests/arcs/core/storage/RawEntityDereferencerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import arcs.core.common.Referencable
import arcs.core.crdt.CrdtEntity
import arcs.core.crdt.CrdtEntity.Reference.Companion.buildReference
import arcs.core.crdt.VersionMap
import arcs.core.data.EntityType
import arcs.core.data.FieldType
import arcs.core.data.RawEntity
import arcs.core.data.Schema
Expand Down Expand Up @@ -89,12 +88,10 @@ class RawEntityDereferencerTest {
)

aliceDriver = DefaultDriverFactory.get().getDriver(
backingKey.childKeyWithComponent("aliceId"),
EntityType(schema)
backingKey.childKeyWithComponent("aliceId")
)!!
bobDriver = DefaultDriverFactory.get().getDriver(
backingKey.childKeyWithComponent("bobId"),
EntityType(schema)
backingKey.childKeyWithComponent("bobId")
)!!

aliceDriver.send(CrdtEntity.Data(VersionMap("alice" to 1), alice, referenceBuilder), 1)
Expand Down
28 changes: 6 additions & 22 deletions javatests/arcs/core/storage/driver/DatabaseDriverProviderTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@ import arcs.core.common.ArcId
import arcs.core.crdt.CrdtEntity
import arcs.core.crdt.CrdtSet
import arcs.core.crdt.CrdtSingleton
import arcs.core.data.CollectionType
import arcs.core.data.EntityType
import arcs.core.data.FieldType
import arcs.core.data.Schema
import arcs.core.data.SchemaFields
import arcs.core.data.SchemaName
import arcs.core.data.SingletonType
import arcs.core.storage.StorageKey
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.keys.DatabaseStorageKey
Expand Down Expand Up @@ -84,7 +81,7 @@ class DatabaseDriverProviderTest {
val volatile = VolatileStorageKey(ArcId.newForTest("myarc"), "foo")

assertSuspendingThrows(IllegalArgumentException::class) {
provider.getDriver(volatile, CrdtEntity.Data::class, DUMMY_ENTITY_TYPE)
provider.getDriver(volatile, CrdtEntity.Data::class)
}
}

Expand All @@ -93,7 +90,7 @@ class DatabaseDriverProviderTest {
val key = DatabaseStorageKey.Persistent("foo", "1234a")

assertSuspendingThrows(IllegalArgumentException::class) {
provider.getDriver(key, CrdtEntity.Data::class, DUMMY_ENTITY_TYPE)
provider.getDriver(key, CrdtEntity.Data::class)
}
}

Expand All @@ -103,7 +100,7 @@ class DatabaseDriverProviderTest {
schemaHashLookup["1234a"] = DUMMY_SCHEMA

assertSuspendingThrows(IllegalArgumentException::class) {
provider.getDriver(key, Int::class, DUMMY_ENTITY_TYPE)
provider.getDriver(key, Int::class)
}
}

Expand All @@ -112,27 +109,15 @@ class DatabaseDriverProviderTest {
val key = DatabaseStorageKey.Persistent("foo", "1234a")
schemaHashLookup["1234a"] = DUMMY_SCHEMA

val entityDriver = provider.getDriver(
key,
CrdtEntity.Data::class,
DUMMY_ENTITY_TYPE
)
val entityDriver = provider.getDriver(key, CrdtEntity.Data::class)
assertThat(entityDriver).isInstanceOf(DatabaseDriver::class.java)
assertThat(entityDriver.storageKey).isEqualTo(key)

val setDriver = provider.getDriver(
key,
CrdtSet.DataImpl::class,
CollectionType(DUMMY_ENTITY_TYPE)
)
val setDriver = provider.getDriver(key, CrdtSet.DataImpl::class)
assertThat(setDriver).isInstanceOf(DatabaseDriver::class.java)
assertThat(setDriver.storageKey).isEqualTo(key)

val singletonDriver = provider.getDriver(
key,
CrdtSingleton.DataImpl::class,
SingletonType(DUMMY_ENTITY_TYPE)
)
val singletonDriver = provider.getDriver(key, CrdtSingleton.DataImpl::class)
assertThat(singletonDriver).isInstanceOf(DatabaseDriver::class.java)
assertThat(singletonDriver.storageKey).isEqualTo(key)
}
Expand All @@ -149,6 +134,5 @@ class DatabaseDriverProviderTest {
),
"1234a"
)
private val DUMMY_ENTITY_TYPE = EntityType(DUMMY_SCHEMA)
}
}
20 changes: 6 additions & 14 deletions javatests/arcs/core/storage/driver/RamDiskDriverProviderTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import arcs.core.storage.CapabilitiesResolver
import arcs.core.storage.StorageKey
import arcs.core.storage.keys.RamDiskStorageKey
import arcs.core.storage.keys.VolatileStorageKey
import arcs.core.type.Tag
import arcs.core.type.Type
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.runBlocking
import org.junit.After
Expand Down Expand Up @@ -68,7 +66,7 @@ class RamDiskDriverProviderTest {
fun getDriver_throwsOnInvalidKey() = runBlocking {
val volatile = VolatileStorageKey(ArcId.newForTest("myarc"), "foo")

provider.getDriver(volatile, Int::class, DummyType)
provider.getDriver(volatile, Int::class)
Unit
}

Expand All @@ -78,9 +76,9 @@ class RamDiskDriverProviderTest {

val key = RamDiskStorageKey("foo")

val driver1 = provider.getDriver(key, Int::class, DummyType)
val driver2 = provider.getDriver(key, Int::class, DummyType)
val driver3 = provider2.getDriver(key, Int::class, DummyType)
val driver1 = provider.getDriver(key, Int::class)
val driver2 = provider.getDriver(key, Int::class)
val driver3 = provider2.getDriver(key, Int::class)

var driver2Value: Int? = null
var driver2Version: Int? = null
Expand All @@ -107,7 +105,7 @@ class RamDiskDriverProviderTest {
@Test
fun removeAllEntities() = runBlocking {
val key = RamDiskStorageKey("foo")
val driver = provider.getDriver(key, Int::class, DummyType)
val driver = provider.getDriver(key, Int::class)
driver.send(42, 1)

provider.removeAllEntities()
Expand All @@ -119,18 +117,12 @@ class RamDiskDriverProviderTest {
@Test
fun removeEntitiesBetween() = runBlocking {
val key = RamDiskStorageKey("foo")
val driver = provider.getDriver(key, Int::class, DummyType)
val driver = provider.getDriver(key, Int::class)
driver.send(42, 1)

provider.removeEntitiesCreatedBetween(1, 2)

// Receiver are not updated, so check memory directly.
assertThat(RamDisk.memory.contains(key)).isFalse()
}

companion object {
object DummyType : Type {
override val tag = Tag.Count
}
}
}
Loading

0 comments on commit d31b8c6

Please sign in to comment.