Skip to content

Commit

Permalink
Refactor code (#3318)
Browse files Browse the repository at this point in the history
* Refactor how FHIR URL is provided

Signed-off-by: Elly Kitoto <[email protected]>

* Delete unused code

Signed-off-by: Elly Kitoto <[email protected]>

* Rename configuration property

Signed-off-by: Elly Kitoto <[email protected]>

* Document AppConfig property

Signed-off-by: Elly Kitoto <[email protected]>

---------

Signed-off-by: Elly Kitoto <[email protected]>
  • Loading branch information
ellykits authored Jun 11, 2024
1 parent ac0fa3d commit 31be373
Show file tree
Hide file tree
Showing 23 changed files with 98 additions and 182 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import org.hl7.fhir.r4.model.ResourceType
import org.jetbrains.annotations.VisibleForTesting
import org.json.JSONObject
import org.smartregister.fhircore.engine.BuildConfig
import org.smartregister.fhircore.engine.OpenSrpApplication
import org.smartregister.fhircore.engine.configuration.app.ConfigService
import org.smartregister.fhircore.engine.configuration.profile.ProfileConfiguration
import org.smartregister.fhircore.engine.configuration.register.RegisterConfiguration
Expand Down Expand Up @@ -92,18 +91,17 @@ constructor(
val configService: ConfigService,
val json: Json,
@ApplicationContext val context: Context,
private var openSrpApplication: OpenSrpApplication?,
) {

@Inject lateinit var knowledgeManager: KnowledgeManager

val configsJsonMap = mutableMapOf<String, String>()
val configCacheMap = mutableMapOf<String, Configuration>()
val localizationHelper: LocalizationHelper by lazy { LocalizationHelper(this) }
private val supportedFileExtensions = listOf("json", "properties")
private var _isNonProxy = BuildConfig.IS_NON_PROXY_APK
private val fhirContext = FhirContext.forR4Cached()

@Inject lateinit var knowledgeManager: KnowledgeManager

private val authConfiguration = configService.provideAuthConfiguration()
private val jsonParser = fhirContext.newJsonParser()

/**
Expand Down Expand Up @@ -405,8 +403,7 @@ constructor(
* Type'?_id='comma,separated,list,of,ids'
*/
@Throws(UnknownHostException::class, HttpException::class)
suspend fun fetchNonWorkflowConfigResources(isInitialLogin: Boolean = true) {
// Reset configurations before loading new ones
suspend fun fetchNonWorkflowConfigResources() {
configCacheMap.clear()
sharedPreferencesHelper.read(SharedPreferenceKey.APP_ID.name, null)?.let { appId ->
val parsedAppId = appId.substringBefore(TYPE_REFERENCE_DELIMITER).trim()
Expand Down Expand Up @@ -638,7 +635,7 @@ constructor(
this.apply {
url =
url
?: """${openSrpApplication?.getFhirServerHost()?.toString()?.trimEnd { it == '/' }}/${this.referenceValue()}"""
?: """${authConfiguration.fhirServerBaseUrl.trimEnd { it == '/' }}/${this.referenceValue()}"""
}

fun writeToFile(resource: Resource): File {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ data class ApplicationConfiguration(
SettingsOptions.INSIGHTS,
),
val logGpsLocation: List<LocationLogOptions> = emptyList(),
val usePractitionerAssignedLocationOnSync: Boolean =
true, // TODO This defaults to scheduling periodic sync, otherwise use sync location ids from
// location selector
val launcherType: LauncherType = LauncherType.REGISTER,
val usePractitionerAssignedLocationOnSync: Boolean = true,
val navigationStartDestination: LauncherType = LauncherType.REGISTER,
) : Configuration()

enum class SyncStrategy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ interface ConfigService {
return tags
}

fun provideConfigurationSyncPageSize(): String

/**
* Provide a list of custom search parameters.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.smartregister.fhircore.engine.di

import android.content.Context
import ca.uhn.fhir.context.FhirContext
import ca.uhn.fhir.parser.IParser
import com.google.gson.Gson
Expand All @@ -25,8 +24,8 @@ import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFact
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import java.net.URL
import java.util.TimeZone
import java.util.concurrent.TimeUnit
import javax.inject.Singleton
Expand All @@ -40,7 +39,6 @@ import okhttp3.Response
import okhttp3.ResponseBody.Companion.toResponseBody
import okhttp3.logging.HttpLoggingInterceptor
import org.smartregister.fhircore.engine.BuildConfig
import org.smartregister.fhircore.engine.OpenSrpApplication
import org.smartregister.fhircore.engine.configuration.app.ConfigService
import org.smartregister.fhircore.engine.data.remote.auth.KeycloakService
import org.smartregister.fhircore.engine.data.remote.auth.OAuthService
Expand Down Expand Up @@ -83,7 +81,7 @@ class NetworkModule {
fun provideOkHttpClient(
tokenAuthenticator: TokenAuthenticator,
sharedPreferencesHelper: SharedPreferencesHelper,
openSrpApplication: OpenSrpApplication?,
configService: ConfigService,
) =
OkHttpClient.Builder()
.addInterceptor(
Expand All @@ -92,15 +90,11 @@ class NetworkModule {
var request = chain.request()
val requestPath = request.url.encodedPath.substring(1)
val resourcePath = if (!_isNonProxy) requestPath.replace("fhir/", "") else requestPath
val host = URL(configService.provideAuthConfiguration().fhirServerBaseUrl).host

openSrpApplication?.let {
if (
(request.url.host == it.getFhirServerHost()?.host) &&
CUSTOM_ENDPOINTS.contains(resourcePath)
) {
val newUrl = request.url.newBuilder().encodedPath("/$resourcePath").build()
request = request.newBuilder().url(newUrl).build()
}
if (request.url.host == host && CUSTOM_ENDPOINTS.contains(resourcePath)) {
val newUrl = request.url.newBuilder().encodedPath("/$resourcePath").build()
request = request.newBuilder().url(newUrl).build()
}

chain.proceed(request)
Expand Down Expand Up @@ -231,11 +225,6 @@ class NetworkModule {
fun provideFhirResourceService(@RegularRetrofit retrofit: Retrofit): FhirResourceService =
retrofit.create(FhirResourceService::class.java)

@Provides
@Singleton
fun provideFHIRBaseURL(@ApplicationContext context: Context): OpenSrpApplication? =
if (context is OpenSrpApplication) context else null

companion object {
const val TIMEOUT_DURATION = 120L
const val AUTHORIZATION = "Authorization"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ class AppConfigService @Inject constructor(@ApplicationContext val context: Cont
),
)

override fun provideConfigurationSyncPageSize(): String {
return "100"
}

companion object {
const val CARETEAM_SYSTEM = "http://fake.tag.com/CareTeam#system"
const val CARETEAM_DISPLAY = "Practitioner CareTeam"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.spyk
import java.net.URL
import java.util.Calendar
import java.util.Date
import kotlinx.coroutines.runBlocking
Expand All @@ -38,7 +37,7 @@ import org.hl7.fhir.r4.model.Identifier
import org.hl7.fhir.r4.model.Patient
import org.hl7.fhir.r4.model.Reference
import org.hl7.fhir.r4.model.StringType
import org.smartregister.fhircore.engine.OpenSrpApplication
import org.smartregister.fhircore.engine.app.AppConfigService
import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry
import org.smartregister.fhircore.engine.data.remote.fhir.resource.FhirResourceDataSource
import org.smartregister.fhircore.engine.data.remote.fhir.resource.FhirResourceService
Expand All @@ -57,6 +56,8 @@ object Faker {
}

private val testDispatcher = UnconfinedTestDispatcher()
private val configService =
AppConfigService(ApplicationProvider.getApplicationContext<HiltTestApplication>())

private val testDispatcherProvider =
object : DispatcherProvider {
Expand Down Expand Up @@ -98,15 +99,9 @@ object Faker {
fhirResourceDataSource = fhirResourceDataSource,
sharedPreferencesHelper = sharedPreferencesHelper,
dispatcherProvider = dispatcherProvider,
configService = mockk(),
configService = configService,
json = json,
context = ApplicationProvider.getApplicationContext<HiltTestApplication>(),
openSrpApplication =
object : OpenSrpApplication() {
override fun getFhirServerHost(): URL {
return URL("http://my_test_fhirbase_url/fhir/")
}
},
),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.mockk
import io.mockk.spyk
import java.net.URL
import javax.inject.Inject
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
Expand All @@ -59,7 +58,6 @@ import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.smartregister.fhircore.engine.OpenSrpApplication
import org.smartregister.fhircore.engine.app.fakes.Faker
import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry.Companion.MANIFEST_PROCESSOR_BATCH_SIZE
import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry.Companion.PAGINATION_NEXT
Expand Down Expand Up @@ -115,12 +113,6 @@ class ConfigurationRegistryTest : RobolectricTest() {
configService = configService,
json = json,
context = ApplicationProvider.getApplicationContext<HiltTestApplication>(),
openSrpApplication =
object : OpenSrpApplication() {
override fun getFhirServerHost(): URL {
return URL("http://my_test_fhirbase_url/fhir/")
}
},
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.google.android.fhir.db.ResourceNotFoundException
import com.google.android.fhir.get
import com.google.android.fhir.logicalId
import com.google.gson.Gson
import dagger.hilt.android.testing.BindValue
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.coEvery
Expand Down Expand Up @@ -72,6 +73,7 @@ import org.junit.Assert
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.smartregister.fhircore.engine.app.AppConfigService
import org.smartregister.fhircore.engine.app.fakes.Faker
import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry
import org.smartregister.fhircore.engine.configuration.app.ConfigService
Expand Down Expand Up @@ -111,31 +113,31 @@ class DefaultRepositoryTest : RobolectricTest() {

@Inject lateinit var fhirPathDataExtractor: FhirPathDataExtractor

@Inject lateinit var configService: ConfigService

@Inject lateinit var fhirEngine: FhirEngine

@Inject lateinit var parser: IParser

@BindValue
val configService: ConfigService =
spyk(AppConfigService(ApplicationProvider.getApplicationContext()))
private val application = ApplicationProvider.getApplicationContext<Application>()
private val configurationRegistry: ConfigurationRegistry = Faker.buildTestConfigurationRegistry()
private lateinit var dispatcherProvider: DefaultDispatcherProvider
private lateinit var sharedPreferenceHelper: SharedPreferencesHelper
private lateinit var defaultRepository: DefaultRepository
private lateinit var spiedConfigService: ConfigService

@Before
fun setUp() {
hiltRule.inject()
dispatcherProvider = DefaultDispatcherProvider()
sharedPreferenceHelper = SharedPreferencesHelper(application, gson)
spiedConfigService = spyk(configService)
defaultRepository =
DefaultRepository(
fhirEngine = fhirEngine,
dispatcherProvider = dispatcherProvider,
sharedPreferencesHelper = sharedPreferenceHelper,
configurationRegistry = configurationRegistry,
configService = spiedConfigService,
configService = configService,
configRulesExecutor = configRulesExecutor,
fhirPathDataExtractor = fhirPathDataExtractor,
parser = parser,
Expand Down Expand Up @@ -313,27 +315,25 @@ class DefaultRepositoryTest : RobolectricTest() {
@Test
fun testCreateShouldNotDuplicateMetaTagsWithSameSystemCode() {
val system = "https://smartregister.org/location-tag-id"
val code = "86453"
val anotherCode = "10200"
val coding = Coding(system, code, "Location")
val anotherCoding = Coding(system, anotherCode, "Location")
val coding = Coding(system, "86453", "Location")
val anotherCoding = Coding(system, "10200", "Location")
val resource = Patient().apply { meta.addTag(coding) }

// Meta contains 1 tag with code 86453
Assert.assertEquals(1, resource.meta.tag.size)
val firstTag = resource.meta.tag.first()
Assert.assertEquals(code, firstTag.code)
Assert.assertEquals("86453", firstTag.code)
Assert.assertEquals(system, firstTag.system)

coEvery { fhirEngine.create(any()) } returns listOf(resource.id)
every { spiedConfigService.provideResourceTags(sharedPreferenceHelper) } returns
every { configService.provideResourceTags(sharedPreferenceHelper) } returns
listOf(coding, anotherCoding)
runBlocking { defaultRepository.create(true, resource) }

// Expecting 2 tags; tag with code 86453 should not be duplicated.
Assert.assertEquals(2, resource.meta.tag.size)
Assert.assertNotNull(resource.meta.lastUpdated)
Assert.assertNotNull(resource.meta.getTag(system, code))
Assert.assertNotNull(resource.meta.getTag(system, "86453"))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ import org.junit.Test
import org.smartregister.fhircore.engine.app.fakes.Faker
import org.smartregister.fhircore.engine.domain.model.ActionParameter
import org.smartregister.fhircore.engine.domain.model.ActionParameterType
import org.smartregister.fhircore.engine.robolectric.RobolectricTest

class QuestionnaireExtensionTest {
class QuestionnaireExtensionTest : RobolectricTest() {
private lateinit var questionniare: Questionnaire
private lateinit var questionniareResponse: QuestionnaireResponse
private lateinit var questionniareResponseItemComponent:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ class FakeConfigService @Inject constructor() : ConfigService {
),
)

override fun provideConfigurationSyncPageSize(): String {
return "100"
}

companion object {
const val CARETEAM_SYSTEM = "http://fake.tag.com/CareTeam#system"
const val CARETEAM_DISPLAY = "Practitioner CareTeam"
Expand Down
Loading

0 comments on commit 31be373

Please sign in to comment.