openEHR / reference-models

openEHR and related reference models in computable form, including UML, XMI, BMM, Ecore, etc
20 stars 19 forks source link

Mappings property of DV_TEXT is missing mandatory field in BMM file #5

Open serefarikan opened 6 years ago

serefarikan commented 6 years ago

@wolandscat In basic types bmm, mapping property of DV_TEXT is defined as follows:

["mappings"] = (P_BMM_CONTAINER_PROPERTY) <
                name = <"mappings">
                type_def = <
                    container_type = <"Set">
                    type = <"TERM_MAPPING">
                >
                cardinality = <|>=0|>
                is_im_runtime = <True>
            >

According to specs, type_def has type P_BMM_CONTAINER_TYPE and this type has a mandatory type_def field . Based on the definition, it feels like type_def should not be mandatory. If I'm wrong, then it should be written in the bmm file but I have no idea what purpose it would serve here...

wolandscat commented 6 years ago

I'm slightly confused: type_def is defined on P_BMM_CONTAINER_PROPERTY, as an optional attribute.

serefarikan commented 6 years ago

Yes. P_BMM_CONTAINER_PROPERTY has type of type_def redefined as type_def: P_BMM_CONTAINER_TYPE

Looking at the documentation for P_BMM_CONTAINER_TYPE we can see that it has a mandatory type_def: P_BMM_TYPE field. So my expectation for the type_def attribute in the snippet above would be that it would have a nested type_def like this:```

["mappings"] = (P_BMM_CONTAINER_PROPERTY) <
                name = <"mappings">
                type_def = <
                    container_type = <"Set">
                    type = <"TERM_MAPPING">
                    -- type_def ... THIS IS THE MISSING MANDATORY FIELD I'M REFERRING TO
                >
                cardinality = <|>=0|>
                is_im_runtime = <True>
            >

I'm completely confused :)

wolandscat commented 6 years ago

I think you may be reading the UML around the wrong way. Here is the relevant part of the diagram.

image

In this diag, the type_def attribute belongs to P_BMM_CONTAINER_PROPERTY, not P_BMM_CONTAINER_TYPE.

serefarikan commented 6 years ago

I know, but I'm not referring to the UML diagram. It is the html version of the specifications that is defining the mandatory field I'm talking about. Please see the html version of the spec for P_BMM_CONTAINER_TYPE

wolandscat commented 6 years ago

Ah now I get it. Sorry, was staring at the wrong bit of the diagram. Firstly, you are right - that type_def should be 0..1, which I will fix right now in the model. Secondly, it's a bit of a bad system - types are specified in this version of P_BMM as either Strings, if they are simple types, or else structures. To achieve that there has to be something that acts like a union structure, which in this case is type and type_def - one must be populated, and the model deserialiser figures out which one to use.

This will all go away in BMM3, which will have a syntax something like Ecore model syntax. Sorry for the confusion! BTW, that type_def is the link coming off the top of P_BMM_CONTAINER_TYPE...

serefarikan commented 6 years ago

Thanks a lot for the clarification!