javalin / javalin-openapi

Annotation processor for compile-time OpenAPI & JsonSchema, with out-of-the-box support for Javalin 5.x, Swagger & ReDoc
https://github.com/javalin/javalin-openapi/wiki
Apache License 2.0
44 stars 17 forks source link

Still array fields in the custom annotations don't appear in the output #125

Closed IvanPizhenko closed 1 year ago

IvanPizhenko commented 1 year ago

Full story is here: https://github.com/javalin/javalin-openapi/issues/93 I can't reopen that issue, so creating this one to keep track of it.

IvanPizhenko commented 1 year ago

Maybe you can add some debug output to the annotation processor, that can be optionally enabled, so we will see what's going on and why it loses array field in my case?

dzikoysk commented 1 year ago

Does it happen only when there's a fallback to default value or it happens also when you'll directly provide new values to the array?

IvanPizhenko commented 1 year ago

I observe it when I directly provide values, but I have default value empty array, which should not appear. I did not try non-empty default value because I don't need that in my code.

dzikoysk commented 1 year ago

Annotation processor should be configurable if I'll extend #127 with a decent API, so for sure we can try to add some kind of a debug mode on request.

IvanPizhenko commented 1 year ago

hmm... and why exactly groovy, why not kotlin script like in gradle?

dzikoysk commented 1 year ago

Groovy is quite compact, it provides all features bundled as library and just works at this point. I don't know much about scripting capabilities of Kotlin, it might be bootstraped in build system like Gradle, but I'm not sure how could I use it as library-only. Another thing is that as long as Kotlin devs would prefer Kotlin, Java devs are more familiar with Groovy anyway.

If you'd like to work on Kotlin support, feel free to submit issue & PR, it'd be quite nice enhancement. Both those scripts should return OpenApiAnnotationProcessorConfigurer interface implementation, so it doesn't really matter from the API perspective which language we use. In the end it's still workaround for limited capabilities of annotation processor API and I never seen such approach, so it's literally experimental adjustment.

dzikoysk commented 1 year ago

I've quickly checked kotlin-script-jsr223 and some related approaches, but it won't work out-of-the-box in the annotation processor environment. It probably requires some kind of a custom layer to properly link classloaders & classpath with compiler & runtime as it doesn't work the same way in annotation processor as in regular Java app. Also, Groovy has way better support in IntelliJ than Kotlin Script, it wouldn't work as good as gradle.kts files.

dzikoysk commented 1 year ago

This is what I get in debug logging using debug mode defined here:

For this code:

https://github.com/javalin/javalin-openapi/blob/4716f14671826c80750eb432f6af69a149cbacf7/openapi-annotation-processor/src/test/kotlin/io/javalin/openapi/processor/UserCasesTest.kt#L12-L45

Logging:

Note: TypeSchemaGenerator#findExtra | Custom annotation: @io.javalin.openapi.processor.UserCasesTest.Schema(allowableValues={"valid", "expired", "revoked"}, format="fingerprint", pattern="^(valid|expired|revoked)$", description="status of the key like valid|expired|revoked")
Note: TypeSchemaGenerator#findExtra | Element values with defaults: {allowableValues()={"valid", "expired", "revoked"}, format()="fingerprint", pattern()="^(valid|expired|revoked)$", description()="status of the key like valid|expired|revoked", example()=""}
Note: TypeSchemaGenerator#findExtra | Visited entry (allowableValues(), {"valid", "expired", "revoked"}) mapped to ["valid","expired","revoked"]
Note: TypeSchemaGenerator#findExtra | Visited entry (format(), "fingerprint") mapped to fingerprint
Note: TypeSchemaGenerator#findExtra | Visited entry (pattern(), "^(valid|expired|revoked)$") mapped to ^(valid|expired|revoked)$
Note: TypeSchemaGenerator#findExtra | Visited entry (description(), "status of the key like valid|expired|revoked") mapped to status of the key like valid|expired|revoked
Note: TypeSchemaGenerator#findExtra | Visited entry (example(), "") mapped to 

