Skip to content

Port connection property lookup is inconsistent with other connection kinds πŸ€–Β #3109

Description

@lwrage

Summary

PortConnection is the only Connection kind with a special case in PropertyImpl.getPropertyValueFromDeclarativeModel, and that special case performs a strictly narrower lookup than the path every other connection kind takes.

  • PortConnection reaches PortConnectionImpl.getPropertyValue(Property, PropertyAcc), whose entire body is pas.addLocal(this).
  • Access, parameter, feature, and feature group connections fall into the else branch and reach ConnectionImpl.getPropertyValueInternal(prop, pas, true, false).

Comparing the two under fromInstanceSlaveCall == true, most of ConnectionImpl's extra work is disabled: addLocalContained is skipped for both the connection and its refinements, and the inherit step is skipped as well, because both are guarded by !fromInstanceSlaveCall. Skipping inherit here is intended β€” PropertyImpl.getPropertyValueInternal walks the instance container for inherited properties separately.

What remains, and what the PortConnection path does not do, is the refinement walk: ConnectionImpl iterates getRefined() and calls pas.addLocal(refined) at each step, and that call is not guarded by fromInstanceSlaveCall. So a refined port connection appears not to pick up property associations declared on its refinement ancestors, while a refined access or parameter connection does.

This is a code-inspection finding, not a reproduced failure. It is filed as a consistency issue rather than as a bug because the difference may well be unreachable: the instantiation-time property caching may already place the value on the connection instance, in which case paa.addLocal(io) in PropertyImpl.getPropertyValueInternal short-circuits and the declarative lookup is never consulted for this property. Confirming or refuting that is the first task on this issue, and no production change should be made before it is settled.

Reproduction

Not yet established. The first work item is to build the smallest valid external AADL model that distinguishes the two paths:

  1. A component implementation declaring a port connection, refined in an extending implementation, with a connection property such as Timing or Actual_Connection_Binding associated on the refinement ancestor rather than on the refinement itself.
  2. The same shape using an access or parameter connection, as a control that is expected to find the value.
  3. Instantiate both and compare the property value on the resulting connection instances.

The model must validate with no unrelated AADL errors before the comparison means anything.

If the two shapes agree, the declarative path is unreachable for this property and there is no defect. If they differ, the port connection case is the wrong one.

Expected behavior

One of two outcomes, decided by the model above:

  • The difference is observable. A refined port connection resolves property associations from its refinement ancestors, exactly as the other connection kinds do. The fix is then to remove the PortConnectionImpl override so that port connections inherit ConnectionImpl.getPropertyValueInternal, together with a regression test and fixture that fail before the removal and pass after.
  • The difference is unreachable. The narrow override is documented in place with a comment explaining why it is safe β€” which property-caching step guarantees the value is already local on the connection instance β€” and this issue closes without a production change.

Either way the asymmetry should stop being implicit. The current code gives no indication of why one connection kind out of five is treated differently.

Relevant code

  • core/org.osate.aadl2/src/org/osate/aadl2/impl/PortConnectionImpl.java, getPropertyValue β€” the narrow path
  • core/org.osate.aadl2/src/org/osate/aadl2/impl/ConnectionImpl.java, getPropertyValueInternal β€” the path taken by every other connection kind, including the getRefined() walk that is the actual difference
  • core/org.osate.aadl2/src/org/osate/aadl2/impl/PropertyImpl.java, getPropertyValueFromDeclarativeModel β€” the instanceof PortConnection branch; and getPropertyValueInternal, whose paa.addLocal(io) short-circuit is what may make the difference unreachable
  • core/org.osate.aadl2/src/org/osate/aadl2/impl/NamedElementImpl.java, getPropertyValueInternal β€” the fromInstanceSlaveCall semantics

Dependencies

Best taken after #3108. That refactor replaces the instanceof chain with overrides of a single method, which reduces the fix in the first outcome above to deleting one override β€” a far smaller and more reviewable diff than editing the dispatch chain. If this turns out to be a user-visible defect worth shipping sooner, the order can be inverted and #3108 rebased with one fewer override to write.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions