Handle overriding property in abstract RoomDatabase when generating Kotlin [257967987]

Fixed

Internal Cleanup

Status Update

No update yet.

Description

da...@google.com

created issue #1

Nov 7, 2022 03:47PM

The current implementation is hacky:

the name is figured out by removing the get part
adding the override modifier and getter function has to be done with KotlinPoet, no XPoet for it.
If the property is internal, the override is public and kotlinc complains, but there is no isInternal() API in XProcessing

Comments

da...@google.com <da...@google.com> #2Dec 21, 2022 05:12PM

I was thinking more about these and I am starting to leans toward disallowing property DAO getter or DAO queries. I think they give a false notion that the property is immutable or fixed, that it won't change, but the getter is not pure as in, it can return different results if called at different times. So I think its best to force users to write functions instead when Kotlin code gen is turned off. It also makes our lives easier.

In other words disallow:

@Dao
interface BookDao {
  @Query("SELECT * FROM Books")
  val allBooks: List<Book>
}

da...@google.com <da...@google.com> #3Dec 21, 2022 05:13PM

Correction: Disallow when Kotlin code gen is turned ON

ap...@google.com <ap...@google.com> #4Dec 22, 2022 03:31PM

Project: platform/frameworks/support
Branch: androidx-main

commit aa2b688d0597b37ec9e73904eb68d94f83b83fd3
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Dec 21 17:19:46 2022

Disallow abstract properties as DAO getters or DAO queries in Kotlin codegen.

Queries that are declared as a property and Room overrides the getter function give a false notion that the property is immutable or has a fixed stored result, that it won't change, however the getter is not pure, it can return different results if called at different times based on the data in the database. Therefore, this change disallows them when generating Kotlin.

Even though a DAO property in the abstract Database is a more appropriate case since Room generated code does cache the created DAO instance, it is encoding implementation logic. Kotlin codegen is greatly simplified when disallowed.

Bug: 127483380
Bug: 257967987
Test: DatabaseProcessorTest and DaoProcessorTest
Relnote: Disallow abstract properties as DAO getters or DAO queries in Kotlin codegen, instead they should be rewritten as functions to avoid the false notion that the property value is immutable and has a fixed stored result.
Change-Id: If6a13382b351fbcf9072a40c496d600cd329fd38

M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/AmbiguousColumnResolverTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/InternalsTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/InvalidationTrackerFlowTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/ItemWithNullableConstructor.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/JvmNameInDaoTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/ListenableFuturePagingSourceTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/MultiTypedPagingSourceTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/NullableCollectionQueryParamTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/Rx2PagingSourceTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/Rx3PagingSourceTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/SuspendingQueryTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/SyncTriggersConcurrencyTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/testutil/PagingDb.kt
M room/room-compiler/src/main/kotlin/androidx/room/processor/DaoProcessor.kt
M room/room-compiler/src/main/kotlin/androidx/room/processor/DatabaseProcessor.kt
M room/room-compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
M room/room-compiler/src/main/kotlin/androidx/room/writer/DatabaseWriter.kt
M room/room-compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
M room/room-compiler/src/test/kotlin/androidx/room/processor/DatabaseProcessorTest.kt
M room/room-compiler/src/test/kotlin/androidx/room/writer/DatabaseKotlinCodeGenTest.kt
D room/room-compiler/src/test/test-data/kotlinCodeGen/database_propertyDao.kt
M room/room-compiler/src/test/test-data/kotlinCodeGen/database_withFtsAndView.kt

https://android-review.googlesource.com/2366712

da...@google.com <da...@google.com> Dec 22, 2022 03:35PM

Marked as fixed.

pr...@google.com <pr...@google.com> #5Mar 22, 2023 08:23PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.room:room-compiler:2.6.0-alpha01

Issue 257967987