Room: TypeConverter for Enums [73132006]

Fixed

Feature Request

androidx-bug-bounty

Status Update

No update yet.

Description

wo...@gmail.com

created issue #1

Feb 9, 2018 10:34AM

Currently I have a lot of enums that I use in my entities. However unlike classes like String and Integer, Room does not know how to persist them.
Therefore I have a long of TypeConverters that convert all my different type of enums.

It would be great if you could add a default type converter for enums.

An implementation that works is to use .name() and valueOf() for serialization / de-serialization.

Comments

yb...@google.com <yb...@google.com> #2Feb 9, 2018 04:54PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

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

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

wo...@gmail.com <wo...@gmail.com> #3Feb 9, 2018 05:02PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

tr...@gmail.com <tr...@gmail.com> #4Mar 1, 2018 03:51PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

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

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

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

yb...@google.com <yb...@google.com> Sep 7, 2018 05:44PM

Assigned to ya...@google.com.

yb...@google.com <yb...@google.com> #5Jul 30, 2020 08:25PM

This seems like a possible candidate for external contributions so moving it to the bug bounty hotlist.

We still need a proper design here hence not super straightforward.

Such design should ensure we are:

safe for proguard (in case we go w/ name)
have some logic for defaults (in case an enum is removed or renamed)
possibly support ordinals (though that might be a case where we can require a type converter)
possibly make it opt-in so that developer explicitly picks an option (e.g. some annotation on the enum to enable this)

el...@google.com <el...@google.com> Oct 19, 2020 03:08PM

Reassigned to el...@google.com.

ap...@google.com <ap...@google.com> #6Nov 11, 2020 06:45PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ae093dec7bfdc578cf0d7cd2ee6c8563ddd4f5ed
Author: Elif Bilgin <elifbilgin@google.com>
Date: Wed Oct 21 11:50:07 2020

Optimization for storing enums in databases.

Creating a default type adapter to persist Enums in Room. Changes made in logic of the findQueryParameterAdapter(...) function where we now try to find a TypeConverter for the collection of an object first, and if not, then try to find one for the type arg.

Test: Baseline tests replicate compiler errors in EnumColumnTypeAdapterTest.java
Bug: 73132006
Change-Id: Ia212097c16fde4302bc7c20cbad1604a996090ad

M room/compiler-processing/src/main/java/androidx/room/compiler/processing/XType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspArrayType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspDeclaredType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeArgumentType.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
A room/compiler/src/main/kotlin/androidx/room/solver/types/EnumColumnTypeAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/EnumColumnTypeAdapterTest.java

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

ap...@google.com <ap...@google.com> #7Nov 24, 2020 06:22PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ed8d4acdb13cba24fcb1a392b36952ddfe790776
Author: Elif Bilgin <elifbilgin@google.com>
Date: Wed Oct 21 11:50:07 2020

Optimization for storing enums in databases.

Creating a default type adapter to persist Enums in Room. Changes made in logic of the findQueryParameterAdapter(...) function where we now try to find a TypeConverter for the collection of an object first, and if not, then try to find one for the type arg.

Test: Baseline tests replicate compiler errors in EnumColumnTypeAdapterTest.java
Bug: 73132006
Change-Id: I4cfa8c7f9ac3dc97ec4591ceef6f97b4cba7a6ad

M room/compiler-processing/src/main/java/androidx/room/compiler/processing/XType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacType.kt
M room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspType.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
A room/compiler/src/main/kotlin/androidx/room/solver/types/EnumColumnTypeAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/EnumColumnTypeAdapterTest.java

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

ap...@google.com <ap...@google.com> #8Dec 1, 2020 04:36PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 4eb2d133f3084759df737bc78b07a4ec8594d089
Author: Elif Bilgin <elifbilgin@google.com>
Date: Thu Nov 12 10:09:28 2020

Adding functionality to handle proguard when storing enums in databases.

