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

Config metadata processor does not recognize builder and inherited builder methods #4390

Closed
tjquinno opened this issue Jun 14, 2022 · 0 comments · Fixed by #4403
Closed

Config metadata processor does not recognize builder and inherited builder methods #4390

tjquinno opened this issue Jun 14, 2022 · 0 comments · Fixed by #4403
Assignees
Labels
2.x Issues for 2.x version branch 3.x Issues for 3.x version branch bug Something isn't working config

Comments

@tjquinno
Copy link
Member

tjquinno commented Jun 14, 2022

Environment Details

  • Helidon Version: 2.x, 3.x
  • Helidon SE or Helidon MP
  • JDK version:
  • OS:
  • Docker version (if applicable):

Problem Description

The config metadata processor fails to recognize the HealthSupport.Builder class as a builder. This is because that class does not declare implements io.helidon.common.Builder but rather extends HelidonRestServiceSupport.Builder<Builder, HealthSupport> which does implement that interface.

Further, adding an explicit implements to work around that problem shows that ConfigMetadataHandler#isBuilderMethod fails to correctly identify inherited builder methods.

Steps to reproduce

cd helidon/health/health
mvn clean install
cat target/classes/META-INF/helidon/config-metadata.json

and you see (in part)

[
  {
    "module": "io.helidon.health",
    "types": [
      {
        "prefix": "health",
        "type": "io.helidon.health.HealthSupport.Builder",
        "inherits": [
          "io.helidon.servicecommon.rest.HelidonRestServiceSupport.Builder"
        ],

Then, explicitly add implements io.helidon.common.Builder<HealthSupport.Builder, HealthSupport> to the HealthSupport.Builder declaration and rebuild. The JSON output contains this:

[
  {
    "module": "io.helidon.health",
    "types": [
      {
        "prefix": "health",
        "type": "io.helidon.health.HealthSupport",
        "inherits": [
          "io.helidon.servicecommon.rest.HelidonRestServiceSupport.Builder"
        ],
        "producers": [
          "io.helidon.health.HealthSupport.Builder#build()",
          "io.helidon.health.HealthSupport#create(io.helidon.config.Config)"
        ],

Because the superclass declares the implements clause (using type parameters) that should be sufficient for the config metadata processor to identify the subclass as a builder; I should not have to add the implements clause redundantly to the subclass.

Note that the output does not include the inherited builder methods. Potential problems (illustrated by HealthSupport.Builder):

  1. ConfigMetadataHandler#processBuilderType restricts its scan using isMine but that seems to ignore inherited methods which, contrary to the comment, have not already been handled. At least they did not seem to be when I stepped through the code.
  2. The comparison of the return type of candidate builder methods to the type of the class being analyzed fails when the builder method is inherited. For example, a superclass builder method returns B (the type parameter) which does not match the type of the concrete inheriting builder class. A workaround would be to add @ConfiguredOption to such methods but that should not be needed.
  3. HealthSupport.Builder extends HelidonRestServiceSupport.Builder which has (among others) the crossOriginConfig method annotated with @ConfiguredOption (to work around item 2 above). But in trying to decide if that method is a builder method the findAnnotation method sees no annotation mirrors in the ExecutableElement for that method. So it does not recognize the method as a builder method and excludes the config information.
@tjquinno tjquinno added bug Something isn't working config 2.x Issues for 2.x version branch 3.x Issues for 3.x version branch labels Jun 14, 2022
@tjquinno tjquinno changed the title Config metadata processor does not recognize builder Config metadata processor does not recognize builder and inherited builder methods Jun 14, 2022
@tomas-langer tomas-langer self-assigned this Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues for 2.x version branch 3.x Issues for 3.x version branch bug Something isn't working config
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants