Closed toddb closed 10 months ago
Hi @toddb , that looks great! Just one small request: would it be possible to add (one or more) test(s) for this (if they make sense to you)? Thanks in advance.
@KevinDockx ha ha, I thought that I had warded that off given the base line that I couldn't find tests on the other "injector". Here's a test that the injector reads the response body and can take either an empty or non-empty stream.
It's not a particularly defensive approach and can be happy to throw more exceptions but as I see it the http context implementation does load up the response body.
Note: most tests die if the default pipeline plumbing doesn't work. I have tested the injector rather than plumbing in the pipeline to "show" that it is going to eTag based on the content.
What is interestingly out of scope are other implementations of the injector that make the design look sensible rather than bloated. Cheers
Writing tests for the other "injector"-services is something that's been on my list for a while - someday I'll get there ;-) Thanks for this!
Thanks for the merge and looking forward to it arriving as a nuget package :-)
Based on #130, this implementation is to refactor the code and make a hard coded path now able to be swappable via service injection:
Overall constraints:
Note on bloat:
Again, all of this allows me then have a different eTag generation strategy outside the context of the request.
@KevinDockx I hope that gives enough context to see if this is in the right direction. Obviously, happy to makes changes as suits. Cheers