vert-x3/vertx-service-proxy

Major regressions between 3.5 and 3.6

Closed this issue · 15 comments

While upgrading code from 3.5.x to 3.6 (the guide for Java developers), I encountered 2 regressions.

Need for @VertxGen

We now need to also add @VertxGen.

Before:

@ProxyGen
public interface WikiDatabaseService { ... }

After:

@ProxyGen
@VertxGen
public interface WikiDatabaseService { ... }

Otherwise, compilation fails.

Static methods aren't being ignored anymore

Code generation used to ignore static methods, such as the idiomatic create methods we have in services:

  static WikiDatabaseService create(JDBCClient dbClient, HashMap<SqlQuery, String> sqlQueries, Handler<AsyncResult<WikiDatabaseService>> readyHandler) {
    return new WikiDatabaseServiceImpl(dbClient, sqlQueries, readyHandler);
  }

  static WikiDatabaseService createProxy(Vertx vertx, String address) {
    return new WikiDatabaseServiceVertxEBProxy(vertx, address);
  }

We now need to explicitly ignore them otherwise compilation fails:

  @GenIgnore
  static WikiDatabaseService create(JDBCClient dbClient, HashMap<SqlQuery, String> sqlQueries, Handler<AsyncResult<WikiDatabaseService>> readyHandler) {
    return new WikiDatabaseServiceImpl(dbClient, sqlQueries, readyHandler);
  }

  @GenIgnore
  static WikiDatabaseService createProxy(Vertx vertx, String address) {
    return new WikiDatabaseServiceVertxEBProxy(vertx, address);
  }

Second one seems like mine error during porting to plain java codegen. I'm trying to reproduce it without success. Can you share the generated code involved?

Maybe I'm wrong, but seems like the error is thrown by rx codegen (I'm testing with 3.6.0.CR1). Proxy and handler are generated correctly:

SEVERE: Could not generate model for io.vertx.guides.wiki.database.WikiDatabaseService#create(io.vertx.ext.jdbc.JDBCClient,java.util.HashMap<io.vertx.guides.wiki.database.SqlQuery,java.lang.String>,io.vertx.core.Handler<io.vertx.core.AsyncResult<io.vertx.guides.wiki.database.WikiDatabaseService>>): type java.util.HashMap<io.vertx.guides.wiki.database.SqlQuery,java.lang.String> is not legal for use for a parameter in code generation
io.vertx.codegen.GenException: type java.util.HashMap<io.vertx.guides.wiki.database.SqlQuery,java.lang.String> is not legal for use for a parameter in code generation
	at io.vertx.codegen.ClassModel.checkParamType(ClassModel.java:275)
	at io.vertx.codegen.ClassModel.getParams(ClassModel.java:1023)
	at io.vertx.codegen.ClassModel.createMethod(ClassModel.java:871)
	at io.vertx.codegen.ClassModel.lambda$traverseType$14(ClassModel.java:720)

Nov 19, 2018 10:45:13 AM io.vertx.codegen.CodeGenProcessor lambda$process$7
INFO: Generated model io.vertx.guides.wiki.database.WikiDatabaseService: io.vertx.guides.wiki.database.WikiDatabaseServiceVertxProxyHandler
Nov 19, 2018 10:45:13 AM io.vertx.codegen.CodeGenProcessor lambda$process$7
INFO: Generated model io.vertx.guides.wiki.database.WikiDatabaseService: io.vertx.guides.wiki.database.WikiDatabaseServiceVertxEBProxy

Looking inside generated classes, everything seems fine

What shall we do with this one? Is it documentation only?

For me this is breaking, it worked before, and the same code could not be upgraded directly to 3.6.

vietj commented

I will try to fix today and see if that is expected or not to require @VertxGen. Previously it was required but it might be because vertx-codegen and vertx-service-proxy were too tightly coupled.

In 3.6 it has been decoupled, so the fix or not will depend on what makes most sense, but it's not a strong thing. We might also make @VertxGen not required in 3.6 and required in master and make this as a v4 breaking change.

Stay tuned

vietj commented

do you have a ready reproducer to use @jponge ?

vietj commented

I will try build the guide and remove the @VertxGen annotation

vietj commented

so there are two separate issues highlighted in this issue right ?

The best reproducer is here: vert-x3/vertx-guide-for-java-devs@a0617b5#diff-3ded3ce14c17ee0f03d4b71f8b2787b8R38

Yes, there are 2 issues: one for static methods but in my other Vert.x examples this seems to have been fixed, while the other is still present (@ProxyGen used to imply @VertxGen).

vietj commented

I'm hitting the rxjava issue and I think it comes from the fact that proxies have been decoupled from vertx-codegen.

Before it was a weird thing, i.e something annotated with @ProxyGen implies it is also an @VertxGen and proxy-gen relaxes the @VertxGen rules by simply ignoring them.

Now, since we need to have both @ProxyGen and @VertxGen (otherwise the RxJava code won't be generated anymore I assume), @VertxGen does not inherit the @ProxyGen rule and thus indeed declaring a static method with an RX type is illegal.

I think the solution is that WikiDatabaseService should not declare an RxJava method and instead it should declare a proxy factory and this method should be used from the Rx generated method.

I will make a PR to demonstrate.

Can we close it?

Yep