jooby icon indicating copy to clipboard operation
jooby copied to clipboard

Same module Generated Classes not being imported in generated MVC resources

Open tpoll opened this issue 1 year ago • 17 comments

We use another maven annotation processor to create immutable data classes (https://immutables.github.io/). When we define these immutable classes inside the same maven module as Jooby, it appears that the MVC processor doesn't generated an import for the jooby generated module class. At runtime, this gives us a NoClassDefFoundError.

Interestingly, if our generated data classes are in a different module, Jooby is properly able to create import statements.

For example public SourceConnectionTestResult testSource(SourceConnectionTestRequest testRequest) a resource defined like this (both the argument and return are immutables generated classes) ends up generating a module

that looks like

var2 = app.post("/source/test", (ctx) -> {
      SourceResource var2 = (SourceResource)provider.get();
      return var2.testSource((SourceConnectionTestRequest)ctx.body(SourceConnectionTestRequest.class));
    });
    var2.setReturnType(SourceConnectionTestResult.class);

While this code is correct, the class doesn't have any import statements for SourceConnectionTestResult or SourceConnectionTestRequest.

I played around with different package configurations, and that doesn't seem to change the error.

My naive guess here is that there's a race/ordering issue with both of these annotation processors running in the same maven module.

tpoll avatar Sep 08 '22 19:09 tpoll

can you share the annotation processors setup?

jknack avatar Sep 08 '22 19:09 jknack

sure,

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.10.1</version>
        <configuration>
          <annotationProcessorPaths>
            <path>
              <groupId>org.immutables</groupId>
              <artifactId>value</artifactId>
              <version>2.9.0</version>
            </path>
            <path>
              <groupId>io.jooby</groupId>
              <artifactId>jooby-apt</artifactId>
              <version>${jooby.version}</version>
            </path>
          </annotationProcessorPaths>
        </configuration>
      </plugin>

is in a parent pom shared on all our modules.

Fwiw we are on jooby 2.14.2.

tpoll avatar Sep 08 '22 20:09 tpoll

@tpoll I use annotation processors in same maven module with Jooby no problem but I have not tried immutables. However I do not use the annotationProcessorPath. Try without using the annotationProcessorPath (sometimes intellij will not like that. Eclipse it works fine if you have the right m2e annotation plugin installed). That is just add the apt jars to the regular classpath with <optional>true</optional> and <scope>provided</scope>

Jooby does its work in the last round (annotation processing) so if immutables does its work in the last round that would be a problem.

However bytecode does not do imports even though I know what you mean. The reason I say this is use bytecode disassembler (bytecodeviewer and Eclipse have it builtin) and not Fern Flower or I presume whatever Intellij has builtin that generates source code.

Look for something like:

checkcast com/mypackage/SourceConnectionTestResult

Just to confirm its actually missing and is using the default package.

agentgt avatar Sep 09 '22 16:09 agentgt

Jooby does its work in the last round (annotation processing) so if immutables does its work in the last round that would be a problem.

@agentgt can you explain why?

jknack avatar Sep 09 '22 16:09 jknack

Because if they (immutables) make the classes in the last round it is a race condition depending on where you put the generated file.

Actually what Jooby does is technically wrong doing things in the last round and then putting it in target/classes (I have forgotten what the APT calls that output path) but Jooby has no choice since it generates bytecode classes and not Java source code.

And you can clearly see the compiler omits a warning:

[WARNING] File for type 'com.blah.SomeController$Module' created in the last round will not be subject to annotation processing.

(which btw is one annoying aspect about jooby producing bytecode is that most if not all of my builds warnings come from the above. I have tried to omit the warning but I don't think its possible w/o removing all apt warnings)

If it's not subjected to annotation processing the TypeMirrors/TypeElements will not be accessible.

I'm sure you must have seen the above at some point?

Anyway if Immutables does anything similar to what Jooby does either Jooby or the other will not see the generated classes.

Still I can't figure out how @tpoll got only the class Simple Name but not the FQN. Also immutables generates source code and not bytecode I think.

Basically I think the real problem is somehow the Immutables generated classes are not making it in the jar (or executable classpath). @tpoll should check that instead of disassembling the bytecode as that can give misleading results (still waiting for confirmation they actually see no FQN reference).

agentgt avatar Sep 09 '22 16:09 agentgt

I tested Immutables like:

@Immutable
@JsonSerialize(as = ImmutableFeatureFlags.class)
@JsonDeserialize(as = ImmutableFeatureFlags.class)
public interface FeatureFlags {
	
	public boolean fooBar();

}

Controller

	@GET("/feature")
	public ModelAndView featureflags(ImmutableFeatureFlags flags) {
	
	}

And checked the bytecode:

             checkcast com/mycompany/ImmutableFeatureFlags

BTW @tpoll you should use the interfaces (the classes you put the annotations on) anyway which are not code generated and therefore should have zero problems:

	@GET("/feature")
	public ModelAndView featureflags(FeatureFlags flags) {
	
	}

agentgt avatar Sep 09 '22 17:09 agentgt

Hmm, I wasn't able to run the bytecode viewer locally (maybe some m1mac issues), but I tried some online hosted decompilers and couldn't see the casts or other qualified names.

Removing the annotation processor paths also didn't seem to solve this problem.

We specifically don't want to use the interfaces for immutables and instead prefer to only use the generated class (we have some specific styles to have a different naming conventions).

We can work around this fairly easily by just moving our generated classes into another module for now, so it's not urgently blocking us.

If I get some free time this weekend I'll try and push up an example minimal repo that shows this problem.

Thanks for all the help and pointers @agentgt

tpoll avatar Sep 09 '22 17:09 tpoll

Hmm, I wasn't able to run the bytecode viewer locally (maybe some m1mac issues), but I tried some online hosted decompilers and couldn't see the casts or other qualified names.

You tried this one? https://github.com/Konloch/bytecode-viewer

I have an m1mac and used it (it should not matter because its pure java but I did have to open it twice). java -jar bytecodeviewer.jar

but I tried some online hosted decompilers and couldn't see the casts or other qualified names.

They are not going to show it correctly. Those are disassemblers not viewers. I know because I replicated the exact same behavior and saw the missing import FQN missing from Fern Flower (disassembler).

What you want is a bytecode viewer.

Ignoring the bytecode viewer did you check the jar to see if the generated immutables classes are there?

agentgt avatar Sep 09 '22 18:09 agentgt

Yeah, that byte code viewer is giving me a cryptic error when trying to run the jar.

The generated classes are in the JAR, and plent of other apps are using them successfully at runtime.

tpoll avatar Sep 09 '22 18:09 tpoll

Darn I was hoping for an easy solution :)

Anyway I generally think its best you compile time separate domain like classes (e.g. value, entity, aggregates etc) from the HTTP layer assuming its that kind of data class. Otherwise you might get Jooby's Context object mixed in with your core domain (I have seen it happen with Servlet stuff back in the day).

agentgt avatar Sep 09 '22 18:09 agentgt

@tpoll are we good? or is there an issue?

@agentgt do you know how lombok does it?

Actually what Jooby does is technically wrong doing things in the last round and then putting it in target/classes (I have forgotten what the APT calls that output path) but Jooby has no choice since it generates bytecode classes and not Java source code.

jknack avatar Sep 12 '22 20:09 jknack

I'm good in the sense that there are very easy workarounds here (just moving the generated classes to another module), but I wasn't able to solve the original issue. I haven't had time yet to push up a minimal reproduction example.

I'm fine if you want to close this issue, and I can re-open later when I have more time to dig into what's going on.

tpoll avatar Sep 13 '22 13:09 tpoll

I would like to see the example, due I can't reproduce it yet.

jknack avatar Sep 13 '22 13:09 jknack

@jknack Don't worry about my earlier comment on the "WARNING". The only solution would be to place the generated classes for Maven projects in src/target/generated-sources/annotations but that generally breaks Eclipse annotatinon processing as well as issues with Gradle (otherwise I would have already filed a bug).

See the annotation processor expects you to make Java source files and not class files. Thus the only solution I could find would be to generate Java files. I would prefer that because my projects are in Java but IIRC I mentioned to you a long time ago how generating byte code directly has advantages such as kotlin compatibility (Micronaut does this and I was told that was one of the reasons as well as avoiding the messiness of source code generators which are not as typesafe as I guess ASM).

As for how lombok does things it is because it is not really an annotation processor. Its a compiler plugin (and it controversially uses unsupported APIs so it might not work in future java versions). The short answer is it modifies class files and does not produce them (well for the most part). However it too has order issues as well with annotation processors.

agentgt avatar Sep 13 '22 14:09 agentgt

I don't like the warning. I did removed by generating the class files as soon as the processor run and optionally writing the META-INF/services file at the last round. Seems to works OK. Do you see an issue here?

Also, don;t think APT only must generated source code. There is a Filer.createClassFile method

jknack avatar Sep 13 '22 14:09 jknack

I don't like the warning. I did removed by generating the class files as soon as the processor run and optionally writing the META-INF/services file at the last round. Seems to works OK. Do you see an issue here?

Yes but my memory is foggy what happened. I believe it is something to do with Eclipse annotation processor. Anyway I tried to fix it at one point as Micronaut doesn't seem to generate the error but it broke something on our end.

Anyway check in the change (either 3.0x or 2.0x) and I will retry to make sure its just us. I'm fairly sure its some issue with processor order. Besides I think I'm one of the last developers still using Eclipse these days...

Also, don;t think APT only must generated source code. There is a Filer.createClassFile method

Yeah I probably should have said that is the normal flow and thus least amount of odd behavior. There are tons of bugs in the annotation processor with both the Eclipse one and the real one (notable is TYPE_USE annotations for both processors). Eclipse has some issues with Filer.createClassFile but I might be mixing that up with its resource issues (if you create resources you can't use the Filer IIRC).

agentgt avatar Sep 13 '22 16:09 agentgt

@agentgt pushed the annotation processor changes on 3.x. Please give it a try and let me know how it goes.

jknack avatar Sep 14 '22 23:09 jknack