Serialization of Feature Group and Abstract Feature Direction

[philip-alldredge]

This is the root cause of osate/osate-ge#16

All directed features have a default direction of IN_OUT. However, abstract features and feature groups can only be specified to be in or out in the language. If a direction is not specified then the feature is bidirectional.

When XText erializes the model to generate the AADL source for a feature group that has a value of IN OUT, the feature group is created with the "in out" specifier. This is not valid AADL and causes errors. From the graphical editor/plugin side of things, I do not believe there is a mechanism to prevent this from happening. It needs to be resolved within the OSATE Core.

Although not ideal, I have a work around. Replacing the value converter for InOutDirection in org.osate.xtext.aadl2.valueconversion.Aadl2ValueConverter with one that returns blank text for IN_OUT values works around the issue. The reason this I do not consider it ideal is that it causes an additional space to be inserted into the resulting text. Perhaps someone with more experience with Xtext can

The workaround is included below.

    @ValueConverter(rule = "InOutDirection")
    public IValueConverter<DirectionType> InOutDirection() {
        return new IValueConverter<DirectionType>() {
            public DirectionType toValue(String string, INode node) {

                return DirectionType.get(string.toLowerCase());
            }

            @Override
            public String toString(DirectionType value) {
                if(value == DirectionType.IN_OUT) {
                    return "";
                } else {
                    return value.getName();
                }
            }
        };
    }

Additional notes: I tried to use the transient value service to mark the value as transient under certain circumstances but the value was serialized anyways.

[reteprelief]

Philip's solution inserts an extra space because Xtext thinks there was a keyword - even though it is an empty string.

The issue is that the DirectionalFeature set method sets a non-null default value. In the case of ports it does not matter since all three direction options are explicit in the grammar. In case of feature and feature group the direction is optional. When serializing Xtext knows whether an optional element should not be shown is if the value is null.

We want to override the set method to not replace null with a non-null default. We can do that specifically for abstract features and feature groups. Or we may be able to do it for the DirectionalFeature if for all other cases all elements are explicitly required or the absence is not supposed to represent one of the values.

[reteprelief]

Philip,

this was a comment to Lutz or Joe - Lutz and I discussed the issue and identified what I summarized in my previous comment.

[philip-alldredge]

Has any progress been made on this?

[lwrage]

@philip-alldredge Can you please test this again? The formatter might get rid of the additional space character automatically.

[philip-alldredge]

Same results. Without patching Aadl2ValueConverter, invalid Aadl in produced. After patching, an extra space is produced.

[lwrage]

OK, thanks. So far I haven't been able to reproduce the issue. Can you provide a small example project?

[philip-alldredge]

Its easiest to reproduce with the graphical editor. Create a feature group, switch it to input and then back to in out. On Sep 19, 2014 12:06 PM, "Lutz Wrage" notifications@github.com wrote:

OK, thanks. So far I haven't been able to reproduce the issue. Can you provide a small example project?

— Reply to this email directly or view it on GitHub https://github.com/osate/osate2-core/issues/409#issuecomment-56205527.

[lwrage]

How does the graphical editor insert a new feature group? For some reason the in out direction is not serialized for a fresh feature group. Something is different after the feature group has been changed. Can you point me to the code in the graphical editor that updates the model for both cases. Also, serializing a model from XMI works, and the in out is not inserted.

[philip-alldredge]

Relevant source code links are below. However, I'm not sure how clear it will be from that. In short when creating the feature group, createOwnedFeatureGroup() is called on the Type and the direction is left as the default. (Which is IN OUT).

However, when modifying it, setDIrection(IN_OUT) is used. I have tried setting it to null, but that sets it to the default which is IN_OUT. I've trying using eUnset, but once again, it results in setting it to the default value which is IN_OUT. Something about the process of setting it is causing Xtext to serialize it even though it is the default value.

I haven't tried making a code mod based on Peter's suggestion of overriding setDirection not to set the value to a non-null default when passing in null. That might work.

Creating: https://github.com/osate/osate-ge/blob/develop/org.osate.ge/src/org/osate/ge/diagrams/common/patterns/FeaturePattern.java#L752 https://github.com/osate/osate-ge/blob/develop/org.osate.ge/src/org/osate/ge/diagrams/common/patterns/FeaturePattern.java#L798 createOwnedFeatureGroup

Editing: https://github.com/osate/osate-ge/blob/develop/org.osate.ge/src/org/osate/ge/diagrams/type/features/SetFeatureDirectionFeature.java#L107

[lwrage]

Anything with a direction has now two boolean attributes, in and out. The direction attribute is still there but it is read-only and derived from the booleans. The code in the graphical editor should do something like the following to set the flags.

public Object modify(final Resource res, final DirectedFeature df) {
    if (df instanceof AbstractFeature || df instanceof FeatureGroup) {
        df.setIn(featDir != DirectionType.IN_OUT && featDir == DirectionType.IN);
        df.setOut(featDir != DirectionType.IN_OUT && featDir == DirectionType.OUT);
    } else {
        df.setIn(featDir == DirectionType.IN_OUT || featDir == DirectionType.IN);
        df.setOut(featDir == DirectionType.IN_OUT || featDir == DirectionType.OUT);
    }
    return null;
}

[philip-alldredge]

Thanks. I'll try to make the change this week.

[lwrage]

Current solution doesn't work correctly with ports.