Test: Baseline tests replicate compiler errors in EnumColumnTypeAdapterTest.java

Bug: 73132006
Change-Id: Ifa0a0bc496d63467a6b54c2c4d264c97a4bde0cd

M room/compiler/src/main/kotlin/androidx/room/ext/javapoet_ext.kt
M room/compiler/src/main/kotlin/androidx/room/solver/types/EnumColumnTypeAdapter.kt
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/EnumColumnTypeAdapterTest.java

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

da...@google.com <da...@google.com> Dec 9, 2020 11:56PM

Marked as fixed.

xi...@robinhood.com <xi...@robinhood.com> #9Aug 31, 2021 10:32PM

Is there a way we can disable this default TypeConverter altogether? Our room DB is quite big and with Room 2.3, we noticed that sometimes we forgot to add our custom Enum converters to @TypeConverters in Room and the default one got picked up which causes unexpected crash. And with the Room default Enum TypeConverter, we are unable to detect such mistake (forgetting to add our custom Enum converters to @TypeConverters ) at compile time.
It would be nice if we can configure our Room DB which just disables Room build-in type converter

yb...@google.com <yb...@google.com> #10Sep 1, 2021 12:17AM

Hmm I'm not super keen on adding a flag to disable it but on the other hand, i understand this new feature may not be great for migration or teams that do want more strict controls.

I think there is not much risk if we add a new flag that is ON by default.

Will discuss w/ the team. Meanwhile, would you be interested in contributing it?

https://github.com/androidx/androidx/blob/androidx-main/CONTRIBUTING.md

yb...@google.com <yb...@google.com> #11Sep 1, 2021 04:57PM

Actually, it wold also be great to know where the automatic one failed for us to better understand the issue.

xi...@robinhood.com <xi...@robinhood.com> #12Sep 2, 2021 11:55PM

Thanks for your reply! Well the automatic one will throw an exception for unrecognized enum values. For example

enum class Sample {
A_DEFAULT, B
}

If we use Room default type converters, it works great until someday we added new field like

