-
Notifications
You must be signed in to change notification settings - Fork 251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync - Generalizing resource downloading #58
Conversation
3306087
to
d5ac30b
Compare
a7d5b24
to
0d6fae1
Compare
fhirengine/src/main/java/com/google/fhirengine/sync/SyncConfiguration.kt
Outdated
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/db/impl/SyncedResource.kt
Show resolved
Hide resolved
example/src/main/java/com/google/fhirengine/example/MainActivityViewModel.kt
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/sync/SyncData.kt
Outdated
Show resolved
Hide resolved
c7410af
to
40c5014
Compare
@@ -89,6 +90,11 @@ internal class DatabaseImpl( | |||
} ?: throw ResourceNotFoundInDbException(type.name, id) | |||
} | |||
|
|||
// TODO implement the last update date query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to merge this after implementing it or merge w/ TODO?
// TODO find a better exception to throw | ||
@Override | ||
public Result sync() throws Exception { | ||
if (syncConfiguration == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think illegal state is proper here. and once we move to kotlin, we can just do checkNotNull
fhirengine/src/main/java/com/google/fhirengine/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/sync/FhirSynchronizer.kt
Outdated
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/sync/ResourceSynchronizer.kt
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/sync/SyncData.kt
Outdated
Show resolved
Hide resolved
fhirengine/src/main/java/com/google/fhirengine/sync/SyncDownloadWorker.kt
Show resolved
Hide resolved
} | ||
} | ||
|
||
class SyncDownloadWorkerFactory( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does WM accept multiple workers?
if not, we should probably receive a delegate here so that it can be combined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll handle this properly as part of #71
fhirengine/src/main/java/com/google/fhirengine/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
40c5014
to
f35a8be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just 1 suggestion then lgtm. thanks!
No description provided.