Fix #700. Add option to ignore broken symlinks in directories

[marwiik]

Here's my suggested solution. I'm open for suggestions if there's a better way to pass along the ignoreBrokenLinks parameter, or any other improvements.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.73%. Comparing base (92fee05) to head (3e32f38).
Report is 144 commits behind head on master.

Files with missing lines	Patch %	Lines
src/main/java/org/vafer/jdeb/DebMaker.java	33.33%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #794      +/-   ##
============================================
+ Coverage     71.03%   71.73%   +0.70%     
- Complexity       96      100       +4     
============================================
  Files             7        7              
  Lines           580      598      +18     
  Branches         75       79       +4     
============================================
+ Hits            412      429      +17     
- Misses          121      122       +1     
  Partials         47       47              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tcurdt]

Thanks for the contribution! And also providing docs and test cases!
Looking OK on a quick glance. We just need to double check if this changes any of the external interfaces.

[marwiik]

Sounds good. Not changing the default behaviour was one of the main considerations for my solution. But to make sure, I could add a second buildData method with the old signature which just calls the new one with the default "false" new parameter. Just in case someone is using the DataBuilder api outside of maven. Is that what you had in mind, or have I overlooked something?

Also, I just realised I forgot to add the new parameter in the doc comment. I'll fix that first.

[tcurdt]

This looks too brittle. The very least we would need some comments on way this should work.

[marwiik]

I agree. It assumes that the format of the exception message is always the same, but I couldn't figure out any other way to find out which file caused the exception.
We could of course skip the isSymbolicLink check, but then it wouldn't apply only to symlinks.

[tcurdt]

AFAIK the only reliable way is to either have a custom exception that wraps the FNF (major fail), or provide the path externally.

I think in this case the only feasible option would be a custom exception that provides the path.
But again we would need to see if this requires changing any signatures. But maybe we just need to catch and re-throw in the producers.

[marwiik]

I pushed a fix with a custom exception, for which we have control of the exception message. Looks a bit cleaner and more robust to me at least.

[tcurdt]

Much better. But I think I would avoid re-using the message variable - and then we should be golden.

[marwiik]

Done

[tcurdt]

Thanks a lot!
Can you rebase it so I can merge it?

[marwiik]

I've never rebased a pull request before, but I think I got it right. The new commits didn't get signed though. :(

[tcurdt]

Awesome! Thanks for the contribution.

[marwiik]

No problem! Thanks for your constructive feedback, I enjoyed it. There's always more to learn.