jooby icon indicating copy to clipboard operation
jooby copied to clipboard

jooby apt: generate source code

Open jknack opened this issue 1 year ago • 29 comments

  • move away from ASM and byte code generation
  • generate source code

ASM does a good job but it is hard to make changes

jknack avatar Jun 17 '23 15:06 jknack

FWIW I use mustache to template java code but change the delimiters to $$.

{{=$$ $$=}}

That seems to keep most syntax highlighting working.

In fact that was one of the many reasons I made jstachio which has a zero dep mode where the code it generates only imports java.base (ie only jdk builtin).

I don’t recommend java poet but some people like it.

I also recommend you just FQN every type reference instead of managing imports.

agentgt avatar Jun 21 '23 14:06 agentgt

I also recommend you just FQN every type reference instead of managing imports.

var can also do some pretty heavy lifting with apt.

SentryMan avatar Jun 26 '23 18:06 SentryMan

should this be a Java annotation processor? Or something else?

jknack avatar Aug 03 '23 19:08 jknack

unless you want to get into some real dodgy stuff, annotation processing is the way to go.

SentryMan avatar Aug 03 '23 19:08 SentryMan

should this be a Java annotation processor? Or something else?

@jknack are you talking about generating source code or analyzing the code?

In terms of analyzing there are three maybe four options but by far the annotation processing route is the best as it is designed for it.

If you are talking about generating code... you could use my static mustache project: https://jstach.io/jstachio/

And choose Zero Dependency mode: https://jstach.io/jstachio/#code_generation_mode

That will keep your APT module small and fast (it still is basically the fastest template engine and without a doubt the lowest footprint if you choose zero dep mode).

There are already folks using it for Open API code generation I believe.

One nice thing Mustache has over Handlebars for code generation is that you can change the delimiter. Often times I do this for Java code generation as braces can get confusing.

Below is just crappy example:

@JStache(template = """
{{=$$ $$=}}

public $$returnType$$ get$$name.capitalize$$() {
  return this.$$name$$;
} 
""")
public record MyJavaFileMode(String returnType, String name) {

  @JStacheLambda // you can put these methods on interfaces like a mixin
  public String capitalize(String input) {
    // capitalize logic I'm too lazy to put in.
  }
}

As the author of handlebars.java I would be curious what you think.

agentgt avatar Aug 03 '23 20:08 agentgt

How "binding" should work?

Today:

{
   mvc(new Controller());
   // DI version:
  mvc(Controller.class);
}

Then we ask ServiceLoader to find the generated controller.

jknack avatar Aug 03 '23 21:08 jknack

Yes you will need to provide... multiple options for that.

Read my doc on what JStachio does:

https://jstach.io/jstachio/#jstachio_modules

and

https://jstach.io/jstachio/io.jstach.jstache/io/jstach/jstache/JStacheCatalog.html

Basically for modular applications you need to generate some java file as a service and tell them to put it in their module-info.java.

agentgt avatar Aug 03 '23 22:08 agentgt

{
    mvc(new ControllerModule());
}

Why this isn't enough?

I don't see the need of using ServiceLocator anymore.

jknack avatar Aug 03 '23 22:08 jknack

I'm not sure I follow

SentryMan avatar Aug 03 '23 22:08 SentryMan

@SentryMan which part?

Said we have Controller and the new apt generates ControllerModule.java (we can call it any other way we want).

So why keeping the ServiceLocator pattern (as we do today) when we have access to the source code.

jknack avatar Aug 03 '23 22:08 jknack

I guess to me it would be cooler if the registration was automatic

SentryMan avatar Aug 03 '23 22:08 SentryMan

No you do not need to but you might want to. I’m sorry I had a brain fart.

The only reason why you might consider it is referencing generated code is some times less desirable particularly if the IDE does not support annotation processor generated code.

For the above it’s only problem with Eclipse + Gradle and NetBeans these days.

agentgt avatar Aug 03 '23 23:08 agentgt

@SentryMan We can't do routes depends on order.

@agentgt It is a good chance to get rid of ServiceLocator here. I think it is the best.

Do any of you have a good name suggestion for the generated controller?

jknack avatar Aug 03 '23 23:08 jknack

@agentgt It is a good chance to get rid of ServiceLocator here. I think it is the best.

My concern is that it would be disruptive. If we do this it needs at bare minimum a minor version change.

Basically you now have to go tell all MVC users to do this special shit.

Cause before it was:

mvc(new MyController(someCollaborators)); // automagic mapping controller to controller module

Now its:

install(new MyControllerModule()); // Im not sure on this to be honest maybe its just mvc()
mvc(new MyController(someCollaborators));

I guess the better question is what is the aversion to the ServiceLoader? (my guess is gradle incremental but you are screwed either way... maybe more so).

agentgt avatar Aug 03 '23 23:08 agentgt

My concern is that it would be disruptive.

Honestly, I don't care. I don't see a huge thing to replace mvnc(Foo) with mvc(new Foo$Route())

If we do this it needs at bare minimum a minor version change.

If all goes well, will be 3.1

I guess the better question is what is the aversion to the ServiceLoader?

Just because we don't need it anymore. It works for byte code generation were we can't reference the class from IDE... that is not the case anymore.

jknack avatar Aug 04 '23 15:08 jknack

Honestly, I don't care. I don't see a huge thing to replace mvnc(Foo) with mvc(new Foo$Route())

I might be being dense on this but for every controller you would have to register two things: the generated code (let us call it "routes" for now) and the annotated controller (let us assume no DI).

The above is really unappealing to me.

If you just register (call mvc or install) on the "routes" aka generated code what happens?

What I think you could do is create a single "routes" installer per package or module (or compile time boundary). That is why I wanted you to take a look at what I did for JStachio: https://jstach.io/jstachio/io.jstach.jstache/io/jstach/jstache/JStacheCatalog.html

It is a package annotation. You put in package-info and it creates mappings of all annotated classes to generated code. Cause let me tell you people would be pissed if the had do something like:

jstachio.registerTemplate(generatedCodeOfTemplate1);
jstachio.registerTemplate(generatedCodeOfTemplate2);
// possibly hundreds more

Its worse for Jooby because you don't just register the generated code but the instance you want to use which is not the case for my problem since models are obviously not singletons.

It basically boils down to the same problem of mapping Class<?> -> generated code. I map annotated model classes to generated template classes.

MapStruct as well does something similar.

What JStachio and MapStruct do is if it can't be found (generated code for given class) via the Service Loader we use reflection by figuring out the class name of the generated code. What I offer over MapStruct is generate Java code (to support module-info and manual registration) as well as old META-INF/services....

@SentryMan can probably provide some details on what Avaje inject does but I imagine it has similar cataloging of class to some generated code.

agentgt avatar Aug 04 '23 15:08 agentgt

I might be being dense on this but for every controller you would have to register two things: the generated code (let us call it "routes" for now) and the annotated controller (let us assume no DI).

why?

I'm saying it is just one line:

{
   mvn(Controller.class);
}

After change:

{
  mvc(new Controller$Route());
}

What JStachio and MapStruct do is if it can't be found (generated code for given class) via the Service Loader we use reflection by figuring out the class name of the generated code. What I offer over MapStruct is generate Java code (to support module-info and manual registration) as well as old META-INF/services....

That is why want to go with pure Java Code. We kill reflection here, no more reflection fallback, no more Service Loader and the most most important is: no need to struggle with module-info.java (which is painful very painful)

jknack avatar Aug 04 '23 15:08 jknack

Yes but what I'm saying is we (my company) do not do:

// I assume folks do this rely on DI
mvc(Controller.class);

We do this:

mvc(new Controller());

My question is do we now need to do this:

mvc(new Controller$Route());
mvc(new Controller());

That is how does the Controller$Route know how to instantiate the actually controller? Now if you are saying we have to do the above I can probably live with it but I will say historically referencing generated code often pisses people off because they get red "squiggles" depending on setup. It is one of the more hated aspects of Dagger.

That being said IDEs are much better now with APT.

agentgt avatar Aug 04 '23 16:08 agentgt

My question is do we now need to do this:

Not at all, just:

mvc(new Controller$Route());

Behind the scene the generated class will deal with how to get an instance of Controller

jknack avatar Aug 04 '23 16:08 jknack

Behind the scene the generated class will deal with how to get an instance of Controller

I assume if one is not using any DI they would use the ServiceRegistry?

I believe that is how it works now I think (well ignoring the whole find the generated code thing the generated code checks the service registry I think).

agentgt avatar Aug 04 '23 16:08 agentgt

I do agree with you BTW that the ServiceLoader or reflection fallback is a pain in the ass. It also breaks incremental build caches. As I think you know if Gradle knows one to one mapping of generated file it is smarter on build (well if you tell it to which youd).

Maven is already developing similar things I think for Maven 4.0.

Anyway I'm slowly being convinced that you are right on this.

agentgt avatar Aug 04 '23 16:08 agentgt

@SentryMan can probably provide some details on what Avaje inject does but I imagine it has similar cataloging of class to some generated code.

Yeah over there typically we use ServiceLoader to load generated registers of the generated classes. For example, in jsonb for a set of POJOs we generate the adapters and a class like this to register them:

@Generated
@MetaData({
  RequestModelJsonAdapter.class,
  ResponseModelJsonAdapter.class,
  //other stuff we generated...
})
public class GeneratedJsonComponent implements Jsonb.GeneratedComponent {

  @Override
  public void register(Jsonb.Builder builder) {
    builder.add(RequestModel.class, RequestModelJsonAdapter::new);
    builder.add(ResponseModel.class, ResponseModelJsonAdapter::new);
    //register other stuff we generated...
  }
}

this generated class is service loaded to auto-register the json handlers. In addition, we read the meta-annotations at compile time too so incremental builds don't break.

SentryMan avatar Aug 04 '23 16:08 SentryMan

I guess worse case scenario is one could have an add on APT module that does the cataloging... oh fuck wait the order matters for the $Route registration.

@SentryMan

Yeah over there typically we use ServiceLoader to load generated registers of the generated classes. For example, in jsonb for a set of POJOs we generate the adapters and a class like this to register them:

At any point you generate a ServiceLoader registration you break gradle incremental isolate.

See https://docs.gradle.org/nightly/userguide/java_plugin.html#sec:incremental_annotation_processing

The fastest category, these look at each annotated element in isolation, creating generated files or validation messages for it. For instance an EntityProcessor could create a <TypeName>Repository for each type annotated with @Entity.

EDIT otherwise you have to use aggregating.

"Aggregating" annotation processors These can aggregate several source files into one or more output files or validation messages. For instance, a ServiceRegistryProcessor could create a single ServiceRegistry with one method for each type annotated with @Service

agentgt avatar Aug 04 '23 16:08 agentgt

I will say javac is super fast and Gradle is damn fast even on aggregating that it doesn't matter that much (I still prefer Maven but Gradle is got some smart stuff).

@jknack I still think I would like some sort of catalog capability like what I offer in JStachio. It doesn't have to use the Service Loader. You just generate a Java class so that when you do

mvc(new ControllerCatalog());
// now I add all my controllers

ControllerCatalog is just like $Route or whatever going to call but it has all of the controllers found in the compile time boundary.

The only issue is order.

I assume the order is now based on the $Route registration and not the actual controller instance.

// before security filters
mvc(new SomeController$Route());
// add security filters
mvc(new AnotherController$Route());

Still a cataloging thing could be used programmatically.

new SomeCatalog().getControllerClasses()
.stream()
.filter(c -> c.getAnnotation(NotSecure.class) != null)
.forEach(this::mvc)
// add security filters
new SomeCatalog().getControllerClasses()
.stream()
.filter(c -> c.getAnnotation(NotSecure.class) == null)
.forEach(this::mvc)

agentgt avatar Aug 04 '23 17:08 agentgt

don't follow what Catalog is?

Then there is no change around order. Everything will works as it does today. Controller/mvc requires order bc of usage of filter.

So, we will prefer source code (instead of byte code). Just replace the mvc class name with the generated class and remove service loader. Nothing else.

jknack avatar Aug 04 '23 17:08 jknack

don't follow what Catalog is?

A catalog just contains all the generated $Route as a list or maybe a Map<Class<?>,Supplier<$RouteImplements>>. It is just a list of all the generated code classes and maybe some way to instantiate them.

Then there is no change around order. Everything will works as it does today. Controller/mvc requires order bc of usage of filter.

I understand. I meant for the catalog case one cannot just tell the catalog (which does not know about what order you want) to register all the $Route it has.

Think of the Catalog as a reflection free DI of Spring Component scanning. E.g. All classes annotated like this. It is basically to ease registration up because regardless for those that do not use DI it is now two steps. You register the route and the thing aka controller that serves the route.

Because of the two steps its more error prone. For example you could setup two registration of routes and they will accidentally instantiate the real controllers in an order that is not expected or worse some sort of circular dependency issue.

You can probably mitigate the above to make it more like one step with something like:

mvc(new SomeController$Route(() -> new SomeController()));

Which may have been what you had in mind all along.

My question is what happens for the case of no route registration:

mvc(new SomeController()); // No route registration. Will this blow up?

Perhaps the signature of mvc should change to prevent that right?

agentgt avatar Aug 04 '23 17:08 agentgt

ASM does a good job but it is hard to make changes

I've seen the examples of this bytecode generation library https://github.com/cojen/Maker and they are much simpler than ASM. So maybe you can do this instead of source code generation.

ogrammer avatar Aug 16 '23 07:08 ogrammer

@ogrammer it looks simple! Still think now the best is to go with source code.

jknack avatar Aug 16 '23 13:08 jknack

I hope future work would keep providing meta data about the routes or controllers. Right now with the MVC API, it's easy to find the controller method by calling Route.getMvcMethod(). And every controller class, path, parameters and return types can be found precisely, even for the Kotlin nullable marks.

I am trying to use these info to generate some calling code in TypeScript. Another approach is using the openapi doc, but the object type could be tricky. Kotlin and TS are good match, but not with JS. For example Koltin Any goes to openapi object, but a better match for object in TS would be Record instead of any.

zzj37 avatar Nov 08 '23 04:11 zzj37