search

amphp / byte-stream

A non-blocking stream abstraction for PHP based on Amp.
https://amphp.org/byte-stream
MIT License
363 stars 31 forks source link

Fix invalid use for ConcurrentIterableIterator #101

Closed ivanpepelko closed 1 year ago

ivanpepelko commented 1 year ago

Also added the test for php engine iterables. The exception was not caught in tests because instance of ConcurrentIterableIterator was always passed to ReadableIterableStream constructor in tests and never hit the branch for other type of iterables.

trowski commented 1 year ago

ConcurrentIteratorIterator is marked internal now. Can you please change the constructor to the following instead and remove the use statement.

    /**
     * @param iterable<mixed, string> $iterable
     */
    public function __construct(iterable $iterable)
    {
        $this->iterator = $iterable instanceof ConcurrentIterator
            ? $iterable
            : Pipeline::fromIterable($iterable)->getIterator();

        $this->onClose = new DeferredFuture;
    }
ivanpepelko commented 1 year ago

I noticed your comment just now... Well, carry on and happy holidays!

trowski commented 1 year ago

Yep, sorry I'm not usually that impatient, but I wanted to work on this and dependent libraries this weekend and this was actually holding things up. ReadableIterableStream is heavily used in dependents.

Thanks and happy holidays to you too!