Merge pull request #24 from ty1824/improveInktMessaging

Add tests for QueryDatabase

build.gradle.kts

@@ -1,5 +1,4 @@
 import java.time.LocalDateTime
-import java.net.URI
 
 plugins {
     idea

dialector-kt/build.gradle.kts

@@ -34,22 +34,6 @@ kover {
     }
 }
 
-//val dokkaOutputDir = "$buildDir/dokka"
-//
-//tasks.getByName("dokkaHtml", DokkaTask::class) {
-//    outputDirectory.set(file(dokkaOutputDir))
-//}
-//
-//val deleteDokkaOutputDir by tasks.register<Delete>("deleteDokkaOutputDirectory") {
-//    delete(dokkaOutputDir)
-//}
-//
-//val javadocJar = tasks.register<Jar>("javadocJar") {
-//    dependsOn(deleteDokkaOutputDir, tasks.dokkaHtml)
-//    archiveClassifier.set("javadoc")
-//    from(dokkaOutputDir)
-//}
-
 publishing {
     repositories {
         maven {
@@ -69,7 +53,6 @@ publishing {
             }
         }
     }
-    repositories.forEach { println((it as MavenArtifactRepository).url)}
     publications {
         register<MavenPublication>("default") {
             from(components["java"])

gradle.properties

@@ -1,12 +1,12 @@
 group = dev.dialector
 
 # Modify this for a custom version for local publishing (will not apply to CI publishing builds)
-version = inkt
+version =
 
 kotlinVersion = 1.8.10
 kspVersion = 1.8.10-1.0.9
 koverVersion = 0.6.1
 kotlinterVersion = 3.13.0
 kotlinLoggingVersion = 4.0.0-beta-19
 slf4jVersion = 2.0.6
-dokkaVersion = 1.7.10
+dokkaVersion = 1.7.10

inkt/build.gradle.kts

@@ -28,6 +28,14 @@ tasks.withType<Test> {
 }
 
 kover {
+    filters {
+        classes {
+            excludes += listOf(
+                "dev.dialector.inkt.example.*"
+            )
+        }
+    }
+
     xmlReport {
         onCheck.set(true)
     }

inkt/src/main/kotlin/dev/dialector/inkt/next/QueryDatabase.kt

@@ -50,16 +50,36 @@ public interface QueryDatabase {
 public fun <K : Any, V> QueryDatabase.query(definition: QueryDefinition<K, V>, key: K): V =
     readTransaction { query(definition, key) }
 
+/**
+ * Convenience function to execute a single query in a read transaction.
+ */
+public fun <V> QueryDatabase.query(definition: QueryDefinition<Unit, V>): V =
+    readTransaction { query(definition) }
+
 /**
  * Convenience function to set a single query value in a write transaction.
  */
 public fun <K : Any, V> QueryDatabase.set(definition: QueryDefinition<K, V>, key: K, value: V) {
     writeTransaction { set(definition, key, value) }
 }
 
+/**
+ * Convenience function to set a single query value in a write transaction.
+ */
+public fun <V> QueryDatabase.set(definition: QueryDefinition<Unit, V>, value: V) {
+    writeTransaction { set(definition, value) }
+}
+
 /**
  * Convenience function to remove a single query value in a write transaction.
  */
 public fun <K : Any, V> QueryDatabase.remove(definition: QueryDefinition<K, V>, key: K) {
     writeTransaction { remove(definition, key) }
 }
+
+/**
+ * Convenience function to remove a single query value in a write transaction.
+ */
+public fun <V> QueryDatabase.remove(definition: QueryDefinition<Unit, V>) {
+    writeTransaction { remove(definition) }
+}

inkt/src/main/kotlin/dev/dialector/inkt/next/QueryDatabaseImpl.kt

@@ -80,20 +80,25 @@ public class QueryDatabaseImpl : QueryDatabase {
                 currentValue.value = value
                 currentValue.changedAt = ++currentRevision
             }
-            else -> {
-                queryStorage[key] = InputValue(value, ++currentRevision)
-            }
+            else -> queryStorage[key] = InputValue(value, ++currentRevision)
         }
     }
 
     private fun <K : Any, V> remove(inputDef: QueryDefinition<K, V>, key: K) {
         getQueryStorage(inputDef).remove(key)
+        ++currentRevision
     }
 
+    /**
+     * Obtains a value for a given query def and key
+     */
     private fun <K : Any, V> query(queryDef: QueryDefinition<K, V>, key: K): V = synchronized(lock) {
         fetch(QueryExecutionContextImpl(this), queryDef, key)
     }
 
+    /**
+     * Obtains a value for a given query def and key using an existing QueryExecutionContext
+     */
     private fun <K : Any, V> fetch(context: QueryExecutionContext, queryDef: QueryDefinition<K, V>, key: K): V {
         val queryKey = QueryKey(queryDef, key)
         val queryStorage = getQueryStorage(queryDef)
@@ -154,6 +159,8 @@ public class QueryDatabaseImpl : QueryDatabase {
 
     /**
      * Checks whether a value is up-to-date based on its dependencies.
+     *
+     * Returns true if the value is considered up-to-date, false if it must be recomputed.
      */
     private fun deepVerify(context: QueryExecutionContext, value: Value<*>): Boolean {
         return when (value) {
@@ -164,7 +171,11 @@ public class QueryDatabaseImpl : QueryDatabase {
                 }
 
                 val noDepsChanged = value.dependencies.none { dep ->
-                    get(dep)?.let { maybeChangedAfter(context, dep, it, value.verifiedAt) } ?: true
+                    // If the dependency exists, check if it may have changed.
+                    // If it does not exist, it has "changed" (likely removed) and thus must be recomputed.
+                    get(dep)?.let {
+                        maybeChangedAfter(context, dep, it, value.verifiedAt)
+                    } ?: true
                 }
 
                 if (noDepsChanged) {
@@ -217,7 +228,7 @@ public class QueryDatabaseImpl : QueryDatabase {
         println("=========================")
     }
 
-    internal class QueryExecutionContextImpl(val database: QueryDatabaseImpl) : QueryExecutionContext {
+    internal class QueryExecutionContextImpl(private val database: QueryDatabaseImpl) : QueryExecutionContext {
         private val queryStack: MutableList<QueryFrame<*>> = mutableListOf()
 
         override fun <K : Any, V> query(definition: QueryDefinition<K, V>, key: K): V = database.fetch(this, definition, key)
@@ -227,7 +238,9 @@ public class QueryDatabaseImpl : QueryDatabase {
         override fun pushFrame(key: QueryKey<*, *>) {
             checkCanceled()
             if (queryStack.any { it.queryKey == key }) {
-                throw IllegalStateException("Cycle detected: $key already in ${queryStack.joinToString { it.queryKey.queryDef.name }}")
+                throw IllegalStateException(
+                    "Cycle detected: $key already in ${queryStack.joinToString { it.queryKey.queryDef.name }}"
+                )
             }
             queryStack.add(QueryFrame(key))
         }

inkt/src/main/kotlin/dev/dialector/inkt/next/QueryDslImpl.kt

@@ -4,20 +4,25 @@ import kotlin.properties.PropertyDelegateProvider
 import kotlin.properties.ReadOnlyProperty
 import kotlin.reflect.KProperty
 
+internal fun <K : Any> notImplementedMessage(name: String, key: K) =
+    "Query '$name' not implemented or value not set for key '$key'"
+
 /**
  * A delegate provider that extracts the property name to use as the query name.
  */
 internal class QueryDefinitionInitializer<K : Any, V> internal constructor(
     private val name: String?,
     private val logic: QueryFunction<K, V>?
 ) : PropertyDelegateProvider<Any?, QueryDefinitionDelegate<K, V>> {
-    override operator fun provideDelegate(thisRef: Any?, property: KProperty<*>): QueryDefinitionDelegate<K, V> =
-        QueryDefinitionDelegate(
+    override operator fun provideDelegate(thisRef: Any?, property: KProperty<*>): QueryDefinitionDelegate<K, V> {
+        val actualName = name ?: property.name
+        return QueryDefinitionDelegate(
             QueryDefinitionImpl(
-                name ?: property.name,
-                logic ?: { throw NotImplementedError("Query '$name' not implemented or value not set for key '$it'") }
+                actualName,
+                logic ?: { throw NotImplementedError(notImplementedMessage(actualName, it)) }
             )
         )
+    }
 }
 
 /**

inkt/src/test/kotlin/dev/dialector/inkt/DatabaseTest.kt

@@ -0,0 +1,109 @@
+package dev.dialector.inkt
+
+import dev.dialector.inkt.next.QueryDatabase
+import dev.dialector.inkt.next.QueryDatabaseImpl
+import dev.dialector.inkt.next.QueryDefinition
+import dev.dialector.inkt.next.defineQuery
+import dev.dialector.inkt.next.query
+import dev.dialector.inkt.next.remove
+import dev.dialector.inkt.next.set
+import org.junit.jupiter.api.Test
+import kotlin.test.BeforeTest
+import kotlin.test.assertEquals
+import kotlin.test.assertFails
+
+class DatabaseTest {
+    private val someInput by defineQuery<Int>("daInput")
+    private val someInputTimesTwo by defineQuery<Int>("derived2") { query(someInput) * 2 }
+    private val doubleArgument by defineQuery<Int, Int> { it + it }
+
+    private var transitiveInvokeCount = 0
+    private val transitive by defineQuery<String, Int> { arg ->
+        transitiveInvokeCount++
+        val doubledSomeInput = query(doubleArgument, query(someInput))
+        doubledSomeInput + arg.length
+    }
+
+    private lateinit var database: QueryDatabase
+
+    @BeforeTest
+    fun init() {
+        database = QueryDatabaseImpl()
+        transitiveInvokeCount = 0
+    }
+
+    @Test
+    fun basicExecution() {
+        // Run each query twice to ensure consistency
+        database.set(someInput, 5)
+        assertEquals(5, database.query(someInput))
+        assertEquals(5, database.query(someInput))
+        assertEquals(10, database.query(someInputTimesTwo))
+        assertEquals(10, database.query(someInputTimesTwo))
+        assertEquals(12, database.query(transitive, "hi"))
+        assertEquals(12, database.query(transitive, "hi"))
+        assertEquals(13, database.query(transitive, "hi!"))
+
+        // Verify that the `transitive` query was only invoked twice, once for each unique argument
+        assertEquals(2, transitiveInvokeCount)
+        transitiveInvokeCount = 0
+
+        // Change someInput and repeat
+        database.set(someInput, 100)
+        assertEquals(100, database.query(someInput))
+        assertEquals(100, database.query(someInput))
+        assertEquals(200, database.query(someInputTimesTwo))
+        assertEquals(200, database.query(someInputTimesTwo))
+        assertEquals(202, database.query(transitive, "hi"))
+        assertEquals(202, database.query(transitive, "hi"))
+        assertEquals(203, database.query(transitive, "hi!"))
+        assertEquals(2, transitiveInvokeCount)
+        transitiveInvokeCount = 0
+
+        // All calls should fail after removing dependency
+        database.remove(someInput)
+        assertFails { database.query(someInput) }
+        assertFails { database.query(someInputTimesTwo) }
+        assertFails { database.query(transitive, "hi") }
+        assertFails { database.query(transitive, "hi!") }
+    }
+
+    @Test
+    fun implementationWithExplicitValue() {
+        assertEquals(4, database.query(doubleArgument, 2))
+        assertEquals(6, database.query(doubleArgument, 3))
+
+        // 2 + 2 = 5
+        database.set(doubleArgument, 2, 5)
+        assertEquals(5, database.query(doubleArgument, 2))
+        // Result for 3 should be unchanged
+        assertEquals(6, database.query(doubleArgument, 3))
+
+        database.remove(doubleArgument, 2)
+        assertEquals(4, database.query(doubleArgument, 2))
+        assertEquals(6, database.query(doubleArgument, 3))
+    }
+
+    val cyclicQuery: QueryDefinition<Int, Int> by defineQuery { arg ->
+        arg + query(cyclicQuery, arg)
+    }
+    val possiblyCyclic: QueryDefinition<Int, Int> by defineQuery { arg ->
+        if (arg < 2 && arg % 2 == 1) {
+            arg
+        } else {
+            query(possiblyCyclic, arg % 2)
+        }
+    }
+
+    @Test
+    fun cyclicQueryDetection() {
+        assertFails { database.query(cyclicQuery, 2) }
+
+        assertEquals(1, database.query(possiblyCyclic, 1))
+        assertEquals(1, database.query(possiblyCyclic, 3))
+        assertEquals(1, database.query(possiblyCyclic, 5))
+        assertFails { database.query(possiblyCyclic, 2) }
+        assertFails { database.query(possiblyCyclic, 4) }
+        assertFails { database.query(possiblyCyclic, 8) }
+    }
+}

inkt/src/test/kotlin/dev/dialector/inkt/DslTest.kt

@@ -3,32 +3,51 @@ package dev.dialector.inkt
 import dev.dialector.inkt.next.QueryContext
 import dev.dialector.inkt.next.QueryDefinition
 import dev.dialector.inkt.next.defineQuery
+import dev.dialector.inkt.next.notImplementedMessage
 import org.junit.jupiter.api.assertThrows
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
 class DslTest {
+    private val unimplemented by defineQuery<String, String>()
+    private val argumentDependent by defineQuery<Int, Int> { it + it }
+    private val unimplementedUnit by defineQuery<Int>("daInput")
+    private val derivedUnit by defineQuery<Int>("derived2") { query(unimplementedUnit) * 2 }
+    private val transitive by defineQuery<String, Int> {
+        val first = query(unimplementedUnit)
+        val second = query(argumentDependent, first)
+        second * second + first
+    }
+
     @Test
-    fun testQueryDefinition() {
-        val input by defineQuery<String, String>()
-        val derived by defineQuery<Int, Int> { it + it }
-        val inputUnit by defineQuery<Int>("daInput")
-        val derivedUnit by defineQuery<Int>("derived2") { query(inputUnit) * 2 }
-        val transitive by defineQuery<String, Int> {
-            val first = query(inputUnit)
-            val second = query(derived, first)
-            second * second
-        }
-
-        assertEquals("derived", derived.name)
+    fun testQueryDefinitionName() {
+        assertEquals("argumentDependent", argumentDependent.name)
         assertEquals("derived2", derivedUnit.name)
-        assertEquals("input", input.name)
-        assertEquals("daInput", inputUnit.name)
+        assertEquals("unimplemented", unimplemented.name)
+        assertEquals("daInput", unimplementedUnit.name)
+    }
 
+    @Test
+    fun testBaseQueryExecution() {
         val context = TestContext()
-        assertEquals(4, context.query(derived, 2))
-        assertThrows<NotImplementedError> { context.query(inputUnit) }
-        assertThrows<NotImplementedError> { context.query(transitive, "aha") }
+        assertEquals(4, context.query(argumentDependent, 2))
+    }
+
+    @Test
+    fun testQueryFailures() {
+        val context = TestContext()
+
+        // No-argument query with no implementation throws exception with overridden query name & Unit as key
+        val e1 = assertThrows<NotImplementedError> { context.query(unimplementedUnit) }
+        assertEquals(notImplementedMessage("daInput", Unit), e1.message)
+
+        // Query with no implementation throws exception with the query name & key
+        val e2 = assertThrows<NotImplementedError> { context.query(unimplemented, "name") }
+        assertEquals(notImplementedMessage("unimplemented", "name"), e2.message)
+
+        // Transitive query failure
+        val e3 = assertThrows<NotImplementedError> { context.query(transitive, "aha") }
+        assertEquals(notImplementedMessage("daInput", Unit), e3.message)
     }
 
     class TestContext : QueryContext {