Exception in DescriptorBuilder when inherited properties have the same name

[fra-pa]

Hi,

the DescriptorBuilder throws an exception if a property with the same name is defined in the inheritance chain of more than one of its super classes:

Error: property <any> already defined; override of <mh:AnotherRoot#mh:any> by <props:Root#props:any> not allowed without redefines
      at DescriptorBuilder._fd7‍.r.DescriptorBuilder.assertNotDefined (lib/descriptor-builder.js:180:11)
      at DescriptorBuilder._fd7‍.r.DescriptorBuilder.addNamedProperty (lib/descriptor-builder.js:138:10)
      at DescriptorBuilder._fd7‍.r.DescriptorBuilder.addProperty (lib/descriptor-builder.js:53:8)
      at DescriptorBuilder.<anonymous> (lib/descriptor-builder.js:227:12)
      at forEach (node_modules/min-dash/dist/index.js:170:20)
      at DescriptorBuilder._fd7‍.r.DescriptorBuilder.addTrait (lib/descriptor-builder.js:202:3)
      at lib/registry.js:185:13
      at Registry._0ac‍.r.Registry.mapTypes (lib/registry.js:165:3)
      at traverseSuper (lib/registry.js:155:10)
      at forEach (node_modules/min-dash/dist/index.js:170:20)
      at Registry._0ac‍.r.Registry.mapTypes (lib/registry.js:162:3)
      at Registry._0ac‍.r.Registry.getEffectiveDescriptor (lib/registry.js:184:8)
      at Moddle._1b8‍.r.Moddle.getType (lib/moddle.js:97:32)
      at Moddle._1b8‍.r.Moddle.create (lib/moddle.js:65:19)
      at Context.<anonymous> (test/spec/properties.js:589:34)
      at processImmediate (internal/timers.js:461:21)

I created a test case with a simplified hierarchy:

mh:MultipleInherited has two properties named any:

• mh:AnotherRoot#any
• props:Root#any

and the DescriptorBuilder does not map them into seperate namespaces.

In my use-case, the problem occurs with the UMLDI meta-model, where all the diagram types inherit from UMLDI:UMLDiagram:

Regards,
Frank

[nikku]

Thanks for opening this bug report.

Before looking into the specifics of your attached PR, let's take at the desired behavior. What is the "common sense" to deconflict such inheritance conflicts? Closest in tree wins? Both properties continue to be accessible, i.e. element.ownedElement and element.UMLDI_ownedElement are accessible? Given the scope (class diagrams) I assume that inheritance implies overriding in a reasonable manner.

[fra-pa]

@nikku

What is the "common sense" to deconflict such inheritance conflicts?

As far as I can see, this is a general problem in OO with multiple inheritance and there doesn't seem to exist a common solution. Different implementations use different ways and they all have advantages and disadvantages.

Closest in tree wins?

Only if the name is unique in the given context and therefore a 'closest in tree' exists. The PR gives an example where this is not the case:

Both properties continue to be accessible, i.e. element.ownedElement and element.UMLDI_ownedElement are accessible

Right, both should still be accessible. But I'd prefere an adressing scheme from the inside out. That is, the two ownedElement are unambiguously identified with UML.Element.ownedElement and UMLDI.UMLDiagramElement.ownedElement. Inside a given context, the parts of the prefix that are equal can be omitted. I.e. in the context of UMLDI.UMLDiagram, the property UMLDI.UMLDiagramElement.ownedElement can be referenced with its full name or ownedElement only, because it resists in the outer context of UMLDI.

This allows unambiguous referencing but needs a more general concept for namespaces. moddle has namespaces only for the scope of a model. Therefore, names can be resolved if they are unique in the whole model.

The PR is a solution in respect to the current implemention for namespaces. But it still fails if the properties belong to the same model (see example above).

The property any exists twice in the context of props and cannot unambiguously be resolved. It's not possible to reference props.Root.any or props.Base.any explicitly.

As noted in the PR, it should be possible to avoid the problem with a proper design because the designer has influence on the naming. But it's not always possible to avoid conflicts if multiple models are involved, as they may come from a third party.

To tackle the problem in a more general way, the concept of a namespace needs to be extended to also include types. This, by the way, makes much sense, as types are in effect namespaces.

