Closed gino-m closed 5 years ago
Filed https://issuetracker.google.com/issues/129183837 to avoid needing to add create() methods to all @Entity classes, for which we already have AutoValue Builders.
@navinsivakumar @scolsen, for the CREATE, UPDATE, and DELETE queue stored in the local database, do you prefer the -Edit suffix (FeatureEdit, RecordEdit, etc.), -Operation (FeatureOperation, RecordOperation), or something else? Thoughts? There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors. ~Leon Bambrick
FYI, committed first working test to my fork: https://github.com/gino-m/ground-android/commit/24740a2041abc8f239b90c6fcf590e6cd91a46e5
@navinsivakumar, I'll send a PR once db entity classes are in so you can start referencing them in the remote sync impl. Like the *Doc objects I defined for Firebase, these are defined independently of value objects (.vo
package) to allow us to use the best suited model for each layer and to avoid data layer abstractions leaking into the value objects.
@navinsivakumar @scolsen, for the CREATE, UPDATE, and DELETE queue stored in the local database, do you prefer the -Edit suffix (FeatureEdit, RecordEdit, etc.), -Operation (FeatureOperation, RecordOperation), or something else? Thoughts?
Edit is nice because it's shorter, but slightly susceptible to misinterpretation since edit ~= modify in the stricter sense of "alter the properties of an existing entity" (i.e. what "update" means). My natural interpretation of the word 'Edit' doesn't include "create" or "delete", but it's a small adjustment. If we're only talking CRUD operations, I don't think it's that confusing once you encounter its use. I like '-Operation' because it's technically more accurate. Edit really only has the benefit of being shorter, which isn't a big benefit given autocomplete and the other conveniences of IDEs. Here are three other alternatives I can think of:
-Op (as an abbreviation for 'operation' pro: short and sweet, con: it isn't necessarily obvious what this is an abbreviation for.) -Change (similar to edit, but somewhat less narrow in meaning. Still susceptible, I think, to an interpretation that considers it as roughly equivalent to "modify" and misses create and delete). -Action (closer to operation. Might be ambiguous depending on the other worlds of discourse bearing on the context--for example, we might already use this term in relation to UI 'actions' performed by users.)
My vote would go to either -Operation or -Action. -Edit is okay too, if there are other reasons the prior suffixes wouldn't work.
+1 to all of the above. -Action is an overloaded term, and has special meaning in the Android domain. I also agree that Edit could be misinterpreted for just UPDATE operation, and not CREATE or DELETE. As you said, the meaning of -Op might not be immediately obvious. Leaning towards -Operation.
@navinsivakumar wdyt?
I prefer "edit" (with a synonym like "mutation" in second place). "Operation" sounds to me like an object representing a long-running operation.
The though also crossed my mind (Operation vs Mutation). Since there's no strong preference let's keep it as-is in the current designs and branch (-Edit).
Also, are you both cool with the suffix -Entity to differentiate database entities (annotated with @androidx.room.Entity) and shared value objects in our data model (e.g. FeatureEntity vs Feature)? Both will be referenced from the DataRepository, and it would would be a bore to fully qualify their package names everywhere..
Works for me.
Sounds good to me as well!
Semi-related: is the class name of the database entity persisted in some way (e.g. part of the Room schema), or can we refactor that relatively easily (without a db migration) in the event we need change names?
(I am confident enough about Entity as a name that I'm not too worried either way, but want to get a sense for how much of a commitment we are making here.)
Room uses the class name to determine the table name by default; we'll be sure to explicitly specify the table names, so tables will be called "feature", "record", "feature_edit", etc. without the -Entity moniker.
Forking discussion from https://github.com/google/ground-android/pull/44#issuecomment-480110666:
Thanks for the replies on #44. @scolsen, your understanding is correct, that's what I was proposing. I also share @navinsivakumar's concerns about persisting shared objects from the canonical model in the local db. Let me try to break down the different data models we have today, and how the Db layer might be scoped:
Map<String, Object>
in this layer, since there's no easy way to customize Firebase APIs deserialization of an arbitrary, potentially nested structure. These are only visible inside com.google.gnd.service.firestore
; FirestoreDataService exposes only Value objects in its API.LocalDatabaseService
that coordinates the various DAOs and DB objects, exposing Value objects externally to the DataRespository layer. This would allow us to use whatever representation is convenient for local db persistence internally (e.g., JSONObject
, String
, or Map<String, Object>
), as long as won't expose those details, and map to the shared value object.Does this make more sense?
FYI @navinsivakumar, We'll likely need to maintain both local and remote ids in the local datastore. Unique IDs can be generated while offline by adding a document via the Firestore API (https://firebase.google.com/docs/firestore/manage-data/add-data), which in theory we shouldn't be calling from the local db store layer. My next PRs will include two IDs to keep the two layers (local and remote stores) entirely independent.
I have local WIP to write Records and Record mutations to the local db, but will need to migrate other functionality to local db first. Potential order of sub-tasks:
To peek at local db:
Start emulator using a debug (not prod) system image: in AVD manager, Create Virtual Devices, then select a Google APIs image (not Google Play) from "x86 images".
You can then access the db directly via command-line shell as per https://stackoverflow.com/a/18280430/9986466:
# Switch to root.
su
# Access the db.
sqlite3 /data/data/com.google.android.gnd/databases/gnd.db
# Run queries.
select * from feature;
Will need to use Room 2.1.0 to allow Room annotations on AutoValue value objects. This may require updating to Android X: https://stackoverflow.com/questions/44869545/android-room-persistence-library-entity-with-autovalue
Blocking #15, #16, #17, #18.