-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for ignoring certain fields in saved request body #5
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions in the comments.
consumeRequestsInOrder :: FindResponse | ||
consumeRequestsInOrder cassetteIORef savedRequest = do | ||
cassette@Cassette { apiCalls, ignoredHeaders } <- readIORef cassetteIORef | ||
cassette@Cassette { apiCalls, ignoredHeaders, ignoredBodyFields } <- readIORef cassetteIORef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about findAnyRequest
- do we want to support ignoredBodyFields
there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do, although I can see now that the code is a little bit confusing:
we actually modify the savedRequest
during the build phase, in buildRequest
, so no matter what matching method we use, it will run on the modified request.
Btw. @kozak the name savedRequest
is very misleading (to me, at least) -- my intuition is that the saved request is the one that we have in the cassette file, while here it's more of currentRequest
or something like that. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok makes sense. I agree that the names are confusing - currentRequest
might be better :)
src/Network/VCR/Types.hs
Outdated
modifyBody :: [Text] -> SavedRequest -> SavedRequest | ||
modifyBody ignoredBodyFields (SavedRequest mn hs url ps body) = SavedRequest mn hs url ps body' | ||
where ignoredBodyFieldsMap = HashMap.fromList $ zip ignoredBodyFields (repeat Null) | ||
bodyMap :: HashMap Text Value = HashMap.difference (fromJust . decode $ L.fromStrict body) ignoredBodyFieldsMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do we want to do if the body doesn't have the expected shape? Should probably be a better error than fromJust
:)
So this is perhaps not the most elegant way of doing that (i.e. the bodies are decoded to a HashMap, the ignored fields are removed and they are decoded back to a bytestring) but it's one that I could do in finite time 😏
A thing to consider for the future is the ability to ignore nested fields. Also, this PR uses
fromJust
in a few places, I'll think of an elegant way of replacing it with something safer.