Skip to content

Commit

Permalink
Handle SQL parser errors on update/insert statements and improve hand…
Browse files Browse the repository at this point in the history
…ling of query error output.

Add QueryTableModified to allow the manual specification of which tables are changed by a Query that uses update/insert/delete.
Update documentation on debugging the KSP processor.
  • Loading branch information
mikedawson committed May 24, 2024
1 parent 3125f24 commit e439bb2
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 27 deletions.
9 changes: 7 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,15 @@ Limitations:
* No support for TypeConverter

## Debugging
Use Gradle in debug mode e.g.:

Use the procedure as per the [KSP documentation](https://github.com/google/ksp/blob/main/DEVELOPMENT.md#debug-a-processor-andor-ksp)
e.g.
1) Kill the Gradle daemon and Kotlin compile daemon
```
./gradlew --no-daemon -Dorg.gradle.debug=true build
$ ./gradlew --stop; pkill -f KotlinCompileDaemon
$ ./gradlew door-testdb:jvmJar --rerun-tasks -Dkotlin.daemon.jvm.options="-Xdebug,-Xrunjdwp:transport=dt_socket\,address=8765\,server=y\,suspend=n"
```
2) Attach the IDE's remote debugger to the port as per the argument (e.g. 8765 as above)

## Automatic REST endpoint generation

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ plugins {


group 'com.github.UstadMobile.door'
version '0.75'
version '0.76'

ext.localProperties = new Properties()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ fun TypeSpec.Builder.addDaoRepoFun(
add("\n)")
} else {
addMakeHttpRequestAndInsertReplicationsCode(daoKSFun, daoKSClass, resolver)
addRepoDelegateToDaoCode(daoKSFun, daoKSClass, resolver)
addRepoDelegateToDaoCode(daoKSFun, daoKSClass, resolver, environment)
}


Expand Down Expand Up @@ -486,7 +486,7 @@ fun TypeSpec.Builder.addDaoRepoFun(
}

else -> {
addRepoDelegateToDaoCode(daoKSFun, daoKSClass, resolver)
addRepoDelegateToDaoCode(daoKSFun, daoKSClass, resolver, environment)
}
}
}
Expand Down Expand Up @@ -521,8 +521,9 @@ fun CodeBlock.Builder.addRepoDelegateToDaoCode(
daoFun: KSFunctionDeclaration,
daoKSClass: KSClassDeclaration,
resolver: Resolver,
environment: SymbolProcessorEnvironment,
) : CodeBlock.Builder{
val modifiedTableName = daoFun.getDaoFunctionModifiedTableName(daoKSClass, resolver)
val modifiedTableName = daoFun.getDaoFunctionModifiedTableName(daoKSClass, resolver, environment)

if(daoFun.hasReturnType(resolver))
add("val _result = ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ class DoorValidatorProcessor(
pgConnection = postgresDbUrl?.takeIf { it.isNotBlank() }?.let { dbUrl ->
try {
Class.forName("org.postgresql.Driver")
DriverManager.getConnection(dbUrl, environment.options[AnnotationProcessorWrapper.OPTION_POSTGRES_TESTUSER],
environment.options[AnnotationProcessorWrapper.OPTION_POSTGRES_TESTPASS])
DriverManager.getConnection(
dbUrl,
environment.options[AnnotationProcessorWrapper.OPTION_POSTGRES_TESTUSER],
environment.options[AnnotationProcessorWrapper.OPTION_POSTGRES_TESTPASS]
)
} catch(e: SQLException) {
logger.error("Door: Postgres check database supplied using doordb_postgres_url, but could not connect: ${e.message}")
null
Expand Down Expand Up @@ -416,12 +419,13 @@ class DoorValidatorProcessor(
dao.getAllFunctions().filter { it.hasAnnotation(Query::class) }.forEach { queryFunDecl ->
try {
val queryAnnotation = queryFunDecl.getAnnotation(Query::class)!!
val postgresQuery = queryFunDecl.getAnnotation(PostgresQuery::class)
val queryFun = queryFunDecl.asMemberOf(dao.asType(emptyList()))

//check that the query parameters on both versions match
val sqliteQueryParams = queryAnnotation.value.getSqlQueryNamedParameters()
val postgresQueryParams = (queryFunDecl.getAnnotation(PostgresQuery::class)?.value ?:
queryAnnotation.value.sqlToPostgresSql()).getSqlQueryNamedParameters()
val postgresQueryParams =
(postgresQuery?.value ?: queryAnnotation.value.sqlToPostgresSql()).getSqlQueryNamedParameters()

if(sqliteQueryParams != postgresQueryParams) {
logger.error("Query parameters don't match: " +
Expand All @@ -435,7 +439,7 @@ class DoorValidatorProcessor(
}.forEach { connectionEntry ->
val query = queryAnnotation.value.let {
if(connectionEntry.key == DoorDbType.POSTGRES) {
queryFunDecl.getAnnotation(PostgresQuery::class)?.value ?: it.sqlToPostgresSql()
postgresQuery?.value ?: it.sqlToPostgresSql()
}else {
it
}
Expand Down Expand Up @@ -466,7 +470,11 @@ class DoorValidatorProcessor(
}
}
}catch(e: Exception) {
logger.error("Error running query for DAO function: $e", queryFunDecl)
logger.error(
"Error running query for DAO function \nquery='$query'," +
"\nran='$queryWithQuestionPlaceholders'\n ",
queryFunDecl
)
if(e !is SQLException) {
e.printStackTrace()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.room.Query
import androidx.room.Update
import com.google.devtools.ksp.processing.KSPLogger
import com.google.devtools.ksp.processing.Resolver
import com.google.devtools.ksp.processing.SymbolProcessorEnvironment
import com.google.devtools.ksp.symbol.*
import com.squareup.kotlinpoet.FunSpec
import com.squareup.kotlinpoet.ParameterSpec
Expand All @@ -14,9 +15,9 @@ import com.squareup.kotlinpoet.ksp.toTypeName
import com.squareup.kotlinpoet.ksp.toTypeParameterResolver
import com.ustadmobile.door.annotation.HttpAccessible
import com.ustadmobile.door.annotation.QueryLiveTables
import com.ustadmobile.door.annotation.QueryTableModified
import com.ustadmobile.door.annotation.RepoHttpBodyParam
import com.ustadmobile.lib.annotationprocessor.core.applyIf
import com.ustadmobile.lib.annotationprocessor.core.isSQLAModifyingQuery
import net.sf.jsqlparser.parser.CCJSqlParserUtil
import net.sf.jsqlparser.statement.Statement
import net.sf.jsqlparser.statement.select.Select
Expand Down Expand Up @@ -184,19 +185,41 @@ fun KSFunctionDeclaration.isOverridden(
fun KSFunctionDeclaration.getDaoFunctionModifiedTableName(
daoDeclaration: KSClassDeclaration,
resolver: Resolver,
environment: SymbolProcessorEnvironment,
): String? {
val thisResolved = this.asMemberOf(daoDeclaration.asType(emptyList()))
val queryAnnotation = getAnnotation(Query::class)
return if(hasAnyAnnotation(Update::class, Insert::class)) {
return (thisResolved.firstParamEntityType(resolver).declaration as KSClassDeclaration).entityTableName
}else if(queryAnnotation != null && queryAnnotation.value.isSQLAModifyingQuery()){
val parsed = CCJSqlParserUtil.parse(queryAnnotation.value)
when(parsed) {
is UpdateStatement -> parsed.table.name
is InsertStatement -> parsed.table.name
else -> null
val specifiedTable = getAnnotation(QueryTableModified::class)?.value

return when {
hasAnyAnnotation(Update::class, Insert::class) -> {
(thisResolved.firstParamEntityType(resolver).declaration as KSClassDeclaration).entityTableName
}
specifiedTable != null -> {
specifiedTable
}
else -> {
try {
when(val parsed = CCJSqlParserUtil.parse(queryAnnotation?.value)) {
is UpdateStatement -> parsed.table.name
is InsertStatement -> parsed.table.name
else -> null
}
}catch(e: Throwable) {
if(
queryAnnotation?.value?.trimStart()?.uppercase()?.let {
it.startsWith("INSERT") || it.startsWith("UPDATE") || it.startsWith("DELETE")
} == true
) {
environment.logger.error(
message = "JSQLParser cannot parse INSERT/UPDATE/DELETE QUERY: ${queryAnnotation.value}: ${e.message}\n" +
"Please add @QueryTableModified annotation to specify the table modified",
symbol = this
)
}

null
}
}
}else {
null
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.ustadmobile.door.annotation

@Retention(AnnotationRetention.BINARY)
@Target(AnnotationTarget.FUNCTION)

/**
* Most of the time when a Query modifies a table (e.g. insert, update, delete) the query parser can
* figure it out.
* When it cannot detect the table name, this annotation is used to manually specify the table name.
*/
annotation class QueryTableModified(val value: String)
24 changes: 20 additions & 4 deletions door-testdb/src/commonMain/kotlin/db3/ExampleEntity3Dao.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ package db3

import androidx.room.Insert
import androidx.room.Query
import com.ustadmobile.door.annotation.DoorDao
import com.ustadmobile.door.annotation.HttpAccessible
import com.ustadmobile.door.annotation.HttpServerFunctionCall
import com.ustadmobile.door.annotation.Repository
import com.ustadmobile.door.annotation.*
import kotlinx.coroutines.flow.Flow

@DoorDao
Expand All @@ -18,6 +15,25 @@ expect abstract class ExampleEntity3Dao {
@Insert
abstract fun insert(exampleEntity3: ExampleEntity3): Long

@Query("""
INSERT OR IGNORE INTO ExampleEntity3(cardNumber, name, lastUpdatedTime)
SELECT :cardNumber AS cardNumber,
:name AS name,
:time AS time
""")
@PostgresQuery("""
INSERT INTO ExampleEntity3(cardNumber, name, lastUpdatedTime)
SELECT :cardNumber AS cardNumber,
:name AS name,
:time AS time
ON CONFLICT(eeUid) DO NOTHING
""")
@QueryTableModified("ExampleEntity3")
//This is a test of the symbol processor e.g. to ensure that differentiated queries are validated against the
// correct DB type
@Suppress("unused")
abstract suspend fun insertDifferently(cardNumber: Int, name: String, time: Long)

@Query("""
INSERT INTO OutgoingReplication(destNodeId, orTableId, orPk1, orPk2, orPk3, orPk4)
SELECT :destination AS destNodeId,
Expand Down

0 comments on commit e439bb2

Please sign in to comment.