Comment Line Relationship (un)link behaviour

[danyeaw]

The Comment Line Test case is failing based on an assert that a multi adapter was found between cinfo.item and cinfo.connected using the IConnect interface.

I'm submitting a...

• Bug report
• Feature request
• Documentation issue or request

Current behavior

gaphor/adapters/tests/test_comment.py:156 (CommentLineTestCase.test_commentline_relationship_unlink)
self = <gaphor.adapters.tests.test_comment.CommentLineTestCase testMethod=test_commentline_relationship_unlink>
def test_commentline_relationship_unlink(self):
"""Test comment line to a relationship item connection and unlink.

Demonstrates defect #103.
"""
clazz1 = self.create(items.ClassItem, UML.Class)
clazz2 = self.create(items.ClassItem, UML.Class)
gen = self.create(items.GeneralizationItem)

self.connect(gen, gen.head, clazz1)
self.connect(gen, gen.tail, clazz2)

assert gen.subject

# now, connect comment to a generalization (relationship)
comment = self.create(items.CommentItem, UML.Comment)
line = self.create(items.CommentLineItem)
self.connect(line, line.head, comment)
self.connect(line, line.tail, gen)

self.assertTrue(gen.subject in comment.subject.annotatedElement)
self.assertTrue(comment.subject in gen.subject.ownedComment)

# FixMe: This should invoke the disconnnect handler of the line's
# handles.

>       gen.unlink()
gaphor/adapters/tests/test_comment.py:183:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
gaphor/diagram/diagramitem.py:234: in unlink
self.canvas.remove(self)
../../.cache/pypoetry/virtualenvs/gaphor-py3.7/lib/python3.7/site-packages/gaphas/canvas.py:155: in remove
self.remove_connections_to_item(item)
../../.cache/pypoetry/virtualenvs/gaphor-py3.7/lib/python3.7/site-packages/gaphas/canvas.py:371: in remove_connections_to_item
disconnect_item(*cinfo)
<decorator-gen-18>:2: in _disconnect_item
???
../../.cache/pypoetry/virtualenvs/gaphor-py3.7/lib/python3.7/site-packages/gaphas/state.py:77: in wrapper
return func(*args, **kwargs)
../../.cache/pypoetry/virtualenvs/gaphor-py3.7/lib/python3.7/site-packages/gaphas/canvas.py:355: in _disconnect_item
callback()
gaphor/ui/diagramtools.py:136: in __call__
adapter.disconnect(handle)
gaphor/adapters/connectors.py:455: in disconnect
connections = self.disconnect_connected_items()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <gaphor.adapters.classes.classconnect.GeneralizationConnect object at 0x7f3749884518>
def disconnect_connected_items(self):
"""
Cause items connected to @line to be disconnected.
This is nessesary if the subject of the @line is to be removed.

Returns a list of (item, handle) pairs that were connected (this
list can be used to connect items again with connect_connected_items()).
"""
line = self.line
canvas = self.canvas
solver = canvas.solver

# First make sure coordinates match
solver.solve()
connections = list(canvas.get_connections(connected=line))
for cinfo in connections:
adapter = component.queryMultiAdapter(
(cinfo.item, cinfo.connected), IConnect
)
>           assert adapter
E AssertionError
gaphor/adapters/connectors.py:405: AssertionError

[amolenaar]

This is the exception that's thrown from Gaphor when doing this in real life:

Adapter is <gaphor.adapters.classes.classconnect.GeneralizationConnect object at 0x114043c50>
gaphor.transaction ERROR Transaction terminated due to an exception, performing a rollback
Traceback (most recent call last):
  File "/Users/arjan/Development/gaphor/gaphor/transaction.py", line 27, in _transactional
    r = func(*args, **kwargs)
  File "/Users/arjan/Development/gaphor/gaphor/ui/diagramtools.py", line 98, in disconnect
    super(DiagramItemConnector, self).disconnect()
  File "/Users/arjan/Development/gaphor/.venv/lib/python3.7/site-packages/gaphas/aspect.py", line 306, in disconnect
    self.item.canvas.disconnect_item(self.item, self.handle)
  File "/Users/arjan/Development/gaphor/.venv/lib/python3.7/site-packages/gaphas/canvas.py", line 343, in disconnect_item
    self._disconnect_item(*cinfo)
  File "</Users/arjan/Development/gaphor/.venv/lib/python3.7/site-packages/decorator.py:decorator-gen-18>", line 2, in _disconnect_item
  File "/Users/arjan/Development/gaphor/.venv/lib/python3.7/site-packages/gaphas/state.py", line 77, in wrapper
    return func(*args, **kwargs)
  File "/Users/arjan/Development/gaphor/.venv/lib/python3.7/site-packages/gaphas/canvas.py", line 355, in _disconnect_item
    callback()
  File "/Users/arjan/Development/gaphor/gaphor/ui/diagramtools.py", line 135, in __call__
    adapter.disconnect(handle)
  File "/Users/arjan/Development/gaphor/gaphor/adapters/connectors.py", line 445, in disconnect
    connections = self.disconnect_connected_items()
  File "/Users/arjan/Development/gaphor/gaphor/adapters/connectors.py", line 395, in disconnect_connected_items
    assert adapter
AssertionError

[amolenaar]

I did a fix for this particular case in #85. The problem I see now is that, when a comment is connected to a relationship item, the comment is not re-attached, once the relationship is attached again. This will require some more investigation on our part.

[danyeaw]

I'm going to leave this issue open until we fix the other comment issue you mentioned above.

[amolenaar]

That sounds okay. There are a few cases that need to be ironed out.

For comments:

1. A comment line can only connect to a diagram item with a subject (model element) attached.
2. In case the model element is unlinked, as is the case for any line item that is no longer connected at both sides, the comment line should either be deleted or remain connected. I'd opt for the second option.
3. This implies however, that when a line item is reconnected, and a subject is linked to that item again, the comment should also be connected again. Otherwise the diagram and model will be out of sync.

The current behaviour is that a comment remains connected to a line item (via a comment-line), however, no relation is made between an already existing comment and the newly created relationship.

We might also argue that given this behaviour, rule 1 is invalid and a comment should be able to connect to any item, regardless of whether it has a subject attached or not.

[amolenaar]

While doing some editing on the meta model, I found that I want to add a link from a comment to an element (in this case generalization) that has no associated model element.

So I think it should be possible to add a comment line to any element, and only link the comment to the model element once that's created. I think that's the solution I'm going for. It's most flexible and works as expected for newcomers.

[amolenaar]

Fixed by #280