Skip to content

Conversation

@conniey
Copy link
Member

@conniey conniey commented Nov 25, 2019

  • Adds an overload to create an IterableStream using Iterable.
  • This considers the case where users already have a synchronous collection, and when they transform it to a Flux, then back again, may result in an IllegalStateException.
  • Adds tests

See #6515 and reactor/reactor-core#1972

@conniey conniey added Client This issue points to a problem in the data-plane of the library. Azure.Core azure-core labels Nov 25, 2019
@conniey conniey self-assigned this Nov 25, 2019
Copy link
Contributor

@hemanttanwar hemanttanwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conniey conniey merged commit 747500f into Azure:master Nov 25, 2019
@conniey conniey deleted the add-ctor branch November 25, 2019 19:53
* @throws NullPointerException if {@code iterable} is {@code null}.
*/
public IterableStream(Iterable<T> iterable) {
this.iterable = Objects.requireNonNull(iterable, "'iterable' cannot be null.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having flux and iterable fields in this class, if an instance of IterableStream is created using an iterable, can this just be converted to this.flux = Flux.fromIterable(Objects.requireNonNull(iterable, "'iterable' cannot be null."));. Simplifies code in other methods too where you don't have to check if flux is null or iterable is null.

Copy link
Member Author

@conniey conniey Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this will result in that illegal state exception again because we are moving from sync to async world then back again.

It was what the code was doing before this overload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core azure-core Client This issue points to a problem in the data-plane of the library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants