Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for pulldata with local entities #6451

Merged
merged 10 commits into from
Oct 17, 2024
2 changes: 1 addition & 1 deletion buildSrc/src/main/java/dependencies/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ object Dependencies {
const val rarepebble_colorpicker = "com.github.martin-stone:hsv-alpha-color-picker-android:3.1.0"
const val commons_io = "commons-io:commons-io:2.5" // Commons 2.6+ introduce java.nio usage that we can't access until our minSdkVersion >= 26 (https://developer.android.com/reference/java/io/File#toPath())
const val opencsv = "com.opencsv:opencsv:5.9"
const val javarosa_online = "org.getodk:javarosa:5.0.0-SNAPSHOT-30ef2a9"
const val javarosa_online = "org.getodk:javarosa:5.0.0-SNAPSHOT-6ce1352"
const val javarosa_local = "org.getodk:javarosa:local"
const val javarosa = javarosa_online
const val karumi_dexter = "com.karumi:dexter:6.2.3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ package org.odk.collect.android.feature.formentry.dynamicpreload
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.odk.collect.android.support.rules.FormEntryActivityTestRule
import org.odk.collect.android.support.StubOpenRosaServer.EntityListItem
import org.odk.collect.android.support.StubOpenRosaServer.MediaFileItem
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.FormEntryPage
import org.odk.collect.android.support.rules.CollectTestRule
import org.odk.collect.android.support.rules.TestRuleChain.chain

/**
Expand All @@ -12,16 +16,36 @@ import org.odk.collect.android.support.rules.TestRuleChain.chain
*/
class DynamicPreLoadedDataPullTest {

private val rule = FormEntryActivityTestRule()
private val rule = CollectTestRule(useDemoProject = false)
private val testDependencies = TestDependencies()

@get:Rule
val copyFormChain: RuleChain = chain()
.around(rule)
val chain: RuleChain = chain(testDependencies).around(rule)

@Test
fun canUsePullDataFunctionToPullDataFromCSV() {
rule.setUpProjectAndCopyForm("pull_data.xml", listOf("fruits.csv"))
.fillNewForm("pull_data.xml", "pull_data")
testDependencies.server.addForm("pull_data.xml", listOf(MediaFileItem("fruits.csv")))

rule.withMatchExactlyProject(testDependencies.server.url)
.startBlankForm("pull_data")
.assertText("The fruit Mango is pulled csv data.")
}

@Test
fun canUsePullDataFunctionToPullDataFromLocalEntities() {
testDependencies.server.addForm("one-question-entity-registration.xml")
testDependencies.server.addForm(
"entity-update-pulldata.xml",
listOf(EntityListItem("people.csv"))
)

rule.withMatchExactlyProject(testDependencies.server.url)
.startBlankForm("One Question Entity Registration")
.fillOutAndFinalize(FormEntryPage.QuestionAndAnswer("Name", "Logan Roy"))

.startBlankForm("Entity Update Pull Data")
.clickOnText("Logan Roy")
.swipeToNextQuestion("Name")
.assertText("Logan Roy")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.odk.collect.android.dynamicpreload.ExternalDataManagerImpl
import org.odk.collect.android.dynamicpreload.handler.ExternalDataHandlerPull
import org.odk.collect.android.tasks.FormLoaderTask.FormEntryControllerFactory
import org.odk.collect.entities.javarosa.filter.LocalEntitiesFilterStrategy
import org.odk.collect.entities.javarosa.filter.PullDataFunctionHandler
import org.odk.collect.entities.javarosa.finalization.EntityFormFinalizationProcessor
import org.odk.collect.entities.storage.EntitiesRepository
import org.odk.collect.settings.keys.ProjectKeys
Expand All @@ -25,7 +26,8 @@ class CollectFormEntryControllerFactory(
}

return FormEntryController(FormEntryModel(formDef)).also {
it.addFunctionHandler(ExternalDataHandlerPull(externalDataManager))
val externalDataHandlerPull = ExternalDataHandlerPull(externalDataManager)
it.addFunctionHandler(PullDataFunctionHandler(entitiesRepository, externalDataHandlerPull))
it.addPostProcessor(EntityFormFinalizationProcessor())

if (settings.getBoolean(ProjectKeys.KEY_LOCAL_ENTITIES)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import java.util.function.Supplier
class LocalEntitiesFilterStrategy(entitiesRepository: EntitiesRepository) :
FilterStrategy {

private val dataAdapter = LocalEntitiesInstanceAdapter(entitiesRepository)
private val instanceAdapter = LocalEntitiesInstanceAdapter(entitiesRepository)

override fun filter(
sourceInstance: DataInstance<*>,
Expand All @@ -32,7 +32,7 @@ class LocalEntitiesFilterStrategy(entitiesRepository: EntitiesRepository) :
evaluationContext: EvaluationContext,
next: Supplier<MutableList<TreeReference>>
): List<TreeReference> {
if (sourceInstance.instanceId == null || !dataAdapter.supportsInstance(sourceInstance.instanceId)) {
if (sourceInstance.instanceId == null || !instanceAdapter.supportsInstance(sourceInstance.instanceId)) {
return next.get()
}

Expand All @@ -43,7 +43,7 @@ class LocalEntitiesFilterStrategy(entitiesRepository: EntitiesRepository) :
val child = candidate.nodeSide.steps[0].name.name
val value = candidate.evalContextSide(sourceInstance, evaluationContext)

val results = dataAdapter.queryEq(
val results = instanceAdapter.queryEq(
sourceInstance.instanceId,
child,
value as String
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.odk.collect.entities.javarosa.filter

import org.javarosa.core.model.condition.EvaluationContext
import org.javarosa.core.model.condition.IFunctionHandler
import org.javarosa.xpath.expr.XPathFuncExpr
import org.odk.collect.entities.javarosa.intance.LocalEntitiesInstanceAdapter
import org.odk.collect.entities.storage.EntitiesRepository

class PullDataFunctionHandler(
entitiesRepository: EntitiesRepository,
private val fallback: IFunctionHandler? = null
) : IFunctionHandler {

private val instanceAdapter = LocalEntitiesInstanceAdapter(entitiesRepository)

override fun getName(): String {
return NAME
}

override fun getPrototypes(): List<Array<Class<Any>>> {
return emptyList()
}

override fun rawArgs(): Boolean {
return true
}

override fun realTime(): Boolean {
return false
}

override fun eval(args: Array<Any>, ec: EvaluationContext): Any {
val instanceId = XPathFuncExpr.toString(args[0])
val child = XPathFuncExpr.toString(args[1])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can move extracting child, filterChild and filterValue down and call only if instanceAdapter.supportsInstance(instanceId) returns true.

val filterChild = XPathFuncExpr.toString(args[2])
val filterValue = XPathFuncExpr.toString(args[3])

return if (instanceAdapter.supportsInstance(instanceId)) {
instanceAdapter.queryEq(instanceId, filterChild, filterValue)!!.firstOrNull()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like queryEq can't return null values so we could update that function by removing ? from the returned type and then get rid of those !!.

?.getFirstChild(child)?.value?.value ?: ""
} else {
fallback?.eval(args, ec) ?: ""
}
}

companion object {
private const val NAME = "pulldata"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class LocalEntitiesInstanceAdapter(private val entitiesRepository: EntitiesRepos

0.until(count).map {
if (it == 0) {
convertToElement(first, true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I wonder if we need that isPartial flag at all... such elements should have at least label and value right so if they don't have it we could treat them as partial and try to populate.
It is set to true in only one place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convertToElement(first)
} else {
TreeElement("item", it, true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I wonder if we need that isPartial flag at all... such elements should have at least label and value right so if they don't have it we could treat them as partial and try to populate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The partial concept is currently too general for something like that as any TreeElement can be "partial". It's not confined to secondary instances with item children.

}
Expand All @@ -33,62 +33,82 @@ class LocalEntitiesInstanceAdapter(private val entitiesRepository: EntitiesRepos
}
} else {
entitiesRepository.getEntities(instanceId).map { entity ->
convertToElement(entity, false)
convertToElement(entity)
}
}
}

fun queryEq(instanceId: String, child: String, value: String): List<TreeElement>? {
return when {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can now simplify this by adding child as a subject of when:

return when (child) {
            EntityItemElement.ID ->...

child == "name" -> {
child == EntityItemElement.ID -> {
val entity = entitiesRepository.getById(
instanceId,
value
)

if (entity != null) {
listOf(convertToElement(entity, false))
listOf(convertToElement(entity))
} else {
emptyList()
}
}

!listOf(EntityItemElement.LABEL, EntityItemElement.VERSION).contains(child) -> {
child == EntityItemElement.LABEL -> {
filterAndConvertEntities(instanceId) { it.label == value }
}

child == EntityItemElement.VERSION -> {
filterAndConvertEntities(instanceId) { it.version == value.toInt() }
}

child == EntityItemElement.TRUNK_VERSION -> {
filterAndConvertEntities(instanceId) { it.trunkVersion == value.toInt() }
}

child == EntityItemElement.BRANCH_ID -> {
filterAndConvertEntities(instanceId) { it.branchId == value }
}

else -> {
val entities = entitiesRepository.getAllByProperty(
instanceId,
child,
value
)

entities.map { convertToElement(it, false) }
entities.map { convertToElement(it) }
}

else -> null
}
}

private fun convertToElement(entity: Entity.Saved, partial: Boolean): TreeElement {
private fun filterAndConvertEntities(
list: String,
filter: (Entity.Saved) -> Boolean
): List<TreeElement> {
val entities = entitiesRepository.getEntities(list)
return entities.filter(filter).map { convertToElement(it) }
}

private fun convertToElement(entity: Entity.Saved): TreeElement {
val name = TreeElement(EntityItemElement.ID)
val label = TreeElement(EntityItemElement.LABEL)
val version = TreeElement(EntityItemElement.VERSION)
val trunkVersion = TreeElement(EntityItemElement.TRUNK_VERSION)
val branchId = TreeElement(EntityItemElement.BRANCH_ID)

if (!partial) {
name.value = StringData(entity.id)
version.value = StringData(entity.version.toString())
branchId.value = StringData(entity.branchId)
name.value = StringData(entity.id)
version.value = StringData(entity.version.toString())
branchId.value = StringData(entity.branchId)

if (entity.label != null) {
label.value = StringData(entity.label)
}
if (entity.label != null) {
label.value = StringData(entity.label)
}

if (entity.trunkVersion != null) {
trunkVersion.value = StringData(entity.trunkVersion.toString())
}
if (entity.trunkVersion != null) {
trunkVersion.value = StringData(entity.trunkVersion.toString())
}

val item = TreeElement("item", entity.index, partial)
val item = TreeElement("item", entity.index, false)
item.addChild(name)
item.addChild(label)
item.addChild(version)
Expand All @@ -97,11 +117,7 @@ class LocalEntitiesInstanceAdapter(private val entitiesRepository: EntitiesRepos

entity.properties.forEach { property ->
val propertyElement = TreeElement(property.first)

if (!partial) {
propertyElement.value = StringData(property.second)
}

propertyElement.value = StringData(property.second)
item.addChild(propertyElement)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class LocalEntitiesInstanceProviderTest {
}

@Test
fun `partial parse returns elements without values for first item and just item for others`() {
fun `partial parse returns the full first item and just item for others`() {
val entity = arrayOf(
Entity.New(
"1",
Expand All @@ -133,11 +133,9 @@ class LocalEntitiesInstanceProviderTest {
assertThat(instance.numChildren, equalTo(2))

val item1 = instance.getChildAt(0)!!
assertThat(item1.isPartial, equalTo(true))
assertThat(item1.isPartial, equalTo(false))
assertThat(item1.numChildren, equalTo(6))
0.until(item1.numChildren).forEach {
assertThat(item1.getChildAt(it).value?.value, equalTo(null))
}
assertThat(item1.getFirstChild("name")!!.value!!.value, equalTo("1"))

val item2 = instance.getChildAt(1)!!
assertThat(item2.isPartial, equalTo(true))
Expand Down
Loading