Result:

{
  "openapi": "3.0.3",
  "info": {
    "title": "",
    "version": ""
  },
  "paths": {
    "/gh-125": {
      "get": {
        "tags": [],
        "parameters": [],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/KeypairCreateResponse"
                }
              }
            }
          }
        },
        "deprecated": false,
        "security": []
      }
    }
  },
  "components": {
    "schemas": {
      "KeypairCreateResponse": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "fingerprint": {
            "type": "string",
            "example": "",
            "allowableValues": [
              "valid",
              "expired",
              "revoked"
            ],
            "format": "fingerprint",
            "pattern": "^(valid|expired|revoked)$",
            "description": "status of the key like valid|expired|revoked"
          }
        },
        "required": [
          "fingerprint"
        ]
      }
    }
  }
}

Could you try to use latest snapshot to check what's the output? Also, if you're using Gradle, you should be able to debug annotation processor as regular lib. Just add annotation processor library as implementation(), so IntelliJ will be able to link sources with Gradle debug execution process.

dzikoysk commented 1 year ago

Quick update: Imports have been slightly changed in experimental scripting API:

https://github.com/javalin/javalin-openapi/blob/9ca6b297d8fba7cd0ba71380b8a0e6a26c4482c6/openapi-annotation-processor/src/test/compile/openapi.groovy#L1-L18

IvanPizhenko commented 1 year ago

So I have the following class:

data class KeyHistoryListItem(
  @get:Schema(
    allowableValues = ["add", "mod", "del"],
    description = "type of revision - add, mod (modify) or del (delete)"
  )
  val revisionType: String,

  @get:Schema(description = "row id of the particular history record")
  val id: Long,

  val keypair: KeyListItem
)

I get following debug output from the annotation processor regarding it:

Note: TypeSchemaGenerator#findAllProperties | Enclosed elements of com.mypackage.KeyHistoryListItem: revisionType,id,keypair,copy(java.lang.String,long,com.mypackage.KeyListItem),equals(java.lang.Object),hashCode(),toString(),KeyHistoryListItem(java.lang.String,long,com.mypackage.KeyListItem),component1(),getRevisionType(),component2(),getId(),component3(),getKeypair()

Nothing else. No log entries from "TypeSchemaGenerator#findExtra".

dzikoysk commented 1 year ago

It sounds pretty strange, because it'd indicate those @get:Schema annotations are not processed at all... but you said you have some of those properties in the scheme 🤔 Sounds like caching issue, I guess you're using clean task, but could you also disable incremental compilation in Kapt: https://kotlinlang.org/docs/kapt.html#incremental-annotation-processing

Also, could you extract this code at some point to standalone project, so I can try to launch that on my own and debug some stuff?

IvanPizhenko commented 1 year ago

It sounds pretty strange, because it'd indicate those @get:Schema annotations are not processed at all... but you said you have some of those properties in the scheme thinking Sounds like caching issue, I guess you're using clean task, but could you also disable incremental compilation in Kapt: https://kotlinlang.org/docs/kapt.html#incremental-annotation-processing

clean alone - no effect clean + kapt.incremental.apt=false - also no effect

IvanPizhenko commented 1 year ago

Also, could you extract this code at some point to standalone project, so I can try to launch that on my own and debug some stuff?

No, it is not possible. This is closed source project and I can't provide you more than example above. The only option I can suggest is to add as many debug output as you need to be able to find the reason.

dzikoysk commented 1 year ago

his is closed source project and I can't provide you more than example

I meant the project setup (only required libs from Javalin) + your exact scenario. Business context is irrelevant, you could rename those properties to xxx yyy zzz.

Let's keep adding that debug info, might be a good thing for the further issues anyway 😅

dzikoysk commented 1 year ago

This code should print any annotation on given property as long as it's present in sources. You've added getter target for annotation and you used it with prefix get:, so Kotlin should add it to sources.

