Consider using FindBinary instead of TryFindBinary, and review clean-up logic

[rws]

Consider using FindBinary instead of TryFindBinary, and review clean-up logic

[rpannekoek]

Hi Dominic. Can you clarify what you think the Issue is in DXA codebase, exactly?

[DominicCronin]

Hi Rick,

The key is in my use of the word "Consider". I'm pointing out inconsistencies rather than saying something is 100% broken. In the meantime, I've learned that the DD4T develop branch has changes that have yet to impact DXA, so perhaps it can/should wait a bit. So take this as a flag that this area needs an architectural review (probably from both DD4T and DXA) as much as a code review. The responsibility for handing back a cleanly obtained binary, or failing, should probably lie with DD4T, and DXA should be able to trust that. Personally, I'd put the responsibility for file system caching (or caching generally) in DD4T. As it is, DXA has an inconsistency.
Obviously, as the two projects come under common governance, this kind of thing ought to be easier. (It will also be easier if DD4T can get rid of the obsolete streaming approach, giving clearer expectations at the API.)

[rpannekoek]

Fixed in DXA 1.7; refactored BinaryFileManager to retrieve Binaries directly from CD API.