Add Validation property to response. #21
Conversation
serialization impl commit removing value.rs interim commit interim commit
mkatychev
force-pushed
the
custom_json_struct
branch
from
b4b2395
to
d8e2f5a
Compare
| Self { | ||
| body: None, | ||
| uri: Value::Null, | ||
| etc: Some(json!({})), |
There was a problem hiding this comment.
Serde does not currently support #[serde(default, flatten)] combination:
thus calling deserialize on a flattened property will always return a Value::Object(<Map, Value>)
| impl<'a> PartialEq for Response<'a> { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.body.eq(&other.body) && self.etc.eq(&other.etc) && self.status.eq(&other.status) | ||
| } | ||
| } |
There was a problem hiding this comment.
self.validator should always be ignored
There was a problem hiding this comment.
could be worth adding a docstring comment to the eq method that notes why we're defining equality to mean what it does (and e.g. why validator doesn't count)
| pub(crate) request: Request, | ||
| pub response: Response, | ||
| pub response: Response<'a>, |
There was a problem hiding this comment.
does the instruction set necessarily have the same expected lifetime as the response?
There was a problem hiding this comment.
As long as both Response.validation holds a BTreeMap<&'a str, Validator>, and InstructionSet.writes holds a HashMap<&'a str, &'a str>, both those properties should live as long as the deserialized Frame object since both those lifetime references start upon deserializing the json file.
filmreel/src/frame.rs
Outdated
| let val = serde_json::to_value(v)?; | ||
| return Ok(Some(val)); | ||
| } | ||
| Ok(None) |
There was a problem hiding this comment.
for fun, since we were just talking about transpose:
self.body // Option<Value>
.map(|v| serde_json::to_value(v)) // Option<Result<Value, E>>
.transpose() // Result<Option<Value>, E>| impl<'a> PartialEq for Response<'a> { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.body.eq(&other.body) && self.etc.eq(&other.etc) && self.status.eq(&other.status) | ||
| } | ||
| } |
There was a problem hiding this comment.
could be worth adding a docstring comment to the eq method that notes why we're defining equality to mean what it does (and e.g. why validator doesn't count)
filmreel/src/response.rs
Outdated
|
|
||
| impl<'a> Eq for Response<'a> {} | ||
|
|
||
| type Validator<'a> = BTreeMap<&'a str, Validation>; |
There was a problem hiding this comment.
Curious as to why you chose a BTreeMap for this rather than a HashMap (possibly worth a comment to explain in-code)
There was a problem hiding this comment.
There needs to be an order of operations for instances like this:
{
"validation": {
"'response'.'body'": {
"strict": false
},
"'response'.'body'.'collection'": {
"strict": true
}
}
}| }; | ||
| let attempts: Option<Attempts> = request | ||
| .get_etc() | ||
| .as_ref() |
There was a problem hiding this comment.
Is the .as_ref() needed?
There was a problem hiding this comment.
pub fn get_etc(&self) -> Option<Value> returns a "naked" Value so I need to create a reference,
otherwise you'd be dealing with a &Option<T> instead of an Option<&T>
Responseintoresponse.rsfileResponse.validationin preparation for comparison modesRequest.bodyanOption<Value>to more accurately reflect empty body requests.