Skip to content

Provide a transparent Flow bridge #394

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

Closed
nicolaferraro opened this issue Oct 2, 2017 · 11 comments
Closed

Provide a transparent Flow bridge #394

nicolaferraro opened this issue Oct 2, 2017 · 11 comments

Comments

@nicolaferraro
Copy link

I've done some tests using the Java 9 multi-release JAR feature and created this project. It produces a JAR with a copy of the 4 interfaces but, as you can see from the tests, those interfaces implement the Flow.xxx equivalent when using the jar with a Java 9 JVM, while they just implement org.reactivestreams.xxx when running on Java 8. A similar approach can be used also for Java 6 if needed.

I'm planning to add such interfaces to Apache Camel, but I know there are many other libraries that want to do something similar, in order to be compatible with any version of Java.

Have you considered adding something similar directly in this repo, maybe in the flow-bridge project?
I can help with the implementation.

@akarnokd
Copy link
Contributor

akarnokd commented Oct 2, 2017

Can you do it with Gradle?

Also there is an asymmetry problem here: if RS implements juc.Flow, that means that juc.Flow code that expects juc.Flow.Publisher can consume RS Publishers but RS code that expects RS Publisher can't talk to Flow.Publisher directly.

@akarnokd
Copy link
Contributor

akarnokd commented Oct 2, 2017

public class FlowVsRS {

    interface FlowPublisher {
        
    }
    
    interface RSPublisher extends FlowPublisher {
        
    }
    
    public static void flowAPI(FlowPublisher src) {
        
    }
    
    public static void rsAPI(RSPublisher src) {
        
    }
    
    public static void main(String[] args) {
        flowAPI(new FlowPublisher() { });
        flowAPI(new RSPublisher() { });
        rsAPI(new RSPublisher() { });
        rsAPI(new FlowPublisher() { }); // <-------- compilation error, incompatible types
    }
}

@nicolaferraro
Copy link
Author

No problems in doing it with Gradle if it is needed.

Yes, the trick helps when say I need to provide a API that returns e.g. a Publisher, not when the API consumes a Publisher as you've shown.

public class FlowVsRS2 {

    interface FlowPublisher {
        
    }
    
    interface RSPublisher extends FlowPublisher {
        
    }
    
    public static RSPublisher uniqueAPI() {
        return new RSPublisher() {};
    }
    
    public static void main(String[] args) {
        RSPublisher rs = uniqueApi();
        FlowPublisher flow = uniqueApi();
        // flow accepts a FlowSubscriber
    }
}

Having a RSPublisher that is also a Flow.Publisher allows you to use the produced publisher as a flow without conversion. You may need to use explicit conversion in the case you've highlighted. You can define e.g. only the RSPublisher interface and, if you need to pass a FlowPublisher, you can convert it using the bridge library.

Also, creating a TransitionalRSPublisher instead of changing the already defined org.reactivestreams.Publisher may help.
In the poc I've used interface default methods to make sure the library user needs to provide e.g. the subscribe method only once. That's why it requires Java 8. To make it compatible with Java 6+, probably the TransitionalRSPublisher must be a abstract class.

So, it does not cover all cases, but it seems useful in many cases.

@akarnokd
Copy link
Contributor

akarnokd commented Oct 3, 2017

Yes, the second difficulty is having two subscribe overloads. As long as distinct Subscribers are used, they work but as soon as one has a mixed Flow+RS Subscriber, you have to manually cast it to either interface type.

Therefore, I'm not convinced combining Flow and RS interfaces is worth it. I find the current route of using the bridge or natively implementing Flow API instead of RS more approachable. If we had a structural type system, such as TypeScript, we wouldn't even need a bridge.

However, it's up to the project leads to decide.

@viktorklang
Copy link
Contributor

I'd think having explicit conversions is much better if the "implicit" coercions aren't bijective.

@zyxist
Copy link

zyxist commented Oct 11, 2017

@viktorklang -> I don't think bijection would be a problem here, if reactive-streams are a multi-release JAR. Here's an example:

  1. library rx-foo targets JDK8 and implements reactive-streams-1.0.1 (w/o multi-release JAR).
  2. library rx-bar targets JDK8 and implements reactive-streams-1.1 (multi-release JAR).
  3. library rx-joe targets JDK9 and implements Flow directly.

Use case 1: you have a JDK8 project:

  • you can use rx-foo and rx-bar, everything works, and your base interfaces are from reactive-streams.
  • Java 8 sees the "legacy" variant of reactive-streams.
  • you can't use rx-joe for obvious reasons.

Use case 2: you have a JDK9 project:

  • you can use all three projects together.
  • the build system resolves the version conflict, and chooses reactive-streams-1.1 as a base.
  • Java 9 notices that reactive-streams-1.1 is a multi-release JAR and chooses the variants that simply extend Flow interfaces.
  • both rx-foo and rx-bar now extend Flow through reactive-streams-1.1,
  • rx-joe uses Flow directly.

Notice that in this scenario there is no class that implements e.g. Publisher, but not Flow.Publisher. The bijection is not necessary, because if you just run it on JDK9, all Publishers from all libraries automatically become Flow.Publishers. And if you are on JDK8, you don't need it either, because you have no Flow :).

The only issue is that no build system currently supports multi-release JAR-s (in other words: you have to assemble it manually). I'm working on a Gradle plugin for Jigsaw, and I'm thinking about adding such a support, but the work hasn't started yet.

@ktoso
Copy link
Contributor

ktoso commented Oct 12, 2017

That's not how multi release jars work or are intended to be used. It would not, and will not work, because mr-jars are strictly a runtime thing, and can not allow something to compile or not compile. Compilation always happens against the "main" class; thus it is not possible to make code that "oh yeah, compiles and works only on jdk9", e.g. such assignment: Flow.Publisher<T> p = thingy, where thingy's class is mr-jar-ed and "oh yeah, under jdk9 it would actually be a Flow.Publisher!".

It's not about "no build tool supports it", it is "this is not the intended use case and will not work", not in javac, not in any other build system. Please read my write-up about what mr-jars can, and cannot, do http://kto.so/2017/09/30/the-practical-truth-about-multi-release-jars/

I'm also going to publish an reactive-streams specific post / guide very soon, so that'll explain the various ways that don't work in reality.

@zyxist
Copy link

zyxist commented Oct 12, 2017

@ktoso -> I've already found your blog post. Good explanation.

@ktoso
Copy link
Contributor

ktoso commented Oct 12, 2017

I've gotten in touch with Alan from Oracle to discuss this deeper,; I'll report back as my findings may have been incorrect - due to testing on an exploded jar on classpath instead proper jar. I'll let you know and update the blog post.

@nicolaferraro
Copy link
Author

Yeah @ktoso, when compiling a Java 9 project, you can assign a my.custom.Publisher object (bundled in a multi-jar) to a Flow.Publisher variable. I've added a test that does exactly this.

IntelliJ is giving me errors, but the test is run correctly by surefire.

@viktorklang
Copy link
Contributor

There's now a converter lib in master, we're discussing naming over here: #401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants