Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (b/140759491).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
diff --git a/room/compiler/src/main/kotlin/androidx/room/log/RLog.kt b/room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
index 1338f21..6df6342 100644
--- a/room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
@@ -18,6 +18,7 @@
package androidx.room.log
+import androidx.room.processor.Context
import androidx.room.vo.Warning
import java.io.StringWriter
import javax.annotation.processing.ProcessingEnvironment
@@ -102,18 +103,12 @@
fun hasErrors() = messages.containsKey(Diagnostic.Kind.ERROR)
- fun writeTo(env: ProcessingEnvironment) {
+ fun writeTo(context: Context) {
+ val printMessage = context.logger.messager::printMessage
messages.forEach { pair ->
val kind = pair.key
pair.value.forEach { (msg, element) ->
- env.messager.printMessage(
- kind,
- if (element != null && element.isFromCompiledClass()) {
- msg.appendElement(env.elementUtils, element)
- } else {
- msg
- },
- element)
+ printMessage(kind, msg, element)
}
}
}
diff --git a/room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt b/room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
index 9bb9c64..a68207a 100644
--- a/room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
@@ -46,30 +46,91 @@
) {
val context = baseContext.fork(executableElement)
+ /**
+ * The processing of the method might happen in multiple steps if we decide to rewrite the
+ * query after the first processing. To allow it while respecting the Context, it is
+ * implemented as a sub procedure in [InternalQueryProcessor].
+ */
fun process(): QueryMethod {
+ val annotation = executableElement.toAnnotationBox(Query::class)?.value
+ context.checker.check(
+ annotation != null, executableElement,
+ ProcessorErrors.MISSING_QUERY_ANNOTATION
+ )
+
+ /**
+ * Run the first process without reporting any errors / warnings as we might be able to
+ * fix them for the developer.
+ */
+ val (initialResult, logs) = context.collectLogs {
+ InternalQueryProcessor(
+ context = it,
+ executableElement = executableElement,
+ dbVerifier = dbVerifier,
+ containing = containing
+ ).processQuery(annotation?.value)
+ }
+ // check if want to swap the query for a better one
+ val finalResult = if (initialResult is ReadQueryMethod) {
+ val rowAdapter = initialResult.queryResultBinder.adapter?.rowAdapter
+ val originalQuery = initialResult.query
+ val finalQuery = rowAdapter?.let {
+ context.queryRewriter?.rewrite(originalQuery, rowAdapter)
+ } ?: originalQuery
+ if (finalQuery != originalQuery) {
+ // ok parse again
+ return InternalQueryProcessor(
+ context = context,
+ executableElement = executableElement,
+ dbVerifier = dbVerifier,
+ containing = containing
+ ).processQuery(finalQuery.original)
+ } else {
+ initialResult
+ }
+ } else {
+ initialResult
+ }
+ if (finalResult == initialResult) {
+ // if we didn't rewrite it, send all logs to the calling context.
+ logs.writeTo(context)
+ }
+ return finalResult
+ }
+}
+
+private class InternalQueryProcessor(
+ val context: Context,
+ val executableElement: ExecutableElement,
+ val containing: DeclaredType,
+ val dbVerifier: DatabaseVerifier? = null
+) {
+ fun processQuery(input: String?): QueryMethod {
val delegate = MethodProcessorDelegate.createFor(context, containing, executableElement)
val returnType = delegate.extractReturnType()
- val annotation = executableElement.toAnnotationBox(Query::class)?.value
- context.checker.check(annotation != null, executableElement,
- ProcessorErrors.MISSING_QUERY_ANNOTATION)
-
- val query = if (annotation != null) {
- val query = SqlParser.parse(annotation.value)
- context.checker.check(query.errors.isEmpty(), executableElement,
- query.errors.joinToString("\n"))
+ val query = if (input != null) {
+ val query = SqlParser.parse(input)
+ context.checker.check(
+ query.errors.isEmpty(), executableElement,
+ query.errors.joinToString("\n")
+ )
validateQuery(query)
- context.checker.check(returnType.kind != TypeKind.ERROR,
- executableElement, ProcessorErrors.CANNOT_RESOLVE_RETURN_TYPE,
- executableElement)
+ context.checker.check(
+ returnType.kind != TypeKind.ERROR,
+ executableElement, ProcessorErrors.CANNOT_RESOLVE_RETURN_TYPE,
+ executableElement
+ )
query
} else {
ParsedQuery.MISSING
}
val returnTypeName = returnType.typeName()
- context.checker.notUnbound(returnTypeName, executableElement,
- ProcessorErrors.CANNOT_USE_UNBOUND_GENERICS_IN_QUERY_METHODS)
+ context.checker.notUnbound(
+ returnTypeName, executableElement,
+ ProcessorErrors.CANNOT_USE_UNBOUND_GENERICS_IN_QUERY_METHODS
+ )
val isPreparedQuery = PREPARED_TYPES.contains(query.type)
val queryMethod = if (isPreparedQuery) {
@@ -82,15 +143,6 @@
}
private fun processQueryMethod(queryMethod: QueryMethod): QueryMethod {
- if (queryMethod is ReadQueryMethod) {
- queryMethod.query.resultInfo?.let { resultInfo ->
- val adapter = queryMethod.queryResultBinder.adapter?.rowAdapter
- if (adapter is PojoRowAdapter) {
- adapter.verifyMapping(context, resultInfo)
- }
- }
- }
-
val missing = queryMethod.sectionToParamMapping
.filter { it.second == null }
.map { it.first.text }
@@ -134,7 +186,8 @@
context.checker.check(
resultBinder.adapter != null,
executableElement,
- ProcessorErrors.cannotFindPreparedQueryResultAdapter(returnType.toString(), query.type))
+ ProcessorErrors.cannotFindPreparedQueryResultAdapter(returnType.toString(), query.type)
+ )
val parameters = delegate.extractQueryParams()
return WriteQueryMethod(
@@ -143,7 +196,8 @@
name = executableElement.simpleName.toString(),
returnType = returnType,
parameters = parameters,
- preparedQueryResultBinder = resultBinder)
+ preparedQueryResultBinder = resultBinder
+ )
}
private fun getQueryMethod(
@@ -156,35 +210,31 @@
context.checker.check(
resultBinder.adapter != null,
executableElement,
- ProcessorErrors.cannotFindQueryResultAdapter(returnType.toString()))
+ ProcessorErrors.cannotFindQueryResultAdapter(returnType.toString())
+ )
val inTransaction = executableElement.hasAnnotation(Transaction::class)
if (query.type == QueryType.SELECT && !inTransaction) {
// put a warning if it is has relations and not annotated w/ transaction
if (rowAdapter is PojoRowAdapter && rowAdapter.relationCollectors.isNotEmpty()) {
- context.logger.w(Warning.RELATION_QUERY_WITHOUT_TRANSACTION,
- executableElement, ProcessorErrors.TRANSACTION_MISSING_ON_RELATION)
+ context.logger.w(
+ Warning.RELATION_QUERY_WITHOUT_TRANSACTION,
+ executableElement, ProcessorErrors.TRANSACTION_MISSING_ON_RELATION
+ )
}
}
val parameters = delegate.extractQueryParams()
- // if we are mapping to a POJO, re-interpret it
- val finalQuery = rowAdapter?.let {
- context.queryRewriter?.rewrite(query, rowAdapter)
- } ?: query
- if (finalQuery != query) {
- // re validate if query has changed
- validateQuery(finalQuery)
- }
return ReadQueryMethod(
element = executableElement,
- query = finalQuery,
+ query = query,
name = executableElement.simpleName.toString(),
returnType = returnType,
parameters = parameters,
inTransaction = inTransaction,
- queryResultBinder = resultBinder)
+ queryResultBinder = resultBinder
+ )
}
companion object {
diff --git a/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt b/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
index c652658..862cb35 100644
--- a/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
@@ -459,6 +459,7 @@
).process()
PojoRowAdapter(
context = subContext,
+ info = resultInfo,
pojo = pojo,
out = typeMirror)
}
@@ -477,6 +478,11 @@
}
}
+ if (rowAdapter != null && rowAdapterLogs?.hasErrors() != true) {
+ rowAdapterLogs?.writeTo(context)
+ return rowAdapter
+ }
+
if ((resultInfo?.columns?.size ?: 1) == 1) {
val singleColumn = findCursorValueReader(typeMirror,
resultInfo?.columns?.get(0)?.type)
@@ -486,7 +492,7 @@
}
// if we tried, return its errors
if (rowAdapter != null) {
- rowAdapterLogs?.writeTo(context.processingEnv)
+ rowAdapterLogs?.writeTo(context)
return rowAdapter
}
if (query.runtimeQueryPlaceholder) {
@@ -500,6 +506,7 @@
).process()
return PojoRowAdapter(
context = context,
+ info = null,
pojo = pojo,
out = typeMirror)
}
diff --git a/room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt b/room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
index 1bd98e9..74d1a49 100644
--- a/room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
@@ -42,67 +42,65 @@
*/
class PojoRowAdapter(
context: Context,
+ private val info: QueryResultInfo?,
val pojo: Pojo,
out: TypeMirror
) : RowAdapter(out) {
+ val mapping: Mapping
+ val relationCollectors: List<RelationCollector>
- var mapping = Mapping(
- matchedFields = pojo.fields,
- unusedColumns = emptyList(),
- unusedFields = emptyList(),
- verified = false
- )
- private set
+ init {
- val relationCollectors: List<RelationCollector> =
- RelationCollector.createCollectors(context, pojo.relations)
-
- fun verifyMapping(context: Context, info: QueryResultInfo) {
// toMutableList documentation is not clear if it copies so lets be safe.
val remainingFields = pojo.fields.mapTo(mutableListOf(), { it })
val unusedColumns = arrayListOf<String>()
val matchedFields: List<Field>
- matchedFields = info.columns.mapNotNull { column ->
- // first check remaining, otherwise check any. maybe developer wants to map the same
- // column into 2 fields. (if they want to post process etc)
- val field = remainingFields.firstOrNull { it.columnName == column.name }
- ?: pojo.findFieldByColumnName(column.name)
- if (field == null) {
- unusedColumns.add(column.name)
- null
- } else {
- remainingFields.remove(field)
- field
+ if (info != null) {
+ matchedFields = info.columns.mapNotNull { column ->
+ // first check remaining, otherwise check any. maybe developer wants to map the same
+ // column into 2 fields. (if they want to post process etc)
+ val field = remainingFields.firstOrNull { it.columnName == column.name }
+ ?: pojo.findFieldByColumnName(column.name)
+ if (field == null) {
+ unusedColumns.add(column.name)
+ null
+ } else {
+ remainingFields.remove(field)
+ field
+ }
}
- }
- if (unusedColumns.isNotEmpty() || remainingFields.isNotEmpty()) {
- val warningMsg = ProcessorErrors.cursorPojoMismatch(
- pojoTypeName = pojo.typeName,
- unusedColumns = unusedColumns,
- allColumns = info.columns.map { it.name },
- unusedFields = remainingFields,
- allFields = pojo.fields
- )
- context.logger.w(Warning.CURSOR_MISMATCH, null, warningMsg)
- }
- val nonNulls = remainingFields.filter { it.nonNull }
- if (nonNulls.isNotEmpty()) {
- context.logger.e(
- ProcessorErrors.pojoMissingNonNull(
+ if (unusedColumns.isNotEmpty() || remainingFields.isNotEmpty()) {
+ val warningMsg = ProcessorErrors.cursorPojoMismatch(
pojoTypeName = pojo.typeName,
- missingPojoFields = nonNulls.map { it.name },
- allQueryColumns = info.columns.map { it.name })
- )
+ unusedColumns = unusedColumns,
+ allColumns = info.columns.map { it.name },
+ unusedFields = remainingFields,
+ allFields = pojo.fields
+ )
+ context.logger.w(Warning.CURSOR_MISMATCH, null, warningMsg)
+ }
+ val nonNulls = remainingFields.filter { it.nonNull }
+ if (nonNulls.isNotEmpty()) {
+ context.logger.e(
+ ProcessorErrors.pojoMissingNonNull(
+ pojoTypeName = pojo.typeName,
+ missingPojoFields = nonNulls.map { it.name },
+ allQueryColumns = info.columns.map { it.name })
+ )
+ }
+ if (matchedFields.isEmpty()) {
+ context.logger.e(ProcessorErrors.cannotFindQueryResultAdapter(out.toString()))
+ }
+ } else {
+ matchedFields = remainingFields.map { it }
+ remainingFields.clear()
}
- if (matchedFields.isEmpty()) {
- context.logger.e(ProcessorErrors.cannotFindQueryResultAdapter(out.toString()))
- }
+ relationCollectors = RelationCollector.createCollectors(context, pojo.relations)
mapping = Mapping(
matchedFields = matchedFields,
unusedColumns = unusedColumns,
- unusedFields = remainingFields,
- verified = true
+ unusedFields = remainingFields
)
}
@@ -120,13 +118,17 @@
override fun onCursorReady(cursorVarName: String, scope: CodeGenScope) {
mapping.fieldsWithIndices = mapping.matchedFields.map {
val indexVar = scope.getTmpVar("_cursorIndexOf${it.name.stripNonJava().capitalize()}")
- val indexMethod = if (mapping.verified) "getColumnIndexOrThrow" else "getColumnIndex"
+ val indexMethod = if (info == null) {
+ "getColumnIndex"
+ } else {
+ "getColumnIndexOrThrow"
+ }
scope.builder().addStatement(
"final $T $L = $T.$L($L, $S)",
TypeName.INT, indexVar, RoomTypeNames.CURSOR_UTIL, indexMethod, cursorVarName,
it.columnName
)
- FieldWithIndex(field = it, indexVar = indexVar, alwaysExists = mapping.verified)
+ FieldWithIndex(field = it, indexVar = indexVar, alwaysExists = info != null)
}
if (relationCollectors.isNotEmpty()) {
relationCollectors.forEach { it.writeInitCode(scope) }
@@ -159,8 +161,7 @@
data class Mapping(
val matchedFields: List<Field>,
val unusedColumns: List<String>,
- val unusedFields: List<Field>,
- internal val verified: Boolean
+ val unusedFields: List<Field>
) {
// set when cursor is ready.
lateinit var fieldsWithIndices: List<FieldWithIndex>
