ozlerhakan / poiji

:candy: A library converting XLS and XLSX files to a list of Java objects based on Apache POI
MIT License
466 stars 134 forks source link

PoijiMultiRowException currently unable to gather multiple errors #272

Closed rob-heaney-itg closed 1 year ago

rob-heaney-itg commented 1 year ago

Since v3.2.0/4.0.0 it seems as though the PoijiMultiRowException is never able to add more than one PoijiRowSpecificException (in other words, it can't provide a full report of all errors in a document), because it stops processing after it hits the first issue.

I think it might be because the HSSFUnmarshaller (line 200) is returning a new PoijiMultiRowException as soon as the errors List is not empty, but the problem is that this is inside the for-loop, and not outside-of/after it, so it will never be able to continue the loop to potentially add more errors.

This is different to the P.O.C that I submitted originally as part of issue https://github.com/ozlerhakan/poiji/issues/233 -- Was this change intentional? Seems like a defect to me.

ozlerhakan commented 1 year ago

Hi @rob-heaney-itg ,

I might have thought a different opinion while revising the code. We can make a small change on that.

rob-heaney-itg commented 1 year ago

I might have thought a different opinion while revising the code. We can make a small change on that.

Thanks for replying @ozlerhakan , ok that's fair enough if you did. In that case, perhaps it's something which is best switchable on/off? Maybe some will prefer that it stop on the first error, and others not?

ozlerhakan commented 1 year ago

We can undo the change @rob-heaney-itg

rob-heaney-itg commented 1 year ago

Thanks very much @ozlerhakan 👍

rob-heaney-itg commented 1 year ago

Hi @ozlerhakan , bad news, I think the issue still applies, due to also being present on HSSFUnmarshaller.java#L102 as well 😞

ozlerhakan commented 1 year ago

Oh really, np we can fix it with 4.1.1

rob-heaney-itg commented 1 year ago

Oh really, np we can fix it with 4.1.1

Thank you @ozlerhakan 👍