cph-cachet / carp.core-kotlin

Infrastructure-agnostic framework for distributed data collection.
https://carp.cachet.dk/core/
MIT License
21 stars 3 forks source link

Add custom participant attribute API backwards compatibilty test #453

Closed Whathecode closed 8 months ago

Whathecode commented 11 months ago

Currently, there was no coverage for adding custom participant attributes to protocols in the backwards compatibility tests. The test resource also serves as an example on how such a JSON request looks like, which was a request in #451 .

Example of a protocol with both a default attribute (the server is expected to have the type information available at compile time), and a custom attributes (the type information is provided by the client in the form of an "input element").

{
    "id": "0d57f923-955e-4693-a606-cca9651aab4a",
    "createdOn": "2023-12-10T18:51:39.511538600Z",
    "version": 0,
    "ownerId": "27879e75-ccc1-4866-9ab3-4ece1b735052",
    "name": "Test protocol",
    "description": "Test description",
    "expectedParticipantData": [
        {
            "attribute": {
                "__type": "dk.cachet.carp.common.application.users.ParticipantAttribute.DefaultParticipantAttribute",
                "inputDataType": "dk.cachet.carp.input.sex"
            }
        },
        {
            "attribute": {
                "__type": "dk.cachet.carp.common.application.users.ParticipantAttribute.CustomParticipantAttribute",
                "input": {
                    "__type": "dk.cachet.carp.common.application.data.input.elements.Text",
                    "prompt": "Social Security Number"
                },
                "inputDataType": "dk.cachet.carp.input.custom.a1eda4bcd0d84ca0ba446cabfa4e84c8"
            }
        },
        {
            "attribute": {
                "__type": "dk.cachet.carp.common.application.users.ParticipantAttribute.CustomParticipantAttribute",
                "input": {
                    "__type": "dk.cachet.carp.common.application.data.input.elements.SelectOne",
                    "prompt": "Favorite OS",
                    "options": [
                        "Windows",
                        "Linux",
                        "Mac"
                    ]
                },
                "inputDataType": "dk.cachet.carp.input.custom.4ee5a0cc0ce5462b94baee4e59415f9b"
            }
        }
    ]
}

The protocol service does not validate inputDataType. Only when setting the participant data on ParticipationService, errors will occur if no type information is available for the provided inputDataType. In the Kotlin APIs, the inputDataType for custom participant attributes is generated under the namespace dk.cachet.carp.input.custom with a GUID appended, but, this is currently no requirement for the API.

bardram commented 11 months ago

LGTM. Only comment / questions is whether it is necessary to use UUID for the custom inputDataType, like

"inputDataType": "dk.cachet.carp.input.custom.4ee5a0cc0ce5462b94baee4e59415f9b"

Can't I call them something useful, like

"inputDataType": "dk.cachet.carp.input.custom.OperatingSystem"
bardram commented 11 months ago

Another question is - when do you use lower versus upper case - what is the difference between these two?

  "attribute": {
                "__type": "dk.cachet.carp.common.application.users.ParticipantAttribute.DefaultParticipantAttribute",
                "inputDataType": "dk.cachet.carp.input.sex"
            }

Why is sex lower case and DefaultParticipantAttribute upper case?

bardram commented 11 months ago

Maybe it would also be a good idea to add an example of how to upload this custom participant data to the ParticipationService? Would you, for example, need to know these strange UUIDs?

Whathecode commented 11 months ago

Can't I call them something useful, like ...

You can, but as mentioned elsewhere, I recommend you to not use CARP core namespaces to rule out any potential conflicts, and just to be clear about id scopes. That said, the .custom namespace is pretty safe as by design the goal is to only add guids there

The reason they are generated by default is because when using an API exposed through a UI, you wouldn't have to directly interact with any ID. It is much more meaningful to display the prompt the user provided to help them identify which is which. In the API, you hold a reference to the object after deserializing, so no ID is needed either. That said, I am not opposed to an API which optionally allows you to set a custom ID.

Another question is - when do you use lower versus upper case - what is the difference between these two?

That's unfortunately something that leaks out from the implementation. The PascalCase types are types in the codebase using default polymorphic serialization, which just uses the namespace.ClassName. The lowercase types are data types, which are handled differently and registered manually, since they are so central to the framework. I think it would make sense that at least all extendable types (data, devices, tasks) adopt this lowercase style, but I see that is not the case atm (devices use PascalCase).

Maybe it would also be a good idea to add an example of how to upload this custom participant data to the ParticipationService? Would you, for example, need to know these strange UUIDs

Yes, but you receive these through the endpoint which tells you what data can be set. Visually, you would use the prompt to discern which is which, but for hardcoded programming scenarios, a known id would indeed be useful. I think there is an example of setting a default attribute id, but I think the operation is the same for custom attributes. Although, you may need to know how to use the 'custom input' data type; that may be missing!

I'll see whether I can include an example in this PR.

Whathecode commented 11 months ago

Yes, but you receive these through the endpoint which tells you what data can be set.

I just noticed that although you do receive expected input elements on PrimaryDeviceDeployment through the expectedParticipantData property, this doesn't help a web interface as part of study management to display these elements.

When you receive known types and you have access to the CARP runtime, you can retrieve the statically defined input types. But, for custom types, you'd need access to the original protocol to discern the input elements (or derived types through which input elements are passed, such as PrimaryDeviceDeployment). This is undesirable coupling.

Incorporating support for retrieving InputElement's as part of ParticipationService will make sense. Maybe they can optionally be requested as part of getParticipantData. Or, a new endpoint to retrieve input elements could be added. I'll give this some thought. Both seem backwards compatible.

Whathecode commented 8 months ago

I moved the relevant tasks which emerged from this discussion to new issues, and will go ahead and merge this PR.