php-edifact / edifact

Tools to process EDI messages in UN/EDIFACT format
GNU Lesser General Public License v3.0
268 stars 86 forks source link

Interpreter Group Nesting Depth Issue #42

Closed mtdavidson closed 8 years ago

mtdavidson commented 8 years ago

Hi All,

I'm working with a DESADV message conforming to the D96A standard ( https://www.stylusstudio.com/edifact/D96A/DESADV.htm ) and am trying to use the interpreter to parse this. For the most part it seems to be fine but it was complaining about a missing UNT segment which was in the file. After looking into it in more detail it seems that its giving up parsing the file after a certain point.

This seems to occur when trying to parse SG16 ( https://www.stylusstudio.com/edifact/D96A/DESADV.htm#part4.3.1 ) I believe this is due to the Interpreter only supporting grouping up to two levels and on the third level it's not parsing it right. Looking at the code this seems to confirm my theory ( https://github.com/sabas/edifact/blob/master/src/EDI/Interpreter.php#L128).

Is this the correct diagnosis or am I missing something? If I'm right would there be any drawbacks to rewriting this in a recursive way so that it can support an infinite level of depth in grouping?

Many Thanks

Mark

sabas commented 8 years ago

Your analysis is correct. I wrote the Interpreter following the COARRI BAPLIE message which limits to 2 levels deep, I never saw 3 levels before... :-D It is structured in a way that generates an associative array with groupings and segment names as key, and the output must be encoded back to EDI without loss of information (so input should coincide with the output if directly encoded without processing)

mtdavidson commented 8 years ago

@sabas do you think you could provide an example of your COARRI message I'll probably write a unit test for the current Interpreter then attempt to rewrite it to cope with infinite groups.

sabas commented 8 years ago

@mtdavidson I'll send them via the email I found on your profile :-)

mtdavidson commented 8 years ago

Great thanks

mtdavidson commented 8 years ago

I've created a pull request that solves this issue #49. Feedback welcome :)

sabas commented 8 years ago

Cool! Merged. Closing the other PRs and I tag a new version