LionWeb-io / lionweb-repository

Reference implementation of LionWeb repository
Apache License 2.0
2 stars 1 forks source link

Fail to remove reference if stored node contains reference meta-pointer with empty targets #18

Closed enikao closed 3 months ago

enikao commented 8 months ago

Assume the repository contains Disk_A.json.

When we store Disk-move-child-partition.json, we expect node ID-4 to have no references. It actually keeps the reference to ID-10.

joswarmer commented 8 months ago

Hi, I tried this on main branch, but I cannot reproduce the problem.

joswarmer commented 8 months ago

Looked at it with Niko, cannot reproduce it together either, so closing this.

enikao commented 8 months ago

I think this issue still exists.

Use branch lw-java/bulk-api and execute test TestRemoveReference.updateSingleNode()

Expected:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=SerializedReferenceValue{metaPointer=MetaPointer{key='Folder-linked-key', version='2023.1', language='-default-key-FileSystem'}, value=[Entry{resolveInfo='null', reference='ID-10'}, Entry{resolveInfo='null', reference='ID-8'}]},
...

actual:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=SerializedReferenceValue{metaPointer=MetaPointer{key='Folder-linked-key', version='2023.1', language='-default-key-FileSystem'}, value=[]},
...

Somehow opposite in TestChangePropertyValue.updateNode3():

expected:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=SerializedReferenceValue{metaPointer=MetaPointer{key='Folder-linked-key', version='2023.1', language='-default-key-FileSystem'}, value=[]},
...

actual:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=SerializedReferenceValue{metaPointer=MetaPointer{key='Folder-linked-key', version='2023.1', language='-default-key-FileSystem'}, value=[Entry{resolveInfo='null', reference='ID-10'}, Entry{resolveInfo='null', reference='ID-8'}]},
...
joswarmer commented 8 months ago

I cannot run the Java code, building the project fails. What I do see is that the file Disk-remove-reference-partition.json you are using is actually the same as Disk_A.json, the references are still in the file. That is not how it was intended, but the expected result might then be different.
Can you try this again after removing the references from this file ?

enikao commented 8 months ago

I copied the json files from lw-repo/main. Now TestRemoveReference works, but still one issue with TestChangePropertyValue.updateNode3():

expected:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=,
...

actual:

SerializedClassifierInstance{id='ID-4', parentNodeID='ID-3',
...
 references=SerializedReferenceValue{metaPointer=MetaPointer{key='Folder-linked-key', version='2023.1', language='-default-key-FileSystem'}, value=[Entry{resolveInfo='null', reference='ID-10'}, Entry{resolveInfo='null', reference='ID-8'}]},
...
joswarmer commented 8 months ago

This seens to be correct, because the two references are in DIsk_A , and applying Disk_Property_value_changed-single-node.json does not remove them. Theese two references are missing in the Disk_Property_value_changed-partition json.

enikao commented 8 months ago

Did I introduce this issue during translation to Java or is the TS test missing the discrepancy?

joswarmer commented 8 months ago

In general the TS testcases check whether the retrieve is identical to the partition.json in the same folder, so this kind of discrepancy will not be found.
In this specific case the references are in the partition.json, so they have probably been changed after you copied them.

In the @lionweb/typescript repo we use the testcases from https://github.com/LionWeb-io/lionweb-integration-testing . These tests are not copied, but are checked out in a setup script. Although they are not automatically updated, it is easy to do a git clean + project setup to update them. Maybe such a setup would make it easier to keep things synchronized.

joswarmer commented 3 months ago

Closing because cannot reproduce it