IvanPizhenko commented 1 year ago

ok, so here is what I get now regarding that class (but still no changes in the output):

Note: TypeSchemaGenerator#findAllProperties | Enclosed elements of mypackage.KeyHistoryListItem: revisionType,id,keypair,copy(java.lang.String,long,mypackage.KeyListItem),equals(java.lang.Object),hashCode(),toString(),KeyHistoryListItem(java.lang.String,long,mypackage.KeyListItem),component1(),getRevisionType(),component2(),getId(),component3(),getKeypair()
Note: TypeSchemaGenerator#findExtra | Annotation: org.jetbrains.annotations.NotNull
Note: TypeSchemaGenerator#findExtra | Annotation: mypackage.extensions.io.javalin.openapi.annotations.Schema
Note: TypeSchemaGenerator#findExtra | Usage: @org.jetbrains.annotations.NotNull
Note: TypeSchemaGenerator#findExtra | Implementation:
Note: package org.jetbrains.annotations;

  @java.lang.annotation.Documented
  @java.lang.annotation.Retention(CLASS)
  @java.lang.annotation.Target({METHOD, FIELD, PARAMETER, LOCAL_VARIABLE})
  public @interface NotNull {

    java.lang.String value() default "";
  }
Note: TypeSchemaGenerator#findExtra | Usage: @mypackage.extensions.io.javalin.openapi.annotations.Schema(allowableValues={"add", "mod", "del"}, description="type of revision - add, mod (modify) or del (delete)")
Note: TypeSchemaGenerator#findExtra | Implementation:
Note: package mypackage.extensions.io.javalin.openapi.annotations;

  @kotlin.annotation.Target(allowedTargets={PROPERTY_GETTER})
  @java.lang.annotation.Retention(RUNTIME)
  @java.lang.annotation.Target({METHOD})
  @kotlin.Metadata(mv={1, 5, 1}, k=1, d1={"..."}, d2={"Lmypackage/extensions/io/javalin/openapi/annotations/Schema;", "", "allowableValues", "", "", "description", "example", "format", "pattern", "()[Ljava/lang/String;", "()Ljava/lang/String;", "shared-lib"})
  public @interface Schema {

    java.lang.String[] allowableValues() default {};

    java.lang.String description() default "";

    java.lang.String example() default "";

    java.lang.String format() default "";

    java.lang.String pattern() default "";
  }
Note: TypeSchemaGenerator#findExtra | Annotation: mypackage.extensions.io.javalin.openapi.annotations.Schema
Note: TypeSchemaGenerator#findExtra | Usage: @mypackage.extensions.io.javalin.openapi.annotations.Schema(description="row id of the particular history record")
Note: TypeSchemaGenerator#findExtra | Implementation:
Note: package mypackage.extensions.io.javalin.openapi.annotations;

  @kotlin.annotation.Target(allowedTargets={PROPERTY_GETTER})
  @java.lang.annotation.Retention(RUNTIME)
  @java.lang.annotation.Target({METHOD})
  @kotlin.Metadata(mv={1, 5, 1}, k=1, d1={"..."}, d2={"Lmypackage/extensions/io/javalin/openapi/annotations/Schema;", "", "allowableValues", "", "", "description", "example", "format", "pattern", "()[Ljava/lang/String;", "()Ljava/lang/String;", "shared-lib"})
  public @interface Schema {

    java.lang.String[] allowableValues() default {};

    java.lang.String description() default "";

    java.lang.String example() default "";

    java.lang.String format() default "";

    java.lang.String pattern() default "";
  }
Note: TypeSchemaGenerator#findExtra | Annotation: org.jetbrains.annotations.NotNull
Note: TypeSchemaGenerator#findExtra | Usage: @org.jetbrains.annotations.NotNull
Note: TypeSchemaGenerator#findExtra | Implementation:
Note: package org.jetbrains.annotations;

  @java.lang.annotation.Documented
  @java.lang.annotation.Retention(CLASS)
  @java.lang.annotation.Target({METHOD, FIELD, PARAMETER, LOCAL_VARIABLE})
  public @interface NotNull {

    java.lang.String value() default "";
  }