IMO, that would be a nice enhancement and if you are open for this, I create a seperate issue for this (though it may take some time to implement).

[nikku]

Thanks for getting back in a timely manner and thanks for providing the information in a good way (digestible for me, at least).

As it looks like UML spec writers did go the extra mile to build things that are hard to reason about, introduced avoidable conflicts (ownedElement) that a small and focused library like moddle will have a hard time to address.

I'm honestly not sure how many additional features we'd need to support UML read and write, end-to-end, in an appropriate manner. How would moddle look like if it supported the full UML spec? How would it perform? And would other tools to build on top of it continue to work?

We do throw a bug right now to indicate that such models are not supported right now. We really need to make sure we support them and all the cryptic edge cases involved before we get rid of this explicit error.

[fra-pa]

@nikku

As it looks like UML spec writers did go the extra mile to build things that are hard to reason about, introduced avoidable conflicts (ownedElement) that a small and focused library like moddle will have a hard time to address.

The UML is a really big beast and I only scratch the surface for the most parts. There are some design decisions that raise questions and I hardly understand. But they are a given industry standard and can hardly be ingored.

I'm primarly interested in the StateMachine package and was searching for a meta-model that provides some infrastructure to fit my needs. I came about moddle because it' nicely small and pragmatic, but also quite flexible.

I'm honestly not sure how many additional features we'd need to support UML read and write, end-to-end, in an appropriate manner. How would moddle look like if it supported the full UML spec? How would it perform? And would other tools to build on top of it continue to work?

It raises trust in the library, that you're serious about compatibility and feel resposible for the dependencies. After all, it's not a hobby project and a business depends on it. I appreciate your professional acting and value a conservative take on changes.

Reaching for full UML implementation is not my intention. With my use-case, I definitely see some limitations in using moddle. In some cases this is not a bad thing, as it forces me to limit myself and stay more pragmatic. But some workarounds are rather ugly and not easy to live with. Especially, if there seems to be an easy fix or enhancement that would avoid this (as it is the case with this issue) and maybe could also be useful for others.

I'd like to help making moddle better, not worse. Of course, I have my own interests and a limited POV.

We do throw a bug right now to indicate that such models are not supported right now. We really need to make sure we support them and all the cryptic edge cases involved before we get rid of this explicit error.

I understand, that you have to keep focus on your use-cases. Integrating code needs review and testing, foreign code even more. Particular for features that only would be nice to have, this may be hard to justify.

I don't take it for granted, that you take time to respond to issues, even if you're not directly affected by them. That's why I take some time to work them out and hopefully make them easier to grasp, maybe helpful.

IMO, the PR is not deeply invasive and I tried to follow your coding style. I don't see negative side effects, but for sure my sight is limited. If you need something to be changed for acceptance (style, more testing, ...), I'm open for suggestions to integrate.

Nevertheless, it'll take your limited time to even take look at it ... plus review and integration testing.

In any case, thanks for your support.

[nikku]

But some workarounds are rather ugly and not easy to live with. Especially, if there seems to be an easy fix or enhancement that would avoid this (as it is the case with this issue) and maybe could also be useful for others.

I totally agree with you on this one and namespace support is definitely missing for attributes at the moment. I'll double check compatibility with some of our own downstream uses of model once I find the time to do so.

In the mean time I believe the best path forward would be for you to work on a fork of this library. You'll likely find additional features to be missing and if you share them, that would be wonderful. If we have an end-to-end view on what capabilities you'd need it will be easier for us to evaluate if it falls into the scope of this library (or not).

[fra-pa]

@nikku

I totally agree with you on this one and namespace support is definitely missing for attributes at the moment. I'll double check compatibility with some of our own downstream uses of model once I find the time to do so.

Thanks in advance for taking time and consider an integration.

In the mean time I believe the best path forward would be for you to work on a fork of this library. You'll likely find additional features to be missing and if you share them, that would be wonderful. If we have an end-to-end view on what capabilities you'd need it will be easier for us to evaluate if it falls into the scope of this library (or not).

I'll create an integration branch in my fork. Then have seperate feature branches from master to allow merging in case they are useful and prove solid. I'd then create PRs to communicate ideas for enhancements. Let me know if you prefere issues.