enum class Sample {
A_DEFAULT, B, C
}
and now server returns new enum value of C. We want our app to treat unrecognized values as default value (That's how our own custom enum Type Converter are implemented) but Room default type converters will throw exception in such case and crashing our old clients.

Of course we can always specify our custom Type Converter to @TypeConverters list in room db definition. But our code base is huge so sometimes one random developer forgot to add that and the app still compiles and runs until someday we realized that our DB is not using our custom TypeConverter.

We want to disable default type converters so can our app get compile time error if anyone forgot to add the custom Type Converter to room db definition. Thanks!

yb...@google.com <yb...@google.com> #13Sep 3, 2021 03:52PM

Makes sense.

We thought about this during design there wasn't a clear way to solve this. e.g. we could require some "defaultValue" annotation on enums for automatic conversion to work but that also break if developer does not own it.

Maybe we'll iterate on this (open to suggestion). Meanwhile, I'll prototype something to disable this with a flag.

yb...@google.com <yb...@google.com> #14Sep 9, 2021 12:33AM

we are thinking of providing something like this where you can toggle them in the TypeCOnverter anotation. I'm not sure if we can catch 2.5 beta though.

PTAL to see if that seems fine or if you have other suggestions, we are open to it. This feels more scalable than compiler flags that are not discoverable.

xi...@robinhood.com <xi...@robinhood.com> #15Sep 9, 2021 05:42PM

Thanks! It seems fine to me. One small question is what is the differences between State.INHERITED and State.ENABLED in

https://android-review.googlesource.com/c/platform/frameworks/support/+/1821993/1/room/room-common/src/main/java/androidx/room/BuiltInTypeConverters.java#47 ?

yb...@google.com <yb...@google.com> #16Sep 9, 2021 06:12PM

so that is one weird thing we need to resolve.

TypeConverters annotations are scoped such that if you put it on a database , all dao's in that db can use it. if you put it in a dao, the methods in the dao can use it. and if you put an entity, only works on that entity.

This is nested as we traverse the code such that @TypeConverters listed in lower layers get priority over higher layers (e.g. a DAO can define how a Date is converted even though there is a different Date converter in the DB)

so coming back to states, inherited makes it just use whatever my parent set so that you can change the state of 1 of the built in converters (e.g. disable enums but keep UUID) w/o affecting other decisions the database made.

Unfortunately, a boolean is not sufficient because we cannot have null as an option in the annotation.

open for suggestions though, not very happy w/ this to be honest.

yb...@google.com <yb...@google.com> #17Sep 18, 2021 01:06AM

first version of CL ready for disabling: https://android-review.googlesource.com/c/platform/frameworks/support/+/1821993

ap...@google.com <ap...@google.com> #18Sep 21, 2021 07:35PM

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

commit c0811e066e1944ac8319ff5cea8e93f7acd70649
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Sep 08 08:55:09 2021

Add ability to disable built-in converters

This CL adds a new property to the TypeConverters annotation to disable
built in converters in certain scopes. This can be useful when the
developer do not want to use built in converters and rather prefer a
compile time error.

I've also updated XProcessing message assertions to receive a list such
that it can handle cases where multiple messages match the given
message.

Bug: 73132006
Test: BuiltInConverterFlagsTest
Relnote: "We have added a new property to the TypeConverters annotation
to let developers disable built in enum and uuid converters. By default,
these converters are on but you can disable them for a certain scope, or
for the whole database. See TypeConverters documentation for details."

Change-Id: I671e8b1a8eb71d3309da04feaf6f9b18719489fc

M room/room-common/src/test/java/androidx/room/AnnotationRetentionPolicyTest.kt
M room/room-compiler/src/main/kotlin/androidx/room/processor/CustomConverterProcessor.kt
M room/room-compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/room-common/api/current.txt
M room/room-common/api/public_plus_experimental_current.txt
M room/room-compiler/src/main/kotlin/androidx/room/processor/cache/Cache.kt
M room/room-common/src/main/java/androidx/room/TypeConverters.java
A room/room-compiler/src/test/kotlin/androidx/room/solver/BuiltInConverterFlagsTest.kt
M room/room-compiler/src/test/kotlin/androidx/room/solver/BasicColumnTypeAdaptersTest.kt
M room/room-compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/CompilationResultSubject.kt
A room/room-compiler/src/main/kotlin/androidx/room/vo/BuiltInConverterFlags.kt
M room/room-compiler/src/main/kotlin/androidx/room/processor/Context.kt
A room/room-compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/DiagnosticMessagesSubject.kt
M room/room-compiler/src/test/kotlin/androidx/room/solver/TypeConverterStoreTest.kt
M room/room-compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
D room/room-compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/DiagnosticMessageSubject.kt
A room/room-common/src/main/java/androidx/room/BuiltInTypeConverters.java
M room/room-common/api/restricted_current.txt

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

yb...@google.com <yb...@google.com> #19Sep 21, 2021 07:41PM

#15, the new API has landed and should be available in the next alpha. This is coming a bit hot before beta so please take a look to ensure it fixes your need.

Thanks!

xi...@robinhood.com <xi...@robinhood.com> #20Sep 21, 2021 10:57PM

Thanks!
From room page,

https://developer.android.com/jetpack/androidx/releases/room, current alpha is 2.4.0-alpha04. By next alpha, do you mean this will be available in 2.4.0-alpha05?

yb...@google.com <yb...@google.com> #21Sep 21, 2021 11:22PM

Yes. you can also use a snapshot from androidx.dev (they are fairly stable despite being snapshots)

xi...@robinhood.com <xi...@robinhood.com> #22Dec 7, 2021 10:56PM

Just want to loop back on this topic. I tried 2.4.0 RC01 and disabled default Enum type converters and it works like a charm. Thanks for supporting this!