Skip to content

Commit

Permalink
Handle Questionnaire submission improvements (#3509)
Browse files Browse the repository at this point in the history
* Separate validationRequest listener to unblock main thread

during questionnaire response submission

* Refactor renderFragment to reduce nesting
  • Loading branch information
LZRS authored Nov 15, 2024
1 parent 2b53916 commit ce12e20
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import ca.uhn.fhir.context.FhirContext
import ca.uhn.fhir.context.support.DefaultProfileValidationSupport
import ca.uhn.fhir.context.support.IValidationSupport
import ca.uhn.fhir.validation.FhirValidator
import dagger.Lazy
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
Expand All @@ -30,6 +31,8 @@ import org.hl7.fhir.common.hapi.validation.support.InMemoryTerminologyServerVali
import org.hl7.fhir.common.hapi.validation.support.UnknownCodeSystemWarningValidationSupport
import org.hl7.fhir.common.hapi.validation.support.ValidationSupportChain
import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator
import org.smartregister.fhircore.engine.util.DispatcherProvider
import org.smartregister.fhircore.engine.util.validation.ResourceValidationRequestHandler

@Module
@InstallIn(SingletonComponent::class)
Expand All @@ -52,4 +55,13 @@ class FhirValidatorModule {
instanceValidator.invalidateCaches()
return fhirContext.newValidator().apply { registerValidatorModule(instanceValidator) }
}

@Provides
@Singleton
fun provideResourceValidationRequestHandler(
fhirValidatorProvider: Lazy<FhirValidator>,
dispatcherProvider: DispatcherProvider,
): ResourceValidationRequestHandler {
return ResourceValidationRequestHandler(fhirValidatorProvider.get(), dispatcherProvider)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright 2021-2024 Ona Systems, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.smartregister.fhircore.engine.util.validation

import ca.uhn.fhir.validation.FhirValidator
import ca.uhn.fhir.validation.ResultSeverityEnum
import ca.uhn.fhir.validation.ValidationResult
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import org.hl7.fhir.r4.model.Resource
import org.smartregister.fhircore.engine.util.DispatcherProvider
import org.smartregister.fhircore.engine.util.extension.referenceValue
import timber.log.Timber

data class ResourceValidationRequest(val resources: List<Resource>) {
constructor(vararg resource: Resource) : this(resource.toList())
}

class ResourceValidationRequestHandler(
private val fhirValidator: FhirValidator,
private val dispatcherProvider: DispatcherProvider,
) {
fun handleResourceValidationRequest(request: ResourceValidationRequest) {
CoroutineScope(dispatcherProvider.io()).launch {
val resources = request.resources
fhirValidator.checkResources(resources).logErrorMessages()
}
}
}

internal data class ResourceValidationResult(
val resource: Resource,
val validationResult: ValidationResult,
) {
val errorMessages
get() = buildString {
val messages =
validationResult.messages.filter {
it.severity.ordinal >= ResultSeverityEnum.WARNING.ordinal
}
if (messages.isNotEmpty()) {
appendLine(resource.referenceValue())
}
for (validationMsg in messages) {
appendLine(
"${validationMsg.locationString} - ${validationMsg.message} -- (${validationMsg.severity})",
)
}
}
}

internal class FhirValidatorResultsWrapper(
val results: List<ResourceValidationResult> = emptyList(),
) {
val errorMessages = results.map { it.errorMessages }

fun logErrorMessages() {
results.forEach {
if (it.errorMessages.isNotBlank()) {
Timber.tag(TAG).e(it.errorMessages)
}
}
}

companion object {
private const val TAG = "FhirValidatorResult"
}
}

internal fun FhirValidator.checkResources(
resources: List<Resource>,
): FhirValidatorResultsWrapper {
return FhirValidatorResultsWrapper(
results =
resources.map {
val result = this@checkResources.validateWithResult(it)
ResourceValidationResult(it, result)
},
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@
* limitations under the License.
*/

package org.smartregister.fhircore.engine.util.extension
package org.smartregister.fhircore.engine.util.validation

import ca.uhn.fhir.validation.FhirValidator
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.spyk
import io.mockk.mockkObject
import io.mockk.unmockkObject
import io.mockk.verify
import javax.inject.Inject
import kotlinx.coroutines.test.runTest
import org.hl7.fhir.instance.model.api.IBaseResource
import org.hl7.fhir.r4.model.CarePlan
import org.hl7.fhir.r4.model.Patient
import org.hl7.fhir.r4.model.Reference
Expand All @@ -32,33 +32,51 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.smartregister.fhircore.engine.robolectric.RobolectricTest
import timber.log.Timber

@HiltAndroidTest
class FhirValidatorExtensionTest : RobolectricTest() {

class ResourceValidationRequestTest : RobolectricTest() {
@get:Rule var hiltRule = HiltAndroidRule(this)

@Inject lateinit var validator: FhirValidator

@Inject lateinit var resourceValidationRequestHandler: ResourceValidationRequestHandler

@Before
fun setUp() {
hiltRule.inject()
}

@Test
fun testCheckResourceValidRunsNoValidationWhenBuildTypeIsNotDebug() = runTest {
val basicResource = CarePlan()
val fhirValidatorSpy = spyk(validator)
val resultsWrapper = fhirValidatorSpy.checkResourceValid(basicResource, isDebug = false)
Assert.assertTrue(resultsWrapper.results.isEmpty())
verify(exactly = 0) { fhirValidatorSpy.validateWithResult(any<IBaseResource>()) }
verify(exactly = 0) { fhirValidatorSpy.validateWithResult(any<String>()) }
fun testHandleResourceValidationRequestValidatesInvalidResourceLoggingErrors() = runTest {
mockkObject(Timber)
val resource =
CarePlan().apply {
id = "test-careplan"
status = CarePlan.CarePlanStatus.ACTIVE
intent = CarePlan.CarePlanIntent.PLAN
subject = Reference("f4bd3e29-f0f8-464e-97af-923b83664ccc")
}
val validationRequest = ResourceValidationRequest(resource)
resourceValidationRequestHandler.handleResourceValidationRequest(validationRequest)
verify {
Timber.e(
withArg<String> {
Assert.assertTrue(
it.contains(
"CarePlan.subject - The syntax of the reference 'f4bd3e29-f0f8-464e-97af-923b83664ccc' looks incorrect, and it should be checked -- (WARNING)",
),
)
},
)
}
unmockkObject(Timber)
}

@Test
fun testCheckResourceValidValidatesResourceStructureWhenCarePlanResourceInvalid() = runTest {
val basicCarePlan = CarePlan()
val resultsWrapper = validator.checkResourceValid(basicCarePlan)
val resultsWrapper = validator.checkResources(listOf(basicCarePlan))
Assert.assertTrue(
resultsWrapper.errorMessages.any {
it.contains(
Expand All @@ -85,13 +103,13 @@ class FhirValidatorExtensionTest : RobolectricTest() {
intent = CarePlan.CarePlanIntent.PLAN
subject = Reference("Task/unknown")
}
val resultsWrapper = validator.checkResourceValid(carePlan)
val resultsWrapper = validator.checkResources(listOf(carePlan))
Assert.assertEquals(1, resultsWrapper.errorMessages.size)
Assert.assertTrue(
resultsWrapper.errorMessages
.first()
.contains(
"The type 'Task' implied by the reference URL Task/unknown is not a valid Target for this element (must be one of [Group, Patient]) - CarePlan.subject",
"CarePlan.subject - The type 'Task' implied by the reference URL Task/unknown is not a valid Target for this element (must be one of [Group, Patient])",
ignoreCase = true,
),
)
Expand All @@ -105,13 +123,13 @@ class FhirValidatorExtensionTest : RobolectricTest() {
intent = CarePlan.CarePlanIntent.PLAN
subject = Reference("unknown")
}
val resultsWrapper = validator.checkResourceValid(carePlan)
val resultsWrapper = validator.checkResources(listOf(carePlan))
Assert.assertEquals(1, resultsWrapper.errorMessages.size)
Assert.assertTrue(
resultsWrapper.errorMessages
.first()
.contains(
"The syntax of the reference 'unknown' looks incorrect, and it should be checked - CarePlan.subject",
"CarePlan.subject - The syntax of the reference 'unknown' looks incorrect, and it should be checked",
ignoreCase = true,
),
)
Expand All @@ -126,7 +144,7 @@ class FhirValidatorExtensionTest : RobolectricTest() {
intent = CarePlan.CarePlanIntent.PLAN
subject = Reference(patient)
}
val resultsWrapper = validator.checkResourceValid(carePlan)
val resultsWrapper = validator.checkResources(listOf(carePlan))
Assert.assertEquals(1, resultsWrapper.errorMessages.size)
Assert.assertTrue(resultsWrapper.errorMessages.first().isBlank())
}
Expand Down
Loading

0 comments on commit ce12e20

Please sign in to comment.