IvanPizhenko commented 1 year ago

Looks like it can see this annotation, but why it is not outputted?

IvanPizhenko commented 1 year ago

Also with this last update I'm getting methods in the JSON outputted in the not sorted manner again. For example, it was: get, post, put Now it is: get, put, post Please fix this. This is extremely important for simplification of changes tracking and merge conflict resolution, when that generated JSON is checked in into Git.

dzikoysk commented 1 year ago

I don't see @CustomAnnotation on public @interface Schema { 🤔 Is it declared in the sources as it was in #93?

dzikoysk commented 1 year ago

This is extremely important for simplification of changes tracking and merge conflict resolution, when that generated JSON is checked in into Git.

Methods are sorted by ordinal number of enum, not name:

routeAnnotation.methods.sortedBy { it.ordinal }

So it shouldn't change between iterations, but for sure, we can switch to name sorting, so it'll be slightly more intuitive.

IvanPizhenko commented 1 year ago

This is extremely important for simplification of changes tracking and merge conflict resolution, when that generated JSON is checked in into Git.

Methods are sorted by ordinal number of enum, not name:

routeAnnotation.methods.sortedBy { it.ordinal }

So it shouldn't change between iterations, but for sure, we can switch to name sorting, so it'll be slightly more intuitive.

Anyway, somehow that's what I get. Please switch it to sorting by name.

dzikoysk commented 1 year ago

Done, should be available in the next snapshot. Btw, the KeyHistoryListItem is defined in module with annotation processor or is it defined in standalone module that is later on linked as dependency?

IvanPizhenko commented 1 year ago

Copying right from source file:

import io.javalin.openapi.CustomAnnotation

@CustomAnnotation
@Target(AnnotationTarget.PROPERTY_GETTER)
annotation class Schema(
  val allowableValues: Array<String> = [],
  val description: String = "",
  val example: String = "",
  val format: String = "",
  val pattern: String = ""
)
dzikoysk commented 1 year ago

Done, should be available in the next snapshot. Btw, the KeyHistoryListItem is defined in module with annotation processor or is it defined in standalone module that is later on linked as dependency?

I'm asking, because @f1qwase just reported this one:

And I wonder if it might be related as it'd behave like that, so @CustomAnnotation is not added to binary file after compiling the submodule and annotation processor wouldn't know about about it, so it reports that annotation is missing just like in your output.

IvanPizhenko commented 1 year ago

KeyHistoryListItem is defined in the module with annotation processor, but Schema is defined in another module w/o annotation processor.

dzikoysk commented 1 year ago

Gotcha, that's kinda a key information. Could you try to use the latest snapshot where it should preserve annotations in binary data? Make sure it's a clean build to force compilation of each module compiled on earlier versions

IvanPizhenko commented 1 year ago

will try tomorrow

IvanPizhenko commented 1 year ago

Still no "@CustomAnnotation" on it:

@kotlin.annotation.Target(allowedTargets={PROPERTY_GETTER})
  @java.lang.annotation.Retention(RUNTIME)
  @java.lang.annotation.Target({METHOD})
  @kotlin.Metadata(mv={1, 5, 1},
....
dzikoysk commented 1 year ago

It means that sadly BINARY visibility is also not visible for annotation processor. To be fair, imo it's a bug/wrong implementation of specification (cache?) as annotation processor should be able to analyze binary annotations by design. Anyway, I've bumped all annotations to runtime, so it should work now, take a look on the latest snapshot.

IvanPizhenko commented 1 year ago

Seems like it works now.

dzikoysk commented 1 year ago

Finally!

I think we can close it now :)

IvanPizhenko commented 1 year ago

Btw, when do you plan to release final version of 5.3.0? Or it follows Javalin releases?

dzikoysk commented 1 year ago

It follows Javalin releases: