Skip to content
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

jooby apt: generate source code #2968

Open
jknack opened this issue Jun 17, 2023 · 30 comments
Open

jooby apt: generate source code #2968

jknack opened this issue Jun 17, 2023 · 30 comments
Milestone

Comments

@jknack
Copy link
Member

jknack commented Jun 17, 2023

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

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

@agentgt
Copy link
Contributor

agentgt commented Jun 21, 2023

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.

@SentryMan
Copy link
Contributor

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

var can also do some pretty heavy lifting with apt.

@jknack
Copy link
Member Author

jknack commented Aug 3, 2023

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

@SentryMan
Copy link
Contributor

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

@agentgt
Copy link
Contributor

agentgt commented Aug 3, 2023

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.

@jknack
Copy link
Member Author

jknack commented Aug 3, 2023

How "binding" should work?

Today:

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

Then we ask ServiceLoader to find the generated controller.

@agentgt
Copy link
Contributor

agentgt commented Aug 3, 2023

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.

@jknack
Copy link
Member Author

jknack commented Aug 3, 2023

{
    mvc(new ControllerModule());
}

Why this isn't enough?

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

@SentryMan
Copy link
Contributor

I'm not sure I follow

@jknack
Copy link
Member Author

jknack commented Aug 3, 2023

@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.

@SentryMan
Copy link
Contributor

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

@agentgt
Copy link
Contributor

agentgt commented Aug 3, 2023

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.

@jknack
Copy link
Member Author

jknack commented Aug 3, 2023

@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?

@agentgt
Copy link
Contributor

agentgt commented Aug 3, 2023

@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).

@jknack
Copy link
Member Author

jknack commented Aug 4, 2023

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.

@agentgt
Copy link
Contributor

agentgt commented Aug 4, 2023

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.

@jknack
Copy link
Member Author

jknack commented Aug 4, 2023

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)

@agentgt
Copy link
Contributor

agentgt commented Aug 4, 2023

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.

@jknack
Copy link
Member Author

jknack commented Aug 4, 2023

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

@agentgt
Copy link
Contributor

agentgt commented Aug 4, 2023

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
Copy link
Contributor

agentgt commented Aug 4, 2023

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.

@SentryMan
Copy link
Contributor

SentryMan commented Aug 4, 2023

@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.

@agentgt
Copy link
Contributor

agentgt commented Aug 4, 2023

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 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
Copy link
Contributor

agentgt commented Aug 4, 2023

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)

@jknack
Copy link
Member Author

jknack commented Aug 4, 2023

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.

@agentgt
Copy link
Contributor

agentgt commented Aug 4, 2023

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?

@ogrammer
Copy link

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.

@jknack
Copy link
Member Author

jknack commented Aug 16, 2023

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

@zzj37
Copy link

zzj37 commented Nov 8, 2023

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.

jknack added a commit that referenced this issue Jun 2, 2024
- Sync code generator to new annotation processor ref #2968
jknack added a commit that referenced this issue Jun 2, 2024
- bind existing `mvc` to new annotation processor
- deprecate existing `mvc` methods
- ref #2968
@jknack
Copy link
Member Author

jknack commented Jun 2, 2024

4d5ea35

@jknack jknack added apt and removed research labels Jun 2, 2024
@jknack jknack added this to the 3.2.0 milestone Jun 2, 2024
jknack added a commit that referenced this issue Jun 3, 2024
 - fix tests add missing edge cases
jknack added a commit that referenced this issue Jun 3, 2024
jknack added a commit that referenced this issue Jun 3, 2024
jknack added a commit that referenced this issue Jun 3, 2024
- move classes to proper package: `apt`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants