* Response: build reader at construction time
* Remove unwrapping/rewrapping DeadlineStream
* Let into_reader() provide Sync + 'static
* Prebuffer Vec use known capacity
Co-authored-by: Martin Algesten <martin@algesten.se>
Previously, ReadWrite had methods `is_poolable` and `written_bytes`, which
were solely for the use of unittests.
This replaces `written_bytes` and `TestStream` with a `struct Recorder`
that implements `ReadWrite` and allows unittests to access its recorded
bytes via an `Arc<Mutex<Vec<u8>>>`. It eliminates `is_poolable`; it's fine
to pool a Stream of any kind.
The new `Recorder` also has some convenience methods that abstract away
boilerplate code from many of our unittests.
I got rid of `Stream::from_vec` and `Stream::from_vec_poolable` because
they depended on `TestStream`. They've been replaced by `NoopStream` for
the pool.rs tests, and `ReadOnlyStream` for constructing `Response`s from
`&str` and some test cases.
We unwrap the stream exactly once per response, and we know that case
will be uncontended for the same reason `SyncWrapper` works:
`into_reader()` takes `self`, so it must have exclusive ownership.
Uncontended mutexes are extremely cheap. This saves us a dependency
at a trivial performance cost.
This fixes an issue where requests with a deadline would do a syscall
on every read, rather than pulling from the buffer. Fixes a problem
reported in #506 where `resp.into_json()` was unnecessarily slow.
If the user reads exactly the number of bytes in the response, then
drops the Read object, streams will never get returned to the pool since
the user never triggered a read past the end of the LimitedRead.
This fixes that by making PoolReturnRead aware of the level below it, so
it can ask if a stream is "done" without actually doing a read.
Also, a refactorign:
Previously, Response contained an Option<Box<Unit>> because the testing
method `from_str()` would construct a Response with no associated Unit.
However, this increased code complexity with no corresponding test
benefit. Instead, construct a fake Unit in from_str().
Also, instead of taking an `Option<Box<Unit>>`, PoolReturnRead now takes
a URL (to figure out host and port for the PoolKey) and an &Agent where
it will return the stream. This cuts interconnectedness somewhat:
PoolReturnRead doesn't need to know about Unit anymore.
I added `ureq::get("http://example.com")` to a toy program and was very confused when it did nothing.
Change `Request` to give a warning if unsent:
```
warning: unused `ureq::Request` that must be used
--> src/main.rs:48:5
|
48 | ureq::get("http://example.com");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: Requests do nothing until consumed by `call()`
```
3rd party TLS connections are currently required to provide a Sync
guarantee. Since ureq will only ever use the connection on a single
thread, this is not necessary.
The reason we end up with this bound is because we want Response and
Error to be Sync, in which case Rust's automatic inferral of Sync
fails.
This change "masks" the Stream in a wrapper making it Sync.
Close#474
Tests use `Response::as_write_vec` to inspect the outgoing HTTP/1.1
request line and headers. The current version has two problems:
1. Called `as_write_vec` when it actually returns a `&[u8]`.
2. Inspects/uses the `Response::stream` without consuming `Response`.
The first problem is trivial, but the second is subtle. Currently all
calls on `Response` that works with the internal `Response::stream`
consumes `self` (`into_string`, `into_reader`).
`Response` is by itself `Send + Sync`, and must be so because the
nested Stream is `Read + Write + Send + Sync`. However for
implementors of `TLSStream`, it would be nice to relax the `Sync`
requirement.
Assumption: If all fields in Response are `Sync` except
`Response::stream`, but any access to `stream` consumes `Response`, we
can consider the entire `Response` `Sync`.
This assumption can help us relax the `TlsStream` `Sync` requirement
in a later PR.
