From 603792f4e865ba3c736565d40672eda3a587ad97 Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Fri, 2 Aug 2024 12:42:01 +0100 Subject: [PATCH 01/13] Rename install API to index for file in knowledge manager (#2634) * Rename install to index for file * Revert workflow unit tests since they use published knowledge manager * Remove extra dependency in workflow build file * Address review comments * Fix smart immunizations test * Update knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt Co-authored-by: aditya-07 * Update knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt Co-authored-by: aditya-07 * Run spotless --------- Co-authored-by: aditya-07 --- .../fhir/knowledge/KnowledgeManager.kt | 187 +++++++++++------- .../db/entities/ImplementationGuideEntity.kt | 7 +- .../fhir/knowledge/KnowledgeManagerTest.kt | 63 +++--- .../care_plan.json | 12 +- .../med-request/med_request_careplan.json | 10 +- .../benchmark/F_CqlEvaluatorBenchmark.kt | 2 +- workflow/build.gradle.kts | 12 +- .../CarePlan/CarePlan.json | 10 +- .../FhirOperatorLibraryEvaluateTest.kt | 8 +- .../workflow/SmartImmunizationAndroidTest.kt | 4 +- .../FhirOperatorLibraryEvaluateJavaTest.kt | 8 +- .../android/fhir/workflow/FhirOperatorTest.kt | 6 +- .../fhir/workflow/SmartImmunizationTest.kt | 4 +- 13 files changed, 194 insertions(+), 139 deletions(-) diff --git a/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt b/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt index e319fa79c9..3a3a956c6f 100644 --- a/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt +++ b/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt @@ -21,22 +21,19 @@ import androidx.room.Room import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.parser.IParser import com.google.android.fhir.knowledge.db.KnowledgeDatabase +import com.google.android.fhir.knowledge.db.entities.ImplementationGuideEntity import com.google.android.fhir.knowledge.db.entities.ResourceMetadataEntity -import com.google.android.fhir.knowledge.db.entities.toEntity import com.google.android.fhir.knowledge.files.NpmFileManager import com.google.android.fhir.knowledge.npm.NpmPackageDownloader import com.google.android.fhir.knowledge.npm.OkHttpNpmPackageDownloader import java.io.File import java.io.FileInputStream -import java.io.FileOutputStream import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.hl7.fhir.instance.model.api.IBaseResource import org.hl7.fhir.r4.context.IWorkerContext import org.hl7.fhir.r4.context.SimpleWorkerContext -import org.hl7.fhir.r4.model.IdType import org.hl7.fhir.r4.model.MetadataResource -import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.hl7.fhir.utilities.npm.NpmPackage import timber.log.Timber @@ -46,9 +43,20 @@ import timber.log.Timber * individually as JSON files or from FHIR NPM packages. * * Coordinates the management of knowledge artifacts by using the three following components: - * - database: indexing knowledge artifacts stored in the local file system, - * - file manager: managing files containing the knowledge artifacts, and - * - NPM downloader: downloading from an NPM package server the knowledge artifacts. + * - knowledgeDatabase: indexing knowledge artifacts stored in the local file system, + * - npmFileManager: managing files containing the knowledge artifacts, and + * - npmPackageDownloader: downloading the knowledge artifacts from an NPM package server . + * + * Knowledge artifacts are scoped by the application. Multiple applications using the knowledge + * manager will not share the same sets of knowledge artifacts. + * + * See [Clinical Reasoning](https://hl7.org/fhir/R4/clinicalreasoning-module.html) for the formal + * definition of knowledge artifacts. In this implementation, however, knowledge artifacts are + * represented as [MetadataResource]s. + * + * **Note** that the list of resources implementing the [MetadataResource] class differs from the + * list of resources implementing the + * [MetadataResource interface](https://www.hl7.org/fhir/R5/metadataresource.html) in FHIR R5. */ class KnowledgeManager internal constructor( @@ -60,9 +68,11 @@ internal constructor( private val knowledgeDao = knowledgeDatabase.knowledgeDao() /** - * Checks if the [fhirNpmPackages] are present in DB. If necessary, downloads the dependencies - * from NPM and imports data from the package manager (populates the metadata of the FHIR - * Resources). + * Downloads and installs the [fhirNpmPackages] from the NPM package server with transitive + * dependencies. The NPM packages will be unzipped to a directory managed by the knowledge + * manager. The resources will be indexed in the database for future retrieval. + * + * FHIR NPM packages already present in the database will be skipped. */ suspend fun install(vararg fhirNpmPackages: FhirNpmPackage) { fhirNpmPackages @@ -81,7 +91,7 @@ internal constructor( try { val localFhirNpmPackageMetadata = npmFileManager.getLocalFhirNpmPackageMetadata(it.name, it.version) - install(it, localFhirNpmPackageMetadata.rootDirectory) + import(it, localFhirNpmPackageMetadata.rootDirectory) install(*localFhirNpmPackageMetadata.dependencies.toTypedArray()) } catch (e: Exception) { Timber.w("Unable to install package ${it.name} ${it.version}") @@ -90,35 +100,70 @@ internal constructor( } /** - * Checks if the [fhirNpmPackage] is present in DB. If necessary, populates the database with the - * metadata of FHIR Resource from the provided [rootDirectory]. + * Imports the content of the [fhirNpmPackage] from the provided [rootDirectory] by indexing the + * metadata of the FHIR resources for future retrieval. + * + * FHIR NPM packages already present in the database will be skipped. */ - suspend fun install(fhirNpmPackage: FhirNpmPackage, rootDirectory: File) { + suspend fun import(fhirNpmPackage: FhirNpmPackage, rootDirectory: File) { // TODO(ktarasenko) copy files to the safe space? - val igId = knowledgeDao.insert(fhirNpmPackage.toEntity(rootDirectory)) - rootDirectory.listFiles()?.sorted()?.forEach { file -> - try { - val resource = jsonParser.parseResource(FileInputStream(file)) - if (resource is Resource) { - val newId = indexResourceFile(igId, resource, file) - resource.setId(IdType(resource.resourceType.name, newId)) - - // Overrides the Id in the file - FileOutputStream(file).use { - it.write(jsonParser.encodeResourceToString(resource).toByteArray()) - } - } else { - Timber.d("Unable to import file: %file") - } - } catch (exception: Exception) { - Timber.d(exception, "Unable to import file: %file") - } + val implementationGuideId = + knowledgeDao.insert( + ImplementationGuideEntity( + 0L, + fhirNpmPackage.canonical ?: "", + fhirNpmPackage.name, + fhirNpmPackage.version, + rootDirectory, + ), + ) + val files = rootDirectory.listFiles() ?: return + files.sorted().forEach { file -> + // Ignore files that are not meta resources instead of throwing exceptions since unzipped + // NPM package might contain other types of files e.g. package.json. + val resource = readMetadataResourceOrNull(file) ?: return@forEach + knowledgeDao.insertResource( + implementationGuideId, + ResourceMetadataEntity( + 0, + resource.resourceType, + resource.url, + resource.name, + resource.version, + file, + ), + ) } } - /** Imports the Knowledge Artifact from the provided [file] to the default dependency. */ - suspend fun install(file: File) { - importFile(null, file) + /** + * Indexes a knowledge artifact as a JSON object in the provided [file]. + * + * This creates a record of the knowledge artifact's metadata and the file's location. When the + * knowledge artifact is requested, knowledge manager will load the content of the file, + * deserialize it and return the resulting FHIR resource. + * + * This operation does not make a copy of the knowledge artifact, nor does it checksum the content + * of the file. Therefore, it cannot be guaranteed that subsequent retrievals of the knowledge + * artifact will produce the same result. Applications using this function must be aware of the + * risk of the content of the file being modified or corrupt, potentially resulting in incorrect + * or inaccurate result of decision support or measure evaluation. + * + * Use this API for knowledge artifacts in immutable files (e.g. in the app's `assets` folder). + */ + suspend fun index(file: File) { + val resource = readMetadataResourceOrThrow(file) + knowledgeDao.insertResource( + null, + ResourceMetadataEntity( + 0L, + resource.resourceType, + resource.url, + resource.name, + resource.version, + file, + ), + ) } /** Loads resources from IGs listed in dependencies. */ @@ -141,7 +186,7 @@ internal constructor( name != null -> knowledgeDao.getResourcesWithName(resType, name) else -> knowledgeDao.getResources(resType) } - return resourceEntities.map { loadResource(it) } + return resourceEntities.map { readMetadataResourceOrThrow(it.resourceFile)!! } } /** Deletes Implementation Guide, cleans up files. */ @@ -155,43 +200,6 @@ internal constructor( } } - private suspend fun importFile(igId: Long?, file: File) { - val resource = - withContext(Dispatchers.IO) { - try { - FileInputStream(file).use(jsonParser::parseResource) - } catch (exception: Exception) { - Timber.d(exception, "Unable to import file: $file. Parsing to FhirResource failed.") - } - } - when (resource) { - is Resource -> { - val newId = indexResourceFile(igId, resource, file) - resource.setId(IdType(resource.resourceType.name, newId)) - - // Overrides the Id in the file - FileOutputStream(file).use { - it.write(jsonParser.encodeResourceToString(resource).toByteArray()) - } - } - } - } - - private suspend fun indexResourceFile(igId: Long?, resource: Resource, file: File): Long { - val metadataResource = resource as? MetadataResource - val res = - ResourceMetadataEntity( - 0L, - resource.resourceType, - metadataResource?.url, - metadataResource?.name, - metadataResource?.version, - file, - ) - - return knowledgeDao.insertResource(igId, res) - } - /** * Loads and initializes a worker context with the specified npm packages. * @@ -220,8 +228,37 @@ internal constructor( } } - private fun loadResource(resourceEntity: ResourceMetadataEntity): IBaseResource { - return jsonParser.parseResource(FileInputStream(resourceEntity.resourceFile)) + /** + * Parses and returns the content of a file containing a FHIR resource in JSON, or null if the + * file does not contain a FHIR resource. + */ + private suspend fun readResourceOrNull(file: File): IBaseResource? = + withContext(Dispatchers.IO) { + try { + FileInputStream(file).use(jsonParser::parseResource) + } catch (e: Exception) { + Timber.e(e, "Unable to load resource from $file") + null + } + } + + /** + * Parses and returns the content of a file containing a FHIR metadata resource in JSON, or null + * if the file does not contain a FHIR metadata resource. + */ + private suspend fun readMetadataResourceOrNull(file: File) = + readResourceOrNull(file) as? MetadataResource + + /** + * Parses and returns the content of a file containing a FHIR metadata resource in JSON, or throws + * an exception if the file does not contain a FHIR metadata resource. + */ + private suspend fun readMetadataResourceOrThrow(file: File): MetadataResource { + val resource = readResourceOrNull(file)!! + check(resource is MetadataResource) { + "Resource ${resource.idElement} is not a MetadataResource" + } + return resource } companion object { diff --git a/knowledge/src/main/java/com/google/android/fhir/knowledge/db/entities/ImplementationGuideEntity.kt b/knowledge/src/main/java/com/google/android/fhir/knowledge/db/entities/ImplementationGuideEntity.kt index 8d2a1804e0..91e53a6d82 100644 --- a/knowledge/src/main/java/com/google/android/fhir/knowledge/db/entities/ImplementationGuideEntity.kt +++ b/knowledge/src/main/java/com/google/android/fhir/knowledge/db/entities/ImplementationGuideEntity.kt @@ -1,5 +1,5 @@ /* - * Copyright 2022-2023 Google LLC + * Copyright 2022-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,7 +19,6 @@ package com.google.android.fhir.knowledge.db.entities import androidx.room.Entity import androidx.room.Index import androidx.room.PrimaryKey -import com.google.android.fhir.knowledge.FhirNpmPackage import java.io.File /** @@ -45,7 +44,3 @@ internal data class ImplementationGuideEntity( /** Directory where the Implementation Guide files are stored */ val rootDirectory: File, ) - -internal fun FhirNpmPackage.toEntity(rootFolder: File): ImplementationGuideEntity { - return ImplementationGuideEntity(0L, canonical ?: "", name, version, rootFolder) -} diff --git a/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt b/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt index abef67cbee..48cdefd60f 100644 --- a/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt +++ b/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt @@ -25,11 +25,14 @@ import com.google.android.fhir.knowledge.db.KnowledgeDatabase import com.google.android.fhir.knowledge.files.NpmFileManager import com.google.common.truth.Truth.assertThat import java.io.File +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest +import org.hl7.fhir.r4.model.BaseResource import org.hl7.fhir.r4.model.Library -import org.hl7.fhir.r4.model.MetadataResource +import org.hl7.fhir.r4.model.Patient import org.hl7.fhir.r4.model.PlanDefinition import org.junit.After +import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -65,7 +68,7 @@ internal class KnowledgeManagerTest { @Test fun `importing IG creates entries in DB`() = runTest { - knowledgeManager.install(fhirNpmPackage, dataFolder) + knowledgeManager.import(fhirNpmPackage, dataFolder) val implementationGuideId = knowledgeDb.knowledgeDao().getImplementationGuide("anc-cds", "0.3.0")!!.implementationGuideId @@ -81,9 +84,10 @@ internal class KnowledgeManagerTest { @Test fun `deleting IG deletes files and DB entries`() = runTest { val igRoot = File(dataFolder.parentFile, "anc-cds.copy") + igRoot.deleteRecursively() igRoot.deleteOnExit() dataFolder.copyRecursively(igRoot) - knowledgeManager.install(fhirNpmPackage, igRoot) + knowledgeManager.import(fhirNpmPackage, igRoot) knowledgeManager.delete(fhirNpmPackage) @@ -93,7 +97,7 @@ internal class KnowledgeManagerTest { @Test fun `imported entries are readable`() = runTest { - knowledgeManager.install(fhirNpmPackage, dataFolder) + knowledgeManager.import(fhirNpmPackage, dataFolder) assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "WHOCommon")) .isNotNull() @@ -112,7 +116,16 @@ internal class KnowledgeManagerTest { } @Test - fun `inserting a library of a different version creates new entry`() = runTest { + fun `indexing non metadata resource should throw an exception`() { + val patient = Patient().apply { id = "Patient/defaultA-A.1.0.0" } + + assertThrows(IllegalStateException::class.java) { + runBlocking { knowledgeManager.index(writeToFile(patient)) } + } + } + + @Test + fun `should index a library of a different version`() = runTest { val libraryAOld = Library().apply { id = "Library/defaultA-A.1.0.0" @@ -128,8 +141,8 @@ internal class KnowledgeManagerTest { version = "A.1.0.1" } - knowledgeManager.install(writeToFile(libraryAOld)) - knowledgeManager.install(writeToFile(libraryANew)) + knowledgeManager.index(writeToFile(libraryAOld)) + knowledgeManager.index(writeToFile(libraryANew)) val resources = knowledgeDb.knowledgeDao().getResources() assertThat(resources).hasSize(2) @@ -137,14 +150,14 @@ internal class KnowledgeManagerTest { val resourceA100 = knowledgeManager .loadResources(resourceType = "Library", name = "defaultA", version = "A.1.0.0") - .single() - assertThat(resourceA100.idElement.toString()).isEqualTo("Library/1") + .single() as Library + assertThat(resourceA100.version).isEqualTo("A.1.0.0") val resourceA101 = knowledgeManager .loadResources(resourceType = "Library", name = "defaultA", version = "A.1.0.1") - .single() - assertThat(resourceA101.idElement.toString()).isEqualTo("Library/2") + .single() as Library + assertThat(resourceA101.version.toString()).isEqualTo("A.1.0.1") } fun `installing from npmPackageManager`() = runTest { @@ -182,19 +195,20 @@ internal class KnowledgeManagerTest { url = commonUrl } - knowledgeManager.install(writeToFile(libraryWithSameUrl)) - knowledgeManager.install(writeToFile(planDefinitionWithSameUrl)) + knowledgeManager.index(writeToFile(libraryWithSameUrl)) + knowledgeManager.index(writeToFile(planDefinitionWithSameUrl)) val resources = knowledgeDb.knowledgeDao().getResources() assertThat(resources).hasSize(2) val libraryLoadedByUrl = - knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() - assertThat(libraryLoadedByUrl.idElement.toString()).isEqualTo("Library/1") + knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() as Library + assertThat(libraryLoadedByUrl.name.toString()).isEqualTo("LibraryName") val planDefinitionLoadedByUrl = knowledgeManager.loadResources(resourceType = "PlanDefinition", url = commonUrl).single() - assertThat(planDefinitionLoadedByUrl.idElement.toString()).isEqualTo("PlanDefinition/2") + as PlanDefinition + assertThat(planDefinitionLoadedByUrl.name.toString()).isEqualTo("PlanDefinitionName") } @Test @@ -215,25 +229,26 @@ internal class KnowledgeManagerTest { version = "0" } - knowledgeManager.install(writeToFile(libraryWithSameUrl)) - knowledgeManager.install(writeToFile(planDefinitionWithSameUrl)) + knowledgeManager.index(writeToFile(libraryWithSameUrl)) + knowledgeManager.index(writeToFile(planDefinitionWithSameUrl)) val resources = knowledgeDb.knowledgeDao().getResources() assertThat(resources).hasSize(2) val libraryLoadedByUrl = - knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() - assertThat(libraryLoadedByUrl.idElement.toString()).isEqualTo("Library/1") + knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() as Library + assertThat(libraryLoadedByUrl.name.toString()).isEqualTo("LibraryName") val planDefinitionLoadedByUrl = knowledgeManager.loadResources(resourceType = "PlanDefinition", url = commonUrl).single() - assertThat(planDefinitionLoadedByUrl.idElement.toString()).isEqualTo("PlanDefinition/2") + as PlanDefinition + assertThat(planDefinitionLoadedByUrl.name.toString()).isEqualTo("PlanDefinitionName") } - private fun writeToFile(metadataResource: MetadataResource): File { - return File(context.filesDir, metadataResource.id).apply { + private fun writeToFile(resource: BaseResource): File { + return File(context.filesDir, resource.id).apply { this.parentFile?.mkdirs() - writeText(jsonParser.encodeResourceToString(metadataResource)) + writeText(jsonParser.encodeResourceToString(resource)) } } } diff --git a/workflow-testing/src/main/resources/plan-definition/cql-applicability-condition/care_plan.json b/workflow-testing/src/main/resources/plan-definition/cql-applicability-condition/care_plan.json index 131bb2fc7d..3779b876d0 100644 --- a/workflow-testing/src/main/resources/plan-definition/cql-applicability-condition/care_plan.json +++ b/workflow-testing/src/main/resources/plan-definition/cql-applicability-condition/care_plan.json @@ -1,9 +1,9 @@ { "resourceType": "CarePlan", - "id": "17", + "id": "Plan-Definition-Example", "contained": [ { "resourceType": "RequestGroup", - "id": "17", + "id": "Plan-Definition-Example", "instantiatesCanonical": [ "http://example.com/PlanDefinition/Plan-Definition-Example" ], "status": "draft", "intent": "proposal", @@ -20,15 +20,15 @@ } } ], "resource": { - "reference": "Task/16" + "reference": "Task/Activity-Example" } } ] }, { "resourceType": "Task", - "id": "16", + "id": "Activity-Example", "instantiatesCanonical": "http://example.com/ActivityDefinition/Activity-Example", "basedOn": [ { - "reference": "RequestGroup/17" + "reference": "RequestGroup/Plan-Definition-Example" } ], "status": "draft", "intent": "proposal", @@ -45,7 +45,7 @@ }, "activity": [ { "reference": { - "reference": "#RequestGroup/17" + "reference": "#RequestGroup/Plan-Definition-Example" } } ] } \ No newline at end of file diff --git a/workflow-testing/src/main/resources/plan-definition/med-request/med_request_careplan.json b/workflow-testing/src/main/resources/plan-definition/med-request/med_request_careplan.json index 8e7b401c7b..f152723d04 100644 --- a/workflow-testing/src/main/resources/plan-definition/med-request/med_request_careplan.json +++ b/workflow-testing/src/main/resources/plan-definition/med-request/med_request_careplan.json @@ -1,10 +1,10 @@ { "resourceType": "CarePlan", - "id": "17", + "id": "MedRequest-Example", "contained": [ { "resourceType": "RequestGroup", - "id": "17", + "id": "MedRequest-Example", "instantiatesCanonical": [ "http://localhost/PlanDefinition/MedRequest-Example" ], @@ -18,14 +18,14 @@ "id": "medication-action-1", "title": "Administer Medication 1", "resource": { - "reference": "medication-action-1-16" + "reference": "medication-action-1-MedicationRequest-1" } } ] }, { "resourceType": "MedicationRequest", - "id": "medication-action-1-16", + "id": "medication-action-1-MedicationRequest-1", "status": "draft", "intent": "order", "medicationCodeableConcept": { @@ -50,7 +50,7 @@ "activity": [ { "reference": { - "reference": "#RequestGroup/17" + "reference": "#RequestGroup/MedRequest-Example" } } ] diff --git a/workflow/benchmark/src/androidTest/java/com/google/android/fhir/workflow/benchmark/F_CqlEvaluatorBenchmark.kt b/workflow/benchmark/src/androidTest/java/com/google/android/fhir/workflow/benchmark/F_CqlEvaluatorBenchmark.kt index dc63e1fc5c..4c3d40fbe1 100644 --- a/workflow/benchmark/src/androidTest/java/com/google/android/fhir/workflow/benchmark/F_CqlEvaluatorBenchmark.kt +++ b/workflow/benchmark/src/androidTest/java/com/google/android/fhir/workflow/benchmark/F_CqlEvaluatorBenchmark.kt @@ -68,7 +68,7 @@ class F_CqlEvaluatorBenchmark { for (entry in patientImmunizationHistory.entry) { fhirEngine.create(entry.resource) } - knowledgeManager.install( + knowledgeManager.index( File(context.filesDir, lib.name).apply { writeText(jsonParser.encodeResourceToString(lib)) }, diff --git a/workflow/build.gradle.kts b/workflow/build.gradle.kts index 83e5d49f56..3d2696e944 100644 --- a/workflow/build.gradle.kts +++ b/workflow/build.gradle.kts @@ -114,8 +114,18 @@ dependencies { testImplementation(libs.androidx.test.core) testImplementation(libs.junit) testImplementation(libs.truth) - testImplementation(project(mapOf("path" to ":knowledge"))) testImplementation(project(":workflow-testing")) + testImplementation(project(":knowledge")) + + configurations.all { + if (name.contains("test", ignoreCase = true)) { + resolutionStrategy.dependencySubstitution { + // To test the workflow library against the latest Knowledge Manager APIs, substitute the + // dependency on the released Knowledge Manager library with the current build. + substitute(module(Dependencies.androidFhirKnowledge)).using(project(":knowledge")) + } + } + } constraints { Dependencies.hapiFhirConstraints().forEach { (libName, constraints) -> diff --git a/workflow/sampledata/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/CarePlan/CarePlan.json b/workflow/sampledata/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/CarePlan/CarePlan.json index 8996cb92fc..50b5053423 100644 --- a/workflow/sampledata/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/CarePlan/CarePlan.json +++ b/workflow/sampledata/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/CarePlan/CarePlan.json @@ -1,10 +1,10 @@ { "resourceType": "CarePlan", - "id": "26", + "id": "IMMZD2DTMeasles", "contained": [ { "resourceType": "RequestGroup", - "id": "26", + "id": "IMMZD2DTMeasles", "instantiatesCanonical": [ "http://fhir.org/guides/who/smart-immunization/PlanDefinition/IMMZD2DTMeasles|0.1.0" ], @@ -28,14 +28,14 @@ } ], "resource": { - "reference": "MedicationRequest/1" + "reference": "MedicationRequest/IMMZD2DTMeaslesMR" } } ] }, { "resourceType": "MedicationRequest", - "id": "1", + "id": "IMMZD2DTMeaslesMR", "meta": { "profile": [ "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-immunizationrequest" @@ -77,7 +77,7 @@ "activity": [ { "reference": { - "reference": "#RequestGroup/26" + "reference": "#RequestGroup/IMMZD2DTMeasles" } } ] diff --git a/workflow/src/androidTest/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateTest.kt b/workflow/src/androidTest/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateTest.kt index 345bc28049..f42ecfb020 100644 --- a/workflow/src/androidTest/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateTest.kt +++ b/workflow/src/androidTest/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateTest.kt @@ -121,7 +121,7 @@ class FhirOperatorLibraryEvaluateTest { } // Load Library that checks if Patient has taken a vaccine - knowledgeManager.install(copy("/immunity-check/ImmunityCheck.json")) + knowledgeManager.index(copy("/immunity-check/ImmunityCheck.json")) // Evaluates a specific Patient val results = @@ -142,8 +142,8 @@ class FhirOperatorLibraryEvaluateTest { } // Load Library that checks if Patient has taken a vaccine - knowledgeManager.install(copy("/immunity-check/ImmunityCheck.json")) - knowledgeManager.install(copy("/immunity-check/FhirHelpers.json")) + knowledgeManager.index(copy("/immunity-check/ImmunityCheck.json")) + knowledgeManager.index(copy("/immunity-check/FhirHelpers.json")) val location = """ @@ -185,7 +185,7 @@ class FhirOperatorLibraryEvaluateTest { } // Load Library that checks if Patient has taken a vaccine - knowledgeManager.install(copy("/immunity-check/ImmunityCheck.json")) + knowledgeManager.index(copy("/immunity-check/ImmunityCheck.json")) // Evaluates a specific Patient val results = diff --git a/workflow/src/androidTest/java/com/google/android/fhir/workflow/SmartImmunizationAndroidTest.kt b/workflow/src/androidTest/java/com/google/android/fhir/workflow/SmartImmunizationAndroidTest.kt index 32f88a275a..f562b3ae13 100644 --- a/workflow/src/androidTest/java/com/google/android/fhir/workflow/SmartImmunizationAndroidTest.kt +++ b/workflow/src/androidTest/java/com/google/android/fhir/workflow/SmartImmunizationAndroidTest.kt @@ -116,7 +116,7 @@ class SmartImmunizationAndroidTest { moveAllIGResourcesIntoFilesDir("smart-imm") - knowledgeManager.install( + knowledgeManager.import( FhirNpmPackage( "who.fhir.immunization", "1.0.0", @@ -136,8 +136,6 @@ class SmartImmunizationAndroidTest { ) .single() - assertThat(planDef.idElement.idPart).isEqualTo("26") - val patient = load( "/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/Patient/Patient-IMMZ-Patient-NoVaxeninfant-f.json", diff --git a/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateJavaTest.kt b/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateJavaTest.kt index be66a40615..96e6982b9d 100644 --- a/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateJavaTest.kt +++ b/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorLibraryEvaluateJavaTest.kt @@ -110,8 +110,8 @@ class FhirOperatorLibraryEvaluateJavaTest { } // Load Library that checks if Patient has taken a vaccine - knowledgeManager.install(writeToFile(load("/immunity-check/ImmunityCheck.json") as Library)) - knowledgeManager.install(writeToFile(load("/immunity-check/FhirHelpers.json") as Library)) + knowledgeManager.index(writeToFile(load("/immunity-check/ImmunityCheck.json") as Library)) + knowledgeManager.index(writeToFile(load("/immunity-check/FhirHelpers.json") as Library)) // Evaluates a specific Patient val results = @@ -137,7 +137,7 @@ class FhirOperatorLibraryEvaluateJavaTest { val library = CqlBuilder.assembleFhirLib(cql, null, null, "TestGetName", "1.0.0") - knowledgeManager.install(writeToFile(library)) + knowledgeManager.index(writeToFile(library)) // Evaluates expression without any extra data val results = @@ -162,7 +162,7 @@ class FhirOperatorLibraryEvaluateJavaTest { val library = CqlBuilder.assembleFhirLib(cql, null, null, "TestSumWithParams", "1.0.0") - knowledgeManager.install(writeToFile(library)) + knowledgeManager.index(writeToFile(library)) val params = Parameters().apply { diff --git a/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorTest.kt b/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorTest.kt index 287faceef8..f9b4ff4ff2 100644 --- a/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorTest.kt +++ b/workflow/src/test/java/com/google/android/fhir/workflow/FhirOperatorTest.kt @@ -61,7 +61,7 @@ class FhirOperatorTest { // Installing ANC CDS to the IGManager val rootDirectory = File(javaClass.getResource("/anc-cds")!!.file) - knowledgeManager.install( + knowledgeManager.import( FhirNpmPackage( "com.google.android.fhir", "1.0.0", @@ -281,7 +281,9 @@ class FhirOperatorTest { } private suspend fun installToIgManager(resource: Resource) { - knowledgeManager.install(writeToFile(resource)) + try { + knowledgeManager.index(writeToFile(resource)) + } catch (_: Exception) {} } private fun writeToFile(resource: Resource): File { diff --git a/workflow/src/test/java/com/google/android/fhir/workflow/SmartImmunizationTest.kt b/workflow/src/test/java/com/google/android/fhir/workflow/SmartImmunizationTest.kt index 85d46f13d2..cdfde7e17f 100644 --- a/workflow/src/test/java/com/google/android/fhir/workflow/SmartImmunizationTest.kt +++ b/workflow/src/test/java/com/google/android/fhir/workflow/SmartImmunizationTest.kt @@ -63,7 +63,7 @@ class SmartImmunizationTest { // Installing SmartImmunizations IG into the IGManager val rootDirectory = File(javaClass.getResource("/smart-imm/ig/")!!.file) - knowledgeManager.install( + knowledgeManager.import( FhirNpmPackage( "who.fhir.immunization", "1.0.0", @@ -83,8 +83,6 @@ class SmartImmunizationTest { ) .firstOrNull() - assertThat(planDef?.idElement?.idPart).isEqualTo("26") - loader.loadFile( "/smart-imm/tests/IMMZ-Patient-NoVaxeninfant-f/Patient/Patient-IMMZ-Patient-NoVaxeninfant-f.json", ::importToFhirEngine, From f67501d4e0cdebfa76256ae0ccac9650c7d02237 Mon Sep 17 00:00:00 2001 From: Kostia Tarasenko Date: Tue, 20 Aug 2024 19:37:03 +0200 Subject: [PATCH 02/13] Update CODEOWNERS to use group instead of individuals (#2659) * Update CODEOWNERS to use group instead of individuals * Update CODEOWNERS - remove comment --- CODEOWNERS | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 512f12d413..f6c08701fd 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,14 +1,7 @@ # This file lists people who get automatically added as Reviewers for Pull Requests. -# -# Note that we intentionally do NOT just include all committers here by using @google/android-fhir, -# nor were we able to get it working using a group such as @google/android-fhir-reviewers; -# details about why are described on https://github.com/google/android-fhir/issues/2320 -# and https://github.com/google/android-fhir/pull/2536. -# -# See https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners # These people for *ALL* Pull Requests: -* @aditya-07 @jingtang10 @MJ1998 @santosh-pingle +* @google/android-fhir-reviewers # These for anything documentation related: docs/* @vorburger From d636c57bcbbba1063e47cecf7bac7381f173bb38 Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 21 Aug 2024 11:36:01 +0100 Subject: [PATCH 03/13] Clean up UI navigation item (#2658) --- .../fhir/datacapture/QuestionnaireFragment.kt | 8 +- .../QuestionnaireNavigationViewUIState.kt | 6 +- .../datacapture/QuestionnaireViewModel.kt | 19 +- .../datacapture/QuestionnaireViewModelTest.kt | 277 +++++++----------- 4 files changed, 123 insertions(+), 187 deletions(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt index 457d8230ad..b6c357d107 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt @@ -160,10 +160,10 @@ class QuestionnaireFragment : Fragment() { } // Set bottom navigation - if (state.bottomNavItems.isNotEmpty()) { + if (state.bottomNavItem != null) { bottomNavContainerFrame.visibility = View.VISIBLE NavigationViewHolder(bottomNavContainerFrame) - .bind(state.bottomNavItems.single().questionnaireNavigationUIState) + .bind(state.bottomNavItem.questionnaireNavigationUIState) } else { bottomNavContainerFrame.visibility = View.GONE } @@ -179,10 +179,10 @@ class QuestionnaireFragment : Fragment() { reviewModeEditButton.visibility = View.GONE // Set bottom navigation - if (state.bottomNavItems.isNotEmpty()) { + if (state.bottomNavItem != null) { bottomNavContainerFrame.visibility = View.VISIBLE NavigationViewHolder(bottomNavContainerFrame) - .bind(state.bottomNavItems.single().questionnaireNavigationUIState) + .bind(state.bottomNavItem.questionnaireNavigationUIState) } else { bottomNavContainerFrame.visibility = View.GONE } diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireNavigationViewUIState.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireNavigationViewUIState.kt index c33a25785d..60bd9e1c7b 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireNavigationViewUIState.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireNavigationViewUIState.kt @@ -16,11 +16,11 @@ package com.google.android.fhir.datacapture -sealed class QuestionnaireNavigationViewUIState(val isShown: Boolean, val isEnabled: Boolean) { - data object Hidden : QuestionnaireNavigationViewUIState(isShown = false, isEnabled = false) +sealed interface QuestionnaireNavigationViewUIState { + data object Hidden : QuestionnaireNavigationViewUIState data class Enabled(val labelText: String? = null, val onClickAction: () -> Unit) : - QuestionnaireNavigationViewUIState(isShown = true, isEnabled = true) + QuestionnaireNavigationViewUIState } data class QuestionnaireNavigationUIState( diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt index 95d75b8460..fbf425a8bc 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt @@ -593,7 +593,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat QuestionnaireState( items = emptyList(), displayMode = DisplayMode.InitMode, - bottomNavItems = emptyList(), + bottomNavItem = null, ), ) @@ -743,13 +743,12 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat QuestionnaireNavigationViewUIState.Hidden }, ) - val bottomNavigationItems = - listOf(QuestionnaireAdapterItem.Navigation(bottomNavigationViewState)) + val bottomNavigation = QuestionnaireAdapterItem.Navigation(bottomNavigationViewState) return QuestionnaireState( items = if (shouldSetNavigationInLongScroll) { - questionnaireItemViewItems + bottomNavigationItems + questionnaireItemViewItems + bottomNavigation } else { questionnaireItemViewItems }, @@ -758,8 +757,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat showEditButton = !isReadOnly, showNavAsScroll = shouldSetNavigationInLongScroll, ), - bottomNavItems = - if (!shouldSetNavigationInLongScroll) bottomNavigationItems else emptyList(), + bottomNavItem = if (!shouldSetNavigationInLongScroll) bottomNavigation else null, ) } @@ -833,18 +831,17 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat QuestionnaireNavigationViewUIState.Hidden }, ) - val bottomNavigationItems = - listOf(QuestionnaireAdapterItem.Navigation(bottomNavigationUiViewState)) + val bottomNavigation = QuestionnaireAdapterItem.Navigation(bottomNavigationUiViewState) return QuestionnaireState( items = if (shouldSetNavigationInLongScroll) { - questionnaireItemViewItems + bottomNavigationItems + questionnaireItemViewItems + bottomNavigation } else { questionnaireItemViewItems }, displayMode = DisplayMode.EditMode(questionnairePagination, shouldSetNavigationInLongScroll), - bottomNavItems = if (!shouldSetNavigationInLongScroll) bottomNavigationItems else emptyList(), + bottomNavItem = if (!shouldSetNavigationInLongScroll) bottomNavigation else null, ) } @@ -1136,7 +1133,7 @@ typealias ItemToParentMap = MutableMap, val displayMode: DisplayMode, - val bottomNavItems: List, + val bottomNavItem: QuestionnaireAdapterItem.Navigation?, ) internal sealed class DisplayMode { diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt index f4de30897b..e31854a0af 100644 --- a/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt +++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt @@ -997,8 +997,8 @@ class QuestionnaireViewModelTest { """ .trimIndent() - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, questionnaireString) - state.set(EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, questionnaireResponseString) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = questionnaireString + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = questionnaireResponseString val viewModel = QuestionnaireViewModel(context, state) runTest { val value = viewModel.getQuestionnaireResponse() @@ -1125,8 +1125,8 @@ class QuestionnaireViewModelTest { """ .trimIndent() - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, questionnaireString) - state.set(EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, questionnaireResponseString) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = questionnaireString + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = questionnaireResponseString val viewModel = QuestionnaireViewModel(context, state) runTest { val value = viewModel.getQuestionnaireResponse() @@ -1321,7 +1321,7 @@ class QuestionnaireViewModelTest { } val serializedQuestionnaire = printer.encodeResourceToString(questionnaire) - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, serializedQuestionnaire) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = serializedQuestionnaire val viewModel = QuestionnaireViewModel(context, state) @@ -1346,7 +1346,7 @@ class QuestionnaireViewModelTest { ) } val serializedQuestionnaire = printer.encodeResourceToString(questionnaire) - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, serializedQuestionnaire) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = serializedQuestionnaire val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -1987,11 +1987,8 @@ class QuestionnaireViewModelTest { ) assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navReview - .isShown, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navReview + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2240,11 +2237,8 @@ class QuestionnaireViewModelTest { ), ) assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navSubmit - .isShown, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navSubmit + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2360,11 +2354,8 @@ class QuestionnaireViewModelTest { ), ) assertThat( - questionnaireState1.bottomNavItems - .single() - .questionnaireNavigationUIState - .navSubmit - .isShown, + questionnaireState1.bottomNavItem!!.questionnaireNavigationUIState.navSubmit + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() @@ -2382,11 +2373,8 @@ class QuestionnaireViewModelTest { ), ) assertThat( - questionnaireState2.bottomNavItems - .single() - .questionnaireNavigationUIState - .navSubmit - .isShown, + questionnaireState2.bottomNavItem!!.questionnaireNavigationUIState.navSubmit + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2414,13 +2402,10 @@ class QuestionnaireViewModelTest { val viewModel = createQuestionnaireViewModel(questionnaire, enableReviewPage = false) val questionnaireState = viewModel.questionnaireStateFlow.first() assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navReview - .isShown, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navReview + is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } } @@ -2445,13 +2430,10 @@ class QuestionnaireViewModelTest { ) val questionnaireState = viewModel.questionnaireStateFlow.first() assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navReview - .isShown, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navReview + is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } } @@ -2471,11 +2453,8 @@ class QuestionnaireViewModelTest { val viewModel = createQuestionnaireViewModel(questionnaire, enableReviewPage = true) val questionnaireState = viewModel.questionnaireStateFlow.first() assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navReview - .isShown, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navReview + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2580,13 +2559,11 @@ class QuestionnaireViewModelTest { viewModel.runViewModelBlocking { viewModel.goToNextPage() assertThat( - viewModel.questionnaireStateFlow.value.bottomNavItems - .single() + viewModel.questionnaireStateFlow.value.bottomNavItem!! .questionnaireNavigationUIState - .navReview - .isShown, + .navReview is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } } @@ -2629,13 +2606,11 @@ class QuestionnaireViewModelTest { val viewModel = createQuestionnaireViewModel(questionnaire, enableReviewPage = false) viewModel.runViewModelBlocking { assertThat( - viewModel.questionnaireStateFlow.value.bottomNavItems - .single() + viewModel.questionnaireStateFlow.value.bottomNavItem!! .questionnaireNavigationUIState - .navReview - .isShown, + .navReview is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } } @@ -2678,11 +2653,9 @@ class QuestionnaireViewModelTest { viewModel.runViewModelBlocking { viewModel.goToNextPage() assertThat( - viewModel.questionnaireStateFlow.value.bottomNavItems - .single() + viewModel.questionnaireStateFlow.value.bottomNavItem!! .questionnaireNavigationUIState - .navReview - .isShown, + .navReview is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2727,11 +2700,9 @@ class QuestionnaireViewModelTest { val viewModel = createQuestionnaireViewModel(questionnaire, enableReviewPage = true) viewModel.runViewModelBlocking { assertThat( - viewModel.questionnaireStateFlow.value.bottomNavItems - .single() + viewModel.questionnaireStateFlow.value.bottomNavItem!! .questionnaireNavigationUIState - .navReview - .isShown, + .navReview is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2755,11 +2726,9 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navReview - .isShown, + .navReview is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2795,7 +2764,7 @@ class QuestionnaireViewModelTest { Questionnaire().apply { id = "a-questionnaire" addItem( - Questionnaire.QuestionnaireItemComponent().apply { + QuestionnaireItemComponent().apply { linkId = "a-linkId" type = Questionnaire.QuestionnaireItemType.BOOLEAN }, @@ -2849,7 +2818,7 @@ class QuestionnaireViewModelTest { Questionnaire().apply { id = "a-questionnaire" addItem( - Questionnaire.QuestionnaireItemComponent().apply { + QuestionnaireItemComponent().apply { linkId = "a-linkId" type = Questionnaire.QuestionnaireItemType.BOOLEAN }, @@ -2889,13 +2858,11 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navSubmit - .isShown, + .navSubmit is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } @Test @@ -2914,11 +2881,9 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navSubmit - .isShown, + .navSubmit is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2939,11 +2904,9 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navSubmit - .isShown, + .navSubmit is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -2970,13 +2933,11 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } @Test @@ -2995,11 +2956,9 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -3020,13 +2979,11 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } @Test @@ -3068,13 +3025,11 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } @Test @@ -3116,11 +3071,9 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -3164,13 +3117,11 @@ class QuestionnaireViewModelTest { assertThat( viewModel.questionnaireStateFlow .first() - .bottomNavItems - .single() + .bottomNavItem!! .questionnaireNavigationUIState - .navCancel - .isShown, + .navCancel is QuestionnaireNavigationViewUIState.Hidden, ) - .isFalse() + .isTrue() } // ==================================================================== // // // @@ -3185,7 +3136,7 @@ class QuestionnaireViewModelTest { Questionnaire().apply { id = "a-questionnaire" addItem( - Questionnaire.QuestionnaireItemComponent().apply { + QuestionnaireItemComponent().apply { linkId = "a-linkId" type = Questionnaire.QuestionnaireItemType.BOOLEAN }, @@ -3193,11 +3144,15 @@ class QuestionnaireViewModelTest { } val viewModel = createQuestionnaireViewModel(questionnaire, showNavigationInLongScroll = true) val questionnaireState = viewModel.questionnaireStateFlow.first() - assertThat(questionnaireState.bottomNavItems.isEmpty()).isTrue() + assertThat(questionnaireState.bottomNavItem).isNull() assertThat(questionnaireState.items.last()) .isInstanceOf(QuestionnaireAdapterItem.Navigation::class.java) val navigationItem = questionnaireState.items.last() as QuestionnaireAdapterItem.Navigation - assertThat(navigationItem.questionnaireNavigationUIState.navSubmit.isEnabled).isTrue() + assertThat( + navigationItem.questionnaireNavigationUIState.navSubmit + is QuestionnaireNavigationViewUIState.Enabled, + ) + .isTrue() } fun `EXTRA_SHOW_NAVIGATION_IN_DEFAULT_LONG_SCROLL not setting should not add navigation item to questionnaireState items`() = @@ -3206,7 +3161,7 @@ class QuestionnaireViewModelTest { Questionnaire().apply { id = "a-questionnaire" addItem( - Questionnaire.QuestionnaireItemComponent().apply { + QuestionnaireItemComponent().apply { linkId = "a-linkId" type = Questionnaire.QuestionnaireItemType.BOOLEAN }, @@ -3216,13 +3171,9 @@ class QuestionnaireViewModelTest { val questionnaireState = viewModel.questionnaireStateFlow.first() assertThat(questionnaireState.items.map { it::class.java }) .doesNotContain(QuestionnaireAdapterItem.Navigation::class.java) - assertThat(questionnaireState.bottomNavItems.isNotEmpty()).isTrue() assertThat( - questionnaireState.bottomNavItems - .single() - .questionnaireNavigationUIState - .navSubmit - .isEnabled, + questionnaireState.bottomNavItem!!.questionnaireNavigationUIState.navSubmit + is QuestionnaireNavigationViewUIState.Enabled, ) .isTrue() } @@ -3399,7 +3350,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) @@ -3444,7 +3395,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) @@ -3504,7 +3455,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val questionnaireResponse = QuestionnaireResponse().apply { @@ -3530,10 +3481,8 @@ class QuestionnaireViewModelTest { }, ) } - state.set( - EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, - printer.encodeResourceToString(questionnaireResponse), - ) + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = + printer.encodeResourceToString(questionnaireResponse) val viewModel = QuestionnaireViewModel(context, state) @@ -4087,8 +4036,8 @@ class QuestionnaireViewModelTest { """ .trimIndent() - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, questionnaireString) - state.set(EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, questionnaireResponseString) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = questionnaireString + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = questionnaireResponseString val viewModel = QuestionnaireViewModel(context, state) runTest { val value = viewModel.getQuestionnaireResponse() @@ -4185,8 +4134,8 @@ class QuestionnaireViewModelTest { """ .trimIndent() - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, questionnaireString) - state.set(EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, questionnaireResponseString) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = questionnaireString + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = questionnaireResponseString val viewModel = QuestionnaireViewModel(context, state) viewModel.clearAllAnswers() runTest { @@ -4828,11 +4777,9 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) - state.set( - EXTRA_QUESTIONNAIRE_LAUNCH_CONTEXT_MAP, - mapOf("patient" to printer.encodeResourceToString(patient)), - ) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) + state[EXTRA_QUESTIONNAIRE_LAUNCH_CONTEXT_MAP] = + mapOf("patient" to printer.encodeResourceToString(patient)) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -4886,7 +4833,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -4930,7 +4877,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) val exception = assertThrows(IllegalStateException::class.java) { @@ -4987,7 +4934,7 @@ class QuestionnaireViewModelTest { ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -5039,7 +4986,7 @@ class QuestionnaireViewModelTest { ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -5091,7 +5038,7 @@ class QuestionnaireViewModelTest { ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -5142,7 +5089,7 @@ class QuestionnaireViewModelTest { ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -6883,7 +6830,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -6933,7 +6880,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -6988,7 +6935,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -7043,7 +6990,7 @@ class QuestionnaireViewModelTest { }, ) } - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) val viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -7170,10 +7117,8 @@ class QuestionnaireViewModelTest { } } - state.set( - EXTRA_QUESTIONNAIRE_JSON_STRING, - printer.encodeResourceToString(questionnaire(emptyList())), - ) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = + printer.encodeResourceToString(questionnaire(emptyList())) // empty initial value var viewModel = QuestionnaireViewModel(context, state) @@ -7187,16 +7132,14 @@ class QuestionnaireViewModelTest { } // initial value is set to false - state.set( - EXTRA_QUESTIONNAIRE_JSON_STRING, + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString( questionnaire( listOf( Questionnaire.QuestionnaireItemInitialComponent().apply { value = BooleanType(false) }, ), ), - ), - ) + ) viewModel = QuestionnaireViewModel(context, state) var enabledDisplayItems: List @@ -7211,16 +7154,14 @@ class QuestionnaireViewModelTest { } // initial value is set to true - state.set( - EXTRA_QUESTIONNAIRE_JSON_STRING, + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString( questionnaire( listOf( Questionnaire.QuestionnaireItemInitialComponent().apply { value = BooleanType(true) }, ), ), - ), - ) + ) viewModel = QuestionnaireViewModel(context, state) viewModel.runViewModelBlocking { @@ -7274,9 +7215,9 @@ class QuestionnaireViewModelTest { val job = this.launch { - viewModel.questionnaireStateFlow.collect { + viewModel.questionnaireStateFlow.collect { questionnaireState -> descriptionResponseItem = - it.items + questionnaireState.items .find { it.asQuestion().questionnaireItem.linkId == "a-description" }!! .asQuestion() this@launch.cancel() @@ -7381,20 +7322,18 @@ class QuestionnaireViewModelTest { showCancelButton: Boolean? = null, showNavigationInLongScroll: Boolean = false, ): QuestionnaireViewModel { - state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire)) + state[EXTRA_QUESTIONNAIRE_JSON_STRING] = printer.encodeResourceToString(questionnaire) questionnaireResponse?.let { - state.set( - EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING, - printer.encodeResourceToString(questionnaireResponse), - ) + state[EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING] = + printer.encodeResourceToString(questionnaireResponse) } - enableReviewPage.let { state.set(EXTRA_ENABLE_REVIEW_PAGE, it) } - showReviewPageFirst.let { state.set(EXTRA_SHOW_REVIEW_PAGE_FIRST, it) } - readOnlyMode.let { state.set(EXTRA_READ_ONLY, it) } - showSubmitButton?.let { state.set(EXTRA_SHOW_SUBMIT_BUTTON, it) } - showCancelButton?.let { state.set(EXTRA_SHOW_CANCEL_BUTTON, it) } - showNavigationInLongScroll.let { state.set(EXTRA_SHOW_NAVIGATION_IN_DEFAULT_LONG_SCROLL, it) } + enableReviewPage.let { state[EXTRA_ENABLE_REVIEW_PAGE] = it } + showReviewPageFirst.let { state[EXTRA_SHOW_REVIEW_PAGE_FIRST] = it } + readOnlyMode.let { state[EXTRA_READ_ONLY] = it } + showSubmitButton?.let { state[EXTRA_SHOW_SUBMIT_BUTTON] = it } + showCancelButton?.let { state[EXTRA_SHOW_CANCEL_BUTTON] = it } + showNavigationInLongScroll.let { state[EXTRA_SHOW_NAVIGATION_IN_DEFAULT_LONG_SCROLL] = it } return QuestionnaireViewModel(context, state) } From c87059f5b2679699082e09f7c5562855f82f0b40 Mon Sep 17 00:00:00 2001 From: FikriMilano Date: Thu, 22 Aug 2024 23:02:49 +0700 Subject: [PATCH 04/13] Add missing response items to repeated group child items (#2657) * Add missing response items to repeated group child items * Update kdoc * spotless --- .../datacapture/QuestionnaireViewModel.kt | 15 +- .../datacapture/QuestionnaireViewModelTest.kt | 135 ++++++++++++++++++ 2 files changed, 147 insertions(+), 3 deletions(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt index fbf425a8bc..35ca593755 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt @@ -419,10 +419,11 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat * Adds empty [QuestionnaireResponseItemComponent]s to `responseItems` so that each * [QuestionnaireItemComponent] in `questionnaireItems` has at least one corresponding * [QuestionnaireResponseItemComponent]. This is because user-provided [QuestionnaireResponse] - * might not contain answers to unanswered or disabled questions. Note : this only applies to - * [QuestionnaireItemComponent]s nested under a group. + * might not contain answers to unanswered or disabled questions. This function should only be + * used for unpacked questionnaire. */ - private fun addMissingResponseItems( + @VisibleForTesting + internal fun addMissingResponseItems( questionnaireItems: List, responseItems: MutableList, ) { @@ -446,6 +447,14 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat responseItems = responseItemMap[it.linkId]!!.single().item, ) } + if (it.type == Questionnaire.QuestionnaireItemType.GROUP && it.repeats) { + responseItemMap[it.linkId]!!.forEach { rItem -> + addMissingResponseItems( + questionnaireItems = it.item, + responseItems = rItem.item, + ) + } + } responseItems.addAll(responseItemMap[it.linkId]!!) } } diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt index e31854a0af..e644e4c6d6 100644 --- a/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt +++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt @@ -1138,6 +1138,141 @@ class QuestionnaireViewModelTest { } } + @Test + fun `should add missing response item inside a repeated group`() { + val questionnaireString = + """ + { + "resourceType": "Questionnaire", + "item": [ + { + "linkId": "1", + "type": "group", + "text": "Repeated Group", + "repeats": true, + "item": [ + { + "linkId": "1-1", + "type": "date", + "extension": [ + { + "url": "http://hl7.org/fhir/StructureDefinition/entryFormat", + "valueString": "yyyy-mm-dd" + } + ] + }, + { + "linkId": "1-2", + "type": "boolean" + } + ] + } + ] + } + """ + .trimIndent() + + val questionnaireResponseString = + """ + { + "resourceType": "QuestionnaireResponse", + "item": [ + { + "linkId": "1", + "text": "Repeated Group", + "item": [ + { + "linkId": "1-1", + "answer": [ + { + "valueDate": "2023-06-14" + } + ] + } + ] + }, + { + "linkId": "1", + "text": "Repeated Group", + "item": [ + { + "linkId": "1-1", + "answer": [ + { + "valueDate": "2023-06-13" + } + ] + } + ] + } + ] + } + """ + .trimIndent() + + val expectedQuestionnaireResponseString = + """ + { + "resourceType": "QuestionnaireResponse", + "item": [ + { + "linkId": "1", + "text": "Repeated Group", + "item": [ + { + "linkId": "1-1", + "answer": [ + { + "valueDate": "2023-06-14" + } + ] + }, + { + "linkId": "1-2" + } + ] + }, + { + "linkId": "1", + "text": "Repeated Group", + "item": [ + { + "linkId": "1-1", + "answer": [ + { + "valueDate": "2023-06-13" + } + ] + }, + { + "linkId": "1-2" + } + ] + } + ] + } + """ + .trimIndent() + + val questionnaire = + printer.parseResource(Questionnaire::class.java, questionnaireString) as Questionnaire + + val response = + printer.parseResource(QuestionnaireResponse::class.java, questionnaireResponseString) + as QuestionnaireResponse + + val expectedResponse = + printer.parseResource(QuestionnaireResponse::class.java, expectedQuestionnaireResponseString) + as QuestionnaireResponse + + val viewModel = createQuestionnaireViewModel(questionnaire, response) + + runTest { + viewModel.addMissingResponseItems(questionnaire.item, response.item) + assertResourceEquals(response, expectedResponse) + } + } + // ==================================================================== // // // // Questionnaire State Flow // From cbde1a9c52d0070e5d9c771559431e614340e3a1 Mon Sep 17 00:00:00 2001 From: FikriMilano Date: Fri, 23 Aug 2024 00:45:11 +0700 Subject: [PATCH 05/13] Add flyover text as secondary dialog title (#2648) * Add flyover text as secondary dialog title * spotless * Add doc --- .../views/factories/DialogSelectViewHolderFactory.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt index 123696c46f..92bc81c359 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt @@ -92,7 +92,10 @@ internal object QuestionnaireItemDialogSelectViewHolderFactory : View.OnClickListener { val fragment = OptionSelectDialogFragment( - title = questionnaireItem.localizedTextSpanned ?: "", + // We use the question text for the dialog title. If there is no question text, we + // use flyover text as it is sometimes used in text fields instead of question text. + title = questionnaireItem.localizedTextSpanned + ?: questionnaireItem.localizedFlyoverSpanned ?: "", config = questionnaireItem.buildConfig(), selectedOptions = selectedOptions, ) From 83fddb902ba4d128d018043116dbe20a56730f17 Mon Sep 17 00:00:00 2001 From: Madhuram Jajoo Date: Mon, 26 Aug 2024 15:11:07 +0530 Subject: [PATCH 06/13] Small changes for more code readability (#2656) * Small changes for more code readability * Updating more kdocs * correcting kdoc format --- .../fhir/db/impl/dao/LocalChangeDaoTest.kt | 4 +- .../com/google/android/fhir/db/Database.kt | 12 ++-- .../android/fhir/db/impl/DatabaseImpl.kt | 36 ++++------ .../fhir/db/impl/dao/LocalChangeDao.kt | 72 ++++++++++++------- 4 files changed, 71 insertions(+), 53 deletions(-) diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/dao/LocalChangeDaoTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/dao/LocalChangeDaoTest.kt index d99c71d21d..b4280911c0 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/dao/LocalChangeDaoTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/db/impl/dao/LocalChangeDaoTest.kt @@ -296,7 +296,7 @@ class LocalChangeDaoTest { localChangeDao.updateResourceIdAndReferences( resourceUuid = patientResourceUuid, oldResource = patient, - updatedResource = updatedPatient, + updatedResourceId = updatedPatient.logicalId, ) // assert that Patient's new ID is reflected in the Patient Resource Change @@ -387,7 +387,7 @@ class LocalChangeDaoTest { localChangeDao.updateResourceIdAndReferences( patientResourceUuid, oldResource = localPatient, - updatedResource = updatedLocalPatient, + updatedResourceId = updatedLocalPatient.logicalId, ) assertThat(updatedReferences.size).isEqualTo(countAboveLimit) } diff --git a/engine/src/main/java/com/google/android/fhir/db/Database.kt b/engine/src/main/java/com/google/android/fhir/db/Database.kt index 251bf21b6c..2c72e0d401 100644 --- a/engine/src/main/java/com/google/android/fhir/db/Database.kt +++ b/engine/src/main/java/com/google/android/fhir/db/Database.kt @@ -125,11 +125,13 @@ internal interface Database { suspend fun deleteUpdates(resources: List) /** - * Updates the [ResourceEntity.serializedResource] and [ResourceEntity.resourceId] corresponding - * to the updatedResource. Updates all the [LocalChangeEntity] for this updated resource as well - * as all the [LocalChangeEntity] referring to this resource in their [LocalChangeEntity.payload] - * Updates the [ResourceEntity.serializedResource] for all the resources which refer to this - * updated resource. + * Updates the existing resource identified by [currentResourceId] with the [updatedResource], + * ensuring all associated references in the database are also updated accordingly. + * + * Implementations of this function should perform the following steps within a transaction: + * 1. Update the corresponding [ResourceEntity]. + * 2. Update associated [LocalChangeEntity] records. + * 3. Update the serialized representation of referring resources. */ suspend fun updateResourceAndReferences( currentResourceId: String, diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt index 7bf364f4e9..cf8ab22c07 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt @@ -290,11 +290,24 @@ internal class DatabaseImpl( val resourceUuid = currentResourceEntity.resourceUuid updateResourceEntity(resourceUuid, updatedResource) + /** + * Update LocalChange records and identify referring resources. + * + * We need to update LocalChange records first because they might contain references to the + * old resource ID that are not readily searchable or present in the latest version of the + * [ResourceEntity] itself. The [LocalChangeResourceReferenceEntity] table helps us identify + * these [LocalChangeEntity] records accurately. + * + * Once LocalChange records are updated, we can then safely update the corresponding + * ResourceEntity records to ensure data consistency. Hence, we obtain the + * [ResourceEntity.resourceUuid]s of the resources from the updated LocalChangeEntity records + * and use them in the next step. + */ val uuidsOfReferringResources = - updateLocalChangeResourceIdAndReferences( + localChangeDao.updateResourceIdAndReferences( resourceUuid = resourceUuid, oldResource = oldResource, - updatedResource = updatedResource, + updatedResourceId = updatedResource.logicalId, ) updateReferringResources( @@ -312,25 +325,6 @@ internal class DatabaseImpl( private suspend fun updateResourceEntity(resourceUuid: UUID, updatedResource: Resource) = resourceDao.updateResourceWithUuid(resourceUuid, updatedResource) - /** - * Update the [LocalChange]s to reflect the change in the resource ID. This primarily includes - * modifying the [LocalChange.resourceId] for the changes of the affected resource. Also, update - * any references in the [LocalChange] which refer to the affected resource. - * - * The function returns a [List<[UUID]>] which corresponds to the [ResourceEntity.resourceUuid] - * which contain references to the affected resource. - */ - private suspend fun updateLocalChangeResourceIdAndReferences( - resourceUuid: UUID, - oldResource: Resource, - updatedResource: Resource, - ) = - localChangeDao.updateResourceIdAndReferences( - resourceUuid = resourceUuid, - oldResource = oldResource, - updatedResource = updatedResource, - ) - /** * Update all [Resource] and their corresponding [ResourceEntity] which refer to the affected * resource. The update of the references in the [Resource] is also expected to reflect in the diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt index a078f80b65..7201905aa9 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt @@ -328,14 +328,14 @@ internal abstract class LocalChangeDao { @Query( """ - SELECT * + SELECT DISTINCT localChangeId FROM LocalChangeResourceReferenceEntity WHERE resourceReferenceValue = :resourceReferenceValue """, ) - abstract suspend fun getLocalChangeReferencesWithValue( + abstract suspend fun getLocalChangeIdsWithReferenceValue( resourceReferenceValue: String, - ): List + ): List @Query( """ @@ -365,21 +365,29 @@ internal abstract class LocalChangeDao { ) /** - * Updates the resource IDs of the [LocalChange] of the updated resource. Updates [LocalChange] - * with references to the updated resource. + * Updates the [LocalChangeEntity]s to reflect the change in the resource ID. + * + * This function performs the following steps: + * 1. Updates the `resourceId` in `LocalChange` entities directly related to the updated resource + * 2. Updates references within `LocalChange` payloads that point to the updated resource + * + * @param resourceUuid The UUID of the resource whose ID has changed + * @param oldResource The original resource with the old ID + * @param updatedResourceId The updated resource ID + * @return A list of UUIDs representing resources that reference the affected resource */ suspend fun updateResourceIdAndReferences( resourceUuid: UUID, oldResource: Resource, - updatedResource: Resource, + updatedResourceId: String, ): List { updateResourceIdInResourceLocalChanges( resourceUuid = resourceUuid, - updatedResourceId = updatedResource.logicalId, + updatedResourceId = updatedResourceId, ) return updateReferencesInLocalChange( oldResource = oldResource, - updatedResource = updatedResource, + updatedResourceId = updatedResourceId, ) } @@ -396,37 +404,51 @@ internal abstract class LocalChangeDao { } /** - * Looks for [LocalChangeEntity] which refer to the updated resource through - * [LocalChangeResourceReferenceEntity]. For each [LocalChangeEntity] which contains reference to - * the updated resource in its payload, we update the payload with the reference and also update - * the corresponding [LocalChangeResourceReferenceEntity]. We delete the original - * [LocalChangeEntity] and create a new one with new [LocalChangeResourceReferenceEntity]s in its - * place. This method returns a list of the [ResourceEntity.resourceUuid] for all the resources - * whose [LocalChange] contained references to the oldResource + * Updates references within [LocalChangeEntity] payloads to reflect a resource ID change. + * + * This function performs the following steps: + * 1. Retrieves [LocalChangeEntity] records that reference the old resource. + * 2. For each [LocalChangeEntity]: + * - Replaces the old resource reference with the new one in its payload. + * - Creates updated [LocalChangeResourceReferenceEntity] objects. + * - Deletes the original [LocalChangeEntity] record, which triggers a cascading delete in + * [LocalChangeResourceReferenceEntity]. + * - Creates a new [LocalChangeEntity] record along with new + * [LocalChangeResourceReferenceEntity] records. + * + * @param oldResource The original resource whose ID has been updated. + * @param updatedResource The updated resource with the new ID. + * @return A list of distinct resource UUIDs for all `LocalChangeEntity` records that referenced + * the old resource. */ private suspend fun updateReferencesInLocalChange( oldResource: Resource, - updatedResource: Resource, + updatedResourceId: String, ): List { val oldReferenceValue = "${oldResource.resourceType.name}/${oldResource.logicalId}" - val updatedReferenceValue = "${updatedResource.resourceType.name}/${updatedResource.logicalId}" - val referringLocalChangeIds = - getLocalChangeReferencesWithValue(oldReferenceValue).map { it.localChangeId }.distinct() - val referringLocalChanges = + val updatedReferenceValue = "${oldResource.resourceType.name}/$updatedResourceId" + + /** + * [getLocalChangeIdsWithReferenceValue] and [getLocalChanges] cannot be combined due to a + * limitation in Room. Fetching [LocalChangeEntity] in chunks is required to avoid the error + * documented in https://github.com/google/android-fhir/issues/2559. + */ + val referringLocalChangeIds = getLocalChangeIdsWithReferenceValue(oldReferenceValue) + val localChangeEntitiesWithOldReferences = referringLocalChangeIds.chunked(SQLITE_LIMIT_MAX_VARIABLE_NUMBER).flatMap { getLocalChanges(it) } - referringLocalChanges.forEach { existingLocalChangeEntity -> + localChangeEntitiesWithOldReferences.forEach { localChangeEntityWithOldReferences -> val updatedLocalChangeEntity = replaceReferencesInLocalChangePayload( - localChange = existingLocalChangeEntity, + localChange = localChangeEntityWithOldReferences, oldReference = oldReferenceValue, updatedReference = updatedReferenceValue, ) .copy(id = DEFAULT_ID_VALUE) val updatedLocalChangeReferences = - getReferencesForLocalChange(existingLocalChangeEntity.id).map { + getReferencesForLocalChange(localChangeEntityWithOldReferences.id).map { localChangeResourceReferenceEntity -> if (localChangeResourceReferenceEntity.resourceReferenceValue == oldReferenceValue) { LocalChangeResourceReferenceEntity( @@ -442,10 +464,10 @@ internal abstract class LocalChangeDao { ) } } - discardLocalChanges(existingLocalChangeEntity.id) + discardLocalChanges(localChangeEntityWithOldReferences.id) createLocalChange(updatedLocalChangeEntity, updatedLocalChangeReferences) } - return referringLocalChanges.map { it.resourceUuid }.distinct() + return localChangeEntitiesWithOldReferences.map { it.resourceUuid }.distinct() } private fun replaceReferencesInLocalChangePayload( From 7adc0ce88c43e84a7c439010f1b6339a9cded54f Mon Sep 17 00:00:00 2001 From: FikriMilano Date: Tue, 27 Aug 2024 16:33:10 +0700 Subject: [PATCH 07/13] Add missing use of viewItem.questionText (#2665) * Use questionnaireViewItem.questionText * spotless --- .../contrib/views/barcode/BarCodeReaderViewHolderFactory.kt | 3 +-- .../views/factories/DialogSelectViewHolderFactory.kt | 3 +-- .../datacapture/views/factories/ReviewViewHolderFactory.kt | 5 ++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/contrib/barcode/src/main/java/com/google/android/fhir/datacapture/contrib/views/barcode/BarCodeReaderViewHolderFactory.kt b/contrib/barcode/src/main/java/com/google/android/fhir/datacapture/contrib/views/barcode/BarCodeReaderViewHolderFactory.kt index a37b545489..96a678b080 100644 --- a/contrib/barcode/src/main/java/com/google/android/fhir/datacapture/contrib/views/barcode/BarCodeReaderViewHolderFactory.kt +++ b/contrib/barcode/src/main/java/com/google/android/fhir/datacapture/contrib/views/barcode/BarCodeReaderViewHolderFactory.kt @@ -22,7 +22,6 @@ import android.widget.TextView import androidx.lifecycle.lifecycleScope import com.google.android.fhir.datacapture.contrib.views.barcode.mlkit.md.LiveBarcodeScanningFragment import com.google.android.fhir.datacapture.extensions.localizedPrefixSpanned -import com.google.android.fhir.datacapture.extensions.localizedTextSpanned import com.google.android.fhir.datacapture.extensions.tryUnwrapContext import com.google.android.fhir.datacapture.views.QuestionnaireViewItem import com.google.android.fhir.datacapture.views.factories.QuestionnaireItemViewHolderDelegate @@ -95,7 +94,7 @@ object BarCodeReaderViewHolderFactory : } else { prefixTextView.visibility = View.GONE } - textQuestion.text = questionnaireViewItem.questionnaireItem.localizedTextSpanned + textQuestion.text = questionnaireViewItem.questionText setInitial(questionnaireViewItem.answers.singleOrNull(), reScanView) } diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt index 92bc81c359..85769b3679 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/DialogSelectViewHolderFactory.kt @@ -32,7 +32,6 @@ import com.google.android.fhir.datacapture.extensions.getRequiredOrOptionalText import com.google.android.fhir.datacapture.extensions.getValidationErrorMessage import com.google.android.fhir.datacapture.extensions.itemControl import com.google.android.fhir.datacapture.extensions.localizedFlyoverSpanned -import com.google.android.fhir.datacapture.extensions.localizedTextSpanned import com.google.android.fhir.datacapture.extensions.tryUnwrapContext import com.google.android.fhir.datacapture.validation.ValidationResult import com.google.android.fhir.datacapture.views.HeaderView @@ -94,7 +93,7 @@ internal object QuestionnaireItemDialogSelectViewHolderFactory : OptionSelectDialogFragment( // We use the question text for the dialog title. If there is no question text, we // use flyover text as it is sometimes used in text fields instead of question text. - title = questionnaireItem.localizedTextSpanned + title = questionnaireViewItem.questionText ?: questionnaireItem.localizedFlyoverSpanned ?: "", config = questionnaireItem.buildConfig(), selectedOptions = selectedOptions, diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt index 32a2de35c3..a6621b2aba 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt @@ -1,5 +1,5 @@ /* - * Copyright 2022-2023 Google LLC + * Copyright 2022-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,7 +26,6 @@ import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility import com.google.android.fhir.datacapture.extensions.getLocalizedInstructionsSpanned import com.google.android.fhir.datacapture.extensions.localizedFlyoverSpanned import com.google.android.fhir.datacapture.extensions.localizedPrefixSpanned -import com.google.android.fhir.datacapture.extensions.localizedTextSpanned import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility import com.google.android.fhir.datacapture.validation.Invalid import com.google.android.fhir.datacapture.views.QuestionnaireViewItem @@ -66,7 +65,7 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l questionnaireViewItem.questionnaireItem.localizedPrefixSpanned, ) question.updateTextAndVisibility( - questionnaireViewItem.questionnaireItem.localizedTextSpanned, + questionnaireViewItem.questionText, ) hint.updateTextAndVisibility( questionnaireViewItem.enabledDisplayItems.getLocalizedInstructionsSpanned(), From 7f4ecd22df746d579ec91f275eb1042ca42cc923 Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 28 Aug 2024 15:02:38 +0100 Subject: [PATCH 08/13] Load resources with url and optional version (#2666) * Load resources with url and optional version * Add back deprecated functions for build --- .../fhir/knowledge/KnowledgeManager.kt | 61 +++++++++++++++ .../fhir/knowledge/db/dao/KnowledgeDao.kt | 12 +++ .../fhir/knowledge/KnowledgeManagerTest.kt | 76 +++++++++++-------- 3 files changed, 117 insertions(+), 32 deletions(-) diff --git a/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt b/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt index 3a3a956c6f..8066cda7d9 100644 --- a/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt +++ b/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt @@ -167,6 +167,7 @@ internal constructor( } /** Loads resources from IGs listed in dependencies. */ + @Deprecated("Load resources using URLs only") suspend fun loadResources( resourceType: String, url: String? = null, @@ -175,6 +176,7 @@ internal constructor( version: String? = null, ): Iterable { val resType = ResourceType.fromCode(resourceType) + val resourceEntities = when { url != null && version != null -> @@ -189,6 +191,65 @@ internal constructor( return resourceEntities.map { readMetadataResourceOrThrow(it.resourceFile)!! } } + /** + * Loads knowledge artifact by its canonical URL and an optional version. + * + * The version can either be passed as a parameter or as part of the URL (using pipe `|` to + * separate the URL and the version). For example, passing the URL + * `http://abc.xyz/fhir/Library|1.0.0` with no version is the same as passing the URL + * `http://abc.xyz/fhir/Library` and version `1.0.0`. + * + * However, if a version is specified both as a parameter and as part of the URL, the two must + * match. + * + * @throws IllegalArgumentException if the url contains more than one pipe `|` + * @throws IllegalArgumentException if the version specified in the URL and the explicit version + * do not match + */ + suspend fun loadResources( + url: String, + version: String? = null, + ): Iterable { + val (canonicalUrl, canonicalVersion) = canonicalizeUrlAndVersion(url, version ?: "") + + val resourceEntities = + if (canonicalVersion == "") { + knowledgeDao.getResource(canonicalUrl) + } else { + listOfNotNull(knowledgeDao.getResource(canonicalUrl, canonicalVersion)) + } + return resourceEntities.map { readMetadataResourceOrThrow(it.resourceFile) } + } + + /** + * Canonicalizes the URL and version. It will extract the version as part of the URL separated by + * pipe `|`. + * + * For example, URL `http://abc.xyz/fhir/Library|1.0.0` will be canonicalized as URL + * `http://abc.xyz/fhir/Library` and version `1.0.0`. + * + * @throws IllegalArgumentException if the URL contains more than one pipe + * @throws IllegalArgumentException if the version specified in the URL and the explicit version + * do not match + */ + private fun canonicalizeUrlAndVersion( + url: String, + version: String, + ): Pair { + if (!url.contains('|')) { + return Pair(url, version) + } + + val parts = url.split('|') + require(parts.size == 2) { "URL $url contains too many parts separated by \"|\"" } + + // If an explicit version is specified, it must match the one in the URL + require(version == "" || version == parts[1]) { + "Version specified in the URL $parts[1] and explicit version $version do not match" + } + return Pair(parts[0], parts[1]) + } + /** Deletes Implementation Guide, cleans up files. */ suspend fun delete(vararg igDependencies: FhirNpmPackage) { igDependencies.forEach { igDependency -> diff --git a/knowledge/src/main/java/com/google/android/fhir/knowledge/db/dao/KnowledgeDao.kt b/knowledge/src/main/java/com/google/android/fhir/knowledge/db/dao/KnowledgeDao.kt index 204a39be50..c9d5cd3bd0 100644 --- a/knowledge/src/main/java/com/google/android/fhir/knowledge/db/dao/KnowledgeDao.kt +++ b/knowledge/src/main/java/com/google/android/fhir/knowledge/db/dao/KnowledgeDao.kt @@ -99,6 +99,7 @@ abstract class KnowledgeDao { url: String, ): ResourceMetadataEntity? + @Deprecated("Load resources using URLs") @Query( "SELECT * from ResourceMetadataEntity WHERE resourceType = :resourceType AND name = :name", ) @@ -107,6 +108,17 @@ abstract class KnowledgeDao { name: String?, ): List + @Query("SELECT * from ResourceMetadataEntity WHERE url = :url") + internal abstract suspend fun getResource( + url: String, + ): List + + @Query("SELECT * from ResourceMetadataEntity WHERE url = :url AND version = :version") + internal abstract suspend fun getResource( + url: String, + version: String, + ): ResourceMetadataEntity + @Query( "SELECT * from ResourceMetadataEntity WHERE resourceMetadataId = :id", ) diff --git a/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt b/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt index 48cdefd60f..c8a26d270e 100644 --- a/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt +++ b/knowledge/src/test/java/com/google/android/fhir/knowledge/KnowledgeManagerTest.kt @@ -99,20 +99,27 @@ internal class KnowledgeManagerTest { fun `imported entries are readable`() = runTest { knowledgeManager.import(fhirNpmPackage, dataFolder) - assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "WHOCommon")) - .isNotNull() - assertThat(knowledgeManager.loadResources(resourceType = "Library", url = "FHIRCommon")) - .isNotNull() - assertThat(knowledgeManager.loadResources(resourceType = "Measure")).hasSize(1) assertThat( knowledgeManager.loadResources( - resourceType = "Measure", + url = "http://fhir.org/guides/who/anc-cds/Library/WHOCommon", + version = "0.3.0", + ), + ) + .hasSize(1) + assertThat( + knowledgeManager.loadResources( + url = "http://fhir.org/guides/who/anc-cds/Library/FHIRCommon", + version = "0.3.0", + ), + ) + .hasSize(1) + assertThat( + knowledgeManager.loadResources( url = "http://fhir.org/guides/who/anc-cds/Measure/ANCIND01", + version = "0.3.0", ), ) - .isNotEmpty() - assertThat(knowledgeManager.loadResources(resourceType = "Measure", url = "Measure/ANCIND01")) - .isNotNull() + .hasSize(1) } @Test @@ -130,14 +137,14 @@ internal class KnowledgeManagerTest { Library().apply { id = "Library/defaultA-A.1.0.0" name = "defaultA" - url = "www.exampleA.com" + url = "www.exampleA.com/Library/defaultA-A.1.0.0" version = "A.1.0.0" } val libraryANew = Library().apply { id = "Library/defaultA-A.1.0.1" name = "defaultA" - url = "www.exampleA.com" + url = "www.exampleA.com/Library/defaultA-A.1.0.1" version = "A.1.0.1" } @@ -149,13 +156,13 @@ internal class KnowledgeManagerTest { val resourceA100 = knowledgeManager - .loadResources(resourceType = "Library", name = "defaultA", version = "A.1.0.0") + .loadResources(url = "www.exampleA.com/Library/defaultA-A.1.0.0", version = "A.1.0.0") .single() as Library assertThat(resourceA100.version).isEqualTo("A.1.0.0") val resourceA101 = knowledgeManager - .loadResources(resourceType = "Library", name = "defaultA", version = "A.1.0.1") + .loadResources(url = "www.exampleA.com/Library/defaultA-A.1.0.1", version = "A.1.0.1") .single() as Library assertThat(resourceA101.version.toString()).isEqualTo("A.1.0.1") } @@ -163,36 +170,42 @@ internal class KnowledgeManagerTest { fun `installing from npmPackageManager`() = runTest { knowledgeManager.install(fhirNpmPackage) - assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "WHOCommon")) - .isNotNull() - assertThat(knowledgeManager.loadResources(resourceType = "Library", url = "FHIRCommon")) - .isNotNull() - assertThat(knowledgeManager.loadResources(resourceType = "Measure")).hasSize(1) assertThat( knowledgeManager.loadResources( - resourceType = "Measure", + url = "http://fhir.org/guides/who/anc-cds/Library/WHOCommon", + version = "0.3.0", + ), + ) + .hasSize(1) + assertThat( + knowledgeManager.loadResources( + url = "http://fhir.org/guides/who/anc-cds/Library/FHIRCommon", + version = "0.3.0", + ), + ) + .hasSize(1) + assertThat( + knowledgeManager.loadResources( url = "http://fhir.org/guides/who/anc-cds/Measure/ANCIND01", + version = "0.3.0", ), ) - .isNotEmpty() - assertThat(knowledgeManager.loadResources(resourceType = "Measure", url = "Measure/ANCIND01")) - .isNotNull() + .hasSize(1) } @Test fun `for different resources with URL loading by URL should be correct`() = runTest { - val commonUrl = "www.sample-url.com" val libraryWithSameUrl = Library().apply { id = "Library/lId" name = "LibraryName" - url = commonUrl + url = "www.sample-url.com/Library/lId" } val planDefinitionWithSameUrl = PlanDefinition().apply { id = "PlanDefinition/pdId" name = "PlanDefinitionName" - url = commonUrl + url = "www.sample-url.com/PlanDefinition/pdId" } knowledgeManager.index(writeToFile(libraryWithSameUrl)) @@ -202,30 +215,29 @@ internal class KnowledgeManagerTest { assertThat(resources).hasSize(2) val libraryLoadedByUrl = - knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() as Library + knowledgeManager.loadResources(url = "www.sample-url.com/Library/lId").single() as Library assertThat(libraryLoadedByUrl.name.toString()).isEqualTo("LibraryName") val planDefinitionLoadedByUrl = - knowledgeManager.loadResources(resourceType = "PlanDefinition", url = commonUrl).single() + knowledgeManager.loadResources(url = "www.sample-url.com/PlanDefinition/pdId").single() as PlanDefinition assertThat(planDefinitionLoadedByUrl.name.toString()).isEqualTo("PlanDefinitionName") } @Test fun `for different resources with URL and Version loading by URL should be correct`() = runTest { - val commonUrl = "www.sample-url.com" val libraryWithSameUrl = Library().apply { id = "Library/lId" name = "LibraryName" - url = commonUrl + url = "www.sample-url.com/Library/lId" version = "0" } val planDefinitionWithSameUrl = PlanDefinition().apply { id = "PlanDefinition/pdId" name = "PlanDefinitionName" - url = commonUrl + url = "www.sample-url.com/PlanDefinition/pdId" version = "0" } @@ -236,11 +248,11 @@ internal class KnowledgeManagerTest { assertThat(resources).hasSize(2) val libraryLoadedByUrl = - knowledgeManager.loadResources(resourceType = "Library", url = commonUrl).single() as Library + knowledgeManager.loadResources(url = "www.sample-url.com/Library/lId").single() as Library assertThat(libraryLoadedByUrl.name.toString()).isEqualTo("LibraryName") val planDefinitionLoadedByUrl = - knowledgeManager.loadResources(resourceType = "PlanDefinition", url = commonUrl).single() + knowledgeManager.loadResources(url = "www.sample-url.com/PlanDefinition/pdId").single() as PlanDefinition assertThat(planDefinitionLoadedByUrl.name.toString()).isEqualTo("PlanDefinitionName") } From f4041daf1e225d96e59379772ac3c5dcd74bb77b Mon Sep 17 00:00:00 2001 From: FikriMilano Date: Tue, 3 Sep 2024 17:25:30 +0700 Subject: [PATCH 09/13] Fix keyboard hiding bug (#2652) * Fix keyboard hiding bug * Update comment --- .../fhir/datacapture/views/OptionSelectDialogFragment.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt index 15ad260a81..cc1565af51 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt @@ -107,12 +107,14 @@ internal class OptionSelectDialogFragment( WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM, ) // Adjust the dialog after the keyboard is on so that OK-CANCEL buttons are visible. - // SOFT_INPUT_ADJUST_RESIZE is deprecated and the suggested alternative - // setDecorFitsSystemWindows is available api level 30 and above. + // Ideally SOFT_INPUT_ADJUST_RESIZE supposed to be used, but in some devices the + // keyboard immediately hide itself after being opened, that's why SOFT_INPUT_ADJUST_PAN + // is used instead. There's no issue with setDecorFitsSystemWindows and is only + // available for api level 30 and above. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { it.setDecorFitsSystemWindows(false) } else { - it.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE) + it.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_PAN) } } } From cc0dc198e0f5dca5d008baaa51f1522d13757702 Mon Sep 17 00:00:00 2001 From: FikriMilano Date: Wed, 4 Sep 2024 12:28:51 +0700 Subject: [PATCH 10/13] Add condition to not populate string value if it's empty (#2521) * Add condition to not populate string value if it's empty * Revert "Add condition to not populate string value if it's empty" This reverts commit 6f652e1409b49c5de5fe840a6959824895c303df. * Filter only non-empty text should be saved * spotless * Address review --- .../fhir/datacapture/views/OptionSelectDialogFragment.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt index cc1565af51..4fb0238ca0 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt @@ -138,7 +138,13 @@ internal class OptionSelectDialogFragment( SelectedOptions( options = currentList.filterIsInstance().map { it.option }, otherOptions = - currentList.filterIsInstance().map { it.currentText }, + currentList + .filterIsInstance() + .filter { + it.currentText.isNotEmpty() + } // Filters out empty answers when the user inputs nothing into a new option choice + // edit text field. + .map { it.currentText }, ), ) } From 65ffde8fb21add8bbb4c1594987f83729386ab5d Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 4 Sep 2024 12:51:23 +0100 Subject: [PATCH 11/13] Bump up knowledge manager version to beta01 (#2667) * Bump up knowledge manager version to beta01 * Update api.md --- buildSrc/src/main/kotlin/Releases.kt | 2 +- docs/use/api.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/kotlin/Releases.kt b/buildSrc/src/main/kotlin/Releases.kt index 89e5b6c34d..7b1e92f953 100644 --- a/buildSrc/src/main/kotlin/Releases.kt +++ b/buildSrc/src/main/kotlin/Releases.kt @@ -81,7 +81,7 @@ object Releases { object Knowledge : LibraryArtifact { override val artifactId = "knowledge" - override val version = "0.1.0-alpha03" + override val version = "0.1.0-beta01" override val name = "Android FHIR Knowledge Manager Library" } diff --git a/docs/use/api.md b/docs/use/api.md index d38c35816e..bf7e90c8f4 100644 --- a/docs/use/api.md +++ b/docs/use/api.md @@ -3,4 +3,4 @@ * [Engine](api/engine/1.0.0/index.html) * [Data Capture](api/data-capture/1.1.0/index.html) * [Workflow](api/workflow/0.1.0-alpha04/index.html) -* [Knowledge](api/knowledge/0.1.0-alpha03/index.html) +* [Knowledge](api/knowledge/0.1.0-beta01/index.html) From d9653b61d03eb3b1d4a570c451269ab7f3fd03c7 Mon Sep 17 00:00:00 2001 From: santosh-pingle <86107848+santosh-pingle@users.noreply.github.com> Date: Wed, 4 Sep 2024 18:53:44 +0530 Subject: [PATCH 12/13] Per question custom style (#2636) * per question custom style * adding missing file. * update default style. * textAppearance support * rename custom attributes. * github documentation for custom style example. * Address review comments. * address review comment. * update text format icon as component icon. * code cleanup. * Code refactoring and cleanup. * custom style example with multiple question items. * Address review comments. * support prefix per question item custom style * Revert dataconfig changes for custom style mapping. * Address review comments. * Address review comments. * Address review comments. * Address review comments. * Address review comment. --------- Co-authored-by: Santosh Pingle --- .../component_per_question_custom_style.json | 149 ++++++++++ .../fhir/catalog/ComponentListViewModel.kt | 6 + .../src/main/res/drawable/ic_location_on.xml | 4 +- .../main/res/drawable/text_format_48dp.xml | 15 + catalog/src/main/res/values-night/colors.xml | 36 +++ catalog/src/main/res/values/colors.xml | 22 ++ catalog/src/main/res/values/strings.xml | 3 + catalog/src/main/res/values/styles.xml | 171 ++++++++++- .../datacapture/extensions/MoreHeaderViews.kt | 38 +++ .../extensions/MoreQuestionItemStyle.kt | 267 ++++++++++++++++++ .../MoreQuestionnaireItemComponents.kt | 21 ++ .../fhir/datacapture/views/GroupHeaderView.kt | 7 + .../fhir/datacapture/views/HeaderView.kt | 7 + datacapture/src/main/res/values/attrs.xml | 20 ++ ...tomize-how-a-Questionnaire-is-displayed.md | 134 +++++++++ 15 files changed, 897 insertions(+), 3 deletions(-) create mode 100644 catalog/src/main/assets/component_per_question_custom_style.json create mode 100644 catalog/src/main/res/drawable/text_format_48dp.xml create mode 100644 catalog/src/main/res/values-night/colors.xml create mode 100644 datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionItemStyle.kt diff --git a/catalog/src/main/assets/component_per_question_custom_style.json b/catalog/src/main/assets/component_per_question_custom_style.json new file mode 100644 index 0000000000..2f2e8f3b86 --- /dev/null +++ b/catalog/src/main/assets/component_per_question_custom_style.json @@ -0,0 +1,149 @@ +{ + "resourceType": "Questionnaire", + "item": [ + { + "linkId": "1", + "text": "Custom style 1", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_1" + } + ] + } + ] + }, + { + "linkId": "2", + "text": "Custom style 2", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_2" + } + ] + } + ] + }, + { + "linkId": "3", + "text": "Custom style 3", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_3" + } + ] + } + ] + }, + { + "linkId": "4", + "text": "Custom style 4", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_4" + } + ] + } + ] + }, + { + "linkId": "5", + "text": "Custom style 5", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_5" + } + ] + } + ] + }, + { + "linkId": "6", + "text": "Custom style 6", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_6" + } + ] + } + ] + }, + { + "linkId": "7", + "text": "Custom style 7", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_7" + } + ] + } + ] + }, + { + "linkId": "8", + "text": "Custom style 8", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_8" + } + ] + } + ] + }, + { + "linkId": "9", + "text": "Custom style 9", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "question_text_view", + "valueString": "CustomStyle_9" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/catalog/src/main/java/com/google/android/fhir/catalog/ComponentListViewModel.kt b/catalog/src/main/java/com/google/android/fhir/catalog/ComponentListViewModel.kt index bdafbcd17a..d9e4637ada 100644 --- a/catalog/src/main/java/com/google/android/fhir/catalog/ComponentListViewModel.kt +++ b/catalog/src/main/java/com/google/android/fhir/catalog/ComponentListViewModel.kt @@ -152,6 +152,11 @@ class ComponentListViewModel(application: Application, private val state: SavedS R.string.component_name_location_widget, "component_location_widget.json", ), + QUESTION_ITEM_CUSTOM_STYLE( + R.drawable.text_format_48dp, + R.string.component_name_per_question_custom_style, + "component_per_question_custom_style.json", + ), } val viewItemList = @@ -177,6 +182,7 @@ class ComponentListViewModel(application: Application, private val state: SavedS ViewItem.ComponentItem(Component.ITEM_ANSWER_MEDIA), ViewItem.ComponentItem(Component.INITIAL_VALUE), ViewItem.ComponentItem(Component.LOCATION_WIDGET), + ViewItem.ComponentItem(Component.QUESTION_ITEM_CUSTOM_STYLE), ) fun isComponent(context: Context, title: String) = diff --git a/catalog/src/main/res/drawable/ic_location_on.xml b/catalog/src/main/res/drawable/ic_location_on.xml index 0f96a89039..9821fffba8 100644 --- a/catalog/src/main/res/drawable/ic_location_on.xml +++ b/catalog/src/main/res/drawable/ic_location_on.xml @@ -1,7 +1,7 @@ + + + + diff --git a/catalog/src/main/res/values-night/colors.xml b/catalog/src/main/res/values-night/colors.xml new file mode 100644 index 0000000000..d80470737a --- /dev/null +++ b/catalog/src/main/res/values-night/colors.xml @@ -0,0 +1,36 @@ + + + + #000000 + #0C0A20 + #201441 + #341F63 + #482A85 + #5C35A6 + #7F5FBA + #A289CF + #C5B3E3 + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #000000 + #000000 + diff --git a/catalog/src/main/res/values/colors.xml b/catalog/src/main/res/values/colors.xml index 8a1561f3da..f9b63e732a 100644 --- a/catalog/src/main/res/values/colors.xml +++ b/catalog/src/main/res/values/colors.xml @@ -102,4 +102,26 @@ #C4C7C5 #8E918F + + + #7A9FFF + #668FFF + #5581FF + #476FFF + #3B5CFF + #3249FF + #2936FF + #2024FF + #1816FF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + #FFFFFF + diff --git a/catalog/src/main/res/values/strings.xml b/catalog/src/main/res/values/strings.xml index 7e09047feb..ae47067784 100644 --- a/catalog/src/main/res/values/strings.xml +++ b/catalog/src/main/res/values/strings.xml @@ -37,6 +37,9 @@ Repeated Group Attachment Location Widget + Per question custom style Default Paginated Review diff --git a/catalog/src/main/res/values/styles.xml b/catalog/src/main/res/values/styles.xml index f88c5ea9fa..f2afa052f3 100644 --- a/catalog/src/main/res/values/styles.xml +++ b/catalog/src/main/res/values/styles.xml @@ -80,7 +80,10 @@ @@ -98,4 +101,170 @@ 2 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt index 022964c9db..fd4690ea26 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt @@ -104,3 +104,41 @@ fun appendAsteriskToQuestionText( } } } + +internal fun applyCustomOrDefaultStyle( + questionnaireItem: Questionnaire.QuestionnaireItemComponent, + prefixTextView: TextView, + questionTextView: TextView, + instructionTextView: TextView, +) { + applyCustomOrDefaultStyle( + context = prefixTextView.context, + view = prefixTextView, + customStyleName = + questionnaireItem.readCustomStyleExtension( + StyleUrl.PREFIX_TEXT_VIEW, + ), + defaultStyleResId = + getStyleResIdFromAttribute(questionTextView.context, R.attr.questionnaireQuestionTextStyle), + ) + applyCustomOrDefaultStyle( + context = questionTextView.context, + view = questionTextView, + customStyleName = + questionnaireItem.readCustomStyleExtension( + StyleUrl.QUESTION_TEXT_VIEW, + ), + defaultStyleResId = + getStyleResIdFromAttribute(questionTextView.context, R.attr.questionnaireQuestionTextStyle), + ) + applyCustomOrDefaultStyle( + context = instructionTextView.context, + view = instructionTextView, + customStyleName = + questionnaireItem.readCustomStyleExtension( + StyleUrl.SUBTITLE_TEXT_VIEW, + ), + defaultStyleResId = + getStyleResIdFromAttribute(questionTextView.context, R.attr.questionnaireSubtitleTextStyle), + ) +} diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionItemStyle.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionItemStyle.kt new file mode 100644 index 0000000000..3067d969bd --- /dev/null +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionItemStyle.kt @@ -0,0 +1,267 @@ +/* + * Copyright 2024 Google LLC + * + * 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 com.google.android.fhir.datacapture.extensions + +import android.content.Context +import android.content.res.TypedArray +import android.view.View +import android.widget.TextView +import androidx.core.content.ContextCompat +import com.google.android.fhir.datacapture.R + +/** + * Applies either a custom style or a default style to the given view based on the provided custom + * style name and default style resource ID. + * + * If the custom style resource name is valid, it applies the custom style to the view. If the + * custom style resource name is not valid or not found, it falls back to applying the default style + * defined by the given style resource ID. It sets the view's tag to resourceId to indicate that the + * custom style has been applied. + * + * @param context the context used to access resources. + * @param view the view to which the style should be applied. + * @param customStyleName the name of the custom style to apply. + * @param defaultStyleResId the default style resource ID to use if no custom style is found. + */ +internal fun applyCustomOrDefaultStyle( + context: Context, + view: View, + customStyleName: String?, + defaultStyleResId: Int, +) { + val customStyleResId = customStyleName?.let { getStyleResIdByName(context, it) } ?: 0 + when { + customStyleResId != 0 -> { + view.tag = customStyleResId + QuestionItemCustomStyle().applyStyle(context, view, customStyleResId) + } + defaultStyleResId != 0 -> { + applyDefaultStyleIfNotApplied(context, view, defaultStyleResId) + } + } +} + +/** + * Applies the default style to the given view if the default style has not already been applied. + * + * This function checks the `view`'s tag to determine if a style has been previously applied. If the + * tag is an integer, it will apply the default style specified by `defaultStyleResId`. After + * applying the style, it resets the view's tag to `null` to indicate that the default style has + * been applied. + * + * @param context The context used to access resources and themes. + * @param view The view to which the default style will be applied. + * @param defaultStyleResId The resource ID of the default style to apply. + */ +private fun applyDefaultStyleIfNotApplied( + context: Context, + view: View, + defaultStyleResId: Int, +) { + (view.tag as? Int)?.let { + QuestionItemDefaultStyle().applyStyle(context, view, defaultStyleResId) + view.tag = null + } +} + +/** + * Retrieves the resource ID of a style given its name. + * + * This function uses the `getIdentifier` method to look up the style resource ID based on the style + * name provided. If the style name is not found, it returns 0. + * + * @param context The context used to access resources. + * @param styleName The name of the style whose resource ID is to be retrieved. + * @return The resource ID of the style, or 0 if the style name is not found. + */ +private fun getStyleResIdByName(context: Context, styleName: String): Int { + return context.resources.getIdentifier(styleName, "style", context.packageName) +} + +/** + * Retrieves the style resource ID associated with a specific attribute from the current theme. + * + * This function obtains the style resource ID that is linked to a given attribute in the current + * theme. It uses the `obtainStyledAttributes` method to fetch the attributes and extract the + * resource ID. + * + * @param context The context to access the current theme and resources. + * @param attr The attribute whose associated style resource ID is to be retrieved. + * @return The resource ID of the style associated with the specified attribute, or 0 if not found. + */ +internal fun getStyleResIdFromAttribute(context: Context, attr: Int): Int { + val typedArray = context.theme.obtainStyledAttributes(intArrayOf(attr)) + val styleResId = typedArray.getResourceId(0, 0) + typedArray.recycle() + return styleResId +} + +internal abstract class QuestionItemStyle { + + /** + * Applies a style to a view. + * + * @param context The context used to apply the style. + * @param view The view to which the style will be applied. + * @param styleResId The resource ID of the style to apply. + */ + abstract fun applyStyle(context: Context, view: View, styleResId: Int) + + /** + * Applies the style from a TypedArray to a view. + * + * @param context The context used to apply the style. + * @param view The view to which the style will be applied. + * @param typedArray The TypedArray containing the style attributes. + */ + internal fun applyStyle(context: Context, view: View, typedArray: TypedArray) { + applyGenericViewStyle(context, view, typedArray) + if (view is TextView) { + applyTextViewSpecificStyle(view, typedArray) + } + typedArray.recycle() + } + + /** + * Abstract function to apply generic view styles from a TypedArray. + * + * @param context The context used to apply the style. + * @param view The view to which the style will be applied. + * @param typedArray The TypedArray containing the style attributes. + */ + abstract fun applyGenericViewStyle(context: Context, view: View, typedArray: TypedArray) + + /** + * Abstract function to apply TextView-specific styles from a TypedArray. + * + * @param textView The TextView to which the style will be applied. + * @param typedArray The TypedArray containing the style attributes. + */ + abstract fun applyTextViewSpecificStyle(textView: TextView, typedArray: TypedArray) + + /** + * Applies the background color from a TypedArray to a view. + * + * @param context The context used to apply the background color. + * @param view The view to which the background color will be applied. + * @param typedArray The TypedArray containing the background color attribute. + * @param index The index of the background color attribute in the TypedArray. + */ + protected fun applyBackgroundColor( + context: Context, + view: View, + typedArray: TypedArray, + index: Int, + ) { + val backgroundColor = + typedArray.getColor(index, ContextCompat.getColor(context, android.R.color.transparent)) + view.setBackgroundColor(backgroundColor) + } + + /** + * Applies the text appearance from a TypedArray to a TextView. + * + * @param textView The TextView to which the text appearance will be applied. + * @param typedArray The TypedArray containing the text appearance attribute. + * @param index The index of the text appearance attribute in the TypedArray. + */ + protected fun applyTextAppearance(textView: TextView, typedArray: TypedArray, index: Int) { + val textAppearance = typedArray.getResourceId(index, -1) + if (textAppearance != -1) { + textView.setTextAppearance(textAppearance) + } + } +} + +internal class QuestionItemCustomStyle : QuestionItemStyle() { + private enum class CustomStyleViewAttributes(val attrId: Int) { + TEXT_APPEARANCE(R.styleable.QuestionnaireCustomStyle_questionnaire_textAppearance), + BACKGROUND(R.styleable.QuestionnaireCustomStyle_questionnaire_background), + } + + override fun applyStyle(context: Context, view: View, styleResId: Int) { + val typedArray = + context.obtainStyledAttributes(styleResId, R.styleable.QuestionnaireCustomStyle) + applyStyle(context, view, typedArray) + } + + override fun applyGenericViewStyle(context: Context, view: View, typedArray: TypedArray) { + for (i in 0 until typedArray.indexCount) { + when (typedArray.getIndex(i)) { + CustomStyleViewAttributes.BACKGROUND.attrId -> { + applyBackgroundColor(context, view, typedArray, i) + } + } + } + } + + override fun applyTextViewSpecificStyle( + textView: TextView, + typedArray: TypedArray, + ) { + for (i in 0 until typedArray.indexCount) { + when (typedArray.getIndex(i)) { + CustomStyleViewAttributes.TEXT_APPEARANCE.attrId -> { + applyTextAppearance(textView, typedArray, i) + } + } + } + } +} + +internal class QuestionItemDefaultStyle : QuestionItemStyle() { + private enum class DefaultStyleViewAttributes(val attrId: Int) { + TEXT_APPEARANCE(android.R.attr.textAppearance), + BACKGROUND(android.R.attr.background), + // Add other attributes you want to apply + } + + override fun applyStyle(context: Context, view: View, styleResId: Int) { + val attrs = DefaultStyleViewAttributes.values().map { it.attrId }.toIntArray() + val typedArray: TypedArray = context.obtainStyledAttributes(styleResId, attrs) + applyStyle(context, view, typedArray) + } + + override fun applyGenericViewStyle(context: Context, view: View, typedArray: TypedArray) { + for (i in 0 until typedArray.indexCount) { + when (DefaultStyleViewAttributes.values()[i]) { + DefaultStyleViewAttributes.BACKGROUND -> { + applyBackgroundColor(context, view, typedArray, i) + } + else -> { + // Ignore view specific attributes. + } + } + } + } + + override fun applyTextViewSpecificStyle( + textView: TextView, + typedArray: TypedArray, + ) { + for (i in 0 until typedArray.indexCount) { + when (DefaultStyleViewAttributes.values()[i]) { + DefaultStyleViewAttributes.TEXT_APPEARANCE -> { + applyTextAppearance(textView, typedArray, i) + } + else -> { + // applyGenericViewDefaultStyle for other attributes. + } + } + } + } +} diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt index 55dd4e5738..0acd26c93f 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt @@ -61,6 +61,13 @@ internal const val EXTENSION_ITEM_CONTROL_URL_ANDROID_FHIR = internal const val EXTENSION_ITEM_CONTROL_SYSTEM_ANDROID_FHIR = "https://github.com/google/android-fhir/questionnaire-item-control" +internal enum class StyleUrl(val url: String) { + BASE("https://github.com/google/android-fhir/tree/master/datacapture/android-style"), + PREFIX_TEXT_VIEW("prefix_text_view"), + QUESTION_TEXT_VIEW("question_text_view"), + SUBTITLE_TEXT_VIEW("subtitle_text_view"), +} + // Below URLs exist and are supported by HL7 internal const val EXTENSION_ANSWER_EXPRESSION_URL: String = @@ -1017,3 +1024,17 @@ val Resource.logicalId: String get() { return this.idElement?.idPart.orEmpty() } + +internal fun QuestionnaireItemComponent.readCustomStyleExtension(styleUrl: StyleUrl): String? { + // Find the base extension + val baseExtension = extension.find { it.url == StyleUrl.BASE.url } + baseExtension?.let { ext -> + // Extract nested extension based on the given StyleUrl + ext.extension.forEach { nestedExt -> + if (nestedExt.url == styleUrl.url) { + return nestedExt.value.asStringValue() + } + } + } + return null +} diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt index 4fff12a122..da85cbac4e 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt @@ -23,6 +23,7 @@ import android.widget.LinearLayout import android.widget.TextView import com.google.android.fhir.datacapture.QuestionnaireViewHolderType import com.google.android.fhir.datacapture.R +import com.google.android.fhir.datacapture.extensions.applyCustomOrDefaultStyle import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility import com.google.android.fhir.datacapture.extensions.getLocalizedInstructionsSpanned import com.google.android.fhir.datacapture.extensions.initHelpViews @@ -60,5 +61,11 @@ class GroupHeaderView(context: Context, attrs: AttributeSet?) : LinearLayout(con questionnaireViewItem.enabledDisplayItems.getLocalizedInstructionsSpanned(), ) visibility = getHeaderViewVisibility(prefix, question, hint) + applyCustomOrDefaultStyle( + questionnaireViewItem.questionnaireItem, + prefixTextView = prefix, + questionTextView = question, + instructionTextView = hint, + ) } } diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt index c48e546f38..7e5e77231d 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt @@ -23,6 +23,7 @@ import android.widget.LinearLayout import android.widget.TextView import com.google.android.fhir.datacapture.R import com.google.android.fhir.datacapture.extensions.appendAsteriskToQuestionText +import com.google.android.fhir.datacapture.extensions.applyCustomOrDefaultStyle import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility import com.google.android.fhir.datacapture.extensions.getLocalizedInstructionsSpanned import com.google.android.fhir.datacapture.extensions.initHelpViews @@ -64,6 +65,12 @@ class HeaderView(context: Context, attrs: AttributeSet?) : LinearLayout(context, // Make the entire view GONE if there is nothing to show. This is to avoid an empty row in the // questionnaire. visibility = getHeaderViewVisibility(prefix, question, hint) + applyCustomOrDefaultStyle( + questionnaireViewItem.questionnaireItem, + prefixTextView = prefix, + questionTextView = question, + instructionTextView = hint, + ) } /** diff --git a/datacapture/src/main/res/values/attrs.xml b/datacapture/src/main/res/values/attrs.xml index 46d076ed59..42cc7e5fb1 100644 --- a/datacapture/src/main/res/values/attrs.xml +++ b/datacapture/src/main/res/values/attrs.xml @@ -188,4 +188,24 @@ extend Theme.Questionnaire. If unspecified, Theme.Questionnaire will be used. --> + + + + + + + + + diff --git a/docs/use/SDCL/Customize-how-a-Questionnaire-is-displayed.md b/docs/use/SDCL/Customize-how-a-Questionnaire-is-displayed.md index 3ed391bded..4ac8947ef7 100644 --- a/docs/use/SDCL/Customize-how-a-Questionnaire-is-displayed.md +++ b/docs/use/SDCL/Customize-how-a-Questionnaire-is-displayed.md @@ -67,6 +67,140 @@ the new theme you just created: ``` +## Custom Style per Question Item + +With this change, you can apply individual custom styles per question item. If a custom style is not mentioned in the question item, the default style will be applied, which is present in the DataCapture module or overridden in the application. + +### Add a Custom Style Extension to the Question Item + +```json +{ + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "prefix_text_view", + "valueString": "CustomStyle_1" + }, + { + "url": "question_text_view", + "valueString": "CustomStyle_1" + }, + { + "url": "subtitle_text_view", + "valueString": "CustomStyle_2" + } + ] + } + ] +} +``` +### Custom Style Extension URL +"https://github.com/google/android-fhir/tree/master/datacapture/android-style" + +It identifies extensions for applying the custom style to a given questionnaire item. + +### Question Item View +* `prefix_text_view`: Used to show the prefix value of the question item. +* `question_text_view`: Used to show the text value of the question item. +* `subtitle_text_view`: Used to show the instructions of the question item. + For more information about supported views, please see the [Question Item View](https://github.com/google/android-fhir/blob/master/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt). + +### Custom Style Values +In the above example: + +`CustomStyle_1` is the custom style for prefix_text_view and question_text_view. +`CustomStyle_2` is the custom style for subtitle_text_view. +Both styles are defined in the application. + +### Custom Style Attributes +* `questionnaire_textAppearance`: Specifies the text appearance for the questionnaire text. Example: `@style/TextAppearance.AppCompat.Headline` +* `questionnaire_background`: Specifies the background for the view. Example: `@color/background_color or #FFFFFF` + +For more information on custom style attributes, please see the [QuestionnaireCustomStyle](https://github.com/google/android-fhir/blob/master/datacapture/src/main/res/values/attrs.xml) + +### Example Custom Styles + +``` + + + + + + + + +``` + +The above custom styles are defined in the `res/values/styles.xml` of the application. + +### questionnaire.json with custom style +``` +{ + "resourceType": "Questionnaire", + "item": [ + { + "linkId": "1", + "text": "Question text custom style", + "type": "display", + "extension": [ + { + "url": "https://github.com/google/android-fhir/tree/master/datacapture/android-style", + "extension": [ + { + "url": "prefix_text_view", + "valueString": "CustomStyle_1" + }, + { + "url": "question_text_view", + "valueString": "CustomStyle_1" + }, + { + "url": "subtitle_text_view", + "valueString": "CustomStyle_2" + } + ] + } + ], + "item": [ + { + "extension": [ + { + "url": "http://hl7.org/fhir/StructureDefinition/questionnaire-displayCategory", + "valueCodeableConcept": { + "coding": [ + { + "system": "http://hl7.org/fhir/questionnaire-display-category", + "code": "instructions" + } + ] + } + } + ], + "linkId": "1.3", + "text": "Instructions custom style.", + "type": "display" + } + ] + } ] +} +``` + ## Custom questionnaire components The Structured Data Capture Library uses From 8301d62b1a2752d4e16e875678d07c3190c80b4c Mon Sep 17 00:00:00 2001 From: santosh-pingle <86107848+santosh-pingle@users.noreply.github.com> Date: Mon, 9 Sep 2024 12:07:27 +0530 Subject: [PATCH 13/13] Resource consolidation after using the AllChangesSquashedBundlePost upload strategy for resource creation. (#2509) * draft singleresourcepost * Remove dead code. * resource consolidation after post http verb request * Remove local changes. * fix unit tests. * unit tests * Update kotlin api docs. * revert local changes. * Resource consolidation as per http verb * address review comments. * order of arguments * code to string conversion * AllChangesBundlePost upload strategy. * remove localchange reference updates code. * unit tests * Address review comments. * Fix unit test. * rename tests. * Code cleaning. * code cleaning. * Address review comments. * Address review comments. * Address review comments. * spotless apply. * build failure. * cleanup. * Address review comments. * Address review comments. * Address review comments. --------- Co-authored-by: Santosh Pingle --- .../android/fhir/db/impl/DatabaseImplTest.kt | 138 +++++++ .../HttpPostResourceConsolidatorTest.kt | 359 ++++++++++++------ .../com/google/android/fhir/FhirEngine.kt | 2 +- .../com/google/android/fhir/MoreResources.kt | 21 +- .../com/google/android/fhir/db/Database.kt | 16 +- .../android/fhir/db/impl/DatabaseImpl.kt | 32 +- .../google/android/fhir/db/impl/JsonUtils.kt | 2 +- .../fhir/db/impl/dao/LocalChangeDao.kt | 2 +- .../android/fhir/db/impl/dao/ResourceDao.kt | 30 +- .../fhir/sync/upload/ResourceConsolidator.kt | 99 ++--- .../fhir/sync/upload/UploadStrategy.kt | 2 +- ...eEntryComponentGeneratorImplementations.kt | 9 +- .../request/TransactionBundleGenerator.kt | 5 + .../google/android/fhir/MoreResourcesTest.kt | 39 +- .../request/TransactionBundleGeneratorTest.kt | 33 +- 15 files changed, 579 insertions(+), 210 deletions(-) diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt index ee0aede8c9..123c474cc5 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt @@ -19,6 +19,8 @@ package com.google.android.fhir.db.impl import android.content.Context import androidx.test.core.app.ApplicationProvider import androidx.test.filters.MediumTest +import ca.uhn.fhir.context.FhirContext +import ca.uhn.fhir.context.FhirVersionEnum import ca.uhn.fhir.rest.gclient.StringClientParam import ca.uhn.fhir.rest.param.ParamPrefixEnum import com.google.android.fhir.DateProvider @@ -4209,6 +4211,142 @@ class DatabaseImplTest { assertThat(searchedObservations[0].logicalId).isEqualTo(locallyCreatedObservationResourceId) } + @Test + fun updateResourcePostSync_shouldUpdateResourceId() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val postSyncResourceId = "patient2" + val newVersionId = "1" + val lastUpdatedRemote = Instant.now() + + database.updateResourcePostSync( + preSyncPatient.logicalId, + postSyncResourceId, + preSyncPatient.resourceType, + newVersionId, + lastUpdatedRemote, + ) + + val patientResourceEntityPostSync = + database.selectEntity(preSyncPatient.resourceType, postSyncResourceId) + assertThat(patientResourceEntityPostSync.resourceId).isEqualTo(postSyncResourceId) + } + + @Test + fun updateResourcePostSync_shouldUpdateResourceMeta() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val postSyncResourceId = "patient2" + val newVersionId = "1" + val lastUpdatedRemote = Instant.now() + + database.updateResourcePostSync( + preSyncPatient.logicalId, + postSyncResourceId, + preSyncPatient.resourceType, + newVersionId, + lastUpdatedRemote, + ) + + val patientResourceEntityPostSync = + database.selectEntity(preSyncPatient.resourceType, postSyncResourceId) + assertThat(patientResourceEntityPostSync.versionId).isEqualTo(newVersionId) + assertThat(patientResourceEntityPostSync.lastUpdatedRemote?.toEpochMilli()) + .isEqualTo(lastUpdatedRemote.toEpochMilli()) + } + + @Test + fun updateResourcePostSync_shouldDeleteOldResourceId() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val postSyncResourceId = "patient2" + + database.updateResourcePostSync( + preSyncPatient.logicalId, + postSyncResourceId, + preSyncPatient.resourceType, + null, + null, + ) + + val exception = + assertThrows(ResourceNotFoundException::class.java) { + runBlocking { database.select(ResourceType.Patient, "patient1") } + } + assertThat(exception.message).isEqualTo("Resource not found with type Patient and id patient1!") + } + + @Test + fun updateResourcePostSync_shouldUpdateReferringResourceReferenceValue() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + val observation = + Observation().apply { + id = "observation1" + subject = Reference().apply { reference = "Patient/patient1" } + } + database.insert(preSyncPatient, observation) + val postSyncResourceId = "patient2" + val newVersionId = "1" + val lastUpdatedRemote = Instant.now() + + database.updateResourcePostSync( + preSyncPatient.logicalId, + postSyncResourceId, + preSyncPatient.resourceType, + newVersionId, + lastUpdatedRemote, + ) + + assertThat( + (database.select(ResourceType.Observation, "observation1") as Observation) + .subject + .reference, + ) + .isEqualTo("Patient/patient2") + } + + @Test + fun updateResourcePostSync_shouldUpdateReferringResourceReferenceValueInLocalChange() = + runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + val observation = + Observation().apply { + id = "observation1" + subject = Reference().apply { reference = "Patient/patient1" } + } + database.insert(preSyncPatient, observation) + val postSyncResourceId = "patient2" + val newVersionId = "1" + val lastUpdatedRemote = Instant.now() + + database.updateResourcePostSync( + preSyncPatient.logicalId, + postSyncResourceId, + preSyncPatient.resourceType, + newVersionId, + lastUpdatedRemote, + ) + + assertThat( + (database.select(ResourceType.Observation, "observation1") as Observation) + .subject + .reference, + ) + .isEqualTo("Patient/patient2") + val observationLocalChanges = + database.getLocalChanges( + observation.resourceType, + observation.logicalId, + ) + val observationReferenceValue = + (FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .parseResource(observationLocalChanges.first().payload) as Observation) + .subject + .reference + assertThat(observationReferenceValue).isEqualTo("Patient/$postSyncResourceId") + } + @Test // https://github.com/google/android-fhir/issues/2512 fun included_results_sort_ascending_should_have_distinct_resources() = runBlocking { /** diff --git a/engine/src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt b/engine/src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt index 67b1bb258c..0fa872aa93 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt @@ -20,15 +20,20 @@ import android.content.Context import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import ca.uhn.fhir.context.FhirContext +import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.FhirServices import com.google.android.fhir.db.Database import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.logicalId import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.runBlocking -import org.hl7.fhir.r4.model.DomainResource +import org.hl7.fhir.r4.model.Bundle +import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent import org.hl7.fhir.r4.model.InstantType +import org.hl7.fhir.r4.model.Meta import org.hl7.fhir.r4.model.Observation +import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.ResourceType import org.junit.After import org.junit.Assert.assertThrows @@ -65,35 +70,21 @@ class HttpPostResourceConsolidatorTest { } @Test - fun consolidate_shouldUpdateResourceId() = runBlocking { - val patientJsonString = - """ - { - "resourceType": "Patient", - "id": "patient1" - } - """ - .trimIndent() - val patient = - FhirContext.forR4Cached().newJsonParser().parseResource(patientJsonString) as DomainResource - database.insert(patient) - val localChanges = database.getLocalChanges(patient.resourceType, patient.logicalId) - - val postSyncPatientJsonString = - """ - { - "resourceType": "Patient", - "id": "patient2", - "meta": { - "versionId": "1" - } - } - """ - .trimIndent() + fun resourceConsolidator_singleResourceUpload_shouldUpdateNewResourceId() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val localChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) val postSyncPatient = - FhirContext.forR4Cached().newJsonParser().parseResource(postSyncPatientJsonString) - as DomainResource - postSyncPatient.meta.lastUpdatedElement = InstantType.now() + Patient().apply { + id = "patient2" + meta = + Meta().apply { + versionId = "1" + lastUpdatedElement = InstantType.now() + } + } + val uploadRequestResult = UploadRequestResult.Success( listOf(ResourceUploadResponseMapping(localChanges, postSyncPatient)), @@ -102,130 +93,264 @@ class HttpPostResourceConsolidatorTest { assertThat(database.select(ResourceType.Patient, "patient2").logicalId) .isEqualTo(postSyncPatient.logicalId) - val exception = assertThrows(ResourceNotFoundException::class.java) { runBlocking { database.select(ResourceType.Patient, "patient1") } } - assertThat(exception.message).isEqualTo("Resource not found with type Patient and id patient1!") } @Test - fun consolidate_dependentResources_shouldUpdateReferenceValue() = runBlocking { - val patientJsonString = - """ - { - "resourceType": "Patient", - "id": "patient1" + fun resourceConsolidator_singleResourceUpload_shouldUpdateReferenceValueOfReferencingResources() = + runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + val observation = + Observation().apply { + id = "observation1" + subject = Reference().apply { reference = "Patient/patient1" } } - """ - .trimIndent() - val patient = - FhirContext.forR4Cached().newJsonParser().parseResource(patientJsonString) as DomainResource - val observationJsonString = + database.insert(preSyncPatient, observation) + val postSyncPatient = + Patient().apply { + id = "patient2" + meta = + Meta().apply { + versionId = "1" + lastUpdatedElement = InstantType.now() + } + } + val localChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) + val uploadRequestResult = + UploadRequestResult.Success( + listOf(ResourceUploadResponseMapping(localChanges, postSyncPatient)), + ) + + resourceConsolidator.consolidate(uploadRequestResult) + + assertThat( + (database.select(ResourceType.Observation, "observation1") as Observation) + .subject + .reference, + ) + .isEqualTo("Patient/patient2") + } + + @Test + fun resourceConsolidator_singleResourceUpload_shouldUpdateReferenceValueOfLocalReferencingResources() = + runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + val observation = + Observation().apply { + id = "observation1" + subject = Reference().apply { reference = "Patient/patient1" } + } + database.insert(preSyncPatient, observation) + val postSyncPatient = + Patient().apply { + id = "patient2" + meta = + Meta().apply { + versionId = "1" + lastUpdatedElement = InstantType.now() + } + } + val localChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) + val uploadRequestResult = + UploadRequestResult.Success( + listOf(ResourceUploadResponseMapping(localChanges, postSyncPatient)), + ) + + resourceConsolidator.consolidate(uploadRequestResult) + + val localChange = database.getLocalChanges(ResourceType.Observation, "observation1").last() + assertThat( + (FhirContext.forR4Cached().newJsonParser().parseResource(localChange.payload) + as Observation) + .subject + .reference, + ) + .isEqualTo("Patient/patient2") + } + + @Test + fun resourceConsolidator_bundleComponentUploadResponse_shouldUpdateNewResourceId() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val localChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) + val bundleEntryComponentJsonString = """ { - "resourceType": "Observation", - "id": "observation1", - "subject": { - "reference": "Patient/patient1" - } + "resourceType": "Bundle", + "id": "bundle1", + "type": "transaction-response", + "entry": [ + { + "response": { + "status": "201 Created", + "location": "Patient/patient2/_history/1", + "etag": "1", + "lastModified": "2024-04-08T11:15:42.648+00:00", + "outcome": { + "resourceType": "OperationOutcome" + } + } + }, + { + "response": { + "status": "201 Created", + "location": "Encounter/8055/_history/1", + "etag": "1", + "lastModified": "2024-04-08T11:15:42.648+00:00", + "outcome": { + "resourceType": "OperationOutcome" + } + } + } + ] } """ .trimIndent() - val observation = - FhirContext.forR4Cached().newJsonParser().parseResource(observationJsonString) - as DomainResource - database.insert(patient, observation) - val postSyncPatientJsonString = - """ - { - "resourceType": "Patient", - "id": "patient2", - "meta": { - "versionId": "1" - } - } - """ - .trimIndent() - val postSyncPatient = - FhirContext.forR4Cached().newJsonParser().parseResource(postSyncPatientJsonString) - as DomainResource - postSyncPatient.meta.lastUpdatedElement = InstantType.now() - val localChanges = database.getLocalChanges(patient.resourceType, patient.logicalId) + + val postSyncResponseBundle = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .parseResource(bundleEntryComponentJsonString) as Bundle + val patientResponseEntry = + (postSyncResponseBundle.entry.first() as BundleEntryComponent).response val uploadRequestResult = UploadRequestResult.Success( - listOf(ResourceUploadResponseMapping(localChanges, postSyncPatient)), + listOf(BundleComponentUploadResponseMapping(localChanges, patientResponseEntry)), ) resourceConsolidator.consolidate(uploadRequestResult) - assertThat( - (database.select(ResourceType.Observation, "observation1") as Observation) - .subject - .reference, - ) - .isEqualTo("Patient/patient2") + assertThat(database.select(ResourceType.Patient, "patient2").logicalId) + .isEqualTo(patientResponseEntry.resourceIdAndType?.first) + + val exception = + assertThrows(ResourceNotFoundException::class.java) { + runBlocking { database.select(ResourceType.Patient, "patient1") } + } + + assertThat(exception.message).isEqualTo("Resource not found with type Patient and id patient1!") } @Test - fun consolidate_localChanges_shouldUpdateReferenceValue() = runBlocking { - val patientJsonString = - """ + fun resourceConsolidator_bundleComponentUploadResponse_shouldUpdateReferenceValueOfReferencingResources() = + runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + val preSyncObservation = + Observation().apply { + id = "observation1" + subject = Reference("Patient/patient1") + } + database.insert(preSyncPatient, preSyncObservation) + val patientLocalChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) + val observationLocalChanges = + database.getLocalChanges(preSyncObservation.resourceType, preSyncObservation.logicalId) + val bundleEntryComponentJsonString = + """ { - "resourceType": "Patient", - "id": "patient1" + "resourceType": "Bundle", + "id": "bundle1", + "type": "transaction-response", + "entry": [ + { + "response": { + "status": "201 Created", + "location": "Patient/patient2/_history/1", + "etag": "1", + "lastModified": "2024-04-08T11:15:42.648+00:00", + "outcome": { + "resourceType": "OperationOutcome" + } + } + }, + { + "response": { + "status": "201 Created", + "location": "Observation/observation2/_history/1", + "etag": "1", + "lastModified": "2024-04-08T11:15:42.648+00:00", + "outcome": { + "resourceType": "OperationOutcome" + } + } + } + ] } """ - .trimIndent() - val patient = - FhirContext.forR4Cached().newJsonParser().parseResource(patientJsonString) as DomainResource - val observationJsonString = + .trimIndent() + + val postSyncResponseBundle = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .parseResource(bundleEntryComponentJsonString) as Bundle + + val patientResponseEntry = + (postSyncResponseBundle.entry.first() as BundleEntryComponent).response + val observationResponseEntry = + (postSyncResponseBundle.entry[1] as BundleEntryComponent).response + + val uploadRequestResult = + UploadRequestResult.Success( + listOf( + BundleComponentUploadResponseMapping(patientLocalChanges, patientResponseEntry), + BundleComponentUploadResponseMapping(observationLocalChanges, observationResponseEntry), + ), + ) + + resourceConsolidator.consolidate(uploadRequestResult) + + assertThat( + (database.select(ResourceType.Observation, "observation2") as Observation) + .subject + .reference, + ) + .isEqualTo("Patient/patient2") + } + + @Test + fun resourceConsolidator_bundleComponentUploadResponse_shouldDiscardLocalChanges() = runBlocking { + val preSyncPatient = Patient().apply { id = "patient1" } + database.insert(preSyncPatient) + val localChanges = + database.getLocalChanges(preSyncPatient.resourceType, preSyncPatient.logicalId) + val bundleEntryComponentJsonString = """ { - "resourceType": "Observation", - "id": "observation1", - "subject": { - "reference": "Patient/patient1" - } + "resourceType": "Bundle", + "id": "bundle1", + "type": "transaction-response", + "entry": [ + { + "response": { + "status": "201 Created", + "location": "Patient/patient2/_history/1", + "etag": "1" + } + } + ] } """ .trimIndent() - val observation = - FhirContext.forR4Cached().newJsonParser().parseResource(observationJsonString) - as DomainResource - database.insert(patient, observation) - val postSyncPatientJsonString = - """ - { - "resourceType": "Patient", - "id": "patient2", - "meta": { - "versionId": "1" - } - } - """ - .trimIndent() - val postSyncPatient = - FhirContext.forR4Cached().newJsonParser().parseResource(postSyncPatientJsonString) - as DomainResource - postSyncPatient.meta.lastUpdatedElement = InstantType.now() - val localChanges = database.getLocalChanges(patient.resourceType, patient.logicalId) + val postSyncResponseBundle = + FhirContext.forCached(FhirVersionEnum.R4) + .newJsonParser() + .parseResource(bundleEntryComponentJsonString) as Bundle + val patientResponseEntry = + (postSyncResponseBundle.entry.first() as BundleEntryComponent).response val uploadRequestResult = UploadRequestResult.Success( - listOf(ResourceUploadResponseMapping(localChanges, postSyncPatient)), + listOf(BundleComponentUploadResponseMapping(localChanges, patientResponseEntry)), ) resourceConsolidator.consolidate(uploadRequestResult) - val localChange = database.getLocalChanges(ResourceType.Observation, "observation1").last() - assertThat( - (FhirContext.forR4Cached().newJsonParser().parseResource(localChange.payload) - as Observation) - .subject - .reference, - ) - .isEqualTo("Patient/patient2") + assertThat(database.getAllLocalChanges()).isEmpty() } } diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index 58f1807e7e..bb4dfb0241 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -112,7 +112,7 @@ interface FhirEngine { * This function initiates multiple server calls to upload local changes. The results of each call * are emitted as [UploadRequestResult] objects, which can be collected using a [Flow]. * - * @param localChangesFetchMode Specifies how to fetch local changes for upload. + * @param uploadStrategy Defines strategies for uploading FHIR resource. * @param upload A suspending function that takes a list of [LocalChange] objects and returns a * [Flow] of [UploadRequestResult] objects. * @return A [Flow] that emits the progress of the synchronization process as [SyncUploadProgress] diff --git a/engine/src/main/java/com/google/android/fhir/MoreResources.kt b/engine/src/main/java/com/google/android/fhir/MoreResources.kt index f444acbe19..e4e84a141d 100644 --- a/engine/src/main/java/com/google/android/fhir/MoreResources.kt +++ b/engine/src/main/java/com/google/android/fhir/MoreResources.kt @@ -1,5 +1,5 @@ /* - * Copyright 2022-2023 Google LLC + * Copyright 2022-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,10 @@ package com.google.android.fhir import java.lang.reflect.InvocationTargetException +import java.time.Instant +import java.util.Date +import org.hl7.fhir.r4.model.IdType +import org.hl7.fhir.r4.model.InstantType import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -58,7 +62,20 @@ fun getResourceClass(resourceType: String): Class { } internal val Resource.versionId: String? - get() = meta.versionId + get() = if (hasMeta()) meta.versionId else null internal val Resource.lastUpdated get() = if (hasMeta()) meta.lastUpdated?.toInstant() else null + +/** + * Updates the meta information of a FHIR [Resource] with the provided version ID and last updated + * timestamp. This extension function sets the version ID and last updated time in the resource's + * metadata. If the provided values are null, the respective fields in the meta will remain + * unchanged. + */ +internal fun Resource.updateMeta(versionId: String?, lastUpdatedRemote: Instant?) { + meta.apply { + versionId?.let { versionIdElement = IdType(it) } + lastUpdatedRemote?.let { lastUpdatedElement = InstantType(Date.from(it)) } + } +} diff --git a/engine/src/main/java/com/google/android/fhir/db/Database.kt b/engine/src/main/java/com/google/android/fhir/db/Database.kt index 2c72e0d401..7f321ae712 100644 --- a/engine/src/main/java/com/google/android/fhir/db/Database.kt +++ b/engine/src/main/java/com/google/android/fhir/db/Database.kt @@ -59,8 +59,20 @@ internal interface Database { suspend fun updateVersionIdAndLastUpdated( resourceId: String, resourceType: ResourceType, - versionId: String, - lastUpdated: Instant, + versionId: String?, + lastUpdated: Instant?, + ) + + /** + * Updates the existing [oldResourceId] with the new [newResourceId]. Even if [oldResourceId] and + * [newResourceId] are the same, it is still necessary to update the resource meta. + */ + suspend fun updateResourcePostSync( + oldResourceId: String, + newResourceId: String, + resourceType: ResourceType, + versionId: String?, + lastUpdated: Instant?, ) /** diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt index cf8ab22c07..408c3a7054 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt @@ -33,13 +33,16 @@ import com.google.android.fhir.db.impl.DatabaseImpl.Companion.UNENCRYPTED_DATABA import com.google.android.fhir.db.impl.dao.ForwardIncludeSearchResult import com.google.android.fhir.db.impl.dao.LocalChangeDao.Companion.SQLITE_LIMIT_MAX_VARIABLE_NUMBER import com.google.android.fhir.db.impl.dao.ReverseIncludeSearchResult +import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.db.impl.entities.ResourceEntity import com.google.android.fhir.index.ResourceIndexer import com.google.android.fhir.logicalId import com.google.android.fhir.search.SearchQuery import com.google.android.fhir.toLocalChange +import com.google.android.fhir.updateMeta import java.time.Instant import java.util.UUID +import org.hl7.fhir.r4.model.IdType import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -161,19 +164,38 @@ internal class DatabaseImpl( override suspend fun updateVersionIdAndLastUpdated( resourceId: String, resourceType: ResourceType, - versionId: String, - lastUpdated: Instant, + versionId: String?, + lastUpdatedRemote: Instant?, ) { db.withTransaction { resourceDao.updateAndIndexRemoteVersionIdAndLastUpdate( resourceId, resourceType, versionId, - lastUpdated, + lastUpdatedRemote, ) } } + override suspend fun updateResourcePostSync( + oldResourceId: String, + newResourceId: String, + resourceType: ResourceType, + versionId: String?, + lastUpdatedRemote: Instant?, + ) { + db.withTransaction { + resourceDao.getResourceEntity(oldResourceId, resourceType)?.let { oldResourceEntity -> + val updatedResource = + (iParser.parseResource(oldResourceEntity.serializedResource) as Resource).apply { + idElement = IdType(newResourceId) + updateMeta(versionId, lastUpdatedRemote) + } + updateResourceAndReferences(oldResourceId, updatedResource) + } + } + } + override suspend fun select(type: ResourceType, id: String): Resource { return resourceDao.getResource(resourceId = id, resourceType = type)?.let { iParser.parseResource(it) as Resource @@ -290,6 +312,10 @@ internal class DatabaseImpl( val resourceUuid = currentResourceEntity.resourceUuid updateResourceEntity(resourceUuid, updatedResource) + if (currentResourceId == updatedResource.logicalId) { + return@withTransaction + } + /** * Update LocalChange records and identify referring resources. * diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/JsonUtils.kt b/engine/src/main/java/com/google/android/fhir/db/impl/JsonUtils.kt index 31be98d1e1..637c17b136 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/JsonUtils.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/JsonUtils.kt @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2023-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt index 7201905aa9..68069b8ab7 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt @@ -421,7 +421,7 @@ internal abstract class LocalChangeDao { * @return A list of distinct resource UUIDs for all `LocalChangeEntity` records that referenced * the old resource. */ - private suspend fun updateReferencesInLocalChange( + internal suspend fun updateReferencesInLocalChange( oldResource: Resource, updatedResourceId: String, ): List { diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt index 449277f31d..0f219d84dd 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt @@ -37,11 +37,11 @@ import com.google.android.fhir.db.impl.entities.StringIndexEntity import com.google.android.fhir.db.impl.entities.TokenIndexEntity import com.google.android.fhir.db.impl.entities.UriIndexEntity import com.google.android.fhir.index.ResourceIndexer -import com.google.android.fhir.index.ResourceIndexer.Companion.createLastUpdatedIndex import com.google.android.fhir.index.ResourceIndexer.Companion.createLocalLastUpdatedIndex import com.google.android.fhir.index.ResourceIndices import com.google.android.fhir.lastUpdated import com.google.android.fhir.logicalId +import com.google.android.fhir.updateMeta import com.google.android.fhir.versionId import java.time.Instant import java.util.Date @@ -87,8 +87,8 @@ internal abstract class ResourceDao { it.copy( resourceId = updatedResource.logicalId, serializedResource = iParser.encodeResourceToString(updatedResource), - lastUpdatedRemote = updatedResource.meta.lastUpdated?.toInstant() ?: it.lastUpdatedRemote, - versionId = updatedResource.meta.versionId, + lastUpdatedRemote = updatedResource.lastUpdated ?: it.lastUpdatedRemote, + versionId = updatedResource.versionId ?: it.versionId, ) updateChanges(entity, updatedResource) } @@ -181,8 +181,8 @@ internal abstract class ResourceDao { abstract suspend fun updateRemoteVersionIdAndLastUpdate( resourceId: String, resourceType: ResourceType, - versionId: String, - lastUpdatedRemote: Instant, + versionId: String?, + lastUpdatedRemote: Instant?, ) @Query( @@ -293,21 +293,13 @@ internal abstract class ResourceDao { suspend fun updateAndIndexRemoteVersionIdAndLastUpdate( resourceId: String, resourceType: ResourceType, - versionId: String, - lastUpdated: Instant, + versionId: String?, + lastUpdatedRemote: Instant?, ) { - updateRemoteVersionIdAndLastUpdate(resourceId, resourceType, versionId, lastUpdated) - // update the remote lastUpdated index - getResourceEntity(resourceId, resourceType)?.let { - val indicesToUpdate = - ResourceIndices.Builder(resourceType, resourceId) - .apply { - addDateTimeIndex( - createLastUpdatedIndex(resourceType, InstantType(Date.from(lastUpdated))), - ) - } - .build() - updateIndicesForResource(indicesToUpdate, resourceType, it.resourceUuid) + getResourceEntity(resourceId, resourceType)?.let { oldResourceEntity -> + val resource = iParser.parseResource(oldResourceEntity.serializedResource) as Resource + resource.updateMeta(versionId, lastUpdatedRemote) + updateResourceWithUuid(oldResourceEntity.resourceUuid, resource) } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt index ea8870b5f5..e946096eef 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt @@ -18,10 +18,11 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.Database +import com.google.android.fhir.lastUpdated import com.google.android.fhir.sync.upload.request.UploadRequestGeneratorMode +import com.google.android.fhir.versionId import org.hl7.fhir.r4.model.Bundle import org.hl7.fhir.r4.model.DomainResource -import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.hl7.fhir.r4.model.codesystems.HttpVerb @@ -56,8 +57,12 @@ internal class DefaultResourceConsolidator(private val database: Database) : Res ) uploadRequestResult.successfulUploadResponseMappings.forEach { when (it) { - is BundleComponentUploadResponseMapping -> updateVersionIdAndLastUpdated(it.output) - is ResourceUploadResponseMapping -> updateVersionIdAndLastUpdated(it.output) + is BundleComponentUploadResponseMapping -> { + updateResourceMeta(it.output) + } + is ResourceUploadResponseMapping -> { + updateResourceMeta(it.output) + } } } } @@ -68,52 +73,57 @@ internal class DefaultResourceConsolidator(private val database: Database) : Res } } - private suspend fun updateVersionIdAndLastUpdated(response: Bundle.BundleEntryResponseComponent) { - if (response.hasEtag() && response.hasLastModified() && response.hasLocation()) { - response.resourceIdAndType?.let { (id, type) -> - database.updateVersionIdAndLastUpdated( - id, - type, - getVersionFromETag(response.etag), - response.lastModified.toInstant(), - ) - } - } - } - - private suspend fun updateVersionIdAndLastUpdated(resource: DomainResource) { - if (resource.hasMeta() && resource.meta.hasVersionId() && resource.meta.hasLastUpdated()) { + private suspend fun updateResourceMeta(response: Bundle.BundleEntryResponseComponent) { + response.resourceIdAndType?.let { (id, type) -> database.updateVersionIdAndLastUpdated( - resource.id, - resource.resourceType, - resource.meta.versionId, - resource.meta.lastUpdated.toInstant(), + id, + type, + response.etag?.let { getVersionFromETag(response.etag) }, + response.lastModified?.let { it.toInstant() }, ) } } + + private suspend fun updateResourceMeta(resource: DomainResource) { + database.updateVersionIdAndLastUpdated( + resource.id, + resource.resourceType, + resource.versionId, + resource.lastUpdated, + ) + } } internal class HttpPostResourceConsolidator(private val database: Database) : ResourceConsolidator { override suspend fun consolidate(uploadRequestResult: UploadRequestResult) = when (uploadRequestResult) { is UploadRequestResult.Success -> { - database.deleteUpdates( - LocalChangeToken( - uploadRequestResult.successfulUploadResponseMappings.flatMap { - it.localChanges.flatMap { localChange -> localChange.token.ids } - }, - ), - ) - uploadRequestResult.successfulUploadResponseMappings.forEach { - when (it) { + uploadRequestResult.successfulUploadResponseMappings.forEach { responseMapping -> + when (responseMapping) { is BundleComponentUploadResponseMapping -> { - // TODO https://github.com/google/android-fhir/issues/2499 - throw NotImplementedError() + responseMapping.localChanges.firstOrNull()?.resourceId?.let { preSyncResourceId -> + database.deleteUpdates( + LocalChangeToken( + responseMapping.localChanges.flatMap { localChange -> localChange.token.ids }, + ), + ) + updateResourcePostSync( + preSyncResourceId, + responseMapping.output, + ) + } } is ResourceUploadResponseMapping -> { - val preSyncResourceId = it.localChanges.firstOrNull()?.resourceId - preSyncResourceId?.let { preSyncResourceId -> - updateResourcePostSync(preSyncResourceId, it.output) + database.deleteUpdates( + LocalChangeToken( + responseMapping.localChanges.flatMap { localChange -> localChange.token.ids }, + ), + ) + responseMapping.localChanges.firstOrNull()?.resourceId?.let { preSyncResourceId -> + database.updateResourceAndReferences( + preSyncResourceId, + responseMapping.output, + ) } } } @@ -128,16 +138,15 @@ internal class HttpPostResourceConsolidator(private val database: Database) : Re private suspend fun updateResourcePostSync( preSyncResourceId: String, - postSyncResource: Resource, + response: Bundle.BundleEntryResponseComponent, ) { - if ( - postSyncResource.hasMeta() && - postSyncResource.meta.hasVersionId() && - postSyncResource.meta.hasLastUpdated() - ) { - database.updateResourceAndReferences( + response.resourceIdAndType?.let { (postSyncResourceID, resourceType) -> + database.updateResourcePostSync( preSyncResourceId, - postSyncResource, + postSyncResourceID, + resourceType, + response.etag?.let { getVersionFromETag(response.etag) }, + response.lastModified?.let { response.lastModified.toInstant() }, ) } } @@ -165,7 +174,7 @@ private fun getVersionFromETag(eTag: String) = * 1. absolute path: `///_history/` * 2. relative path: `//_history/` */ -private val Bundle.BundleEntryResponseComponent.resourceIdAndType: Pair? +internal val Bundle.BundleEntryResponseComponent.resourceIdAndType: Pair? get() = location ?.split("/") diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/UploadStrategy.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/UploadStrategy.kt index a926aac516..cb23ce6d3a 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/UploadStrategy.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/UploadStrategy.kt @@ -90,7 +90,7 @@ private constructor( * Not yet implemented - Fetches all local changes, generates one patch per resource, and uploads * them in a single bundled POST request. */ - private object AllChangesSquashedBundlePost : + object AllChangesSquashedBundlePost : UploadStrategy( LocalChangesFetchMode.AllChanges, PatchGeneratorMode.PerResource, diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt index 28486b559d..9997b473d6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/BundleEntryComponentGeneratorImplementations.kt @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2023-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,13 @@ internal class HttpPutForCreateEntryComponentGenerator(useETagForUpload: Boolean } } +internal class HttpPostForCreateEntryComponentGenerator(useETagForUpload: Boolean) : + BundleEntryComponentGenerator(Bundle.HTTPVerb.POST, useETagForUpload) { + override fun getEntryResource(patch: Patch): IBaseResource { + return FhirContext.forCached(FhirVersionEnum.R4).newJsonParser().parseResource(patch.payload) + } +} + internal class HttpPatchForUpdateEntryComponentGenerator(useETagForUpload: Boolean) : BundleEntryComponentGenerator(Bundle.HTTPVerb.PATCH, useETagForUpload) { override fun getEntryResource(patch: Patch): IBaseResource { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt index d622eee5d9..91735dd616 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/request/TransactionBundleGenerator.kt @@ -97,6 +97,7 @@ internal class TransactionBundleGenerator( private val createMapping = mapOf( Bundle.HTTPVerb.PUT to this::putForCreateBasedBundleComponentMapper, + Bundle.HTTPVerb.POST to this::postForCreateBasedBundleComponentMapper, ) private val updateMapping = @@ -143,6 +144,10 @@ internal class TransactionBundleGenerator( useETagForUpload: Boolean, ): BundleEntryComponentGenerator = HttpPutForCreateEntryComponentGenerator(useETagForUpload) + private fun postForCreateBasedBundleComponentMapper( + useETagForUpload: Boolean, + ): BundleEntryComponentGenerator = HttpPostForCreateEntryComponentGenerator(useETagForUpload) + private fun patchForUpdateBasedBundleComponentMapper( useETagForUpload: Boolean, ): BundleEntryComponentGenerator = HttpPatchForUpdateEntryComponentGenerator(useETagForUpload) diff --git a/engine/src/test/java/com/google/android/fhir/MoreResourcesTest.kt b/engine/src/test/java/com/google/android/fhir/MoreResourcesTest.kt index 9290dacedc..acec8ff664 100644 --- a/engine/src/test/java/com/google/android/fhir/MoreResourcesTest.kt +++ b/engine/src/test/java/com/google/android/fhir/MoreResourcesTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2022 Google LLC + * Copyright 2022-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,10 @@ package com.google.android.fhir import android.os.Build import com.google.common.truth.Truth.assertThat +import java.time.Instant +import java.util.* +import org.hl7.fhir.r4.model.InstantType +import org.hl7.fhir.r4.model.Meta import org.hl7.fhir.r4.model.Patient import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType @@ -43,4 +47,37 @@ class MoreResourcesTest { fun `getResourceClass() by resource type should return resource class`() { assertThat(getResourceClass(ResourceType.Patient)).isEqualTo(Patient::class.java) } + + @Test + fun `updateMeta should update resource meta with given versionId and lastUpdated`() { + val versionId = "1" + val instantValue = Instant.now() + val resource = Patient().apply { id = "patient" } + + resource.updateMeta(versionId, instantValue) + + assertThat(resource.meta.versionId).isEqualTo(versionId) + assertThat(resource.meta.lastUpdatedElement.value) + .isEqualTo(InstantType(Date.from(instantValue)).value) + } + + @Test + fun `updateMeta should not change existing meta if new values are null`() { + val versionId = "1" + val instantValue = InstantType(Date.from(Instant.now())) + val resource = + Patient().apply { + id = "patient" + meta = + Meta().apply { + this.versionId = versionId + lastUpdatedElement = instantValue + } + } + + resource.updateMeta(null, null) + + assertThat(resource.meta.versionId).isEqualTo(versionId) + assertThat(resource.meta.lastUpdatedElement.value).isEqualTo(instantValue.value) + } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt index 998ffcb4e5..a4050cb791 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/request/TransactionBundleGeneratorTest.kt @@ -103,6 +103,23 @@ class TransactionBundleGeneratorTest { } } + @Test + fun `getGenerator() should not throw exception for create by POST`() = runBlocking { + val exception = + kotlin + .runCatching { + TransactionBundleGenerator.Factory.getGenerator( + Bundle.HTTPVerb.POST, + Bundle.HTTPVerb.PATCH, + generatedBundleSize = 500, + useETagForUpload = true, + ) + } + .exceptionOrNull() + + assert(exception !is IllegalArgumentException) { "IllegalArgumentException was thrown" } + } + @Test fun `generate() should return Bundle Entry without if-match when useETagForUpload is false`() = runBlocking { @@ -259,22 +276,6 @@ class TransactionBundleGeneratorTest { assertThat(exception.localizedMessage).isEqualTo("Creation using PATCH is not supported.") } - @Test - fun `getGenerator() should through exception for create by POST`() { - val exception = - assertThrows(IllegalArgumentException::class.java) { - runBlocking { - TransactionBundleGenerator.Factory.getGenerator( - Bundle.HTTPVerb.POST, - Bundle.HTTPVerb.PATCH, - generatedBundleSize = 500, - useETagForUpload = true, - ) - } - } - assertThat(exception.localizedMessage).isEqualTo("Creation using POST is not supported.") - } - @Test fun `getGenerator() should through exception for update by DELETE`() { val exception =