fix: sdk 189 deduplication for metrics android #361
Conversation
Signed-off-by: Debanjan Chatterjee <[email protected]>
Signed-off-by: Debanjan Chatterjee <[email protected]>
Signed-off-by: Debanjan Chatterjee <[email protected]>
Signed-off-by: Debanjan Chatterjee <[email protected]>
Signed-off-by: Debanjan Chatterjee <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
| // companion object{ | ||
| // internal fun JsonAdapter.manipulate(): JsonAdapter { | ||
| companion object { | ||
| // internal fun JsonAdapter.manipulate(): JsonAdapter { |
There was a problem hiding this comment.
Do we need this commented-out code?
|
|
||
| fun getMetricsFirst(limit: Long, callback: (List<MetricModelWithId<out Number>>) -> Unit) | ||
|
|
||
| // fun getMetricsAndErrorFirst(limit : Long, callback : (List<MetricModel<Number>>, List<ErrorModel>) -> Unit) |
There was a problem hiding this comment.
Same here, do we need this commented out part?
| init { | ||
| jsonAdapter = MoshiAdapter() | ||
| override fun shutdown(shutdownExecutor: Boolean) { | ||
| TODO("Not yet implemented") |
There was a problem hiding this comment.
You should probably comment this out, this part of the code is prone to crash if executed
| endpoint: String, | ||
| responseClass: Class<T> | ||
| ): Future<HttpResponse<T>> { | ||
| TODO("Not yet implemented") |
There was a problem hiding this comment.
TODO markups should be commented out, this part of the code is prone to crash if executed.
| endpoint: String, | ||
| responseTypeAdapter: RudderTypeAdapter<T> | ||
| ): Future<HttpResponse<T>> { | ||
| TODO("Not yet implemented") |
| responseTypeAdapter: RudderTypeAdapter<T>, | ||
| isGzipEnabled: Boolean | ||
| ): Future<HttpResponse<T>> { | ||
| TODO("Not yet implemented") |
| } | ||
|
|
||
| override fun setInterceptor(httpInterceptor: HttpInterceptor) { | ||
| TODO("Not yet implemented") |
| jsonAdapter, TestExecutor() | ||
| ) | ||
| @Test | ||
| fun `test uploadWithSuccessfulUpload`() { |
There was a problem hiding this comment.
For Kotlin tests it would be nice to start applying Given-When-Then naming 😃 .
@Test
fun `given some test data, when the service is successful, upload is successful`() {
val mockSnapshot = TestDataGenerator.mockSnapshot()
val callbackMock = mock<(Boolean) -> Unit>()
val mockWebService = MockPostWebService(200)
val uploadMediator = DefaultUploadMediator("https://api.example.com", mock(), mock(), true, mockWebService)
uploadMediator.upload(mockSnapshot, callbackMock)
verify(callbackMock, times(1)).invoke(true)
}
@Test
fun `given some test data, when the service is unsuccessful, upload is not successful`() {
val mockSnapshot = TestDataGenerator.mockSnapshot()
val callbackMock = mock<(Boolean) -> Unit>()
val mockWebService = MockPostWebService(400)
val uploadMediator = DefaultUploadMediator("https://api.example.com", mock(), mock(), true, mockWebService)
uploadMediator.upload(mockSnapshot, callbackMock)
verify(callbackMock, times(1)).invoke(false)
}
| internal class DefaultSnapshotCreator(private val libraryMetadata: LibraryMetadata, | ||
| private val apiVersion: Int, private val jsonAdapter: | ||
| JsonAdapter): SnapshotCreator { | ||
| companion object { |
There was a problem hiding this comment.
In Kotlin, files and classes are completely decoupled. You can also declare private top-level functions and variables and they'll be accessible only to the classes within the same file. Top-level declarations instead of companion objects for such cases of private static data can be preferred and are more Kotlin-like. If however, for whatever reason, you have to produce a Java class with Kotlin code such that the class has a static member, you must resort to a companion object and a special annotation.
private const val SOURCE_KEY = "source"
private const val METRICS_KEY = "metrics"
private const val ERROR_KEY = "errors"
private const val VERSION_KEY = "version"
private const val MESSAGE_ID_KEY = "message_id"
internal class DefaultSnapshotCreator(
private val libraryMetadata: LibraryMetadata,
private val apiVersion: Int, private val jsonAdapter:
JsonAdapter
) : SnapshotCreator {
| uploadedErrorModel: ErrorModel, | ||
| success: Boolean) -> Unit)? = null | ||
| private val snapshotCapturer: SnapshotCapturer, | ||
| // private val libraryMetadata: LibraryMetadata |
There was a problem hiding this comment.
Is this commented-out code needed?
Fixes # (issue)
Type of change
Checklist:
Supports Deduplication for metrics.
Snapshots are saved periodically in separate database.
Snapshots are synced with server at intervals.