Implement Feature and Record local db entities and DAOs (#19) #41
Conversation
gino-m
changed the title
gino-m
changed the title
|
Sorry the PR is already getting pretty big. Could you kindly review and pull into The main TODO left from this PR is to implement Record values serialization, which I'll implement using JSON for easy debuggability. Note that this PR only includes DAOs for Feature and Record entities and edits, and not the local cache of Projects. For now the user will need a connection to join the project; the ability to join/leave projects offline shouldn't block testing offline sync for Features and Records though. |
gino-m
changed the title
gnd/src/main/java/com/google/android/gnd/repository/local/Coordinates.java
Show resolved
Hide resolved
gnd/src/main/java/com/google/android/gnd/repository/local/Edit.java
Outdated
Show resolved
Hide resolved
gnd/src/main/java/com/google/android/gnd/repository/local/EntityState.java
Outdated
Show resolved
Hide resolved
gnd/src/main/java/com/google/android/gnd/repository/local/Edit.java
Outdated
Show resolved
Hide resolved
gnd/src/main/java/com/google/android/gnd/repository/local/FeatureEntity.java
Outdated
Show resolved
Hide resolved
gnd/src/main/java/com/google/android/gnd/repository/local/IntEnum.java
Outdated
Show resolved
Hide resolved
| public String key; | ||
|
|
||
| // TODO: Add Converter to convert to/from JSON. | ||
| @Ignore @Nullable public Value oldValue; |
There was a problem hiding this comment.
Aren't the Value fields specific to record edits? Why put them on the shared Edit instead of RecordEditEntity?
There was a problem hiding this comment.
The idea was to use the same mechanism to represent edits on Features and Records to a) allow code sharing and b) to facilitate adding custom attributes to Features and/or pre-defined attributes to Record in the future.
I've removed Value from the db persistence as per our other discussions and using a generic representation analogous to Firestore (JSON) for key-value pairs of changed values. Wdyt?
There was a problem hiding this comment.
Hmm, using the JSON means that each edit would store the entire state of the feature or record, right? Instead of just storing the values for the particular key that is to be updated. That could make merging logic trickier if we ever need to support concurrent modifications -- it isn't as clear to see which keys were explicitly modified in this edit, so we might need to overwrite the entire feature state (including keys that the user didn't explicitly set when creating the edit).
We might have discussed this issue before, but I don't remember what we decided :).
|
Responses and six new commits. PTAL! |
|
@navinsivakumar Also, in what packages should these classes go? The remote data service interface is in c.g.a.gnd.service.RemoteDataService, with implementation in c.g.a.gnd.service.firestore. Shall we create interface c.g.a.gnd.service.LocalDataService and put impl in c.g.a.service.room as well? |
There was a problem hiding this comment.
@gino-m packages sound good -- data model in c.g.a.gnd.repository.local, LocalDataService in gnd.service with implementation in its own subpackage.
gnd/src/main/java/com/google/android/gnd/repository/local/IntEnum.java
Outdated
Show resolved
Hide resolved
| public String key; | ||
|
|
||
| // TODO: Add Converter to convert to/from JSON. | ||
| @Ignore @Nullable public Value oldValue; |
There was a problem hiding this comment.
Hmm, using the JSON means that each edit would store the entire state of the feature or record, right? Instead of just storing the values for the particular key that is to be updated. That could make merging logic trickier if we ever need to support concurrent modifications -- it isn't as clear to see which keys were explicitly modified in this edit, so we might need to overwrite the entire feature state (including keys that the user didn't explicitly set when creating the edit).
We might have discussed this issue before, but I don't remember what we decided :).
Sharing an early draft for discussion based on reversable edits instead of rollback snapshots. Once we've agreed on the final sync algorithm design I'll update this to reflect our discussions.