json resource runtime error: dictionary changed size during iteration about pyecore HOT 15 CLOSED

Hi @TerryKingston

Thanks a lot for the issue and for your investigation on this! From what I see in your fix, it seems that I made a rookie mistake. Your fix sounds good to me, I would perhaps adjust it like this (not tested, so it may not work properly):

for inst, refs in dict(self._load_href).items():
    self.process_inst(inst, refs)

In order to avoid the refs = self._load_href[inst].

Could you post your test case so I can reproduce the bug? I will investigate in depth tomorrow (I will also change a little bit the naming, which is not very clear, my bad, I made this special resource quite quickly).

In the meantime, feel free to propose a pull request if you have the time ;) it will be my pleasure to merge it.

from pyecore.

@TerryKingston So, I checked and try to reproduce the issue, but I cannot find a test case. I guess that there must be a case with containment/non-containement references?
Otherwise, I think that the fix your proposed is great, but I would like to replicate first the issue so I can be sure that there is no corner case ;) (and add it to the test case set).

from pyecore.

@aranega

Sorry for the late response,

your suggested fix works, I encountered the bug when importing a resource with at least 3 levels of nesting. Not 100% sure if that is what causes it to go down the bugged path, but I'll put together a test case shortly.

from pyecore.

@TerryKingston Thanks for the intel, I will check this case asap on my side! Indeed, containment level could be an issue from what I see in the algorithm. Thanks again!

from pyecore.

@aranega

I successfully reproduced the error with a test case in a fork, and created a pull request #32 . (I apologize if I didn't submit correctly, I'm somewhat new to this whole process)

Thanks,

from pyecore.

@TerryKingston Thanks a lot for the PR and the test case, indeed, I can reproduce the issue. Your PR is just fine, but could you just propose it against the develop branch instead of master? (I will also release some other fixes)

I look in depth with your test case, but I'm not sure how you produced the input .json. It seems that there is a kind of "desync" between the .ecore and the .json model and part of the issue seems to come from this: in the .ecore, the children relation is not set as containment, consequently, the json structure that represents each inner element could not be serialized as it this (the object in the children should not be contained like this) . Also, the fact that the children and parent relations are opposite implies that the parent relation should not appear in the .json (I can easily introduce a workaround for this though).

Currently, if I change the .ecore in order to introduce containment="True" for the children relationship and I patch the json deserializer (and fix an other issue in the Resource your test highlighted), I can sucessfully open the g1.json model, even without the fix we talked about in the previous messages.

from pyecore.

@aranega

Thanks for looking into this issue. I suspected there might be issues with including the parent relationship in contained elements. yes, having both the parent and child relationships expressed in the JSON is redundant. This output is from an app called starUML, which is technically not supported and does not claim to export in ecore. However, because of the work you've done in creating a JSON deserializer, It looked like all I would have to do is add the 'eClass' properties to the JSON output and define a corresponding metamodel.

Because I'm adding attributes in a "pre-process" routine, I could easily remove the redundant parent attributes. But, if having them is technically still valid Ecore JSON I would suggest the updates be made to account for them, as you've mentioned. If not, the tool should not have to deal with them, (it should be able to expect well formed input, IMO). Please let me know which solution you prefer and I'll account for it in my input JSON.

from pyecore.

@TerryKingston Thanks a lot for all the details! Ok, I see, I know starUML2, but I never looked to its json export. I think I can manage the redundant attribute, the json export I try to mimic is the following, which does write redundant attribute, but this is not an issue. Being able to understand the "two" formats is always better for people producing json from other inputs/sources. I hope I will be able to find a solution that does not impact performances.

The only "problem" I see in the input json is the fact that the children relationship is not defined as containment in the .ecore. Regarding the json structure, this attribute is mandatory in the metamodel and should be modified on your side.

What I suggest is the following. On my side:

• I fix the redundant attribute issue
• I fix some other bug your test case highlighted (almost done)

And in your side, of course, if that's ok with you:

• modify the .ecore to add the containment=true attribute to the children relationship
• propose a new PR targeting the develop branch with the updated .ecore (as you already did, and still if you have time to do this)

With all of this, I will propose a new version shortly to deal with everything and with some other fixes :)

from pyecore.

@aranega

The missing containment="true" attribute is definitely a bug on my part. I've added it to the .ecore and switched the PR to develop branch. (Again, apologies if I still haven't made the PR correctly).

Thanks!

from pyecore.

@TerryKingston Sorry for the delay... Thanks for the PR, everything was great. I'm going to work on to turn the tests green again and I will release a new version asap!

from pyecore.

The new version is here, let me know if it works for you!

from pyecore.

@aranega

I got the new version and it seems to fix the issue, but I'm encountering the RuntimeError: dictionary changed size during iteration error on a different portion of the deserialization. This could very likely be issues with my input or metamodel. I'll try to reproduce this in another test case and get back to you with what I find.

I'll make a more complex test that more closely matches my input, even if it turns out to be a bug with my data I'll submit a PR with the tests for your review.

Thanks for all your help.

from pyecore.

@TerryKingston Ouch, no luck, but in a way, that could be interesting to know if indeed the issue come from the input data or from other bugs in the json resource (by the way, I fixed some code in the json deserializer, but I finally didn't touch the for inst, refs in self._load_href.items(): line in order to be sure that this fix does not hide other potential issues).

I will also try to build models that could break the json deserializer on my side.

from pyecore.

@aranega

It looks like I was missing the containment=true in another place in my ecore file. This fix works, I was just giving it bad metamodel data.

I found my bug before preparing any further test cases, so I don't have anything more to contribute at this time.

Thanks!

from pyecore.

@TerryKingston Thanks a lot for your contribution, it always helps to have some new test cases!
I close the issue for now, feel free to reopen it if you encouter another issue related to this.

from pyecore.